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 <[email protected]> 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 <[email protected]> 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 >> >> >>
