I had a closer look in the source code. First of all the Configuration Admin spec doesn't limit in any way the types supported in the Dictionary, so
a) primitive arrays b) object arrays c) collection of objects d) primitives and e) objects are all supported. IMHO write back should support all of those types (no matter where the value originated from, i.e. from a metatype set via web console, set via Configuration Admin API directly, ....) Also the round trip via write back should not modify those types. Currently the write back properly supports all of those except for - multitype collections - nested arrays - nested collections So I don't agree with > The point that we for the writeback don't > convert is hence, technically, a bug. I would rather say the opposite: Write back and in general the .config file format supports almost all types (despite the documentation) except for the edge cases listed above. So I see actually only two issues here: 1) Document how to deploy object arrays via .config files 2) Create issue for the not-yet supported edge cases of the write back (or rather the underlying Felix Configuration Handler), but I don't think it is worth actually fixing those, as those are really edge cases Does that make sense? Konrad > On 5. Mar 2019, at 13:58, Karl Pauls <karlpa...@gmail.com> wrote: > > Again, there are several angles We have something documented in > sling. That is only talking about a capital "I" for object array and > points to the felix config impl that will take an object array and > serialize with an "I". The point that we for the writeback don't > convert is hence, technically, a bug. > > We can desite that we want to change what we support and that would be > fine by me. All I'm pointing out is that even if we do that, we still > have something that doesn't seem sound if all we do is adding > lowercases to the allowed types.We might not care or we might try to > come up with a new definition (and add some backwards compatibility > around it). > > That said, it seems like yes, we might want to consider to say: > allowed types are upper and lower case array of primitives which will > both result in a primitive array but the normative version is the > lower case and we will backward compat exiting configs with object > arrays to the new way (which would match what we have right now for > the most part - except that we only do the bc conversion if the > webconsole has been used). However, alternatively we could just fix > the writeback to convert to none primitive array. I don't feel > strongly about it - just trying to point out what I see. > > Wrt. the metatype spec, yes, it seems like it should be primitive or > list of things - not sure I would add that to the supported types > (seems like a head-age nobody is asking for - but YMMV), afaik, we > don't really claim it has something todo with the metatype in the > documentation. > > regards, > > Karl > > On Tue, Mar 5, 2019 at 12:35 PM Konrad Windszus <konra...@gmx.de> wrote: >> >> Slight correction >> a Collection of Objects is supposed to be serialized e.g. as I(1,2) >> >> I am wondering under which circumstances an array of Objects, e.g. I[1,2] is >> being emitted? >> >> >>> On 5. Mar 2019, at 12:28, Konrad Windszus <konra...@gmx.de> wrote: >>> >>> Well, if I read the OSGi metatype spec correctly >>> (https://osgi.org/specification/osgi.cmpn/7.0.0/service.metatype.html#org.osgi.service.metatype.annotations.AttributeType) >>> it is supposed to be a primitive array (serialized e.g. as i[1,2]) for a >>> positive cardinality and a Collection of Objects (serialized e.g. as >>> I[1,2]) for a negative cardinality. >>> AFAICT the write back behaves accordingly to that spec, i.e. it will only >>> emit primitives for types having a positive cardinality and otherwise an >>> array of proper classes. >>> >>> Is that a wrong understanding? >>> Konrad >>> >>>> On 5. Mar 2019, at 09:21, Karl Pauls <karlpa...@gmail.com> wrote: >>>> >>>> I think the writeback is a bug as it stands right now. AFAICS, the >>>> webconsole always converts arrays to primitive arrays and the >>>> writeback just serialises what is there as is. Given that the >>>> documentation only mentions a capital "I", the writeback should be >>>> converting primitive arrays before writing back. >>>> >>>> I don't mind changing the documentation to allow lower case "i" as >>>> well - assuming that really already works in all places - however, it >>>> will cause some inconsistencies because if you then put a config in >>>> the repository with a capital "I" it will first create a config with a >>>> none primitive array. Next, if you modify it using the webconsole it >>>> will turn into a primitive array which will be written back as lower >>>> case "i". Not sure that is a real problem but it is somewhat confusing >>>> as the config in the repository now has a lower case "i". >>>> >>>> In other words, I think we are really talking about two things - a bug >>>> in the writeback and a potential broadening of the supported types. >>>> >>>> regards, >>>> >>>> Karl >>>> >>>> On Mon, Mar 4, 2019 at 7:06 PM Carsten Ziegeler <cziege...@apache.org> >>>> wrote: >>>>> >>>>> +1 its always good to document reality...unless its a bug of course :) >>>>> >>>>> Carsten >>>>> >>>>> >>>>> Georg Henzler wrote >>>>>> If it works I'm +1 to document it... >>>>>> >>>>>> On 2019-02-27 14:15, Konrad Windszus wrote: >>>>>>> Despite the documentation at >>>>>>> https://sling.apache.org/documentation/bundles/configuration-installer-factory.html#configuration-files-config >>>>>>> >>>>>>> <https://github.com/apache/felix/blob/trunk/configadmin/src/main/java/org/apache/felix/cm/file/ConfigurationHandler.java> >>>>>>> >>>>>>> also primitive types are supported (with lower-case type information >>>>>>> characters, compare with >>>>>>> https://github.com/apache/felix/blob/ad2aabb04c754f86c6417c437256500dd61a4ffb/configadmin/src/main/java/org/apache/felix/cm/file/ConfigurationHandler.java#L92). >>>>>>> >>>>>>> Is there a reason why this is not documented? >>>>>>> AFAIK the lowercase is also used for the JCR Installer write back, >>>>>>> e.g. in case the metatype defines something like "<AD id="levels" >>>>>>> type="Integer" cardinality="2147483647" name="Absolute Levels" >>>>>>> description="List of absolute parent levels. Example: Absolute parent >>>>>>> level 1 of '/foo/bar/test' is '/foo/bar'."/>" >>>>>>> >>>>>>> In case the cardinality is a large positive integer it is always >>>>>>> supposed to be an array of primitives (compare with >>>>>>> https://osgi.org/specification/osgi.cmpn/7.0.0/service.metatype.html#org.osgi.service.metatype.annotations.AttributeType). >>>>>>> >>>>>>> Does anything speak against extending that documentation in that regard? >>>>>>> >>>>>>> Thanks, >>>>>>> Konrad >>>>> -- >>>>> Carsten Ziegeler >>>>> Adobe Research Switzerland >>>>> cziege...@apache.org >>>> >>>> >>>> >>>> -- >>>> Karl Pauls >>>> karlpa...@gmail.com >>> >> > > > -- > Karl Pauls > karlpa...@gmail.com
