[
https://issues.apache.org/jira/browse/SLING-5721?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15296016#comment-15296016
]
Georg Henzler commented on SLING-5721:
--------------------------------------
This approach is nice as it allows to use simple classes with one-arg
constructors for e.g. {{@RequestAttribute}} as well. Just a few things that
should be considered:
* the pull request 138 also supports Lists (and arrays) like
{{List<com.mycomp.myproj.Link> links}} (derived from a String[] JCR Property) -
could you support this with your patch as well?
* Checking {{constructors.length==1}} is problematic - you don't always have
control over the classes to be used here (might be part of some other API) and
even if it can be controlled, there are many cases where more than one
constructor makes sense for other use cases (e.g. the Link class in my
particular real-life example has a clone constructor besides the one-arg String
constructor)
* Performance: {{elementClass.getConstructors()}} and
{{candidateConstructor.getGenericParameterTypes()}} will be called many times
for injectors where this approach is not applicable - not sure if there is a
real impact. Maybe adding an additional clause to {{else if (elementType
instanceof Class)}} to ensure {{java.lang.*}} classes are excluded generally
would be enough to ensure there is no problem here.
> 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.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)
