Scopes and collections for repositories #1149

mikereiche · 2021-06-17T22:30:54Z

This adds scope and collection support to repositories via the
DynamicInvocationHandler and PseudoArgs.
It also adds annotations for scopes and collections.

Closes #963

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).

Adds DynamicProxyable and DynamicInvocationHandler to set scope/collection/options on PseudoArgs when calling operations via repository interfaces. Closes #963.

This adds scope and collection support to repositories via the DynamicInvocationHandler and PseudoArgs. It also adds annotations for scopes and collections. Closes #963

src/main/java/org/springframework/data/couchbase/core/CouchbaseTemplate.java

.../java/org/springframework/data/couchbase/core/ExecutableFindByAnalyticsOperationSupport.java

src/main/java/org/springframework/data/couchbase/core/ReactiveCouchbaseOperations.java

src/main/java/org/springframework/data/couchbase/core/ReactiveCouchbaseTemplateSupport.java

...g/springframework/data/couchbase/repository/auditing/ReactiveCouchbaseAuditingRegistrar.java

...main/java/org/springframework/data/couchbase/repository/support/CouchbaseRepositoryBase.java

...a/org/springframework/data/couchbase/repository/support/CrudMethodMetadataPostProcessor.java

mikereiche · 2021-06-21T23:15:26Z

...ain/java/org/springframework/data/couchbase/repository/support/DynamicInvocationHandler.java

+			throw ite.getCause();
+		}
+		return result;
+	}


similar to CrudMethodMetadataPostProcessor - there is no need to reset the thread-local data after the call as this is in the same thread.

...org/springframework/data/couchbase/repository/support/SimpleReactiveCouchbaseRepository.java

src/main/java/org/springframework/data/couchbase/SimpleCouchbaseClientFactory.java

src/main/java/org/springframework/data/couchbase/core/CouchbaseTemplate.java

src/main/java/org/springframework/data/couchbase/core/mapping/Expiry.java

src/main/java/org/springframework/data/couchbase/core/convert/MappingCouchbaseConverter.java

daschl · 2021-06-23T08:38:42Z

src/main/java/org/springframework/data/couchbase/repository/CouchbaseRepository.java

@@ -43,4 +47,8 @@
 	@Override
 	List<T> findAllById(Iterable<ID> iterable);

+	CouchbaseEntityInformation<T, String> getEntityInformation();


is it ok to add this here @mp911de ?

[Mike Reiche] - This is no longer necessary since I refactored it into CouchbaseRepositoryBase.

@mikereiche based on your comment this needs to be removed then from this PR

Yes. I will move it to SimpleCouchbaseRepository and SimpleReactiveCouchbaseRepository.

Edit: I will leave this here - I tried moving it to SimpleCouchbaseRepository, but casting the 'target' in DynamicInvocationHandler to a SimpleCouchbaseRepository results in a ClassCastException due to being loaded in a different class loader.

…ositories

daschl

This is getting close -- a couple comments/nits, but the big one is still that the interfaces should not get the new public signatures I think (it's in the comments)

...ain/java/org/springframework/data/couchbase/core/mapping/BasicCouchbasePersistentEntity.java

src/main/java/org/springframework/data/couchbase/core/query/Meta.java

src/main/java/org/springframework/data/couchbase/core/query/OptionsBuilder.java

...main/java/org/springframework/data/couchbase/repository/support/CouchbaseRepositoryBase.java

daschl · 2021-07-15T07:50:30Z

...main/java/org/springframework/data/couchbase/repository/support/CouchbaseRepositoryBase.java

+	}
+
+	/**
+	 * Get the Scope from <br>


dangling comment block?

daschl · 2021-07-15T07:51:45Z

...a/org/springframework/data/couchbase/repository/support/CrudMethodMetadataPostProcessor.java

@@ -161,10 +166,10 @@ public Object invoke(MethodInvocation invocation) throws Throwable {
 				try {
 					return invocation.proceed();
 				} finally {
-					TransactionSynchronizationManager.unbindResource(method);
+					// TransactionSynchronizationManager.unbindResource(method);


The original CrudMethodMetadataPostProcessor - copied from the JPA project - included code to bind/unbind the 'method' and push/pop the 'invocation'. The problem with unbinding/popping is that for reactive methods, that might take place before the information is consumed in the calling method. The unbinding/popping does not seem necessary as this all occurs in the same thread.

When I brought up the issue of the unbind/pop being done before the reactive methods consumed the data, the proposed solution was to capture the data in those methods before the reactive code, and pass it as an additional argument to the reactive code - which would require replicating every repository method with another one with an additional argument - which is what I proposed in the beginning - and which was rejected - and. I was told to use this mechanism instead.

...a/org/springframework/data/couchbase/repository/support/CrudMethodMetadataPostProcessor.java

...org/springframework/data/couchbase/repository/support/SimpleReactiveCouchbaseRepository.java

This adds scope and collection support to repositories via the DynamicInvocationHandler and PseudoArgs. It also adds annotations for scopes and collections. Closes #963

mikereiche · 2021-07-09T03:41:43Z

src/main/java/org/springframework/data/couchbase/core/ExecutableFindByIdOperationSupport.java

 			return new ExecutableFindByIdSupport<>(template, domainType, scope, collection, options, fields);
 		}

 		@Override
 		public FindByIdInScope<T> project(String... fields) {
-			Assert.notEmpty(fields, "Fields must not be null nor empty.");
+			Assert.notEmpty(fields, "Fields must not be null.");


Empty list means do not use the project option.

mikereiche · 2021-07-09T04:01:59Z

...main/java/org/springframework/data/couchbase/core/ExecutableFindByQueryOperationSupport.java

@@ -126,8 +127,9 @@ public T firstValue() {
 		@Override
 		public FindByQueryWithProjection<T> distinct(final String[] distinctFields) {
 			Assert.notNull(distinctFields, "distinctFields must not be null!");
+			String[] dFields = distinctFields.length == 1 && "-".equals(distinctFields[0]) ? null : distinctFields;
 			return new ExecutableFindByQuerySupport<>(template, domainType, returnType, query, scanConsistency, scope,


Coming from an annotation, this cannot be null.
But a non-null but empty distinctFields means distinct on all fields
So to indicate do not use distinct, we use {"-"} from the annotation, and here we change it to null.

mikereiche · 2021-07-21T12:30:11Z

...a/org/springframework/data/couchbase/repository/support/CrudMethodMetadataPostProcessor.java

@@ -161,10 +166,10 @@ public Object invoke(MethodInvocation invocation) throws Throwable {
 				try {
 					return invocation.proceed();
 				} finally {
-					TransactionSynchronizationManager.unbindResource(method);
+					// TransactionSynchronizationManager.unbindResource(method);


The original CrudMethodMetadataPostProcessor - copied from the JPA project - included code to bind/unbind the 'method' and push/pop the 'invocation'. The problem with unbinding/popping is that for reactive methods, that might take place before the information is consumed in the calling method. The unbinding/popping does not seem necessary as this all occurs in the same thread.

When I brought up the issue of the unbind/pop being done before the reactive methods consumed the data, the proposed solution was to capture the data in those methods before the reactive code, and pass it as an additional argument to the reactive code - which would require replicating every repository method with another one with an additional argument - which is what I proposed in the beginning - and which was rejected - and. I was told to use this mechanism instead.

...a/org/springframework/data/couchbase/repository/support/CrudMethodMetadataPostProcessor.java

* Add support for scopes and collections for repositories. Adds DynamicProxyable and DynamicInvocationHandler to set scope/collection/options on PseudoArgs when calling operations via repository interfaces. Closes #963. Co-authored-by: mikereiche <[email protected]>

This reverts commit 1581712.

mikereiche added 2 commits June 10, 2021 13:54

Add support for scopes and collections for repositories.

c53378e

Adds DynamicProxyable and DynamicInvocationHandler to set scope/collection/options on PseudoArgs when calling operations via repository interfaces. Closes #963.

Scopes and Collections for Repositories.

161d70c

This adds scope and collection support to repositories via the DynamicInvocationHandler and PseudoArgs. It also adds annotations for scopes and collections. Closes #963

mikereiche requested a review from daschl June 17, 2021 22:30

mikereiche force-pushed the datacouch_963_scopes_and_collections_for_repositories branch 2 times, most recently from a58d95c to cabef91 Compare June 21, 2021 23:48

mikereiche commented Jun 21, 2021

View reviewed changes

mikereiche force-pushed the datacouch_963_scopes_and_collections_for_repositories branch from cabef91 to 948811b Compare June 22, 2021 03:55

daschl suggested changes Jun 23, 2021

View reviewed changes

Merge branch 'main' into datacouch_963_scopes_and_collections_for_rep…

39233d9

…ositories

mikereiche force-pushed the datacouch_963_scopes_and_collections_for_repositories branch from 948811b to 39233d9 Compare June 23, 2021 19:36

mikereiche force-pushed the datacouch_963_scopes_and_collections_for_repositories branch 2 times, most recently from 9eb6e9a to 3aaeaa0 Compare July 9, 2021 05:27

daschl suggested changes Jul 15, 2021

View reviewed changes

mikereiche force-pushed the datacouch_963_scopes_and_collections_for_repositories branch from 3aaeaa0 to eb2555c Compare July 16, 2021 00:18

mikereiche mentioned this pull request Jul 21, 2021

need to implement Stream queries #1154

Closed

Scopes and Collections for Repositories.

4f6b678

This adds scope and collection support to repositories via the DynamicInvocationHandler and PseudoArgs. It also adds annotations for scopes and collections. Closes #963

mikereiche force-pushed the datacouch_963_scopes_and_collections_for_repositories branch from eb2555c to 4f6b678 Compare July 23, 2021 20:48

mikereiche commented Jul 23, 2021

View reviewed changes

daschl approved these changes Jul 27, 2021

View reviewed changes

mikereiche merged commit 7cde6b9 into main Aug 9, 2021

This was referenced Aug 10, 2021

Support configuring a default scan consistency. #1062

Closed

Support an @Expiry, @Expiration or @TTL annotation #1060

Closed

mikereiche added a commit that referenced this pull request Aug 11, 2021

Revert "Scopes and collections for repositories (#1149)"

0bdecef

This reverts commit 1581712.

mikereiche deleted the datacouch_963_scopes_and_collections_for_repositories branch August 11, 2021 22:07

Scopes and collections for repositories #1149

Scopes and collections for repositories #1149

Uh oh!

Conversation

mikereiche commented Jun 17, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mikereiche Jun 21, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

daschl Jun 23, 2021 • edited by mikereiche Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daschl Jul 15, 2021 • edited by mikereiche Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikereiche Jul 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daschl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

daschl Jul 15, 2021

Choose a reason for hiding this comment

Uh oh!

daschl Jul 15, 2021

Choose a reason for hiding this comment

Uh oh!

mikereiche Jul 21, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mikereiche Jul 9, 2021

Choose a reason for hiding this comment

Uh oh!

mikereiche Jul 9, 2021

Choose a reason for hiding this comment

Uh oh!

mikereiche Jul 21, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

daschl Jun 23, 2021 •

edited by mikereiche

Loading

daschl Jul 15, 2021 •

edited by mikereiche

Loading

mikereiche Jul 23, 2021 •

edited

Loading