[
https://issues.apache.org/jira/browse/SLING-5721?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15300667#comment-15300667
]
Georg Henzler commented on SLING-5721:
--------------------------------------
*Value Converter SPI in general*
Before we just light-heatedly introduce a ValueConverter SPI just for this
issue we should probably go through a discussion thread in the mailing list or
even better, create a wiki page to discuss what we expect from it - a few
things to consider:
* Hibernate has custom types (e.g.
http://what-when-how.com/hibernate/inheritance-and-custom-types-hibernate/)
that allows to map multiple DB properties to one Type (people are complaining
that JPA is still not supporting this in version 2.1). For this use case the
current SPI does not work
* "I don't expect the list/array conversion could be migrated into
ValueConverters as it is actually one level below where the SPI operates" -
does this have to be like that? I feel like this is a decision that has to be
taken carefully (I already fear the interface ValueConverter2)
* What about interfaces and class hierarchies? (currently the ValueConverter
SPI is just maintaining a Map<Type,Converter> and only converting if it is an
exact type match)
*For this particular use case*
bq. Your pull request also makes the same (bad) assumption I originally made
with respect to a single-argument constructor. What happens when the converted
type can only be constructed using a multi-argument constructor or through a
static method?
To have solution with a converter for complicated mappings where I don't have
the control over the code is nice, but is relevant for 1% of the use cases (if
even). Most of the time I do have the custom type under my control (but it must
not be limited to just one constructor, that was my main problem with your
initial ModelFactoryPatch). And for those 99% of the cases, I do not want to
write code when the framework can make the correct assumptions. (an example of
how bad things can get was J2EE, that did not have nice defaults and was not
easy to use - let's not make Sling Models to a J2EE-ish framework)
In general I think a cross-cutting solution would be nice - but I think it will
not work until Injector.getValue(...) will not pass in the type to the
injector. This is probably something to think about for Sling Models 2.0 (and
not just for a "little feature")
bq. performance
I really (still) don't want to implement com.mycomp.myproj.LinkValueConverter
and a com.mycomp.myproj.EventDateValueConverter for my use case in the
description - to allow this with the current ValueConverter SPI we would have
to add a {{DefaultValueConverter implements ValueConverter<Object,Object>}}
that runs through any value that Sling Models injects => bad performance.
Additionally to even make it possible, {{valueConverters.put(container.to,
container);}} in {{bindValueConverter()}} would also have to be changed to
support inheritance.
> 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)
