Fix 1441 in 4_4_x - scope and collection not considered in repo delete. #1465

mikereiche · 2022-06-16T22:22:35Z

This is a more localized change than the one in main.

Closes #1464.

You have read the Spring Data contribution guidelines.
There is a ticket in the bug tracker for the project in our JIRA.
You use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
You submit test cases (unit or integration tests) that back your changes.
You added yourself as author in the headers of the classes you touched. Amend the date range in the Apache license header if needed. For new types, add the license header (copy from another file and set the current year only).

This is a more localized change than the one in main. Closes #1464.

mikereiche · 2022-06-16T23:17:15Z

...framework/data/couchbase/repository/ReactiveCouchbaseRepositoryKeyValueIntegrationTests.java

@@ -60,7 +60,7 @@ public class ReactiveCouchbaseRepositoryKeyValueIntegrationTests extends Cluster

 	@Autowired ReactiveUserRepository userRepository;

-	@Autowired ReactiveAirportRepository airportRepository;
+	@Autowired ReactiveAirportRepository reactiveAirportRepository;



I had to rename the ReactiveAiportRepository to exactly match the bean-name for disambiguation - as the ReactiveAirportRepositoryAnnotated is also a ReactiveAirportRepository.

mikereiche · 2022-06-16T23:18:37Z

...work/data/couchbase/repository/query/CouchbaseRepositoryQueryCollectionIntegrationTests.java


+@SpringJUnitConfig(Config.class)
 @IgnoreWhen(missesCapabilities = { Capabilities.QUERY, Capabilities.COLLECTIONS }, clusterTypes = ClusterType.MOCKED)


the missing @SpringJunitConfig was causing the @Autowired not to work, and requiring a hack in beforeAll()

daschl

Just a couple comments, but looks good overall.

daschl · 2022-06-17T13:57:58Z

...java/org/springframework/data/couchbase/repository/query/AbstractReactiveCouchbaseQuery.java

-		this.findOperationWithProjection = findOp;
+		this.findOp = (ReactiveFindByQuery<?>) (operations.findByQuery(type).inScope(method.getScope())
+				.inCollection(method.getCollection()));
+		this.removeOp = (ReactiveRemoveByQuery<?>) (operations.removeByQuery(type).inScope(method.getScope())


can it be casted to findBy and RemoveBy at the same time?

daschl · 2022-06-17T13:58:33Z

src/main/java/org/springframework/data/couchbase/repository/query/AbstractCouchbaseQuery.java

@@ -65,6 +67,8 @@ public AbstractCouchbaseQuery(CouchbaseQueryMethod method, CouchbaseOperations o
 		ExecutableFindByQuery<?> findOp = operations.findByQuery(type);
 		findOp = (ExecutableFindByQuery<?>) (findOp.inScope(method.getScope()).inCollection(method.getCollection()));
 		this.findOperationWithProjection = findOp;
+		this.removeOp = (ExecutableRemoveByQuery<?>) (operations.removeByQuery(type).inScope(method.getScope())


same as comment below, is it possible that not both casts are possible?

If I understand the comment correctly - both are fine. the first one is operations.findByQuery(), the second is operations.removeByQuery(). I'm actually not sure why a cast is even required - because ExecutableRemoveByQuery (eventually) extends the type returned by withCollection() (RemoveByQueryWithOptions).

daschl · 2022-06-17T14:01:27Z

src/test/java/org/springframework/data/couchbase/domain/ReactiveAirportRepository.java

@@ -99,6 +101,15 @@
 	@ScanConsistency(query = QueryScanConsistency.REQUEST_PLUS)
 	Mono<Airport> findByIata(String iata);

+	@Query("#{#n1ql.delete} WHERE #{#n1ql.filter} and  iata = $1 #{#n1ql.returning}")


nit: extra whitespace between and and iata, also below

Oof. Formatter does not reformat quoted strings!

daschl · 2022-06-17T14:01:45Z

...framework/data/couchbase/repository/ReactiveCouchbaseRepositoryKeyValueIntegrationTests.java

@@ -60,7 +60,7 @@ public class ReactiveCouchbaseRepositoryKeyValueIntegrationTests extends Cluster

 	@Autowired ReactiveUserRepository userRepository;

-	@Autowired ReactiveAirportRepository airportRepository;
+	@Autowired ReactiveAirportRepository reactiveAirportRepository;



Fix 1441 in 4_4_x - scope and collection not considered in repo delete.

3f15c10

This is a more localized change than the one in main. Closes #1464.

mikereiche requested a review from daschl June 16, 2022 22:22

mikereiche commented Jun 16, 2022

View reviewed changes

daschl approved these changes Jun 17, 2022

View reviewed changes

Update ReactiveAirportRepository.java

33e32fc

mikereiche merged commit 857a364 into 4.4.x Jun 17, 2022

mikereiche deleted the datacouch_1464_fix_1441_in_4_4_x_consider_scope_and_collection_in_repo_delete branch June 21, 2022 17:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix 1441 in 4_4_x - scope and collection not considered in repo delete. #1465

Fix 1441 in 4_4_x - scope and collection not considered in repo delete. #1465

mikereiche commented Jun 16, 2022

mikereiche Jun 16, 2022

daschl Jun 17, 2022

mikereiche Jun 16, 2022

daschl left a comment

daschl Jun 17, 2022

daschl Jun 17, 2022

mikereiche Jun 17, 2022

daschl Jun 17, 2022

mikereiche Jun 17, 2022

daschl Jun 17, 2022


		@SpringJUnitConfig(Config.class)
		@IgnoreWhen(missesCapabilities = { Capabilities.QUERY, Capabilities.COLLECTIONS }, clusterTypes = ClusterType.MOCKED)

Fix 1441 in 4_4_x - scope and collection not considered in repo delete. #1465

Fix 1441 in 4_4_x - scope and collection not considered in repo delete. #1465

Conversation

mikereiche commented Jun 16, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daschl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment