From cc8194928f46e1a05d74d744560f61b65f96a32e Mon Sep 17 00:00:00 2001 From: mikereiche Date: Wed, 14 Oct 2020 17:12:18 -0700 Subject: [PATCH] DATACOUCH-625 - Save doesn't return updated entity if immutable. 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. --- .../core/CouchbaseTemplateSupport.java | 22 ++++- .../ReactiveInsertByIdOperationSupport.java | 18 ++-- .../ReactiveUpsertByIdOperationSupport.java | 6 +- .../convert/MappingCouchbaseConverter.java | 1 + .../couchbase/domain/ComparableEntity.java | 94 +++++++++++++++++++ .../data/couchbase/domain/Course.java | 40 +------- .../data/couchbase/domain/PersonValue.java | 61 ++++++++++++ .../domain/PersonValueRepository.java | 27 ++++++ .../data/couchbase/domain/Submission.java | 42 +-------- .../data/couchbase/domain/UserSubmission.java | 42 +-------- .../domain/UserSubmissionRepository.java | 2 +- ...aseRepositoryKeyValueIntegrationTests.java | 20 ++++ 12 files changed, 241 insertions(+), 134 deletions(-) create mode 100644 src/test/java/org/springframework/data/couchbase/domain/ComparableEntity.java create mode 100644 src/test/java/org/springframework/data/couchbase/domain/PersonValue.java create mode 100644 src/test/java/org/springframework/data/couchbase/domain/PersonValueRepository.java diff --git a/src/main/java/org/springframework/data/couchbase/core/CouchbaseTemplateSupport.java b/src/main/java/org/springframework/data/couchbase/core/CouchbaseTemplateSupport.java index dc4403f3..04b4ccac 100644 --- a/src/main/java/org/springframework/data/couchbase/core/CouchbaseTemplateSupport.java +++ b/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 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 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; diff --git a/src/main/java/org/springframework/data/couchbase/core/ReactiveInsertByIdOperationSupport.java b/src/main/java/org/springframework/data/couchbase/core/ReactiveInsertByIdOperationSupport.java index c76acf37..181bbf26 100644 --- a/src/main/java/org/springframework/data/couchbase/core/ReactiveInsertByIdOperationSupport.java +++ b/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 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 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 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 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); } } diff --git a/src/main/java/org/springframework/data/couchbase/core/ReactiveUpsertByIdOperationSupport.java b/src/main/java/org/springframework/data/couchbase/core/ReactiveUpsertByIdOperationSupport.java index 9dee8776..c4e8fb69 100644 --- a/src/main/java/org/springframework/data/couchbase/core/ReactiveUpsertByIdOperationSupport.java +++ b/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) { diff --git a/src/main/java/org/springframework/data/couchbase/core/convert/MappingCouchbaseConverter.java b/src/main/java/org/springframework/data/couchbase/core/convert/MappingCouchbaseConverter.java index 6be2ea50..4ab824f7 100644 --- a/src/main/java/org/springframework/data/couchbase/core/convert/MappingCouchbaseConverter.java +++ b/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); diff --git a/src/test/java/org/springframework/data/couchbase/domain/ComparableEntity.java b/src/test/java/org/springframework/data/couchbase/domain/ComparableEntity.java new file mode 100644 index 00000000..78cc8831 --- /dev/null +++ b/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; + } +} diff --git a/src/test/java/org/springframework/data/couchbase/domain/Course.java b/src/test/java/org/springframework/data/couchbase/domain/Course.java index 50bd0246..6b7bc71f 100644 --- a/src/test/java/org/springframework/data/couchbase/domain/Course.java +++ b/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; - } } diff --git a/src/test/java/org/springframework/data/couchbase/domain/PersonValue.java b/src/test/java/org/springframework/data/couchbase/domain/PersonValue.java new file mode 100644 index 00000000..b98046c0 --- /dev/null +++ b/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(); + } + +} diff --git a/src/test/java/org/springframework/data/couchbase/domain/PersonValueRepository.java b/src/test/java/org/springframework/data/couchbase/domain/PersonValueRepository.java new file mode 100644 index 00000000..825ea2d7 --- /dev/null +++ b/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 { + +} diff --git a/src/test/java/org/springframework/data/couchbase/domain/Submission.java b/src/test/java/org/springframework/data/couchbase/domain/Submission.java index 1522693a..fddffde0 100644 --- a/src/test/java/org/springframework/data/couchbase/domain/Submission.java +++ b/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; - } } diff --git a/src/test/java/org/springframework/data/couchbase/domain/UserSubmission.java b/src/test/java/org/springframework/data/couchbase/domain/UserSubmission.java index 78d4975b..a301b5da 100644 --- a/src/test/java/org/springframework/data/couchbase/domain/UserSubmission.java +++ b/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; - } } diff --git a/src/test/java/org/springframework/data/couchbase/domain/UserSubmissionRepository.java b/src/test/java/org/springframework/data/couchbase/domain/UserSubmissionRepository.java index d73f8b72..1a3d17b5 100644 --- a/src/test/java/org/springframework/data/couchbase/domain/UserSubmissionRepository.java +++ b/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. diff --git a/src/test/java/org/springframework/data/couchbase/repository/CouchbaseRepositoryKeyValueIntegrationTests.java b/src/test/java/org/springframework/data/couchbase/repository/CouchbaseRepositoryKeyValueIntegrationTests.java index 0d3db0f3..5b18b05f 100644 --- a/src/test/java/org/springframework/data/couchbase/repository/CouchbaseRepositoryKeyValueIntegrationTests.java +++ b/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 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() {