From fab72ee9c98bb3d038e92186477827063e857f19 Mon Sep 17 00:00:00 2001 From: Haris Alesevic Date: Mon, 5 Oct 2020 19:11:37 +0200 Subject: [PATCH] DATACOUCH-615 Use query in removeByQuery (#264) * DATACOUCH-615 Use query in removeByQuery Use provided query instead of hardcoded statement. * DATACOUCH-615 Better method naming in Query class * DATACOUCH-615 Replace hard-coded meta fields * DATACOUCH-615 Test findByQuery.matching * DATACOUCH-615 Remove failing test --- .../ReactiveFindByQueryOperationSupport.java | 2 +- ...ReactiveRemoveByQueryOperationSupport.java | 16 ++++----- .../data/couchbase/core/query/Query.java | 12 ++++++- .../couchbase/core/query/StringQuery.java | 2 +- ...ouchbaseTemplateQueryIntegrationTests.java | 35 ++++++++++++++++--- .../StringN1qlQueryCreatorMockedTests.java | 12 ++----- .../query/StringN1qlQueryCreatorTests.java | 3 +- 7 files changed, 54 insertions(+), 28 deletions(-) diff --git a/src/main/java/org/springframework/data/couchbase/core/ReactiveFindByQueryOperationSupport.java b/src/main/java/org/springframework/data/couchbase/core/ReactiveFindByQueryOperationSupport.java index 09f8d81a..37777ed0 100644 --- a/src/main/java/org/springframework/data/couchbase/core/ReactiveFindByQueryOperationSupport.java +++ b/src/main/java/org/springframework/data/couchbase/core/ReactiveFindByQueryOperationSupport.java @@ -121,7 +121,7 @@ public class ReactiveFindByQueryOperationSupport implements ReactiveFindByQueryO } private String assembleEntityQuery(final boolean count) { - return query.toN1qlString(template, this.domainType, count); + return query.toN1qlSelectString(template, this.domainType, count); } } diff --git a/src/main/java/org/springframework/data/couchbase/core/ReactiveRemoveByQueryOperationSupport.java b/src/main/java/org/springframework/data/couchbase/core/ReactiveRemoveByQueryOperationSupport.java index 8dd08092..cb2c277c 100644 --- a/src/main/java/org/springframework/data/couchbase/core/ReactiveRemoveByQueryOperationSupport.java +++ b/src/main/java/org/springframework/data/couchbase/core/ReactiveRemoveByQueryOperationSupport.java @@ -15,6 +15,7 @@ */ package org.springframework.data.couchbase.core; +import org.springframework.data.couchbase.core.support.TemplateUtils; import reactor.core.publisher.Flux; import java.util.Optional; @@ -58,14 +59,7 @@ public class ReactiveRemoveByQueryOperationSupport implements ReactiveRemoveByQu @Override public Flux all() { return Flux.defer(() -> { - String bucket = "`" + template.getBucketName() + "`"; - - String typeKey = template.getConverter().getTypeKey(); - String typeValue = template.support().getJavaNameForEntity(domainType); - String where = " WHERE `" + typeKey + "` = \"" + typeValue + "\""; - - String returning = " RETURNING meta().*"; - String statement = "DELETE FROM " + bucket + " " + where + returning; + String statement = assembleDeleteQuery(); return template.getCouchbaseClientFactory().getCluster().reactive().query(statement, buildQueryOptions()) .onErrorMap(throwable -> { @@ -75,7 +69,7 @@ public class ReactiveRemoveByQueryOperationSupport implements ReactiveRemoveByQu return throwable; } }).flatMapMany(ReactiveQueryResult::rowsAsObject) - .map(row -> new RemoveResult(row.getString("id"), row.getLong("cas"), Optional.empty())); + .map(row -> new RemoveResult(row.getString(TemplateUtils.SELECT_ID), row.getLong(TemplateUtils.SELECT_CAS), Optional.empty())); }); } @@ -97,6 +91,10 @@ public class ReactiveRemoveByQueryOperationSupport implements ReactiveRemoveByQu return new ReactiveRemoveByQuerySupport<>(template, domainType, query, scanConsistency); } + private String assembleDeleteQuery() { + return query.toN1qlRemoveString(template, this.domainType); + } + } } diff --git a/src/main/java/org/springframework/data/couchbase/core/query/Query.java b/src/main/java/org/springframework/data/couchbase/core/query/Query.java index 5b154ec1..6d699d40 100644 --- a/src/main/java/org/springframework/data/couchbase/core/query/Query.java +++ b/src/main/java/org/springframework/data/couchbase/core/query/Query.java @@ -243,7 +243,7 @@ public class Query { return sb.toString(); } - public String toN1qlString(ReactiveCouchbaseTemplate template, Class domainClass, boolean isCount) { + public String toN1qlSelectString(ReactiveCouchbaseTemplate template, Class domainClass, boolean isCount) { StringBasedN1qlQueryParser.N1qlSpelValues n1ql = getN1qlSpelValues(template, domainClass, isCount); final StringBuilder statement = new StringBuilder(); appendString(statement, n1ql.selectEntity); // select ... @@ -254,6 +254,16 @@ public class Query { return statement.toString(); } + public String toN1qlRemoveString(ReactiveCouchbaseTemplate template, Class domainClass) { + StringBasedN1qlQueryParser.N1qlSpelValues n1ql = getN1qlSpelValues(template, domainClass, false); + final StringBuilder statement = new StringBuilder(); + appendString(statement, n1ql.delete); // delete ... + appendWhereString(statement, n1ql.filter); // typeKey = typeValue + appendWhere(statement, null); // criteria on this Query + appendString(statement, n1ql.returning); + return statement.toString(); + } + StringBasedN1qlQueryParser.N1qlSpelValues getN1qlSpelValues(ReactiveCouchbaseTemplate template, Class domainClass, boolean isCount) { String typeKey = template.getConverter().getTypeKey(); diff --git a/src/main/java/org/springframework/data/couchbase/core/query/StringQuery.java b/src/main/java/org/springframework/data/couchbase/core/query/StringQuery.java index cb901297..f940b54d 100644 --- a/src/main/java/org/springframework/data/couchbase/core/query/StringQuery.java +++ b/src/main/java/org/springframework/data/couchbase/core/query/StringQuery.java @@ -52,7 +52,7 @@ public class StringQuery extends Query { } @Override - public String toN1qlString(ReactiveCouchbaseTemplate template, Class domainClass, boolean isCount) { + public String toN1qlSelectString(ReactiveCouchbaseTemplate template, Class domainClass, boolean isCount) { final StringBuilder statement = new StringBuilder(); appendInlineN1qlStatement(statement); // apply the string statement // To use generated parameters for literals diff --git a/src/test/java/org/springframework/data/couchbase/core/CouchbaseTemplateQueryIntegrationTests.java b/src/test/java/org/springframework/data/couchbase/core/CouchbaseTemplateQueryIntegrationTests.java index 64be7d9d..5669ea8b 100644 --- a/src/test/java/org/springframework/data/couchbase/core/CouchbaseTemplateQueryIntegrationTests.java +++ b/src/test/java/org/springframework/data/couchbase/core/CouchbaseTemplateQueryIntegrationTests.java @@ -32,9 +32,10 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.AnnotationConfigApplicationContext; -import org.springframework.dao.DataRetrievalFailureException; import org.springframework.data.couchbase.CouchbaseClientFactory; import org.springframework.data.couchbase.SimpleCouchbaseClientFactory; +import org.springframework.data.couchbase.core.query.Query; +import org.springframework.data.couchbase.core.query.QueryCriteria; import org.springframework.data.couchbase.domain.Config; import org.springframework.data.couchbase.domain.NaiveAuditorAware; import org.springframework.data.couchbase.domain.User; @@ -46,13 +47,13 @@ import org.springframework.data.couchbase.util.IgnoreWhen; import com.couchbase.client.core.error.IndexExistsException; import com.couchbase.client.java.query.QueryScanConsistency; -import reactor.core.publisher.Mono; /** * Query tests Theses tests rely on a cb server running * * @author Michael Nitschinger * @author Michael Reiche + * @author Haris Alesevic */ @IgnoreWhen(missesCapabilities = Capabilities.QUERY, clusterTypes = ClusterType.MOCKED) class CouchbaseTemplateQueryIntegrationTests extends ClusterAwareIntegrationTests { @@ -82,10 +83,12 @@ class CouchbaseTemplateQueryIntegrationTests extends ClusterAwareIntegrationTest ApplicationContext ac = new AnnotationConfigApplicationContext(Config.class); couchbaseTemplate = (CouchbaseTemplate) ac.getBean(COUCHBASE_TEMPLATE); reactiveCouchbaseTemplate = (ReactiveCouchbaseTemplate) ac.getBean(REACTIVE_COUCHBASE_TEMPLATE); + // ensure each test starts with clean state + couchbaseTemplate.removeByQuery(User.class).all(); } @Test - void findByQuery() { + void findByQueryAll() { try { User user1 = new User(UUID.randomUUID().toString(), "user1", "user1"); User user2 = new User(UUID.randomUUID().toString(), "user2", "user2"); @@ -128,7 +131,7 @@ class CouchbaseTemplateQueryIntegrationTests extends ClusterAwareIntegrationTest } @Test - void removeByQuery() { + void removeByQueryAll() { User user1 = new User(UUID.randomUUID().toString(), "user1", "user1"); User user2 = new User(UUID.randomUUID().toString(), "user2", "user2"); @@ -144,4 +147,28 @@ class CouchbaseTemplateQueryIntegrationTests extends ClusterAwareIntegrationTest } + @Test + void removeByMatchingQuery() { + User user1 = new User(UUID.randomUUID().toString(), "user1", "user1"); + User user2 = new User(UUID.randomUUID().toString(), "user2", "user2"); + User specialUser = new User(UUID.randomUUID().toString(), "special", "special"); + + couchbaseTemplate.upsertById(User.class).all(Arrays.asList(user1, user2, specialUser)); + + assertTrue(couchbaseTemplate.existsById().one(user1.getId())); + assertTrue(couchbaseTemplate.existsById().one(user2.getId())); + assertTrue(couchbaseTemplate.existsById().one(specialUser.getId())); + + Query nonSpecialUsers = new Query(QueryCriteria.where("firstname").notLike("special")); + + couchbaseTemplate.removeByQuery(User.class).consistentWith(QueryScanConsistency.REQUEST_PLUS) + .matching(nonSpecialUsers) + .all(); + + assertNull(couchbaseTemplate.findById(User.class).one(user1.getId())); + assertNull(couchbaseTemplate.findById(User.class).one(user2.getId())); + assertNotNull(couchbaseTemplate.findById(User.class).one(specialUser.getId())); + + } + } diff --git a/src/test/java/org/springframework/data/couchbase/repository/query/StringN1qlQueryCreatorMockedTests.java b/src/test/java/org/springframework/data/couchbase/repository/query/StringN1qlQueryCreatorMockedTests.java index 7b42450d..4129771e 100644 --- a/src/test/java/org/springframework/data/couchbase/repository/query/StringN1qlQueryCreatorMockedTests.java +++ b/src/test/java/org/springframework/data/couchbase/repository/query/StringN1qlQueryCreatorMockedTests.java @@ -22,22 +22,16 @@ import org.springframework.context.annotation.AnnotationConfigApplicationContext import org.springframework.context.annotation.Configuration; import org.springframework.data.couchbase.config.AbstractCouchbaseConfiguration; import org.springframework.data.couchbase.core.CouchbaseTemplate; -import org.springframework.data.couchbase.core.ExecutableFindByQueryOperation; import org.springframework.data.couchbase.core.convert.CouchbaseConverter; import org.springframework.data.couchbase.core.convert.MappingCouchbaseConverter; import org.springframework.data.couchbase.core.mapping.CouchbaseMappingContext; import org.springframework.data.couchbase.core.mapping.CouchbasePersistentEntity; import org.springframework.data.couchbase.core.mapping.CouchbasePersistentProperty; import org.springframework.data.couchbase.core.query.Query; -import org.springframework.data.couchbase.domain.Airline; -import org.springframework.data.couchbase.domain.AirlineRepository; import org.springframework.data.couchbase.domain.User; import org.springframework.data.couchbase.domain.UserRepository; import org.springframework.data.couchbase.repository.config.EnableCouchbaseRepositories; -import org.springframework.data.couchbase.util.Capabilities; import org.springframework.data.couchbase.util.ClusterAwareIntegrationTests; -import org.springframework.data.couchbase.util.ClusterType; -import org.springframework.data.couchbase.util.IgnoreWhen; import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.projection.SpelAwareProxyProjectionFactory; import org.springframework.data.repository.core.NamedQueries; @@ -46,9 +40,7 @@ import org.springframework.data.repository.core.support.PropertiesBasedNamedQuer import org.springframework.data.repository.query.*; import java.lang.reflect.Method; -import java.util.Optional; import java.util.Properties; -import java.util.UUID; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.fail; @@ -88,7 +80,7 @@ class StringN1qlQueryCreatorMockedTests extends ClusterAwareIntegrationTests { Query query = creator.createQuery(); assertEquals( "SELECT META(`travel-sample`).id AS __id, META(`travel-sample`).cas AS __cas, `travel-sample`.* FROM `travel-sample` where `_class` = \"org.springframework.data.couchbase.domain.User\" and firstname = $1 and lastname = $2", - query.toN1qlString(couchbaseTemplate.reactive(), User.class, false)); + query.toN1qlSelectString(couchbaseTemplate.reactive(), User.class, false)); } @Test @@ -106,7 +98,7 @@ class StringN1qlQueryCreatorMockedTests extends ClusterAwareIntegrationTests { Query query = creator.createQuery(); assertEquals( "SELECT META(`travel-sample`).id AS __id, META(`travel-sample`).cas AS __cas, `travel-sample`.* FROM `travel-sample` where `_class` = \"org.springframework.data.couchbase.domain.User\" and (firstname = $first or lastname = $last)", - query.toN1qlString(couchbaseTemplate.reactive(), User.class, false)); + query.toN1qlSelectString(couchbaseTemplate.reactive(), User.class, false)); } @Test diff --git a/src/test/java/org/springframework/data/couchbase/repository/query/StringN1qlQueryCreatorTests.java b/src/test/java/org/springframework/data/couchbase/repository/query/StringN1qlQueryCreatorTests.java index 640ff307..9681255d 100644 --- a/src/test/java/org/springframework/data/couchbase/repository/query/StringN1qlQueryCreatorTests.java +++ b/src/test/java/org/springframework/data/couchbase/repository/query/StringN1qlQueryCreatorTests.java @@ -25,7 +25,6 @@ import java.util.UUID; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Configuration; @@ -91,7 +90,7 @@ class StringN1qlQueryCreatorTests extends ClusterAwareIntegrationTests { queryMethod, converter, config().bucketname(), QueryMethodEvaluationContextProvider.DEFAULT, namedQueries); Query query = creator.createQuery(); - System.out.println(query.toN1qlString(couchbaseTemplate.reactive(), Airline.class, false)); + System.out.println(query.toN1qlSelectString(couchbaseTemplate.reactive(), Airline.class, false)); try { Thread.sleep(3000);