diff --git a/pom.xml b/pom.xml index 565738b5bb..25f6ef8b37 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-commons - 3.2.0-SNAPSHOT + 3.2.0-GH-2915-SNAPSHOT Spring Data Core Core Spring concepts underpinning every Spring Data module. diff --git a/src/main/java/org/springframework/data/mapping/PersistentProperty.java b/src/main/java/org/springframework/data/mapping/PersistentProperty.java index 3c950db3a4..fc1b2d3704 100644 --- a/src/main/java/org/springframework/data/mapping/PersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/PersistentProperty.java @@ -262,6 +262,16 @@ default Association

getRequiredAssociation() { */ boolean isWritable(); + /** + * Returns whether the current property is readable through {@link PersistentPropertyAccessor}, i.e. if it is not + * {@link #isTransient()}, if the value can be set on the current instance or read to create a new instance as per + * {@link #getWither()} or via Kotlin Copy methods. + * + * @return + * @since 3.2 + */ + boolean isReadable(); + /** * Returns whether the current property is immutable, i.e. if there is no setter or the backing {@link Field} is * {@code final}. diff --git a/src/main/java/org/springframework/data/mapping/model/AbstractPersistentProperty.java b/src/main/java/org/springframework/data/mapping/model/AbstractPersistentProperty.java index 46290436ea..6cfd42db58 100644 --- a/src/main/java/org/springframework/data/mapping/model/AbstractPersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/model/AbstractPersistentProperty.java @@ -27,6 +27,7 @@ import org.springframework.data.mapping.Association; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; +import org.springframework.data.util.KotlinReflectionUtils; import org.springframework.data.util.Lazy; import org.springframework.data.util.ReflectionUtils; import org.springframework.data.util.TypeInformation; @@ -70,6 +71,7 @@ public abstract class AbstractPersistentProperty

private final Method setter; private final Field field; private final Method wither; + private final Lazy readable; private final boolean immutable; public AbstractPersistentProperty(Property property, PersistentEntity owner, @@ -103,11 +105,27 @@ public AbstractPersistentProperty(Property property, PersistentEntity owne this.field = property.getField().orElse(null); this.wither = property.getWither().orElse(null); - if (setter == null && (field == null || Modifier.isFinal(field.getModifiers()))) { - this.immutable = true; - } else { - this.immutable = false; - } + this.immutable = setter == null && (field == null || Modifier.isFinal(field.getModifiers())); + this.readable = Lazy.of(() -> { + + if (setter != null) { + return true; + } + + if (wither != null) { + return true; + } + + if (KotlinReflectionUtils.isDataClass(owner.getType()) && KotlinCopyMethod.hasKotlinCopyMethodFor(this)) { + return true; + } + + if (field != null && !Modifier.isFinal(field.getModifiers())) { + return true; + } + + return false; + }); } protected abstract Association

createAssociation(); @@ -170,6 +188,7 @@ public Method getWither() { } @Nullable + @Override public Field getField() { return this.field; } @@ -190,6 +209,11 @@ public boolean isWritable() { return !isTransient(); } + @Override + public boolean isReadable() { + return !isTransient() && readable.get(); + } + @Override public boolean isImmutable() { return immutable; @@ -313,10 +337,8 @@ private Set> detectEntityTypes(SimpleTypeHolder simpleTypes) Set> result = detectEntityTypes(typeToStartWith); - return result.stream() - .filter(it -> !simpleTypes.isSimpleType(it.getType())) - .filter(it -> !it.getType().equals(ASSOCIATION_TYPE)) - .collect(Collectors.toSet()); + return result.stream().filter(it -> !simpleTypes.isSimpleType(it.getType())) + .filter(it -> !it.getType().equals(ASSOCIATION_TYPE)).collect(Collectors.toSet()); } private Set> detectEntityTypes(@Nullable TypeInformation source) { diff --git a/src/main/java/org/springframework/data/mapping/model/BytecodeUtil.java b/src/main/java/org/springframework/data/mapping/model/BytecodeUtil.java index a0575a2334..4da59c3ccd 100644 --- a/src/main/java/org/springframework/data/mapping/model/BytecodeUtil.java +++ b/src/main/java/org/springframework/data/mapping/model/BytecodeUtil.java @@ -165,7 +165,7 @@ static boolean isAccessible(Class type) { /** * Checks whether the class is accessible by inspecting modifiers (i.e. whether the class is {@code private}). * - * @param type must not be {@literal null}. + * @param modifiers modifiers to check. * @return {@literal true} if the {@code modifiers} do not indicate the private flag. * @see Modifier#isPrivate(int) */ @@ -177,7 +177,7 @@ static boolean isAccessible(int modifiers) { * Checks whether the modifiers express {@literal default} (not * {@literal private}/{@literal protected}/{@literal public}). * - * @param type must not be {@literal null}. + * @param modifiers modifiers to check. * @return {@literal true} if the {@code modifiers} indicate {@literal default}. * @see Modifier#isPrivate(int) */ diff --git a/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java b/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java index 58b790a383..302a93f2d9 100644 --- a/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java +++ b/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java @@ -1029,7 +1029,7 @@ private static void visitSetProperty0(PersistentEntity entity, PersistentP visitWithProperty(entity, property, mv, internalClassName, wither); } - if (hasKotlinCopyMethod(property)) { + if (KotlinDetector.isKotlinType(entity.getType()) && KotlinCopyMethod.hasKotlinCopyMethodFor(property)) { visitKotlinCopy(entity, property, mv, internalClassName); } @@ -1429,38 +1429,7 @@ public int compareTo(PropertyStackAddress o) { * @return {@literal true} if object mutation is supported. */ static boolean supportsMutation(PersistentProperty property) { - - if (property.isImmutable()) { - - if (property.getWither() != null) { - return true; - } - - if (hasKotlinCopyMethod(property)) { - return true; - } - } - - return (property.usePropertyAccess() && property.getSetter() != null) - || (property.getField() != null && !Modifier.isFinal(property.getField().getModifiers())); - } - - /** - * Check whether the owning type of {@link PersistentProperty} declares a {@literal copy} method or {@literal copy} - * method with parameter defaulting. - * - * @param property must not be {@literal null}. - * @return - */ - private static boolean hasKotlinCopyMethod(PersistentProperty property) { - - Class type = property.getOwner().getType(); - - if (isAccessible(type) && KotlinDetector.isKotlinType(type)) { - return KotlinCopyMethod.findCopyMethod(type).filter(it -> it.supportsProperty(property)).isPresent(); - } - - return false; + return (property.usePropertyAccess() && property.getSetter() != null) || property.isReadable(); } /** diff --git a/src/main/java/org/springframework/data/mapping/model/InstantiationAwarePropertyAccessor.java b/src/main/java/org/springframework/data/mapping/model/InstantiationAwarePropertyAccessor.java index dba291c608..07b35c2ad2 100644 --- a/src/main/java/org/springframework/data/mapping/model/InstantiationAwarePropertyAccessor.java +++ b/src/main/java/org/springframework/data/mapping/model/InstantiationAwarePropertyAccessor.java @@ -17,7 +17,6 @@ import java.util.function.Function; -import org.springframework.core.KotlinDetector; import org.springframework.data.mapping.InstanceCreatorMetadata; import org.springframework.data.mapping.Parameter; import org.springframework.data.mapping.PersistentEntity; @@ -76,7 +75,7 @@ public void setProperty(PersistentProperty property, @Nullable Object value) PersistentEntity> owner = property.getOwner(); PersistentPropertyAccessor delegate = delegateFunction.apply(this.bean); - if (!property.isImmutable() || property.getWither() != null || KotlinDetector.isKotlinType(owner.getType())) { + if (property.isReadable()) { delegate.setProperty(property, value); this.bean = delegate.getBean(); diff --git a/src/main/java/org/springframework/data/mapping/model/KotlinCopyMethod.java b/src/main/java/org/springframework/data/mapping/model/KotlinCopyMethod.java index 404fc2cd5d..246548fa09 100644 --- a/src/main/java/org/springframework/data/mapping/model/KotlinCopyMethod.java +++ b/src/main/java/org/springframework/data/mapping/model/KotlinCopyMethod.java @@ -97,6 +97,20 @@ public static Optional findCopyMethod(Class type) { }); } + /** + * Check whether the owning type of {@link PersistentProperty} declares a {@literal copy} method or {@literal copy} + * method with parameter defaulting. + * + * @param property must not be {@literal null}. + * @return + */ + public static boolean hasKotlinCopyMethodFor(PersistentProperty property) { + + Class type = property.getOwner().getType(); + + return KotlinCopyMethod.findCopyMethod(type).filter(it -> it.supportsProperty(property)).isPresent(); + } + public Method getPublicCopyMethod() { return this.publicCopyMethod; } diff --git a/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java b/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java index 4f2ee2cef3..c465f59340 100755 --- a/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java @@ -178,6 +178,31 @@ public void considersPropertyWithReadOnlyMetaAnnotationReadOnly() { .satisfies(it -> assertThat(it.isWritable()).isFalse()); } + @Test // GH-2915 + public void treatsReadOnlyAsReadable() { + + assertThat(getProperty(ClassWithReadOnlyProperties.class, "readOnlyProperty")) + .satisfies(it -> assertThat(it.isReadable()).isTrue()); + } + + @Test // GH-2915 + public void considersReadableForWither() { + + assertThat(getProperty(ClassWithWither.class, "nonReadable")) + .satisfies(it -> assertThat(it.isReadable()).isFalse()); + + assertThat(getProperty(ClassWithWither.class, "immutable")).satisfies(it -> assertThat(it.isReadable()).isTrue()); + } + + @Test // GH-2915 + public void considersReadableForKotlinDataClass() { + + assertThat(getProperty(SingleSettableProperty.class, "version")) + .satisfies(it -> assertThat(it.isReadable()).isFalse()); + + assertThat(getProperty(SingleSettableProperty.class, "id")).satisfies(it -> assertThat(it.isReadable()).isTrue()); + } + @Test // DATACMNS-556 public void doesNotRejectNonSpringDataAnnotationsUsedOnBothFieldAndAccessor() { getProperty(TypeWithCustomAnnotationsOnBothFieldAndAccessor.class, "field"); @@ -460,7 +485,8 @@ public String getProperty() { @Retention(RetentionPolicy.RUNTIME) @Target(value = { FIELD, METHOD, ANNOTATION_TYPE }) @Id - public @interface MyId {} + public @interface MyId { + } static class FieldAccess { String name; @@ -493,6 +519,21 @@ static class ClassWithReadOnlyProperties { @CustomReadOnly String customReadOnlyProperty; } + static class ClassWithWither { + + private final String nonReadable = ""; + + private final String immutable; + + public ClassWithWither(String immutable) { + this.immutable = immutable; + } + + public ClassWithWither withImmutable(String v) { + return new ClassWithWither(v); + } + } + static class TypeWithCustomAnnotationsOnBothFieldAndAccessor { @Nullable String field;