Skip to content

Commit 3871020

Browse files
GH-2371 - Respect @Property information in projection.
Domain attributes can be renamed using the `@Property` annotation when being mapped to and read from graph properties. That renaming must also happening when using projections. As with #2451 in 582fd7d this is kind of a problem with the existing filter mechanism that works after the stack has dealt with the actual properties. After all properties and paths are computed, they will be translated as late as possible before checked for inclusions: - When writing just before it is checked whether to add them to the properties parameter or not - When reading after they have been map-projected from the database into the result set but before they are checked for inclusions. Also addressed here is the a workaround for writes: In case a user had an interface based projection and used the graph property name as accessor, a write would succeed, however a read would fail. A property accessor has now been registered with the default projection factory that translates a failure to access an entity property (attribute) once. This fixes #2371.
1 parent 35a9550 commit 3871020

File tree

9 files changed

+471
-29
lines changed

9 files changed

+471
-29
lines changed

src/main/java/org/springframework/data/neo4j/core/mapping/PropertyFilter.java

+73-8
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,18 @@
1515
*/
1616
package org.springframework.data.neo4j.core.mapping;
1717

18-
import org.apiguardian.api.API;
19-
import org.springframework.data.mapping.PropertyPath;
20-
import org.springframework.util.StringUtils;
21-
2218
import java.util.HashSet;
2319
import java.util.Map;
2420
import java.util.Optional;
2521
import java.util.Set;
2622
import java.util.concurrent.ConcurrentHashMap;
2723

24+
import org.apiguardian.api.API;
25+
import org.springframework.data.mapping.PropertyPath;
26+
import org.springframework.data.neo4j.core.schema.Property;
27+
import org.springframework.lang.Nullable;
28+
import org.springframework.util.StringUtils;
29+
2830
/**
2931
* Something that makes sense of propertyPaths by having an understanding of projection classes.
3032
*/
@@ -45,6 +47,24 @@ public static PropertyFilter acceptAll() {
4547

4648
public abstract boolean isNotFiltering();
4749

50+
static String toDotPath(PropertyPath propertyPath, @Nullable String lastSegment) {
51+
52+
if (lastSegment == null) {
53+
return propertyPath.toDotPath();
54+
}
55+
StringBuilder dotPath = new StringBuilder();
56+
while (propertyPath != null) {
57+
if (propertyPath.hasNext()) {
58+
dotPath.append(propertyPath.getSegment()).append(".");
59+
propertyPath = propertyPath.next();
60+
} else {
61+
break;
62+
}
63+
}
64+
dotPath.append(lastSegment);
65+
return dotPath.toString();
66+
}
67+
4868
private static class FilteringPropertyFilter extends PropertyFilter {
4969
private final Set<Class<?>> rootClasses;
5070
private final Map<String, Boolean> projectingPropertyPaths;
@@ -64,10 +84,23 @@ private FilteringPropertyFilter(Map<PropertyPath, Boolean> propertiesMap, NodeDe
6484
.map(NodeDescription::getUnderlyingClass)
6585
.forEach(rootClasses::add);
6686

87+
Neo4jPersistentEntity<?> entity = (Neo4jPersistentEntity<?>) nodeDescription;
6788
projectingPropertyPaths = new ConcurrentHashMap<>();
6889
propertiesMap.keySet()
69-
.forEach(propertyPath ->
70-
projectingPropertyPaths.put(propertyPath.toDotPath(), propertiesMap.get(propertyPath)));
90+
.forEach(propertyPath -> {
91+
String lastSegment = null;
92+
if (!propertyPath.hasNext()) {
93+
Neo4jPersistentProperty property = entity.getPersistentProperty(
94+
propertyPath.getLeafProperty().getSegment());
95+
if (property != null && property.findAnnotation(Property.class) != null) {
96+
lastSegment = property.getPropertyName();
97+
}
98+
}
99+
projectingPropertyPaths.put(
100+
PropertyFilter.toDotPath(propertyPath, lastSegment),
101+
propertiesMap.get(propertyPath)
102+
);
103+
});
71104
}
72105

73106
@Override
@@ -87,7 +120,8 @@ public boolean contains(String dotPath, Class<?> typeToCheck) {
87120

88121
return Integer.compare(depth2, depth1);
89122
})
90-
.filter(d -> dotPath.contains(d) && dotPath.startsWith(d)).findFirst();
123+
.filter(d -> dotPath.contains(d) && dotPath.startsWith(d))
124+
.findFirst();
91125

92126
return projectingPropertyPaths.containsKey(dotPath)
93127
|| (dotPath.contains(".") && candidate.isPresent() && projectingPropertyPaths.get(candidate.get()));
@@ -141,6 +175,19 @@ public String toDotPath() {
141175
return dotPath;
142176
}
143177

178+
public String toDotPath(@Nullable String lastSegment) {
179+
180+
if (lastSegment == null) {
181+
return this.toDotPath();
182+
}
183+
184+
int idx = dotPath.lastIndexOf('.');
185+
if (idx < 0) {
186+
return lastSegment;
187+
}
188+
return dotPath.substring(0, idx + 1) + lastSegment;
189+
}
190+
144191
public Class<?> getType() {
145192
return type;
146193
}
@@ -165,6 +212,24 @@ private String appendToDotPath(String pathPart) {
165212
private String prependDotPathWith(String pathPart) {
166213
return dotPath.isEmpty() ? pathPart : pathPart + "." + dotPath;
167214
}
168-
}
169215

216+
public String getSegment() {
217+
218+
int idx = dotPath.indexOf(".");
219+
if (idx < 0) {
220+
idx = dotPath.length();
221+
}
222+
return dotPath.substring(0, idx);
223+
}
224+
225+
public RelaxedPropertyPath getLeafProperty() {
226+
227+
int idx = dotPath.lastIndexOf('.');
228+
if (idx < 0) {
229+
return this;
230+
}
231+
232+
return new RelaxedPropertyPath(dotPath.substring(idx + 1), this.type);
233+
}
234+
}
170235
}

src/main/java/org/springframework/data/neo4j/repository/query/QueryFragments.java

+13-3
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,11 @@
3333
import org.neo4j.cypherdsl.core.StatementBuilder;
3434
import org.springframework.data.mapping.PropertyPath;
3535
import org.springframework.data.neo4j.core.mapping.CypherGenerator;
36+
import org.springframework.data.neo4j.core.mapping.Neo4jPersistentProperty;
3637
import org.springframework.data.neo4j.core.mapping.PropertyFilter;
3738
import org.springframework.data.neo4j.core.mapping.Neo4jPersistentEntity;
3839
import org.springframework.data.neo4j.core.mapping.NodeDescription;
40+
import org.springframework.data.neo4j.core.schema.Property;
3941
import org.springframework.lang.Nullable;
4042

4143
/**
@@ -86,9 +88,7 @@ public void setReturnExpression(Expression returnExpression, boolean isScalarVal
8688
}
8789

8890
public boolean includeField(PropertyFilter.RelaxedPropertyPath fieldName) {
89-
return this.returnTuple == null
90-
? PropertyFilter.acceptAll().contains(fieldName.toDotPath(), fieldName.getType())
91-
: this.returnTuple.filteredProperties.contains(fieldName.toDotPath(), fieldName.getType());
91+
return this.returnTuple == null || this.returnTuple.include(fieldName);
9292
}
9393

9494
public void setOrderBy(Collection<SortItem> orderBy) {
@@ -179,5 +179,15 @@ private ReturnTuple(NodeDescription<?> nodeDescription, Map<PropertyPath, Boolea
179179
this.filteredProperties = PropertyFilter.from(filteredProperties, nodeDescription);
180180
this.isDistinct = isDistinct;
181181
}
182+
183+
boolean include(PropertyFilter.RelaxedPropertyPath fieldName) {
184+
String dotPath = nodeDescription.getGraphProperty(fieldName.getSegment())
185+
.filter(Neo4jPersistentProperty.class::isInstance)
186+
.map(Neo4jPersistentProperty.class::cast)
187+
.filter(p -> p.findAnnotation(Property.class) != null)
188+
.map(p -> fieldName.toDotPath(p.getPropertyName()))
189+
.orElseGet(fieldName::toDotPath);
190+
return this.filteredProperties.contains(dotPath, fieldName.getType());
191+
}
182192
}
183193
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
/*
2+
* Copyright 2011-2022 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.neo4j.repository.support;
17+
18+
import java.beans.PropertyDescriptor;
19+
import java.lang.reflect.Method;
20+
import java.util.concurrent.atomic.AtomicReference;
21+
22+
import org.aopalliance.intercept.MethodInterceptor;
23+
import org.aopalliance.intercept.MethodInvocation;
24+
import org.springframework.beans.BeanUtils;
25+
import org.springframework.beans.BeanWrapper;
26+
import org.springframework.beans.NotReadablePropertyException;
27+
import org.springframework.data.neo4j.core.mapping.Neo4jMappingContext;
28+
import org.springframework.data.neo4j.core.mapping.Neo4jPersistentEntity;
29+
import org.springframework.data.neo4j.core.mapping.Neo4jPersistentProperty;
30+
import org.springframework.data.neo4j.core.schema.Property;
31+
import org.springframework.data.projection.MethodInterceptorFactory;
32+
import org.springframework.data.util.DirectFieldAccessFallbackBeanWrapper;
33+
import org.springframework.lang.Nullable;
34+
import org.springframework.util.Assert;
35+
import org.springframework.util.ReflectionUtils;
36+
37+
/**
38+
* Basically a lenient property accessing method interceptor, first trying the entity property (or attribute), than
39+
* a potentially renamed attribute via {@link Property}.
40+
*
41+
* @author Michael J. Simons
42+
*/
43+
final class EntityAndGraphPropertyAccessingMethodInterceptor implements MethodInterceptor {
44+
45+
static MethodInterceptorFactory createMethodInterceptorFactory(Neo4jMappingContext mappingContext) {
46+
return new MethodInterceptorFactory() {
47+
@Override
48+
public MethodInterceptor createMethodInterceptor(Object source, Class<?> targetType) {
49+
return new EntityAndGraphPropertyAccessingMethodInterceptor(source, mappingContext);
50+
}
51+
52+
@Override public boolean supports(Object source, Class<?> targetType) {
53+
return true;
54+
}
55+
};
56+
}
57+
58+
private final BeanWrapper target;
59+
60+
private EntityAndGraphPropertyAccessingMethodInterceptor(Object target, Neo4jMappingContext ctx) {
61+
62+
Assert.notNull(target, "Proxy target must not be null!");
63+
this.target = new GraphPropertyAndDirectFieldAccessFallbackBeanWrapper(target, ctx);
64+
}
65+
66+
@Nullable
67+
@Override
68+
public Object invoke(@SuppressWarnings("null") MethodInvocation invocation) throws Throwable {
69+
70+
Method method = invocation.getMethod();
71+
72+
if (ReflectionUtils.isObjectMethod(method)) {
73+
return invocation.proceed();
74+
}
75+
76+
PropertyDescriptor descriptor = BeanUtils.findPropertyForMethod(method);
77+
78+
if (descriptor == null) {
79+
throw new IllegalStateException("Invoked method is not a property accessor!");
80+
}
81+
82+
if (!isSetterMethod(method, descriptor)) {
83+
return target.getPropertyValue(descriptor.getName());
84+
}
85+
86+
if (invocation.getArguments().length != 1) {
87+
throw new IllegalStateException("Invoked setter method requires exactly one argument!");
88+
}
89+
90+
target.setPropertyValue(descriptor.getName(), invocation.getArguments()[0]);
91+
return null;
92+
}
93+
94+
private static boolean isSetterMethod(Method method, PropertyDescriptor descriptor) {
95+
return method.equals(descriptor.getWriteMethod());
96+
}
97+
98+
/**
99+
* this version of the {@link DirectFieldAccessFallbackBeanWrapper} checks if there's an attribute on the entity
100+
* annotated with {@link Property} mapping it to a different graph property when it fails to access the original
101+
* attribute If so, that property is accessed. If not, the original exception is rethrown.
102+
* This helps in projections such as described here
103+
* https://stackoverflow.com/questions/68938823/sdn6-projection-interfaces-with-property-mapping
104+
* that could have been used as workaround prior to fixing 2371.
105+
*/
106+
static class GraphPropertyAndDirectFieldAccessFallbackBeanWrapper extends DirectFieldAccessFallbackBeanWrapper {
107+
108+
private final Neo4jMappingContext ctx;
109+
110+
GraphPropertyAndDirectFieldAccessFallbackBeanWrapper(Object target, Neo4jMappingContext ctx) {
111+
super(target);
112+
this.ctx = ctx;
113+
}
114+
115+
@Override
116+
public Object getPropertyValue(String propertyName) {
117+
try {
118+
return super.getPropertyValue(propertyName);
119+
} catch (NotReadablePropertyException e) {
120+
Neo4jPersistentEntity<?> entity = ctx.getPersistentEntity(super.getRootClass());
121+
122+
AtomicReference<String> value = new AtomicReference<>();
123+
if (entity != null) {
124+
entity.doWithProperties(
125+
(org.springframework.data.mapping.PropertyHandler<Neo4jPersistentProperty>) p -> {
126+
if (p.findAnnotation(Property.class) != null && p.getPropertyName()
127+
.equals(propertyName)) {
128+
value.compareAndSet(null, p.getFieldName());
129+
}
130+
});
131+
if (value.get() != null) {
132+
return super.getPropertyValue(value.get());
133+
}
134+
}
135+
throw e;
136+
}
137+
}
138+
}
139+
}

src/main/java/org/springframework/data/neo4j/repository/support/Neo4jRepositoryFactory.java

+13
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
import org.springframework.data.neo4j.repository.query.Neo4jQueryLookupStrategy;
2626
import org.springframework.data.neo4j.repository.query.QuerydslNeo4jPredicateExecutor;
2727
import org.springframework.data.neo4j.repository.query.SimpleQueryByExampleExecutor;
28+
import org.springframework.data.projection.ProjectionFactory;
29+
import org.springframework.data.projection.SpelAwareProxyProjectionFactory;
2830
import org.springframework.data.querydsl.QuerydslPredicateExecutor;
2931
import org.springframework.data.querydsl.QuerydslUtils;
3032
import org.springframework.data.repository.core.RepositoryInformation;
@@ -122,4 +124,15 @@ protected Optional<QueryLookupStrategy> getQueryLookupStrategy(Key key,
122124

123125
return Optional.of(new Neo4jQueryLookupStrategy(neo4jOperations, mappingContext, evaluationContextProvider));
124126
}
127+
128+
@Override
129+
protected ProjectionFactory getProjectionFactory() {
130+
131+
ProjectionFactory projectionFactory = super.getProjectionFactory();
132+
if (projectionFactory instanceof SpelAwareProxyProjectionFactory) {
133+
((SpelAwareProxyProjectionFactory) projectionFactory).registerMethodInvokerFactory(
134+
EntityAndGraphPropertyAccessingMethodInterceptor.createMethodInterceptorFactory(mappingContext));
135+
}
136+
return projectionFactory;
137+
}
125138
}

src/main/java/org/springframework/data/neo4j/repository/support/ReactiveNeo4jRepositoryFactory.java

+13
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
import org.springframework.data.neo4j.repository.query.ReactiveNeo4jQueryLookupStrategy;
2828
import org.springframework.data.neo4j.repository.query.ReactiveQuerydslNeo4jPredicateExecutor;
2929
import org.springframework.data.neo4j.repository.query.SimpleReactiveQueryByExampleExecutor;
30+
import org.springframework.data.projection.ProjectionFactory;
31+
import org.springframework.data.projection.SpelAwareProxyProjectionFactory;
3032
import org.springframework.data.querydsl.QuerydslUtils;
3133
import org.springframework.data.querydsl.ReactiveQuerydslPredicateExecutor;
3234
import org.springframework.data.repository.core.RepositoryInformation;
@@ -132,4 +134,15 @@ public void setBeanFactory(BeanFactory beanFactory) throws BeansException {
132134
});
133135
}
134136
}
137+
138+
@Override
139+
protected ProjectionFactory getProjectionFactory() {
140+
141+
ProjectionFactory projectionFactory = super.getProjectionFactory();
142+
if (projectionFactory instanceof SpelAwareProxyProjectionFactory) {
143+
((SpelAwareProxyProjectionFactory) projectionFactory).registerMethodInvokerFactory(
144+
EntityAndGraphPropertyAccessingMethodInterceptor.createMethodInterceptorFactory(mappingContext));
145+
}
146+
return projectionFactory;
147+
}
135148
}

0 commit comments

Comments
 (0)