feat(object-mapping): support mapping types with restricted access (#1668)

injectives · web-flow · commit 26731c6da5a7 · 2025-07-07T15:42:25.000+01:00
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 -&gt; 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
diff --git a/driver/src/main/java/org/neo4j/driver/Value.java b/driver/src/main/java/org/neo4j/driver/Value.java
@@ -16,6 +16,7 @@
  */
 package org.neo4j.driver;
 
+import java.lang.reflect.AccessibleObject;
 import java.time.DateTimeException;
 import java.time.LocalDate;
 import java.time.LocalDateTime;
@@ -724,8 +725,14 @@ public interface Value extends MapAccessor, MapAccessorWithDefaultValue {
      *     <li>Maximum matching properties.</li>
      *     <li>Minimum mismatching properties.</li>
      * </ol>
-     * The constructor search is done in the order defined by the {@link Class#getDeclaredConstructors} and is
-     * finished either when a full match is found with no mismatches or once all constructors have been visited.
+     * The constructor search is done in the order defined by the {@link Class#getDeclaredConstructors}.
+     * <p>
+     * Only constructors that are accessible or can be made accessible using {@link AccessibleObject#trySetAccessible()}
+     * are included in the search. If multiple constructors have the same number of matching and mismatching
+     * properties, the first constructor that is accessible by default is selected.
+     * <p>
+     * The search finishes as soon as a constructor that is accessible by default and matches all properties is found.
+     * Otherwise, it finishes once all constructors have been visited.
      * <p>
      * At least 1 property match must be present for mapping to work.
      * <p>
diff --git a/driver/src/main/java/org/neo4j/driver/internal/value/mapping/ConstructorFinder.java b/driver/src/main/java/org/neo4j/driver/internal/value/mapping/ConstructorFinder.java
@@ -20,31 +20,44 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
-import org.neo4j.driver.Values;
+import java.util.Set;
+import org.neo4j.driver.Value;
 import org.neo4j.driver.internal.value.InternalValue;
 import org.neo4j.driver.mapping.Property;
 import org.neo4j.driver.types.MapAccessor;
+import org.neo4j.driver.types.Type;
+import org.neo4j.driver.types.TypeSystem;
 
 class ConstructorFinder {
+    private static final TypeSystem TS = TypeSystem.getDefault();
+
+    private static final Set<Type> ENTITY_TYPES = Set.of(TS.NODE(), TS.RELATIONSHIP());
+
     @SuppressWarnings("unchecked")
     public <T> Optional<ObjectMetadata<T>> findConstructor(MapAccessor mapAccessor, Class<T> targetClass) {
         PropertiesMatch<T> bestPropertiesMatch = null;
         var constructors = targetClass.getDeclaredConstructors();
         var propertyNamesSize = mapAccessor.size();
         for (var constructor : constructors) {
-            var accessible = false;
-            try {
-                accessible = constructor.canAccess(null);
-            } catch (Throwable e) {
-                // ignored
-            }
-            if (!accessible) {
-                continue;
-            }
             var matchNumbers = matchPropertyNames(mapAccessor, constructor);
             if (bestPropertiesMatch == null
                     || (matchNumbers.match() >= bestPropertiesMatch.match()
                             && matchNumbers.mismatch() < bestPropertiesMatch.mismatch())) {
+                // no match yet or better match
+                if (matchNumbers.isAccessible()) {
+                    bestPropertiesMatch = (PropertiesMatch<T>) matchNumbers;
+                    if (bestPropertiesMatch.match() == propertyNamesSize && bestPropertiesMatch.mismatch() == 0) {
+                        break;
+                    }
+                } else if (constructor.trySetAccessible()) {
+                    bestPropertiesMatch = (PropertiesMatch<T>) matchNumbers;
+                    // no break as an accessible may be available
+                }
+            } else if (matchNumbers.match() == bestPropertiesMatch.match()
+                    && matchNumbers.mismatch() == bestPropertiesMatch.mismatch()
+                    && matchNumbers.isAccessible()
+                    && !bestPropertiesMatch.isAccessible()) {
+                // identical match, but the new one is accessible
                 bestPropertiesMatch = (PropertiesMatch<T>) matchNumbers;
                 if (bestPropertiesMatch.match() == propertyNamesSize && bestPropertiesMatch.mismatch() == 0) {
                     break;
@@ -66,19 +79,37 @@ private <T> PropertiesMatch<T> matchPropertyNames(MapAccessor mapAccessor, Const
         for (var parameter : parameters) {
             var propertyNameAnnotation = parameter.getAnnotation(Property.class);
             var propertyName = propertyNameAnnotation != null ? propertyNameAnnotation.value() : parameter.getName();
-            var value = mapAccessor.get(propertyName);
-            if (value != null) {
+            if (contains(mapAccessor, propertyName)) {
                 match++;
             } else {
                 mismatch++;
             }
             arguments.add(new Argument(
-                    propertyName,
-                    parameter.getParameterizedType(),
-                    value != null ? (InternalValue) value : (InternalValue) Values.NULL));
+                    propertyName, parameter.getParameterizedType(), (InternalValue) mapAccessor.get(propertyName)));
+        }
+        return new PropertiesMatch<>(match, mismatch, constructor, arguments, isAccessible(constructor));
+    }
+
+    private boolean contains(MapAccessor mapAccessor, String propertyName) {
+        if (mapAccessor instanceof Value value) {
+            if (ENTITY_TYPES.contains(value.type())) {
+                return value.asEntity().containsKey(propertyName);
+            } else {
+                return mapAccessor.containsKey(propertyName);
+            }
+        } else {
+            return mapAccessor.containsKey(propertyName);
+        }
+    }
+
+    private boolean isAccessible(Constructor<?> constructor) {
+        try {
+            return constructor.canAccess(null);
+        } catch (Exception e) {
+            return false;
         }
-        return new PropertiesMatch<>(match, mismatch, constructor, arguments);
     }
 
-    private record PropertiesMatch<T>(int match, int mismatch, Constructor<T> constructor, List<Argument> arguments) {}
+    private record PropertiesMatch<T>(
+            int match, int mismatch, Constructor<T> constructor, List<Argument> arguments, boolean isAccessible) {}
 }
diff --git a/driver/src/main/java/org/neo4j/driver/internal/value/mapping/ObjectInstantiator.java b/driver/src/main/java/org/neo4j/driver/internal/value/mapping/ObjectInstantiator.java
@@ -23,7 +23,7 @@ class ObjectInstantiator {
 
     <T> T instantiate(ObjectMetadata<T> metadata) {
         var constructor = metadata.constructor();
-        var targetTypeName = constructor.getDeclaringClass().getCanonicalName();
+        var targetTypeName = constructor.getDeclaringClass().getName();
         var initargs = initargs(targetTypeName, metadata.arguments());
         try {
             return constructor.newInstance(initargs);
diff --git a/driver/src/main/java/org/neo4j/driver/internal/value/mapping/ObjectMapper.java b/driver/src/main/java/org/neo4j/driver/internal/value/mapping/ObjectMapper.java
@@ -45,6 +45,6 @@ public T map(MapAccessor mapAccessor, Class<T> targetClass) {
                 .findConstructor(mapAccessor, targetClass)
                 .map(OBJECT_INSTANTIATOR::instantiate)
                 .orElseThrow(() -> new ValueException(
-                        "No suitable constructor has been found for '%s'".formatted(targetClass.getCanonicalName())));
+                        "No suitable constructor has been found for '%s'".formatted(targetClass.getName())));
     }
 }
diff --git a/driver/src/test/java/org/neo4j/driver/ObjectMappingTests.java b/driver/src/test/java/org/neo4j/driver/ObjectMappingTests.java
@@ -18,6 +18,7 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import java.time.Duration;
 import java.time.LocalDate;
@@ -37,6 +38,7 @@
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
+import org.neo4j.driver.exceptions.value.ValueException;
 import org.neo4j.driver.internal.InternalIsoDuration;
 import org.neo4j.driver.internal.InternalNode;
 import org.neo4j.driver.internal.InternalPoint2D;
@@ -189,4 +191,119 @@ public ValueHolderWithOptionalNumber(
             this(string, bytes, bool, Long.MIN_VALUE);
         }
     }
+
+    @Test
+    void shouldWorkWithLocalRecord() {
+        // given
+        var string = "string";
+        var bool = false;
+
+        var properties =
+                Map.ofEntries(Map.entry("string", Values.value(string)), Map.entry("bool", Values.value(bool)));
+
+        record LocalRecord(String string, boolean bool) {}
+
+        // when
+        var valueHolder = Values.value(properties).as(LocalRecord.class);
+
+        // then
+        assertEquals(string, valueHolder.string());
+        assertEquals(bool, valueHolder.bool());
+    }
+
+    @Test
+    void shouldAcceptLocalRecordAsValue() {
+        // given
+        var string = "string";
+        var bool = false;
+        record LocalRecord(String string, boolean bool) {}
+        var recordValue = new LocalRecord(string, bool);
+
+        // when
+        var value = Values.value(recordValue);
+
+        // then
+        assertEquals(string, value.get("string").asString());
+        assertEquals(bool, value.get("bool").asBoolean());
+    }
+
+    @Test
+    void shouldWorkWithPrivateRecord() {
+        // given
+        var string = "string";
+        var bool = false;
+
+        var properties =
+                Map.ofEntries(Map.entry("string", Values.value(string)), Map.entry("bool", Values.value(bool)));
+
+        // when
+        var valueHolder = Values.value(properties).as(PrivateRecord.class);
+
+        // then
+        assertEquals(string, valueHolder.string());
+        assertEquals(bool, valueHolder.bool());
+    }
+
+    @Test
+    void shouldAcceptPrivateRecordAsValue() {
+        // given
+        var string = "string";
+        var bool = false;
+        var recordValue = new PrivateRecord(string, bool);
+
+        // when
+        var value = Values.value(recordValue);
+
+        // then
+        assertEquals(string, value.get("string").asString());
+        assertEquals(bool, value.get("bool").asBoolean());
+    }
+
+    private record PrivateRecord(String string, boolean bool) {}
+
+    @Test
+    void shouldSelectAccessibleOnIdenticalMatch() {
+        // given
+        var string = "string";
+        var bool = false;
+        var number = 0;
+
+        var properties = Map.ofEntries(
+                Map.entry("string", Values.value(string)),
+                Map.entry("bool", Values.value(bool)),
+                Map.entry("number", Values.value(number)));
+
+        // when
+        var valueHolder = Values.value(properties).as(IdenticalMatch.class);
+
+        // then
+        assertEquals(string, valueHolder.string);
+        assertEquals(number, valueHolder.number);
+    }
+
+    public static class IdenticalMatch {
+        String string;
+        boolean bool;
+        long number;
+
+        private IdenticalMatch(@Property("string") String string, @Property("bool") boolean bool) {
+            this.string = string;
+            this.bool = bool;
+        }
+
+        public IdenticalMatch(@Property("string") String string, @Property("number") int number) {
+            this.string = string;
+            this.number = number;
+        }
+    }
+
+    @Test
+    void shouldFindNoMatch() {
+        // given
+        var properties = Map.ofEntries(Map.entry("value", Values.value("value")));
+
+        // when & then
+        var exception = assertThrows(
+                ValueException.class, () -> Values.value(properties).as(ValueHolder.class));
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -45,6 +45,6 @@ public T map(MapAccessor mapAccessor, Class<T> targetClass) {`
`45`	`45`	`.findConstructor(mapAccessor, targetClass)`
`46`	`46`	`.map(OBJECT_INSTANTIATOR::instantiate)`
`47`	`47`	`.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())));`
`49`	`49`	`}`
`50`	`50`	`}`