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

Georg Henzler commented on SLING-5721:
--------------------------------------

*For this particular use case*
[~justinedelson] Using value converters here is too complicated IMHO, because 
it would force a project to register a value converter for each type that 
should be allowed in models (for the two properties given in the description I 
would already have to register a com.mycomp.myproj.LinkValueConverter and a 
com.mycomp.myproj.EventDateValueConverter. This really should happen 
automatically - [PR 138|https://github.com/apache/sling/pull/138] does this 
already without extra complexity and without introducing a performance penalty.

*Value Converter SPI in general*

bq. What kind of converters would be shipped then with Sling Models OOTB

I agree with [~kwin]'s comment that we should know more than just this one use 
case (for which it even does not work in an easy way) to justify a 
ValueConverter SPI. Also there is already a lot of type conversion going on in 
the injectors (as the interface asks for the expected type in 
https://github.com/apache/sling/blob/trunk/bundles/extensions/models/api/src/main/java/org/apache/sling/models/spi/Injector.java#L52)
 - If we really go down the ValueConverter SPI path I would like to see a 
feature branch that proves that at least all this List/array conversion code in 
the injectors can be then moved centrally to a ValueConverter. 

So my suggestion is to refactor [PR 
138|https://github.com/apache/sling/pull/138] a bit to use @ValueMapValue 
(instead of a new API) - I will have time to do this next week.

> 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
>         Attachments: SLING-5721-valueconverter-spi.diff, 
> SLING-5721-valueconverter-spi2.diff, SLING-5721.patch
>
>
> 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