Sounds good, I think we should also improve the decorator to check whether it is decorating a ValueMap (as opposed to a map) and then potentially use the methods of the wrapped ValueMap for conversion as well
Carsten Stefan Seifert wrote > thinking about the arguments from different directions i came up with a more > simple proposal which is perhaps the best compromise: > > - we do not touch the conversion implementation in the jcr.resource > implementations which has a lot of good conversion rules, but also some > questionable we do not want to apply all. > - we do not add and conversion support in the Sling API > - we do not use osgi conversion service > - instead we just add some more conversion rules for Byte, Short, BigInteger, > Calendar, Date which are currently missing in internal implementation of the > ValueMapDecorator, and apply some optimiziations to the implementation. this > is outlined in SLING-6420. > > with this we have no code reuse, but code reuse would lead to either > - potential overengineering adding new services interfaces and > implementations for conversion > - having to deal with the questionable rules from jcr.resource in the generic > impl as well > - introducing dependencies to the Sling API > - bloating things > > if no one objects I would follow this way in SLING-6420, which would also > help solve the initial mock problem SLING-6416. > > Stefan > > >> -----Original Message----- >> From: Stefan Seifert [mailto:sseif...@pro-vision.de] >> Sent: Monday, December 19, 2016 12:41 PM >> To: dev@sling.apache.org >> Subject: [api] type conversion in ValueMapDecorator >> >> SLING-6416 revealed a problem in the type conversion logic of the >> ValueMapDecorator [1] of the Sling API not supporting BigDecimal >> conversions. >> >> as the TODO indicates (present there for nearly 8 years) the current >> conversion logic is not very smart (and not very efficient), and supports >> less conversions than the JCR value map implementation. in jcr.resource a >> set of internal converters takes care of the conversion (NumberConverter, >> CalendarConverter, BooleanConverter etc. [2]). >> >> the contract of the ValueMap interface does not define which conversions >> should be supported exactly, but one might expect that a map enhanced with >> ValueMapDecorator should behave at least for the conversion rules the same >> way as the JCR value map. >> >> in this case we should move these converter implementation to the sling API >> and use them in both ValueMapDecorator and jcr.resource implementation. >> >> WDYT? >> >> stefan >> >> [1] >> https://github.com/apache/sling/blob/trunk/bundles/api/src/main/java/org/ap >> ache/sling/api/wrappers/ValueMapDecorator.java#L64 >> [2] >> https://github.com/apache/sling/tree/trunk/bundles/jcr/resource/src/main/ja >> va/org/apache/sling/jcr/resource/internal/helper >> >> > > > -- Carsten Ziegeler Adobe Research Switzerland cziege...@apache.org
