From a93c870b45a362326482f073afb49c69378236ff Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Fri, 9 Jun 2023 07:17:53 +0200 Subject: [PATCH 1/4] Prepare issue branch. --- pom.xml | 2 +- spring-data-mongodb-benchmarks/pom.xml | 2 +- spring-data-mongodb-distribution/pom.xml | 2 +- spring-data-mongodb/pom.xml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pom.xml b/pom.xml index 5225d1e3df..9074164110 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-mongodb-parent - 4.2.0-SNAPSHOT + 4.2.x-4379-SNAPSHOT pom Spring Data MongoDB diff --git a/spring-data-mongodb-benchmarks/pom.xml b/spring-data-mongodb-benchmarks/pom.xml index 2de4b6b635..1e0a519d25 100644 --- a/spring-data-mongodb-benchmarks/pom.xml +++ b/spring-data-mongodb-benchmarks/pom.xml @@ -7,7 +7,7 @@ org.springframework.data spring-data-mongodb-parent - 4.2.0-SNAPSHOT + 4.2.x-4379-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb-distribution/pom.xml b/spring-data-mongodb-distribution/pom.xml index 060a6d0dd9..fb47df3eb8 100644 --- a/spring-data-mongodb-distribution/pom.xml +++ b/spring-data-mongodb-distribution/pom.xml @@ -15,7 +15,7 @@ org.springframework.data spring-data-mongodb-parent - 4.2.0-SNAPSHOT + 4.2.x-4379-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/pom.xml b/spring-data-mongodb/pom.xml index 921254ca44..b99b0b3502 100644 --- a/spring-data-mongodb/pom.xml +++ b/spring-data-mongodb/pom.xml @@ -13,7 +13,7 @@ org.springframework.data spring-data-mongodb-parent - 4.2.0-SNAPSHOT + 4.2.x-4379-SNAPSHOT ../pom.xml From ff137eca8a43346e90a57f6edfd768777abcadb4 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Fri, 9 Jun 2023 07:28:53 +0200 Subject: [PATCH 2/4] Map collection and fields for $lookup aggregation against type. This commit enables using a type parameter to define the from collection of a lookup aggregation stage. In doing so we can derive the target collection name from the type and use the given information to also map the from field against the domain object to so that the user is able to operate on property names instead of the target db field name. --- .../AggregationOperationContext.java | 24 +++++ .../core/aggregation/LookupOperation.java | 64 +++++++++-- .../TypeBasedAggregationOperationContext.java | 14 +++ .../aggregation/AggregationTestUtils.java | 102 ++++++++++++++++++ .../aggregation/LookupOperationUnitTests.java | 59 +++++++++- 5 files changed, 251 insertions(+), 12 deletions(-) create mode 100644 spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationTestUtils.java diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AggregationOperationContext.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AggregationOperationContext.java index 3fdc1b125c..60363db917 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AggregationOperationContext.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AggregationOperationContext.java @@ -23,6 +23,7 @@ import org.bson.codecs.configuration.CodecRegistry; import org.springframework.beans.BeanUtils; import org.springframework.data.mongodb.CodecRegistryProvider; +import org.springframework.data.mongodb.MongoCollectionUtils; import org.springframework.data.mongodb.core.aggregation.ExposedFields.FieldReference; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -78,6 +79,29 @@ default Document getMappedObject(Document document) { */ FieldReference getReference(String name); + /** + * Obtain the target field name for a given field/type combination. + * + * @param type The type containing the field. + * @param field The property/field name + * @return never {@literal null}. + * @since 4.2 + */ + default String getMappedFieldName(Class type, String field) { + return field; + } + + /** + * Obtain the collection name for a given {@link Class type} combination. + * + * @param type + * @return never {@literal null}. + * @since 4.2 + */ + default String getCollection(Class type) { + return MongoCollectionUtils.getPreferredCollectionName(type); + } + /** * Returns the {@link Fields} exposed by the type. May be a {@literal class} or an {@literal interface}. The default * implementation uses {@link BeanUtils#getPropertyDescriptors(Class) property descriptors} discover fields from a diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/LookupOperation.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/LookupOperation.java index 44d0f1569e..4e4abfb1b3 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/LookupOperation.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/LookupOperation.java @@ -39,7 +39,7 @@ */ public class LookupOperation implements FieldsExposingAggregationOperation, InheritsFieldsAggregationOperation { - private final String from; + private Object from; @Nullable // private final Field localField; @@ -97,6 +97,22 @@ public LookupOperation(String from, @Nullable Let let, AggregationPipeline pipel */ public LookupOperation(String from, @Nullable Field localField, @Nullable Field foreignField, @Nullable Let let, @Nullable AggregationPipeline pipeline, Field as) { + this((Object) from, localField, foreignField, let, pipeline, as); + } + + /** + * Creates a new {@link LookupOperation} for the given combination of {@link Field}s and {@link AggregationPipeline + * pipeline}. + * + * @param from must not be {@literal null}. Can be eiter the target collection name or a {@link Class}. + * @param localField can be {@literal null} if {@literal pipeline} is present. + * @param foreignField can be {@literal null} if {@literal pipeline} is present. + * @param let can be {@literal null} if {@literal localField} and {@literal foreignField} are present. + * @param as must not be {@literal null}. + * @since 4.1 + */ + private LookupOperation(Object from, @Nullable Field localField, @Nullable Field foreignField, @Nullable Let let, + @Nullable AggregationPipeline pipeline, Field as) { Assert.notNull(from, "From must not be null"); if (pipeline == null) { @@ -125,12 +141,14 @@ public Document toDocument(AggregationOperationContext context) { Document lookupObject = new Document(); - lookupObject.append("from", from); + lookupObject.append("from", getCollectionName(context)); + if (localField != null) { lookupObject.append("localField", localField.getTarget()); } + if (foreignField != null) { - lookupObject.append("foreignField", foreignField.getTarget()); + lookupObject.append("foreignField", getForeignFieldName(context)); } if (let != null) { lookupObject.append("let", let.toDocument(context).get("$let", Document.class).get("vars")); @@ -144,6 +162,16 @@ public Document toDocument(AggregationOperationContext context) { return new Document(getOperator(), lookupObject); } + String getCollectionName(AggregationOperationContext context) { + return from instanceof Class type ? context.getCollection(type) : from.toString(); + } + + String getForeignFieldName(AggregationOperationContext context) { + + return from instanceof Class type ? context.getMappedFieldName(type, foreignField.getTarget()) + : foreignField.getTarget(); + } + @Override public String getOperator() { return "$lookup"; @@ -158,16 +186,28 @@ public static FromBuilder newLookup() { return new LookupOperationBuilder(); } - public static interface FromBuilder { + public interface FromBuilder { /** * @param name the collection in the same database to perform the join with, must not be {@literal null} or empty. * @return never {@literal null}. */ LocalFieldBuilder from(String name); + + /** + * Use the given type to determine name of the foreign collection and map + * {@link ForeignFieldBuilder#foreignField(String)} against it to consider eventually present + * {@link org.springframework.data.mongodb.core.mapping.Field} annotations. + * + * @param type the type of the target collection in the same database to perform the join with, must not be + * {@literal null}. + * @return never {@literal null}. + * @since 4.2 + */ + LocalFieldBuilder from(Class type); } - public static interface LocalFieldBuilder extends PipelineBuilder { + public interface LocalFieldBuilder extends PipelineBuilder { /** * @param name the field from the documents input to the {@code $lookup} stage, must not be {@literal null} or @@ -177,7 +217,7 @@ public static interface LocalFieldBuilder extends PipelineBuilder { ForeignFieldBuilder localField(String name); } - public static interface ForeignFieldBuilder { + public interface ForeignFieldBuilder { /** * @param name the field from the documents in the {@code from} collection, must not be {@literal null} or empty. @@ -246,7 +286,7 @@ default AsBuilder pipeline(AggregationOperation... stages) { LookupOperation as(String name); } - public static interface AsBuilder extends PipelineBuilder { + public interface AsBuilder extends PipelineBuilder { /** * @param name the name of the new array field to add to the input documents, must not be {@literal null} or empty. @@ -264,7 +304,7 @@ public static interface AsBuilder extends PipelineBuilder { public static final class LookupOperationBuilder implements FromBuilder, LocalFieldBuilder, ForeignFieldBuilder, AsBuilder { - private @Nullable String from; + private @Nullable Object from; private @Nullable Field localField; private @Nullable Field foreignField; private @Nullable ExposedField as; @@ -288,6 +328,14 @@ public LocalFieldBuilder from(String name) { return this; } + @Override + public LocalFieldBuilder from(Class type) { + + Assert.notNull(type, "'From' must not be null"); + from = type; + return this; + } + @Override public AsBuilder foreignField(String name) { 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 fd54514cbb..efd135d328 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 @@ -92,6 +92,20 @@ public FieldReference getReference(String name) { return getReferenceFor(field(name)); } + @Override + public String getCollection(Class type) { + + MongoPersistentEntity persistentEntity = mappingContext.getPersistentEntity(type); + return persistentEntity != null ? persistentEntity.getCollection() : AggregationOperationContext.super.getCollection(type); + } + + @Override + public String getMappedFieldName(Class type, String field) { + + PersistentPropertyPath persistentPropertyPath = mappingContext.getPersistentPropertyPath(field, type); + return persistentPropertyPath.getLeafProperty().getFieldName(); + } + @Override public Fields getFields(Class type) { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationTestUtils.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationTestUtils.java new file mode 100644 index 0000000000..57e8316224 --- /dev/null +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationTestUtils.java @@ -0,0 +1,102 @@ +/* + * Copyright 2023. 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 + * + * http://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. + */ + +/* + * Copyright 2023 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 + * + * http://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.mongodb.core.aggregation; + +import org.springframework.data.mapping.context.MappingContext; +import org.springframework.data.mongodb.core.convert.MappingMongoConverter; +import org.springframework.data.mongodb.core.convert.NoOpDbRefResolver; +import org.springframework.data.mongodb.core.convert.QueryMapper; +import org.springframework.data.mongodb.core.mapping.MongoMappingContext; +import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; +import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty; +import org.springframework.data.mongodb.test.util.MongoTestMappingContext; + +/** + * @author Christoph Strobl + * @since 2023/06 + */ +public final class AggregationTestUtils { + + public static AggregationContextBuilder strict(Class type) { + + AggregationContextBuilder builder = new AggregationContextBuilder<>(); + builder.strict = true; + return builder.forType(type); + } + + public static AggregationContextBuilder relaxed(Class type) { + + AggregationContextBuilder builder = new AggregationContextBuilder<>(); + builder.strict = false; + return builder.forType(type); + } + + public static class AggregationContextBuilder { + + Class targetType; + MappingContext, MongoPersistentProperty> mappingContext; + QueryMapper queryMapper; + boolean strict; + + public AggregationContextBuilder forType(Class type) { + + this.targetType = type; + return (AggregationContextBuilder) this; + } + + public AggregationContextBuilder using( + MappingContext, MongoPersistentProperty> mappingContext) { + + this.mappingContext = mappingContext; + return this; + } + + public AggregationContextBuilder using(QueryMapper queryMapper) { + + this.queryMapper = queryMapper; + return this; + } + + public T ctx() { + // + if (targetType == null) { + return (T) Aggregation.DEFAULT_CONTEXT; + } + + MappingContext, MongoPersistentProperty> ctx = mappingContext != null ? mappingContext : MongoTestMappingContext.newTestContext().init(); + QueryMapper qm = queryMapper != null ? queryMapper + : new QueryMapper(new MappingMongoConverter(NoOpDbRefResolver.INSTANCE, ctx)); + return (T) (strict ? new TypeBasedAggregationOperationContext(targetType, ctx, qm) + : new RelaxedTypeBasedAggregationOperationContext(targetType, ctx, qm)); + } + } +} diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/LookupOperationUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/LookupOperationUnitTests.java index 45b63763cf..1127ad6eac 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/LookupOperationUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/LookupOperationUnitTests.java @@ -25,6 +25,7 @@ import org.bson.Document; import org.junit.jupiter.api.Test; import org.springframework.data.mongodb.core.DocumentTestUtils; +import org.springframework.data.mongodb.core.mapping.Field; import org.springframework.data.mongodb.core.query.Criteria; /** @@ -92,7 +93,7 @@ private Document extractDocumentFromLookupOperation(LookupOperation lookupOperat @Test // DATAMONGO-1326 public void builderRejectsNullFromField() { - assertThatIllegalArgumentException().isThrownBy(() -> LookupOperation.newLookup().from(null)); + assertThatIllegalArgumentException().isThrownBy(() -> LookupOperation.newLookup().from((String) null)); } @Test // DATAMONGO-1326 @@ -195,10 +196,10 @@ void buildsLookupWithJustPipeline() { void buildsLookupWithLocalAndForeignFieldAsWellAsLetAndPipeline() { LookupOperation lookupOperation = Aggregation.lookup().from("restaurants") // - .localField("restaurant_name") - .foreignField("name") + .localField("restaurant_name") // + .foreignField("name") // .let(newVariable("orders_drink").forField("drink")) // - .pipeline(match(ctx -> new Document("$expr", new Document("$in", List.of("$$orders_drink", "$beverages"))))) + .pipeline(match(ctx -> new Document("$expr", new Document("$in", List.of("$$orders_drink", "$beverages"))))) // .as("matches"); assertThat(lookupOperation.toDocument(Aggregation.DEFAULT_CONTEXT)).isEqualTo(""" @@ -216,4 +217,54 @@ void buildsLookupWithLocalAndForeignFieldAsWellAsLetAndPipeline() { }} """); } + + @Test // GH-4379 + void unmappedLookupWithFromExtractedFromType() { + + LookupOperation lookupOperation = Aggregation.lookup().from(Restaurant.class) // + .localField("restaurant_name") // + .foreignField("name") // + .as("restaurants"); + + assertThat(lookupOperation.toDocument(Aggregation.DEFAULT_CONTEXT)).isEqualTo(""" + { $lookup: + { + from: "restaurant", + localField: "restaurant_name", + foreignField: "name", + as: "restaurants" + } + }} + """); + } + + @Test // GH-4379 + void mappedLookupWithFromExtractedFromType() { + + LookupOperation lookupOperation = Aggregation.lookup().from(Restaurant.class) // + .localField("restaurant_name") // + .foreignField("name") // + .as("restaurants"); + + + assertThat(lookupOperation.toDocument(AggregationTestUtils.strict(Restaurant.class).ctx())).isEqualTo(""" + { $lookup: + { + from: "sites", + localField: "restaurant_name", + foreignField: "rs_name", + as: "restaurants" + } + }} + """); + } + + @org.springframework.data.mongodb.core.mapping.Document("sites") + static class Restaurant { + + String id; + + @Field("rs_name") // + String name; + } } From b46aab2c28cc78a6f0f283d623001e5d9f7a5f21 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Mon, 12 Jun 2023 11:04:27 +0200 Subject: [PATCH 3/4] Map collection and fields for $graphLookup aggregation against type. This commit enables using a type parameter to define the from collection of a graphLookup aggregation stage. In doing so we can derive the target collection name from the type and use the given information to also map the from field against the domain object to so that the user is able to operate on property names instead of the target db field name. --- .../aggregation/GraphLookupOperation.java | 45 +++++++++++--- .../GraphLookupOperationUnitTests.java | 58 ++++++++++++++++++- 2 files changed, 94 insertions(+), 9 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/GraphLookupOperation.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/GraphLookupOperation.java index a2fe238627..02b3fb4ea8 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/GraphLookupOperation.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/GraphLookupOperation.java @@ -46,7 +46,7 @@ public class GraphLookupOperation implements InheritsFieldsAggregationOperation private static final Set> ALLOWED_START_TYPES = new HashSet>( Arrays.> asList(AggregationExpression.class, String.class, Field.class, Document.class)); - private final String from; + private final Object from; private final List startWith; private final Field connectFrom; private final Field connectTo; @@ -55,7 +55,7 @@ public class GraphLookupOperation implements InheritsFieldsAggregationOperation private final @Nullable Field depthField; private final @Nullable CriteriaDefinition restrictSearchWithMatch; - private GraphLookupOperation(String from, List startWith, Field connectFrom, Field connectTo, Field as, + private GraphLookupOperation(Object from, List startWith, Field connectFrom, Field connectTo, Field as, @Nullable Long maxDepth, @Nullable Field depthField, @Nullable CriteriaDefinition restrictSearchWithMatch) { this.from = from; @@ -82,7 +82,7 @@ public Document toDocument(AggregationOperationContext context) { Document graphLookup = new Document(); - graphLookup.put("from", from); + graphLookup.put("from", getCollectionName(context)); List mappedStartWith = new ArrayList<>(startWith.size()); @@ -99,7 +99,7 @@ public Document toDocument(AggregationOperationContext context) { graphLookup.put("startWith", mappedStartWith.size() == 1 ? mappedStartWith.iterator().next() : mappedStartWith); - graphLookup.put("connectFromField", connectFrom.getTarget()); + graphLookup.put("connectFromField", getForeignFieldName(context)); graphLookup.put("connectToField", connectTo.getTarget()); graphLookup.put("as", as.getName()); @@ -118,6 +118,16 @@ public Document toDocument(AggregationOperationContext context) { return new Document(getOperator(), graphLookup); } + String getCollectionName(AggregationOperationContext context) { + return from instanceof Class type ? context.getCollection(type) : from.toString(); + } + + String getForeignFieldName(AggregationOperationContext context) { + + return from instanceof Class type ? context.getMappedFieldName(type, connectFrom.getTarget()) + : connectFrom.getTarget(); + } + @Override public String getOperator() { return "$graphLookup"; @@ -128,7 +138,7 @@ public ExposedFields getFields() { List fields = new ArrayList<>(2); fields.add(new ExposedField(as, true)); - if(depthField != null) { + if (depthField != null) { fields.add(new ExposedField(depthField, true)); } return ExposedFields.from(fields.toArray(new ExposedField[0])); @@ -146,6 +156,17 @@ public interface FromBuilder { * @return never {@literal null}. */ StartWithBuilder from(String collectionName); + + /** + * Use the given type to determine name of the foreign collection and map + * {@link ConnectFromBuilder#connectFrom(String)} against it to consider eventually present + * {@link org.springframework.data.mongodb.core.mapping.Field} annotations. + * + * @param type must not be {@literal null}. + * @return never {@literal null}. + * @since 4.2 + */ + StartWithBuilder from(Class type); } /** @@ -218,7 +239,7 @@ public interface ConnectToBuilder { static final class GraphLookupOperationFromBuilder implements FromBuilder, StartWithBuilder, ConnectFromBuilder, ConnectToBuilder { - private @Nullable String from; + private @Nullable Object from; private @Nullable List startWith; private @Nullable String connectFrom; @@ -231,6 +252,14 @@ public StartWithBuilder from(String collectionName) { return this; } + @Override + public StartWithBuilder from(Class type) { + + Assert.notNull(type, "Type must not be null"); + this.from = type; + return this; + } + @Override public ConnectFromBuilder startWith(String... fieldReferences) { @@ -321,7 +350,7 @@ public GraphLookupOperationBuilder connectTo(String fieldName) { */ public static final class GraphLookupOperationBuilder { - private final String from; + private final Object from; private final List startWith; private final Field connectFrom; private final Field connectTo; @@ -329,7 +358,7 @@ public static final class GraphLookupOperationBuilder { private @Nullable Field depthField; private @Nullable CriteriaDefinition restrictSearchWithMatch; - protected GraphLookupOperationBuilder(String from, List startWith, String connectFrom, + protected GraphLookupOperationBuilder(Object from, List startWith, String connectFrom, String connectTo) { this.from = from; diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/GraphLookupOperationUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/GraphLookupOperationUnitTests.java index 5280b603f6..6d85b31ffb 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/GraphLookupOperationUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/GraphLookupOperationUnitTests.java @@ -22,6 +22,7 @@ import org.bson.Document; import org.junit.jupiter.api.Test; import org.springframework.data.mongodb.core.Person; +import org.springframework.data.mongodb.core.mapping.Field; import org.springframework.data.mongodb.core.query.Criteria; /** @@ -34,7 +35,7 @@ public class GraphLookupOperationUnitTests { @Test // DATAMONGO-1551 public void rejectsNullFromCollection() { - assertThatIllegalArgumentException().isThrownBy(() -> GraphLookupOperation.builder().from(null)); + assertThatIllegalArgumentException().isThrownBy(() -> GraphLookupOperation.builder().from((String) null)); } @Test // DATAMONGO-1551 @@ -158,4 +159,59 @@ public void depthFieldShouldUseTargetFieldInsteadOfAlias() { assertThat(document).containsEntry("$graphLookup.depthField", "foo.bar"); } + + @Test // GH-4379 + void unmappedLookupWithFromExtractedFromType() { + + GraphLookupOperation graphLookupOperation = GraphLookupOperation.builder() // + .from(Employee.class) // + .startWith(LiteralOperators.Literal.asLiteral("hello")) // + .connectFrom("manager") // + .connectTo("name") // + .as("reportingHierarchy"); + + assertThat(graphLookupOperation.toDocument(Aggregation.DEFAULT_CONTEXT)).isEqualTo(""" + { $graphLookup: + { + from: "employee", + startWith : { $literal : "hello" }, + connectFromField: "manager", + connectToField: "name", + as: "reportingHierarchy" + } + }} + """); + } + + @Test // GH-4379 + void mappedLookupWithFromExtractedFromType() { + + GraphLookupOperation graphLookupOperation = GraphLookupOperation.builder() // + .from(Employee.class) // + .startWith(LiteralOperators.Literal.asLiteral("hello")) // + .connectFrom("manager") // + .connectTo("name") // + .as("reportingHierarchy"); + + assertThat(graphLookupOperation.toDocument(AggregationTestUtils.strict(Employee.class).ctx())).isEqualTo(""" + { $graphLookup: + { + from: "employees", + startWith : { $literal : "hello" }, + connectFromField: "reportsTo", + connectToField: "name", + as: "reportingHierarchy" + } + }} + """); + } + + @org.springframework.data.mongodb.core.mapping.Document("employees") + static class Employee { + + String id; + + @Field("reportsTo") + String manager; + } } From 7103ca22283666dc7630148ce5e7ca6d96213f66 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 10 Jul 2023 10:15:30 +0200 Subject: [PATCH 4/4] Polishing. Reformatting, trailing whitespaces. --- .../AggregationOperationContext.java | 6 ++--- .../core/aggregation/LookupOperation.java | 8 +++---- ...DelegatingAggregationOperationContext.java | 5 ++-- .../aggregation/AggregationTestUtils.java | 23 ++++--------------- 4 files changed, 13 insertions(+), 29 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AggregationOperationContext.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AggregationOperationContext.java index 60363db917..0ceb1c84d7 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AggregationOperationContext.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AggregationOperationContext.java @@ -81,7 +81,7 @@ default Document getMappedObject(Document document) { /** * Obtain the target field name for a given field/type combination. - * + * * @param type The type containing the field. * @param field The property/field name * @return never {@literal null}. @@ -103,7 +103,7 @@ default String getCollection(Class type) { } /** - * Returns the {@link Fields} exposed by the type. May be a {@literal class} or an {@literal interface}. The default + * Returns the {@link Fields} exposed by the type. Can be a {@literal class} or an {@literal interface}. The default * implementation uses {@link BeanUtils#getPropertyDescriptors(Class) property descriptors} discover fields from a * {@link Class}. * @@ -133,7 +133,7 @@ default Fields getFields(Class type) { /** * This toggle allows the {@link AggregationOperationContext context} to use any given field name without checking for - * its existence. Typically the {@link AggregationOperationContext} fails when referencing unknown fields, those that + * its existence. Typically, the {@link AggregationOperationContext} fails when referencing unknown fields, those that * are not present in one of the previous stages or the input source, throughout the pipeline. * * @return a more relaxed {@link AggregationOperationContext}. diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/LookupOperation.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/LookupOperation.java index 4e4abfb1b3..3cfda35cef 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/LookupOperation.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/LookupOperation.java @@ -109,7 +109,7 @@ public LookupOperation(String from, @Nullable Field localField, @Nullable Field * @param foreignField can be {@literal null} if {@literal pipeline} is present. * @param let can be {@literal null} if {@literal localField} and {@literal foreignField} are present. * @param as must not be {@literal null}. - * @since 4.1 + * @since 4.2 */ private LookupOperation(Object from, @Nullable Field localField, @Nullable Field foreignField, @Nullable Let let, @Nullable AggregationPipeline pipeline, Field as) { @@ -142,11 +142,11 @@ public Document toDocument(AggregationOperationContext context) { Document lookupObject = new Document(); lookupObject.append("from", getCollectionName(context)); - + if (localField != null) { lookupObject.append("localField", localField.getTarget()); } - + if (foreignField != null) { lookupObject.append("foreignField", getForeignFieldName(context)); } @@ -198,7 +198,7 @@ public interface FromBuilder { * Use the given type to determine name of the foreign collection and map * {@link ForeignFieldBuilder#foreignField(String)} against it to consider eventually present * {@link org.springframework.data.mongodb.core.mapping.Field} annotations. - * + * * @param type the type of the target collection in the same database to perform the join with, must not be * {@literal null}. * @return never {@literal null}. diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/PrefixingDelegatingAggregationOperationContext.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/PrefixingDelegatingAggregationOperationContext.java index 4d0ee5ff6c..27b1ab6737 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/PrefixingDelegatingAggregationOperationContext.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/PrefixingDelegatingAggregationOperationContext.java @@ -30,9 +30,8 @@ /** * {@link AggregationOperationContext} implementation prefixing non-command keys on root level with the given prefix. - * Useful when mapping fields to domain specific types while having to prefix keys for query purpose. - *
- * Fields to be excluded from prefixing my be added to a {@literal denylist}. + * Useful when mapping fields to domain specific types while having to prefix keys for query purpose.
+ * Fields to be excluded from prefixing can be added to a {@literal denylist}. * * @author Christoph Strobl * @author Mark Paluch diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationTestUtils.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationTestUtils.java index 57e8316224..35e45915f5 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationTestUtils.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationTestUtils.java @@ -5,7 +5,7 @@ * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * 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, @@ -14,35 +14,18 @@ * limitations under the License. */ -/* - * Copyright 2023 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 - * - * http://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.mongodb.core.aggregation; import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.mongodb.core.convert.MappingMongoConverter; import org.springframework.data.mongodb.core.convert.NoOpDbRefResolver; import org.springframework.data.mongodb.core.convert.QueryMapper; -import org.springframework.data.mongodb.core.mapping.MongoMappingContext; import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty; import org.springframework.data.mongodb.test.util.MongoTestMappingContext; /** * @author Christoph Strobl - * @since 2023/06 */ public final class AggregationTestUtils { @@ -92,7 +75,9 @@ public T ctx() { return (T) Aggregation.DEFAULT_CONTEXT; } - MappingContext, MongoPersistentProperty> ctx = mappingContext != null ? mappingContext : MongoTestMappingContext.newTestContext().init(); + MappingContext, MongoPersistentProperty> ctx = mappingContext != null + ? mappingContext + : MongoTestMappingContext.newTestContext().init(); QueryMapper qm = queryMapper != null ? queryMapper : new QueryMapper(new MappingMongoConverter(NoOpDbRefResolver.INSTANCE, ctx)); return (T) (strict ? new TypeBasedAggregationOperationContext(targetType, ctx, qm)