From 94d343012b7f1b74d35e1033d00c12ea6b193066 Mon Sep 17 00:00:00 2001 From: Dmitriy Tverdiakov <11927660+injectives@users.noreply.github.com> Date: Mon, 7 Jul 2025 15:42:25 +0100 Subject: [PATCH] feat(object-mapping): support mapping types with restricted access (#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 --- .../src/main/java/org/neo4j/driver/Value.java | 11 +- .../value/mapping/ConstructorFinder.java | 65 +++++++--- .../value/mapping/ObjectInstantiator.java | 2 +- .../internal/value/mapping/ObjectMapper.java | 2 +- .../org/neo4j/driver/ObjectMappingTests.java | 117 ++++++++++++++++++ 5 files changed, 176 insertions(+), 21 deletions(-) diff --git a/driver/src/main/java/org/neo4j/driver/Value.java b/driver/src/main/java/org/neo4j/driver/Value.java index 744d41566d..178d1420db 100644 --- 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; @@ -656,8 +657,14 @@ public interface Value extends MapAccessor, MapAccessorWithDefaultValue { *
  • Maximum matching properties.
    • *
  • Minimum mismatching properties.
    • * - * 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}. + *

    + * 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. + *

    + * 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. *

    * At least 1 property match must be present for mapping to work. *

    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 index 53b6e60204..bc76b28625 100644 --- 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 ENTITY_TYPES = Set.of(TS.NODE(), TS.RELATIONSHIP()); + @SuppressWarnings("unchecked") public Optional> findConstructor(MapAccessor mapAccessor, Class targetClass) { PropertiesMatch 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) matchNumbers; + if (bestPropertiesMatch.match() == propertyNamesSize && bestPropertiesMatch.mismatch() == 0) { + break; + } + } else if (constructor.trySetAccessible()) { + bestPropertiesMatch = (PropertiesMatch) 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) matchNumbers; if (bestPropertiesMatch.match() == propertyNamesSize && bestPropertiesMatch.mismatch() == 0) { break; @@ -66,19 +79,37 @@ private PropertiesMatch 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(int match, int mismatch, Constructor constructor, List arguments) {} + private record PropertiesMatch( + int match, int mismatch, Constructor constructor, List 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 index 2cfd7f18ba..ae22af80a4 100644 --- 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 instantiate(ObjectMetadata 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 index c18d413b62..bee69336db 100644 --- 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 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 index ea106fa1d4..faaa251a27 100644 --- 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)); + } }