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

Fields named "id" in nested MongoDB objects are not updated by PUT requests #2280

Open
mjustin opened this issue Jun 27, 2023 · 5 comments
Open
Assignees
Labels
status: feedback-provided Feedback has been provided

Comments

@mjustin
Copy link

mjustin commented Jun 27, 2023

PUT requests for nested MongoDB objects with an exposed "id" field update other fields in the nested objects, but not the "id" field. This is true for lists of nested objects, and as of Spring Data REST 4.0.3 (due I think to #2174) for directly nested objects.

This feels like it's in a similar vein to spring-projects/spring-data-mongodb#3351, in that nested MongoDB objects aren't really themselves entities, but Spring Data MongoDB & Spring Data REST treat them as such.

I ran across this issue when upgrading a project from Spring Boot 2.7.4 to 3.1.1, since Spring Data REST changed non-list nested object behavior, changing the existing behavior of the application.

Example

Build

build.gradle

plugins {
    id 'java'
    id 'org.springframework.boot' version '3.1.1'
    id 'io.spring.dependency-management' version '1.1.0'
}
java { sourceCompatibility = '17' }
repositories { mavenCentral() }
dependencies {
    implementation 'org.springframework.boot:spring-boot-starter-data-mongodb'
    implementation 'org.springframework.boot:spring-boot-starter-data-rest'
    testImplementation 'org.springframework.boot:spring-boot-starter-test'
    testImplementation 'org.springframework.boot:spring-boot-testcontainers'
    testImplementation 'org.testcontainers:junit-jupiter'
    testImplementation 'org.testcontainers:mongodb'
}
tasks.named('test') { useJUnitPlatform() }

Application

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;

@SpringBootApplication
public class ExampleApplication {
    public static void main(String[] args) {
        SpringApplication.run(ExampleApplication.class, args);
    }
}
import java.util.Objects;

public class Nested {
    private String id;
    private Integer value;

    public Nested() {
    }
    public Nested(String id, Integer value) {
        this.id = id;
        this.value = value;
    }

    public String getId() {return id;}
    public void setId(String id) {this.id = id;}
    public Integer getValue() {return value;}
    public void setValue(Integer value) {this.value = value;}

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (!(o instanceof Nested that)) return false;
        return Objects.equals(id, that.id) && Objects.equals(value, that.value);
    }

    @Override public int hashCode() {return Objects.hash(id, value);}
    @Override public String toString() {return "[%s,%s]".formatted(id, value);}
}
public class Example {
    private String id;
    private Nested nested;

    public Example() {
    }

    public Example(String id, Nested nested) {
        this.id = id;
        this.nested = nested;
    }

    public String getId() {return id;}
    public void setId(String id) {this.id = id;}
    public Nested getNested() {return nested;}
    public void setNested(Nested nested) {this.nested = nested;}
}
import java.util.List;

public class ListExample {
    private String id;
    private List<Nested> list;

    public ListExample() {
    }

    public ListExample(String id, List<Nested> list) {
        this.id = id;
        this.list = list;
    }

    public String getId() {return id;}
    public void setId(String id) {this.id = id;}
    public List<Nested> getList() {return list;}
    public void setList(List<Nested> list) {this.list = list;}
}
import org.springframework.data.repository.CrudRepository;

public interface ExampleRepository extends CrudRepository<Example, String> {}
import org.springframework.data.repository.CrudRepository;

public interface ListExampleRepository extends CrudRepository<ListExample, String> { }
import org.springframework.data.rest.core.config.RepositoryRestConfiguration;
import org.springframework.data.rest.webmvc.config.RepositoryRestConfigurer;
import org.springframework.stereotype.Component;
import org.springframework.web.servlet.config.annotation.CorsRegistry;

@Component
public class ExampleRepositoryConfigurer implements RepositoryRestConfigurer {
    @Override
    public void configureRepositoryRestConfiguration(RepositoryRestConfiguration configuration, CorsRegistry corsRegistry) {
        configuration.exposeIdsFor(Nested.class);
    }
}

Tests

import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.json.AutoConfigureJsonTesters;
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.json.JacksonTester;
import org.springframework.http.MediaType;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.ResultActions;
import org.testcontainers.junit.jupiter.Testcontainers;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

@SpringBootTest
@AutoConfigureMockMvc
@AutoConfigureJsonTesters
@Testcontainers
public class IssueTest {
    @Autowired
    private ExampleRepository repository;

    @Autowired
    protected MockMvc mockMvc;

    @Autowired
    protected JacksonTester<Example> jsonTester;

    @Test
    public void issue() throws Exception {
        repository.save(new Example("a", new Nested("A", 1)));
        performUpdateRequest(new Example("a", new Nested("B", 2)));
        Example saved = repository.findById("a").orElseThrow();
        assertEquals(2, saved.getNested().getValue()); // Succeeds
        assertEquals("B", saved.getNested().getId()); // Fails
    }

    private ResultActions performUpdateRequest(Example input) throws Exception {
        return mockMvc.perform(put("/examples/{id}", input.getId())
                .accept(MediaType.APPLICATION_JSON)
                .contentType(MediaType.APPLICATION_JSON)
                .content(jsonTester.write(input).getJson()))
                .andExpect(status().isOk());
    }
}

=>

expected: <B> but was: <A>
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.json.AutoConfigureJsonTesters;
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.json.JacksonTester;
import org.springframework.http.MediaType;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.ResultActions;
import org.testcontainers.junit.jupiter.Testcontainers;

import java.util.List;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

@SpringBootTest
@AutoConfigureMockMvc
@AutoConfigureJsonTesters
@Testcontainers
public class ListIssueTest {
    @Autowired
    private ListExampleRepository repository;

    @Autowired
    protected MockMvc mockMvc;

    @Autowired
    protected JacksonTester<ListExample> jsonTester;

    @Test
    public void issue() throws Exception {
        List<Nested> initialList =
                List.of(new Nested("A", 1), new Nested("B", 2), new Nested("C", 3));
        List<Nested> updatedList =
                List.of(new Nested("B", 2), new Nested("D", 4), new Nested("A", 1), new Nested("E", 5));

        repository.save(new ListExample("a", initialList));
        performUpdateRequest(new ListExample("a", updatedList));
        ListExample saved = repository.findById("a").orElseThrow();
        assertEquals(updatedList, saved.getList());
    }

    private ResultActions performUpdateRequest(ListExample input) throws Exception {
        return mockMvc.perform(put("/listExamples/{id}", input.getId())
                .accept(MediaType.APPLICATION_JSON)
                .contentType(MediaType.APPLICATION_JSON)
                .content(jsonTester.write(input).getJson()))
                .andExpect(status().isOk());
    }
}

=>

expected: <[[B,2], [D,4], [A,1], [E,5]]> but was: <[[A,2], [B,4], [C,1], [E,5]]>

Workaround

One workaround I've discovered is to rename the id field but keep the getId() method, marking it as @Transient.

    public static class Nested {
        @Field("_id")
        @JsonIgnore
        private String nestedId;

        @Transient
        public String getId() {return nestedId;}
        public void setId(String id) {this.nestedId = id;}
@odrotbohm
Copy link
Member

Would you mind trying the latest snapshots? I wonder if the fix for #2287 fixes this issue here as well?

@odrotbohm odrotbohm added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 11, 2023
@spring-projects-issues
Copy link

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Sep 18, 2023
@mjustin
Copy link
Author

mjustin commented Sep 18, 2023

@odrotbohm I've tested this issue against the now-released version 4.1.4 for the spring-data-rest-core & spring-data-rest-webmvc dependencies. My test scenarios still fail even with the upgrade.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue labels Sep 18, 2023
@odrotbohm
Copy link
Member

Any chance you can bundle this up into a minimal reproducer?

@mjustin
Copy link
Author

mjustin commented Sep 19, 2023

Any chance you can bundle this up into a minimal reproducer?

Sure; I basically have one locally already. It might be a day or two before I can get to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: feedback-provided Feedback has been provided
Projects
None yet
Development

No branches or pull requests

3 participants