DATACOUCH-625 - Save doesn't return updated entity if immutable. (#274)

Modified applyUpdatedCas() to return accessor.getBean() which will
occur if the version property is Immutable.
Created applyUpdateId() that is analogous to applyUpdatedCas()
Factored out the equals() method that was used by a number of
test entity objects.

Co-authored-by: mikereiche <michael.reiche@couchbase.com>

src/main/java/org/springframework/data/couchbase/core/CouchbaseTemplateSupport.java

@@ -89,14 +89,28 @@ class CouchbaseTemplateSupport implements ApplicationContextAware {
 		return accessor.getBean();
 	}
 
-	public void applyUpdatedCas(final Object entity, final long cas) {
+	public Object applyUpdatedCas(final Object entity, final long cas) {
 		final ConvertingPropertyAccessor<Object> accessor = getPropertyAccessor(entity);
 		final CouchbasePersistentEntity<?> persistentEntity = mappingContext.getRequiredPersistentEntity(entity.getClass());
 		final CouchbasePersistentProperty versionProperty = persistentEntity.getVersionProperty();
 
 		if (versionProperty != null) {
 			accessor.setProperty(versionProperty, cas);
+			return accessor.getBean();
 		}
+		return entity;
+	}
+
+	public Object applyUpdatedId(final Object entity, Object id) {
+		final ConvertingPropertyAccessor<Object> accessor = getPropertyAccessor(entity);
+		final CouchbasePersistentEntity<?> persistentEntity = mappingContext.getRequiredPersistentEntity(entity.getClass());
+		final CouchbasePersistentProperty idProperty = persistentEntity.getIdProperty();
+
+		if (idProperty != null) {
+			accessor.setProperty(idProperty, id);
+			return accessor.getBean();
+		}
+		return entity;
 	}
 
 	public long getCas(final Object entity) {
@@ -106,9 +120,9 @@ class CouchbaseTemplateSupport implements ApplicationContextAware {
 
 		long cas = 0;
 		if (versionProperty != null) {
-			Object casObject  =  (Number)accessor.getProperty(versionProperty);
-			if (casObject instanceof Number){
-				cas = ((Number)casObject).longValue();
+			Object casObject = (Number) accessor.getProperty(versionProperty);
+			if (casObject instanceof Number) {
+				cas = ((Number) casObject).longValue();
 			}
 		}
 		return cas;

src/main/java/org/springframework/data/couchbase/core/ReactiveInsertByIdOperationSupport.java

@@ -73,8 +73,8 @@ public class ReactiveInsertByIdOperationSupport implements ReactiveInsertByIdOpe
 				CouchbaseDocument converted = template.support().encodeEntity(o);
 				return template.getCollection(collection).reactive()
 						.insert(converted.getId(), converted.export(), buildInsertOptions()).map(result -> {
-							template.support().applyUpdatedCas(object, result.cas());
-							return o;
+							Object updatedObject = template.support().applyUpdatedId(o, converted.getId());
+							return (T) template.support().applyUpdatedCas(updatedObject, result.cas());
 						});
 			}).onErrorMap(throwable -> {
 				if (throwable instanceof RuntimeException) {
@@ -97,7 +97,7 @@ public class ReactiveInsertByIdOperationSupport implements ReactiveInsertByIdOpe
 			} else if (durabilityLevel != DurabilityLevel.NONE) {
 				options.durability(durabilityLevel);
 			}
-			if (expiry != null && ! expiry.isZero()) {
+			if (expiry != null && !expiry.isZero()) {
 				options.expiry(expiry);
 			} else if (domainType.isAnnotationPresent(Document.class)) {
 				Document documentAnn = domainType.getAnnotation(Document.class);
@@ -110,26 +110,30 @@ public class ReactiveInsertByIdOperationSupport implements ReactiveInsertByIdOpe
 		@Override
 		public TerminatingInsertById<T> inCollection(final String collection) {
 			Assert.hasText(collection, "Collection must not be null nor empty.");
-			return new ReactiveInsertByIdSupport<>(template, domainType, collection, persistTo, replicateTo, durabilityLevel, expiry);
+			return new ReactiveInsertByIdSupport<>(template, domainType, collection, persistTo, replicateTo, durabilityLevel,
+					expiry);
 		}
 
 		@Override
 		public InsertByIdWithCollection<T> withDurability(final DurabilityLevel durabilityLevel) {
 			Assert.notNull(durabilityLevel, "Durability Level must not be null.");
-			return new ReactiveInsertByIdSupport<>(template, domainType, collection, persistTo, replicateTo, durabilityLevel, expiry);
+			return new ReactiveInsertByIdSupport<>(template, domainType, collection, persistTo, replicateTo, durabilityLevel,
+					expiry);
 		}
 
 		@Override
 		public InsertByIdWithCollection<T> withDurability(final PersistTo persistTo, final ReplicateTo replicateTo) {
 			Assert.notNull(persistTo, "PersistTo must not be null.");
 			Assert.notNull(replicateTo, "ReplicateTo must not be null.");
-			return new ReactiveInsertByIdSupport<>(template, domainType, collection, persistTo, replicateTo, durabilityLevel, expiry);
+			return new ReactiveInsertByIdSupport<>(template, domainType, collection, persistTo, replicateTo, durabilityLevel,
+					expiry);
 		}
 
 		@Override
 		public InsertByIdWithDurability<T> withExpiry(final Duration expiry) {
 			Assert.notNull(expiry, "expiry must not be null.");
-			return new ReactiveInsertByIdSupport<>(template, domainType, collection, persistTo, replicateTo, durabilityLevel, expiry);
+			return new ReactiveInsertByIdSupport<>(template, domainType, collection, persistTo, replicateTo, durabilityLevel,
+					expiry);
 		}
 	}
 

src/main/java/org/springframework/data/couchbase/core/ReactiveUpsertByIdOperationSupport.java

@@ -15,6 +15,8 @@
  */
 package org.springframework.data.couchbase.core;
 
+import org.springframework.data.couchbase.core.mapping.CouchbasePersistentEntity;
+import org.springframework.data.couchbase.core.mapping.CouchbasePersistentProperty;
 import org.springframework.data.couchbase.core.mapping.Document;
 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
@@ -73,8 +75,8 @@ public class ReactiveUpsertByIdOperationSupport implements ReactiveUpsertByIdOpe
 				CouchbaseDocument converted = template.support().encodeEntity(o);
 				return template.getCollection(collection).reactive()
 						.upsert(converted.getId(), converted.export(), buildUpsertOptions()).map(result -> {
-							template.support().applyUpdatedCas(object, result.cas());
-							return o;
+							Object updatedObject = template.support().applyUpdatedId(o, converted.getId());
+							return (T) template.support().applyUpdatedCas(updatedObject, result.cas());
 						});
 			}).onErrorMap(throwable -> {
 				if (throwable instanceof RuntimeException) {

src/main/java/org/springframework/data/couchbase/core/convert/MappingCouchbaseConverter.java

@@ -541,6 +541,7 @@ public class MappingCouchbaseConverter extends AbstractCouchbaseConverter implem
 				generatedValueInfo = idProperty.findAnnotation(GeneratedValue.class);
 				String generatedId = generateId(generatedValueInfo, prefixes, suffixes, idAttributes);
 				target.setId(generatedId);
+				// this is not effective if id is Immutable, and accessor.setProperty() returns a new object in getBean()
 				accessor.setProperty(idProperty, generatedId);
 			} else {
 				target.setId(id);

src/test/java/org/springframework/data/couchbase/domain/ComparableEntity.java

@@ -0,0 +1,94 @@
+/*
+ * Copyright 2020 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
+ *
+ *        https://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.couchbase.domain;
+
+import java.lang.reflect.Field;
+
+/**
+ * Comparable entity base class for tests
+ *
+ * @author Michael Reiche
+ */
+public class ComparableEntity {
+
+	/**
+	 * equals() method that recursively calls equals on on fields
+	 * 
+	 * @param that
+	 * @return
+	 * @throws RuntimeException
+	 */
+	@Override
+	public boolean equals(Object that) throws RuntimeException {
+		if (this == that) {
+			return true;
+		}
+		if (that == null
+				|| !(this.getClass().isAssignableFrom(that.getClass()) || that.getClass().isAssignableFrom(this.getClass()))) {
+			return false;
+		}
+		// check that all the fields in this have an equal field in that
+		for (Field f : this.getClass().getFields()) {
+			if (!same(f, this, that)) {
+				return false;
+			}
+		}
+		// check that all the fields in that have an equal field in this
+		for (Field f : that.getClass().getFields()) {
+			if (!same(f, that, this)) {
+				return false;
+			}
+		}
+		// check that all the declared fields in this have an equal field in that
+		for (Field f : this.getClass().getDeclaredFields()) {
+			if (!same(f, this, that)) {
+				return false;
+			}
+		}
+		// check that all the declared fields in that have an equal field in this
+		for (Field f : that.getClass().getDeclaredFields()) {
+			if (!same(f, that, this)) {
+				return false;
+			}
+		}
+		return true;
+	}
+
+	private static boolean same(Field f, Object a, Object b) {
+		Object thisField = null;
+		Object thatField = null;
+
+		try {
+			thisField = f.get(a);
+			thatField = f.get(b);
+		} catch (IllegalAccessException e) {
+			// assume that the important fields are in toString()
+			thisField = a.toString();
+			thatField = b.toString();
+		}
+		if (thisField == null && thatField == null) {
+			return true;
+		}
+		if (thisField == null && thatField != null) {
+			return false;
+		}
+		if (!thisField.equals(thatField)) {
+			return false;
+		}
+		return true;
+	}
+}

src/test/java/org/springframework/data/couchbase/domain/Course.java

@@ -25,7 +25,7 @@ import java.lang.reflect.Field;
  *
  * @author Michael Reiche
  */
-public class Course {
+public class Course extends ComparableEntity {
 	@Id private final String id;
 	private final String userId;
 	private final String room;
@@ -52,42 +52,4 @@ public class Course {
 		return sb.toString();
 	}
 
-	@Override
-	public boolean equals(Object that) throws RuntimeException {
-		if (this == that)
-			return true;
-		if (that == null || getClass() != that.getClass())
-			return false;
-		for (Field f : getClass().getFields()) {
-			if (!same(f, this, that))
-				return false;
-		}
-		for (Field f : getClass().getDeclaredFields()) {
-			if (!same(f, this, that))
-				return false;
-		}
-		return true;
-	}
-
-	private static boolean same(Field f, Object a, Object b) {
-		Object thisField = null;
-		Object thatField = null;
-
-		try {
-			f.get(a);
-			f.get(b);
-		} catch (IllegalAccessException e) {
-			throw new RuntimeException(e);
-		}
-		if (thisField == null && thatField == null) {
-			return true;
-		}
-		if (thisField == null && thatField != null) {
-			return false;
-		}
-		if (!thisField.equals(thatField)) {
-			return false;
-		}
-		return true;
-	}
 }

src/test/java/org/springframework/data/couchbase/domain/PersonValue.java

@@ -0,0 +1,61 @@
+/*
+ * Copyright 2020 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
+ *
+ *        https://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.couchbase.domain;
+
+import lombok.Value;
+import lombok.With;
+import org.springframework.data.annotation.Id;
+import org.springframework.data.annotation.Version;
+import org.springframework.data.couchbase.core.mapping.Document;
+import org.springframework.data.couchbase.core.mapping.Field;
+import org.springframework.data.couchbase.core.mapping.id.GeneratedValue;
+import org.springframework.data.couchbase.core.mapping.id.GenerationStrategy;
+
+/**
+ * PersonValue entity for tests
+ *
+ * @author Michael Reiche
+ */
+
+@Value
+@Document
+public class PersonValue {
+	@Id @GeneratedValue(strategy = GenerationStrategy.UNIQUE)
+	@With String id;
+//	@Version @With
+	long version;
+	@Field String firstname;
+	@Field String lastname;
+
+	public PersonValue(String id, long version, String firstname, String lastname) {
+		this.id = id;
+		this.version = version;
+		this.firstname = firstname;
+		this.lastname = lastname;
+	}
+
+	public String toString() {
+		StringBuilder sb = new StringBuilder();
+		sb.append("PersonValue : {");
+		sb.append(" id : " + getId());
+		sb.append(", version : " + version);
+		sb.append(", firstname : " + firstname);
+		sb.append(", lastname : " + lastname);
+		sb.append(" }");
+		return sb.toString();
+	}
+
+}

src/test/java/org/springframework/data/couchbase/domain/PersonValueRepository.java

@@ -0,0 +1,27 @@
+/*
+ * Copyright 2020 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
+ *
+ *        https://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.couchbase.domain;
+
+import org.springframework.data.repository.CrudRepository;
+
+/**
+ * PersonValue repository for tests
+ *
+ * @author Michael Reiche
+ */
+public interface PersonValueRepository extends CrudRepository<PersonValue, String> {
+
+}

src/test/java/org/springframework/data/couchbase/domain/Submission.java

@@ -16,14 +16,12 @@
 
 package org.springframework.data.couchbase.domain;
 
-import java.lang.reflect.Field;
-
 /**
  * Submission entity for tests
  *
  * @author Michael Reiche
  */
-public class Submission {
+public class Submission extends ComparableEntity {
 	private final String id;
 	private final String userId;
 	private final String talkId;
@@ -58,42 +56,4 @@ public class Submission {
 		return sb.toString();
 	}
 
-	@Override
-	public boolean equals(Object that) throws RuntimeException {
-		if (this == that)
-			return true;
-		if (that == null || getClass() != that.getClass())
-			return false;
-		for (Field f : getClass().getFields()) {
-			if (!same(f, this, that))
-				return false;
-		}
-		for (Field f : getClass().getDeclaredFields()) {
-			if (!same(f, this, that))
-				return false;
-		}
-		return true;
-	}
-
-	private static boolean same(Field f, Object a, Object b) {
-		Object thisField = null;
-		Object thatField = null;
-
-		try {
-			f.get(a);
-			f.get(b);
-		} catch (IllegalAccessException e) {
-			throw new RuntimeException(e);
-		}
-		if (thisField == null && thatField == null) {
-			return true;
-		}
-		if (thisField == null && thatField != null) {
-			return false;
-		}
-		if (!thisField.equals(thatField)) {
-			return false;
-		}
-		return true;
-	}
 }

src/test/java/org/springframework/data/couchbase/domain/UserSubmission.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2020 the original author or authors
+ * Copyright 2020 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.
@@ -33,7 +33,7 @@ import java.util.List;
 @Document
 @TypeAlias("user")
 @CompositeQueryIndex(fields = { "id", "username", "email" })
-public class UserSubmission {
+public class UserSubmission extends ComparableEntity {
 	private String id;
 	private String username;
 	private String email;
@@ -52,42 +52,4 @@ public class UserSubmission {
 		this.courses = courses;
 	}
 
-	@Override
-	public boolean equals(Object that) throws RuntimeException {
-		if (this == that)
-			return true;
-		if (that == null || getClass() != that.getClass())
-			return false;
-		for (Field f : getClass().getFields()) {
-			if (!same(f, this, that))
-				return false;
-		}
-		for (Field f : getClass().getDeclaredFields()) {
-			if (!same(f, this, that))
-				return false;
-		}
-		return true;
-	}
-
-	private static boolean same(Field f, Object a, Object b) {
-		Object thisField = null;
-		Object thatField = null;
-
-		try {
-			f.get(a);
-			f.get(b);
-		} catch (IllegalAccessException e) {
-			throw new RuntimeException(e);
-		}
-		if (thisField == null && thatField == null) {
-			return true;
-		}
-		if (thisField == null && thatField != null) {
-			return false;
-		}
-		if (!thisField.equals(thatField)) {
-			return false;
-		}
-		return true;
-	}
 }

src/test/java/org/springframework/data/couchbase/domain/UserSubmissionRepository.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2020 the original author or authors
+ * Copyright 2020 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.

src/test/java/org/springframework/data/couchbase/repository/CouchbaseRepositoryKeyValueIntegrationTests.java

@@ -18,6 +18,8 @@ package org.springframework.data.couchbase.repository;
 
 import static org.junit.jupiter.api.Assertions.*;
 
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -33,6 +35,8 @@ import org.springframework.data.couchbase.core.CouchbaseTemplate;
 import org.springframework.data.couchbase.domain.Course;
 import org.springframework.data.couchbase.domain.Library;
 import org.springframework.data.couchbase.domain.LibraryRepository;
+import org.springframework.data.couchbase.domain.PersonValue;
+import org.springframework.data.couchbase.domain.PersonValueRepository;
 import org.springframework.data.couchbase.domain.Submission;
 import org.springframework.data.couchbase.domain.SubscriptionToken;
 import org.springframework.data.couchbase.domain.SubscriptionTokenRepository;
@@ -60,6 +64,7 @@ public class CouchbaseRepositoryKeyValueIntegrationTests extends ClusterAwareInt
 	@Autowired LibraryRepository libraryRepository;
 	@Autowired SubscriptionTokenRepository subscriptionTokenRepository;
 	@Autowired UserSubmissionRepository userSubmissionRepository;
+	@Autowired PersonValueRepository personValueRepository;
 	@Autowired CouchbaseTemplate couchbaseTemplate;
 
 	@Test
@@ -90,6 +95,21 @@ public class CouchbaseRepositoryKeyValueIntegrationTests extends ClusterAwareInt
 		userRepository.delete(user);
 	}
 
+	@Test
+	@IgnoreWhen(clusterTypes = ClusterType.MOCKED)
+	void saveAndFindImmutableById() throws NoSuchMethodException, InvocationTargetException, IllegalAccessException {
+		PersonValue personValue = new PersonValue(null, 0, "f", "l");
+		//assertFalse(personValueRepository.existsById(personValue.getId()));
+		personValue = personValueRepository.save(personValue);
+		Optional<PersonValue> found = personValueRepository.findById(personValue.getId());
+		assertTrue(found.isPresent());
+		Method m = PersonValue.class.getMethod("equals", Object.class);
+		m.invoke(personValue, new Object[] { found.get() });
+		personValue.equals(found.get());
+		assertEquals(personValue, found.get());
+		personValueRepository.delete(personValue);
+	}
+
 	@Test // DATACOUCH-564
 	@IgnoreWhen(clusterTypes = ClusterType.MOCKED)
 	void saveAndFindByIdWithList() {