Hi > Am 06.02.2015 um 01:45 schrieb Alexander Klimetschek <aklim...@adobe.com>: > > Let me recap: > 0. adaptTo was invented for jsp scripts
Not only, but also :-) It was essentially adapted from Eclipse to get a different view of the same object. E.g. get the Node „view“ on a node-based Resource. As such the intent was always for this functionality to be available once the system is up and running. This goes from request processing scripts or servlets to services required at run time — even background processing. > 1. we are now using adaptTo() everywhere because of convenience and we don't > want to depend on a service That’s not necessarily bad. The problem is during startup and shutdown where the adaptTo dependencies cannot be validated by the system. > 2. we figure out it doesn't work in some startup cases You are safe to say, its not reliable during startup and shutdown of the system, right. > 3. we add a new mock adapter service and depend on that in our code Which I never liked in the first place :-) > > Isn't it simpler to use a proper service in the first place then? Exactly, that was my proposal as well: adaptTo is a really good convenience. For a number of use cases it is just better to have a provider service available. > > And fix those cases that are only available via AdapterManager-adaptTo() to > also have a proper service equivalent? +1 > > 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. +1 Regards Felix > > Cheers, > Alex > >> On 08.12.2014, at 03:00, Robert Munteanu <rob...@lmn.ro> wrote: >> >> Hi Bertrand, >> >> On Mon, Dec 8, 2014 at 12:37 PM, Bertrand Delacretaz >> <bdelacre...@apache.org> wrote: >>> Hi Robert, >>> >>> On Mon, Dec 8, 2014 at 10:45 AM, Robert Munteanu <romb...@apache.org> 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 >
