DATAGRAPH-1286 - Support generated ids in derived finder methods.

This introduces NativeIdFilterFunction that is applied when a derived finder method hits a native id property.

This is a backport of c0050a1c3f into 5.1.x that closes #1792.
This commit is contained in:
Michael Simons
2020-01-20 12:39:58 +01:00
parent 52a63fe911
commit 6fa6158e90
12 changed files with 234 additions and 42 deletions

View File

@@ -143,6 +143,13 @@ public class Neo4jPersistentProperty extends AnnotationBasedPersistentProperty<N
return propertyType.idProperty;
}
/**
* @return True if this property describes the internal ID property.
*/
public boolean isInternalIdProperty() {
return propertyType == PropertyType.INTERNAL_ID_PROPERTY;
}
PropertyType getPropertyType() {
return propertyType;
}

View File

@@ -18,9 +18,13 @@ package org.springframework.data.neo4j.repository.query;
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
import java.util.function.Predicate;
import org.neo4j.ogm.cypher.BooleanOperator;
import org.neo4j.ogm.cypher.Filters;
import org.springframework.data.mapping.PersistentPropertyPath;
import org.springframework.data.neo4j.mapping.Neo4jMappingContext;
import org.springframework.data.neo4j.mapping.Neo4jPersistentProperty;
import org.springframework.data.neo4j.repository.query.filter.FilterBuilder;
import org.springframework.data.repository.query.parser.Part;
@@ -39,17 +43,24 @@ class FilterBuildersDefinition {
private final Part basePart;
private final List<FilterBuilder> filterBuilders;
private final Predicate<Part> isInternalIdProperty;
static UnstartedBuild forType(Class<?> entityType) {
return new UnstartedBuild(entityType);
static UnstartedBuild forType(Neo4jMappingContext mappingContext, Class<?> entityType) {
return new UnstartedBuild(mappingContext, entityType);
}
private FilterBuildersDefinition(Class<?> entityType, Part basePart) {
private FilterBuildersDefinition(Neo4jMappingContext mappingContext, Class<?> entityType, Part basePart) {
this.entityType = entityType;
this.basePart = basePart;
this.filterBuilders = new LinkedList<>();
this.filterBuilders.add(FilterBuilder.forPartAndEntity(basePart, entityType, BooleanOperator.NONE));
this.isInternalIdProperty = part -> {
PersistentPropertyPath<Neo4jPersistentProperty> path = mappingContext
.getPersistentPropertyPath(part.getProperty());
Neo4jPersistentProperty possibleIdProperty = path.getRequiredLeafProperty();
return possibleIdProperty.isInternalIdProperty();
};
this.filterBuilders.add(FilterBuilder.forPartAndEntity(basePart, entityType, BooleanOperator.NONE,
isInternalIdProperty));
}
TemplatedQuery buildTemplatedQuery() {
@@ -66,24 +77,28 @@ class FilterBuildersDefinition {
}
FilterBuildersDefinition and(Part part) {
this.filterBuilders.add(FilterBuilder.forPartAndEntity(part, entityType, BooleanOperator.AND));
this.filterBuilders.add(FilterBuilder.forPartAndEntity(part, entityType, BooleanOperator.AND, isInternalIdProperty));
return this;
}
FilterBuildersDefinition or(Part part) {
this.filterBuilders.add(FilterBuilder.forPartAndEntity(part, entityType, BooleanOperator.OR));
this.filterBuilders.add(FilterBuilder.forPartAndEntity(part, entityType, BooleanOperator.OR, isInternalIdProperty));
return this;
}
static class UnstartedBuild {
private final Neo4jMappingContext mappingContext;
private final Class<?> entityType;
UnstartedBuild(Class<?> entityType) {
UnstartedBuild(Neo4jMappingContext mappingContext, Class<?> entityType) {
this.mappingContext = mappingContext;
this.entityType = entityType;
}
FilterBuildersDefinition startWith(Part firstPart) {
return new FilterBuildersDefinition(entityType, firstPart);
return new FilterBuildersDefinition(mappingContext, entityType, firstPart);
}
}
}

View File

@@ -112,19 +112,6 @@ public class GraphQueryMethod extends QueryMethod {
}
}
}
/*
//Java 8 only
Parameter[] parameters = method.getParameters();
for (int i = 0; i < method.getParameterCount(); i++) {
if (parameters[i].isAnnotationPresent(Depth.class)) {
if (parameters[i].getType() == Integer.class || parameters[i].getType() == int.class) {
return i;
}
else {
throw new IllegalArgumentException("Depth parameter in " + method.getName() + " must be an integer");
}
}
}*/
return null;
}

View File

@@ -22,6 +22,7 @@ import org.neo4j.ogm.metadata.MetaData;
import org.neo4j.ogm.session.Session;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.data.neo4j.mapping.Neo4jMappingContext;
import org.springframework.data.repository.query.RepositoryQuery;
import org.springframework.data.repository.query.ResultProcessor;
import org.springframework.data.repository.query.parser.PartTree;
@@ -49,11 +50,11 @@ public class PartTreeNeo4jQuery extends AbstractGraphRepositoryQuery {
super(graphQueryMethod, metaData, session);
Class<?> domainType = graphQueryMethod.getEntityInformation().getJavaType();
this.graphQueryMethod = graphQueryMethod;
this.tree = new PartTree(graphQueryMethod.getName(), domainType);
this.queryTemplate = new TemplatedQueryCreator(this.tree, domainType).createQuery();
this.queryTemplate = new TemplatedQueryCreator(this.tree,
(Neo4jMappingContext) this.graphQueryMethod.getMappingContext(), domainType).createQuery();
}
@Override

View File

@@ -18,6 +18,7 @@ package org.springframework.data.neo4j.repository.query;
import java.util.Iterator;
import org.springframework.data.domain.Sort;
import org.springframework.data.neo4j.mapping.Neo4jMappingContext;
import org.springframework.data.repository.query.parser.AbstractQueryCreator;
import org.springframework.data.repository.query.parser.Part;
import org.springframework.data.repository.query.parser.PartTree;
@@ -31,17 +32,19 @@ import org.springframework.data.repository.query.parser.PartTree;
*/
class TemplatedQueryCreator extends AbstractQueryCreator<TemplatedQuery, FilterBuildersDefinition> {
private final Neo4jMappingContext mappingContext;
private final Class<?> entityType;
public TemplatedQueryCreator(PartTree tree, Class<?> entityType) {
public TemplatedQueryCreator(PartTree tree, Neo4jMappingContext mappingContext, Class<?> entityType) {
super(tree);
this.mappingContext = mappingContext;
this.entityType = entityType;
}
@Override
protected FilterBuildersDefinition create(Part part, Iterator<Object> iterator) {
return FilterBuildersDefinition.forType(entityType) //
return FilterBuildersDefinition.forType(mappingContext, entityType) //
.startWith(part);
}

View File

@@ -20,6 +20,7 @@ import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.Stack;
import java.util.function.Predicate;
import org.neo4j.ogm.cypher.BooleanOperator;
import org.neo4j.ogm.cypher.Filter;
@@ -41,27 +42,40 @@ public abstract class FilterBuilder {
protected Part part;
protected BooleanOperator booleanOperator;
protected Class<?> entityType;
protected Predicate<Part> isInternalIdProperty = part -> false;
public static FilterBuilder forPartAndEntity(Part part, Class<?> entityType, BooleanOperator booleanOperator) {
public static FilterBuilder forPartAndEntity(Part part, Class<?> entityType, BooleanOperator booleanOperator,
Predicate<Part> isInternalIdProperty) {
FilterBuilder filterBuilder;
switch (part.getType()) {
case NEAR:
return new DistanceComparisonBuilder(part, booleanOperator, entityType);
filterBuilder = new DistanceComparisonBuilder(part, booleanOperator, entityType);
break;
case BETWEEN:
return new BetweenComparisonBuilder(part, booleanOperator, entityType);
filterBuilder = new BetweenComparisonBuilder(part, booleanOperator, entityType);
break;
case NOT_CONTAINING:
case CONTAINING:
return resolveMatchingContainsFilterBuilder(part, entityType, booleanOperator);
filterBuilder = resolveMatchingContainsFilterBuilder(part, entityType, booleanOperator);
break;
case IS_NULL:
case IS_NOT_NULL:
return new IsNullFilterBuilder(part, booleanOperator, entityType);
filterBuilder = new IsNullFilterBuilder(part, booleanOperator, entityType);
break;
case EXISTS:
return new ExistsFilterBuilder(part, booleanOperator, entityType);
filterBuilder = new ExistsFilterBuilder(part, booleanOperator, entityType);
break;
case TRUE:
case FALSE:
return new BooleanComparisonBuilder(part, booleanOperator, entityType);
filterBuilder = new BooleanComparisonBuilder(part, booleanOperator, entityType);
break;
default:
return new PropertyComparisonBuilder(part, booleanOperator, entityType);
filterBuilder = new PropertyComparisonBuilder(part, booleanOperator, entityType);
break;
}
filterBuilder.isInternalIdProperty = isInternalIdProperty;
return filterBuilder;
}
FilterBuilder(Part part, BooleanOperator booleanOperator, Class<?> entityType) {

View File

@@ -0,0 +1,87 @@
/*
* Copyright 2011-2021 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.repository.query.filter;
import static org.neo4j.ogm.cypher.ComparisonOperator.*;
import java.util.HashMap;
import java.util.Map;
import org.neo4j.ogm.cypher.ComparisonOperator;
import org.neo4j.ogm.cypher.Filter;
import org.neo4j.ogm.cypher.function.FilterFunction;
/**
* This is a specialised filter function taking care of filtering native id properties.
*
* @author Michael J. Simons
* @soundtrack Freddie Mercury - Never Boring
*/
final class NativeIdFilterFunction implements FilterFunction<Object> {
// This function belongs somewhat more into OGM than SDN. The reason having it here is simple: The filter is build
// explicitly and not via reflection and we don't want to have yet another shim managing separate possible versions
// OGM like we already have with the embedded support, entity instantiator and some other things. ^ms
private final ComparisonOperator operator;
private final Object value;
private Filter filter;
NativeIdFilterFunction(ComparisonOperator operator, Object value) {
this.operator = operator;
this.value = value;
}
@Override
public Object getValue() {
return this.value;
}
@Override
public Filter getFilter() {
return filter;
}
@Override
public void setFilter(Filter filter) {
this.filter = filter;
}
@Override
public String expression(String nodeIdentifier) {
switch (operator) {
case EQUALS:
case GREATER_THAN:
case GREATER_THAN_EQUAL:
case LESS_THAN:
case LESS_THAN_EQUAL:
case IN:
return String.format("id(%s) %s $`%s` ", nodeIdentifier, operator.getValue(), filter.uniqueParameterName());
default:
throw new IllegalArgumentException("Unsupported comparision operator for an ID attribute.");
}
}
@Override
public Map<String, Object> parameters() {
Map<String, Object> map = new HashMap<>();
if (operator.isOneOf(EQUALS, GREATER_THAN, GREATER_THAN_EQUAL, LESS_THAN, LESS_THAN_EQUAL, IN)) {
map.put(filter.uniqueParameterName(), filter.getTransformedPropertyValue());
}
return map;
}
}

View File

@@ -45,7 +45,14 @@ class PropertyComparisonBuilder extends FilterBuilder {
Object value = params.pop();
Filter filter = new Filter(nestedAttributes.isEmpty() ? propertyName() : nestedAttributes.getLeafPropertySegment(), convertToComparisonOperator(part.getType()), value);
Filter filter;
String propertyName = nestedAttributes.isEmpty() ? propertyName() : nestedAttributes.getLeafPropertySegment();
if (isInternalIdProperty.test(part)) {
filter = new Filter(new NativeIdFilterFunction(convertToComparisonOperator(part.getType()), value));
filter.setPropertyName(propertyName);
} else {
filter = new Filter(propertyName, convertToComparisonOperator(part.getType()), value);
}
filter.setOwnerEntityType(entityType);
filter.setBooleanOperator(booleanOperator);
filter.setNegated(isNegated());

View File

@@ -28,8 +28,6 @@ import org.neo4j.ogm.typeconversion.UuidStringConverter;
*/
public class NodeWithUUIDAsId {
private Long id;
@Id @GeneratedValue(strategy = UuidStrategy.class) @Convert(UuidStringConverter.class) private UUID myNiceId;
private String someProperty;
@@ -38,10 +36,6 @@ public class NodeWithUUIDAsId {
this.someProperty = someProperty;
}
public Long getId() {
return id;
}
public UUID getMyNiceId() {
return myNiceId;
}

View File

@@ -145,6 +145,12 @@ public interface UserRepository extends PersonRepository<User, Long> {
Slice<User> findByNameAndRatingsStars(String name, int stars, Pageable pageable);
Page<User> findAllByIdIn(Iterable<Long> id, Pageable pageable);
List<User> findAllByIdInAndNameLike(Iterable<Long> id, String name);
List<User> findAllByIdAndName(Long id, String name);
@Query("invalid")
void invalidQuery();

View File

@@ -19,9 +19,11 @@ import static org.assertj.core.api.Assertions.*;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.*;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Iterator;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
@@ -704,6 +706,59 @@ public class DerivedQueryTests extends MultiDriverTestClass {
assertFalse(page.hasNext());
}
@Test // DATAGRAPH-1286
public void findAllByIdShouldSupportPageableParameter() {
List<Long> ids = new ArrayList<>();
for (int i = 0; i < 10; i++) {
User u = new User("U" + i);
userRepository.save(u);
ids.add(u.getId());
}
// Just make sure stuff exists and OGM correctly uses id(n) instead of n.id :(
Optional<User> randomUser = userRepository.findById(ids.get(ids.size() - 2));
assertThat(randomUser).isPresent().map(User::getName).hasValue("U8");
// Assert findAllById works _at all_
Iterable<User> allUsers = userRepository.findAllById(ids);
assertThat(allUsers).hasSize(ids.size());
Pageable pageable = PageRequest.of(0, 2);
Page<User> page = userRepository.findAllByIdIn(ids.subList(0, 4), pageable);
assertThat(page.getSize()).isEqualTo(2);
assertThat(page.getContent()).hasSize(2);
assertThat(page.hasNext()).isTrue();
}
@Test // DATAGRAPH-1286
public void findByIdInInDerivedQueryMethodShouldWork() {
List<Long> ids = new ArrayList<>();
for (int i = 0; i < 10; i++) {
User u = new User("U" + i);
userRepository.save(u);
ids.add(u.getId());
}
List<User> users = userRepository.findAllByIdInAndNameLike(ids.subList(0, 4), "U*");
assertThat(users).hasSize(4);
}
@Test // DATAGRAPH-1286
public void findByIdEqualsInDerivedQueryMethodShouldWork() {
List<Long> ids = new ArrayList<>();
for (int i = 0; i < 10; i++) {
User u = new User("U" + i);
userRepository.save(u);
ids.add(u.getId());
}
List<User> users = userRepository.findAllByIdAndName(ids.get(2), "U2");
assertThat(users).hasSize(1);
}
@Test // DATAGRAPH-1093
public void shouldFindNodeEntitiesByAttributeIgnoringCase() {
executeUpdate("CREATE (:Director {name:'Patty Jenkins'})\n" + //

View File

@@ -15,6 +15,7 @@
*/
package org.springframework.data.neo4j.repository;
import static org.assertj.core.api.Assertions.*;
import static org.hamcrest.CoreMatchers.*;
import static org.junit.Assert.*;
@@ -91,6 +92,18 @@ public class Neo4jRepositoryTests extends MultiDriverTestClass {
assertThat(nodeWithUUIDAsIdRepository.count(), is(0L));
}
@Test // DATAGRAPH-1286
public void findByIdEqualsInDerivedQueryMethodShouldWork() {
NodeWithUUIDAsId entity = new NodeWithUUIDAsId("someProperty");
nodeWithUUIDAsIdRepository.save(entity);
Optional<NodeWithUUIDAsId> retrievedEntity = nodeWithUUIDAsIdRepository
.findOneByMyNiceIdAndSomeProperty(entity.getMyNiceId(), entity.getSomeProperty());
assertThat(retrievedEntity.isPresent()).isTrue();
assertThat(retrievedEntity.get()).isEqualTo(entity);
}
@Configuration
@EnableNeo4jRepositories
@EnableTransactionManagement
@@ -111,4 +124,7 @@ public class Neo4jRepositoryTests extends MultiDriverTestClass {
interface SampleEntityRepository extends Neo4jRepository<SampleEntity, Long> {}
interface NodeWithUUIDAsIdRepository extends Neo4jRepository<NodeWithUUIDAsId, UUID> {}
interface NodeWithUUIDAsIdRepository extends Neo4jRepository<NodeWithUUIDAsId, UUID> {
Optional<NodeWithUUIDAsId> findOneByMyNiceIdAndSomeProperty(UUID id, String someProperty);
}