spring-projects / spring-data-rest

Simplifies building hypermedia-driven REST web services on top of Spring Data repositories
https://spring.io/projects/spring-data-rest
Apache License 2.0
913 stars 561 forks source link

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

Closed alienisty closed 10 months ago

alienisty commented 11 months ago

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

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;
    }
  }

// ...