Overwrites duplicate values with the values in the latest file

[nangelovaTeladoc]

The main changes intend to fix the multi-file inheritance precedence order.

This PR is regarding magynhard#7 and magynhard#6, but does NOT address magynhard#5

The desired behavior is described in the README (emphasis mine):

If you want to inherit from several files, you can specify a list (Array) of files. They are merged from top to bottom, so the latest file "wins" - that means it overwrites duplicate values if they exist with the values in the latest file where they occur.

Before the current changes, the reality was that the precedence order was exactly the opposite - the topmost file would "win".

This was caused because the deeper_merge method from the deep_merge gem was used.
According to this issue, the deeper_merge! version should be used instead for the cases where we want hash values to actually be overwritten (instead of just augmented with new keys). The documentation of deep_merge is not very clear on that.

Other changes in this PR:

• Remove the internal config param from the public YAML.ext_load_file method
• Remove the YamlExtendHelper used to encode/decode booleans (no longer needed, see Overwrites duplicate values with the values in the latest file #1 (comment))
• Bump the major gem version to 1.0.0 (from 0.2.5), because of breaking changes in behavior
• A few minor tweaks to the README.md - fixing references to class methods to use . instead of # (which is normally used for instance methods).

Let's have the following file structure as an example for the inheritance problem (test case added to specs)

with file contents of:

# parent_L1_A.yml
overwritten_until_child: parent L1 A value
overwritten_until_L2_C: parent L1 A value
overwritten_until_L2_B: parent L1 A value
overwritten_until_L2_A: parent L1 A value
overwritten_until_L1_B: parent L1 A value
own_parent_L1_A: parent L1 A value

# parent_L1_B.yml
overwritten_until_child: parent L1 B value
overwritten_until_L2_C: parent L1 B value
overwritten_until_L2_B: parent L1 B value
overwritten_until_L2_A: parent L1 B value
overwritten_until_L1_B: parent L1 B value
own_parent_L1_B: parent L1 B value

# parent_L2_A.yml
extends:
  - parent_L1_A.yml
  - parent_L1_B.yml

overwritten_until_child: parent L2 A value
overwritten_until_L2_C: parent L2 A value
overwritten_until_L2_B: parent L2 A value
overwritten_until_L2_A: parent L2 A value
own_parent_L2_A: parent L2 A value

# parent_L2_B.yml
overwritten_until_child: parent L2 B value
overwritten_until_L2_C: parent L2 B value
overwritten_until_L2_B: parent L2 B value
own_parent_L2_B: parent L2 B value

# parent_L2_C.yml
overwritten_until_child: parent L2 C value
overwritten_until_L2_C: parent L2 C value
own_parent_L2_C: parent L2 C value

# child.yml
extends: 
  - parent_L2_A.yml
  - parent_L2_B.yml
  - parent_L2_C.yml

overwritten_until_child: child value

own_child: child value

The expected result of YAML.ext_load_file 'child.yml' should then be:

overwritten_until_child: 'child value'
overwritten_until_L2_C: 'parent L2 C value'
overwritten_until_L2_B: 'parent L2 B value'
overwritten_until_L2_A: 'parent L2 A value'
overwritten_until_L1_B: 'parent L1 B value'
own_parent_L1_A: 'parent L1 A value'
own_parent_L1_B: 'parent L1 B value'
own_parent_L2_A: 'parent L2 A value'
own_parent_L2_B: 'parent L2 B value'
own_parent_L2_C: 'parent L2 C value'
own_child: 'child value'

but instead of that, we were getting:

overwritten_until_child: 'child value'      # expected
own_child: 'child value'                    # expected
overwritten_until_L2_C: 'parent L2 A value' # NOT expected, should have been 'parent L2 C value'
overwritten_until_L2_B: 'parent L2 A value' # NOT expected, should have been 'parent L2 B value'
overwritten_until_L2_A: 'parent L2 A value' # expected
own_parent_L2_A: 'parent L2 A value'        # expected
overwritten_until_L1_B: 'parent L1 A value' # NOT expected, should have been 'parent L1 B value'
own_parent_L1_A: 'parent L1 A value'        # expected
own_parent_L1_B: 'parent L1 B value'        # expected
own_parent_L2_B: 'parent L2 B value'        # expected
own_parent_L2_C: 'parent L2 C value'        # expected

Nil values
Let's have the following file structure as an example for the inheritance problem with nil values (test case added to specs)

with file contents of:

# base.yml
overwritten_nil: null
not_overwritten_string: string value
overwritten_false: false
not_overwritten_true: true

# extended.yml
extends:
  - base.yml

overwritten_nil: string value
not_overwritten_string: null
overwritten_false: null
not_overwritten_true: null

The expected result of YAML.ext_load_file extended.yml should then be:

overwritten_nil: string value
not_overwritten_string: string value
overwritten_false: nil
not_overwritten_true: true

but instead of that, we were getting:

overwritten_nil: string value        # expected
not_overwritten_string: string value # expected
overwritten_false: false             # NOT expected, should have been nil
not_overwritten_true: true           # expected

For reference, here's how the underlying deep_merge gem handles cases with nil values:

* { key: false }.deeper_merge({ key:  nil })  --> { key: nil }
* { key: nil }.deeper_merge({ key: false })   --> { key: false }

* { key: false }.deeper_merge!({ key:  nil }) --> { key: nil }
* { key: nil }.deeper_merge!({ key: false })  --> { key: false }

[skyuchukov-teladoc]

It will probably be a good idea to bump the major version (to 1.0.0?) as the behavior is changed (breaking changes).

[skyuchukov-teladoc]

Changes look good, let's document the fixes in the PR's description, we can later copy these to other PR's / wikis, etc.
I think it is OK to commit an image file for the docs to the repo, as long as it is not included in the gem bundle file that is going to be published.

[skyuchukov-teladoc]

@nangelovaTeladoc Seems with the change to deeper_merge! we can get rid of all the encode/decode boolean functionality - it was created as a workaround for issues in deep_merge (no bang version):
danielsdeleo/deep_merge#23 (comment)

[NicoLugil]

Seems many people have been working on similar things in paralle.
You might also want to have a look at magynhard#8 which solves magynhard#6

[magynhard]

Hi guys, thank you for your great work. I've figured out by playing around with your PR, that your solution also already addresses magynhard#6

I like your work and I would be happy to have your changes integrated into the original project.
Are you planning any more changes or are you finished here?

I would like to add some more tests for some cases related to the merging order after adding your changes and make a new 1.x release then.

[nangelovaTeladoc]

Hi @magynhard,
Sounds good. Currently, we are not planning to make more changes.
We will be happy to use the 1.x version.

[magynhard]

@nangelovaTeladoc so can you merge this PR into the master and create a PR on the original github repo please?

[skyuchukov-teladoc]

@nangelovaTeladoc
The changes look good, there are passing specs that cover the functionality concerning bools (helper was removed).

@magynhard
You are right, this indeed fixes magynhard#6, we failed to correctly understand the expected behavior there initially. Thanks for the heads up!

The PR's description was updated to reflect that.
Also mentioned explicitly how the extend functionality is supposed to work when having nil values (specs added).

Will now merge this and open a PR to the upstream repo.