From 4aa823722aa5f32fdd17557854cab72e28183d0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Basl=C3=A9?= Date: Thu, 10 Mar 2016 22:06:05 +0100 Subject: [PATCH] DATACOUCH-212 - Take Version/CAS into account on save This change takes the `Version` annotated field into account when performing a `save` (either in the `CouchbaseTemplate` or transitively in a `CouchbaseRepository`). Having such a field with a non-zero value means that a CAS is available, activating optimistic locking. Version 1.4 and below of Spring Data Couchbase was taking this CAS into account, whereas in 2.0 we switched to using `upsert` internally, which ignores the CAS. This change brings the behavior of `save` (when there's a non-zero CAS) closer to what was observed in 1.4 versions: an `OptimisticLockingFailureException` can now be raised in case of CAS mismatch. --- .../SimpleCouchbaseRepositoryTests.java | 89 +++++++++++++++++++ .../couchbase/core/CouchbaseTemplate.java | 76 +++++++++++----- .../support/SimpleCouchbaseRepository.java | 1 - 3 files changed, 141 insertions(+), 25 deletions(-) diff --git a/src/integration/java/org/springframework/data/couchbase/repository/SimpleCouchbaseRepositoryTests.java b/src/integration/java/org/springframework/data/couchbase/repository/SimpleCouchbaseRepositoryTests.java index 937c5dda..9dbf8687 100644 --- a/src/integration/java/org/springframework/data/couchbase/repository/SimpleCouchbaseRepositoryTests.java +++ b/src/integration/java/org/springframework/data/couchbase/repository/SimpleCouchbaseRepositoryTests.java @@ -22,6 +22,8 @@ import java.util.Arrays; import java.util.List; import com.couchbase.client.java.Bucket; +import com.couchbase.client.java.document.JsonDocument; +import com.couchbase.client.java.error.CASMismatchException; import com.couchbase.client.java.view.Stale; import com.couchbase.client.java.view.ViewQuery; import org.junit.Before; @@ -30,8 +32,12 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.dao.OptimisticLockingFailureException; +import org.springframework.data.annotation.Id; +import org.springframework.data.annotation.Version; import org.springframework.data.couchbase.IntegrationTestApplicationConfig; import org.springframework.data.couchbase.core.CouchbaseQueryExecutionException; +import org.springframework.data.couchbase.core.mapping.Document; import org.springframework.data.couchbase.repository.config.RepositoryOperationsMapping; import org.springframework.data.couchbase.repository.support.CouchbaseRepositoryFactory; import org.springframework.data.couchbase.repository.support.IndexManager; @@ -58,11 +64,13 @@ public class SimpleCouchbaseRepositoryTests { private IndexManager indexManager; private UserRepository repository; + private VersionedDataRepository versionedDataRepository; @Before public void setup() throws Exception { RepositoryFactorySupport factory = new CouchbaseRepositoryFactory(operationsMapping, indexManager); repository = factory.getRepository(UserRepository.class); + versionedDataRepository = factory.getRepository(VersionedDataRepository.class); } @Test @@ -180,4 +188,85 @@ public class SimpleCouchbaseRepositoryTests { } } } + + @Test + public void shouldTakeVersionIntoAccountWhenDoingMultipleUpdates() { + final String key = "versionedUserTest"; + VersionedData initial = new VersionedData(key, "ABCD"); + versionedDataRepository.save(initial); + assertNotEquals(0L, initial.version); + + VersionedData fetch1 = versionedDataRepository.findOne(key); + assertNotSame(initial, fetch1); + assertEquals(fetch1.version, initial.version); + + JsonDocument bypass = client.get(key); + bypass.content().put("data", "BBBB"); + JsonDocument bypassed = client.upsert(bypass); + + assertNotEquals(bypassed.cas(), fetch1.version); + System.out.println(bypassed.cas()); + + try { + fetch1.setData("ZZZZ"); + versionedDataRepository.save(fetch1); + fail("Expected CAS failure"); + } catch (OptimisticLockingFailureException e) { + //success + assertTrue("optimistic locking should have CASMismatchException as cause, got " + e.getCause(), + e.getCause() instanceof CASMismatchException); + } finally { + client.remove(key); + } + } + + public interface VersionedDataRepository extends CouchbaseRepository { } + + @Document + public static class VersionedData { + + @Id + private final String key; + + @Version + public long version = 0L; + + private String data; + + public VersionedData(String key, String data) { + this.key = key; + this.data = data; + } + + public String getKey() { + return key; + } + + public String getData() { + return data; + } + + public void setData(String data) { + this.data = data; + } + + @Override + public String toString() { + return this.key + " " + this.data; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + VersionedData vd = (VersionedData) o; + return key.equals(vd.key); + } + + @Override + public int hashCode() { + return key.hashCode(); + } + } + } diff --git a/src/main/java/org/springframework/data/couchbase/core/CouchbaseTemplate.java b/src/main/java/org/springframework/data/couchbase/core/CouchbaseTemplate.java index 9c8885d4..f97105f7 100644 --- a/src/main/java/org/springframework/data/couchbase/core/CouchbaseTemplate.java +++ b/src/main/java/org/springframework/data/couchbase/core/CouchbaseTemplate.java @@ -34,6 +34,8 @@ import com.couchbase.client.java.document.Document; import com.couchbase.client.java.document.RawJsonDocument; import com.couchbase.client.java.document.json.JsonObject; import com.couchbase.client.java.error.CASMismatchException; +import com.couchbase.client.java.error.DocumentAlreadyExistsException; +import com.couchbase.client.java.error.DocumentDoesNotExistException; import com.couchbase.client.java.error.TranscodingException; import com.couchbase.client.java.query.N1qlQuery; import com.couchbase.client.java.query.N1qlQueryResult; @@ -220,7 +222,7 @@ public class CouchbaseTemplate implements CouchbaseOperations, ApplicationEventP @Override public void save(Object objectToSave, PersistTo persistTo, ReplicateTo replicateTo) { - doPersist(objectToSave, persistTo, replicateTo, false, false); + doPersist(objectToSave, persistTo, replicateTo, PersistType.SAVE); } @Override @@ -231,7 +233,7 @@ public class CouchbaseTemplate implements CouchbaseOperations, ApplicationEventP @Override public void save(Collection batchToSave, PersistTo persistTo, ReplicateTo replicateTo) { for (Object o : batchToSave) { - doPersist(o, persistTo, replicateTo, false, false); + doPersist(o, persistTo, replicateTo, PersistType.SAVE); } } @@ -242,7 +244,7 @@ public class CouchbaseTemplate implements CouchbaseOperations, ApplicationEventP @Override public void insert(Object objectToInsert, PersistTo persistTo, ReplicateTo replicateTo) { - doPersist(objectToInsert, persistTo, replicateTo, true, false); + doPersist(objectToInsert, persistTo, replicateTo, PersistType.INSERT); } @Override @@ -253,7 +255,7 @@ public class CouchbaseTemplate implements CouchbaseOperations, ApplicationEventP @Override public void insert(Collection batchToInsert, PersistTo persistTo, ReplicateTo replicateTo) { for (Object o : batchToInsert) { - doPersist(o, persistTo, replicateTo, true, false); + doPersist(o, persistTo, replicateTo, PersistType.INSERT); } } @@ -264,7 +266,7 @@ public class CouchbaseTemplate implements CouchbaseOperations, ApplicationEventP @Override public void update(Object objectToUpdate, PersistTo persistTo, ReplicateTo replicateTo) { - doPersist(objectToUpdate, persistTo, replicateTo, false, true); + doPersist(objectToUpdate, persistTo, replicateTo, PersistType.UPDATE); } @Override @@ -275,7 +277,7 @@ public class CouchbaseTemplate implements CouchbaseOperations, ApplicationEventP @Override public void update(Collection batchToUpdate, PersistTo persistTo, ReplicateTo replicateTo) { for (Object o : batchToUpdate) { - doPersist(o, persistTo, replicateTo, false, true); + doPersist(o, persistTo, replicateTo, PersistType.UPDATE); } } @@ -509,11 +511,9 @@ public class CouchbaseTemplate implements CouchbaseOperations, ApplicationEventP } private void doPersist(Object objectToPersist, final PersistTo persistTo, final ReplicateTo replicateTo, - final boolean failOnExist, final boolean failOnMissing) { + final PersistType persistType) { ensureNotIterable(objectToPersist); - final String operationDesc = failOnExist ? "Insert" : failOnMissing ? "Update" : "Upsert"; - final ConvertingPropertyAccessor accessor = getPropertyAccessor(objectToPersist); final CouchbasePersistentEntity persistentEntity = mappingContext.getPersistentEntity(objectToPersist.getClass()); final CouchbasePersistentProperty versionProperty = persistentEntity.getVersionProperty(); @@ -529,15 +529,23 @@ public class CouchbaseTemplate implements CouchbaseOperations, ApplicationEventP public Boolean doInBucket() throws InterruptedException, ExecutionException { Document doc = encodeAndWrap(converted, version); Document storedDoc; + boolean checkVersion = version != null && version > 0L; try { - if (!failOnExist && !failOnMissing) { - storedDoc = client.upsert(doc, persistTo, replicateTo); - } - else if (failOnMissing) { - storedDoc = client.replace(doc, persistTo, replicateTo); - } - else { - storedDoc = client.insert(doc, persistTo, replicateTo); + switch (persistType) { + case SAVE: + if (checkVersion) { + storedDoc = client.replace(doc, persistTo, replicateTo); + } else { + storedDoc = client.upsert(doc, persistTo, replicateTo); + } + break; + case UPDATE: + storedDoc = client.replace(doc, persistTo, replicateTo); + break; + case INSERT: + default: + storedDoc = client.insert(doc, persistTo, replicateTo); + break; } if (persistentEntity.hasVersionProperty() && storedDoc != null && storedDoc.cas() != 0) { @@ -546,13 +554,11 @@ public class CouchbaseTemplate implements CouchbaseOperations, ApplicationEventP return true; } return false; - } - catch (CASMismatchException e) { - throw new OptimisticLockingFailureException(operationDesc + - " document with version value failed: " + version); - } - catch (Exception e) { - handleWriteResultError(operationDesc + " document failed: " + e.getMessage(), e); + } catch (CASMismatchException e) { + throw new OptimisticLockingFailureException(persistType.getSpringDataOperationName() + + " document with version value failed: " + version, e); + } catch (Exception e) { + handleWriteResultError(persistType.getSpringDataOperationName() + " document failed: " + e.getMessage(), e); return false; //this could be skipped if WriteResultChecking.EXCEPTION } } @@ -645,4 +651,26 @@ public class CouchbaseTemplate implements CouchbaseOperations, ApplicationEventP public void setDefaultConsistency(Consistency consistency) { this.configuredConsistency = consistency; } + + private enum PersistType { + SAVE("Save", "Upsert"), + INSERT("Insert", "Insert"), + UPDATE("Update", "Replace"); + + private final String sdkOperationName; + private final String springDataOperationName; + + PersistType(String sdkOperationName, String springDataOperationName) { + this.sdkOperationName = sdkOperationName; + this.springDataOperationName = springDataOperationName; + } + + public String getSdkOperationName() { + return sdkOperationName; + } + + public String getSpringDataOperationName() { + return springDataOperationName; + } + } } diff --git a/src/main/java/org/springframework/data/couchbase/repository/support/SimpleCouchbaseRepository.java b/src/main/java/org/springframework/data/couchbase/repository/support/SimpleCouchbaseRepository.java index 28a53a1a..08214668 100644 --- a/src/main/java/org/springframework/data/couchbase/repository/support/SimpleCouchbaseRepository.java +++ b/src/main/java/org/springframework/data/couchbase/repository/support/SimpleCouchbaseRepository.java @@ -82,7 +82,6 @@ public class SimpleCouchbaseRepository implements Co @Override public S save(S entity) { Assert.notNull(entity, "Entity must not be null!"); - couchbaseOperations.save(entity); return entity; }