Libelektra: Toml/Directoryvalue problems

[bauhaus93]

I've got a problem with the toml plugin (#3292) when I'm using it with the directoryvalue plugin and am not sure how to resolve it.

I use the tomltype metakey in my plugin to represent the different TOML structures, like simple tables, table arrays or inline tables. However, the directoryvalue plugin moves this metakey from the original key to the newly created __dirdata key, if the table key is not a leaf.
As a result, the original key loses it's table attribution, writing it as a plain key/value assignment (to an empty value).
The new __dirdata key however will be written as a table (which can't represent any value in a TOML file, so it's value gets lost).

Steps to reproduce

# Mount test file and use the directoryvalue plugin
sudo kdb mount test.toml user/test toml type directoryvalue

# Add a simple table
kdb meta-set 'user/test/table' 'tomltype' 'simpletable'

# Output matches expected output
cat `kdb file user/test`
#> [table]

# Add a field under the table, making 'table' non-leaf
kdb set 'user/test/table/field' '1'

# Output is not what is expected
cat `kdb file user/test`
#> table = "@@EMPTY"
#> table.field = 1
#> [table.___dirdata]

kdb rm -r user/test
sudo kdb umount user/test

Expected output

I expected the second file ouput to be of the form:

[table]
field = 1
__dirdata = "@@EMPTY"

Solution ideas

I thought of two possible solutions, each with it's problems:

• Special handling of the tomltype metakey in the directoryvalue plugin (But is it clean to leak this plugin-specific metadata into another plugin?). Maybe this can be more generalized, like, per default, the directoryvalue plugin considers metakeys to be bound to their values. But there are also some special metakeys (liketomltype) which should be assigned to their keys instead.
• Remove the tomltype metakey completely and derive tables where applicable.
  This would however have a negative impact on the file format preservation of the toml plugin. Additionally, we would have to stick to one kind of representation on writing the file, when ambiguities occur.
  For example, when having 2+ keys under a common keyspace: Write either a simple table or an inline table for them.
  (The same would apply to arrays with multiple subkeys per element: Either write it as a table array or as an array of inline tables).

[mpranj]

Thank you for reporting this. Maybe I am missing some point but why are you using the directoryvalue plugin at all? Correct me if I am mistaken but directoryvalue is intended for storage plugins which are only able to store values only in leafs. I would assume that directoryvalue would cause all kinds of problems for other storage plugins as well. Do we need to support this at all?

[bauhaus93]

One case, where the toml plugin needs directoryvalue would be for arrays, where the array root key also has a value assigned to it.

On a second thought, kind of the same problem happens, if a value gets assigned to a key which indicates a TOML specific structure (holds a tomltype metakey). For this keys, no value can be written normally. This problem however, would also not be solved by the directoryvalue plugin, as far as I understand.

Yeah, I'm not sure if this is needed to be supported at all.

[markus2330]

I agree that this is a minor and definitely-not-urgent problem.

@sanssecours do you maybe want to give input here?

Remove the tomltype metakey completely

This might be interesting also beyond the scope of this issue. Maybe it is possible to represent a table with null keys, so something like:

kdb set 'user/tests/storage/table/a' '0'
kdb set 'user/tests/storage/table/b' '1'

we get

table.a = 0
table.b = 1

and after

kdb set 'user/tests/storage/table'

we get

[table]
a = 0
b = 1

[sanssecours]

@sanssecours do you maybe want to give input here?

All ideas presented here – the two from Jakob (bauhaus93) and the one from Markus (markus2330) – sound reasonable to me. In general I do not like plugin specific metadata (that only applies to a single plugin) very much, although I can understand the appeal of it. From a simplicity standpoint I think I would prefer the last suggestion by Jakob or the one by Markus (if it works).

[kodebach]

Maybe my idea from #3256 should also be considered. I think this would also solve the problem.

I don't think null keys work. Not sure about TOML, but e.g. JSON has an explicit null value, so what happens with

table = null
table.a = 0
table.b = 1

(assuming either TOML has null or its another format that is TOML+null)

Once that is read into a KeySet it would be indistinguishable from

[table]
a = 0
b = 1

I also agree with sanssecours that plugin specific metadata is not nice (if we don't need it). But if the directoryvalue plugin would use metadata instead of ___dirdata it wouldn't be plugin specific metadata, since it is used to communicate to other plugins.

---

Also the obvious solution, (and IMHO the huge problem with directoryvalue) toml needs to check every key for a direct child with basename __dirdata and, if it exists take the metadata from there. That however, makes the directoryvalue plugin very useless and is why I suggested the library with elektraIsDirectoryKeyWithValue. Then toml itself could deal with non-leaf keys with value any way it sees fit. (After all a proper storage plugin should be able to deal with all possible KeySets) The directoryvalue plugin could still be kept around for other plugins that don't have the problems that toml has.

[mpranj]

a proper storage plugin should be able to deal with all possible KeySet s

We can have plugins that do not support all KeySets, but the default storage plugin should definitely handle all cases.

@bauhaus93 will the toml plugin eventually be able to handle metadata? Afaik it is currently unsupported.

When I wrote mmapstorage I had to support metadata, otherwise it would have been unsuitable for the cache. I would assume that toml will need to be able to store metadata. We could work around all these edge cases by storing additional information (or the key value in case of directoryvalue) in metadata. Regarding that I agree with @kodebach that #3256 makes sense.

[bauhaus93]

Will the toml plugin eventually be able to handle metadata? Afaik it is currently unsupported.

Yes, I definitely intend to add handling of metadata for the plugin. I think it will be needed for the default storage test #3478 to be successful, so I may implement it during resolving that.

[markus2330]

Not sure about TOML

I did not find anything: https://toml.io/en/v1.0.0-rc.2 and I also do not know of any format where your described indistinguishable variants would exist. (Not surprising as serialization formats usually do not have different serialization variants and configuration file formats usually do not have explicit null.)

Then toml itself could deal with non-leaf keys with value any way it sees fit.

Yes, I agree it should be storage specific (potentially with helper libraries). For different formats different representations simply fit better. See also #3256

The directoryvalue plugin could still be kept around for other plugins that don't have the problems that toml has.

@sanssecours What is your thought on that? Is directoryvalue actually the best solution for YAML? Or would you do it in a library if you would implement everything again?

[kodebach]

I also do not know of any format where your described indistinguishable variants would exist

Possibly HOCON. It has null, since it is JSON superset. It also has the "Paths as keys" feature. Although the object merging might make it so that my example is not a problem.

[sanssecours]

@sanssecours What is your thought on that?

As far as I can remember directoryvalue worked reasonably well for the YAML plugins I wrote.

Is directoryvalue actually the best solution for YAML?

As there is usually not a _“best solution”_, probably not 😅. One problem of the plugin is certainly that ___dirdata entries can not be escaped.

Or would you do it in a library if you would implement everything again?

That depends on the library, and how complicated it would be for a storage plugin developer to add support for “directory values”.

[markus2330]

One problem of the plugin is certainly that ___dirdata entries can not be escaped.

Good point, this speaks very much for the proposal of @kodebach