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

injectives · injectives · commit feb02c6ae87a · 2025-07-07T10:20:17.000+01:00
This update introduces an additional `TryAccessible` annotation that enables the Object Mapper to use `AccessibleObject#trySetAccessible()` when seaching for constructor to use. 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
@TryAccessible
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. As long as `AccessibleObject#trySetAccessible()` is successful, 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
@@ -31,6 +31,7 @@
 import org.neo4j.driver.exceptions.value.Uncoercible;
 import org.neo4j.driver.internal.value.NullValue;
 import org.neo4j.driver.mapping.Property;
+import org.neo4j.driver.mapping.TryAccessible;
 import org.neo4j.driver.types.Entity;
 import org.neo4j.driver.types.IsoDuration;
 import org.neo4j.driver.types.MapAccessor;
@@ -724,8 +725,13 @@ 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 (see {@link TryAccessible}) are included in the
+     * search.
+     * <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,46 @@
 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.mapping.TryAccessible;
 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 tryAccess = targetClass.isAnnotationPresent(TryAccessible.class);
         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 (tryAccess && 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 +81,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/main/java/org/neo4j/driver/mapping/TryAccessible.java b/driver/src/main/java/org/neo4j/driver/mapping/TryAccessible.java
@@ -0,0 +1,53 @@
+/*
+ * Copyright (c) "Neo4j"
+ * Neo4j Sweden AB [https://neo4j.com]
+ *
+ * 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
+ *
+ *     http://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.neo4j.driver.mapping;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+import java.lang.reflect.AccessibleObject;
+import org.neo4j.driver.util.Preview;
+
+/**
+ * Enables the Object Mapper to use {@link AccessibleObject#trySetAccessible()} when seaching for constructor to use.
+ * <p>
+ * When multiple constructors have the same number of matched and mismatched properties, the first consctructor that is
+ * accessible by default (does not require {@link AccessibleObject#trySetAccessible()}) is selected.
+ * <p>
+ * Example (using the <a href=https://github.com/neo4j-graph-examples/movies>Neo4j Movies Database</a>):
+ * <pre>
+ * {@code
+ * // assuming the following local Java record
+ * @TryAccessible
+ * record Movie(String title, String tagline, long released) {}
+ * // the nodes may be mapped to Movie instances
+ * var movies = driver.executableQuery("MATCH (movie:Movie) RETURN movie")
+ *         .execute()
+ *         .records()
+ *         .stream()
+ *         .map(record -> record.get("movie").as(Movie.class))
+ *         .toList();
+ * }
+ * </pre>
+ *
+ * @since 5.28.8
+ */
+@Target(ElementType.TYPE)
+@Retention(RetentionPolicy.RUNTIME)
+@Preview(name = "Object mapping")
+public @interface TryAccessible {}
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;
@@ -46,6 +48,7 @@
 import org.neo4j.driver.internal.value.NodeValue;
 import org.neo4j.driver.internal.value.RelationshipValue;
 import org.neo4j.driver.mapping.Property;
+import org.neo4j.driver.mapping.TryAccessible;
 import org.neo4j.driver.types.IsoDuration;
 import org.neo4j.driver.types.Point;
 
@@ -189,4 +192,90 @@ 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)));
+
+        @TryAccessible
+        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 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());
+    }
+
+    @TryAccessible
+    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`	`}`