Agreed on being careful, which is why I brought this up.
The reasons to move over are:
It provides a well documented set of features that is comprehensive and wider
then any implementation we currently have.
https://osgi.org/specification/osgi.cmpn/7.0.0/util.converter.html
An example being :
List<String> result = valueMap.get("multiProperty",new
TypeReference<List<String>>() {});
Additionally it would provide a level of consistency that we don't currently
have. Because we don't really have "an" implementation of conversion. We leave
conversion up to the ValueMap implementations and so far that can be different.
In fact, while working on this, I discovered that the ValueMapDecorator will
return different results depending on whether you've wrapped a ValueMap or a
non-ValueMap. Because the ObjectConverter class that we created internally will
return an empty array if a conversion fails versus the ValueMap which will
return null.
*NOTE* nothing is at risk by implementing default methods on the ValueMap
because everything currently implements their own version of those methods. The
risk only comes out if an existing ValueMap implementation defaults to the
default method. Or if a new ValueMap implementation is created, which at that
point, it shouldn't be a problem.
- Jason
On Fri, Nov 16, 2018, at 11:42 AM, Robert Munteanu wrote:
> Hi Jason,
>
> On Fri, 2018-11-16 at 11:26 -0500, Jason E Bailey wrote:
> > As part of
> > https://issues.apache.org/jira/browse/SLING-8116
> >
> > Which came about in the comments for
> > https://issues.apache.org/jira/browse/SLING-7934
> >
> > I discovered that the Converter works differently then our current
> > rules for handling conversions in the ValueMap.
> >
> > Supports conversions to an array of primitive types
> > valueMap.put("prop1", new String[] { "12", "2" });
> > valueMap.get("prop1", int[].class) -> returns a populated int[]
> >
> > Supports arrays to scalar
> > valueMap.put("prop1", new String[] { "12", "2" });
> > valueMap.get("prop1", int.class) -> returns the Integer 12
> >
> > These are just the two I have identified. There is mostly likely a
> > few more subtle differences on top of this.
> > After reviewing the Converter, I believe that this would be an
> > invaluable addition to the framework, but that comes with a cost of
> > handing off the rules of conversion to a separate utility.
> >
> > If anyone has issues with this, say it now.
>
> I have nothing against changing the underlying implementation.
>
> But we have to be _very_ careful with any kind of behaviour change. As
> a general rule we aim to never break backwards compatibility unless
> there is a very good reason for it.
>
> What would be the reasons for moving to the converter from our own
> implementation?
>
> Thanks,
>
> Robert
>
