Child data views and maps

[Xfel]

When putting a list of DataViews to a data view, serializing it using DataFormat.HOCON and reading it again, it becomes a list of Maps. This appears to be an issue with MemoryDataView: If the set method is passed a simple map, it is stored as a DataView internally. If passing a list of maps, however (which is what the HOCON translator does), it will remain a list of maps.

[dualspiral]

@gabizou

I've been looking at this, and while any solution here shouldn't be too difficult to implement (I have a mostly working solution with tests for this specific case), I'm hesitant to commit any change to this because I think that the change to do this might be further ranging than first thought.

The problem is indeed that lists (and maps, for that matter), allow for there to be child maps that do not get converted into DataViews. This wouldn't be so bad, except that DataViews are stored exactly the same way as maps are in Configurate. Thus, serialisation and deserialisation becomes a problem - someone might have stored a list of maps, wanting them to remain as maps, rather than DataViews (though they could just use DataView#getValues(), existing code may not do so), and we have no idea how to rebuild a view exactly as it was stored.

So, the options I can think of are (in no particular order):

1. We update the type serialiser responsible to include some sort of metadata to differentiate DataViews and maps. This might be the best solution, but is a breakage that might be difficult to catch all cases with, especially with third party usage.
2. We assume that any lists containing maps will actually contain data views, but allow DataViews to otherwise contain maps - which might be breaking for some. We'd need to add logic to wrap a map in a MemoryDataView in this case (which is possible by passing a Map<DataQuery, Map<?, ?>> into the structure) - but this may cause confusion because what you put in isn't going to to match exactly with what you get out.
3. We remove support for stock maps in MemoryDataView, or indicate this for DataView in general, and convert any map into a DataView if it's being added to a DataView.

When I realised this, I was part way to implementing it the second way, but I'm not entirely convinced this is the best way to do it. I'm leaning towards option 3, where we just eliminate maps from the container, but whatever we do, it's going to be breaking, at least in terms of behaviour.

[gabizou]

We update the type serialiser responsible to include some sort of metadata to differentiate DataViews and maps. This might be the best solution, but is a breakage that might be difficult catch all cases with, esepecially with third party usage.

I elect for the first option. Much like having to stick an artificial boolean key value on NBTTagCompounds to deserialize a boolean value instead of a short value is one of those ways we'll have to tackle it.

[Xfel]

I'd like to put out a remark here: When serializing data into configuration nodes, it is very likely that this is done to allow users to edit it in the text file. In that case, it is easy to overlook those private keys.

Another thing I noticed is that if I use getMap on a key where I stored a DataView before, said DataView will automatically be converted to a map and returned. Wouldn't that be the most flexible option?

[dualspiral]

My current work on this suffixes keys with $DataView, so if you serialised a DataView that contains child DataViews, this would keep your node structure intact, but make it obvious where the data views were.

However, I've been thinking about this and there are edge cases this won't account for, namely, if you have a list of Objects in a list, some of which might be data views. In that case, I'd actually blow away the non-DataViews, so I'm beginning to think that actually, I'm going to need to create a private key/value pair. I do understand your concern about editing it, but because it's HOCON, I'll investigate adding a comment to the key suggesting it's auto generated and not to touch the node.

I'm not sure I'm getting what you mean by your second point @Xfel - yes, getMap does that and it's how the translator currently works - which is why you always get maps back instead of DataViews. It was the basis for my option 3 - which would essentially flatten nested DataViews - so you wouldn't necessarily get back what you put in. This option is trying to replicate a structure as close as possible - though there is the point where some type information could be lost, but that's another issue altogether.

[Deamon5550]

Its been on my todo list for a while to remove maps and indeed any object from being stored in a dataview without being converted to a dataview. Right now another issues is that you can put any old non-serializable object into a dataview and it will just accept it, and then brick itself when you try and actually serialize the dataview. I think we want to catch those objects when they are inserted and ensure that a dataview contains only primative types, lists, and child dataviews.

[ImMorpheus]

Moving to common for the same reason as #3126

DataView implementation has been moved so this is an implementation issue.

Note: it still needs to be tested after the configurate 3.7 PR

[zml2008]

so uh I'm pretty sure that as if my configurate 3.7 PR maps will now be translated as DataViews only, since I didn't really realize that was a distinction data views had :) at least for now translation is consistent, but it might still make sense to explicitly specify this type distinction.