Skip to content

Commit 94d3430

Browse files
committed
feat(object-mapping): support mapping types with restricted access (neo4j#1668)
The updated Object Mapping implementation attempts to make constructors with restricted access available to it. If successful, it considers them during the constructor search. However, it prefers constructors that are accessible by default when they have the same number of matched and mismatched properties. Given the objective of providing an easy and type-safe way of accessing user-defined values in `MapAccessor`, it would would sometimes to convenient to define the target type and map to it within the same method. For example: ```java record Movie(String title, String tagline, long released) {} var movies = driver.executableQuery("MATCH (movie:Movie) RETURN movie") .execute() .records() .stream() .map(record -> record.get("movie").as(Movie.class)) .toList(); ``` However, such Java `record` has restricted access from Object Mapping implementation perspective. This update makes it possible to map to such types. In addition, the following bugs have been fixed: - `null` type name was used in exception message when local `record` mapping failed - nonexistent properties were considered as matched on nodes and relationships
1 parent a6328e4 commit 94d3430

File tree

5 files changed

+176
-21
lines changed

5 files changed

+176
-21
lines changed

driver/src/main/java/org/neo4j/driver/Value.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
*/
1717
package org.neo4j.driver;
1818

19+
import java.lang.reflect.AccessibleObject;
1920
import java.time.DateTimeException;
2021
import java.time.LocalDate;
2122
import java.time.LocalDateTime;
@@ -656,8 +657,14 @@ public interface Value extends MapAccessor, MapAccessorWithDefaultValue {
656657
* <li>Maximum matching properties.</li>
657658
* <li>Minimum mismatching properties.</li>
658659
* </ol>
659-
* The constructor search is done in the order defined by the {@link Class#getDeclaredConstructors} and is
660-
* finished either when a full match is found with no mismatches or once all constructors have been visited.
660+
* The constructor search is done in the order defined by the {@link Class#getDeclaredConstructors}.
661+
* <p>
662+
* Only constructors that are accessible or can be made accessible using {@link AccessibleObject#trySetAccessible()}
663+
* are included in the search. If multiple constructors have the same number of matching and mismatching
664+
* properties, the first constructor that is accessible by default is selected.
665+
* <p>
666+
* The search finishes as soon as a constructor that is accessible by default and matches all properties is found.
667+
* Otherwise, it finishes once all constructors have been visited.
661668
* <p>
662669
* At least 1 property match must be present for mapping to work.
663670
* <p>

driver/src/main/java/org/neo4j/driver/internal/value/mapping/ConstructorFinder.java

Lines changed: 48 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,31 +20,44 @@
2020
import java.util.ArrayList;
2121
import java.util.List;
2222
import java.util.Optional;
23-
import org.neo4j.driver.Values;
23+
import java.util.Set;
24+
import org.neo4j.driver.Value;
2425
import org.neo4j.driver.internal.value.InternalValue;
2526
import org.neo4j.driver.mapping.Property;
2627
import org.neo4j.driver.types.MapAccessor;
28+
import org.neo4j.driver.types.Type;
29+
import org.neo4j.driver.types.TypeSystem;
2730

2831
class ConstructorFinder {
32+
private static final TypeSystem TS = TypeSystem.getDefault();
33+
34+
private static final Set<Type> ENTITY_TYPES = Set.of(TS.NODE(), TS.RELATIONSHIP());
35+
2936
@SuppressWarnings("unchecked")
3037
public <T> Optional<ObjectMetadata<T>> findConstructor(MapAccessor mapAccessor, Class<T> targetClass) {
3138
PropertiesMatch<T> bestPropertiesMatch = null;
3239
var constructors = targetClass.getDeclaredConstructors();
3340
var propertyNamesSize = mapAccessor.size();
3441
for (var constructor : constructors) {
35-
var accessible = false;
36-
try {
37-
accessible = constructor.canAccess(null);
38-
} catch (Throwable e) {
39-
// ignored
40-
}
41-
if (!accessible) {
42-
continue;
43-
}
4442
var matchNumbers = matchPropertyNames(mapAccessor, constructor);
4543
if (bestPropertiesMatch == null
4644
|| (matchNumbers.match() >= bestPropertiesMatch.match()
4745
&& matchNumbers.mismatch() < bestPropertiesMatch.mismatch())) {
46+
// no match yet or better match
47+
if (matchNumbers.isAccessible()) {
48+
bestPropertiesMatch = (PropertiesMatch<T>) matchNumbers;
49+
if (bestPropertiesMatch.match() == propertyNamesSize && bestPropertiesMatch.mismatch() == 0) {
50+
break;
51+
}
52+
} else if (constructor.trySetAccessible()) {
53+
bestPropertiesMatch = (PropertiesMatch<T>) matchNumbers;
54+
// no break as an accessible may be available
55+
}
56+
} else if (matchNumbers.match() == bestPropertiesMatch.match()
57+
&& matchNumbers.mismatch() == bestPropertiesMatch.mismatch()
58+
&& matchNumbers.isAccessible()
59+
&& !bestPropertiesMatch.isAccessible()) {
60+
// identical match, but the new one is accessible
4861
bestPropertiesMatch = (PropertiesMatch<T>) matchNumbers;
4962
if (bestPropertiesMatch.match() == propertyNamesSize && bestPropertiesMatch.mismatch() == 0) {
5063
break;
@@ -66,19 +79,37 @@ private <T> PropertiesMatch<T> matchPropertyNames(MapAccessor mapAccessor, Const
6679
for (var parameter : parameters) {
6780
var propertyNameAnnotation = parameter.getAnnotation(Property.class);
6881
var propertyName = propertyNameAnnotation != null ? propertyNameAnnotation.value() : parameter.getName();
69-
var value = mapAccessor.get(propertyName);
70-
if (value != null) {
82+
if (contains(mapAccessor, propertyName)) {
7183
match++;
7284
} else {
7385
mismatch++;
7486
}
7587
arguments.add(new Argument(
76-
propertyName,
77-
parameter.getParameterizedType(),
78-
value != null ? (InternalValue) value : (InternalValue) Values.NULL));
88+
propertyName, parameter.getParameterizedType(), (InternalValue) mapAccessor.get(propertyName)));
89+
}
90+
return new PropertiesMatch<>(match, mismatch, constructor, arguments, isAccessible(constructor));
91+
}
92+
93+
private boolean contains(MapAccessor mapAccessor, String propertyName) {
94+
if (mapAccessor instanceof Value value) {
95+
if (ENTITY_TYPES.contains(value.type())) {
96+
return value.asEntity().containsKey(propertyName);
97+
} else {
98+
return mapAccessor.containsKey(propertyName);
99+
}
100+
} else {
101+
return mapAccessor.containsKey(propertyName);
102+
}
103+
}
104+
105+
private boolean isAccessible(Constructor<?> constructor) {
106+
try {
107+
return constructor.canAccess(null);
108+
} catch (Exception e) {
109+
return false;
79110
}
80-
return new PropertiesMatch<>(match, mismatch, constructor, arguments);
81111
}
82112

83-
private record PropertiesMatch<T>(int match, int mismatch, Constructor<T> constructor, List<Argument> arguments) {}
113+
private record PropertiesMatch<T>(
114+
int match, int mismatch, Constructor<T> constructor, List<Argument> arguments, boolean isAccessible) {}
84115
}

driver/src/main/java/org/neo4j/driver/internal/value/mapping/ObjectInstantiator.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class ObjectInstantiator {
2323

2424
<T> T instantiate(ObjectMetadata<T> metadata) {
2525
var constructor = metadata.constructor();
26-
var targetTypeName = constructor.getDeclaringClass().getCanonicalName();
26+
var targetTypeName = constructor.getDeclaringClass().getName();
2727
var initargs = initargs(targetTypeName, metadata.arguments());
2828
try {
2929
return constructor.newInstance(initargs);

driver/src/main/java/org/neo4j/driver/internal/value/mapping/ObjectMapper.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,6 @@ public T map(MapAccessor mapAccessor, Class<T> targetClass) {
4545
.findConstructor(mapAccessor, targetClass)
4646
.map(OBJECT_INSTANTIATOR::instantiate)
4747
.orElseThrow(() -> new ValueException(
48-
"No suitable constructor has been found for '%s'".formatted(targetClass.getCanonicalName())));
48+
"No suitable constructor has been found for '%s'".formatted(targetClass.getName())));
4949
}
5050
}

driver/src/test/java/org/neo4j/driver/ObjectMappingTests.java

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import static org.junit.jupiter.api.Assertions.assertEquals;
2020
import static org.junit.jupiter.api.Assertions.assertNull;
21+
import static org.junit.jupiter.api.Assertions.assertThrows;
2122

2223
import java.time.Duration;
2324
import java.time.LocalDate;
@@ -37,6 +38,7 @@
3738
import org.junit.jupiter.params.ParameterizedTest;
3839
import org.junit.jupiter.params.provider.Arguments;
3940
import org.junit.jupiter.params.provider.MethodSource;
41+
import org.neo4j.driver.exceptions.value.ValueException;
4042
import org.neo4j.driver.internal.InternalIsoDuration;
4143
import org.neo4j.driver.internal.InternalNode;
4244
import org.neo4j.driver.internal.InternalPoint2D;
@@ -189,4 +191,119 @@ public ValueHolderWithOptionalNumber(
189191
this(string, bytes, bool, Long.MIN_VALUE);
190192
}
191193
}
194+
195+
@Test
196+
void shouldWorkWithLocalRecord() {
197+
// given
198+
var string = "string";
199+
var bool = false;
200+
201+
var properties =
202+
Map.ofEntries(Map.entry("string", Values.value(string)), Map.entry("bool", Values.value(bool)));
203+
204+
record LocalRecord(String string, boolean bool) {}
205+
206+
// when
207+
var valueHolder = Values.value(properties).as(LocalRecord.class);
208+
209+
// then
210+
assertEquals(string, valueHolder.string());
211+
assertEquals(bool, valueHolder.bool());
212+
}
213+
214+
@Test
215+
void shouldAcceptLocalRecordAsValue() {
216+
// given
217+
var string = "string";
218+
var bool = false;
219+
record LocalRecord(String string, boolean bool) {}
220+
var recordValue = new LocalRecord(string, bool);
221+
222+
// when
223+
var value = Values.value(recordValue);
224+
225+
// then
226+
assertEquals(string, value.get("string").asString());
227+
assertEquals(bool, value.get("bool").asBoolean());
228+
}
229+
230+
@Test
231+
void shouldWorkWithPrivateRecord() {
232+
// given
233+
var string = "string";
234+
var bool = false;
235+
236+
var properties =
237+
Map.ofEntries(Map.entry("string", Values.value(string)), Map.entry("bool", Values.value(bool)));
238+
239+
// when
240+
var valueHolder = Values.value(properties).as(PrivateRecord.class);
241+
242+
// then
243+
assertEquals(string, valueHolder.string());
244+
assertEquals(bool, valueHolder.bool());
245+
}
246+
247+
@Test
248+
void shouldAcceptPrivateRecordAsValue() {
249+
// given
250+
var string = "string";
251+
var bool = false;
252+
var recordValue = new PrivateRecord(string, bool);
253+
254+
// when
255+
var value = Values.value(recordValue);
256+
257+
// then
258+
assertEquals(string, value.get("string").asString());
259+
assertEquals(bool, value.get("bool").asBoolean());
260+
}
261+
262+
private record PrivateRecord(String string, boolean bool) {}
263+
264+
@Test
265+
void shouldSelectAccessibleOnIdenticalMatch() {
266+
// given
267+
var string = "string";
268+
var bool = false;
269+
var number = 0;
270+
271+
var properties = Map.ofEntries(
272+
Map.entry("string", Values.value(string)),
273+
Map.entry("bool", Values.value(bool)),
274+
Map.entry("number", Values.value(number)));
275+
276+
// when
277+
var valueHolder = Values.value(properties).as(IdenticalMatch.class);
278+
279+
// then
280+
assertEquals(string, valueHolder.string);
281+
assertEquals(number, valueHolder.number);
282+
}
283+
284+
public static class IdenticalMatch {
285+
String string;
286+
boolean bool;
287+
long number;
288+
289+
private IdenticalMatch(@Property("string") String string, @Property("bool") boolean bool) {
290+
this.string = string;
291+
this.bool = bool;
292+
}
293+
294+
public IdenticalMatch(@Property("string") String string, @Property("number") int number) {
295+
this.string = string;
296+
this.number = number;
297+
}
298+
}
299+
300+
@Test
301+
void shouldFindNoMatch() {
302+
// given
303+
var properties = Map.ofEntries(Map.entry("value", Values.value("value")));
304+
305+
// when & then
306+
var exception = assertThrows(
307+
ValueException.class, () -> Values.value(properties).as(ValueHolder.class));
308+
}
192309
}

0 commit comments

Comments
 (0)