Hi Granted, consolidation probably makes sense, but there are caveats:
* Bloat: Please consider to not bloat the conversion functionality and carefully consider which conversion really makes sense and which is just nice-to-have-because-JCR-has-it. * API: I suggest to break this out of the Sling API bundle into a separate bundle. One of the problems I see we have with the API bundle is that is becoming more and more a collection of API with independent lifecycle. Particularly the Resource API has its own life warranting its own bundle. But this type conversion is another one. (And yes I know we also have a packaging problem in the API bundle, which might have to be fixed) Regards Felix > Am 19.12.2016 um 12:59 schrieb Julian Sedding <jsedd...@gmail.com>: > > +1 It sounds sensible to move the logic to Sling API and re-use it in > the JCR value map. > > Regards > Julian > > On Mon, Dec 19, 2016 at 12:40 PM, Stefan Seifert <sseif...@pro-vision.de> > wrote: >> 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/apache/sling/api/wrappers/ValueMapDecorator.java#L64 >> [2] >> https://github.com/apache/sling/tree/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/helper >> >>
