Skip to content

The merge+patch implementation does not conform to RFC 7386 #2325

@alienisty

Description

@alienisty

The current implementation of the mergePatch algorithm fails to properly handle merge of arrays, in fact, if the target object field;

  • is an array, we can't patch it's content at all.
  • is a collection, we can change existing elements, but we can only append one element.

The following test cases (to add to DomainObjectReaderUnitTests) will fail with the current implementation even if the expectations are valid according to RFC 7386:

  @BeforeEach
  void setUp() {

    KeyValueMappingContext<?, ?> mappingContext = new KeyValueMappingContext<>();
    // ...
    mappingContext.getPersistentEntity(ArrayListHolder.class);
    // ...
  }
     
  //...

  @Test // issue #2325
  void arraysCanMutateAndAppendDuringMerge() throws Exception {
    ObjectMapper mapper = new ObjectMapper();
    ArrayHolder target = new ArrayHolder(new String[] { "ancient", "old", "older" });
    JsonNode node = mapper.readTree("{ \"array\" : [ \"new\", \"old\", \"newer\", \"bleeding edge\" ] }");

    ArrayHolder updated = reader.doMerge((ObjectNode) node, target, mapper);
    assertThat(updated.array).containsExactly("new", "old", "newer", "bleeding edge");
  }

  @Test // issue #2325
  void arraysCanAppendMoreThanOneElementDuringMerge() throws Exception {
    ObjectMapper mapper = new ObjectMapper();
    ArrayListHolder target = new ArrayListHolder( "ancient", "old", "older" );
    JsonNode node = mapper.readTree("{ \"values\" : [ \"ancient\", \"old\", \"older\", \"new\", \"newer\" ] }");

    ArrayListHolder updated = reader.doMerge((ObjectNode) node, target, mapper);
    assertThat(updated.values).containsExactly("ancient", "old", "older", "new", "newer");
  }

  @Test // issue #2325
  void arraysCanRemoveElementsDuringMerge() throws Exception {
    ObjectMapper mapper = new ObjectMapper();
    ArrayHolder target = new ArrayHolder(new String[] { "ancient", "old", "older" });
    JsonNode node = mapper.readTree("{ \"array\" : [ \"ancient\" ] }");

    ArrayHolder updated = reader.doMerge((ObjectNode) node, target, mapper);
    assertThat(updated.array).containsExactly("ancient");
  }

// ...

  static class ArrayListHolder {
    Collection<String> values;

    ArrayListHolder(String... values) {
      this.values = new ArrayList<>(Arrays.asList(values));
    }
    
    public void setValues(Collection<String> values) {
      this.values = values;
    }
  }

// ...

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions