Only project id and cas if required. (#1404)

Closes #1402.

Author: mikereiche

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

@@ -144,7 +144,7 @@ public <T> T decodeEntity(String id, String source, Long cas, Class<T> entityCla
 
 		persistentEntity = couldBePersistentEntity(readEntity.getClass());
 
-		if (cas != 0 && persistentEntity.getVersionProperty() != null) {
+		if (cas != null && cas != 0 && persistentEntity.getVersionProperty() != null) {
 			accessor.setProperty(persistentEntity.getVersionProperty(), cas);
 		}
 		N1qlJoinResolver.handleProperties(persistentEntity, accessor, template.reactive(), id, scope, collection);

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

@@ -16,6 +16,8 @@
 
 package org.springframework.data.couchbase.core;
 
+import com.couchbase.client.core.error.CouchbaseException;
+import org.springframework.data.couchbase.core.support.TemplateUtils;
 import reactor.core.publisher.Mono;
 
 import java.lang.reflect.InaccessibleObjectException;
@@ -88,8 +90,16 @@ public Mono<CouchbaseDocument> encodeEntity(final Object entityToEncode) {
 	public <T> Mono<T> decodeEntity(String id, String source, Long cas, Class<T> entityClass, String scope,
 			String collection) {
 		return Mono.fromSupplier(() -> {
-			final CouchbaseDocument converted = new CouchbaseDocument(id);
-			converted.setId(id);
+			// this is the entity class defined for the repository. It may not be the class of the document that was read
+			// we will reset it after reading the document
+			//
+			// This will fail for the case where:
+			// 1) The version is defined in the concrete class, but not in the abstract class; and
+			// 2) The constructor takes a "long version" argument resulting in an exception would be thrown if version in
+			// the source is null.
+			// We could expose from the MappingCouchbaseConverter determining the persistent entity from the source,
+			// but that is a lot of work to do every time just for this very rare and avoidable case.
+			// TypeInformation<? extends R> typeToUse = typeMapper.readType(source, type);
 
 			CouchbasePersistentEntity persistentEntity = couldBePersistentEntity(entityClass);
 
@@ -98,19 +108,43 @@ public <T> Mono<T> decodeEntity(String id, String source, Long cas, Class<T> ent
 				// to unwrap. This results in List<String[]> being unwrapped past String[] to String, so this may also be a
 				// Collection (or Array) of entityClass. We have no way of knowing - so just assume it is what we are told.
 				// if this is a Collection or array, only the first element will be returned.
+				final CouchbaseDocument converted = new CouchbaseDocument(id);
 				Set<Map.Entry<String, Object>> set = ((CouchbaseDocument) translationService.decode(source, converted))
 						.getContent().entrySet();
 				return (T) set.iterator().next().getValue();
 			}
 
-			if (cas != 0 && persistentEntity.getVersionProperty() != null) {
-				converted.put(persistentEntity.getVersionProperty().getName(), cas);
+			if (id == null) {
+				throw new CouchbaseException(TemplateUtils.SELECT_ID + " was null. Either use #{#n1ql.selectEntity} or project "
+						+ TemplateUtils.SELECT_ID);
+			}
+
+			final CouchbaseDocument converted = new CouchbaseDocument(id);
+
+			// if possible, set the version property in the source so that if the constructor has a long version argument,
+			// it will have a value and not fail (as null is not a valid argument for a long argument). This possible failure
+			// can be avoid by defining the argument as Long instead of long.
+			// persistentEntity is still the (possibly abstract) class specified in the repository definition
+			// it's possible that the abstract class does not have a version property, and this won't be able to set the version
+			if (persistentEntity.getVersionProperty() != null) {
+				if (cas == null) {
+					throw new CouchbaseException("version/cas in the entity but " + TemplateUtils.SELECT_CAS
+							+ " was not in result. Either use #{#n1ql.selectEntity} or project " + TemplateUtils.SELECT_CAS);
+				}
+				if (cas != 0) {
+					converted.put(persistentEntity.getVersionProperty().getName(), cas);
+				}
 			}
 
+			// if the constructor has an argument that is long version, then construction will fail if the 'version'
+			// is not available as 'null' is not a legal value for a long. Changing the arg to "Long version" would solve this.
+			// (Version doesn't come from 'source', it comes from the cas argument to decodeEntity)
 			T readEntity = converter.read(entityClass, (CouchbaseDocument) translationService.decode(source, converted));
 			final ConvertingPropertyAccessor<T> accessor = getPropertyAccessor(readEntity);
 
-			if (persistentEntity.getVersionProperty() != null) {
+			persistentEntity = couldBePersistentEntity(readEntity.getClass());
+
+			if (cas != null && cas != 0 && persistentEntity.getVersionProperty() != null) {
 				accessor.setProperty(persistentEntity.getVersionProperty(), cas);
 			}
 			N1qlJoinResolver.handleProperties(persistentEntity, accessor, template, id, scope, collection);

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

@@ -162,9 +162,7 @@ public N1qlSpelValues createN1qlSpelValues(String bucketName, String collection,
 		String b = collection != null ? collection : bucketName;
 		Assert.isTrue(!(distinctFields != null && fields != null),
 				"only one of project(fields) and distinct(distinctFields) can be specified");
-		String entity = "META(" + i(b) + ").id AS " + SELECT_ID + ", META(" + i(b) + ").cas AS " + SELECT_CAS + ", "
-				+ i(typeField);
-		String count = "COUNT(*) AS " + CountFragment.COUNT_ALIAS;
+		String entityFields = "";
 		String selectEntity;
 		if (distinctFields != null) {
 			String distinctFieldsStr = getProjectedOrDistinctFields(b, domainClass, typeField, fields, distinctFields);
@@ -175,47 +173,79 @@ public N1qlSpelValues createN1qlSpelValues(String bucketName, String collection,
 				selectEntity = "SELECT DISTINCT " + distinctFieldsStr + " FROM " + i(b);
 			}
 		} else if (isCount) {
-			selectEntity = "SELECT " + count + " FROM " + i(b);
+			selectEntity = "SELECT " + "COUNT(*) AS " + CountFragment.COUNT_ALIAS + " FROM " + i(b);
 		} else {
 			String projectedFields = getProjectedOrDistinctFields(b, domainClass, typeField, fields, distinctFields);
-			selectEntity = "SELECT " + entity + (!projectedFields.isEmpty() ? ", " : " ") + projectedFields + " FROM " + i(b);
+			entityFields = projectedFields;
+			selectEntity = "SELECT " + projectedFields + " FROM " + i(b);
 		}
 		String typeSelection = "`" + typeField + "` = \"" + typeValue + "\"";
 
 		String delete = N1QLExpression.delete().from(b).toString();
 		String returning = " returning " + N1qlUtils.createReturningExpressionForDelete(b).toString();
 
-		return new N1qlSpelValues(selectEntity, entity, i(b).toString(), typeSelection, delete, returning);
+		return new N1qlSpelValues(selectEntity, entityFields, i(b).toString(), typeSelection, delete, returning);
 	}
 
 	private String getProjectedOrDistinctFields(String b, Class resultClass, String typeField, String[] fields,
 			String[] distinctFields) {
 		if (distinctFields != null && distinctFields.length != 0) {
 			return i(distinctFields).toString();
 		}
-		String projectedFields = i(b) + ".*"; // if we can't get further information of the fields needed project everything
+		String projectedFields;
 		if (resultClass != null && !Modifier.isAbstract(resultClass.getModifiers())) {
 			PersistentEntity persistentEntity = couchbaseConverter.getMappingContext().getPersistentEntity(resultClass);
 			StringBuilder sb = new StringBuilder();
 			getProjectedFieldsInternal(b, null, sb, persistentEntity, typeField, fields, distinctFields != null);
 			projectedFields = sb.toString();
+		} else {
+			projectedFields = i(b) + ".*, " + "META(`" + b + "`).id AS " + SELECT_ID + ", META(`" + b + "`).cas AS "
+					+ SELECT_CAS; // if we can't get further information of the fields needed, then project everything
 		}
 		return projectedFields;
 	}
 
 	private void getProjectedFieldsInternal(String bucketName, CouchbasePersistentProperty parent, StringBuilder sb,
 			PersistentEntity persistentEntity, String typeField, String[] fields, boolean forDistinct) {
 
+		sb.append(i(typeField));
+
 		if (persistentEntity != null) {
 			Set<String> fieldList = fields != null ? new HashSet<>(Arrays.asList(fields)) : null;
 
 			// do not include the id and cas metadata fields.
 
 			persistentEntity.doWithProperties((PropertyHandler<CouchbasePersistentProperty>) prop -> {
 				if (prop == persistentEntity.getIdProperty() && parent == null) {
+					if (forDistinct) {
+						return;
+					}
+					if (sb.length() > 0) {
+						sb.append(", ");
+					}
+					PersistentPropertyPath<CouchbasePersistentProperty> path = couchbaseConverter.getMappingContext()
+							.getPersistentPropertyPath(prop.getName(), persistentEntity.getTypeInformation().getType());
+					String projectField = N1qlQueryCreator.addMetaIfRequired(bucketName, path, prop, persistentEntity).toString();
+					sb.append(projectField + " AS " + SELECT_ID);
+					if (fieldList != null) {
+						fieldList.remove(prop.getFieldName());
+					}
 					return;
 				}
 				if (prop == persistentEntity.getVersionProperty() && parent == null) {
+					if (forDistinct) {
+						return;
+					}
+					if (sb.length() > 0) {
+						sb.append(", ");
+					}
+					PersistentPropertyPath<CouchbasePersistentProperty> path = couchbaseConverter.getMappingContext()
+							.getPersistentPropertyPath(prop.getName(), persistentEntity.getTypeInformation().getType());
+					String projectField = N1qlQueryCreator.addMetaIfRequired(bucketName, path, prop, persistentEntity).toString();
+					sb.append(projectField + " AS " + SELECT_CAS);
+					if (fieldList != null) {
+						fieldList.remove(prop.getFieldName());
+					}
 					return;
 				}
 				if (prop.getFieldName().equals(typeField)) // typeField already projected

src/test/java/org/springframework/data/couchbase/repository/query/StringN1qlQueryCreatorMockedTests.java

@@ -89,7 +89,7 @@ void createsQueryCorrectly() throws Exception {
 
 		Query query = creator.createQuery();
 		assertEquals(
-				"SELECT META(`travel-sample`).id AS __id, META(`travel-sample`).cas AS __cas, `_class`, `createdBy`, `createdDate`, `lastModifiedBy`, `lastModifiedDate`, `firstname`, `lastname`, `subtype` FROM `travel-sample` where `_class` = \"abstractuser\" and firstname = $1 and lastname = $2",
+				"SELECT `_class`, META(`travel-sample`).`cas` AS __cas, `createdBy`, `createdDate`, `lastModifiedBy`, `lastModifiedDate`, META(`travel-sample`).`id` AS __id, `firstname`, `lastname`, `subtype` FROM `travel-sample` where `_class` = \"abstractuser\" and firstname = $1 and lastname = $2",
 				query.toN1qlSelectString(couchbaseTemplate.reactive(), User.class, false));
 	}
 
@@ -108,7 +108,7 @@ void createsQueryCorrectly2() throws Exception {
 
 		Query query = creator.createQuery();
 		assertEquals(
-				"SELECT META(`travel-sample`).id AS __id, META(`travel-sample`).cas AS __cas, `_class`, `createdBy`, `createdDate`, `lastModifiedBy`, `lastModifiedDate`, `firstname`, `lastname`, `subtype` FROM `travel-sample` where `_class` = \"abstractuser\" and (firstname = $first or lastname = $last)",
+				"SELECT `_class`, META(`travel-sample`).`cas` AS __cas, `createdBy`, `createdDate`, `lastModifiedBy`, `lastModifiedDate`, META(`travel-sample`).`id` AS __id, `firstname`, `lastname`, `subtype` FROM `travel-sample` where `_class` = \"abstractuser\" and (firstname = $first or lastname = $last)",
 				query.toN1qlSelectString(couchbaseTemplate.reactive(), User.class, false));
 	}