diff --git a/pom.xml b/pom.xml index 15b2d67f47..795669a69e 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-mongodb-parent - 2.3.0.BUILD-SNAPSHOT + 2.3.0.DATAMONGO-2390-SNAPSHOT pom Spring Data MongoDB diff --git a/spring-data-mongodb-benchmarks/pom.xml b/spring-data-mongodb-benchmarks/pom.xml index c4766040c1..3e792bdd8e 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 - 2.3.0.BUILD-SNAPSHOT + 2.3.0.DATAMONGO-2390-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb-distribution/pom.xml b/spring-data-mongodb-distribution/pom.xml index ed39c63e76..5e77abc0b6 100644 --- a/spring-data-mongodb-distribution/pom.xml +++ b/spring-data-mongodb-distribution/pom.xml @@ -14,7 +14,7 @@ org.springframework.data spring-data-mongodb-parent - 2.3.0.BUILD-SNAPSHOT + 2.3.0.DATAMONGO-2390-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/pom.xml b/spring-data-mongodb/pom.xml index 25cf02b5d5..b0f9c5be22 100644 --- a/spring-data-mongodb/pom.xml +++ b/spring-data-mongodb/pom.xml @@ -11,7 +11,7 @@ org.springframework.data spring-data-mongodb-parent - 2.3.0.BUILD-SNAPSHOT + 2.3.0.DATAMONGO-2390-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java index 79e1d603ac..569112cf10 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java @@ -2190,6 +2190,10 @@ protected AggregationResults doAggregate(Aggregation aggregation, String options.getComment().ifPresent(aggregateIterable::comment); + if(options.hasExecutionTimeLimit()) { + aggregateIterable = aggregateIterable.maxTime(options.getMaxTime().toMillis(), TimeUnit.MILLISECONDS); + } + MongoIterable iterable = aggregateIterable.map(val -> { rawResult.add(val); diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java index 0dcab3e87b..b7cbe483d4 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java @@ -1073,6 +1073,10 @@ private Flux aggregateAndMap(MongoCollection collection, List cursor; private final Optional collation; private final Optional comment; + private Duration maxTime = Duration.ZERO; /** * Creates a new {@link AggregationOptions}. @@ -129,7 +132,11 @@ public static AggregationOptions fromDocument(Document document) { : null; String comment = document.getString(COMMENT); - return new AggregationOptions(allowDiskUse, explain, cursor, collation, comment); + AggregationOptions options = new AggregationOptions(allowDiskUse, explain, cursor, collation, comment); + if (document.containsKey(MAX_TIME)) { + options.maxTime = Duration.ofMillis(document.getLong(MAX_TIME)); + } + return options; } /** @@ -206,6 +213,14 @@ public Optional getComment() { return comment; } + /** + * @return the time limit for processing. {@link Duration#ZERO} is used for the default unbounded behavior. + * @since 2.3 + */ + public Duration getMaxTime() { + return maxTime; + } + /** * Returns a new potentially adjusted copy for the given {@code aggregationCommandObject} with the configuration * applied. @@ -233,6 +248,10 @@ Document applyAndReturnPotentiallyChangedCommand(Document command) { collation.map(Collation::toDocument).ifPresent(val -> result.append(COLLATION, val)); } + if (hasExecutionTimeLimit() && !result.containsKey(MAX_TIME)) { + result.append(MAX_TIME, maxTime.toMillis()); + } + return result; } @@ -251,9 +270,17 @@ public Document toDocument() { collation.ifPresent(val -> document.append(COLLATION, val.toDocument())); comment.ifPresent(val -> document.append(COMMENT, val)); + if (hasExecutionTimeLimit()) { + document.append(MAX_TIME, maxTime.toMillis()); + } + return document; } + public boolean hasExecutionTimeLimit() { + return !maxTime.isZero() && !maxTime.isNegative(); + } + /* (non-Javadoc) * @see java.lang.Object#toString() */ @@ -279,6 +306,7 @@ public static class Builder { private @Nullable Document cursor; private @Nullable Collation collation; private @Nullable String comment; + private @Nullable Duration maxTime; /** * Defines whether to off-load intensive sort-operations to disk. @@ -355,13 +383,33 @@ public Builder comment(@Nullable String comment) { return this; } + /** + * Set the time limit for processing. + * + * @param maxTime {@link Duration#ZERO} is used for the default unbounded behavior. {@link Duration#isNegative() + * Negative} values will be ignored. + * @return this. + * @sinve 2.3 + */ + public Builder maxTime(@Nullable Duration maxTime) { + + this.maxTime = maxTime; + return this; + } + /** * Returns a new {@link AggregationOptions} instance with the given configuration. * * @return */ public AggregationOptions build() { - return new AggregationOptions(allowDiskUse, explain, cursor, collation, comment); + + AggregationOptions options = new AggregationOptions(allowDiskUse, explain, cursor, collation, comment); + if (maxTime != null) { + options.maxTime = maxTime; + } + + return options; } } } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoOperationsUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoOperationsUnitTests.java index 0a2cba248a..69cc26f8ae 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoOperationsUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoOperationsUnitTests.java @@ -22,11 +22,11 @@ import org.bson.Document; import org.bson.conversions.Bson; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.dao.DataAccessException; import org.springframework.data.geo.Point; import org.springframework.data.mapping.MappingException; @@ -49,7 +49,7 @@ * @author Thomas Darimont * @author Christoph Strobl */ -@RunWith(MockitoJUnitRunner.class) +@ExtendWith(MockitoExtension.class) public abstract class MongoOperationsUnitTests { @Mock CollectionCallback collectionCallback; @@ -59,7 +59,7 @@ public abstract class MongoOperationsUnitTests { Person person; List persons; - @Before + @BeforeEach public final void operationsSetUp() { person = new Person("Oliver"); diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateCollationTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateCollationTests.java index 70bf70777c..8950571c0c 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateCollationTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateCollationTests.java @@ -60,6 +60,11 @@ public MongoClient mongoClient() { protected String getDatabaseName() { return "collation-tests"; } + + @Override + protected boolean autoIndexCreation() { + return false; + } } @Autowired MongoTemplate template; diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTransactionTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTransactionTests.java index 92654379a4..da8ab558ee 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTransactionTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTransactionTests.java @@ -85,6 +85,11 @@ protected String getDatabaseName() { return DB_NAME; } + @Override + protected boolean autoIndexCreation() { + return false; + } + @Bean MongoTransactionManager txManager(MongoDbFactory dbFactory) { return new MongoTransactionManager(dbFactory); diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java index eed5cbd9df..0b7c37562e 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java @@ -22,6 +22,7 @@ import lombok.Data; import java.math.BigInteger; +import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -29,21 +30,22 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.concurrent.TimeUnit; import java.util.regex.Pattern; import org.assertj.core.api.Assertions; import org.bson.Document; import org.bson.conversions.Bson; import org.bson.types.ObjectId; -import org.junit.Before; -import org.junit.Ignore; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatcher; import org.mockito.Mock; import org.mockito.Mockito; -import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.junit.jupiter.MockitoSettings; +import org.mockito.quality.Strictness; import org.springframework.beans.factory.annotation.Value; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationListener; @@ -120,7 +122,7 @@ * @author Mark Paluch * @author Michael J. Simons */ -@RunWith(MockitoJUnitRunner.class) +@MockitoSettings(strictness = Strictness.LENIENT) public class MongoTemplateUnitTests extends MongoOperationsUnitTests { MongoTemplate template; @@ -144,8 +146,8 @@ public class MongoTemplateUnitTests extends MongoOperationsUnitTests { MappingMongoConverter converter; MongoMappingContext mappingContext; - @Before - public void setUp() { + @BeforeEach + public void beforeEach() { when(findIterable.iterator()).thenReturn(cursor); when(factory.getDb()).thenReturn(db); @@ -177,6 +179,7 @@ public void setUp() { when(aggregateIterable.allowDiskUse(any())).thenReturn(aggregateIterable); when(aggregateIterable.batchSize(anyInt())).thenReturn(aggregateIterable); when(aggregateIterable.map(any())).thenReturn(aggregateIterable); + when(aggregateIterable.maxTime(anyLong(), any())).thenReturn(aggregateIterable); when(aggregateIterable.into(any())).thenReturn(Collections.emptyList()); when(distinctIterable.collation(any())).thenReturn(distinctIterable); when(distinctIterable.map(any())).thenReturn(distinctIterable); @@ -206,34 +209,36 @@ public void rejectsNullMongoClient() { .isThrownBy(() -> new MongoTemplate((com.mongodb.client.MongoClient) null, "database")); } - @Test(expected = IllegalArgumentException.class) // DATAMONGO-1870 - public void removeHandlesMongoExceptionProperly() throws Exception { + @Test // DATAMONGO-1870 + public void removeHandlesMongoExceptionProperly() { MongoTemplate template = mockOutGetDb(); - template.remove(null, "collection"); + assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> template.remove(null, "collection")); } @Test - public void defaultsConverterToMappingMongoConverter() throws Exception { + public void defaultsConverterToMappingMongoConverter() { MongoTemplate template = new MongoTemplate(mongo, "database"); assertThat(ReflectionTestUtils.getField(template, "mongoConverter") instanceof MappingMongoConverter).isTrue(); } - @Test(expected = InvalidDataAccessApiUsageException.class) + @Test public void rejectsNotFoundMapReduceResource() { GenericApplicationContext ctx = new GenericApplicationContext(); ctx.refresh(); template.setApplicationContext(ctx); - template.mapReduce("foo", "classpath:doesNotExist.js", "function() {}", Person.class); + + assertThatExceptionOfType(InvalidDataAccessApiUsageException.class) + .isThrownBy(() -> template.mapReduce("foo", "classpath:doesNotExist.js", "function() {}", Person.class)); } - @Test(expected = InvalidDataAccessApiUsageException.class) // DATAMONGO-322 + @Test // DATAMONGO-322 public void rejectsEntityWithNullIdIfNotSupportedIdType() { Object entity = new NotAutogenerateableId(); - template.save(entity); + assertThatExceptionOfType(InvalidDataAccessApiUsageException.class).isThrownBy(() -> template.save(entity)); } @Test // DATAMONGO-322 @@ -463,7 +468,7 @@ public void geoNearShouldIgnoreReadPreferenceWhenNotSet() { } @Test // DATAMONGO-1334 - @Ignore("TODO: mongo3 - a bit hard to tests with the immutable object stuff") + @Disabled("TODO: mongo3 - a bit hard to tests with the immutable object stuff") public void mapReduceShouldUseZeroAsDefaultLimit() { MongoCursor cursor = mock(MongoCursor.class); @@ -1261,6 +1266,27 @@ public void aggreateStreamShouldUseCollationFromOptionsEvenIfDefaultCollationIsP verify(aggregateIterable).collation(eq(com.mongodb.client.model.Collation.builder().locale("fr").build())); } + @Test // DATAMONGO-2390 + public void aggregateShouldNoApplyZeroOrNegativeMaxTime() { + + template.aggregate( + newAggregation(Sith.class, project("id")).withOptions(newAggregationOptions().maxTime(Duration.ZERO).build()), + AutogenerateableId.class, Document.class); + template.aggregate(newAggregation(Sith.class, project("id")).withOptions( + newAggregationOptions().maxTime(Duration.ofSeconds(-1)).build()), AutogenerateableId.class, Document.class); + + verify(aggregateIterable, never()).maxTime(anyLong(), any()); + } + + @Test // DATAMONGO-2390 + public void aggregateShouldApplyMaxTimeIfSet() { + + template.aggregate(newAggregation(Sith.class, project("id")).withOptions( + newAggregationOptions().maxTime(Duration.ofSeconds(10)).build()), AutogenerateableId.class, Document.class); + + verify(aggregateIterable).maxTime(eq(10000L), eq(TimeUnit.MILLISECONDS)); + } + @Test // DATAMONGO-1854 public void findAndReplaceShouldUseCollationWhenPresent() { @@ -1299,7 +1325,7 @@ public void findAndReplaceShouldUseDefaultCollationWhenPresent() { assertThat(options.getValue().getCollation().getLocale()).isEqualTo("de_AT"); } - @Test // DATAMONGO-18545 + @Test // DATAMONGO-1854 public void findAndReplaceShouldUseCollationEvenIfDefaultCollationIsPresent() { template.findAndReplace(new BasicQuery("{}").collation(Collation.of("fr")), new Sith()); @@ -1798,7 +1824,7 @@ private MongoTemplate mockOutGetDb() { @Override protected MongoOperations getOperationsForExceptionHandling() { MongoTemplate template = spy(this.template); - when(template.getDb()).thenThrow(new MongoException("Error!")); + lenient().when(template.getDb()).thenThrow(new MongoException("Error!")); return template; } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateValidationTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateValidationTests.java index d0b68acf2e..1983634d06 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateValidationTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateValidationTests.java @@ -74,6 +74,11 @@ public MongoClient mongoClient() { protected String getDatabaseName() { return "validation-tests"; } + + @Override + protected boolean autoIndexCreation() { + return false; + } } @Autowired MongoTemplate template; diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/NoExplicitIdTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/NoExplicitIdTests.java index 19a373068e..5e74309c05 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/NoExplicitIdTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/NoExplicitIdTests.java @@ -59,6 +59,11 @@ protected String getDatabaseName() { public MongoClient mongoClient() { return MongoTestUtils.client(); } + + @Override + protected boolean autoIndexCreation() { + return false; + } } @Autowired MongoOperations mongoOps; diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateUnitTests.java index 2daed014b9..1953178935 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateUnitTests.java @@ -23,22 +23,26 @@ import reactor.core.publisher.Mono; import reactor.test.StepVerifier; +import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.TimeUnit; import org.assertj.core.api.Assertions; import org.bson.Document; import org.bson.conversions.Bson; import org.bson.types.ObjectId; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.ArgumentCaptor; import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.junit.jupiter.MockitoSettings; +import org.mockito.quality.Strictness; import org.reactivestreams.Publisher; import org.springframework.beans.factory.annotation.Value; import org.springframework.context.ApplicationContext; @@ -89,7 +93,8 @@ * @author Mark Paluch * @author Christoph Strobl */ -@RunWith(MockitoJUnitRunner.Silent.class) +@ExtendWith(MockitoExtension.class) +@MockitoSettings(strictness = Strictness.LENIENT) public class ReactiveMongoTemplateUnitTests { ReactiveMongoTemplate template; @@ -112,8 +117,8 @@ public class ReactiveMongoTemplateUnitTests { MappingMongoConverter converter; MongoMappingContext mappingContext; - @Before - public void setUp() { + @BeforeEach + public void beforeEach() { when(factory.getExceptionTranslator()).thenReturn(exceptionTranslator); when(factory.getMongoDatabase()).thenReturn(db); @@ -147,6 +152,7 @@ public void setUp() { when(findPublisher.first()).thenReturn(findPublisher); when(aggregatePublisher.allowDiskUse(anyBoolean())).thenReturn(aggregatePublisher); when(aggregatePublisher.collation(any())).thenReturn(aggregatePublisher); + when(aggregatePublisher.maxTime(anyLong(), any())).thenReturn(aggregatePublisher); when(aggregatePublisher.first()).thenReturn(findPublisher); this.mappingContext = new MongoMappingContext(); @@ -582,7 +588,37 @@ public void aggregateShouldHonorOptionsComment() { verify(aggregatePublisher).comment("expensive"); } - @Test // DATAMONGO-18545 + @Test // DATAMONGO-2390 + public void aggregateShouldNoApplyZeroOrNegativeMaxTime() { + + template + .aggregate(newAggregation(MongoTemplateUnitTests.Sith.class, project("id")).withOptions( + newAggregationOptions().maxTime(Duration.ZERO).build()), AutogenerateableId.class, Document.class) + .subscribe(); + template + .aggregate( + newAggregation(MongoTemplateUnitTests.Sith.class, project("id")) + .withOptions(newAggregationOptions().maxTime(Duration.ofSeconds(-1)).build()), + AutogenerateableId.class, Document.class) + .subscribe(); + + verify(aggregatePublisher, never()).maxTime(anyLong(), any()); + } + + @Test // DATAMONGO-2390 + public void aggregateShouldApplyMaxTimeIfSet() { + + template + .aggregate( + newAggregation(MongoTemplateUnitTests.Sith.class, project("id")) + .withOptions(newAggregationOptions().maxTime(Duration.ofSeconds(10)).build()), + AutogenerateableId.class, Document.class) + .subscribe(); + + verify(aggregatePublisher).maxTime(eq(10000L), eq(TimeUnit.MILLISECONDS)); + } + + @Test // DATAMONGO-1854 public void findAndReplaceShouldUseCollationWhenPresent() { template.findAndReplace(new BasicQuery("{}").collation(Collation.of("fr")), new Jedi()).subscribe(); @@ -593,7 +629,7 @@ public void findAndReplaceShouldUseCollationWhenPresent() { assertThat(options.getValue().getCollation().getLocale()).isEqualTo("fr"); } - @Test // DATAMONGO-18545 + @Test // DATAMONGO-1854 public void findAndReplaceShouldUseDefaultCollationWhenPresent() { template.findAndReplace(new BasicQuery("{}"), new Sith()).subscribe(); @@ -604,7 +640,7 @@ public void findAndReplaceShouldUseDefaultCollationWhenPresent() { assertThat(options.getValue().getCollation().getLocale()).isEqualTo("de_AT"); } - @Test // DATAMONGO-18545 + @Test // DATAMONGO-1854 public void findAndReplaceShouldUseCollationEvenIfDefaultCollationIsPresent() { template.findAndReplace(new BasicQuery("{}").collation(Collation.of("fr")), new MongoTemplateUnitTests.Sith())