Scope and collection API for template. #1133

mikereiche · 2021-04-26T19:53:02Z

Scope and collection API for template.

Closes #963.
Original pull request: #1071.

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

…name. Queries on field annotated properties have original name. They should use getFieldName() which will use the annotated if it exists.

Scope and Collection API for template. Closes #963. Original pull request: #1071.

mikereiche · 2021-04-26T21:12:43Z

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

+			if (cmc.isAutoIndexCreation()) {
+				indexCreator = new CouchbasePersistentEntityIndexCreator(cmc, this);
+			}
+		}
 	}


just formatting changes

mikereiche · 2021-04-26T21:14:36Z

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

+ *
+ * @author Christoph Strobl
+ * @since 2.0
+ */


I'm just adding javadoc that was missing. As near as I can figure, the original author was Christoph

mikereiche · 2021-04-26T21:18:46Z

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

-		return new ExecutableFindByQuerySupport<T>(template, domainType, domainType, ALL_QUERY,
-				QueryScanConsistency.NOT_BOUNDED, null, null);
+		return new ExecutableFindByQuerySupport<T>(template, domainType, domainType, ALL_QUERY, null, null, null, null,
+				null);


replace the default QueryScanConsistency.NOT_BOUNDED with null.
If it is not null, it will override the scanConsistency on the QueryOptions, if provided.

mikereiche · 2021-04-26T21:32:15Z

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

+							.getCollection(pArgs.getCollection()).reactive()
+							.replace(converted.getId(), converted.export(), buildReplaceOptions(pArgs.getOptions(), object, converted))
+							.flatMap(result -> support.applyUpdatedId(object, converted.getId())
+									.flatMap(replacedObject -> (Mono<T>) support.applyUpdatedCas(replacedObject, result.cas())))


The cast to Mono is necessary here because support (ReactiveTemplateSupport) is not parameterized with the entity type, the method just returns an Object. One day I will fix that.

mikereiche · 2021-04-26T21:44:20Z

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

+						.append('`').toString();
+			}
+		}
+	}


I don't know why this appears after rebasing. It should have already been in master (?????)

mikereiche · 2021-04-26T22:01:45Z

...ingframework/data/couchbase/repository/ReactiveCouchbaseRepositoryQueryIntegrationTests.java

@@ -72,7 +76,7 @@ void shouldSaveAndFindAll() {
 		Airport vie = null;
 		Airport jfk = null;
 		try {
-			vie = new Airport("airports::vie", "vie", "loww");
+			vie = new Airport("airports::vie", "vie", "low1");


After running tests, I found some "vie", "loww" Airport records. So I made them unique to figure out what test was creating them and not deleting them.

mikereiche · 2021-04-26T22:03:09Z

src/test/java/org/springframework/data/couchbase/util/UnmanagedTestCluster.java

@@ -59,7 +59,7 @@ ClusterType type() {

 	@Override
 	TestClusterConfig _start() throws Exception {
-		bucketname = UUID.randomUUID().toString();
+		bucketname = "my_bucket"; // UUID.randomUUID().toString();


Reuse "my_bucket" if it already exists. Creating the indexes, scopes and collections takes time.

if you do that, you also remove the natural isolation of each test, since indexes and/or data might be left behind or still on there. If you take that into account it is fine, just wanted to mention it.

There are some tests that don't delete everything they've inserted. So I will remove this change from the changeset and leave it for later when I have had time to verify that all the tests clean up after themselves.

Closes #1131. Original pull request: #1135. Co-authored-by: mikereiche <[email protected]>

Closes #1130. Co-authored-by: mikereiche <[email protected]>

See #1104.

See #1126

See #1127

daschl

This change looks veeery solid already, most of my comments are nits and small changes -- should be able to merge afterwards

daschl · 2021-05-17T08:31:35Z

pom.xml

@@ -173,6 +173,7 @@
        <dependency>
            <groupId>io.projectreactor</groupId>
            <artifactId>reactor-test</artifactId>
+            <version>3.1.0.RELEASE</version>


latest one is 3.4.6, might cause issues with testing when using such an old version?

Updated to 3.4.6. Seems ok.

changed one instance and removed the other (duplicate).

daschl · 2021-05-17T08:32:28Z

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

+	/**
+	 * {@inheritDoc}
+	 */
+	public void setThreadLocalArgs(PseudoArgs pseudoArgs) {


since this is something internal, can it 1) be package private or 2) if not, add a comment that it is internal?

Changed to package private.

Unused. Removed.

daschl · 2021-05-17T09:05:23Z

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

 import com.couchbase.client.java.kv.ReplicateTo;

+/**
+ * Insert Operations


copy paste error ?

fixed. For real, this time.
I'm not sure what happened to the things that are marked "fixed". Either I just marked them "fixed" without actually fixing them, or a pull/rebase un-fixed them.

daschl · 2021-05-17T09:08:56Z

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

+	/**
+	 * @param pseudoArgs - the metaArgs to set on the ThreadLocal field of the CouchbaseOperations
+	 */
+	void setThreadLocalArgs(PseudoArgs<?> pseudoArgs);


I wonder if this should also be named getPseudoArgs and setPseudoArgs? the thread local would be an implementation detail

daschl · 2021-05-17T09:09:04Z

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

+	void setThreadLocalArgs(PseudoArgs<?> pseudoArgs);
+
+	/**
+	 * @@return the metaArgs from the ThreadLocal field of the CouchbaseOperations


daschl · 2021-05-17T09:40:14Z

src/main/java/org/springframework/data/couchbase/core/support/InScope.java

 *
 * @author Michael Reiche
 * @param <T> - the entity class
 */
-public interface WithCollection<T> {
-	Object inCollection(String collectionName);
+public interface InScope<T> {


isn't T unused?

Yes, it's not used. Ideally, the methods in all these With* interfaces would return a parameterized interface as in the Override and implementation. However, there is no common interface for all the different operations. So I just define it to return Object, and T is not used. I would like to leave the parameter there, in hopes that I eventually figure out how to define it properly.

interface FindByQueryInScope<T> extends FindByQueryInCollection<T>, InScope<T> { /** * With a different scope * * @param scope the scope to use. */ @Override FindByQueryInCollection<T> inScope(String scope); }

daschl · 2021-05-17T09:40:54Z

src/main/java/org/springframework/data/couchbase/core/support/WithAnalyticsConsistency.java

@@ -18,11 +18,16 @@
 import com.couchbase.client.java.analytics.AnalyticsScanConsistency;

 /**
- * A common interface for all of Insert, Replace, Upsert that take consistency
+ * Interface for operations that take AnalyticsScanConsistency
 *
 * @author Michael Reiche
 * @param <T> - the entity class
 */
 public interface WithAnalyticsConsistency<T> {


Yes, it's not used. Ideally, the methods in all these With* interfaces would return a parameterized interface as in the Override and implementation. However, there is no common interface for all the different operations. So I just define it to return Object, and T is not used. I would like to leave the parameter there, in hopes that I eventually figure out how to define it properly.

daschl · 2021-05-17T09:41:07Z

src/main/java/org/springframework/data/couchbase/core/support/WithAnalyticsOptions.java

+ * @author Michael Reiche
+ * @param <T> - the entity class
+ */
+public interface WithAnalyticsOptions<T> {


Yes it's unused. But inCollection() should return an operation, and I was hoping to figure out how to achieve that eventually.

Object inCollection(String collection);

daschl · 2021-05-17T09:41:33Z

src/main/java/org/springframework/data/couchbase/core/support/WithAnalyticsQuery.java

@@ -18,11 +18,16 @@
 import org.springframework.data.couchbase.core.query.AnalyticsQuery;

 /**
- * A common interface for all of Insert, Replace, Upsert that take Query
+ * Interface for operations that take AnalyticsQuery
 *
 * @author Michael Reiche
 * @param <T> - the entity class
 */
 public interface WithAnalyticsQuery<T> {


I'll stop with the T unused comment, please look through the others if valid too.

Yes, it's not used. Ideally, the methods in all these With* interfaces would return a parameterized interface as in the Override and implementation. However, there is no common interface for all the different operations. So I just define it to return Object, and T is not used. I would like to leave the parameter there, in hopes that I eventually figure out how to define it properly.

daschl · 2021-05-17T09:43:18Z

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

@@ -86,7 +87,8 @@ protected Object doExecute(CouchbaseQueryMethod method, ResultProcessor processo
 		ExecutableFindByQuery<?> find = typeToRead == null ? findOperationWithProjection //
 				: findOperationWithProjection; // not yet implemented in core .as(typeToRead);

-		String collection = "_default._default";// method.getEntityInformation().getCollectionName(); // not yet implemented
+		// TODO (maybe) // method.getEntityInformation().getCollectionName(); // not yet implemented


should a ticket be filed for this?

This is irrelevant now. This _default._default is ignored.

…name. Queries on field annotated properties have original name. They should use getFieldName() which will use the annotated if it exists.

Scope and Collection API for template. Closes #963. Original pull request: #1071.

…com:spring-projects/spring-data-couchbase into datacouch_963_scope_and_collection_api_main

mikereiche · 2021-05-31T15:32:04Z

pom.xml

@@ -173,6 +173,7 @@
        <dependency>
            <groupId>io.projectreactor</groupId>
            <artifactId>reactor-test</artifactId>
+            <version>3.1.0.RELEASE</version>


changed one instance and removed the other (duplicate).

mikereiche · 2021-05-31T15:33:29Z

pom.xml

+                    <source>8</source>
+                    <target>8</target>
+                </configuration>
+            </plugin>


removed. Shouldn't need this.

mikereiche · 2021-05-31T15:36:44Z

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

+	/**
+	 * {@inheritDoc}
+	 */
+	public void setThreadLocalArgs(PseudoArgs pseudoArgs) {


Unused. Removed.

mikereiche · 2021-05-31T15:40:32Z

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

-			return new ExecutableFindByQuerySupport<>(template, domainType, returnType, query, scanConsistency, collection,
-					distinctFields);
+			return new ExecutableFindByQuerySupport<>(template, domainType, returnType, query, scanConsistency, scope,
+					collection, options, distinctFields);
 		}

 		@Override


InCollection (below) got moved to the end. It is not missing.

mikereiche · 2021-05-31T15:45:49Z

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

 import com.couchbase.client.java.kv.ReplicateTo;

+/**
+ * Insert Operations


fixed. For real, this time.
I'm not sure what happened to the things that are marked "fixed". Either I just marked them "fixed" without actually fixing them, or a pull/rebase un-fixed them.

mikereiche · 2021-05-31T16:15:25Z

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


+/**
+ * Insert Operations


mikereiche · 2021-05-31T16:26:48Z

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

@@ -31,4 +31,6 @@
 	 * The field name can be different from the actual property name by using a custom annotation.
 	 */
 	String getFieldName();
+
+	Boolean isExpirationProperty();


Changed to unboxed boolean.
But leaving as "is..." to be consistent with isIdProperty() and isVersionProperty() from PersistentProperty.

boolean isIdProperty();
boolean isVersionProperty();

mikereiche · 2021-05-31T16:28:49Z

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

+@Documented
+@Retention(RetentionPolicy.RUNTIME)
+@Target({ ElementType.FIELD, ElementType.ANNOTATION_TYPE })
+public @interface Expiration {


It represents META().expiration so I named it Expiration. Did you still want it renamed? I could go either way.

mikereiche · 2021-05-31T16:30:44Z

src/main/java/org/springframework/data/couchbase/core/support/InCollection.java

+ * @author Michael Reiche
+ * @param <T> - the entity class
+ */
+public interface InCollection<T> {


Yes it's unused. But inCollection() should return an operation, and I was hoping to figure out how to achieve that eventually.

Object inCollection(String collection);

mikereiche · 2021-05-31T16:31:56Z

src/main/java/org/springframework/data/couchbase/core/support/WithAnalyticsOptions.java

+ * @author Michael Reiche
+ * @param <T> - the entity class
+ */
+public interface WithAnalyticsOptions<T> {


Yes it's unused. But inCollection() should return an operation, and I was hoping to figure out how to achieve that eventually.

Object inCollection(String collection);

…com:spring-projects/spring-data-couchbase into datacouch_963_scope_and_collection_api_main

Scope and Collection API for template. Closes #963. Original pull request: #1071.

DATACOUCH-1055 - Queries on field annotated properties have original …

c008466

…name. Queries on field annotated properties have original name. They should use getFieldName() which will use the annotated if it exists.

mikereiche force-pushed the datacouch_963_scope_and_collection_api_main branch from a11a3ff to bb8e690 Compare April 26, 2021 21:38

Scope and Collection API for template.

46072dc

Scope and Collection API for template. Closes #963. Original pull request: #1071.

mikereiche force-pushed the datacouch_963_scope_and_collection_api_main branch from bb8e690 to 46072dc Compare April 26, 2021 21:56

mikereiche commented Apr 26, 2021

View reviewed changes

mikereiche and others added 2 commits May 4, 2021 07:25

Add override of toN1qlRemoveString() to StringQuery. (#1135)

36de313

Closes #1131. Original pull request: #1135. Co-authored-by: mikereiche <[email protected]>

Expose ObjectMapper used by JsonSerializer (#1132)

a806631

Closes #1130. Co-authored-by: mikereiche <[email protected]>

daschl self-requested a review May 6, 2021 14:07

gregturn and others added 3 commits May 13, 2021 15:41

Update CI to JDK 16.

c814a10

See #1104.

Updated changelog.

3cce0b3

See #1126

Updated changelog.

5cf142d

See #1127

daschl suggested changes May 17, 2021

View reviewed changes

mikereiche added 5 commits May 18, 2021 17:45

DATACOUCH-1055 - Queries on field annotated properties have original …

da5eebe

…name. Queries on field annotated properties have original name. They should use getFieldName() which will use the annotated if it exists.

Scope and Collection API for template.

0d1d779

Scope and Collection API for template. Closes #963. Original pull request: #1071.

Merge branch 'datacouch_963_scope_and_collection_api_main' of github.…

2349784

…com:spring-projects/spring-data-couchbase into datacouch_963_scope_and_collection_api_main

Merge branch 'datacouch_963_scope_and_collection_api_main' of github.…

5b1402d

…com:spring-projects/spring-data-couchbase into datacouch_963_scope_and_collection_api_main

Merge branch 'datacouch_963_scope_and_collection_api_main' of github.…

b4b9b10

…com:spring-projects/spring-data-couchbase into datacouch_963_scope_and_collection_api_main

mikereiche commented May 31, 2021

View reviewed changes

mikereiche added 4 commits May 31, 2021 13:45

Merge branch 'datacouch_963_scope_and_collection_api_main' of github.…

7d85b55

…com:spring-projects/spring-data-couchbase into datacouch_963_scope_and_collection_api_main

Merge branch 'datacouch_963_scope_and_collection_api_main' of github.…

56b984b

…com:spring-projects/spring-data-couchbase into datacouch_963_scope_and_collection_api_main

Merge branch 'datacouch_963_scope_and_collection_api_main' of github.…

3db9e30

…com:spring-projects/spring-data-couchbase into datacouch_963_scope_and_collection_api_main

Merge branch 'datacouch_963_scope_and_collection_api_main' of github.…

ef59886

…com:spring-projects/spring-data-couchbase into datacouch_963_scope_and_collection_api_main

daschl approved these changes Jun 1, 2021

View reviewed changes

mikereiche merged commit 43889e6 into main Jun 2, 2021

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

Scope and collection API for template. (#1133)

42b7fb3

Scope and Collection API for template. Closes #963. Original pull request: #1071.

mikereiche deleted the datacouch_963_scope_and_collection_api_main branch August 11, 2021 22:11

Scope and collection API for template. #1133

Scope and collection API for template. #1133

Conversation

mikereiche commented Apr 26, 2021

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

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

mikereiche May 17, 2021 • edited Loading

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

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

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

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

mikereiche May 17, 2021 •

edited

Loading