From 6a446cbc7f894e3cd0d24d57927348e04058f706 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 20 Apr 2017 10:27:33 +0200 Subject: [PATCH] DATAMONGO-1325 - Polishing. Add since tag to new method. Set year of inception in copyright header. Add ticket reference to test. Minor code reformatting. Add integration test. Change query keyword for query-by-example from $sample to $example to prevent accidental collisions. Remove Mongo 3.4-next build profile due to removed Mongo 3.4 driver snapshots. Original pull request: #452. --- .travis.yml | 1 - .../mongodb/core/aggregation/Aggregation.java | 11 +- .../core/aggregation/SampleOperation.java | 5 +- .../mongodb/core/convert/QueryMapper.java | 100 +++++++++--------- .../data/mongodb/core/query/Criteria.java | 48 +++++---- .../core/aggregation/AggregationTests.java | 18 ++++ .../aggregation/SampleOperationUnitTests.java | 13 ++- 7 files changed, 110 insertions(+), 86 deletions(-) diff --git a/.travis.yml b/.travis.yml index 6cc3238bd..65e358877 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,7 +9,6 @@ before_script: env: matrix: - PROFILE=ci - - PROFILE=mongo34-next - PROFILE=mongo35-next # Current MongoDB version is 2.4.2 as of 2016-04, see https://github.com/travis-ci/travis-ci/issues/3694 diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/Aggregation.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/Aggregation.java index 72b09e482..a56bd8f59 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/Aggregation.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/Aggregation.java @@ -17,7 +17,6 @@ package org.springframework.data.mongodb.core.aggregation; import static org.springframework.data.mongodb.core.aggregation.Fields.*; -import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -25,15 +24,10 @@ import org.bson.Document; import org.springframework.data.domain.Sort; import org.springframework.data.domain.Sort.Direction; import org.springframework.data.mongodb.core.aggregation.CountOperation.CountOperationBuilder; -import org.springframework.data.mongodb.core.aggregation.ExposedFields.DirectFieldReference; -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.FacetOperation.FacetOperationBuilder; -import org.springframework.data.mongodb.core.aggregation.FieldsExposingAggregationOperation.InheritsFieldsAggregationOperation; import org.springframework.data.mongodb.core.aggregation.GraphLookupOperation.StartWithBuilder; import org.springframework.data.mongodb.core.aggregation.ReplaceRootOperation.ReplaceRootDocumentOperationBuilder; import org.springframework.data.mongodb.core.aggregation.ReplaceRootOperation.ReplaceRootOperationBuilder; -import org.springframework.data.mongodb.core.aggregation.Fields.*; import org.springframework.data.mongodb.core.query.Criteria; import org.springframework.data.mongodb.core.query.CriteriaDefinition; import org.springframework.data.mongodb.core.query.NearQuery; @@ -384,15 +378,16 @@ public class Aggregation { } /** - * Creates a new {@link SampleOperation} to selects the specified number of documents from its input randomly. + * Creates a new {@link SampleOperation} to select the specified number of documents from its input randomly. * * @param sampleSize must not be less than zero. * @return + * @since 2.0 */ public static SampleOperation sample(long sampleSize) { return new SampleOperation(sampleSize); } - + /** * Creates a new {@link MatchOperation} using the given {@link Criteria}. * diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/SampleOperation.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/SampleOperation.java index c727370b6..26e6aead8 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/SampleOperation.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/SampleOperation.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2017 the original author or authors. + * Copyright 2017 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. @@ -26,7 +26,8 @@ import org.springframework.util.Assert; * * @author Gustavo de Geus * @since 2.0 - * @see MongoDB Aggregation Framework: $sample + * @see MongoDB Aggregation Framework: + * $sample */ public class SampleOperation implements AggregationOperation { diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java index 3384f94fa..a66ed7487 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java @@ -57,7 +57,7 @@ import com.mongodb.DBRef; /** * A helper class to encapsulate any modifications of a Query object before it gets submitted to the database. - * + * * @author Jon Brisbin * @author Oliver Gierke * @author Patryk Wasik @@ -82,7 +82,7 @@ public class QueryMapper { /** * Creates a new {@link QueryMapper} with the given {@link MongoConverter}. - * + * * @param converter must not be {@literal null}. */ public QueryMapper(MongoConverter converter) { @@ -102,7 +102,7 @@ public class QueryMapper { /** * Replaces the property keys used in the given {@link Document} with the appropriate keys by using the * {@link PersistentEntity} metadata. - * + * * @param query must not be {@literal null}. * @param entity can be {@literal null}. * @return @@ -155,7 +155,7 @@ public class QueryMapper { /** * Maps fields used for sorting to the {@link MongoPersistentEntity}s properties.
* Also converts properties to their {@code $meta} representation if present. - * + * * @param sortObject * @param entity * @return @@ -179,7 +179,7 @@ public class QueryMapper { /** * Maps fields to retrieve to the {@link MongoPersistentEntity}s properties.
* Also onverts and potentially adds missing property {@code $meta} representation. - * + * * @param fieldsObject * @param entity * @return @@ -217,7 +217,7 @@ public class QueryMapper { /** * Extracts the mapped object value for given field out of rawValue taking nested {@link Keyword}s into account - * + * * @param field * @param rawValue * @return @@ -250,7 +250,7 @@ public class QueryMapper { /** * Returns the given {@link Document} representing a keyword by mapping the keyword's value. - * + * * @param keyword the {@link Document} representing a keyword (e.g. {@code $ne : … } ) * @param entity * @return @@ -280,7 +280,7 @@ public class QueryMapper { /** * Returns the mapped keyword considered defining a criteria for the given property. - * + * * @param property * @param keyword * @return @@ -299,7 +299,7 @@ public class QueryMapper { /** * Returns the mapped value for the given source object assuming it's a value for the given * {@link MongoPersistentProperty}. - * + * * @param value the source object to be mapped * @param property the property the value is a value for * @param newKey the key the value will be bound to eventually @@ -368,7 +368,7 @@ public class QueryMapper { * requires conversion to a {@link org.springframework.data.mongodb.core.mapping.DBRef} object. We check whether the * type of the given value is compatible with the type of the given document field in order to deal with potential * query field exclusions, since MongoDB uses the {@code int} {@literal 0} as an indicator for an excluded field. - * + * * @param documentField must not be {@literal null}. * @param value * @return @@ -399,7 +399,7 @@ public class QueryMapper { /** * Retriggers mapping if the given source is a {@link Document} or simply invokes the - * + * * @param source * @param entity * @return @@ -432,7 +432,7 @@ public class QueryMapper { /** * Converts the given source Object to a mongo type with the type information of the original source type omitted. * Subclasses may overwrite this method to retain the type information of the source type on the resulting mongo type. - * + * * @param source * @param entity * @return the converted mongo type or null if source is null @@ -447,7 +447,7 @@ public class QueryMapper { /** * Converts the given source assuming it's actually an association to another object. - * + * * @param source * @param property * @return @@ -486,7 +486,7 @@ public class QueryMapper { /** * Checks whether the given value is a {@link Document}. - * + * * @param value can be {@literal null}. * @return */ @@ -506,7 +506,7 @@ public class QueryMapper { /** * Creates a new {@link Entry} for the given {@link Field} with the given value. - * + * * @param field must not be {@literal null}. * @param value can be {@literal null}. * @return @@ -517,7 +517,7 @@ public class QueryMapper { /** * Creates a new {@link Entry} with the given key and value. - * + * * @param key must not be {@literal null} or empty. * @param value can be {@literal null} * @return @@ -543,7 +543,7 @@ public class QueryMapper { /** * Converts the given raw id value into either {@link ObjectId} or {@link String}. - * + * * @param id * @return */ @@ -566,7 +566,7 @@ public class QueryMapper { /** * Returns whether the given {@link Object} is a keyword, i.e. if it's a {@link Document} with a keyword key. - * + * * @param candidate * @return */ @@ -588,7 +588,7 @@ public class QueryMapper { /** * Returns whether the given {@link String} is a MongoDB keyword. The default implementation will check against the * set of registered keywords returned by {@link #getKeywords()}. - * + * * @param candidate * @return */ @@ -598,7 +598,7 @@ public class QueryMapper { /** * Value object to capture a query keyword representation. - * + * * @author Oliver Gierke */ static class Keyword { @@ -624,7 +624,7 @@ public class QueryMapper { /** * Returns whether the current keyword is the {@code $exists} keyword. - * + * * @return */ public boolean isExists() { @@ -637,7 +637,7 @@ public class QueryMapper { /** * Returns whether the current keyword is the {@code $geometry} keyword. - * + * * @return * @since 1.8 */ @@ -646,13 +646,13 @@ public class QueryMapper { } /** - * Returns wheter the current keyword indicates a sample object. - * + * Returns whether the current keyword indicates a {@link Example} object. + * * @return * @since 1.8 */ public boolean isSample() { - return "$sample".equalsIgnoreCase(key); + return "$example".equalsIgnoreCase(key); } public boolean hasIterableValue() { @@ -671,7 +671,7 @@ public class QueryMapper { /** * Value object to represent a field and its meta-information. - * + * * @author Oliver Gierke */ protected static class Field { @@ -682,7 +682,7 @@ public class QueryMapper { /** * Creates a new {@link DocumentField} without meta-information but the given name. - * + * * @param name must not be {@literal null} or empty. */ public Field(String name) { @@ -693,7 +693,7 @@ public class QueryMapper { /** * Returns a new {@link DocumentField} with the given name. - * + * * @param name must not be {@literal null} or empty. * @return */ @@ -703,7 +703,7 @@ public class QueryMapper { /** * Returns whether the current field is the id field. - * + * * @return */ public boolean isIdField() { @@ -714,7 +714,7 @@ public class QueryMapper { * Returns the underlying {@link MongoPersistentProperty} backing the field. For path traversals this will be the * property that represents the value to handle. This means it'll be the leaf property for plain paths or the * association property in case we refer to an association somewhere in the path. - * + * * @return */ public MongoPersistentProperty getProperty() { @@ -723,7 +723,7 @@ public class QueryMapper { /** * Returns the {@link MongoPersistentEntity} that field is conatined in. - * + * * @return */ public MongoPersistentEntity getPropertyEntity() { @@ -732,7 +732,7 @@ public class QueryMapper { /** * Returns whether the field represents an association. - * + * * @return */ public boolean isAssociation() { @@ -741,7 +741,7 @@ public class QueryMapper { /** * Returns the key to be used in the mapped document eventually. - * + * * @return */ public String getMappedKey() { @@ -750,7 +750,7 @@ public class QueryMapper { /** * Returns whether the field references an association in case it refers to a nested field. - * + * * @return */ public boolean containsAssociation() { @@ -768,7 +768,7 @@ public class QueryMapper { /** * Extension of {@link DocumentField} to be backed with mapping metadata. - * + * * @author Oliver Gierke * @author Thomas Darimont */ @@ -785,7 +785,7 @@ public class QueryMapper { /** * Creates a new {@link MetadataBackedField} with the given name, {@link MongoPersistentEntity} and * {@link MappingContext}. - * + * * @param name must not be {@literal null} or empty. * @param entity must not be {@literal null}. * @param context must not be {@literal null}. @@ -798,7 +798,7 @@ public class QueryMapper { /** * Creates a new {@link MetadataBackedField} with the given name, {@link MongoPersistentEntity} and * {@link MappingContext} with the given {@link MongoPersistentProperty}. - * + * * @param name must not be {@literal null} or empty. * @param entity must not be {@literal null}. * @param context must not be {@literal null}. @@ -841,7 +841,7 @@ public class QueryMapper { .orElseGet(() -> DEFAULT_ID_NAMES.contains(name)); } - /* + /* * (non-Javadoc) * @see org.springframework.data.mongodb.core.convert.QueryMapper.Field#getProperty() */ @@ -850,7 +850,7 @@ public class QueryMapper { return association == null ? property : association.getInverse(); } - /* + /* * (non-Javadoc) * @see org.springframework.data.mongodb.core.convert.QueryMapper.Field#getEntity() */ @@ -860,7 +860,7 @@ public class QueryMapper { return property == null ? null : mappingContext.getPersistentEntity(property).orElse(null); } - /* + /* * (non-Javadoc) * @see org.springframework.data.mongodb.core.convert.QueryMapper.Field#isAssociation() */ @@ -869,7 +869,7 @@ public class QueryMapper { return association != null; } - /* + /* * (non-Javadoc) * @see org.springframework.data.mongodb.core.convert.QueryMapper.Field#getAssociation() */ @@ -880,7 +880,7 @@ public class QueryMapper { /** * Finds the association property in the {@link PersistentPropertyPath}. - * + * * @return */ private final Association findAssociation() { @@ -914,7 +914,7 @@ public class QueryMapper { /** * Returns the {@link PersistentPropertyPath} for the given pathExpression. - * + * * @param pathExpression * @return */ @@ -951,7 +951,7 @@ public class QueryMapper { /** * Return the {@link Converter} to be used to created the mapped key. Default implementation will use * {@link PropertyToFieldNameConverter}. - * + * * @return */ protected Converter getPropertyConverter() { @@ -961,7 +961,7 @@ public class QueryMapper { /** * Return the {@link Converter} to use for creating the mapped key of an association. Default implementation is * {@link AssociationConverter}. - * + * * @return * @since 1.7 */ @@ -991,7 +991,7 @@ public class QueryMapper { } } - /* + /* * (non-Javadoc) * @see org.springframework.data.mongodb.core.convert.QueryMapper.Field#getTypeHint() */ @@ -1028,7 +1028,7 @@ public class QueryMapper { /** * Maps the property name while retaining potential positional operator {@literal $}. - * + * * @param property * @return */ @@ -1070,7 +1070,7 @@ public class QueryMapper { /** * Converter to skip all properties after an association property was rendered. - * + * * @author Oliver Gierke */ protected static class AssociationConverter implements Converter { @@ -1080,7 +1080,7 @@ public class QueryMapper { /** * Creates a new {@link AssociationConverter} for the given {@link Association}. - * + * * @param association must not be {@literal null}. */ public AssociationConverter(Association association) { @@ -1089,7 +1089,7 @@ public class QueryMapper { this.property = association.getInverse(); } - /* + /* * (non-Javadoc) * @see org.springframework.core.convert.converter.Converter#convert(java.lang.Object) */ diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Criteria.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Criteria.java index 27f5856ce..1406de996 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Criteria.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Criteria.java @@ -45,7 +45,7 @@ import com.mongodb.BasicDBList; /** * Central class for creating queries. It follows a fluent API style so that you can easily chain together multiple * criteria. Static import of the 'Criteria.where' method will improve readability. - * + * * @author Thomas Risberg * @author Oliver Gierke * @author Thomas Darimont @@ -82,7 +82,7 @@ public class Criteria implements CriteriaDefinition { /** * Static factory method to create a Criteria using the provided key - * + * * @param key * @return */ @@ -92,7 +92,7 @@ public class Criteria implements CriteriaDefinition { /** * Static factory method to create a {@link Criteria} matching an example object. - * + * * @param example must not be {@literal null}. * @return * @see Criteria#alike(Example) @@ -104,7 +104,7 @@ public class Criteria implements CriteriaDefinition { /** * Static factory method to create a {@link Criteria} matching an example object. - * + * * @param example must not be {@literal null}. * @return * @see Criteria#alike(Example) @@ -116,7 +116,7 @@ public class Criteria implements CriteriaDefinition { /** * Static factory method to create a Criteria using the provided key - * + * * @return */ public Criteria and(String key) { @@ -125,7 +125,7 @@ public class Criteria implements CriteriaDefinition { /** * Creates a criterion using equality - * + * * @param o * @return */ @@ -381,7 +381,7 @@ public class Criteria implements CriteriaDefinition { /** * Syntactical sugar for {@link #is(Object)} making obvious that we create a regex predicate. - * + * * @param pattern * @return */ @@ -420,8 +420,10 @@ public class Criteria implements CriteriaDefinition { * * @param circle must not be {@literal null} * @return - * @see MongoDB Query operator: $geoWithin - * @see MongoDB Query operator: $centerSphere + * @see MongoDB Query operator: + * $geoWithin + * @see MongoDB Query operator: + * $centerSphere */ public Criteria withinSphere(Circle circle) { @@ -436,7 +438,8 @@ public class Criteria implements CriteriaDefinition { * * @param shape * @return - * @see MongoDB Query operator: $geoWithin + * @see MongoDB Query operator: + * $geoWithin */ public Criteria within(Shape shape) { @@ -467,7 +470,8 @@ public class Criteria implements CriteriaDefinition { * * @param point must not be {@literal null} * @return - * @see MongoDB Query operator: $nearSphere + * @see MongoDB Query operator: + * $nearSphere */ public Criteria nearSphere(Point point) { @@ -480,7 +484,7 @@ public class Criteria implements CriteriaDefinition { /** * Creates criterion using {@code $geoIntersects} operator which matches intersections of the given {@code geoJson} * structure and the documents one. Requires MongoDB 2.4 or better. - * + * * @param geoJson must not be {@literal null}. * @return * @since 1.8 @@ -498,7 +502,8 @@ public class Criteria implements CriteriaDefinition { * * @param maxDistance * @return - * @see MongoDB Query operator: $maxDistance + * @see MongoDB Query operator: + * $maxDistance */ public Criteria maxDistance(double maxDistance) { @@ -514,7 +519,7 @@ public class Criteria implements CriteriaDefinition { /** * Creates a geospatial criterion using a {@literal $minDistance} operation, for use with {@literal $near} or * {@literal $nearSphere}. - * + * * @param minDistance * @return * @since 1.7 @@ -535,7 +540,8 @@ public class Criteria implements CriteriaDefinition { * * @param c * @return - * @see MongoDB Query operator: $elemMatch + * @see MongoDB Query operator: + * $elemMatch */ public Criteria elemMatch(Criteria c) { criteria.put("$elemMatch", c.getCriteriaObject()); @@ -544,14 +550,14 @@ public class Criteria implements CriteriaDefinition { /** * Creates a criterion using the given object as a pattern. - * + * * @param sample * @return * @since 1.8 */ public Criteria alike(Example sample) { - criteria.put("$sample", sample); + criteria.put("$example", sample); this.criteriaChain.add(this); return this; } @@ -561,7 +567,7 @@ public class Criteria implements CriteriaDefinition { *

* Note that mongodb doesn't support an $or operator to be wrapped in a $not operator. *

- * + * * @throws IllegalArgumentException if {@link #orOperator(Criteria...)} follows a not() call directly. * @param criteria */ @@ -575,7 +581,7 @@ public class Criteria implements CriteriaDefinition { *

* Note that mongodb doesn't support an $nor operator to be wrapped in a $not operator. *

- * + * * @throws IllegalArgumentException if {@link #norOperator(Criteria...)} follows a not() call directly. * @param criteria */ @@ -589,7 +595,7 @@ public class Criteria implements CriteriaDefinition { *

* Note that mongodb doesn't support an $and operator to be wrapped in a $not operator. *

- * + * * @throws IllegalArgumentException if {@link #andOperator(Criteria...)} follows a not() call directly. * @param criteria */ @@ -771,7 +777,7 @@ public class Criteria implements CriteriaDefinition { /** * Checks the given objects for equality. Handles {@link Pattern} and arrays correctly. - * + * * @param left * @param right * @return 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 d979d1115..9d8e1c1ca 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 @@ -1612,6 +1612,24 @@ public class AggregationTests { skip(100L)); } + @Test // DATAMONGO-1325 + public void shouldApplySampleCorrectly() { + + assumeTrue(mongoVersion.isGreaterThanOrEqualTo(THREE_DOT_TWO)); + + createUserWithLikesDocuments(); + + TypedAggregation agg = newAggregation(UserWithLikes.class, // + unwind("likes"), // + sample(3) // + ); + + assertThat(agg.toString(), is(notNullValue())); + + AggregationResults result = mongoTemplate.aggregate(agg, LikeStats.class); + assertThat(result.getMappedResults().size(), is(3)); + } + @Test // DATAMONGO-1457 public void sliceShouldBeAppliedCorrectly() { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/SampleOperationUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/SampleOperationUnitTests.java index 394f53550..b6233b76c 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/SampleOperationUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/SampleOperationUnitTests.java @@ -31,24 +31,29 @@ public class SampleOperationUnitTests { private static final String SIZE = "size"; private static final String OP = "$sample"; - @Test(expected = IllegalArgumentException.class) + @Test(expected = IllegalArgumentException.class) // DATAMONGO-1325 public void rejectsNegativeSample() { new SampleOperation(-1L); } - @Test(expected = IllegalArgumentException.class) + @Test(expected = IllegalArgumentException.class) // DATAMONGO-1325 public void rejectsZeroSample() { new SampleOperation(0L); } - @Test + @Test // DATAMONGO-1325 public void rendersSampleOperation() { + long sampleSize = 5L; + SampleOperation sampleOperation = Aggregation.sample(sampleSize); + Document sampleOperationDocument = sampleOperation.toDocument(Aggregation.DEFAULT_CONTEXT); + assertNotNull(sampleOperationDocument.get(OP)); assertThat(sampleOperationDocument.get(OP), is(instanceOf(Document.class))); - Document sampleSizeDocument = sampleOperationDocument.get(OP, Document.class); + + Document sampleSizeDocument = sampleOperationDocument.get(OP, Document.class); assertEquals(sampleSize, sampleSizeDocument.get(SIZE)); } }