Skip to content

Introduce PersistentProperty.isReadable #2916

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<groupId>org.springframework.data</groupId>
<artifactId>spring-data-commons</artifactId>
<version>3.2.0-SNAPSHOT</version>
<version>3.2.0-GH-2915-SNAPSHOT</version>

<name>Spring Data Core</name>
<description>Core Spring concepts underpinning every Spring Data module.</description>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,16 @@ default Association<P> 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}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -70,6 +71,7 @@ public abstract class AbstractPersistentProperty<P extends PersistentProperty<P>
private final Method setter;
private final Field field;
private final Method wither;
private final Lazy<Boolean> readable;
private final boolean immutable;

public AbstractPersistentProperty(Property property, PersistentEntity<?, P> owner,
Expand Down Expand Up @@ -103,11 +105,27 @@ public AbstractPersistentProperty(Property property, PersistentEntity<?, P> 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<P> createAssociation();
Expand Down Expand Up @@ -170,6 +188,7 @@ public Method getWither() {
}

@Nullable
@Override
public Field getField() {
return this.field;
}
Expand All @@ -190,6 +209,11 @@ public boolean isWritable() {
return !isTransient();
}

@Override
public boolean isReadable() {
return !isTransient() && readable.get();
}

@Override
public boolean isImmutable() {
return immutable;
Expand Down Expand Up @@ -313,10 +337,8 @@ private Set<TypeInformation<?>> detectEntityTypes(SimpleTypeHolder simpleTypes)

Set<TypeInformation<?>> 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<TypeInformation<?>> detectEntityTypes(@Nullable TypeInformation<?> source) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
*/
Expand All @@ -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)
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -76,7 +75,7 @@ public void setProperty(PersistentProperty<?> property, @Nullable Object value)
PersistentEntity<?, ? extends PersistentProperty<?>> owner = property.getOwner();
PersistentPropertyAccessor<T> 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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,20 @@ public static Optional<KotlinCopyMethod> 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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down