From 9d292f64b9e68aa245980bb456efbe667c453add Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 9 Dec 2013 16:24:54 +0100 Subject: [PATCH] DATAMONGO-806 - Fixed invalid rendering of id field references. Tweaked the rendering of projection operations to always use the field based reference lookup to make sure the reference gets rendered aliased. Moved value calculation logic into FieldReference. Original pull request: #101. --- .../core/aggregation/ExposedFields.java | 17 +++-- .../core/aggregation/ProjectionOperation.java | 4 +- .../TypeBasedAggregationOperationContext.java | 14 ++-- .../core/aggregation/AggregationTests.java | 64 ++++++++++--------- ...dAggregationOperationContextUnitTests.java | 14 ++++ 5 files changed, 66 insertions(+), 47 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/ExposedFields.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/ExposedFields.java index b6bd75fd1..03d7df561 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/ExposedFields.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/ExposedFields.java @@ -343,13 +343,6 @@ public class ExposedFields implements Iterable { this.field = field; } - /** - * @return - */ - public boolean isSynthetic() { - return field.synthetic; - } - /** * Returns the raw, unqualified reference, i.e. the field reference without a {@literal $} prefix. * @@ -361,6 +354,16 @@ public class ExposedFields implements Iterable { return field.synthetic ? target : String.format("%s.%s", Fields.UNDERSCORE_ID, target); } + /** + * Returns the referenve value for the given field reference. Will return 1 for a synthetic, unaliased field or the + * raw rendering of the reference otherwise. + * + * @return + */ + public Object getReferenceValue() { + return field.synthetic && !field.isAliased() ? 1 : toString(); + } + /* * (non-Javadoc) * @see java.lang.Object#toString() diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/ProjectionOperation.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/ProjectionOperation.java index 540ac9087..a00944262 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/ProjectionOperation.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/ProjectionOperation.java @@ -21,7 +21,6 @@ import java.util.Collections; import java.util.List; import org.springframework.data.mongodb.core.aggregation.ExposedFields.ExposedField; -import org.springframework.data.mongodb.core.aggregation.ExposedFields.FieldReference; import org.springframework.data.mongodb.core.aggregation.ProjectionOperation.ProjectionOperationBuilder.FieldProjection; import org.springframework.util.Assert; @@ -626,8 +625,7 @@ public class ProjectionOperation implements FieldsExposingAggregationOperation { if (value == null || Boolean.TRUE.equals(value)) { // check whether referenced field exists in the context - FieldReference reference = context.getReference(field.getTarget()); - return reference.isSynthetic() && !field.isAliased() ? 1 : reference.toString(); + return context.getReference(field).getReferenceValue(); } else if (Boolean.FALSE.equals(value)) { diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/TypeBasedAggregationOperationContext.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/TypeBasedAggregationOperationContext.java index 1ed1e9ff4..c800c419c 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/TypeBasedAggregationOperationContext.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/TypeBasedAggregationOperationContext.java @@ -88,14 +88,16 @@ public class TypeBasedAggregationOperationContext implements AggregationOperatio */ @Override public FieldReference getReference(String name) { - - PersistentPropertyPath propertyPath = mappingContext.getPersistentPropertyPath(name, type); - - return getReferenceFor(field(propertyPath.getLeafProperty().getName(), - propertyPath.toDotPath(MongoPersistentProperty.PropertyToFieldNameConverter.INSTANCE))); + return getReferenceFor(field(name)); } private FieldReference getReferenceFor(Field field) { - return new FieldReference(new ExposedField(field, true)); + + PersistentPropertyPath propertyPath = mappingContext.getPersistentPropertyPath( + field.getTarget(), type); + Field mappedField = field(propertyPath.getLeafProperty().getName(), + propertyPath.toDotPath(MongoPersistentProperty.PropertyToFieldNameConverter.INSTANCE)); + + return new FieldReference(new ExposedField(mappedField, true)); } } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationTests.java index 188b56f73..6188b8380 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationTests.java @@ -34,7 +34,6 @@ import java.util.Scanner; import org.joda.time.LocalDateTime; import org.junit.After; import org.junit.Before; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -749,39 +748,10 @@ public class AggregationTests { assertThat((Integer) items.get(1).get("y"), is(1)); } - static class User { - - @Id String id; - List msgs; - - public User() {} - - public User(String id, PushMessage... msgs) { - this.id = id; - this.msgs = Arrays.asList(msgs); - } - } - - static class PushMessage { - - @Id String id; - String content; - Date createDate; - - public PushMessage() {} - - public PushMessage(String id, String content, Date createDate) { - this.id = id; - this.content = content; - this.createDate = createDate; - } - } - /** * @see DATAMONGO-806 */ @Test - @Ignore public void shouldAllowGroupByIdFields() { mongoTemplate.dropCollection(User.class); @@ -806,7 +776,6 @@ public class AggregationTests { AggregationResults results = mongoTemplate.aggregate(agg, User.class, DBObject.class); List mappedResults = results.getMappedResults(); - System.out.println(mappedResults); DBObject firstItem = mappedResults.get(0); assertThat(firstItem.get("_id"), is(notNullValue())); @@ -896,4 +865,37 @@ public class AggregationTests { } } + /** + * @see DATAMONGO-806 + */ + static class User { + + @Id String id; + List msgs; + + public User() {} + + public User(String id, PushMessage... msgs) { + this.id = id; + this.msgs = Arrays.asList(msgs); + } + } + + /** + * @see DATAMONGO-806 + */ + static class PushMessage { + + @Id String id; + String content; + Date createDate; + + public PushMessage() {} + + public PushMessage(String id, String content, Date createDate) { + this.id = id; + this.content = content; + this.createDate = createDate; + } + } } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/TypeBasedAggregationOperationContextUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/TypeBasedAggregationOperationContextUnitTests.java index 1047464f1..6aae2af59 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/TypeBasedAggregationOperationContextUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/TypeBasedAggregationOperationContextUnitTests.java @@ -23,7 +23,10 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.data.annotation.Id; import org.springframework.data.mapping.model.MappingException; +import org.springframework.data.mongodb.core.aggregation.ExposedFields.ExposedField; +import org.springframework.data.mongodb.core.aggregation.ExposedFields.FieldReference; import org.springframework.data.mongodb.core.convert.DbRefResolver; import org.springframework.data.mongodb.core.convert.MappingMongoConverter; import org.springframework.data.mongodb.core.convert.QueryMapper; @@ -76,12 +79,23 @@ public class TypeBasedAggregationOperationContextUnitTests { assertThat(context.getReference(field), is(context.getReference("bar.name"))); } + /** + * @see DATAMONGO-806 + */ + @Test + public void aliasesIdFieldCorrectly() { + + AggregationOperationContext context = getContext(Foo.class); + assertThat(context.getReference("id"), is(new FieldReference(new ExposedField(Fields.field("id", "_id"), true)))); + } + private TypeBasedAggregationOperationContext getContext(Class type) { return new TypeBasedAggregationOperationContext(type, context, mapper); } static class Foo { + @Id String id; Bar bar; }