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)); + } }