From 41fa412cb2b7c1214f08cbf0ee35f84442c7add7 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 dc4403f3d..04b4ccac9 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 @@ public T decodeEntity(String id, String source, long cas, Class entityCla 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 @@ public long getCas(final Object entity) { 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 c76acf371..181bbf26e 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 Mono one(T object) { 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 @@ private InsertOptions buildInsertOptions() { } 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 @@ private InsertOptions buildInsertOptions() { @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 9dee87766..c4e8fb690 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 Mono one(T object) { 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 6be2ea505..4ab824f75 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 void doWithPersistentProperty(final CouchbasePersistentProperty prop) { 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 000000000..78cc8831a --- /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 50bd02460..6b7bc71f1 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 @@ * * @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 String toString() { 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 000000000..b98046c01 --- /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 000000000..825ea2d79 --- /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 1522693a2..fddffde06 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 String toString() { 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 78d4975b5..a301b5da6 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 @@ @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 void setCourses(List courses) { 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 d73f8b725..1a3d17b5a 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 0d3db0f33..5b18b05f0 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 @@ 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.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 @@ void saveAndFindById() { 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() {