Skip to content

Commit 0eb3ced

Browse files
committed
feat(object-mapping): support mapping types with restricted access
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 will prefer 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 it in 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 891ca3f commit 0eb3ced

File tree

5 files changed

+147
-21
lines changed

5 files changed

+147
-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;
@@ -724,8 +725,14 @@ public interface Value extends MapAccessor, MapAccessorWithDefaultValue {
724725
* <li>Maximum matching properties.</li>
725726
* <li>Minimum mismatching properties.</li>
726727
* </ol>
727-
* The constructor search is done in the order defined by the {@link Class#getDeclaredConstructors} and is
728-
* finished either when a full match is found with no mismatches or once all constructors have been visited.
728+
* The constructor search is done in the order defined by the {@link Class#getDeclaredConstructors}.
729+
* <p>
730+
* Only constructors that are accessible or can be made accessible using {@link AccessibleObject#trySetAccessible()}
731+
* are included in the search. If multiple constructors have the same number of matching and mismatching
732+
* properties, the first constructor that is accessible by default is selected.
733+
* <p>
734+
* The search finishes as soon as a constructor that is accessible by default and matches all properties is found.
735+
* Otherwise, it finishes once all constructors have been visited.
729736
* <p>
730737
* At least 1 property match must be present for mapping to work.
731738
* <p>

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

Lines changed: 50 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,31 +20,46 @@
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 {
53+
if (constructor.trySetAccessible()) {
54+
bestPropertiesMatch = (PropertiesMatch<T>) matchNumbers;
55+
// no break as an accessible may be available
56+
}
57+
}
58+
} else if (matchNumbers.match() == bestPropertiesMatch.match()
59+
&& matchNumbers.mismatch() == bestPropertiesMatch.mismatch()
60+
&& matchNumbers.isAccessible()
61+
&& !bestPropertiesMatch.isAccessible()) {
62+
// identical match, but the new one is accessible
4863
bestPropertiesMatch = (PropertiesMatch<T>) matchNumbers;
4964
if (bestPropertiesMatch.match() == propertyNamesSize && bestPropertiesMatch.mismatch() == 0) {
5065
break;
@@ -66,19 +81,37 @@ private <T> PropertiesMatch<T> matchPropertyNames(MapAccessor mapAccessor, Const
6681
for (var parameter : parameters) {
6782
var propertyNameAnnotation = parameter.getAnnotation(Property.class);
6883
var propertyName = propertyNameAnnotation != null ? propertyNameAnnotation.value() : parameter.getName();
69-
var value = mapAccessor.get(propertyName);
70-
if (value != null) {
84+
if (contains(mapAccessor, propertyName)) {
7185
match++;
7286
} else {
7387
mismatch++;
7488
}
7589
arguments.add(new Argument(
76-
propertyName,
77-
parameter.getParameterizedType(),
78-
value != null ? (InternalValue) value : (InternalValue) Values.NULL));
90+
propertyName, parameter.getParameterizedType(), (InternalValue) mapAccessor.get(propertyName)));
91+
}
92+
return new PropertiesMatch<>(match, mismatch, constructor, arguments, isAccessible(constructor));
93+
}
94+
95+
private boolean contains(MapAccessor mapAccessor, String propertyName) {
96+
if (mapAccessor instanceof Value value) {
97+
if (ENTITY_TYPES.contains(value.type())) {
98+
return value.asEntity().containsKey(propertyName);
99+
} else {
100+
return mapAccessor.containsKey(propertyName);
101+
}
102+
} else {
103+
return mapAccessor.containsKey(propertyName);
104+
}
105+
}
106+
107+
private boolean isAccessible(Constructor<?> constructor) {
108+
try {
109+
return constructor.canAccess(null);
110+
} catch (Exception e) {
111+
return false;
79112
}
80-
return new PropertiesMatch<>(match, mismatch, constructor, arguments);
81113
}
82114

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

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: 86 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,88 @@ 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 shouldWorkWithPrivateRecord() {
216+
// given
217+
var string = "string";
218+
var bool = false;
219+
220+
var properties =
221+
Map.ofEntries(Map.entry("string", Values.value(string)), Map.entry("bool", Values.value(bool)));
222+
223+
// when
224+
var valueHolder = Values.value(properties).as(PrivateRecord.class);
225+
226+
// then
227+
assertEquals(string, valueHolder.string());
228+
assertEquals(bool, valueHolder.bool());
229+
}
230+
231+
private record PrivateRecord(String string, boolean bool) {}
232+
233+
@Test
234+
void shouldSelectAccessibleOnIdenticalMatch() {
235+
// given
236+
var string = "string";
237+
var bool = false;
238+
var number = 0;
239+
240+
var properties = Map.ofEntries(
241+
Map.entry("string", Values.value(string)),
242+
Map.entry("bool", Values.value(bool)),
243+
Map.entry("number", Values.value(number)));
244+
245+
// when
246+
var valueHolder = Values.value(properties).as(IdenticalMatch.class);
247+
248+
// then
249+
assertEquals(string, valueHolder.string);
250+
assertEquals(number, valueHolder.number);
251+
}
252+
253+
public static class IdenticalMatch {
254+
String string;
255+
boolean bool;
256+
long number;
257+
258+
private IdenticalMatch(@Property("string") String string, @Property("bool") boolean bool) {
259+
this.string = string;
260+
this.bool = bool;
261+
}
262+
263+
public IdenticalMatch(@Property("string") String string, @Property("number") int number) {
264+
this.string = string;
265+
this.number = number;
266+
}
267+
}
268+
269+
@Test
270+
void shouldFindNoMatch() {
271+
// given
272+
var properties = Map.ofEntries(Map.entry("value", Values.value("value")));
273+
274+
// when & then
275+
var exception = assertThrows(
276+
ValueException.class, () -> Values.value(properties).as(ValueHolder.class));
277+
}
192278
}

0 commit comments

Comments
 (0)