DATAMONGO-1682 - Polishing.

mp911de · mp911de · commit e4a982ab59dc · 2017-07-05T10:36:46.000+02:00
Require non-null arguments in DefaultReactiveIndexOperations constructor. Remove superfluous publisher creation indirections. Use StepVerifier.verifyComplete() to verify the step sequence.

Use provided entity type in template API to construct index operations.
diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/DefaultIndexOperations.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/DefaultIndexOperations.java
@@ -23,8 +23,8 @@
 
 import org.bson.Document;
 import org.springframework.dao.DataAccessException;
-import org.springframework.data.mongodb.core.convert.QueryMapper;
 import org.springframework.data.mongodb.MongoDbFactory;
+import org.springframework.data.mongodb.core.convert.QueryMapper;
 import org.springframework.data.mongodb.core.index.IndexDefinition;
 import org.springframework.data.mongodb.core.index.IndexInfo;
 import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity;
@@ -37,7 +37,7 @@
 
 /**
  * Default implementation of {@link IndexOperations}.
- * 
+ *
  * @author Mark Pollack
  * @author Oliver Gierke
  * @author Komi Innocent
@@ -55,7 +55,7 @@ public class DefaultIndexOperations implements IndexOperations {
 
 	/**
 	 * Creates a new {@link DefaultIndexOperations}.
-	 * 
+	 *
 	 * @param mongoDbFactory must not be {@literal null}.
 	 * @param collectionName must not be {@literal null}.
 	 * @param queryMapper must not be {@literal null}.
@@ -97,24 +97,22 @@ public String ensureIndex(final IndexDefinition indexDefinition) {
 
 			Document indexOptions = indexDefinition.getIndexOptions();
 
-			if (indexOptions != null) {
-
-				IndexOptions ops = IndexConverters.indexDefinitionToIndexOptionsConverter().convert(indexDefinition);
+			if (indexOptions == null) {
+				return collection.createIndex(indexDefinition.getIndexKeys());
+			}
 
-				if (indexOptions.containsKey(PARTIAL_FILTER_EXPRESSION_KEY)) {
+			IndexOptions ops = IndexConverters.indexDefinitionToIndexOptionsConverter().convert(indexDefinition);
 
-					Assert.isInstanceOf(Document.class, indexOptions.get(PARTIAL_FILTER_EXPRESSION_KEY));
+			if (indexOptions.containsKey(PARTIAL_FILTER_EXPRESSION_KEY)) {
 
-					ops.partialFilterExpression( mapper.getMappedObject(
-							(Document) indexOptions.get(PARTIAL_FILTER_EXPRESSION_KEY), lookupPersistentEntity(type, collectionName)));
-				}
+				Assert.isInstanceOf(Document.class, indexOptions.get(PARTIAL_FILTER_EXPRESSION_KEY));
 
-				return collection.createIndex(indexDefinition.getIndexKeys(), ops);
+				ops.partialFilterExpression(mapper.getMappedObject((Document) indexOptions.get(PARTIAL_FILTER_EXPRESSION_KEY),
+						lookupPersistentEntity(type, collectionName)));
 			}
-			return collection.createIndex(indexDefinition.getIndexKeys());
-		}
 
-		);
+			return collection.createIndex(indexDefinition.getIndexKeys(), ops);
+		});
 	}
 
 	private MongoPersistentEntity<?> lookupPersistentEntity(Class<?> entityType, String collection) {
diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/DefaultReactiveIndexOperations.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/DefaultReactiveIndexOperations.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2016 the original author or authors.
+ * Copyright 2016-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.
@@ -29,10 +29,9 @@
 import org.springframework.util.Assert;
 
 import com.mongodb.client.model.IndexOptions;
-import com.mongodb.reactivestreams.client.ListIndexesPublisher;
 
 /**
- * Default implementation of {@link IndexOperations}.
+ * Default implementation of {@link ReactiveIndexOperations}.
  *
  * @author Mark Paluch
  * @author Christoph Strobl
@@ -52,21 +51,28 @@ public class DefaultReactiveIndexOperations implements ReactiveIndexOperations {
 	 *
 	 * @param mongoOperations must not be {@literal null}.
 	 * @param collectionName must not be {@literal null}.
+	 * @param queryMapper must not be {@literal null}.
 	 */
 	public DefaultReactiveIndexOperations(ReactiveMongoOperations mongoOperations, String collectionName,
 			QueryMapper queryMapper) {
-
-		this(mongoOperations, collectionName, queryMapper, null);
+		this(mongoOperations, collectionName, queryMapper, Optional.empty());
 	}
 
 	/**
 	 * Creates a new {@link DefaultReactiveIndexOperations}.
 	 *
 	 * @param mongoOperations must not be {@literal null}.
 	 * @param collectionName must not be {@literal null}.
+	 * @param queryMapper must not be {@literal null}.
+	 * @param type used for mapping potential partial index filter expression, must not be {@literal null}.
 	 */
 	public DefaultReactiveIndexOperations(ReactiveMongoOperations mongoOperations, String collectionName,
 			QueryMapper queryMapper, Class<?> type) {
+		this(mongoOperations, collectionName, queryMapper, Optional.of(type));
+	}
+
+	private DefaultReactiveIndexOperations(ReactiveMongoOperations mongoOperations, String collectionName,
+			QueryMapper queryMapper, Optional<Class<?>> type) {
 
 		Assert.notNull(mongoOperations, "ReactiveMongoOperations must not be null!");
 		Assert.notNull(collectionName, "Collection must not be null!");
@@ -75,7 +81,7 @@ public DefaultReactiveIndexOperations(ReactiveMongoOperations mongoOperations, S
 		this.mongoOperations = mongoOperations;
 		this.collectionName = collectionName;
 		this.queryMapper = queryMapper;
-		this.type = Optional.ofNullable(type);
+		this.type = type;
 	}
 
 	/* (non-Javadoc)
@@ -87,51 +93,44 @@ public Mono<String> ensureIndex(final IndexDefinition indexDefinition) {
 
 			Document indexOptions = indexDefinition.getIndexOptions();
 
-			if (indexOptions != null) {
-
-				IndexOptions ops = IndexConverters.indexDefinitionToIndexOptionsConverter().convert(indexDefinition);
+			if (indexOptions == null) {
+				return collection.createIndex(indexDefinition.getIndexKeys());
+			}
 
-				if (indexOptions.containsKey(PARTIAL_FILTER_EXPRESSION_KEY)) {
+			IndexOptions ops = IndexConverters.indexDefinitionToIndexOptionsConverter().convert(indexDefinition);
 
-					Assert.isInstanceOf(Document.class, indexOptions.get(PARTIAL_FILTER_EXPRESSION_KEY));
+			if (indexOptions.containsKey(PARTIAL_FILTER_EXPRESSION_KEY)) {
 
-					MongoPersistentEntity<?> entity = type
-							.map(val -> (MongoPersistentEntity) queryMapper.getMappingContext().getRequiredPersistentEntity(val))
-							.orElseGet(() -> lookupPersistentEntity(collectionName));
+				Assert.isInstanceOf(Document.class, indexOptions.get(PARTIAL_FILTER_EXPRESSION_KEY));
 
-					ops = ops.partialFilterExpression(
-							queryMapper.getMappedObject((Document) indexOptions.get(PARTIAL_FILTER_EXPRESSION_KEY), entity));
-				}
+				MongoPersistentEntity<?> entity = type
+						.map(val -> (MongoPersistentEntity) queryMapper.getMappingContext().getRequiredPersistentEntity(val))
+						.orElseGet(() -> lookupPersistentEntity(collectionName));
 
-				return collection.createIndex(indexDefinition.getIndexKeys(), ops);
+				ops = ops.partialFilterExpression(
+						queryMapper.getMappedObject(indexOptions.get(PARTIAL_FILTER_EXPRESSION_KEY, Document.class), entity));
 			}
 
-			return collection.createIndex(indexDefinition.getIndexKeys());
+			return collection.createIndex(indexDefinition.getIndexKeys(), ops);
+
 		}).next();
 	}
 
 	private MongoPersistentEntity<?> lookupPersistentEntity(String collection) {
 
 		Collection<? extends MongoPersistentEntity<?>> entities = queryMapper.getMappingContext().getPersistentEntities();
 
-		for (MongoPersistentEntity<?> entity : entities) {
-			if (entity.getCollection().equals(collection)) {
-				return entity;
-			}
-		}
-
-		return null;
+		return entities.stream() //
+				.filter(entity -> entity.getCollection().equals(collection)) //
+				.findFirst() //
+				.orElse(null);
 	}
 
 	/* (non-Javadoc)
 	 * @see org.springframework.data.mongodb.core.ReactiveIndexOperations#dropIndex(java.lang.String)
 	 */
 	public Mono<Void> dropIndex(final String name) {
-
-		return mongoOperations.execute(collectionName, collection -> {
-
-			return Mono.from(collection.dropIndex(name));
-		}).flatMap(success -> Mono.<Void> empty()).next();
+		return mongoOperations.execute(collectionName, collection -> collection.dropIndex(name)).then();
 	}
 
 	/* (non-Javadoc)
@@ -146,11 +145,8 @@ public Mono<Void> dropAllIndexes() {
 	 */
 	public Flux<IndexInfo> getIndexInfo() {
 
-		return mongoOperations.execute(collectionName, collection -> {
-
-			ListIndexesPublisher<Document> indexesPublisher = collection.listIndexes(Document.class);
-
-			return Flux.from(indexesPublisher).map(IndexConverters.documentToIndexInfoConverter()::convert);
-		});
+		return mongoOperations.execute(collectionName, collection -> collection.listIndexes(Document.class)) //
+				.map(IndexConverters.documentToIndexInfoConverter()::convert);
 	}
+
 }
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
@@ -529,7 +529,8 @@ public IndexOperations indexOps(String collectionName) {
 	}
 
 	public IndexOperations indexOps(Class<?> entityClass) {
-		return new DefaultIndexOperations(getMongoDbFactory(), determineCollectionName(entityClass), queryMapper);
+		return new DefaultIndexOperations(getMongoDbFactory(), determineCollectionName(entityClass), queryMapper,
+				entityClass);
 	}
 
 	public BulkOperations bulkOps(BulkMode bulkMode, String collectionName) {
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
@@ -308,7 +308,8 @@ public ReactiveIndexOperations indexOps(String collectionName) {
 	 * @see org.springframework.data.mongodb.core.ReactiveMongoOperations#reactiveIndexOps(java.lang.Class)
 	 */
 	public ReactiveIndexOperations indexOps(Class<?> entityClass) {
-		return new DefaultReactiveIndexOperations(this, determineCollectionName(entityClass), this.queryMapper);
+		return new DefaultReactiveIndexOperations(this, determineCollectionName(entityClass), this.queryMapper,
+				entityClass);
 	}
 
 	public String getCollectionName(Class<?> entityClass) {
@@ -1914,8 +1915,7 @@ String determineCollectionName(Class<?> entityClass) {
 					"No class parameter provided, entity collection can't be determined!");
 		}
 
-		MongoPersistentEntity<?> entity = mappingContext.getRequiredPersistentEntity(entityClass);
-		return entity.getCollection();
+		return mappingContext.getRequiredPersistentEntity(entityClass).getCollection();
 	}
 
 	private static MappingMongoConverter getDefaultMongoConverter() {
diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/DefaultReactiveIndexOperationsTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/DefaultReactiveIndexOperationsTests.java
@@ -16,12 +16,10 @@
 package org.springframework.data.mongodb.core;
 
 import static org.assertj.core.api.Assertions.*;
-import static org.hamcrest.core.Is.*;
 import static org.junit.Assume.*;
 import static org.springframework.data.mongodb.core.index.PartialIndexFilter.*;
 import static org.springframework.data.mongodb.core.query.Criteria.*;
 
-import reactor.core.publisher.Mono;
 import reactor.test.StepVerifier;
 
 import java.util.function.Predicate;
@@ -86,10 +84,10 @@ public void setUp() {
 		String collectionName = this.template.getCollectionName(DefaultIndexOperationsIntegrationTestsSample.class);
 
 		this.collection = this.template.getMongoDatabase().getCollection(collectionName, Document.class);
-		Mono.from(this.collection.dropIndexes()).subscribe();
-
 		this.indexOps = new DefaultReactiveIndexOperations(template, collectionName,
 				new QueryMapper(template.getConverter()));
+
+		StepVerifier.create(this.collection.dropIndexes()).expectNextCount(1).verifyComplete();
 	}
 
 	private void queryMongoVersionIfNecessary() {
@@ -103,7 +101,7 @@ private void queryMongoVersionIfNecessary() {
 	@Test // DATAMONGO-1518
 	public void shouldCreateIndexWithCollationCorrectly() {
 
-		assumeThat(mongoVersion.isGreaterThanOrEqualTo(THREE_DOT_FOUR), is(true));
+		assumeTrue(mongoVersion.isGreaterThanOrEqualTo(THREE_DOT_FOUR));
 
 		IndexDefinition id = new Index().named("with-collation").on("xyz", Direction.ASC)
 				.collation(Collation.of("de_AT").caseFirst(CaseFirst.off()));
@@ -131,13 +129,13 @@ public void shouldCreateIndexWithCollationCorrectly() {
 
 					assertThat(result).isEqualTo(expected);
 				}) //
-				.thenAwait();
+				.verifyComplete();
 	}
 
 	@Test // DATAMONGO-1682
 	public void shouldApplyPartialFilterCorrectly() {
 
-		assumeThat(mongoVersion.isGreaterThanOrEqualTo(THREE_DOT_TWO), is(true));
+		assumeTrue(mongoVersion.isGreaterThanOrEqualTo(THREE_DOT_TWO));
 
 		IndexDefinition id = new Index().named("partial-with-criteria").on("k3y", Direction.ASC)
 				.partial(of(where("q-t-y").gte(10)));
@@ -148,13 +146,13 @@ public void shouldApplyPartialFilterCorrectly() {
 				.consumeNextWith(indexInfo -> {
 					assertThat(indexInfo.getPartialFilterExpression()).isEqualTo("{ \"q-t-y\" : { \"$gte\" : 10 } }");
 				}) //
-				.thenAwait();
+				.verifyComplete();
 	}
 
 	@Test // DATAMONGO-1682
 	public void shouldApplyPartialFilterWithMappedPropertyCorrectly() {
 
-		assumeThat(mongoVersion.isGreaterThanOrEqualTo(THREE_DOT_TWO), is(true));
+		assumeTrue(mongoVersion.isGreaterThanOrEqualTo(THREE_DOT_TWO));
 
 		IndexDefinition id = new Index().named("partial-with-mapped-criteria").on("k3y", Direction.ASC)
 				.partial(of(where("quantity").gte(10)));
@@ -164,13 +162,13 @@ public void shouldApplyPartialFilterWithMappedPropertyCorrectly() {
 		StepVerifier.create(indexOps.getIndexInfo().filter(this.indexByName("partial-with-mapped-criteria"))) //
 				.consumeNextWith(indexInfo -> {
 					assertThat(indexInfo.getPartialFilterExpression()).isEqualTo("{ \"qty\" : { \"$gte\" : 10 } }");
-				}).thenAwait();
+				}).verifyComplete();
 	}
 
 	@Test // DATAMONGO-1682
 	public void shouldApplyPartialDBOFilterCorrectly() {
 
-		assumeThat(mongoVersion.isGreaterThanOrEqualTo(THREE_DOT_TWO), is(true));
+		assumeTrue(mongoVersion.isGreaterThanOrEqualTo(THREE_DOT_TWO));
 
 		IndexDefinition id = new Index().named("partial-with-dbo").on("k3y", Direction.ASC)
 				.partial(of(new org.bson.Document("qty", new org.bson.Document("$gte", 10))));
@@ -181,14 +179,14 @@ public void shouldApplyPartialDBOFilterCorrectly() {
 				.consumeNextWith(indexInfo -> {
 					assertThat(indexInfo.getPartialFilterExpression()).isEqualTo("{ \"qty\" : { \"$gte\" : 10 } }");
 				}) //
-				.thenAwait();
+				.verifyComplete();
 
 	}
 
 	@Test // DATAMONGO-1682
 	public void shouldFavorExplicitMappingHintViaClass() {
 
-		assumeThat(mongoVersion.isGreaterThanOrEqualTo(THREE_DOT_TWO), is(true));
+		assumeTrue(mongoVersion.isGreaterThanOrEqualTo(THREE_DOT_TWO));
 
 		IndexDefinition id = new Index().named("partial-with-inheritance").on("k3y", Direction.ASC)
 				.partial(of(where("age").gte(10)));

Original file line number	Diff line number	Diff line change
`@@ -529,7 +529,8 @@ public IndexOperations indexOps(String collectionName) {`
`529`	`529`	`}`
`530`	`530`
`531`	`531`	`public IndexOperations indexOps(Class<?> entityClass) {`
`532`		`- return new DefaultIndexOperations(getMongoDbFactory(), determineCollectionName(entityClass), queryMapper);`
	`532`	`+ return new DefaultIndexOperations(getMongoDbFactory(), determineCollectionName(entityClass), queryMapper,`
	`533`	`+ entityClass);`
`533`	`534`	`}`
`534`	`535`
`535`	`536`	`public BulkOperations bulkOps(BulkMode bulkMode, String collectionName) {`
Original file line number	Diff line number	Diff line change
`@@ -308,7 +308,8 @@ public ReactiveIndexOperations indexOps(String collectionName) {`
`308`	`308`	`* @see org.springframework.data.mongodb.core.ReactiveMongoOperations#reactiveIndexOps(java.lang.Class)`
`309`	`309`	`*/`
`310`	`310`	`public ReactiveIndexOperations indexOps(Class<?> entityClass) {`
`311`		`- return new DefaultReactiveIndexOperations(this, determineCollectionName(entityClass), this.queryMapper);`
	`311`	`+ return new DefaultReactiveIndexOperations(this, determineCollectionName(entityClass), this.queryMapper,`
	`312`	`+ entityClass);`
`312`	`313`	`}`
`313`	`314`
`314`	`315`	`public String getCollectionName(Class<?> entityClass) {`
`@@ -1914,8 +1915,7 @@ String determineCollectionName(Class<?> entityClass) {`
`1914`	`1915`	`"No class parameter provided, entity collection can't be determined!");`
`1915`	`1916`	`}`
`1916`	`1917`
`1917`		`- MongoPersistentEntity<?> entity = mappingContext.getRequiredPersistentEntity(entityClass);`
`1918`		`- return entity.getCollection();`
	`1918`	`+ return mappingContext.getRequiredPersistentEntity(entityClass).getCollection();`
`1919`	`1919`	`}`
`1920`	`1920`
`1921`	`1921`	`private static MappingMongoConverter getDefaultMongoConverter() {`