Hi Konrad, I finally had a chance to review the injectors and the various conversion scenarios. Here are my comments.
The only place where primitive -> wrapper array conversions are done is in the ValueMap injector. This is due to the specific implementation details of the JcrModifiableValueMap (namely that it doesn't support primitive types). IIRC, you yourself requested this feature to be added to the ValueMap injector. There is also code in there which tries to handle the case where a wrapper array (Integer[]) is passed and gets the primitive array (int[]) out of the ValueMap. IMHO, this is very specific to the ValueMap injector and would not make sense in a different context. Perhaps it could be added to the RequestAttribute injector or the SlingBindings injector, but in either case, this code absolutely belongs in the injector as the ValueMap injector needs the *original* class to get the value out of the ValueMap properly converted. However, in your original email, you conflate this with simple (i.e. non-array) primitive -> wrapper conversion. This, as you noted subsequently, always happens, but is done automatically by refection. All that happens in the ModelAdapterFactory is recognize that this can happen and then allow the JRE/JVM to do its business. I don't see the issue with this. In general, injectors shouldn't be responsible for type conversions. They should receive the original type and, optionally, use this for some purpose. But type conversion should be done either in the ModelAdapterFactory (for Adaptables) or in the JRE/JVM. I did realize while looking through your email that while simple object adaptation was supported for all injection types, only field injection supports adapting a List<Adaptable> to a list of some adapter type. This should be applied to method and constructor injection. Created https://issues.apache.org/jira/browse/SLING-3895 for this. Regards, Justin On Thu, Aug 7, 2014 at 1:30 PM, Konrad Windszus <konra...@gmx.de> wrote: > Currently in Sling Models we do have support for four different kind of type > conversions > > a) from primitive to wrapper and vice-versa (also within arrays), e.g. > Integer to int > b) from single item to one item collection (both List and Collection), e.g. > List<Integer> to Integer > c) from Adaptable to supported AdapterType > d) from subtype to super type > > That conversion logic must either be implemented in the Injector itself (if > it does evaluate the type) or the ModelAdapterFactory is taking care of that. > In the latter case the support differs for field and method: > - Method injection supports only c), d) > - Field injection supports a), b), c) and d) > > The support for these conversions differs a lot between the different > injectors. > 1) BindingsInjector, checks for type itself, does neither support a), b), c) > nor d)! > 2) ChildResource does no type checking, therefore uses the > ModelAdapterFactory for the conversions > 3) OSGiService checks for the type itself, does neither support a), b), c) > nor d)! > 4) RequestAttribute checks for type itself, does neither support a), b), c) > nor d)! > 5) ResourceResolverInjector does no type checking, therefore uses the > ModelAdapterFactory for the conversions > 6) ValueMapInjector, checks for type itself, supports a), b), c) > > > I have several proposals regarding that: > ====== > - let all injectors support all conversions > - let both method and field injection support all conversions > - consolidate the code in one place > - document the conversions which are supported. > > WDYT? > Konrad
