From 51282d66fcb9f3a3c1751b572e747ea8fcc37e10 Mon Sep 17 00:00:00 2001 From: mikereiche Date: Thu, 21 Oct 2021 14:28:12 -0700 Subject: [PATCH] Field named id treated as document id. Closes #1258. --- .../convert/MappingCouchbaseConverter.java | 28 +++++++---- .../repository/query/N1qlQueryCreator.java | 13 +++-- .../query/StringBasedN1qlQueryParser.java | 10 ++-- ...ouchbaseTemplateQueryIntegrationTests.java | 16 ++++++ .../data/couchbase/domain/AssessmentDO.java | 43 ++++++++++++++++ .../query/N1qlQueryCreatorTests.java | 50 ++++++++++++------- 6 files changed, 122 insertions(+), 38 deletions(-) create mode 100644 src/test/java/org/springframework/data/couchbase/domain/AssessmentDO.java diff --git a/src/main/java/org/springframework/data/couchbase/core/convert/MappingCouchbaseConverter.java b/src/main/java/org/springframework/data/couchbase/core/convert/MappingCouchbaseConverter.java index 05b6b4486..421560203 100644 --- a/src/main/java/org/springframework/data/couchbase/core/convert/MappingCouchbaseConverter.java +++ b/src/main/java/org/springframework/data/couchbase/core/convert/MappingCouchbaseConverter.java @@ -52,6 +52,7 @@ import org.springframework.data.mapping.Association; import org.springframework.data.mapping.AssociationHandler; import org.springframework.data.mapping.MappingException; +import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentPropertyAccessor; import org.springframework.data.mapping.PreferredConstructor.Parameter; import org.springframework.data.mapping.PropertyHandler; @@ -271,7 +272,8 @@ public void doWithPersistentProperty(final CouchbasePersistentProperty prop) { || prop.isAnnotationPresent(N1qlJoin.class)) { return; } - Object obj = prop.isIdProperty() && parent == null ? source.getId() : getValueInternal(prop, source, instance); + Object obj = prop == entity.getIdProperty() && parent == null ? source.getId() + : getValueInternal(prop, source, instance, entity); accessor.setProperty(prop, obj); } @@ -286,7 +288,7 @@ private boolean isIdConstructionProperty(final CouchbasePersistentProperty prope entity.doWithAssociations((AssociationHandler) association -> { CouchbasePersistentProperty inverseProp = association.getInverse(); - Object obj = getValueInternal(inverseProp, source, instance); + Object obj = getValueInternal(inverseProp, source, instance, entity); accessor.setProperty(inverseProp, obj); }); @@ -302,8 +304,8 @@ private boolean isIdConstructionProperty(final CouchbasePersistentProperty prope * @return the actual property value. */ protected Object getValueInternal(final CouchbasePersistentProperty property, final CouchbaseDocument source, - final Object parent) { - return new CouchbasePropertyValueProvider(source, spELContext, parent).getPropertyValue(property); + final Object parent, PersistentEntity entity) { + return new CouchbasePropertyValueProvider(source, spELContext, parent, entity).getPropertyValue(property); } /** @@ -318,7 +320,7 @@ protected Object getValueInternal(final CouchbasePersistentProperty property, fi private ParameterValueProvider getParameterProvider( final CouchbasePersistentEntity entity, final CouchbaseDocument source, final DefaultSpELExpressionEvaluator evaluator, final Object parent) { - CouchbasePropertyValueProvider provider = new CouchbasePropertyValueProvider(source, evaluator, parent); + CouchbasePropertyValueProvider provider = new CouchbasePropertyValueProvider(source, evaluator, parent, entity); PersistentEntityParameterValueProvider parameterProvider = new PersistentEntityParameterValueProvider<>( entity, provider, parent); @@ -503,7 +505,7 @@ protected void writeInternal(final Object source, final CouchbaseDocument target final TreeMap suffixes = new TreeMap<>(); final TreeMap idAttributes = new TreeMap<>(); - target.setExpiration((int)(entity.getExpiryDuration().getSeconds())); + target.setExpiration((int) (entity.getExpiryDuration().getSeconds())); entity.doWithProperties(new PropertyHandler() { @Override @@ -926,19 +928,25 @@ private class CouchbasePropertyValueProvider implements PropertyValueProvider R getPropertyValue(final CouchbasePersistentProperty property) { String expression = property.getSpelExpression(); Object value = expression != null ? evaluator.evaluate(expression) : source.get(property.getFieldName()); - if (property.isIdProperty() && parent == null) { + if (property == entity.getIdProperty() && parent == null) { return (R) source.getId(); } if (value == null) { diff --git a/src/main/java/org/springframework/data/couchbase/repository/query/N1qlQueryCreator.java b/src/main/java/org/springframework/data/couchbase/repository/query/N1qlQueryCreator.java index 37cacc2f9..e4b3279e1 100644 --- a/src/main/java/org/springframework/data/couchbase/repository/query/N1qlQueryCreator.java +++ b/src/main/java/org/springframework/data/couchbase/repository/query/N1qlQueryCreator.java @@ -32,6 +32,7 @@ import org.springframework.data.couchbase.core.query.QueryCriteria; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; +import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.repository.query.ParameterAccessor; @@ -57,6 +58,7 @@ public class N1qlQueryCreator extends AbstractQueryCreator private final QueryMethod queryMethod; private final CouchbaseConverter converter; private final String bucketName; + private final PersistentEntity entity; public N1qlQueryCreator(final PartTree tree, final ParameterAccessor accessor, final QueryMethod queryMethod, final CouchbaseConverter converter, final String bucketName) { @@ -67,13 +69,14 @@ public N1qlQueryCreator(final PartTree tree, final ParameterAccessor accessor, f this.queryMethod = queryMethod; this.converter = converter; this.bucketName = bucketName; + this.entity = converter.getMappingContext().getPersistentEntity(queryMethod.getReturnedObjectType()); } @Override protected QueryCriteria create(final Part part, final Iterator iterator) { PersistentPropertyPath path = context.getPersistentPropertyPath(part.getProperty()); CouchbasePersistentProperty property = path.getLeafProperty(); - return from(part, property, where(addMetaIfRequired(bucketName, path, property)), iterator); + return from(part, property, where(addMetaIfRequired(bucketName, path, property, entity)), iterator); } @Override @@ -107,7 +110,7 @@ protected QueryCriteria and(final Part part, final QueryCriteria base, final Ite PersistentPropertyPath path = context.getPersistentPropertyPath(part.getProperty()); CouchbasePersistentProperty property = path.getLeafProperty(); - return from(part, property, base.and(addMetaIfRequired(bucketName, path, property)), iterator); + return from(part, property, base.and(addMetaIfRequired(bucketName, path, property, entity)), iterator); } @Override @@ -186,11 +189,11 @@ private QueryCriteria from(final Part part, final CouchbasePersistentProperty pr public static N1QLExpression addMetaIfRequired(String bucketName, final PersistentPropertyPath persistentPropertyPath, - final CouchbasePersistentProperty property) { - if (property.isIdProperty()) { + final CouchbasePersistentProperty property, final PersistentEntity entity) { + if (entity != null && property == entity.getIdProperty()) { return path(meta(i(bucketName)), i(META_ID_PROPERTY)); } - if (property.isVersionProperty()) { + if (property == entity.getVersionProperty()) { return path(meta(i(bucketName)), i(META_CAS_PROPERTY)); } if (property.isExpirationProperty()) { diff --git a/src/main/java/org/springframework/data/couchbase/repository/query/StringBasedN1qlQueryParser.java b/src/main/java/org/springframework/data/couchbase/repository/query/StringBasedN1qlQueryParser.java index 0013bf47f..3aa200729 100644 --- a/src/main/java/org/springframework/data/couchbase/repository/query/StringBasedN1qlQueryParser.java +++ b/src/main/java/org/springframework/data/couchbase/repository/query/StringBasedN1qlQueryParser.java @@ -29,7 +29,6 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; -import com.couchbase.client.core.error.CouchbaseException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.data.couchbase.core.convert.CouchbaseConverter; @@ -51,6 +50,7 @@ import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.util.Assert; +import com.couchbase.client.core.error.CouchbaseException; import com.couchbase.client.core.error.InvalidArgumentException; import com.couchbase.client.java.json.JsonArray; import com.couchbase.client.java.json.JsonObject; @@ -213,10 +213,10 @@ private void getProjectedFieldsInternal(String bucketName, CouchbasePersistentPr persistentEntity.doWithProperties(new PropertyHandler() { @Override public void doWithPersistentProperty(final CouchbasePersistentProperty prop) { - if (prop.isIdProperty() && parent == null) { + if (prop == persistentEntity.getIdProperty() && parent == null) { return; } - if (prop.isVersionProperty()) { + if (prop == persistentEntity.getVersionProperty() && parent == null) { return; } String projectField = null; @@ -224,7 +224,7 @@ public void doWithPersistentProperty(final CouchbasePersistentProperty prop) { if (fieldList == null || fieldList.contains(prop.getFieldName())) { PersistentPropertyPath path = couchbaseConverter.getMappingContext() .getPersistentPropertyPath(prop.getName(), resultClass.getType()); - projectField = N1qlQueryCreator.addMetaIfRequired(bucketName, path, prop).toString(); + projectField = N1qlQueryCreator.addMetaIfRequired(bucketName, path, prop, persistentEntity).toString(); if (sb.length() > 0) { sb.append(", "); } @@ -250,7 +250,7 @@ public void doWithPersistentProperty(final CouchbasePersistentProperty prop) { // needs further discussion as removing a field from an entity could cause this and not necessarily be an error if (fieldList != null && !fieldList.isEmpty()) { throw new CouchbaseException( - "projected fields (" + fieldList + ") not found in entity: " + persistentEntity.getName()); + "projected fields (" + fieldList + ") not found in entity: " + persistentEntity.getName()); } } else { for (String field : fields) { 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 db0b797c5..f7987c6a0 100644 --- a/src/test/java/org/springframework/data/couchbase/core/CouchbaseTemplateQueryIntegrationTests.java +++ b/src/test/java/org/springframework/data/couchbase/core/CouchbaseTemplateQueryIntegrationTests.java @@ -35,6 +35,7 @@ import org.springframework.data.couchbase.core.query.QueryCriteria; import org.springframework.data.couchbase.domain.Address; import org.springframework.data.couchbase.domain.Airport; +import org.springframework.data.couchbase.domain.AssessmentDO; import org.springframework.data.couchbase.domain.Course; import org.springframework.data.couchbase.domain.NaiveAuditorAware; import org.springframework.data.couchbase.domain.Submission; @@ -132,6 +133,21 @@ void findByMatchingQuery() { assertEquals(1, foundUsers.size()); } + @Test + void findAssessmentDO() { + AssessmentDO ado = new AssessmentDO(); + ado.setEventTimestamp(44444444);// this is also an @IdAttribute + ado.setId("123"); + ado = couchbaseTemplate.upsertById(AssessmentDO.class).one(ado); + + Query specialUsers = new Query(QueryCriteria.where(i("id")).is(ado.getId())); + final List foundUsers = couchbaseTemplate.findByQuery(AssessmentDO.class) + .withConsistency(QueryScanConsistency.REQUEST_PLUS).matching(specialUsers).all(); + assertEquals("123", foundUsers.get(0).getId(), "id"); + assertEquals("44444444", foundUsers.get(0).getDocumentId(), "documentId"); + assertEquals(ado, foundUsers.get(0)); + } + @Test void findByMatchingQueryProjected() { diff --git a/src/test/java/org/springframework/data/couchbase/domain/AssessmentDO.java b/src/test/java/org/springframework/data/couchbase/domain/AssessmentDO.java new file mode 100644 index 000000000..90a8282df --- /dev/null +++ b/src/test/java/org/springframework/data/couchbase/domain/AssessmentDO.java @@ -0,0 +1,43 @@ +/* + * Copyright 2012-2021 the original author or authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.data.couchbase.domain; + +import lombok.Data; +import lombok.NoArgsConstructor; + +import org.springframework.data.annotation.Id; +import org.springframework.data.couchbase.core.mapping.Document; +import org.springframework.data.couchbase.core.mapping.Field; +import org.springframework.data.couchbase.core.mapping.id.GeneratedValue; +import org.springframework.data.couchbase.core.mapping.id.GenerationStrategy; +import org.springframework.data.couchbase.core.mapping.id.IdAttribute; + +/** + * @author Michael Reiche + */ +@Document() +@Data +@NoArgsConstructor +public class AssessmentDO { + @Id @GeneratedValue(strategy = GenerationStrategy.USE_ATTRIBUTES) private String documentId; + + @Field @IdAttribute private long eventTimestamp; + + @Field("docType") private String documentType; + + @Field private String id; +} diff --git a/src/test/java/org/springframework/data/couchbase/repository/query/N1qlQueryCreatorTests.java b/src/test/java/org/springframework/data/couchbase/repository/query/N1qlQueryCreatorTests.java index a7f12ccd0..52fe975bf 100644 --- a/src/test/java/org/springframework/data/couchbase/repository/query/N1qlQueryCreatorTests.java +++ b/src/test/java/org/springframework/data/couchbase/repository/query/N1qlQueryCreatorTests.java @@ -37,10 +37,13 @@ import org.springframework.data.couchbase.domain.User; import org.springframework.data.couchbase.domain.UserRepository; import org.springframework.data.mapping.context.MappingContext; +import org.springframework.data.projection.SpelAwareProxyProjectionFactory; +import org.springframework.data.repository.core.support.DefaultRepositoryMetadata; import org.springframework.data.repository.query.DefaultParameters; import org.springframework.data.repository.query.ParameterAccessor; import org.springframework.data.repository.query.Parameters; import org.springframework.data.repository.query.ParametersParameterAccessor; +import org.springframework.data.repository.query.QueryMethod; import org.springframework.data.repository.query.parser.PartTree; import com.couchbase.client.java.json.JsonArray; @@ -69,9 +72,10 @@ void createsQueryCorrectly() throws Exception { String input = "findByFirstname"; PartTree tree = new PartTree(input, User.class); Method method = UserRepository.class.getMethod(input, String.class); - - N1qlQueryCreator creator = new N1qlQueryCreator(tree, getAccessor(getParameters(method), "Oliver"), null, converter, - bucketName); + QueryMethod queryMethod = new QueryMethod(method, new DefaultRepositoryMetadata(UserRepository.class), + new SpelAwareProxyProjectionFactory()); + N1qlQueryCreator creator = new N1qlQueryCreator(tree, getAccessor(getParameters(method), "Oliver"), queryMethod, + converter, bucketName); Query query = creator.createQuery(); assertEquals(query.export(), " WHERE " + where(i("firstname")).is("Oliver").export()); @@ -82,9 +86,10 @@ void createsQueryFieldAnnotationCorrectly() throws Exception { String input = "findByMiddlename"; PartTree tree = new PartTree(input, Person.class); Method method = PersonRepository.class.getMethod(input, String.class); - - N1qlQueryCreator creator = new N1qlQueryCreator(tree, getAccessor(getParameters(method), "Oliver"), null, converter, - bucketName); + QueryMethod queryMethod = new QueryMethod(method, new DefaultRepositoryMetadata(UserRepository.class), + new SpelAwareProxyProjectionFactory()); + N1qlQueryCreator creator = new N1qlQueryCreator(tree, getAccessor(getParameters(method), "Oliver"), queryMethod, + converter, bucketName); Query query = creator.createQuery(); assertEquals(query.export(), " WHERE " + where(i("nickname")).is("Oliver").export()); @@ -95,10 +100,12 @@ void queryParametersArray() throws Exception { String input = "findByFirstnameIn"; PartTree tree = new PartTree(input, User.class); Method method = UserRepository.class.getMethod(input, String[].class); + QueryMethod queryMethod = new QueryMethod(method, new DefaultRepositoryMetadata(UserRepository.class), + new SpelAwareProxyProjectionFactory()); Query expected = (new Query()).addCriteria(where(i("firstname")).in("Oliver", "Charles")); N1qlQueryCreator creator = new N1qlQueryCreator(tree, - getAccessor(getParameters(method), new Object[] { new Object[] { "Oliver", "Charles" } }), null, converter, - bucketName); + getAccessor(getParameters(method), new Object[] { new Object[] { "Oliver", "Charles" } }), queryMethod, + converter, bucketName); Query query = creator.createQuery(); // Query expected = (new Query()).addCriteria(where("firstname").in("Oliver", "Charles")); @@ -115,11 +122,12 @@ void queryParametersJsonArray() throws Exception { String input = "findByFirstnameIn"; PartTree tree = new PartTree(input, User.class); Method method = UserRepository.class.getMethod(input, JsonArray.class); - + QueryMethod queryMethod = new QueryMethod(method, new DefaultRepositoryMetadata(UserRepository.class), + new SpelAwareProxyProjectionFactory()); JsonArray jsonArray = JsonArray.create(); jsonArray.add("Oliver"); jsonArray.add("Charles"); - N1qlQueryCreator creator = new N1qlQueryCreator(tree, getAccessor(getParameters(method), jsonArray), null, + N1qlQueryCreator creator = new N1qlQueryCreator(tree, getAccessor(getParameters(method), jsonArray), queryMethod, converter, bucketName); Query query = creator.createQuery(); @@ -137,11 +145,13 @@ void queryParametersList() throws Exception { String input = "findByFirstnameIn"; PartTree tree = new PartTree(input, User.class); Method method = UserRepository.class.getMethod(input, String[].class); + QueryMethod queryMethod = new QueryMethod(method, new DefaultRepositoryMetadata(UserRepository.class), + new SpelAwareProxyProjectionFactory()); List list = new LinkedList<>(); list.add("Oliver"); list.add("Charles"); N1qlQueryCreator creator = new N1qlQueryCreator(tree, getAccessor(getParameters(method), new Object[] { list }), - null, converter, bucketName); + queryMethod, converter, bucketName); Query query = creator.createQuery(); Query expected = (new Query()).addCriteria(where(i("firstname")).in("Oliver", "Charles")); @@ -159,8 +169,10 @@ void createsAndQueryCorrectly() throws Exception { String input = "findByFirstnameAndLastname"; PartTree tree = new PartTree(input, User.class); Method method = UserRepository.class.getMethod(input, String.class, String.class); - N1qlQueryCreator creator = new N1qlQueryCreator(tree, getAccessor(getParameters(method), "John", "Doe"), null, - converter, bucketName); + QueryMethod queryMethod = new QueryMethod(method, new DefaultRepositoryMetadata(UserRepository.class), + new SpelAwareProxyProjectionFactory()); + N1qlQueryCreator creator = new N1qlQueryCreator(tree, getAccessor(getParameters(method), "John", "Doe"), + queryMethod, converter, bucketName); Query query = creator.createQuery(); assertEquals(" WHERE " + where(i("firstname")).is("John").and(i("lastname")).is("Doe").export(), query.export()); @@ -171,9 +183,10 @@ void createsQueryFindByIdIsNotNullAndFirstname() throws Exception { String input = "findByIdIsNotNullAndFirstnameEquals"; PartTree tree = new PartTree(input, User.class); Method method = UserRepository.class.getMethod(input, String.class); - - N1qlQueryCreator creator = new N1qlQueryCreator(tree, getAccessor(getParameters(method), "Oliver"), null, converter, - bucketName); + QueryMethod queryMethod = new QueryMethod(method, new DefaultRepositoryMetadata(UserRepository.class), + new SpelAwareProxyProjectionFactory()); + N1qlQueryCreator creator = new N1qlQueryCreator(tree, getAccessor(getParameters(method), "Oliver"), queryMethod, + converter, bucketName); Query query = creator.createQuery(); assertEquals(query.export(), @@ -185,9 +198,10 @@ void createsQueryFindByVersionEqualsAndAndFirstname() throws Exception { String input = "findByVersionEqualsAndFirstnameEquals"; PartTree tree = new PartTree(input, User.class); Method method = UserRepository.class.getMethod(input, Long.class, String.class); - + QueryMethod queryMethod = new QueryMethod(method, new DefaultRepositoryMetadata(UserRepository.class), + new SpelAwareProxyProjectionFactory()); N1qlQueryCreator creator = new N1qlQueryCreator(tree, - getAccessor(getParameters(method), 1611287177404088320L, "Oliver"), null, converter, bucketName); + getAccessor(getParameters(method), 1611287177404088320L, "Oliver"), queryMethod, converter, bucketName); Query query = creator.createQuery(); assertEquals(query.export(), " WHERE " + where(x("META(`" + bucketName + "`).`cas`")).is(1611287177404088320L)