Let me recap:
0. adaptTo was invented for jsp scripts
1. we are now using adaptTo() everywhere because of convenience and we don't 
want to depend on a service
2. we figure out it doesn't work in some startup cases
3. we add a new mock adapter service and depend on that in our code

Isn't it simpler to use a proper service in the first place then?

And fix those cases that are only available via AdapterManager-adaptTo() to 
also have a proper service equivalent?

Note that you can have this case within one bundle already, i.e. if you have 
code in service A that depends on adaptTo() of an AdapterManager B in the same 
bundle, with A being started before B. The rule from that for me was to never 
use adaptTo() for the stuff from within a bundle - which was just lazy and can 
be done with better OO design anyway.

Cheers,
Alex

> On 08.12.2014, at 03:00, Robert Munteanu <[email protected]> wrote:
> 
> Hi Bertrand,
> 
> On Mon, Dec 8, 2014 at 12:37 PM, Bertrand Delacretaz
> <[email protected]> wrote:
>> Hi Robert,
>> 
>> On Mon, Dec 8, 2014 at 10:45 AM, Robert Munteanu <[email protected]> wrote:
>>> ...I'd
>>> like to commit this today or tomorrow so any feedback is welcome....
>> 
>> Looks good to me except the SLING-4217 patch is missing the
>> Adaption*.java classes.
> 
> I continued the latest patches at [1], makes it easier to see what
> changes, including between iterations of the same patch. This includes
> the missing classes.
> 
>> 
>> Also, about the "TODO - is it possible to remove more than one factory
>> per service reference" comment - I'm not sure what the impact of this
>> is, is it just about being able to do something in an easier way, or
>> is there a risk of leaving unwanted things registered?
> 
> There's a risk of leaving an unwanted 'Adaption' service registered,
> which would incorrectly signal that the AdapterFactory is still
> registered. Nonetheless, the AdapterManager would still function
> correctly, the Adaption service is just used for signalling consumers
> of adaptTo when a certain adaption is available.
> 
> Note that in the latest iteration I simply log an ERROR in such a
> scenario to simplify the code ( see [2] ).
> 
> [1]: https://reviews.apache.org/r/28751/diff/#
> [2]: https://reviews.apache.org/r/28751/diff/3/#chunk3.22
> 
> 
> 
> -- 
> Sent from my (old) computer

Reply via email to