Hi Konrad, I don't know about those names (@InjectSlingValue specifically - what's a "Sling Value"?), but I think making @Inject work via meta-annotations make sense. That support already exists for @Source, would just need to be extended to work with @Inject as well. I just did a little refactoring to make the meta-annotation support easier to extend in the future.
Whatever you submit, please just ensure there are tests included. Regards, Justin On Tue, Mar 18, 2014 at 10:54 AM, Konrad Windszus <konra...@gmx.de> wrote: > Hi Justin, > thanks for your answer. What about if I come up with a patch for additional > annotations like > @InjectSlingValue and @InjectOsgiService > which are just another way of annotating fields/methods and combine logically > both the Inject and the Source. In case of InjectOsgiService one could even > include the optional attribute filter which would add the @Filter then. For > that I would like to use the meta annotation concept (i.e. annotation the > @InjectSlingValue with @Inject and @Source). Since Java does not come with > support for meta annotations out of the box, one could just copy the > according method from the Spring Core: > https://github.com/spring-projects/spring-framework/blob/master/spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java#L94 > and use that in the ModelAdapterFactory. > > Would you accept such a patch which would basically comprise of > a) an additional annotation per injector > b) additionally evaluating the annotations on annotations within the > ModelAdapterFactory > > Thanks, > Konrad > > On 17 Mar 2014, at 19:01, Justin Edelson <jus...@justinedelson.com> wrote: > >> Hi Konrad, >> (a) is correct and is intentional. Is there an actual situation where >> this would happen accidentially? In my experience, a new BVP is not >> added every day and has a broad impact. If you add a BVP without >> regressing your application, that's a problem into itself. >> >> (b) and (c) would only be correct if the type of the OSGi service is >> something to which your child resource or request attribute could be >> adapted, which seems highly unlikely to me. >> >> The JCR operations (property lookup and child node lookups) should be >> well-optimized by implementations. If there was an injector which >> executed a query, that would be an example of a place where the >> injector *should* require @Source or some other annotation to >> explicitly include the injector. >> >> If you want to introduce a "strict" mode which requires @Source, feel >> free to submit a patch. But I don't think this makes sense as the >> default. >> >> Regards, >> Justin >> >> On Mon, Mar 17, 2014 at 1:24 PM, Konrad Windszus <konra...@gmx.de> wrote: >>> Hi, >>> I am a little bit worried that model classes which leverage the Sling >>> Models annotations might be slow and break fast. >>> >>> If you use the annotation @Inject without @Source all injectors are asked >>> until one returns a value. >>> Since almost all injectors depend on the fieldname and cover the same >>> namespace, models can break very easily. >>> Let me give three examples for broken models: >>> >>> a) If I use in my model >>> @Inject >>> String myattribute; >>> which used to be resolved by the ValueMap injector and I just introduced a >>> new Scripting variable with the name "myattribute" >>> (https://cwiki.apache.org/confluence/display/SLING/Adding+New+Scripting+Variables), >>> my model would no longer work. >>> b) The same would apply, if I have a Sling resource with a value named >>> "service" which used to be (by coincidence) also the field name of my >>> injected OSGi service in the model. >>> c) The same for a new request attribute, whose name conflicts with the >>> field name of my injected OSGi service in the model >>> >>> Regarding the performance, I fear that the lookup in the value map from the >>> second injector might take some time (since JCR access is necessary in most >>> cases to do that). For example if I have a model class only relying on OSGi >>> services a lot of time would be wasted by asking other injectors. >>> >>> To summarize: Isn't it always advisable to use the @Source annotation to >>> prevent those kind of name clashes and performance issues? Shouldn't that >>> be explicitly stated in the documentation? >>> What is your opinion on that? >>> >>> What about making the @Source mandatory for all injectors but the ValueMap >>> injector? >>> Thanks, >>> Konrad >>> >>> >>> >
