Skip to content

Composite property issue: removing entry doesn't work #2449

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

Closed
gonzalad opened this issue Nov 22, 2021 · 3 comments
Closed

Composite property issue: removing entry doesn't work #2449

gonzalad opened this issue Nov 22, 2021 · 3 comments
Assignees
Labels
status: duplicate A duplicate of another issue

Comments

@gonzalad
Copy link

gonzalad commented Nov 22, 2021

Hello,

I'm using SDN 6.2.0.

When I remove an entry from a composite property.
Then I expect the entry to be removed from the database node.
But the entry is not removed.

I think the issue originates from the Neo4j query generated by SDN in CypherGenerator#prepareSaveOf (I assume the += in the second match):

OPTIONAL MATCH (hlp:`Sample`) WHERE id(hlp) = $__id__ WITH hlp
WHERE hlp IS NULL CREATE (sampleEntity:`Sample`) SET sampleEntity = $__properties__ RETURN sampleEntity 
UNION
MATCH (sampleEntity:`Sample`) WHERE id(sampleEntity) = $__id__ SET sampleEntity += $__properties__ RETURN sampleEntity

Thanks !

Sample

@Node("Sample")
public class SampleEntity {


    @GeneratedValue
    @Id
    private Long id;

    private String name;

    @CompositeProperty(prefix = "somePrefix")
    private Map<String, Object> composite = new HashMap<>();

   ...getters/setters...
}

Test case:

@SpringBootTest
class Sdn6Test {

    @Autowired
    private SampleRepository sampleRepository;

    @Test
    void removeEntryInCompositePropertyShouldRemoveEntryInDb() {

        Map composite = new HashMap(Map.ofEntries(
            Map.entry("entry1", "value1"),
            Map.entry("entry2", "value2")
        ));
        SampleEntity entity = createSample(composite);
        // removing a single entry 
        entity.getComposite().remove("entry1");
        sampleRepository.save(entity);

        Optional<SampleEntity> entityInDatabase = sampleRepository.findById(entity.getId());

         // **error** returns 2 instead of 1
        assertThat(entityInDatabase.orElseThrow().getComposite()).hasSize(1);
    }

    private SampleEntity createSample(Map composite) {
        SampleEntity entity = new SampleEntity();
        entity.setName("sample");
        entity.setComposite(new HashMap<>(composite));
        return sampleRepository.save(entity);
    }
}

Reproducing the issue

The issue can be reproduced by running the unit test from this sample project:

https://github.com/gonzalad/sdn6-tests/tree/issue-composite-properties by running the unit test: Sdn6Test#removeEntryInCompositePropertyShouldRemoveEntryInDb (https://github.com/gonzalad/sdn6-tests/blob/issue-composite-properties/src/test/java/com/example/sdn6/Sdn6Test.java#L29)

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 22, 2021
@meistermeier meistermeier self-assigned this Nov 22, 2021
@michael-simons michael-simons added status: duplicate A duplicate of another issue and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 23, 2021
@michael-simons
Copy link
Collaborator

Hi @gonzalad thanks for taking the time to fill an issue.

This one is a duplicate of #2280.

Elements in a composite property must be set explicitly to literal null or to Values.NULL to be removed.
This can be done either in a custom converter if you run such a thing for your entity or in client code such as in your case.

Why is that? Because SDN6 is "session free" or in other words, it doesn't keep a cache of your loaded entities around and therefor does not know what was there when an entity was loaded.

In Neo4j null properties are equivalent to non-existing properties.

Therefor your test can be fixed like this:

@Test
void removeEntryInCompositePropertyShouldRemoveEntryInDb() {

    Map composite = new HashMap(Map.ofEntries(
        Map.entry("entry1", "value1"),
        Map.entry("entry2", "value2")
    ));
    SampleEntity entity = createSample(composite);
    assertThat(entity.getComposite()).hasSize(2);
    entity.getComposite().put("entry1", null);
    sampleRepository.save(entity);

    Optional<SampleEntity> entityInDatabase = sampleRepository.findById(entity.getId());

    assertThat(entityInDatabase)
        .isPresent()
        .hasValueSatisfying(v -> {
            assertThat(v.getName()).isEqualTo(entity.getName());
            assertThat(v.getComposite())
                .hasSize(1)
                .containsEntry("entry2", Values.value("value2"));
        });
}

I personally would add a method such as removeEntry(String key to your entity for this purpose and inside that method doing what I did in the test (entity.getComposite().put("entry1", null);)

@gonzalad
Copy link
Author

Thanks @michael-simons, sorry I missed the initial issue !

@michael-simons
Copy link
Collaborator

My pleasure, and no problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate A duplicate of another issue
Projects
None yet
Development

No branches or pull requests

4 participants