diff --git a/src/main/asciidoc/repository.adoc b/src/main/asciidoc/repository.adoc index b1cf4e8b..19e533f2 100644 --- a/src/main/asciidoc/repository.adoc +++ b/src/main/asciidoc/repository.adoc @@ -379,6 +379,7 @@ One aspect that is often needed and doesn't have a direct equivalent in the Spri data, because the latest version hasn't been indexed yet. This gives the best performance at the expense of consistency. Note that weaker consistencies can lead to data being returned that doesn't match the criteria of a derived query. +One trickier case is when documents are deleted from Couchbase but views have not yet caught up to the deletion. With weak consistency this can mean that a view would return IDs that are not in the database anymore, leading to null entities. The `CouchbaseTemplate`s `findByView` and `findBySpatialView` methods will remove such stale deleted entities from their result in order to avoid having nulls in the returned collections. Similarly, `CouchbaseRepository`'s `deleteAll` method will ignore documents that the backing view provided but the SDK remove operation couldn't find. If one wants to have stronger consistency, there are two possibilities described in the next sections. diff --git a/src/main/java/org/springframework/data/couchbase/core/CouchbaseExceptionTranslator.java b/src/main/java/org/springframework/data/couchbase/core/CouchbaseExceptionTranslator.java index 369d4c4f..a0a246ff 100644 --- a/src/main/java/org/springframework/data/couchbase/core/CouchbaseExceptionTranslator.java +++ b/src/main/java/org/springframework/data/couchbase/core/CouchbaseExceptionTranslator.java @@ -33,6 +33,7 @@ import com.couchbase.client.java.error.BucketDoesNotExistException; import com.couchbase.client.java.error.CASMismatchException; import com.couchbase.client.java.error.DesignDocumentException; import com.couchbase.client.java.error.DocumentAlreadyExistsException; +import com.couchbase.client.java.error.DocumentDoesNotExistException; import com.couchbase.client.java.error.DurabilityException; import com.couchbase.client.java.error.InvalidPasswordException; import com.couchbase.client.java.error.RequestTooBigException; @@ -90,6 +91,10 @@ public class CouchbaseExceptionTranslator implements PersistenceExceptionTransla return new DuplicateKeyException(ex.getMessage(), ex); } + if (ex instanceof DocumentDoesNotExistException) { + return new DataRetrievalFailureException(ex.getMessage(), ex); + } + if (ex instanceof CASMismatchException || ex instanceof DocumentConcurrentlyModifiedException || ex instanceof ReplicaNotConfiguredException diff --git a/src/main/java/org/springframework/data/couchbase/core/CouchbaseOperations.java b/src/main/java/org/springframework/data/couchbase/core/CouchbaseOperations.java index c815ece8..89a77dfc 100644 --- a/src/main/java/org/springframework/data/couchbase/core/CouchbaseOperations.java +++ b/src/main/java/org/springframework/data/couchbase/core/CouchbaseOperations.java @@ -190,9 +190,13 @@ public interface CouchbaseOperations { /** * Query a View for a list of documents of type T. *

- *

There is no need to {@link ViewQuery#includeDocs(boolean) set includeDocs} explicitly, because it will be set to - * true all the time. It is valid to pass in a empty constructed {@link ViewQuery} object.

+ *

There is no need to {@link ViewQuery#includeDocs(boolean) set includeDocs} explicitly, since this method will + * manage the retrieval of documents internally. It is valid to pass in a empty constructed {@link ViewQuery} object.

*

+ *

Weak consistency in the query (stale(Stale.TRUE)) can lead to some documents being unreachable due + * to their deletion not having been indexed. These deleted null documents are eliminated from the result of this + * method.

+ *

*

This method does not work with reduced views, because they by design do not contain references to original * objects. Use the provided {@link #queryView} method for more flexibility and direct access.

* @@ -217,9 +221,13 @@ public interface CouchbaseOperations { /** * Query a Spatial View for a list of documents of type T. + *

+ *

There is no need to {@link SpatialViewQuery#includeDocs(boolean) set includeDocs} explicitly, since this method + * will manage the retrieval of documents internally. It is valid to pass in a empty constructed {@link SpatialViewQuery} object.

*

- *

It is valid to pass in a empty constructed {@link SpatialViewQuery} object.

- *

+ *

Weak consistency in the query (stale(Stale.TRUE)) can lead to some documents being unreachable due + * to their deletion not having been indexed. These deleted null documents are eliminated from the result of this + * method.

* * @param query the SpatialViewQuery object (also specifying view design document and view name). * @param entityClass the entity to map to. @@ -248,6 +256,10 @@ public interface CouchbaseOperations { * statement contains placeholders. *
* Use {@link N1qlQuery}'s factory methods to construct such a Query.

+ *

+ *

Weak consistency in the query (eg. ScanConsistency.NOT_BOUND) can lead to some documents being + * unreachable due to their deletion not having been indexed. These deleted null documents are eliminated from the + * result of this method.

* * @param n1ql the N1QL query. * @param entityClass the target class for the returned entities. @@ -266,6 +278,10 @@ public interface CouchbaseOperations { * statement contains placeholders. *
* Use {@link N1qlQuery}'s factory methods to construct such a Query.

+ *

+ *

Weak consistency in the query (eg. ScanConsistency.NOT_BOUND) can lead to some documents being + * unreachable due to their deletion not having been indexed. These deleted null documents are eliminated from the + * result of this method.

* * @param n1ql the N1QL query. * @param fragmentClass the target class for the returned fragments. 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 bd8305d7..4384fc22 100644 --- a/src/main/java/org/springframework/data/couchbase/core/CouchbaseTemplate.java +++ b/src/main/java/org/springframework/data/couchbase/core/CouchbaseTemplate.java @@ -293,7 +293,10 @@ public class CouchbaseTemplate implements CouchbaseOperations, ApplicationEventP @Override public List findByView(ViewQuery query, Class entityClass) { - query.includeDocs(false); //FIXME the doc says it is set to true... + //we'll always need to get documents, as a RawJsonDocument, so we should force includeDocs(false) + //so that the caller doesn't set a bad target class unintentionally, pre-loading with a bad type. + query.includeDocs(false); + //we'll always map the document to the entity, hence reduce never makes sense. query.reduce(false); try { @@ -307,7 +310,11 @@ public class CouchbaseTemplate implements CouchbaseOperations, ApplicationEventP final List result = new ArrayList(allRows.size()); for (final ViewRow row : allRows) { - result.add(mapToEntity(row.id(), row.document(RawJsonDocument.class), entityClass)); + //cope with potential weak consistency and deletions + T entity = mapToEntity(row.id(), row.document(RawJsonDocument.class), entityClass); + if (entity != null) { + result.add(entity); + } } return result; @@ -329,7 +336,9 @@ public class CouchbaseTemplate implements CouchbaseOperations, ApplicationEventP @Override public List findBySpatialView(SpatialViewQuery query, Class entityClass) { - //note: don't make any assumption about includeDocs and let the user decide + //we'll always need to get documents, as a RawJsonDocument, so we should force includeDocs(false) + //so that the caller doesn't set a bad target class unintentionally, pre-loading with a bad type. + query.includeDocs(false); try { final SpatialViewResult response = querySpatialView(query); @@ -342,7 +351,11 @@ public class CouchbaseTemplate implements CouchbaseOperations, ApplicationEventP final List result = new ArrayList(allRows.size()); for (final SpatialViewRow row : allRows) { - result.add(mapToEntity(row.id(), row.document(RawJsonDocument.class), entityClass)); + //cope with potential weak consistency and deletions + T entity = mapToEntity(row.id(), row.document(RawJsonDocument.class), entityClass); + if (entity != null) { + result.add(entity); + } } return result; @@ -379,7 +392,7 @@ public class CouchbaseTemplate implements CouchbaseOperations, ApplicationEventP throw new CouchbaseQueryExecutionException("Unable to retrieve enough metadata for N1QL to entity mapping, " + "have you selected " + SELECT_ID + " and " + SELECT_CAS + "?"); } - json = json.removeKey("_ID").removeKey("_CAS"); + json = json.removeKey(SELECT_ID).removeKey(SELECT_CAS); RawJsonDocument entityDoc = RawJsonDocument.create(id, json.toString(), cas); T decoded = mapToEntity(id, entityDoc, entityClass); result.add(decoded); 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 23837bad..81acda32 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 @@ -21,10 +21,12 @@ import java.util.ArrayList; import java.util.List; import com.couchbase.client.java.document.json.JsonArray; +import com.couchbase.client.java.error.DocumentDoesNotExistException; import com.couchbase.client.java.view.ViewQuery; import com.couchbase.client.java.view.ViewResult; import com.couchbase.client.java.view.ViewRow; +import org.springframework.dao.DataRetrievalFailureException; import org.springframework.data.couchbase.core.CouchbaseOperations; import org.springframework.data.couchbase.core.query.View; import org.springframework.data.couchbase.repository.CouchbaseRepository; @@ -180,7 +182,12 @@ public class SimpleCouchbaseRepository implements Co ViewResult response = couchbaseOperations.queryView(query); for (ViewRow row : response) { - couchbaseOperations.remove(row.id()); + try { + couchbaseOperations.remove(row.id()); + } catch (DataRetrievalFailureException e) { + //ignore stale deletions + if (!(e.getCause() instanceof DocumentDoesNotExistException)) throw e; + } } } diff --git a/src/test/java/org/springframework/data/couchbase/repository/RepositoryIndexUsageTest.java b/src/test/java/org/springframework/data/couchbase/repository/RepositoryIndexUsageTest.java index 97fbde66..7b32860e 100644 --- a/src/test/java/org/springframework/data/couchbase/repository/RepositoryIndexUsageTest.java +++ b/src/test/java/org/springframework/data/couchbase/repository/RepositoryIndexUsageTest.java @@ -2,6 +2,7 @@ package org.springframework.data.couchbase.repository; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.mockito.Matchers.any; import static org.mockito.Mockito.*; @@ -11,6 +12,7 @@ import java.util.List; import com.couchbase.client.java.Bucket; import com.couchbase.client.java.document.json.JsonObject; +import com.couchbase.client.java.error.DocumentDoesNotExistException; import com.couchbase.client.java.query.N1qlQuery; import com.couchbase.client.java.view.ViewQuery; import com.couchbase.client.java.view.ViewResult; @@ -19,6 +21,7 @@ import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; +import org.springframework.dao.DataRetrievalFailureException; import org.springframework.data.couchbase.core.CouchbaseOperations; import org.springframework.data.couchbase.core.convert.CouchbaseConverter; import org.springframework.data.couchbase.core.query.Consistency; @@ -39,8 +42,10 @@ public class RepositoryIndexUsageTest { public void initMocks() { ViewRow mockCountRow1 = mock(ViewRow.class); when(mockCountRow1.value()).thenReturn("100"); + when(mockCountRow1.id()).thenReturn("id1"); ViewRow mockCountRow2 = mock(ViewRow.class); when(mockCountRow2.value()).thenReturn("200"); + when(mockCountRow2.id()).thenReturn("id2"); List allCountRows = Arrays.asList(mockCountRow1, mockCountRow2); ViewResult mockCountResult = mock(ViewResult.class); @@ -164,4 +169,20 @@ public class RepositoryIndexUsageTest { String statement = query.getString("statement"); assertTrue("Expected " + expectedLimitClause + " in " + statement, statement.contains(expectedLimitClause)); } + + @Test + public void testDeleteAllSwallowsDocumentDoesNotExistException() { + doThrow(new DataRetrievalFailureException("ignored", new DocumentDoesNotExistException())).when(couchbaseOperations).remove("id1"); + doThrow(new DataRetrievalFailureException("thrown")).when(couchbaseOperations).remove("id2"); + try { + repository.deleteAll(); + fail("Expected DataRetrievalFailureException on id2"); + } catch (DataRetrievalFailureException e) { + if (!"thrown".equals(e.getMessage())) { + fail("DataRetrievalFailureException caused by DocumentDoesNotExistException should have been ignored"); + } + } + verify(couchbaseOperations).remove("id1"); + verify(couchbaseOperations).remove("id2"); + } } \ No newline at end of file