Map "deep" merge only adds new items, but not override existing values

[alinakovalenko]

I read 2 configs one by one:
1:

key:
1: 1
2: 2
3: 3

2:

key:
1: 2
2: 3
4: 5

I specified JsonMerge for a Map<Integer, Integer> property and as a result got:
{1: 1, 2: 2, 3: 3, 4: 5}

But I expected:
{1: 2, 2: 3, 3: 3, 4: 5}

Without JsonMerge I got:
{1: 2, 2: 3, 4: 5}

Update:
Map<String, Integer> works properly - I got expected result

[cowtowncoder]

Please add a reproducible test. Explanation is unfortunately not enough to reproduce the problem.

[alinakovalenko]

1.yaml

key1:
  1: 1
  2: 2
  3: 3

key2:
  1: 1
  2: 2
  3: 3

2.yaml

key1:
  1: 2
  2: 3
  4: 5

key2:
  1: 2
  2: 3
  4: 5

Class

package com.example.test;

import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;
import lombok.Data;
import lombok.ToString;

import java.util.HashMap;
import java.util.Map;

@Data
@ToString
@JsonIgnoreProperties(ignoreUnknown = true)
public class TestMap {
    @JsonProperty("key1")
    private Map<String, Integer> mapStringInteger = new HashMap<>();

    @JsonProperty("key2")
    private Map<Integer, Integer> mapIntegerInteger = new HashMap<>();
}

Test

package com.example.test;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.powermock.core.classloader.annotations.PowerMockIgnore;
import org.powermock.modules.junit4.PowerMockRunner;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;

import java.io.File;

@RunWith(PowerMockRunner.class) @PowerMockIgnore("javax.management.*")
public class MapTest {

    private TestMap testMap = new TestMap();
    private final ObjectMapper mapper = new ObjectMapper(new YAMLFactory());

    @Test
    public void testMap() {
        mapper.setDefaultMergeable(true);

        try {
            File f1 = new File("src/test/config/1.yaml");
            File f2 = new File("src/test/config/2.yaml");
            testMap = mapper.readerForUpdating(testMap).readValue(f1);
            System.out.println(testMap);
            testMap = mapper.readerForUpdating(testMap).readValue(f2);
            System.out.println(testMap);
            System.out.println(testMap.getMapIntegerInteger().get(1));
            System.out.println(testMap.getMapStringInteger().get("1"));
        } catch (Exception e) {
            // handle exceptions
            e.printStackTrace();
        }
    }
}

[cowtowncoder]

Thank you for the test!

Hmmh. Ok, so I assume YAML is not important here and json would reproduce same problems.

But I am more suspicious of Lombok: it has sometimes been a pita causing issues of its own.
For now I assume it is not relevant, however, and can simplify test to remove it (lombok being one dependency I do not allow for any code within jackson).

[alinakovalenko]

The same without Lombok

New class without Lombok

package com.uber.money.instantpay.configuration.test;

import com.fasterxml.jackson.annotation.JsonProperty;

import java.util.HashMap;
import java.util.Map;

public class TestMap1 {
    public Map<String, Integer> getMapStringInteger() {
        return mapStringInteger;
    }

    @JsonProperty("key1")
    public void setMapStringInteger(Map<String, Integer> mapStringInteger) {
        this.mapStringInteger = mapStringInteger;
    }

    public Map<Integer, Integer> getMapIntegerInteger() {
        return mapIntegerInteger;
    }

    @JsonProperty("key2")
    public void setMapIntegerInteger(Map<Integer, Integer> mapIntegerInteger) {
        this.mapIntegerInteger = mapIntegerInteger;
    }

    private Map<String, Integer> mapStringInteger = new HashMap<>();

    private Map<Integer, Integer> mapIntegerInteger = new HashMap<>();
}

Tests:

package com.uber.money.instantpay.configuration.test;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.powermock.core.classloader.annotations.PowerMockIgnore;
import org.powermock.modules.junit4.PowerMockRunner;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;

import java.io.File;
import static org.junit.Assert.assertEquals;

@RunWith(PowerMockRunner.class) @PowerMockIgnore("javax.management.*")
public class MapTest {

    private TestMap testMap = new TestMap();
    private TestMap1 testMap1 = new TestMap1();
    private final ObjectMapper mapper = new ObjectMapper(new YAMLFactory());

    @Test
    public void testMap() {
        mapper.setDefaultMergeable(true);

        try {
            File f1 = new File("src/test/config/1.yaml");
            File f2 = new File("src/test/config/2.yaml");
            testMap = mapper.readerForUpdating(testMap).readValue(f1);
            testMap1 = mapper.readerForUpdating(testMap1).readValue(f1);
            testMap = mapper.readerForUpdating(testMap).readValue(f2);
            testMap1 = mapper.readerForUpdating(testMap1).readValue(f2);
            System.out.println(testMap.getMapIntegerInteger().get(1));
            System.out.println(testMap.getMapStringInteger().get("1"));
            assertEquals(testMap.getMapIntegerInteger().get(1), testMap1.getMapIntegerInteger().get(1));
            assertEquals(testMap.getMapStringInteger().get("1"), testMap1.getMapStringInteger().get("1"));
        } catch (Exception e) {
            // handle exceptions
            e.printStackTrace();
        }
    }
}

[cowtowncoder]

Thank you!

[cowtowncoder]

Odd. Using JSON version passes... I'll have to figure out how to test yaml, since this module can not have a dependency to yaml format. And see if I can get that to fail.

[cowtowncoder]

Actually I also can not reproduce this with YAML: both assertions pass.

But looking at output it looks like maybe assertions made are incorrect (or incomplete)?
It looks as if value for key1 is merged as expected, but key2 not so.

And this actually seems similar between JSON and YAML, as I would expect.

[cowtowncoder]

So I suspect there's something odd going on with coercion of key... or, could also be difference between handling of non-default key.
So I think I have something to go on at this point.

[alinakovalenko]

These assertions should pass - it shows that with lombok and without we got the same result
if you will try

assertEquals(testMap.getMapIntegerInteger().get(1), testMap.getMapStringInteger().get("1"));

this will fail (key1 and key2 the same in both files)
it means that if I try to read property to Map<Srting, Integer> it would be overrided (we get 2 as result of testMap.getMapStringInteger().get("1")), if Map<Integer, Integer> - would not be overrided (we get 1 as result of testMap.getMapIntegerInteger().get(1) )

[cowtowncoder]

@alinakovalenko Thank you again for reporting the issue (which will be fixed for 2.9.3), as well as providing reproduction!

[cowtowncoder]

@alinakovalenko Typically assertions are used to show the problem before fix, so I was assuming they should fail instead of passing. So that was my misunderstanding. Not a big deal once I understood that.
And then was able to figure out what was the difference in handling; turns out one code path was not handling overriding of existing values properly (even though it actually checked for that correctly).