From f7b07ed9636baf9acbd62ad47be83a5b9b34bb9d Mon Sep 17 00:00:00 2001 From: Michael Simons Date: Mon, 25 Jul 2022 14:20:13 +0200 Subject: [PATCH] GH-2572 - Filter out records that could not be mapped to allow Cypher `OPTIONAL` returns. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes #2572 and is a tough call: The query presented there is totally valid but returns not an empty result but a record containing only one `null` element due to the way the Cyper `OPTIONAL` keyword works. We could try parsing the query, but this will be an endless rabbit whole in the future to come. The approach here is to check now whether a result contains exactly one record with one `NULL` value. If that is the case, no exception is thrown but `null` returned, which is later filtered on the clients. That however drops the checking for non-null values there, so that methods don’t throw in those cases but returns empty optionals or empty / smaller lists. --- .../data/neo4j/core/DefaultNeo4jClient.java | 7 +-- .../core/DefaultReactiveNeo4jClient.java | 2 +- ...elegatingMappingFunctionWithNullCheck.java | 52 ------------------- .../data/neo4j/core/Neo4jClient.java | 1 + .../data/neo4j/core/ReactiveNeo4jClient.java | 1 + .../mapping/DefaultNeo4jEntityConverter.java | 31 ++++++----- .../mapping/NoRootNodeMappingException.java | 24 +++++++-- ...atingMappingFunctionWithNullCheckTest.java | 45 ---------------- .../data/neo4j/core/Neo4jClientTest.java | 13 +++-- .../neo4j/core/ReactiveNeo4jClientTest.java | 2 +- .../neo4j/integration/issues/IssuesIT.java | 45 ++++++++++++++++ .../integration/issues/ReactiveIssuesIT.java | 40 ++++++++++++++ .../neo4j/integration/issues/TestBase.java | 5 ++ .../issues/gh2572/GH2572BaseEntity.java | 30 +++++++++++ .../issues/gh2572/GH2572Child.java | 39 ++++++++++++++ .../issues/gh2572/GH2572Parent.java | 38 ++++++++++++++ .../issues/gh2572/GH2572Repository.java | 43 +++++++++++++++ .../integration/issues/gh2572/MyStrategy.java | 35 +++++++++++++ .../gh2572/ReactiveGH2572Repository.java | 38 ++++++++++++++ 19 files changed, 369 insertions(+), 122 deletions(-) delete mode 100644 src/main/java/org/springframework/data/neo4j/core/DelegatingMappingFunctionWithNullCheck.java delete mode 100644 src/test/java/org/springframework/data/neo4j/core/DelegatingMappingFunctionWithNullCheckTest.java create mode 100644 src/test/java/org/springframework/data/neo4j/integration/issues/gh2572/GH2572BaseEntity.java create mode 100644 src/test/java/org/springframework/data/neo4j/integration/issues/gh2572/GH2572Child.java create mode 100644 src/test/java/org/springframework/data/neo4j/integration/issues/gh2572/GH2572Parent.java create mode 100644 src/test/java/org/springframework/data/neo4j/integration/issues/gh2572/GH2572Repository.java create mode 100644 src/test/java/org/springframework/data/neo4j/integration/issues/gh2572/MyStrategy.java create mode 100644 src/test/java/org/springframework/data/neo4j/integration/issues/gh2572/ReactiveGH2572Repository.java diff --git a/src/main/java/org/springframework/data/neo4j/core/DefaultNeo4jClient.java b/src/main/java/org/springframework/data/neo4j/core/DefaultNeo4jClient.java index e42d61cd3..eda806568 100644 --- a/src/main/java/org/springframework/data/neo4j/core/DefaultNeo4jClient.java +++ b/src/main/java/org/springframework/data/neo4j/core/DefaultNeo4jClient.java @@ -19,6 +19,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.concurrent.locks.ReentrantReadWriteLock; @@ -442,7 +443,7 @@ final class DefaultNeo4jClient implements Neo4jClient { public RecordFetchSpec mappedBy( @SuppressWarnings("HiddenField") BiFunction mappingFunction) { - this.mappingFunction = new DelegatingMappingFunctionWithNullCheck<>(mappingFunction); + this.mappingFunction = mappingFunction; return this; } @@ -468,7 +469,7 @@ final class DefaultNeo4jClient implements Neo4jClient { try (QueryRunner statementRunner = getQueryRunner(this.databaseSelection, this.impersonatedUser)) { Result result = runnableStatement.runWith(statementRunner); - Optional optionalValue = result.stream().map(partialMappingFunction(typeSystem)).findFirst(); + Optional optionalValue = result.stream().map(partialMappingFunction(typeSystem)).filter(Objects::nonNull).findFirst(); ResultSummaries.process(result.consume()); return optionalValue; } catch (RuntimeException e) { @@ -483,7 +484,7 @@ final class DefaultNeo4jClient implements Neo4jClient { try (QueryRunner statementRunner = getQueryRunner(this.databaseSelection, this.impersonatedUser)) { Result result = runnableStatement.runWith(statementRunner); - Collection values = result.stream().map(partialMappingFunction(typeSystem)).collect(Collectors.toList()); + Collection values = result.stream().map(partialMappingFunction(typeSystem)).filter(Objects::nonNull).collect(Collectors.toList()); ResultSummaries.process(result.consume()); return values; } catch (RuntimeException e) { diff --git a/src/main/java/org/springframework/data/neo4j/core/DefaultReactiveNeo4jClient.java b/src/main/java/org/springframework/data/neo4j/core/DefaultReactiveNeo4jClient.java index e00f2ab99..3efd9e4d9 100644 --- a/src/main/java/org/springframework/data/neo4j/core/DefaultReactiveNeo4jClient.java +++ b/src/main/java/org/springframework/data/neo4j/core/DefaultReactiveNeo4jClient.java @@ -398,7 +398,7 @@ final class DefaultReactiveNeo4jClient implements ReactiveNeo4jClient { @Override public RecordFetchSpec mappedBy(@SuppressWarnings("HiddenField") BiFunction mappingFunction) { - this.mappingFunction = new DelegatingMappingFunctionWithNullCheck<>(mappingFunction); + this.mappingFunction = mappingFunction; return this; } diff --git a/src/main/java/org/springframework/data/neo4j/core/DelegatingMappingFunctionWithNullCheck.java b/src/main/java/org/springframework/data/neo4j/core/DelegatingMappingFunctionWithNullCheck.java deleted file mode 100644 index ba2e7360c..000000000 --- a/src/main/java/org/springframework/data/neo4j/core/DelegatingMappingFunctionWithNullCheck.java +++ /dev/null @@ -1,52 +0,0 @@ -/* - * Copyright 2011-2022 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.core; - -import java.util.function.BiFunction; - -import org.neo4j.driver.Record; -import org.neo4j.driver.types.TypeSystem; - -/** - * A delegating mapping function that first calls the delegate to get a record map and then checks the returned value - * for {@literal null} and in the case of a null value, an {@link IllegalStateException} will be thrown. - *

- * This class has been introduced instead of {@code Function#andThen} notion to be able to throw a decent exception - * containing some information about the delegate used and which record was problematic. - * - * @author Michael J. Simons - * @param The expected type of this function - * @soundtrack Manowar - Fighting The World - * @since 6.0 - */ -class DelegatingMappingFunctionWithNullCheck implements BiFunction { - - BiFunction delegate; - - DelegatingMappingFunctionWithNullCheck(BiFunction delegate) { - this.delegate = delegate; - } - - @Override - public T apply(TypeSystem typeSystem, Record record) { - T t = delegate.apply(typeSystem, record); - if (t == null) { - throw new IllegalStateException( - "Mapping function " + delegate + " returned illegal null value for record " + record); - } - return t; - } -} diff --git a/src/main/java/org/springframework/data/neo4j/core/Neo4jClient.java b/src/main/java/org/springframework/data/neo4j/core/Neo4jClient.java index 11cb44359..f9ecb9761 100644 --- a/src/main/java/org/springframework/data/neo4j/core/Neo4jClient.java +++ b/src/main/java/org/springframework/data/neo4j/core/Neo4jClient.java @@ -43,6 +43,7 @@ import org.springframework.lang.Nullable; public interface Neo4jClient { LogAccessor cypherLog = new LogAccessor(LogFactory.getLog("org.springframework.data.neo4j.cypher")); + LogAccessor log = new LogAccessor(LogFactory.getLog(Neo4jClient.class)); static Neo4jClient create(Driver driver) { diff --git a/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jClient.java b/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jClient.java index 0baf42e78..6f4ece98f 100644 --- a/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jClient.java +++ b/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jClient.java @@ -46,6 +46,7 @@ import org.springframework.lang.Nullable; public interface ReactiveNeo4jClient { LogAccessor cypherLog = new LogAccessor(LogFactory.getLog("org.springframework.data.neo4j.cypher")); + LogAccessor log = new LogAccessor(LogFactory.getLog(ReactiveNeo4jClient.class)); static ReactiveNeo4jClient create(Driver driver) { diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jEntityConverter.java b/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jEntityConverter.java index d22861029..f90d77d3e 100644 --- a/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jEntityConverter.java +++ b/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jEntityConverter.java @@ -104,6 +104,7 @@ final class DefaultNeo4jEntityConverter implements Neo4jEntityConverter { } @Override + @Nullable public R read(Class targetType, MapAccessor mapAccessor) { knownObjects.nextRecord(); @@ -112,12 +113,9 @@ final class DefaultNeo4jEntityConverter implements Neo4jEntityConverter { @SuppressWarnings("unchecked") // ¯\_(ツ)_/¯ Neo4jPersistentEntity rootNodeDescription = (Neo4jPersistentEntity) nodeDescriptionStore.getNodeDescription(targetType); MapAccessor queryRoot = determineQueryRoot(mapAccessor, rootNodeDescription); - if (queryRoot == null) { - throw new NoRootNodeMappingException(String.format("Could not find mappable nodes or relationships inside %s for %s", mapAccessor, rootNodeDescription)); - } try { - return map(queryRoot, queryRoot, rootNodeDescription); + return queryRoot == null ? null : map(queryRoot, queryRoot, rootNodeDescription); } catch (Exception e) { throw new MappingException("Error mapping " + mapAccessor, e); } @@ -157,26 +155,35 @@ final class DefaultNeo4jEntityConverter implements Neo4jEntityConverter { // Prefer the candidates over candidates previously seen List finalCandidates = matchingNodes.isEmpty() ? seenMatchingNodes : matchingNodes; - MapAccessor queryRoot = null; if (finalCandidates.size() > 1) { throw new MappingException("More than one matching node in the record"); } else if (!finalCandidates.isEmpty()) { if (mapAccessor.size() > 1) { - queryRoot = mergeRootNodeWithRecord(finalCandidates.get(0), mapAccessor); + return mergeRootNodeWithRecord(finalCandidates.get(0), mapAccessor); } else { - queryRoot = finalCandidates.get(0); + return finalCandidates.get(0); } } else { + int cnt = 0; + Value firstValue = Values.NULL; for (Value value : recordValues) { - if (value.hasType(mapType) && !(value.hasType(nodeType) || value.hasType( - relationshipType))) { - queryRoot = value; - break; + if (cnt == 0) { + firstValue = value; } + if (value.hasType(mapType) && !(value.hasType(nodeType) || value.hasType(relationshipType))) { + return value; + } + ++cnt; + } + + // Cater for results that have one single, null column. This is the case for MATCH (x) OPTIONAL MATCH (something) RETURN something + if (cnt == 1 && firstValue.isNull()) { + return null; } } - return queryRoot; + + throw new NoRootNodeMappingException(mapAccessor, rootNodeDescription); } private Collection createDynamicLabelsProperty(TypeInformation type, Collection dynamicLabels) { diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/NoRootNodeMappingException.java b/src/main/java/org/springframework/data/neo4j/core/mapping/NoRootNodeMappingException.java index e85a7338d..dab80959f 100644 --- a/src/main/java/org/springframework/data/neo4j/core/mapping/NoRootNodeMappingException.java +++ b/src/main/java/org/springframework/data/neo4j/core/mapping/NoRootNodeMappingException.java @@ -15,7 +15,12 @@ */ package org.springframework.data.neo4j.core.mapping; +import java.util.Formattable; +import java.util.Formatter; +import java.util.Locale; + import org.apiguardian.api.API; +import org.neo4j.driver.types.MapAccessor; import org.springframework.data.mapping.MappingException; /** @@ -29,9 +34,22 @@ import org.springframework.data.mapping.MappingException; * @since 6.0.2 */ @API(status = API.Status.INTERNAL, since = "6.0.2") -public final class NoRootNodeMappingException extends MappingException { +public final class NoRootNodeMappingException extends MappingException implements Formattable { - public NoRootNodeMappingException(String s) { - super(s); + private MapAccessor mapAccessor; + private Neo4jPersistentEntity entity; + + public NoRootNodeMappingException(MapAccessor mapAccessor, Neo4jPersistentEntity entity) { + super(String.format("Could not find mappable nodes or relationships inside %s for %s", mapAccessor, entity)); + this.mapAccessor = mapAccessor; + this.entity = entity; + } + + @Override + public void formatTo(Formatter formatter, int flags, int width, int precision) { + String className = entity.getUnderlyingClass().getSimpleName(); + formatter.format("Could not find mappable nodes or relationships inside %s for %s:%s", mapAccessor, + className.substring(0, 1).toLowerCase( + Locale.ROOT), String.join(":", entity.getStaticLabels())); } } diff --git a/src/test/java/org/springframework/data/neo4j/core/DelegatingMappingFunctionWithNullCheckTest.java b/src/test/java/org/springframework/data/neo4j/core/DelegatingMappingFunctionWithNullCheckTest.java deleted file mode 100644 index 5de92e463..000000000 --- a/src/test/java/org/springframework/data/neo4j/core/DelegatingMappingFunctionWithNullCheckTest.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright 2011-2022 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.core; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatIllegalStateException; -import static org.mockito.Mockito.mock; - -import org.junit.jupiter.api.Test; -import org.neo4j.driver.Record; -import org.neo4j.driver.types.TypeSystem; - -/** - * @author Michael J. Simons - */ -class DelegatingMappingFunctionWithNullCheckTest { - - @Test - void shouldBeHappyWithNonNullValues() { - DelegatingMappingFunctionWithNullCheck function = new DelegatingMappingFunctionWithNullCheck( - (typeSystem, record) -> "Tada."); - assertThat(function.apply(mock(TypeSystem.class), mock(Record.class))).isEqualTo("Tada."); - } - - @Test - void shouldThrowExceptions() { - DelegatingMappingFunctionWithNullCheck function = new DelegatingMappingFunctionWithNullCheck( - (typeSystem, record) -> null); - assertThatIllegalStateException().isThrownBy(() -> function.apply(mock(TypeSystem.class), mock(Record.class))) - .withMessageMatching("Mapping function .* returned illegal null value for record .*"); - } -} diff --git a/src/test/java/org/springframework/data/neo4j/core/Neo4jClientTest.java b/src/test/java/org/springframework/data/neo4j/core/Neo4jClientTest.java index b5e6617e3..46a339a1a 100644 --- a/src/test/java/org/springframework/data/neo4j/core/Neo4jClientTest.java +++ b/src/test/java/org/springframework/data/neo4j/core/Neo4jClientTest.java @@ -17,7 +17,6 @@ package org.springframework.data.neo4j.core; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; -import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import static org.assertj.core.api.Assumptions.assumeThat; import static org.mockito.Mockito.any; import static org.mockito.Mockito.anyMap; @@ -438,23 +437,27 @@ class Neo4jClientTest { when(session.run(anyString(), anyMap())).thenReturn(result); when(result.stream()).thenReturn(Stream.of(record1, record2)); + when(result.consume()).thenReturn(resultSummary); when(record1.get("name")).thenReturn(Values.value("michael")); Neo4jClient client = Neo4jClient.create(driver); - assertThatIllegalStateException() - .isThrownBy(() -> client.query("MATCH (n) RETURN n").fetchAs(BikeOwner.class).mappedBy((t, r) -> { + Collection owners = client.query("MATCH (n) RETURN n").fetchAs(BikeOwner.class) + .mappedBy((t, r) -> { if (r == record1) { return new BikeOwner(r.get("name").asString(), Collections.emptyList()); } else { return null; } - }).all()); - + }).all(); + assertThat(owners).hasSize(1); verifyDatabaseSelection(null); verify(session).run(eq("MATCH (n) RETURN n"), MockitoHamcrest.argThat(new MapAssertionMatcher(Collections.emptyMap()))); verify(result).stream(); + verify(result).consume(); + verify(resultSummary).notifications(); + verify(resultSummary).hasPlan(); verify(record1).get("name"); verify(session).close(); } diff --git a/src/test/java/org/springframework/data/neo4j/core/ReactiveNeo4jClientTest.java b/src/test/java/org/springframework/data/neo4j/core/ReactiveNeo4jClientTest.java index 404243aba..266a36467 100644 --- a/src/test/java/org/springframework/data/neo4j/core/ReactiveNeo4jClientTest.java +++ b/src/test/java/org/springframework/data/neo4j/core/ReactiveNeo4jClientTest.java @@ -453,7 +453,7 @@ class ReactiveNeo4jClientTest { } }).all(); - StepVerifier.create(bikeOwners).expectNextCount(1).verifyError(); + StepVerifier.create(bikeOwners).expectNextCount(1).verifyComplete(); verifyDatabaseSelection(null); diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/IssuesIT.java b/src/test/java/org/springframework/data/neo4j/integration/issues/IssuesIT.java index 0eece5b95..b5872f72a 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/issues/IssuesIT.java +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/IssuesIT.java @@ -113,6 +113,8 @@ import org.springframework.data.neo4j.integration.issues.gh2533.EntitiesAndProje import org.springframework.data.neo4j.integration.issues.gh2533.GH2533Repository; import org.springframework.data.neo4j.integration.issues.gh2542.TestNode; import org.springframework.data.neo4j.integration.issues.gh2542.TestNodeRepository; +import org.springframework.data.neo4j.integration.issues.gh2572.GH2572Repository; +import org.springframework.data.neo4j.integration.issues.gh2572.GH2572Child; import org.springframework.data.neo4j.integration.misc.ConcreteImplementationTwo; import org.springframework.data.neo4j.repository.config.EnableNeo4jRepositories; import org.springframework.data.neo4j.repository.query.QueryFragmentsAndParameters; @@ -158,6 +160,7 @@ class IssuesIT extends TestBase { setupGH2323(transaction); setupGH2328(transaction); setupGH2459(transaction); + setupGH2572(transaction); transaction.commit(); } @@ -789,6 +792,48 @@ class IssuesIT extends TestBase { .isThrownBy(() -> repository.save(secondNode)); } + @Test + @Tag("GH-2572") + void allShouldFetchCorrectNumberOfChildNodes(@Autowired GH2572Repository GH2572Repository) { + List dogsForPerson = GH2572Repository.getDogsForPerson("GH2572Parent-2"); + assertThat(dogsForPerson).hasSize(2); + } + + @Test + @Tag("GH-2572") + void allShouldNotFailWithoutMatchingRootNodes(@Autowired GH2572Repository GH2572Repository) { + List dogsForPerson = GH2572Repository.getDogsForPerson("GH2572Parent-1"); + assertThat(dogsForPerson).isEmpty(); + } + + @Test + @Tag("GH-2572") + void oneShouldFetchCorrectNumberOfChildNodes(@Autowired GH2572Repository GH2572Repository) { + Optional optionalChild = GH2572Repository.findOneDogForPerson("GH2572Parent-2"); + assertThat(optionalChild).map(GH2572Child::getName).hasValue("a-pet"); + } + + @Test + @Tag("GH-2572") + void oneShouldNotFailWithoutMatchingRootNodes(@Autowired GH2572Repository GH2572Repository) { + Optional optionalChild = GH2572Repository.findOneDogForPerson("GH2572Parent-1"); + assertThat(optionalChild).isEmpty(); + } + + @Test + @Tag("GH-2572") + void getOneShouldFetchCorrectNumberOfChildNodes(@Autowired GH2572Repository GH2572Repository) { + GH2572Child gh2572Child = GH2572Repository.getOneDogForPerson("GH2572Parent-2"); + assertThat(gh2572Child.getName()).isEqualTo("a-pet"); + } + + @Test + @Tag("GH-2572") + void getOneShouldNotFailWithoutMatchingRootNodes(@Autowired GH2572Repository GH2572Repository) { + GH2572Child gh2572Child = GH2572Repository.getOneDogForPerson("GH2572Parent-1"); + assertThat(gh2572Child).isNull(); + } + @Configuration @EnableTransactionManagement @EnableNeo4jRepositories(namedQueriesLocation = "more-custom-queries.properties") diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/ReactiveIssuesIT.java b/src/test/java/org/springframework/data/neo4j/integration/issues/ReactiveIssuesIT.java index 6728d1986..1f61f6f6c 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/issues/ReactiveIssuesIT.java +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/ReactiveIssuesIT.java @@ -62,6 +62,8 @@ import org.springframework.data.neo4j.integration.issues.gh2500.Device; import org.springframework.data.neo4j.integration.issues.gh2500.Group; import org.springframework.data.neo4j.integration.issues.gh2533.EntitiesAndProjections; import org.springframework.data.neo4j.integration.issues.gh2533.ReactiveGH2533Repository; +import org.springframework.data.neo4j.integration.issues.gh2572.GH2572Child; +import org.springframework.data.neo4j.integration.issues.gh2572.ReactiveGH2572Repository; import org.springframework.data.neo4j.repository.config.EnableReactiveNeo4jRepositories; import org.springframework.data.neo4j.test.BookmarkCapture; import org.springframework.data.neo4j.test.Neo4jIntegrationTest; @@ -86,6 +88,7 @@ class ReactiveIssuesIT extends TestBase { setupGH2289(transaction); setupGH2328(transaction); + setupGH2572(transaction); transaction.commit(); } @@ -374,6 +377,43 @@ class ReactiveIssuesIT extends TestBase { .verifyComplete(); } + @Test + @Tag("GH-2572") + void allShouldFetchCorrectNumberOfChildNodes(@Autowired ReactiveGH2572Repository reactiveGH2572Repository) { + reactiveGH2572Repository.getDogsForPerson("GH2572Parent-2") + .as(StepVerifier::create) + .expectNextCount(2L) + .verifyComplete(); + } + + @Test + @Tag("GH-2572") + void allShouldNotFailWithoutMatchingRootNodes(@Autowired ReactiveGH2572Repository reactiveGH2572Repository) { + reactiveGH2572Repository.getDogsForPerson("GH2572Parent-1") + .as(StepVerifier::create) + .expectNextCount(0L) + .verifyComplete(); + } + + @Test + @Tag("GH-2572") + void oneShouldFetchCorrectNumberOfChildNodes(@Autowired ReactiveGH2572Repository reactiveGH2572Repository) { + reactiveGH2572Repository.findOneDogForPerson("GH2572Parent-2") + .map(GH2572Child::getName) + .as(StepVerifier::create) + .expectNext("a-pet") + .verifyComplete(); + } + + @Test + @Tag("GH-2572") + void oneShouldNotFailWithoutMatchingRootNodes(@Autowired ReactiveGH2572Repository reactiveGH2572Repository) { + reactiveGH2572Repository.findOneDogForPerson("GH2572Parent-1") + .as(StepVerifier::create) + .expectNextCount(0L) + .verifyComplete(); + } + @Configuration @EnableTransactionManagement @EnableReactiveNeo4jRepositories(namedQueriesLocation = "more-custom-queries.properties") diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/TestBase.java b/src/test/java/org/springframework/data/neo4j/integration/issues/TestBase.java index f58ffe6ea..d1973861a 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/issues/TestBase.java +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/TestBase.java @@ -112,6 +112,11 @@ abstract class TestBase { .get(0).asString()); } + protected static void setupGH2572(QueryRunner queryRunner) { + queryRunner.run("CREATE (p:GH2572Parent {id: 'GH2572Parent-1', name:'no-pets'})"); + queryRunner.run("CREATE (p:GH2572Parent {id: 'GH2572Parent-2', name:'one-pet'}) <-[:IS_PET]- (:GH2572Child {id: 'GH2572Child-3', name: 'a-pet'})"); + queryRunner.run("MATCH (p:GH2572Parent {id: 'GH2572Parent-2'}) CREATE (p) <-[:IS_PET]- (:GH2572Child {id: 'GH2572Child-4', name: 'another-pet'})"); + } protected static void assertLabels(BookmarkCapture bookmarkCapture, List ids) { try (Session session = neo4jConnectionSupport.getDriver().session(bookmarkCapture.createSessionConfig())) { diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/gh2572/GH2572BaseEntity.java b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2572/GH2572BaseEntity.java new file mode 100644 index 000000000..c94c4c2a1 --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2572/GH2572BaseEntity.java @@ -0,0 +1,30 @@ +/* + * Copyright 2011-2022 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.issues.gh2572; + +import org.springframework.data.neo4j.core.schema.GeneratedValue; +import org.springframework.data.neo4j.core.schema.Id; + +/** + * @author Michael J. Simons + * @param The concrete type + */ +abstract class GH2572BaseEntity> { + + @Id + @GeneratedValue(value = MyStrategy.class) + protected String id; +} diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/gh2572/GH2572Child.java b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2572/GH2572Child.java new file mode 100644 index 000000000..9bfc1ae4f --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2572/GH2572Child.java @@ -0,0 +1,39 @@ +/* + * Copyright 2011-2022 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.issues.gh2572; + +import lombok.AllArgsConstructor; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.Setter; + +import org.springframework.data.neo4j.core.schema.Node; +import org.springframework.data.neo4j.core.schema.Relationship; + +/** + * @author Michael J. Simons + */ +@Getter +@Setter +@NoArgsConstructor +@AllArgsConstructor +@Node +public class GH2572Child extends GH2572BaseEntity { + private String name; + + @Relationship(value = "IS_PET", direction = Relationship.Direction.OUTGOING) + private GH2572Parent owner; +} diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/gh2572/GH2572Parent.java b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2572/GH2572Parent.java new file mode 100644 index 000000000..ebac28d09 --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2572/GH2572Parent.java @@ -0,0 +1,38 @@ +/* + * Copyright 2011-2022 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.issues.gh2572; + +import lombok.AllArgsConstructor; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.Setter; + +import org.springframework.data.neo4j.core.schema.Node; + +/** + * @author Michael J. Simons + */ +@Getter +@Setter +@NoArgsConstructor +@AllArgsConstructor +@Node +public class GH2572Parent extends GH2572BaseEntity { + + private String name; + + private int age; +} diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/gh2572/GH2572Repository.java b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2572/GH2572Repository.java new file mode 100644 index 000000000..491a3de28 --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2572/GH2572Repository.java @@ -0,0 +1,43 @@ +/* + * Copyright 2011-2022 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.issues.gh2572; + +import java.util.List; +import java.util.Optional; + +import org.springframework.data.neo4j.repository.Neo4jRepository; +import org.springframework.data.neo4j.repository.query.Query; + +/** + * @author Michael J. Simons + */ +public interface GH2572Repository extends Neo4jRepository { + + @Query("MATCH(person:GH2572Parent {id: $id}) " + + "OPTIONAL MATCH (person)<-[:IS_PET]-(dog:GH2572Child) " + + "RETURN dog") + List getDogsForPerson(String id); + + @Query("MATCH(person:GH2572Parent {id: $id}) " + + "OPTIONAL MATCH (person)<-[:IS_PET]-(dog:GH2572Child) " + + "RETURN dog ORDER BY dog.name ASC LIMIT 1") + Optional findOneDogForPerson(String id); + + @Query("MATCH(person:GH2572Parent {id: $id}) " + + "OPTIONAL MATCH (person)<-[:IS_PET]-(dog:GH2572Child) " + + "RETURN dog ORDER BY dog.name ASC LIMIT 1") + GH2572Child getOneDogForPerson(String id); +} diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/gh2572/MyStrategy.java b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2572/MyStrategy.java new file mode 100644 index 000000000..a69a32720 --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2572/MyStrategy.java @@ -0,0 +1,35 @@ +/* + * Copyright 2011-2022 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.issues.gh2572; + +import java.util.concurrent.atomic.AtomicInteger; + +import org.springframework.data.neo4j.core.schema.IdGenerator; + +/** + * Not actually needed during the test, but added to recreate the issue scenario as much as possible. + * + * @author Michael J. Simons + */ +public final class MyStrategy implements IdGenerator { + + private final AtomicInteger sequence = new AtomicInteger(10); + + @Override + public String generateId(String primaryLabel, Object entity) { + return primaryLabel + sequence.incrementAndGet(); + } +} diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/gh2572/ReactiveGH2572Repository.java b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2572/ReactiveGH2572Repository.java new file mode 100644 index 000000000..257b053c7 --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2572/ReactiveGH2572Repository.java @@ -0,0 +1,38 @@ +/* + * Copyright 2011-2022 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.issues.gh2572; + +import reactor.core.publisher.Flux; +import reactor.core.publisher.Mono; + +import org.springframework.data.neo4j.repository.ReactiveNeo4jRepository; +import org.springframework.data.neo4j.repository.query.Query; + +/** + * @author Michael J. Simons + */ +public interface ReactiveGH2572Repository extends ReactiveNeo4jRepository { + + @Query("MATCH(person:GH2572Parent {id: $id}) " + + "OPTIONAL MATCH (person)<-[:IS_PET]-(dog:GH2572Child) " + + "RETURN dog") + Flux getDogsForPerson(String id); + + @Query("MATCH(person:GH2572Parent {id: $id}) " + + "OPTIONAL MATCH (person)<-[:IS_PET]-(dog:GH2572Child) " + + "RETURN dog ORDER BY dog.name ASC LIMIT 1") + Mono findOneDogForPerson(String id); +}