You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by Carsten Ziegeler <cz...@apache.org> on 2018/11/18 10:05:09 UTC
Re: [DISCUSS] [POTENTIALLY-BREAKING] implementing the Osgi Converter
As you said, we don't have a well documented set of default conversion
rules at the moment, so we can argue that today it works because of pure
luck.
With the converter we have a well defined set of rules and - once the
default method is used by implementations - we have the same conversion
across all implementations, which I think is a huge benefit.
The obvious question is, do we have any chance of finding out if we
break someone? I would assume if all of our integration tests still pass
we're on a pretty good road. (of course this would require that we
change the implementations to use the new default method)
Regards
Carsten
Am 16.11.2018 um 19:08 schrieb Jason E Bailey:
> 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
>>
--
Carsten Ziegeler
Adobe Research Switzerland
cziegeler@apache.org
Re: [DISCUSS] [POTENTIALLY-BREAKING] implementing the Osgi Converter
Posted by Jason E Bailey <je...@apache.org>.
I've implemented the initial modifications and the pull request is here
https://github.com/apache/sling-org-apache-sling-api/pull/8
Right now, I've centralized the custom converters. At some point I may implement a Converter service which would allow you to request a Converter that has been per-configured. I'm imagining something along the lines of how the adaptTo functionality has a view in the System console that provides insight into all the possible adaptions. It would be useful to show an end user all of the possible conversions they could use. Course that might not be practical.
- Jason
On Mon, Nov 19, 2018, at 6:58 AM, David Bosschaert wrote:
> Hi all,
>
> On Mon, 19 Nov 2018 at 07:22, Carsten Ziegeler <cz...@apache.org> wrote:
>
> > In addition to what has already been said, the converter might differ in
> > three ways:
> >
> > a) additional rules
> > If the converter supports more conversions than what we have today,
> > that's not an issue
> > b) missing rules
> > If the converter does not support a conversion that we have today, we
> > can simply add the missing rule
> > c) different rule
> > If the converter converts the same object into a target class
> > differently from what we do today, than this might be an issue. However,
> > I guess this can only happen for things like date conversion. In these
> > case we can simply add our own rule on top of the converter
> >
> >
> Yes, the OSGi Converter supports custom rules which allow tweaking its
> behaviour, it's documented here:
> https://osgi.org/specification/osgi.cmpn/7.0.0/util.converter.html#util.converter-customizing.converters
>
> Best regards,
>
> David
Re: [DISCUSS] [POTENTIALLY-BREAKING] implementing the Osgi Converter
Posted by Jason E Bailey <je...@apache.org>.
Agreed,
My initial plan is to create a util class to encompass additional rules that we assign so that there is one source of truth. I'll document any test cases that have to be modified and changes that result from this so that I can get as many eyes on it as possible and I will fight every natural instinct I have and go slowly so that it's not a surprise to anyone.
- Jason
On Mon, Nov 19, 2018, at 6:58 AM, David Bosschaert wrote:
> Hi all,
>
> On Mon, 19 Nov 2018 at 07:22, Carsten Ziegeler <cz...@apache.org> wrote:
>> In addition to what has already been said, the converter might differ in
>> three ways:
>>
>> a) additional rules
>> If the converter supports more conversions than what we have today,
>> that's not an issue
>> b) missing rules
>> If the converter does not support a conversion that we have today, we
>> can simply add the missing rule
>> c) different rule
>> If the converter converts the same object into a target class
>> differently from what we do today, than this might be an issue. However,
>> I guess this can only happen for things like date conversion. In these
>> case we can simply add our own rule on top of the converter
>
> Yes, the OSGi Converter supports custom rules which allow tweaking its behaviour, it's documented here: https://osgi.org/specification/osgi.cmpn/7.0.0/util.converter.html#util.converter-customizing.converters
>
> Best regards,
>
> David
Re: [DISCUSS] [POTENTIALLY-BREAKING] implementing the Osgi Converter
Posted by David Bosschaert <da...@gmail.com>.
Hi all,
On Mon, 19 Nov 2018 at 07:22, Carsten Ziegeler <cz...@apache.org> wrote:
> In addition to what has already been said, the converter might differ in
> three ways:
>
> a) additional rules
> If the converter supports more conversions than what we have today,
> that's not an issue
> b) missing rules
> If the converter does not support a conversion that we have today, we
> can simply add the missing rule
> c) different rule
> If the converter converts the same object into a target class
> differently from what we do today, than this might be an issue. However,
> I guess this can only happen for things like date conversion. In these
> case we can simply add our own rule on top of the converter
>
>
Yes, the OSGi Converter supports custom rules which allow tweaking its
behaviour, it's documented here:
https://osgi.org/specification/osgi.cmpn/7.0.0/util.converter.html#util.converter-customizing.converters
Best regards,
David
Re: [DISCUSS] [POTENTIALLY-BREAKING] implementing the Osgi Converter
Posted by Carsten Ziegeler <cz...@apache.org>.
In addition to what has already been said, the converter might differ in
three ways:
a) additional rules
If the converter supports more conversions than what we have today,
that's not an issue
b) missing rules
If the converter does not support a conversion that we have today, we
can simply add the missing rule
c) different rule
If the converter converts the same object into a target class
differently from what we do today, than this might be an issue. However,
I guess this can only happen for things like date conversion. In these
case we can simply add our own rule on top of the converter
Regards
Carsten
Am 18.11.2018 um 11:05 schrieb Carsten Ziegeler:
> As you said, we don't have a well documented set of default conversion
> rules at the moment, so we can argue that today it works because of pure
> luck.
>
> With the converter we have a well defined set of rules and - once the
> default method is used by implementations - we have the same conversion
> across all implementations, which I think is a huge benefit.
>
> The obvious question is, do we have any chance of finding out if we
> break someone? I would assume if all of our integration tests still pass
> we're on a pretty good road. (of course this would require that we
> change the implementations to use the new default method)
>
> Regards
> Carsten
>
> Am 16.11.2018 um 19:08 schrieb Jason E Bailey:
>> 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
>>>
>
--
Carsten Ziegeler
Adobe Research Switzerland
cziegeler@apache.org