Skip to content

Commit

Permalink
GH-2728 - Check current Cypher-DSL dialect before issuing queries.
Browse files Browse the repository at this point in the history
Make sure we ignore invalid dialect config (i.e. we actually have neo4j 5, but are on 4 dialect in a crucial mapping path.
  • Loading branch information
meistermeier committed May 30, 2023
1 parent 4dbbb0b commit d890a65
Show file tree
Hide file tree
Showing 14 changed files with 557 additions and 4 deletions.
4 changes: 2 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
<parent>
<groupId>org.springframework.data.build</groupId>
<artifactId>spring-data-parent</artifactId>
<version>3.1.1-SNAPSHOT</version>
<version>3.1.0</version>
</parent>

<groupId>org.springframework.data</groupId>
Expand Down Expand Up @@ -115,7 +115,7 @@
<skipUnitTests>${skipTests}</skipUnitTests>

<spring-data-commons-docs.dir>../../../../spring-data-commons/src/main/asciidoc</spring-data-commons-docs.dir>
<springdata.commons>3.1.1-SNAPSHOT</springdata.commons>
<springdata.commons>3.1.0</springdata.commons>
</properties>

<dependencyManagement>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,7 @@ private <T> T processNestedRelations(
relatedInternalId = stateMachine.getObjectId(relatedValueToStore);
} else {
savedEntity = saveRelatedNode(newRelatedObject, targetEntity, includeProperty, currentPropertyPath);
relatedInternalId = IdentitySupport.getElementId(savedEntity);
relatedInternalId = TemplateSupport.rendererCanUseElementIdIfPresent(renderer) ? savedEntity.elementId() : Long.toString(savedEntity.id());
stateMachine.markEntityAsProcessed(relatedValueToStore, relatedInternalId);
if (relatedValueToStore instanceof MappingSupport.RelationshipPropertiesWithEntityHolder) {
Object entity = ((MappingSupport.RelationshipPropertiesWithEntityHolder) relatedValueToStore).getRelatedEntity();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -952,7 +952,7 @@ private <T> Mono<T> processNestedRelations(Neo4jPersistentEntity<?> sourceEntity
queryOrSave = Mono.just(Tuples.of(relatedInternalId, new AtomicReference<>()));
} else {
queryOrSave = saveRelatedNode(newRelatedObject, targetEntity, includeProperty, currentPropertyPath)
.map(entity -> Tuples.of(new AtomicReference<>(IdentitySupport.getElementId(entity)), new AtomicReference<>(entity)))
.map(entity -> Tuples.of(new AtomicReference<>(TemplateSupport.rendererCanUseElementIdIfPresent(renderer) ? entity.elementId() : Long.toString(entity.id())), new AtomicReference<>(entity)))
.doOnNext(t -> {
var relatedInternalId = t.getT1().get();
stateMachine.markEntityAsProcessed(relatedValueToStore, relatedInternalId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import org.neo4j.cypherdsl.core.Node;
import org.neo4j.cypherdsl.core.Relationship;
import org.neo4j.cypherdsl.core.Statement;
import org.neo4j.cypherdsl.core.renderer.Renderer;
import org.neo4j.driver.types.Entity;
import org.neo4j.driver.types.MapAccessor;
import org.neo4j.driver.types.TypeSystem;
Expand Down Expand Up @@ -410,6 +411,15 @@ static <T> String retrieveOrSetRelatedId(
return Objects.requireNonNull(relatedInternalId);
}

/**
* Checks if the renderer is configured in such a way that it will use element id or apply toString(id(n)) workaround.
* @return {@literal true} if renderer will use elementId
*/
static boolean rendererCanUseElementIdIfPresent(Renderer renderer) {
return renderer.render(Cypher.returning(Functions.elementId(Cypher.anyNode("n"))).build())
.equals("RETURN elementId(n)");
}

private TemplateSupport() {
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*
* Copyright 2011-2023 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.gh2728;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.neo4j.driver.Driver;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Bean;
import org.springframework.data.neo4j.core.ReactiveDatabaseSelectionProvider;
import org.springframework.data.neo4j.core.transaction.Neo4jBookmarkManager;
import org.springframework.data.neo4j.core.transaction.ReactiveNeo4jTransactionManager;
import org.springframework.data.neo4j.repository.ReactiveNeo4jRepository;
import org.springframework.data.neo4j.test.BookmarkCapture;
import org.springframework.data.neo4j.test.Neo4jExtension;
import org.springframework.data.neo4j.test.Neo4jIntegrationTest;
import org.springframework.data.neo4j.test.Neo4jReactiveTestConfiguration;
import org.springframework.transaction.ReactiveTransactionManager;

/**
* @author Michael J. Simons
*/
@Neo4jIntegrationTest
public abstract class AbstractReactiveTestBase {

protected static Neo4jExtension.Neo4jConnectionSupport neo4jConnectionSupport;

@Autowired
private TestEntityWithGeneratedDeprecatedId1Repository generatedDeprecatedIdRepository;

@Autowired
private TestEntityWithAssignedId1Repository assignedIdRepository;

@Test
public void testGeneratedDeprecatedIds() {
TestEntityWithGeneratedDeprecatedId2 t2 = new TestEntityWithGeneratedDeprecatedId2(null, "v2");
TestEntityWithGeneratedDeprecatedId1 t1 = new TestEntityWithGeneratedDeprecatedId1(null, "v1", t2);

TestEntityWithGeneratedDeprecatedId1 result = generatedDeprecatedIdRepository.save(t1).block();

TestEntityWithGeneratedDeprecatedId1 freshRetrieved = generatedDeprecatedIdRepository.findById(result.getId()).block();

Assertions.assertNotNull(result.getRelatedEntity());
Assertions.assertNotNull(freshRetrieved.getRelatedEntity());
}

/**
* This is a test to ensure if the fix for the failing test above will continue to work for
* assigned ids. For broader test cases please return false for isCypher5Compatible in (Reactive)RepositoryIT
*/
@Test
public void testAssignedIds() {
TestEntityWithAssignedId2 t2 = new TestEntityWithAssignedId2("second", "v2");
TestEntityWithAssignedId1 t1 = new TestEntityWithAssignedId1("first", "v1", t2);

TestEntityWithAssignedId1 result = assignedIdRepository.save(t1).block();

TestEntityWithAssignedId1 freshRetrieved = assignedIdRepository.findById(result.getAssignedId()).block();

Assertions.assertNotNull(result.getRelatedEntity());
Assertions.assertNotNull(freshRetrieved.getRelatedEntity());
}

interface TestEntityWithGeneratedDeprecatedId1Repository extends ReactiveNeo4jRepository<TestEntityWithGeneratedDeprecatedId1, Long> {
}

interface TestEntityWithAssignedId1Repository extends ReactiveNeo4jRepository<TestEntityWithAssignedId1, String> {
}

abstract static class Config extends Neo4jReactiveTestConfiguration {

@Bean
public Driver driver() {

return neo4jConnectionSupport.getDriver();
}

@Bean
public BookmarkCapture bookmarkCapture() {
return new BookmarkCapture();
}

@Override
public ReactiveTransactionManager reactiveTransactionManager(Driver driver, ReactiveDatabaseSelectionProvider databaseSelectionProvider) {

BookmarkCapture bookmarkCapture = bookmarkCapture();
return new ReactiveNeo4jTransactionManager(driver, databaseSelectionProvider, Neo4jBookmarkManager.create(bookmarkCapture));
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/*
* Copyright 2011-2023 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.gh2728;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.neo4j.driver.Driver;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Bean;
import org.springframework.data.neo4j.core.DatabaseSelectionProvider;
import org.springframework.data.neo4j.core.transaction.Neo4jBookmarkManager;
import org.springframework.data.neo4j.core.transaction.Neo4jTransactionManager;
import org.springframework.data.neo4j.repository.Neo4jRepository;
import org.springframework.data.neo4j.test.BookmarkCapture;
import org.springframework.data.neo4j.test.Neo4jExtension;
import org.springframework.data.neo4j.test.Neo4jImperativeTestConfiguration;
import org.springframework.data.neo4j.test.Neo4jIntegrationTest;
import org.springframework.transaction.PlatformTransactionManager;

/**
* @author Gerrit Meier
*/
@Neo4jIntegrationTest
public abstract class AbstractTestBase {

protected static Neo4jExtension.Neo4jConnectionSupport neo4jConnectionSupport;

@Autowired
private TestEntityWithGeneratedDeprecatedId1Repository generatedDeprecatedIdRepository;

@Autowired
private TestEntityWithAssignedId1Repository assignedIdRepository;

@Test
public void testGeneratedDeprecatedIds() {
TestEntityWithGeneratedDeprecatedId2 t2 = new TestEntityWithGeneratedDeprecatedId2(null, "v2");
TestEntityWithGeneratedDeprecatedId1 t1 = new TestEntityWithGeneratedDeprecatedId1(null, "v1", t2);

TestEntityWithGeneratedDeprecatedId1 result = generatedDeprecatedIdRepository.save(t1);

TestEntityWithGeneratedDeprecatedId1 freshRetrieved = generatedDeprecatedIdRepository.findById(result.getId()).get();

Assertions.assertNotNull(result.getRelatedEntity());
Assertions.assertNotNull(freshRetrieved.getRelatedEntity());
}

/**
* This is a test to ensure if the fix for the failing test above will continue to work for
* assigned ids. For broader test cases please return false for isCypher5Compatible in (Reactive)RepositoryIT
*/
@Test
public void testAssignedIds() {
TestEntityWithAssignedId2 t2 = new TestEntityWithAssignedId2("second", "v2");
TestEntityWithAssignedId1 t1 = new TestEntityWithAssignedId1("first", "v1", t2);

TestEntityWithAssignedId1 result = assignedIdRepository.save(t1);

TestEntityWithAssignedId1 freshRetrieved = assignedIdRepository.findById(result.getAssignedId()).get();

Assertions.assertNotNull(result.getRelatedEntity());
Assertions.assertNotNull(freshRetrieved.getRelatedEntity());
}

interface TestEntityWithGeneratedDeprecatedId1Repository extends Neo4jRepository<TestEntityWithGeneratedDeprecatedId1, Long> {
}

interface TestEntityWithAssignedId1Repository extends Neo4jRepository<TestEntityWithAssignedId1, String> {
}

abstract static class Config extends Neo4jImperativeTestConfiguration {

@Bean
public Driver driver() {

return neo4jConnectionSupport.getDriver();
}

@Bean
public BookmarkCapture bookmarkCapture() {
return new BookmarkCapture();
}

@Override
public PlatformTransactionManager transactionManager(Driver driver,
DatabaseSelectionProvider databaseNameProvider) {
BookmarkCapture bookmarkCapture = bookmarkCapture();
return new Neo4jTransactionManager(driver, databaseNameProvider,
Neo4jBookmarkManager.create(bookmarkCapture));
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright 2011-2023 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.gh2728;

import org.springframework.context.annotation.Configuration;
import org.springframework.data.neo4j.repository.config.EnableNeo4jRepositories;
import org.springframework.data.neo4j.test.Neo4jIntegrationTest;
import org.springframework.transaction.annotation.EnableTransactionManagement;

/**
* @author Michael J. Simons
*/
@Neo4jIntegrationTest
public class CorrectConfigIT extends AbstractTestBase {

@Configuration
@EnableTransactionManagement
@EnableNeo4jRepositories(considerNestedRepositories = true)
static class Config extends AbstractTestBase.Config {
@Override
public boolean isCypher5Compatible() {
return AbstractTestBase.neo4jConnectionSupport.isCypher5SyntaxCompatible();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright 2011-2023 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.gh2728;

import org.springframework.context.annotation.Configuration;
import org.springframework.data.neo4j.repository.config.EnableReactiveNeo4jRepositories;
import org.springframework.data.neo4j.test.Neo4jIntegrationTest;
import org.springframework.transaction.annotation.EnableTransactionManagement;

/**
* @author Michael J. Simons
*/
@Neo4jIntegrationTest
public class CorrectReactiveConfigIT extends AbstractReactiveTestBase {

@Configuration
@EnableTransactionManagement
@EnableReactiveNeo4jRepositories(considerNestedRepositories = true)
static class Config extends AbstractReactiveTestBase.Config {
@Override
public boolean isCypher5Compatible() {
return AbstractReactiveTestBase.neo4jConnectionSupport.isCypher5SyntaxCompatible();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Copyright 2011-2023 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.gh2728;

import org.springframework.data.neo4j.core.schema.Id;
import org.springframework.data.neo4j.core.schema.Node;
import org.springframework.data.neo4j.core.schema.Property;
import org.springframework.data.neo4j.core.schema.Relationship;

/**
* @author Gerrit Meier
*/
@Node
public class TestEntityWithAssignedId1 {

@Id
private String assignedId;

@Property("value_one")
private String valueOne;

@Relationship("related_to")
private TestEntityWithAssignedId2 relatedEntity;

public TestEntityWithAssignedId1(String assignedId, String valueOne, TestEntityWithAssignedId2 relatedEntity) {
this.assignedId = assignedId;
this.valueOne = valueOne;
this.relatedEntity = relatedEntity;
}
public String getAssignedId() {
return assignedId;
}

public TestEntityWithAssignedId2 getRelatedEntity() {
return relatedEntity;
}
}
Loading

0 comments on commit d890a65

Please sign in to comment.