I see that most of the suggestions have been implemented and the feedback was mainly positive. What are the next steps here? Can you create a branch containing that piece of code and in parallel create a JIRA ticket for that contribution? Thanks Konrad
On 07 Jan 2014, at 20:55, Justin Edelson <[email protected]> wrote: > Hi Georg- > Thanks for the suggestion. I think I found a way to handle this with > PhantomReferences. > > Justin > > On Tue, Jan 7, 2014 at 11:47 AM, Georg Henzler > <[email protected]> wrote: >> Hi Justin, >> >> I have a solution for the unget problem - for a recent project I have >> created a class OsgiServiceProxy (also used for the howm-grown injection >> mechanism) that allows to use a service without having access to the request >> (using java.lang.reflect.Proxy, see [1],[2]). The proxy gets/ungets the >> service for every method call (not ideal from a performance perspective, but >> it is surprisingly fast and we didn't have a significant performance penalty >> here). It could probably be rewritten to use a ServiceTracker (to be more >> efficient), but it works as is and the beauty in the solution is, that you >> can use/store the service like a service reference because the service proxy >> never holds on to the actual service between method calls. >> >> Regards >> Georg >> >> [1] Usage >> MyService myService = OsgiServiceProxy.create(abstractComponent.getClass(), >> MyService.class); >> >> >> [2] Impl >> public final class OsgiServiceProxy implements InvocationHandler { >> >> private static final Logger LOG = >> LoggerFactory.getLogger(OsgiServiceProxy.class); >> >> /** >> * Creates a proxy for a OSGI service. >> * >> * @param hostClass The class that uses the service >> * @param osgiServiceClass the service class >> * @return proxy to the service >> */ >> public static <T> T create(final Class<?> hostClass, final Class<T> >> osgiServiceClass) { >> OsgiServiceProxy osgiServiceProxy = new OsgiServiceProxy(hostClass, >> osgiServiceClass); >> Object service = Proxy.newProxyInstance(hostClass.getClassLoader(), >> new Class[] { osgiServiceClass }, osgiServiceProxy); >> @SuppressWarnings("unchecked") >> T retVal = (T) service; >> return retVal; >> } >> >> private Class<?> hostClass; >> private Class<?> osgiServiceClass; >> >> private OsgiServiceProxy(final Class<?> hostClass, final Class<?> >> osgiServiceClass) { >> this.hostClass = hostClass; >> this.osgiServiceClass = osgiServiceClass; >> } >> >> @Override >> public Object invoke(final Object proxyObj, final Method method, final >> Object[] methodArgs) throws Throwable { >> LOG.debug("OsgiServiceProxy: Invocation on service {} method {}", >> this.osgiServiceClass, method); >> BundleContext componentTypeBundleContext = >> FrameworkUtil.getBundle(this.hostClass).getBundleContext(); >> ServiceReference serviceReference = >> componentTypeBundleContext.getServiceReference(this.osgiServiceClass.getName()); >> >> if (serviceReference == null) { >> throw new IllegalStateException("Cannot call " + >> method.getName() + " as service " + osgiServiceClass + " is not available"); >> } >> >> LOG.debug("OsgiServiceProxy: Retrieved service ref for {} ", >> this.osgiServiceClass); >> Object result = null; >> try { >> Object service = >> componentTypeBundleContext.getService(serviceReference); >> result = method.invoke(service, methodArgs); >> } finally { >> LOG.debug("OsgiServiceProxy: Ungetting service ref for {}", >> this.osgiServiceClass); >> componentTypeBundleContext.ungetService(serviceReference); >> } >> return result; >> >> } >> >> } >> >> >> >> >> >>> That's unfortunately correct. What I'm in the process of doing is >>> changing the OSGiServiceInjector so that if the adaptable is a >>> request, it uses SlingScriptHelper intead of the bundle context so at >>> least the references are released if you use a request as the >>> adaptable. But I don't see a good way to handle this otherwise. Any >>> ideas? >>> >>> Regards, >>> Justin >>> >>>> >>>> Cheers, >>>> Konrad >>>> >>>> >>>> On 24 Dec 2013, at 22:16, Justin Edelson <[email protected]> >>>> wrote: >>>> >>>>> Thanks everyone for your feedback. I've updated both the wiki and >>>>> implementation to include support for: >>>>> * declaring an injection as being provided specifically by a >>>>> particular injector, using the @Source annotation (as well as adding >>>>> this annotation to @Filter) >>>>> * composition support (without new annotations) >>>>> * switched package to .annotations from .api >>>>> * using BundleTracker rather than BundleListener >>>>> * a new injector type for child resources >>>>> >>>>> I also tried to add some reference information to the wiki. >>>>> >>>>> I think this captured all of the feedback received so far. Thanks again. >>>>> >>>>> Justin >>>>> >>>>> On Sat, Dec 21, 2013 at 1:47 AM, Georg Henzler >>>>> <[email protected]> wrote: >>>>>> >>>>>> Hi all, >>>>>> >>>>>> first of all I have to say that I'm really happy to see that effort is >>>>>> being >>>>>> made to come up with a annotation based model binding mechansim. We've >>>>>> been >>>>>> using our own-grown for a while, but a standard is better! :) >>>>>> >>>>>> I also think it would be useful to inject "sub models". Using only the >>>>>> @Inject annotation is ambiguous though, as the class could be either an >>>>>> OSGi >>>>>> Service or a sub model. A solution for this could be to use an >>>>>> annotation >>>>>> like @SubModel and make OSGi services the default. >>>>>> >>>>>> @Inject @SubModel >>>>>> private ImageModel image; // using the field name as context path for >>>>>> the >>>>>> sub model as default, in this case ./image >>>>>> >>>>>> @Inject @SubModel(path="image2") // path explicitly provided, here >>>>>> ./image2 >>>>>> private ImageModel anotherImage; >>>>>> >>>>>> @Inject // assumed to be an OSGi service for non-primitive types >>>>>> private SomeOtherClass myService; >>>>>> >>>>>> >>>>>> -Georg >>>>>> >>>>>> >>>>>> Am 20.12.2013 15:19, schrieb Justin Edelson: >>>>>> >>>>>>> Hi Konrad, >>>>>>> Thanks for the clarification. >>>>>>> >>>>>>> It seems straightforward enough to be able to adapt the injected value >>>>>>> if it is not assignable to the field's class. >>>>>>> >>>>>>> @Inject >>>>>>> private ImageModel image >>>>>>> >>>>>>> image would be a Resource object natively which could then be adapted >>>>>>> to the ImageModel class. >>>>>>> >>>>>>> Justin >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Fri, Dec 20, 2013 at 8:08 AM, Konrad Windszus <[email protected]> >>>>>>> wrote: >>>>>>>> >>>>>>>> >>>>>>>> Hi Justin, >>>>>>>> let me give a concrete example where switching resource nodes is >>>>>>>> actually >>>>>>>> useful: I do have a composition model of two image models (i.e. the >>>>>>>> same >>>>>>>> class). Obviously they cannot share the same node, as both models are >>>>>>>> referring to the same value names. Therefore an approach similar to >>>>>>>> <sling:include path="..." resourceType=".."> would be very useful on >>>>>>>> the >>>>>>>> model side. I admit that in the case of models it is a little bit >>>>>>>> different, >>>>>>>> because we are not doing real request dispatching here. Rather I want >>>>>>>> to >>>>>>>> have a way to tell the factory (or only the ValueMap injector) to act >>>>>>>> on a >>>>>>>> certain sub node of the request resource instead of the request >>>>>>>> resource >>>>>>>> itself. That way we could tell the instance1 of the image model to >>>>>>>> act on >>>>>>>> subnode 'image1" and the instance2 of that model to act on subnode >>>>>>>> "image2". >>>>>>>> >>>>>>>> Regards, >>>>>>>> Konrad >>>>>>>> >>>>>>>> Am 20.12.2013 um 13:41 schrieb Justin Edelson >>>>>>>> <[email protected]>: >>>>>>>> >>>>>>>>> Hi Konrad, >>>>>>>>> This makes sense, except for the part about "switch the current >>>>>>>>> resource"? What do you mean by this? It seems we should be treating >>>>>>>>> the request resource (which is what I think of as the "current" >>>>>>>>> resource) as immutable. >>>>>>>>> >>>>>>>>> Regards, >>>>>>>>> Justin >>>>>>>>> >>>>>>>>> On Fri, Dec 20, 2013 at 5:31 AM, Konrad Windszus <[email protected]> >>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Hi Justin, >>>>>>>>>> another useful feature just came to my mind (in fact we are using >>>>>>>>>> it in >>>>>>>>>> our own annotation framework) which is composition. Would it be >>>>>>>>>> much effort >>>>>>>>>> to allow injecting one model into another? >>>>>>>>>> We do have the following usecase for that (although this is CQ, I >>>>>>>>>> guess >>>>>>>>>> there is a similar usecase in Sling only): >>>>>>>>>> >>>>>>>>>> You have a model for an image with title, alternative text. >>>>>>>>>> You have a model for multiline text fields and alignment. >>>>>>>>>> There exist resourceTypes for each of the models as well as a >>>>>>>>>> composite >>>>>>>>>> resourceType multilineImage. >>>>>>>>>> For the composite resourceType I would like to reuse the existing >>>>>>>>>> models, but I cannot split up the view (i.e. the JSPs and work with >>>>>>>>>> sling:include), because the html is somehow intertwined. >>>>>>>>>> Therefore I would define another composite model exposing the >>>>>>>>>> models >>>>>>>>>> for both image and multiline and use that composite model in my >>>>>>>>>> JSP. >>>>>>>>>> >>>>>>>>>> It would be great if for the injection of other models it would be >>>>>>>>>> possible to switch the current resource as well (i.e. descent into >>>>>>>>>> subnode >>>>>>>>>> image). >>>>>>>>>> That do you think about that? >>>>>>>>>> >>>>>>>>>> Thanks in advance, >>>>>>>>>> Konrad >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Am 19.12.2013 um 18:07 schrieb Justin Edelson >>>>>>>>>> <[email protected]>: >>>>>>>>>> >>>>>>>>>>> Hi, >>>>>>>>>>> I've published a page to the wiki about a concept I've been >>>>>>>>>>> working on >>>>>>>>>>> to consolidate the various appproaches I have seen in the wild to >>>>>>>>>>> model object creation. I'm calling this YAMF for now, although >>>>>>>>>>> ideally >>>>>>>>>>> we'll just call it Sling Models :) >>>>>>>>>>> >>>>>>>>>>> Without repeating the whole contents of the wiki page, at a high >>>>>>>>>>> level, this is a purely annotation driven approach supporting both >>>>>>>>>>> classes and interfaces. Your model class simply needs to declare >>>>>>>>>>> from >>>>>>>>>>> which other classes it can be adapted: >>>>>>>>>>> >>>>>>>>>>> @Model(adaptables=Resource.class) >>>>>>>>>>> >>>>>>>>>>> And then annotate the fields (for classes) and methods (for >>>>>>>>>>> interfaces) which need injection: >>>>>>>>>>> >>>>>>>>>>> @Inject >>>>>>>>>>> private String propertyName; >>>>>>>>>>> >>>>>>>>>>> You can inject properties, OSGi services, request attributes, and >>>>>>>>>>> entries from SlingBindings. >>>>>>>>>>> >>>>>>>>>>> New injector types can be created through an SPI. >>>>>>>>>>> >>>>>>>>>>> Additional annotations are supported for special cases: >>>>>>>>>>> >>>>>>>>>>> @Optional - mark a field/method as optional. >>>>>>>>>>> @Filter - provide a filter (i.e. for OSGi services) >>>>>>>>>>> @Named - specify a name (other than the default field/method name) >>>>>>>>>>> to >>>>>>>>>>> use for the inejction lookup. >>>>>>>>>>> >>>>>>>>>>> More detail can be found here: >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> https://cwiki.apache.org/confluence/display/SLING/YAMF+-+Yet+Another+Model+Factory >>>>>>>>>>> >>>>>>>>>>> The working code is up in my whiteboard: >>>>>>>>>>> >>>>>>>>>>> https://svn.apache.org/repos/asf/sling/whiteboard/justin/yamf/ >>>>>>>>>>> >>>>>>>>>>> Look forward to your feedback. >>>>>>>>>>> >>>>>>>>>>> Regards, >>>>>>>>>>> Justin >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>> >>>>>> >>>> >>
