[ 
https://issues.apache.org/jira/browse/SLING-5721?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15282516#comment-15282516
 ] 

Konrad Windszus commented on SLING-5721:
----------------------------------------

Regarding your questions:
bq. Currently the injector only reacts if the annotation @ValueMapObject is 
present....

This makes sense IMHO, but what speaks against putting that logic into the 
{{ValueMapInjector}} directly? Without annotation just do the regular mapping 
(from JCR types to according Java types), with annotation try to map to custom 
types as well. That way it is less code duplication and less annotations to 
remember.

bq. AbstractInjector.getValueMap() does not "unfold" a request adaptable to a 
value map (but only a resource) 

This is correct, and is intended. Two injectors use that method:
# {{ValueMapInjector}} is only supported on objects adaptable to ValueMaps 
directly (see 
http://sling.apache.org/documentation/bundles/models.html#available-injectors). 
Therefore the annotation {{ValueMapValue}} sets the via attribute to 
{{resource}} in case you are dealing with a {{request}} 
(https://github.com/apache/sling/blob/c9e59667d8f9cd698bc33a51f3e6a22e85d4a952/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/injectors/ValueMapInjector.java#L173).
# {{ResourcePathInjector}} if a name is given, which will then be looked up 
from the current resource's value map.

So a change like you propose sounds rather like an enhancement which will 
change the semantics a bit. Feel free to create a dedicated issue for that.


> Extend Sling Models to allow mapping of ValueMap values to custom types
> -----------------------------------------------------------------------
>
>                 Key: SLING-5721
>                 URL: https://issues.apache.org/jira/browse/SLING-5721
>             Project: Sling
>          Issue Type: New Feature
>          Components: Extensions
>    Affects Versions: Sling Models API 1.2.2
>            Reporter: Georg Henzler
>
> At the moment it is already possible to map child resources via 
> @ChildResource to a custom type (that itself only needs to be adaptable from 
> that child resource), but for a plain property it is not yet possible: 
> {code}
> @Inject
> com.mycomp.myproj.Link homePath; // jcr property of type String, does not 
> work currently
> @Inject
> com.mycomp.myproj.EventDate eventDate; // jcr property of type Date, does not 
> work currently
> @Inject
> java.util.Date regularDate; // jcr property of type Date, already works due 
> to standard type conversion available in ValueMap
> {code}
> For custom types, the convention would be that there needs to be a public 
> constructor with one argument with the type of the object as returned by 
> valueMap.get(name). That way a custom type can easily work with situations, 
> where the type of the JCR property is not fixed (e.g. if a property can be of 
> type String or Date, the custom type can just provide both constructors).
> {code}
> @ValueMapObject // opposed to @ValueMapValue
> com.mycomp.myproj.Link homePath; // will call constructor new 
> Link(homePathString) if possible as homePath property exists with type String 
> {code}
> Furthermore it should be possible to map JCR array types:
> {code}
> @ValueMapObject 
> List<com.mycomp.myproj.Link> links; // would expect a String[] and should put 
> the corresponding Link objects into a list (if the JCR type is String, it 
> should use that one element to fill the list with one Link
> @ValueMapObject 
> com.mycomp.myproj.Link[] links; // arrays should work in the same way
> {code}
> See github patch for a fairly straight-forward implementation the above 
> approach. An alternative approach would be to extend @ValueMapValue (I'm open 
> for both approaches, @ValueMapObject is more explicit in model classes).



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to