Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep config item order in ConfigObject #365

Open
WilliamStone opened this issue Dec 28, 2015 · 9 comments
Open

Keep config item order in ConfigObject #365

WilliamStone opened this issue Dec 28, 2015 · 9 comments

Comments

@WilliamStone
Copy link

In version 1.3.0 the map implementation used in ConfigObject is HashMap, which doesn't keep entry order. LinkedHashMap keeps order without significant performance hit. In certain cases config item order is important or very convenient to perform certain user control via config file. And keeping item order has no obvious harm IMHO. So I suggest replace HashMap by LinkedHashMap in ConfigObject.

@havocp
Copy link
Collaborator

havocp commented Dec 28, 2015

See #35 for discussion. With the heavy use of merging to support system props and default configs, it would be problematic in practice to make order meaningful in a Config. There is no way yet proposed to merge or override the order. Alternate solutions may be to use an array or put some type of priority number in an object.

@WilliamStone
Copy link
Author

Hi, thanks for your reply! I read #35 and I see the difficulty we face. Yet I still think key ordering should be a "really-good-to-have" feature, so I'll try to give some thinkings on this topic.

I see you mentioned it's hard to find certain intuitive principle about ordering, considering merging/withFallBack features. I understand the principle, if exists, must be simple and universal, so that it can be applied to every corner of the config library. I don't know the library deep enough like you, but I want to try from user's perspective to draw a principle: Rule A: What shows later is more important. The reverse rule is also effective: Rule B: What is more important shows later. Let me explain:

  1. in same file:
    Above principle means in following config file:
    a=1
    b=2
    a=3
    c=4
    a=5
    The final result(item and order) should be:
    b=2
    c=4
    a=5
    Current library behavior in picking value of duplicate keys follows the principle.
  2. merging or withFallBack
    This scenario means there are at least 2 config files: One is reference file and one is main file.
    Your description in ConfigImpl doesn't respect key order #35 (imagine cutting this in half at any line, and the second half is combined with the first using withFallback) implies merging and plain copying/pasting should be effectively equal. If we think in the way that the main file must first know there does have a reference file exists that can be refer to, then we can define that the reference file is former one and main file is latter one, which means main file should follow reference file and keys in main file are prefer to those in reference file.

Take the scenario you described as example: reference.conf has keys in order a b c and application.conf has c a but doesn't specify b

--- reference.conf ---

a=1
b=2
c=3

--- application.conf ---

c=4

The result should be:

a=1
b=2
c=4

The reasoning is that keys from application.conf should be more important than those of reference.conf, so items in reference.conf can be overrided. According to Rule B keys in application.conf should be at tail of merge result. It's easier to think if we imaginarily copy the two files together into same file, reference.conf over application.conf(according to Rule B), we can get the same result.

  1. Multiple level merging
    You also mentioned it is important that withFallback continues to be an associative operation. And I think the principle doesn't harm this. In my understanding associative operation means that
    configX.withFallBack(configY).withFallBack(configZ) ...... (A)
    should be equal to
    configX.withFallBack(configY.withFallBack(configZ)) ...... (B)
    If we follow the principle, we'll see that in final merge result keys from configZ shows first, followed by keys from configY, then configX. And duplicate keys are removed, just keeping the last one. For example:

--- configZ ---

a=1
b=2
a=3
c=4

--- configY ---

c=5

--- configX ---

b=6

--- A.1 Result of configX.withFallBack(configY) ---

c=5
b=6

--- A.2 Result of configX.withFallBack(configY).withFallBack(configZ) ---

a=3
c=5
b=6

--- B.1 Result of configY.withFallBack(configZ)

b=2
a=3
c=5

--- B.2 Result of configX.withFallBack(configY.withFallBack(configZ)) ---

a=3
c=5
b=6

So A.2 == B.2.

So in my consideration the principle is consistent and intuitive, however there is something I didn't considered though, which is config object equality you mentioned. I didn't understand the question well and I guess your meaning is that if we cannot draw a principle to unify key ordering of different associative operation result, then the equality would be broken. If the above principle can give us a stable result ordering, then we should not have to consider equality issue. I have to re-mention that I just considered this whole thing from user perspective and there may be some big mistake in my reasoning so please take this as a reference and I hope this can help implementing the key ordering feature sooner.

@havocp
Copy link
Collaborator

havocp commented Dec 31, 2015

Thanks for digging in on this! Some unit tests that are pretty relevant are these associativity tests here:
https://github.com/typesafehub/config/blob/master/config/src/test/scala/com/typesafe/config/impl/ConfigTest.scala#L25-L442

It might be simple to add some that think about ordering, and see what can be made to work...

A principle that's simply "order is what you would get if you pasted all the files into one and kept the last instance of each key" maybe works, though it's not going to be what people want sometimes perhaps... if you have a,b,c,d and then later override a, a ends up at the end of the list... maybe that's fine.

Another issue that arises; say people use system property overrides like -Da=10, -Db=12; Java's Properties I don't think is ordered (?), so we will get a and b in no meaningful order ... presumably in that case we'd want to keep the order from reference.conf or application.conf if any... but that means we need to preserve the information that the system property overrides have no meaningful order. So Config would now need to have "ordered mode" and "unordered mode" or something along those lines?
Or alternatively, we'd use an arbitrary/random order for anything overridden via system props.

This is going to mean that any app that depends on key order has a "land mine" in wait for the unwary user who specifies -Dfoo=bar and in so doing accidentally re-orders keys. That seems like a problem to me, but in evaluating its seriousness maybe collecting some use-cases for ordering would be helpful.

@ryantheleach
Copy link

The main usecase I see for ordering is that the user of the application has edited a user config, in the order that they wish it to be in, the application makes a modification to the config (for whatever reason, adding nodes during a software upgrade, changing based on user input) and the order of the file modified should be preserved where possible.

If you are dumping the combined inherited results of multiple configs, I think the user will understand that their order might change slightly, but not be completely shuffled by a hash.

@havocp
Copy link
Collaborator

havocp commented Jan 10, 2016

There's a separate API intended for applications to modify configs fwiw
http://typesafehub.github.io/config/latest/api/
This is only for single files, but it preserves much more than order (also comments etc)

@havocp
Copy link
Collaborator

havocp commented Mar 4, 2016

#300 would mean supporting this in ConfigDocument (single doc parse tree level)

@garyelephant
Copy link

@WilliamStone @havocp I'm going to fork this project and modify the map implementation used in ConfigObject from HashMap to LinkedHashMap, because in my case, all configuration files don't need merge or override. Can you please me the place of the map implementation used in ConfigObject, I failed to find the codes.

@garyelephant
Copy link

I forked this project and replace HashMap with LinkedHashMap, it works, please see: https://github.com/InterestingLab/config

@havocp
Copy link
Collaborator

havocp commented Nov 25, 2018

I’m glad that is working for you!

For anyone considering that route, I’d advise that when relying on order you do not use ConfigFactory.load* APIs (only ConfigFactory.parse*), do not use reference.conf and do not use Config.withFallback. In other words use the library strictly as a file parser don’t do any merging or system property overrides.

The ConfigDocument API is the right way to extend the “strictly a parser” use case in the main codebase. It’s the API that gives you raw parse tree information rather than a “configuration object”

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants