From 3af755512a5436a92fc724a831ac84c2aab1dbec Mon Sep 17 00:00:00 2001 From: Michael Simons Date: Thu, 6 Aug 2020 17:52:44 +0200 Subject: [PATCH] DATAGRAPH-1351 - Apply possible conversions for ids during delete. Co-authored-by: Dennis Crissman Co-authored-by: Rosetta Roberts --- .../data/neo4j/core/Neo4jTemplate.java | 4 +- .../neo4j/core/ReactiveNeo4jTemplate.java | 4 +- .../DefaultNeo4jPersistentProperty.java | 2 +- .../imperative/Neo4jOperationsIT.java | 91 ++++++++++++++++--- .../reactive/ReactiveNeo4jOperationsIT.java | 87 ++++++++++++++++-- .../shared/PersonWithCustomId.java | 79 ++++++++++++++++ 6 files changed, 241 insertions(+), 26 deletions(-) create mode 100644 src/test/java/org/springframework/data/neo4j/integration/shared/PersonWithCustomId.java diff --git a/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java b/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java index 2d426fc2f..ec1f89547 100644 --- a/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java +++ b/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java @@ -316,7 +316,7 @@ public final class Neo4jTemplate implements Neo4jOperations, BeanFactoryAware { log.debug(() -> String.format("Deleting entity with id %s ", id)); Statement statement = cypherGenerator.prepareDeleteOf(entityMetaData, condition); - ResultSummary summary = this.neo4jClient.query(renderer.render(statement)).in(getDatabaseName()).bind(id) + ResultSummary summary = this.neo4jClient.query(renderer.render(statement)).in(getDatabaseName()).bind(convertIdValues(id)) .to(nameOfParameter).run(); log.debug(() -> String.format("Deleted %d nodes and %d relationships.", summary.counters().nodesDeleted(), @@ -333,7 +333,7 @@ public final class Neo4jTemplate implements Neo4jOperations, BeanFactoryAware { log.debug(() -> String.format("Deleting all entities with the following ids: %s ", ids)); Statement statement = cypherGenerator.prepareDeleteOf(entityMetaData, condition); - ResultSummary summary = this.neo4jClient.query(renderer.render(statement)).in(getDatabaseName()).bind(ids) + ResultSummary summary = this.neo4jClient.query(renderer.render(statement)).in(getDatabaseName()).bind(convertIdValues(ids)) .to(nameOfParameter).run(); log.debug(() -> String.format("Deleted %d nodes and %d relationships.", summary.counters().nodesDeleted(), diff --git a/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java b/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java index 05e5219b2..8dd3ab0d5 100644 --- a/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java +++ b/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java @@ -329,7 +329,7 @@ public final class ReactiveNeo4jTemplate implements ReactiveNeo4jOperations, Bea Statement statement = cypherGenerator.prepareDeleteOf(entityMetaData, condition); return getDatabaseName().flatMap(databaseName -> this.neo4jClient.query(() -> renderer.render(statement)) - .in(databaseName.getValue()).bind(ids).to(nameOfParameter).run().then()); + .in(databaseName.getValue()).bind(convertIdValues(ids)).to(nameOfParameter).run().then()); } @Override @@ -343,7 +343,7 @@ public final class ReactiveNeo4jTemplate implements ReactiveNeo4jOperations, Bea Statement statement = cypherGenerator.prepareDeleteOf(entityMetaData, condition); return getDatabaseName().flatMap(databaseName -> this.neo4jClient.query(() -> renderer.render(statement)) - .in(databaseName.getValue()).bind(id).to(nameOfParameter).run().then()); + .in(databaseName.getValue()).bind(convertIdValues(id)).to(nameOfParameter).run().then()); } @Override diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jPersistentProperty.java b/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jPersistentProperty.java index 246860438..ad555354e 100644 --- a/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jPersistentProperty.java +++ b/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jPersistentProperty.java @@ -172,7 +172,7 @@ class DefaultNeo4jPersistentProperty extends AnnotationBasedPersistentProperty people = neo4jOperations.findAll(statement, singletonMap("name", TEST_PERSON1_NAME), - PersonWithAllConstructor.class); + List people = neo4jOperations + .findAll(statement, singletonMap("name", TEST_PERSON1_NAME), + PersonWithAllConstructor.class); assertThat(people).hasSize(1); } @@ -276,15 +289,67 @@ class Neo4jOperationsIT { } } + TransactionWork createPersonWithCustomId(PersonWithCustomId.PersonId assignedId) { + + return tx -> tx.run("CREATE (n:PersonWithCustomId) SET n.id = $id ", + Values.parameters("id", assignedId.getId())).consume(); + } + + @Test + void deleteByCustomId() { + + PersonWithCustomId.PersonId id = new PersonWithCustomId.PersonId(customIdValueGenerator.incrementAndGet()); + try (Session session = driver.session(getSessionConfig())) { + session.writeTransaction(createPersonWithCustomId(id)); + } + + assertThat(neo4jOperations.count(PersonWithCustomId.class)).isEqualTo(1L); + neo4jOperations.deleteById(id, PersonWithCustomId.class); + + try (Session session = driver.session(getSessionConfig())) { + Result result = session.run("MATCH (p:PersonWithCustomId) return count(p) as count"); + assertThat(result.single().get("count").asLong()).isEqualTo(0); + } + } + + @Test + void deleteAllByCustomId() { + + List ids = Stream.generate(customIdValueGenerator::incrementAndGet) + .map(PersonWithCustomId.PersonId::new) + .limit(2) + .collect(Collectors.toList()); + try ( + Session session = driver.session(getSessionConfig()); + ) { + ids.forEach(id -> session.writeTransaction(createPersonWithCustomId(id))); + } + + assertThat(neo4jOperations.count(PersonWithCustomId.class)).isEqualTo(2L); + neo4jOperations.deleteAllById(ids, PersonWithCustomId.class); + + try (Session session = driver.session(getSessionConfig())) { + Result result = session.run("MATCH (p:PersonWithCustomId) return count(p) as count"); + assertThat(result.single().get("count").asLong()).isEqualTo(0); + } + } + @Configuration @EnableTransactionManagement static class Config extends AbstractNeo4jConfig { @Bean + @Override public Driver driver() { return neo4jConnectionSupport.getDriver(); } + @Bean + @Override + public Neo4jConversions neo4jConversions() { + return new Neo4jConversions(singletonList(new PersonWithCustomId.CustomPersonIdConverter())); + } + @Override // needed here because there is no implicit registration of entities upfront some methods under test protected Collection getMappingBasePackages() { return singletonList(PersonWithAllConstructor.class.getPackage().getName()); diff --git a/src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveNeo4jOperationsIT.java b/src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveNeo4jOperationsIT.java index 8c9ef20e0..643860e9e 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveNeo4jOperationsIT.java +++ b/src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveNeo4jOperationsIT.java @@ -28,7 +28,10 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicLong; import java.util.function.Function; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Tag; @@ -43,14 +46,18 @@ import org.neo4j.driver.Result; import org.neo4j.driver.Session; import org.neo4j.driver.SessionConfig; import org.neo4j.driver.Transaction; +import org.neo4j.driver.TransactionWork; import org.neo4j.driver.Value; import org.neo4j.driver.Values; +import org.neo4j.driver.summary.ResultSummary; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.data.neo4j.config.AbstractReactiveNeo4jConfig; import org.springframework.data.neo4j.core.ReactiveNeo4jOperations; +import org.springframework.data.neo4j.core.convert.Neo4jConversions; import org.springframework.data.neo4j.integration.shared.PersonWithAllConstructor; +import org.springframework.data.neo4j.integration.shared.PersonWithCustomId; import org.springframework.data.neo4j.integration.shared.ThingWithGeneratedId; import org.springframework.data.neo4j.test.Neo4jIntegrationTest; import org.springframework.data.neo4j.test.Neo4jExtension.*; @@ -71,6 +78,7 @@ class ReactiveNeo4jOperationsIT { private final Driver driver; private final ReactiveNeo4jOperations neo4jOperations; + private final AtomicLong customIdValueGenerator = new AtomicLong(); private Long person1Id; private Long person2Id; @@ -94,16 +102,19 @@ class ReactiveNeo4jOperationsIT { @BeforeEach void setupData() { - Transaction transaction = driver.session(getSessionConfig()).beginTransaction(); - transaction.run("MATCH (n) detach delete n"); + try ( + Session session = driver.session(getSessionConfig()); + Transaction transaction = session.beginTransaction(); + ) { + transaction.run("MATCH (n) detach delete n"); - person1Id = transaction.run("CREATE (n:PersonWithAllConstructor) SET n.name = $name RETURN id(n)", - Values.parameters("name", TEST_PERSON1_NAME)).next().get(0).asLong(); - person2Id = transaction.run("CREATE (n:PersonWithAllConstructor) SET n.name = $name RETURN id(n)", - Values.parameters("name", TEST_PERSON2_NAME)).next().get(0).asLong(); + person1Id = transaction.run("CREATE (n:PersonWithAllConstructor) SET n.name = $name RETURN id(n) AS id", + Values.parameters("name", TEST_PERSON1_NAME)).single().get("id").asLong(); + person2Id = transaction.run("CREATE (n:PersonWithAllConstructor) SET n.name = $name RETURN id(n) AS id", + Values.parameters("name", TEST_PERSON2_NAME)).single().get("id").asLong(); - transaction.commit(); - transaction.close(); + transaction.commit(); + } } @Test @@ -265,8 +276,36 @@ class ReactiveNeo4jOperationsIT { } } + TransactionWork createPersonWithCustomId(PersonWithCustomId.PersonId assignedId) { + + return tx -> tx.run("CREATE (n:PersonWithCustomId) SET n.id = $id ", + Values.parameters("id", assignedId.getId())).consume(); + } + + @Test + void deleteByCustomId() { + + PersonWithCustomId.PersonId id = new PersonWithCustomId.PersonId(customIdValueGenerator.incrementAndGet()); + try (Session session = driver.session(getSessionConfig())) { + session.writeTransaction(createPersonWithCustomId(id)); + } + + StepVerifier.create(neo4jOperations.count(PersonWithCustomId.class)) + .expectNext(1L) + .verifyComplete(); + + StepVerifier.create(neo4jOperations.deleteById(id, PersonWithCustomId.class)) + .verifyComplete(); + + try (Session session = driver.session(getSessionConfig())) { + Result result = session.run("MATCH (p:PersonWithCustomId) return count(p) as count"); + assertThat(result.single().get("count").asLong()).isEqualTo(0); + } + } + @Test void deleteAllById() { + StepVerifier .create(neo4jOperations.deleteAllById(Arrays.asList(person1Id, person2Id), PersonWithAllConstructor.class)) .verifyComplete(); @@ -277,6 +316,32 @@ class ReactiveNeo4jOperationsIT { } } + @Test + void deleteAllByCustomId() { + + List ids = Stream.generate(customIdValueGenerator::incrementAndGet) + .map(PersonWithCustomId.PersonId::new) + .limit(2) + .collect(Collectors.toList()); + try ( + Session session = driver.session(getSessionConfig()); + ) { + ids.forEach(id -> session.writeTransaction(createPersonWithCustomId(id))); + } + + StepVerifier.create(neo4jOperations.count(PersonWithCustomId.class)) + .expectNext(2L) + .verifyComplete(); + + StepVerifier.create(neo4jOperations.deleteAllById(ids, PersonWithCustomId.class)) + .verifyComplete(); + + try (Session session = driver.session(getSessionConfig())) { + Result result = session.run("MATCH (p:PersonWithCustomId) return count(p) as count"); + assertThat(result.single().get("count").asLong()).isEqualTo(0); + } + } + @Configuration @EnableTransactionManagement static class Config extends AbstractReactiveNeo4jConfig { @@ -286,6 +351,12 @@ class ReactiveNeo4jOperationsIT { return neo4jConnectionSupport.getDriver(); } + @Bean + @Override + public Neo4jConversions neo4jConversions() { + return new Neo4jConversions(singletonList(new PersonWithCustomId.CustomPersonIdConverter())); + } + @Override // needed here because there is no implicit registration of entities upfront some methods under test protected Collection getMappingBasePackages() { return singletonList(PersonWithAllConstructor.class.getPackage().getName()); diff --git a/src/test/java/org/springframework/data/neo4j/integration/shared/PersonWithCustomId.java b/src/test/java/org/springframework/data/neo4j/integration/shared/PersonWithCustomId.java new file mode 100644 index 000000000..08aa3bbb1 --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/shared/PersonWithCustomId.java @@ -0,0 +1,79 @@ +/* + * Copyright 2011-2020 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.neo4j.integration.shared; + +import lombok.Value; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + +import org.neo4j.driver.Values; +import org.springframework.core.convert.TypeDescriptor; +import org.springframework.core.convert.converter.GenericConverter; +import org.springframework.data.neo4j.core.schema.Id; + +/** + * @author Rosetta Roberts + * @author Michael J. Simons + */ +@Value +public class PersonWithCustomId { + + /** + * Custom ID type for a person object. + */ + @Value + public static class PersonId { + + // Be aware that this is not the native (aka generated) Neo4j id. + // Natively generated IDs are only possible directly on a long field. + // This is an assigned id. + private final Long id; + } + + /** + * Converted needed to deal with the above custom type. Without that converter, an association would be assumed. + */ + public static class CustomPersonIdConverter implements GenericConverter { + + @Override + public Set getConvertibleTypes() { + return new HashSet<>(Arrays.asList( + new ConvertiblePair(PersonId.class, org.neo4j.driver.Value.class), + new ConvertiblePair(org.neo4j.driver.Value.class, PersonId.class) + )); + } + + @Override + public Object convert(Object o, TypeDescriptor type1, TypeDescriptor type2) { + if (o == null) { + return null; + } + + if (PersonId.class.isAssignableFrom(type1.getType())) { + return Values.value(((PersonId) o).getId()); + } else { + return new PersonId(((org.neo4j.driver.Value) o).asLong()); + } + } + } + + @Id + private final PersonId id; + + private final String name; +}