Skip to content

Commit

Permalink
fix: Use contextual Cypher-DSL configuration for rendering statements…
Browse files Browse the repository at this point in the history
… in various Cypher-DSL based query extension.

Fixes #2927.
  • Loading branch information
michael-simons committed Jul 19, 2024
1 parent 73bd2c0 commit c37adb0
Show file tree
Hide file tree
Showing 17 changed files with 173 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,7 @@ default TerminatingFind<T> matching(String query) {
* @return new instance of {@link TerminatingFind}.
* @throws IllegalArgumentException if statement is {@literal null}.
*/
default TerminatingFind<T> matching(Statement statement, @Nullable Map<String, Object> parameter) {

return matching(statement.getCypher(), TemplateSupport.mergeParameters(statement, parameter));
}
TerminatingFind<T> matching(Statement statement, @Nullable Map<String, Object> parameter);

/**
* Set the filter {@link Statement statement} to be used.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.util.List;
import java.util.Map;

import org.neo4j.cypherdsl.core.Statement;
import org.springframework.data.neo4j.repository.query.QueryFragmentsAndParameters;
import org.springframework.util.Assert;

Expand Down Expand Up @@ -87,7 +88,6 @@ public <T1> FindWithQuery<T1> as(Class<T1> returnType) {
public TerminatingFind<T> matching(String query, Map<String, Object> parameters) {

Assert.notNull(query, "Query must not be null");

return new ExecutableFindSupport<>(template, domainType, returnType, query, parameters);
}

Expand All @@ -100,6 +100,12 @@ public TerminatingFind<T> matching(QueryFragmentsAndParameters queryFragmentsAnd
return new ExecutableFindSupport<>(template, domainType, returnType, queryFragmentsAndParameters);
}

@Override
public TerminatingFind<T> matching(Statement statement, Map<String, Object> parameter) {

return matching(template.render(statement), TemplateSupport.mergeParameters(statement, parameter));
}

@Override
public T oneValue() {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1248,6 +1248,10 @@ <T, R> List<R> doSave(Iterable<R> instances, Class<T> domainType) {
});
}

String render(Statement statement) {
return renderer.render(statement);
}

final class DefaultExecutableQuery<T> implements ExecutableQuery<T> {

private final PreparedQuery<T> preparedQuery;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,7 @@ default TerminatingFind<T> matching(String query) {
* @return new instance of {@link TerminatingFind}.
* @throws IllegalArgumentException if statement is {@literal null}.
*/
default TerminatingFind<T> matching(Statement statement, @Nullable Map<String, Object> parameter) {
return matching(statement.getCypher(), TemplateSupport.mergeParameters(statement, parameter));
}
TerminatingFind<T> matching(Statement statement, @Nullable Map<String, Object> parameter);

/**
* Set the filter {@link Statement statement} to be used.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.util.Collections;
import java.util.Map;

import org.neo4j.cypherdsl.core.Statement;
import org.springframework.data.neo4j.repository.query.QueryFragmentsAndParameters;
import org.springframework.util.Assert;

Expand Down Expand Up @@ -101,6 +102,12 @@ public TerminatingFind<T> matching(QueryFragmentsAndParameters queryFragmentsAnd
return new ExecutableFindSupport<>(template, domainType, returnType, queryFragmentsAndParameters);
}

@Override
public TerminatingFind<T> matching(Statement statement, Map<String, Object> parameter) {

return matching(template.render(statement), TemplateSupport.mergeParameters(statement, parameter));
}

@Override
public Mono<T> one() {
return doFind(TemplateSupport.FetchType.ONE).single();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1300,6 +1300,10 @@ public <T> ExecutableSave<T> save(Class<T> domainType) {
return new ReactiveFluentOperationSupport(this).save(domainType);
}

String render(Statement statement) {
return this.renderer.render(statement);
}

final class DefaultReactiveExecutableQuery<T> implements ExecutableQuery<T> {

private final PreparedQuery<T> preparedQuery;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.util.Map;
import java.util.Optional;
import java.util.function.BiFunction;
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.function.UnaryOperator;

Expand All @@ -44,15 +45,20 @@
final class CypherdslBasedQuery extends AbstractNeo4jQuery {

static CypherdslBasedQuery create(Neo4jOperations neo4jOperations, Neo4jMappingContext mappingContext,
Neo4jQueryMethod queryMethod, ProjectionFactory projectionFactory) {
Neo4jQueryMethod queryMethod, ProjectionFactory projectionFactory,
Function<Statement, String> renderer) {

return new CypherdslBasedQuery(neo4jOperations, mappingContext, queryMethod, Neo4jQueryType.DEFAULT, projectionFactory);
return new CypherdslBasedQuery(neo4jOperations, mappingContext, queryMethod, Neo4jQueryType.DEFAULT, projectionFactory, renderer);
}

private final Function<Statement, String> renderer;

private CypherdslBasedQuery(Neo4jOperations neo4jOperations,
Neo4jMappingContext mappingContext,
Neo4jQueryMethod queryMethod, Neo4jQueryType queryType, ProjectionFactory projectionFactory) {
Neo4jQueryMethod queryMethod, Neo4jQueryType queryType, ProjectionFactory projectionFactory,
Function<Statement, String> renderer) {
super(neo4jOperations, mappingContext, queryMethod, queryType, projectionFactory);
this.renderer = renderer;
}

@Override
Expand Down Expand Up @@ -86,7 +92,7 @@ protected <T> PreparedQuery<T> prepareQuery(Class<T> returnedType,

Map<String, Object> boundParameters = statement.getCatalog().getParameters();
return PreparedQuery.queryFor(returnedType)
.withCypherQuery(statement.getCypher())
.withCypherQuery(renderer.apply(statement))
.withParameters(boundParameters)
.usingMappingFunction(mappingFunction)
.build();
Expand All @@ -98,7 +104,7 @@ protected Optional<PreparedQuery<Long>> getCountQuery(Neo4jParameterAccessor par
// We verified this above
Statement countStatement = (Statement) parameterAccessor.getValues()[1];
return Optional.of(PreparedQuery.queryFor(Long.class)
.withCypherQuery(countStatement.getCypher())
.withCypherQuery(renderer.apply(countStatement))
.withParameters(countStatement.getCatalog().getParameters()).build());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import java.lang.reflect.Method;

import org.apiguardian.api.API;
import org.neo4j.cypherdsl.core.renderer.Configuration;
import org.neo4j.cypherdsl.core.renderer.Renderer;
import org.springframework.data.neo4j.core.Neo4jOperations;
import org.springframework.data.neo4j.core.mapping.Neo4jMappingContext;
import org.springframework.data.projection.ProjectionFactory;
Expand All @@ -40,12 +42,14 @@ public final class Neo4jQueryLookupStrategy implements QueryLookupStrategy {
private final Neo4jMappingContext mappingContext;
private final Neo4jOperations neo4jOperations;
private final QueryMethodEvaluationContextProvider evaluationContextProvider;
private final Configuration configuration;

public Neo4jQueryLookupStrategy(Neo4jOperations neo4jOperations, Neo4jMappingContext mappingContext,
QueryMethodEvaluationContextProvider evaluationContextProvider) {
QueryMethodEvaluationContextProvider evaluationContextProvider, Configuration configuration) {
this.neo4jOperations = neo4jOperations;
this.mappingContext = mappingContext;
this.evaluationContextProvider = evaluationContextProvider;
this.configuration = configuration;
}

/* (non-Javadoc)
Expand All @@ -65,7 +69,7 @@ public RepositoryQuery resolveQuery(Method method, RepositoryMetadata metadata,
return StringBasedNeo4jQuery.create(neo4jOperations, mappingContext, evaluationContextProvider, queryMethod,
factory);
} else if (queryMethod.isCypherBasedProjection()) {
return CypherdslBasedQuery.create(neo4jOperations, mappingContext, queryMethod, factory);
return CypherdslBasedQuery.create(neo4jOperations, mappingContext, queryMethod, factory, Renderer.getRenderer(configuration)::render);
} else {
return PartTreeNeo4jQuery.create(neo4jOperations, mappingContext, queryMethod, factory);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.util.Collection;
import java.util.Map;
import java.util.function.BiFunction;
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.function.UnaryOperator;

Expand All @@ -42,15 +43,19 @@
final class ReactiveCypherdslBasedQuery extends AbstractReactiveNeo4jQuery {

static ReactiveCypherdslBasedQuery create(ReactiveNeo4jOperations neo4jOperations, Neo4jMappingContext mappingContext,
Neo4jQueryMethod queryMethod, ProjectionFactory projectionFactory) {
Neo4jQueryMethod queryMethod, ProjectionFactory projectionFactory, Function<Statement, String> renderer) {

return new ReactiveCypherdslBasedQuery(neo4jOperations, mappingContext, queryMethod, Neo4jQueryType.DEFAULT, projectionFactory);
return new ReactiveCypherdslBasedQuery(neo4jOperations, mappingContext, queryMethod, Neo4jQueryType.DEFAULT, projectionFactory, renderer);
}

private final Function<Statement, String> renderer;

private ReactiveCypherdslBasedQuery(ReactiveNeo4jOperations neo4jOperations,
Neo4jMappingContext mappingContext,
Neo4jQueryMethod queryMethod, Neo4jQueryType queryType, ProjectionFactory projectionFactory) {
Neo4jQueryMethod queryMethod, Neo4jQueryType queryType, ProjectionFactory projectionFactory,
Function<Statement, String> renderer) {
super(neo4jOperations, mappingContext, queryMethod, queryType, projectionFactory);
this.renderer = renderer;
}

@Override
Expand All @@ -68,7 +73,7 @@ protected <T> PreparedQuery<T> prepareQuery(Class<T> returnedType, Collection<Pr

Map<String, Object> boundParameters = statement.getCatalog().getParameters();
return PreparedQuery.queryFor(returnedType)
.withCypherQuery(statement.getCypher())
.withCypherQuery(renderer.apply(statement))
.withParameters(boundParameters)
.usingMappingFunction(mappingFunction)
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import java.lang.reflect.Method;

import org.apiguardian.api.API;
import org.neo4j.cypherdsl.core.renderer.Configuration;
import org.neo4j.cypherdsl.core.renderer.Renderer;
import org.springframework.data.neo4j.core.ReactiveNeo4jOperations;
import org.springframework.data.neo4j.core.mapping.Neo4jMappingContext;
import org.springframework.data.projection.ProjectionFactory;
Expand All @@ -40,12 +42,14 @@ public final class ReactiveNeo4jQueryLookupStrategy implements QueryLookupStrate
private final ReactiveNeo4jOperations neo4jOperations;
private final Neo4jMappingContext mappingContext;
private final QueryMethodEvaluationContextProvider evaluationContextProvider;
private final Configuration configuration;

public ReactiveNeo4jQueryLookupStrategy(ReactiveNeo4jOperations neo4jOperations, Neo4jMappingContext mappingContext,
QueryMethodEvaluationContextProvider evaluationContextProvider) {
QueryMethodEvaluationContextProvider evaluationContextProvider, Configuration configuration) {
this.neo4jOperations = neo4jOperations;
this.mappingContext = mappingContext;
this.evaluationContextProvider = evaluationContextProvider;
this.configuration = configuration;
}

/* (non-Javadoc)
Expand All @@ -65,7 +69,7 @@ public RepositoryQuery resolveQuery(Method method, RepositoryMetadata metadata,
return ReactiveStringBasedNeo4jQuery.create(neo4jOperations, mappingContext, evaluationContextProvider,
queryMethod, projectionFactory);
} else if (queryMethod.isCypherBasedProjection()) {
return ReactiveCypherdslBasedQuery.create(neo4jOperations, mappingContext, queryMethod, projectionFactory);
return ReactiveCypherdslBasedQuery.create(neo4jOperations, mappingContext, queryMethod, projectionFactory, Renderer.getRenderer(configuration)::render);
} else {
return ReactivePartTreeNeo4jQuery.create(neo4jOperations, mappingContext, queryMethod, projectionFactory);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@

import java.util.Optional;

import org.neo4j.cypherdsl.core.renderer.Configuration;
import org.springframework.beans.BeansException;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.data.neo4j.core.Neo4jOperations;
import org.springframework.data.neo4j.core.mapping.Neo4jMappingContext;
import org.springframework.data.neo4j.core.mapping.Neo4jPersistentEntity;
Expand Down Expand Up @@ -51,6 +54,8 @@ final class Neo4jRepositoryFactory extends RepositoryFactorySupport {

private final Neo4jMappingContext mappingContext;

private Configuration cypherDSLConfiguration = Configuration.defaultConfig();

Neo4jRepositoryFactory(Neo4jOperations neo4jOperations, Neo4jMappingContext mappingContext) {

this.neo4jOperations = neo4jOperations;
Expand Down Expand Up @@ -122,6 +127,14 @@ protected Class<?> getRepositoryBaseClass(RepositoryMetadata metadata) {
return SimpleNeo4jRepository.class;
}

@Override
public void setBeanFactory(BeanFactory beanFactory) throws BeansException {
super.setBeanFactory(beanFactory);
this.cypherDSLConfiguration = beanFactory
.getBeanProvider(Configuration.class)
.getIfAvailable(Configuration::defaultConfig);
}

/*
* (non-Javadoc)
* @see org.springframework.data.repository.core.support.RepositoryFactorySupport#getQueryLookupStrategy(org.springframework.data.repository.query.QueryLookupStrategy.Key, org.springframework.data.repository.query.EvaluationContextProvider)
Expand All @@ -130,7 +143,7 @@ protected Class<?> getRepositoryBaseClass(RepositoryMetadata metadata) {
protected Optional<QueryLookupStrategy> getQueryLookupStrategy(Key key,
QueryMethodEvaluationContextProvider evaluationContextProvider) {

return Optional.of(new Neo4jQueryLookupStrategy(neo4jOperations, mappingContext, evaluationContextProvider));
return Optional.of(new Neo4jQueryLookupStrategy(neo4jOperations, mappingContext, evaluationContextProvider, cypherDSLConfiguration));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import java.util.Optional;

import org.neo4j.cypherdsl.core.renderer.Configuration;
import org.springframework.beans.BeansException;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.beans.factory.ListableBeanFactory;
Expand Down Expand Up @@ -55,6 +56,8 @@ final class ReactiveNeo4jRepositoryFactory extends ReactiveRepositoryFactorySupp

private final Neo4jMappingContext mappingContext;

private Configuration cypherDSLConfiguration = Configuration.defaultConfig();

ReactiveNeo4jRepositoryFactory(ReactiveNeo4jOperations neo4jOperations, Neo4jMappingContext mappingContext) {

this.neo4jOperations = neo4jOperations;
Expand Down Expand Up @@ -133,7 +136,7 @@ protected Optional<QueryLookupStrategy> getQueryLookupStrategy(Key key,
QueryMethodEvaluationContextProvider evaluationContextProvider) {

return Optional
.of(new ReactiveNeo4jQueryLookupStrategy(neo4jOperations, mappingContext, evaluationContextProvider));
.of(new ReactiveNeo4jQueryLookupStrategy(neo4jOperations, mappingContext, evaluationContextProvider, cypherDSLConfiguration));
}

@Override
Expand All @@ -148,6 +151,10 @@ public void setBeanFactory(BeanFactory beanFactory) throws BeansException {
factory.addAdvice(advice);
});
}

this.cypherDSLConfiguration = beanFactory
.getBeanProvider(Configuration.class)
.getIfAvailable(Configuration::defaultConfig);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ abstract class AbstractElementIdTestBase {
void setupData(LogbackCapture logbackCapture, @Autowired Driver driver, @Autowired BookmarkCapture bookmarkCapture) {

logbackCapture.addLogger("org.springframework.data.neo4j.cypher.deprecation", Level.WARN);
logbackCapture.addLogger("org.springframework.data.neo4j.cypher", Level.DEBUG);
try (Session session = driver.session()) {
session.run("MATCH (n) DETACH DELETE n").consume();
bookmarkCapture.seedWith(session.lastBookmarks());
Expand Down Expand Up @@ -78,6 +79,7 @@ static void assertThatLogMessageDoNotIndicateIDUsage(LogbackCapture logbackCaptu
List<String> formattedMessages = logbackCapture.getFormattedMessages();
assertThat(formattedMessages)
.noneMatch(s -> s.contains("Neo.ClientNotification.Statement.FeatureDeprecationWarning") ||
s.contains("The query used a deprecated function. ('id' is no longer supported)"));
s.contains("The query used a deprecated function. ('id' is no longer supported)") ||
s.matches("(?s).*toString\\(id\\(.*")); // No deprecations are logged when deprecated function call is nested. Anzeige ist raus.
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,17 @@
import java.util.List;
import java.util.Map;

import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.neo4j.cypherdsl.core.Cypher;
import org.neo4j.cypherdsl.core.Statement;
import org.neo4j.driver.Driver;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.data.neo4j.core.DatabaseSelectionProvider;
import org.springframework.data.neo4j.core.Neo4jTemplate;
import org.springframework.data.neo4j.core.transaction.Neo4jBookmarkManager;
import org.springframework.data.neo4j.core.transaction.Neo4jTransactionManager;
import org.springframework.data.neo4j.repository.Neo4jRepository;
Expand Down Expand Up @@ -351,6 +355,28 @@ RETURN count(*)"""),
}
}

@Test
@Tag("GH-2927")
void fluentOpsMustUseCypherDSLConfig(
LogbackCapture logbackCapture,
@Autowired Driver driver,
@Autowired BookmarkCapture bookmarkCapture,
@Autowired Neo4jTemplate neo4jTemplate) {

try (var session = driver.session(bookmarkCapture.createSessionConfig())) {
session.run("MERGE (n:" + Thing.THING_LABEL + "{foo: 'bar'})").consume();
}

var thingNode = Cypher.node(Thing.THING_LABEL);
var cypherStatement = Statement.builder()
.match(thingNode)
.where(Cypher.elementId(thingNode).eq(Cypher.literalOf("test")))
.returning(thingNode)
.build();
neo4jTemplate.find(Thing.class).matching(cypherStatement).one();
assertThatLogMessageDoNotIndicateIDUsage(logbackCapture);
}

@Configuration
@EnableTransactionManagement
@EnableNeo4jRepositories(considerNestedRepositories = true)
Expand Down
Loading

0 comments on commit c37adb0

Please sign in to comment.