DATACMNS-1322 - Detect wither methods.

We now detect withher methods (withId(…)) that create a new object instance that contains the new property value. We support those wither methods to create new object instances from property updates. Classes following a wither pattern declare a withXXX(…) method that accepts the property value and return a new, instance of its own type associated with the property value. Along with this change we removed the ability to update final fields that worked by accident using reflection.

class ValueClass {
	@Wither String id;
}

class ValueClass {
	final String id;

	ValueClass withId(String id) {
		return new ValueClass(id);
	}
}

src/test/java/org/springframework/data/mapping/PersistentPropertyAccessorUnitTests.java

@@ -17,6 +17,8 @@ package org.springframework.data.mapping;
 
 import static org.assertj.core.api.Assertions.*;
 
+import lombok.AllArgsConstructor;
+import lombok.Data;
 import lombok.Value;
 
 import org.junit.Test;
@@ -93,7 +95,8 @@ public class PersistentPropertyAccessorUnitTests {
 		Customer customer;
 	}
 
-	@Value
+	@Data
+	@AllArgsConstructor
 	static class Customer {
 		String firstname;
 	}

src/test/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactoryTests.java

@@ -46,8 +46,8 @@ import org.springframework.util.StringUtils;
 @RunWith(Parameterized.class)
 public class ClassGeneratingPropertyAccessorFactoryTests {
 
-	private final ClassGeneratingPropertyAccessorFactory factory = new ClassGeneratingPropertyAccessorFactory();
-	private final SampleMappingContext mappingContext = new SampleMappingContext();
+	private final static ClassGeneratingPropertyAccessorFactory factory = new ClassGeneratingPropertyAccessorFactory();
+	private final static SampleMappingContext mappingContext = new SampleMappingContext();
 
 	private final Object bean;
 	private final String propertyName;
@@ -67,7 +67,8 @@ public class ClassGeneratingPropertyAccessorFactoryTests {
 
 		List<Object[]> parameters = new ArrayList<>();
 		List<String> propertyNames = Arrays.asList("privateField", "packageDefaultField", "protectedField", "publicField",
-				"privateProperty", "packageDefaultProperty", "protectedProperty", "publicProperty", "syntheticProperty");
+				"privateProperty", "packageDefaultProperty", "protectedProperty", "publicProperty", "syntheticProperty",
+				"immutable", "wither");
 
 		parameters.addAll(parameters(new InnerPrivateType(), propertyNames, Object.class));
 		parameters
@@ -106,7 +107,7 @@ public class ClassGeneratingPropertyAccessorFactoryTests {
 		assertThat(factory.isSupported(mappingContext.getRequiredPersistentEntity(bean.getClass()))).isTrue();
 	}
 
-	@Test // DATACMNS-809
+	@Test // DATACMNS-809, // DATACMNS-1322
 	public void shouldSetAndGetProperty() throws Exception {
 
 		assumeTrue(StringUtils.hasText(propertyName));
@@ -114,9 +115,15 @@ public class ClassGeneratingPropertyAccessorFactoryTests {
 		assertThat(getProperty(bean, propertyName)).satisfies(property -> {
 
 			PersistentPropertyAccessor persistentPropertyAccessor = getPersistentPropertyAccessor(bean);
+			if (property.isImmutable() && property.getWither() == null) {
 
-			persistentPropertyAccessor.setProperty(property, "value");
-			assertThat(persistentPropertyAccessor.getProperty(property)).isEqualTo("value");
+				assertThatThrownBy(() -> persistentPropertyAccessor.setProperty(property, "value"))
+						.isInstanceOf(UnsupportedOperationException.class);
+			} else {
+
+				persistentPropertyAccessor.setProperty(property, "value");
+				assertThat(persistentPropertyAccessor.getProperty(property)).isEqualTo("value");
+			}
 		});
 	}
 
@@ -183,6 +190,8 @@ public class ClassGeneratingPropertyAccessorFactoryTests {
 		protected String protectedField;
 		public String publicField;
 		private String backing;
+		private final String immutable = "";
+		private final String wither;
 
 		@AccessType(Type.PROPERTY) private String privateProperty;
 
@@ -192,6 +201,14 @@ public class ClassGeneratingPropertyAccessorFactoryTests {
 
 		@AccessType(Type.PROPERTY) private String publicProperty;
 
+		private InnerPrivateType() {
+			this.wither = "";
+		}
+
+		private InnerPrivateType(String wither) {
+			this.wither = wither;
+		}
+
 		private String getPrivateProperty() {
 			return privateProperty;
 		}
@@ -232,6 +249,14 @@ public class ClassGeneratingPropertyAccessorFactoryTests {
 		public void setSyntheticProperty(String syntheticProperty) {
 			backing = syntheticProperty;
 		}
+
+		public String getWither() {
+			return wither;
+		}
+
+		public InnerPrivateType withWither(String wither) {
+			return new InnerPrivateType(wither);
+		}
 	}
 
 	// DATACMNS-809
@@ -248,6 +273,8 @@ public class ClassGeneratingPropertyAccessorFactoryTests {
 		protected String protectedField;
 		public String publicField;
 		private String backing;
+		private final String immutable = "";
+		private final String wither;
 
 		@AccessType(Type.PROPERTY) private String privateProperty;
 
@@ -257,6 +284,14 @@ public class ClassGeneratingPropertyAccessorFactoryTests {
 
 		@AccessType(Type.PROPERTY) private String publicProperty;
 
+		InnerPackageDefaultType() {
+			this.wither = "";
+		}
+
+		private InnerPackageDefaultType(String wither) {
+			this.wither = wither;
+		}
+
 		private String getPrivateProperty() {
 			return privateProperty;
 		}
@@ -297,6 +332,14 @@ public class ClassGeneratingPropertyAccessorFactoryTests {
 		public void setSyntheticProperty(String syntheticProperty) {
 			backing = syntheticProperty;
 		}
+
+		public String getWither() {
+			return wither;
+		}
+
+		public InnerPrivateType withWither(String wither) {
+			return new InnerPrivateType(wither);
+		}
 	}
 
 	// DATACMNS-809
@@ -308,6 +351,8 @@ public class ClassGeneratingPropertyAccessorFactoryTests {
 		protected String protectedField;
 		public String publicField;
 		private String backing;
+		private final String immutable = "";
+		private final String wither;
 
 		@AccessType(Type.PROPERTY) private String privateProperty;
 
@@ -317,6 +362,14 @@ public class ClassGeneratingPropertyAccessorFactoryTests {
 
 		@AccessType(Type.PROPERTY) private String publicProperty;
 
+		InnerProtectedType() {
+			this.wither = "";
+		}
+
+		private InnerProtectedType(String wither) {
+			this.wither = wither;
+		}
+
 		private String getPrivateProperty() {
 			return privateProperty;
 		}
@@ -357,6 +410,14 @@ public class ClassGeneratingPropertyAccessorFactoryTests {
 		public void setSyntheticProperty(String syntheticProperty) {
 			backing = syntheticProperty;
 		}
+
+		public String getWither() {
+			return wither;
+		}
+
+		public InnerPrivateType withWither(String wither) {
+			return new InnerPrivateType(wither);
+		}
 	}
 
 	// DATACMNS-809
@@ -368,6 +429,8 @@ public class ClassGeneratingPropertyAccessorFactoryTests {
 		protected String protectedField;
 		public String publicField;
 		private String backing;
+		private final String immutable = "";
+		private final String wither;
 
 		@AccessType(Type.PROPERTY) private String privateProperty;
 
@@ -377,6 +440,14 @@ public class ClassGeneratingPropertyAccessorFactoryTests {
 
 		@AccessType(Type.PROPERTY) private String publicProperty;
 
+		InnerPublicType() {
+			this.wither = "";
+		}
+
+		private InnerPublicType(String wither) {
+			this.wither = wither;
+		}
+
 		private String getPrivateProperty() {
 			return privateProperty;
 		}
@@ -417,6 +488,14 @@ public class ClassGeneratingPropertyAccessorFactoryTests {
 		public void setSyntheticProperty(String syntheticProperty) {
 			backing = syntheticProperty;
 		}
+
+		public String getWither() {
+			return wither;
+		}
+
+		public InnerPrivateType withWither(String wither) {
+			return new InnerPrivateType(wither);
+		}
 	}
 
 	public static class SubtypeOfTypeInOtherPackage extends TypeInOtherPackage {}

src/test/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorPackageDefaultType.java

@@ -30,6 +30,8 @@ class ClassGeneratingPropertyAccessorPackageDefaultType {
 	protected String protectedField;
 	public String publicField;
 	private String backing;
+	private final String immutable = "";
+	private final String wither;
 
 	@AccessType(Type.PROPERTY) private String privateProperty;
 
@@ -39,6 +41,14 @@ class ClassGeneratingPropertyAccessorPackageDefaultType {
 
 	@AccessType(Type.PROPERTY) private String publicProperty;
 
+	public ClassGeneratingPropertyAccessorPackageDefaultType() {
+		this.wither = "";
+	}
+
+	private ClassGeneratingPropertyAccessorPackageDefaultType(String wither) {
+		this.wither = wither;
+	}
+
 	private String getPrivateProperty() {
 		return privateProperty;
 	}
@@ -79,4 +89,12 @@ class ClassGeneratingPropertyAccessorPackageDefaultType {
 	public void setSyntheticProperty(String syntheticProperty) {
 		backing = syntheticProperty;
 	}
+
+	public String getWither() {
+		return wither;
+	}
+
+	public ClassGeneratingPropertyAccessorPackageDefaultType withWither(String wither) {
+		return new ClassGeneratingPropertyAccessorPackageDefaultType(wither);
+	}
 }

src/test/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorPublicType.java

@@ -30,6 +30,8 @@ public class ClassGeneratingPropertyAccessorPublicType {
 	protected String protectedField;
 	public String publicField;
 	private String backing;
+	private final String immutable = "";
+	private final String wither;
 	private Integer aa;
 	private int bb;
 
@@ -41,6 +43,14 @@ public class ClassGeneratingPropertyAccessorPublicType {
 
 	@AccessType(Type.PROPERTY) private String publicProperty;
 
+	public ClassGeneratingPropertyAccessorPublicType() {
+		this.wither = "";
+	}
+
+	private ClassGeneratingPropertyAccessorPublicType(String wither) {
+		this.wither = wither;
+	}
+
 	private String getPrivateProperty() {
 		return privateProperty;
 	}
@@ -82,6 +92,14 @@ public class ClassGeneratingPropertyAccessorPublicType {
 		backing = syntheticProperty;
 	}
 
+	public String getWither() {
+		return wither;
+	}
+
+	public ClassGeneratingPropertyAccessorPublicType withWither(String wither) {
+		return new ClassGeneratingPropertyAccessorPublicType(wither);
+	}
+
 	public Object set(Object e) {
 
 		aa = (Integer) e;

src/test/java/org/springframework/data/mapping/model/PersistentPropertyAccessorTests.java

@@ -0,0 +1,128 @@
+/*
+ * Copyright 2018 the original author or authors.
+ *
+ * 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.springframework.data.mapping.model;
+
+import static org.assertj.core.api.Assertions.*;
+
+import lombok.Data;
+import lombok.Value;
+import lombok.experimental.Wither;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.function.Function;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+import org.springframework.data.mapping.PersistentPropertyAccessor;
+import org.springframework.data.mapping.context.SampleMappingContext;
+import org.springframework.data.mapping.context.SamplePersistentProperty;
+
+/**
+ * Unit tests for {@link PersistentPropertyAccessor} through {@link BeanWrapper} and
+ * {@link ClassGeneratingPropertyAccessorFactory}.
+ *
+ * @author Mark Paluch
+ */
+@RunWith(Parameterized.class)
+public class PersistentPropertyAccessorTests {
+
+	private final static SampleMappingContext mappingContext = new SampleMappingContext();
+
+	private final Function<Object, PersistentPropertyAccessor> propertyAccessorFunction;
+
+	public PersistentPropertyAccessorTests(Function<Object, PersistentPropertyAccessor> propertyAccessor,
+			String displayName) {
+
+		this.propertyAccessorFunction = propertyAccessor;
+	}
+
+	@Parameters(name = "{1}")
+	@SuppressWarnings("unchecked")
+	public static List<Object[]> parameters() {
+
+		List<Object[]> parameters = new ArrayList<>();
+
+		ClassGeneratingPropertyAccessorFactory factory = new ClassGeneratingPropertyAccessorFactory();
+
+		Function<Object, PersistentPropertyAccessor> beanWrapper = BeanWrapper::new;
+		Function<Object, PersistentPropertyAccessor> classGenerating = it -> factory
+				.getPropertyAccessor(mappingContext.getRequiredPersistentEntity(it.getClass()), it);
+
+		parameters.add(new Object[] { beanWrapper, "BeanWrapper" });
+		parameters.add(new Object[] { classGenerating, "ClassGeneratingPropertyAccessorFactory" });
+
+		return parameters;
+	}
+
+	@Test // DATACMNS-1322
+	public void shouldSetProperty() {
+
+		DataClass bean = new DataClass();
+		PersistentPropertyAccessor accessor = propertyAccessorFunction.apply(bean);
+		SamplePersistentProperty property = getProperty(bean, "id");
+
+		accessor.setProperty(property, "value");
+
+		assertThat(accessor.getBean()).hasFieldOrPropertyWithValue("id", "value");
+		assertThat(accessor.getBean()).isSameAs(bean);
+		assertThat(accessor.getProperty(property)).isEqualTo("value");
+	}
+
+	@Test // DATACMNS-1322
+	public void shouldWitherProperty() {
+
+		ValueClass bean = new ValueClass("foo", "bar");
+		PersistentPropertyAccessor accessor = propertyAccessorFunction.apply(bean);
+		SamplePersistentProperty property = getProperty(bean, "id");
+
+		accessor.setProperty(property, "value");
+
+		assertThat(accessor.getBean()).hasFieldOrPropertyWithValue("id", "value");
+		assertThat(accessor.getBean()).isNotSameAs(bean);
+		assertThat(accessor.getProperty(property)).isEqualTo("value");
+	}
+
+	@Test // DATACMNS-1322
+	public void shouldRejectImmutablePropertyUpdate() {
+
+		ValueClass bean = new ValueClass("foo", "bar");
+		PersistentPropertyAccessor accessor = propertyAccessorFunction.apply(bean);
+		SamplePersistentProperty property = getProperty(bean, "immutable");
+
+		assertThatThrownBy(() -> accessor.setProperty(property, "value")).isInstanceOf(UnsupportedOperationException.class);
+	}
+
+	private static SamplePersistentProperty getProperty(Object bean, String propertyName) {
+
+		return mappingContext.getRequiredPersistentEntity(bean.getClass()).getRequiredPersistentProperty(propertyName);
+	}
+
+	@Data
+	static class DataClass {
+		String id;
+	}
+
+	@Value
+
+	static class ValueClass {
+		@Wither String id;
+		String immutable;
+	}
+
+}

src/test/java/org/springframework/data/mapping/model/PropertyUnitTests.java

@@ -0,0 +1,81 @@
+/*
+ * Copyright 2018 the original author or authors.
+ *
+ * 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.springframework.data.mapping.model;
+
+import static org.assertj.core.api.Assertions.*;
+
+import lombok.Value;
+import lombok.experimental.Wither;
+
+import org.junit.Test;
+import org.springframework.data.util.ClassTypeInformation;
+import org.springframework.util.ReflectionUtils;
+
+/**
+ * Unit tests for {@link Property}.
+ *
+ * @author Mark Paluch
+ */
+public class PropertyUnitTests {
+
+	@Test // DATACMNS-1322
+	public void shouldNotFindWitherMethod() {
+
+		assertThat(Property
+				.of(ClassTypeInformation.from(ImmutableType.class), ReflectionUtils.findField(ImmutableType.class, "id"))
+				.getWither()).isEmpty();
+		assertThat(Property
+				.of(ClassTypeInformation.from(ImmutableType.class), ReflectionUtils.findField(ImmutableType.class, "name"))
+				.getWither()).isEmpty();
+	}
+
+	@Test // DATACMNS-1322
+	public void shouldDiscoverWitherMethod() {
+
+		Property property = Property.of(ClassTypeInformation.from(WitherType.class),
+				ReflectionUtils.findField(WitherType.class, "id"));
+
+		assertThat(property.getWither()).isPresent().hasValueSatisfying(actual -> {
+			assertThat(actual.getName()).isEqualTo("withId");
+			assertThat(actual.getReturnType()).isEqualTo(WitherType.class);
+		});
+	}
+
+	@Value
+	static class ImmutableType {
+		String id;
+		String name;
+
+		ImmutableType withId(Long id) {
+			return null;
+		}
+
+		ImmutableType withName(Object id) {
+			return null;
+		}
+
+		Object withName(String id) {
+			return null;
+		}
+	}
+
+	@Value
+	@Wither
+	static class WitherType {
+		String id;
+		String name;
+	}
+}

src/test/java/org/springframework/data/mapping/model/subpackage/TypeInOtherPackage.java

@@ -29,6 +29,8 @@ public class TypeInOtherPackage {
 	protected String protectedField;
 	public String publicField;
 	private String backing;
+	private final String immutable = "";
+	private final String wither;
 
 	@AccessType(Type.PROPERTY) private String privateProperty;
 
@@ -38,6 +40,14 @@ public class TypeInOtherPackage {
 
 	@AccessType(Type.PROPERTY) private String publicProperty;
 
+	public TypeInOtherPackage() {
+		this.wither = "";
+	}
+
+	private TypeInOtherPackage(String wither) {
+		this.wither = wither;
+	}
+
 	@SuppressWarnings("unused")
 	private String getPrivateProperty() {
 		return privateProperty;
@@ -80,4 +90,12 @@ public class TypeInOtherPackage {
 	public void setSyntheticProperty(String syntheticProperty) {
 		backing = syntheticProperty;
 	}
+
+	public String getWither() {
+		return wither;
+	}
+
+	public TypeInOtherPackage withWither(String wither) {
+		return new TypeInOtherPackage(wither);
+	}
 }