DATAMONGO-1678 - Polishing.

Use Lombok's Value annotation for immutable value objects. Use IllegalArgumentException for NonNull validation exceptions. Introduce missing generics. Use static methods where possible. Remove unused WriteConcernResolver. Trim whitespaces, formatting.

Original pull request: #472.

lombok.config

@@ -0,0 +1,2 @@
+lombok.nonNull.exceptionType = IllegalArgumentException
+lombok.log.fieldName = LOG

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/BulkOperations.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2015-2016 the original author or authors.
+ * Copyright 2015-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.
@@ -28,7 +28,7 @@ import com.mongodb.bulk.BulkWriteResult;
  * 2.6 and make use of low level bulk commands on the protocol level. This interface defines a fluent API to add
  * multiple single operations or list of similar operations in sequence which can then eventually be executed by calling
  * {@link #execute()}.
- * 
+ *
  * @author Tobias Trelle
  * @author Oliver Gierke
  * @since 1.9
@@ -49,7 +49,7 @@ public interface BulkOperations {
 
 	/**
 	 * Add a single insert to the bulk operation.
-	 * 
+	 *
 	 * @param documents the document to insert, must not be {@literal null}.
 	 * @return the current {@link BulkOperations} instance with the insert added, will never be {@literal null}.
 	 */
@@ -57,7 +57,7 @@ public interface BulkOperations {
 
 	/**
 	 * Add a list of inserts to the bulk operation.
-	 * 
+	 *
 	 * @param documents List of documents to insert, must not be {@literal null}.
 	 * @return the current {@link BulkOperations} instance with the insert added, will never be {@literal null}.
 	 */
@@ -65,7 +65,7 @@ public interface BulkOperations {
 
 	/**
 	 * Add a single update to the bulk operation. For the update request, only the first matching document is updated.
-	 * 
+	 *
 	 * @param query update criteria, must not be {@literal null}.
 	 * @param update {@link Update} operation to perform, must not be {@literal null}.
 	 * @return the current {@link BulkOperations} instance with the update added, will never be {@literal null}.
@@ -74,7 +74,7 @@ public interface BulkOperations {
 
 	/**
 	 * Add a list of updates to the bulk operation. For each update request, only the first matching document is updated.
-	 * 
+	 *
 	 * @param updates Update operations to perform.
 	 * @return the current {@link BulkOperations} instance with the update added, will never be {@literal null}.
 	 */
@@ -82,7 +82,7 @@ public interface BulkOperations {
 
 	/**
 	 * Add a single update to the bulk operation. For the update request, all matching documents are updated.
-	 * 
+	 *
 	 * @param query Update criteria.
 	 * @param update Update operation to perform.
 	 * @return the current {@link BulkOperations} instance with the update added, will never be {@literal null}.
@@ -91,7 +91,7 @@ public interface BulkOperations {
 
 	/**
 	 * Add a list of updates to the bulk operation. For each update request, all matching documents are updated.
-	 * 
+	 *
 	 * @param updates Update operations to perform.
 	 * @return The bulk operation.
 	 * @return the current {@link BulkOperations} instance with the update added, will never be {@literal null}.
@@ -101,7 +101,7 @@ public interface BulkOperations {
 	/**
 	 * Add a single upsert to the bulk operation. An upsert is an update if the set of matching documents is not empty,
 	 * else an insert.
-	 * 
+	 *
 	 * @param query Update criteria.
 	 * @param update Update operation to perform.
 	 * @return The bulk operation.
@@ -112,7 +112,7 @@ public interface BulkOperations {
 	/**
 	 * Add a list of upserts to the bulk operation. An upsert is an update if the set of matching documents is not empty,
 	 * else an insert.
-	 * 
+	 *
 	 * @param updates Updates/insert operations to perform.
 	 * @return The bulk operation.
 	 * @return the current {@link BulkOperations} instance with the update added, will never be {@literal null}.
@@ -121,7 +121,7 @@ public interface BulkOperations {
 
 	/**
 	 * Add a single remove operation to the bulk operation.
-	 * 
+	 *
 	 * @param remove the {@link Query} to select the documents to be removed, must not be {@literal null}.
 	 * @return the current {@link BulkOperations} instance with the removal added, will never be {@literal null}.
 	 */
@@ -129,7 +129,7 @@ public interface BulkOperations {
 
 	/**
 	 * Add a list of remove operations to the bulk operation.
-	 * 
+	 *
 	 * @param removes the remove operations to perform, must not be {@literal null}.
 	 * @return the current {@link BulkOperations} instance with the removal added, will never be {@literal null}.
 	 */
@@ -137,9 +137,9 @@ public interface BulkOperations {
 
 	/**
 	 * Execute all bulk operations using the default write concern.
-	 * 
+	 *
 	 * @return Result of the bulk operation providing counters for inserts/updates etc.
-	 * @throws {@link BulkOperationException} if an error occurred during bulk processing.
+	 * @throws org.springframework.data.mongodb.BulkOperationException if an error occurred during bulk processing.
 	 */
 	BulkWriteResult execute();
 }

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/DefaultBulkOperations.java

@@ -15,10 +15,11 @@
  */
 package org.springframework.data.mongodb.core;
 
-import lombok.Data;
+import lombok.NonNull;
+import lombok.Value;
 
 import java.util.ArrayList;
-import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 import java.util.stream.Collectors;
@@ -54,6 +55,7 @@ import com.mongodb.client.model.WriteModel;
  * @author Tobias Trelle
  * @author Oliver Gierke
  * @author Christoph Strobl
+ * @author Mark Paluch
  * @since 1.9
  */
 class DefaultBulkOperations implements BulkOperations {
@@ -61,15 +63,13 @@ class DefaultBulkOperations implements BulkOperations {
 	private final MongoOperations mongoOperations;
 	private final String collectionName;
 	private final BulkOperationContext bulkOperationContext;
+	private final List<WriteModel<Document>> models = new ArrayList<>();
 
-	private WriteConcernResolver writeConcernResolver;
 	private PersistenceExceptionTranslator exceptionTranslator;
 	private WriteConcern defaultWriteConcern;
 
 	private BulkWriteOptions bulkOptions;
 
-	List<WriteModel<Document>> models = new ArrayList<>();
-
 	/**
 	 * Creates a new {@link DefaultBulkOperations} for the given {@link MongoOperations}, collection name and
 	 * {@link BulkOperationContext}.
@@ -90,7 +90,7 @@ class DefaultBulkOperations implements BulkOperations {
 		this.collectionName = collectionName;
 		this.bulkOperationContext = bulkOperationContext;
 		this.exceptionTranslator = new MongoExceptionTranslator();
-		this.bulkOptions = initBulkOperation();
+		this.bulkOptions = getBulkWriteOptions(bulkOperationContext.getBulkMode());
 	}
 
 	/**
@@ -102,22 +102,12 @@ class DefaultBulkOperations implements BulkOperations {
 		this.exceptionTranslator = exceptionTranslator == null ? new MongoExceptionTranslator() : exceptionTranslator;
 	}
 
-	/**
-	 * Configures the {@link WriteConcernResolver} to be used. Defaults to {@link DefaultWriteConcernResolver}.
-	 *
-	 * @param writeConcernResolver can be {@literal null}.
-	 */
-	public void setWriteConcernResolver(WriteConcernResolver writeConcernResolver) {
-		this.writeConcernResolver = writeConcernResolver == null ? DefaultWriteConcernResolver.INSTANCE
-				: writeConcernResolver;
-	}
-
 	/**
 	 * Configures the default {@link WriteConcern} to be used. Defaults to {@literal null}.
 	 *
 	 * @param defaultWriteConcern can be {@literal null}.
 	 */
-	public void setDefaultWriteConcern(WriteConcern defaultWriteConcern) {
+	void setDefaultWriteConcern(WriteConcern defaultWriteConcern) {
 		this.defaultWriteConcern = defaultWriteConcern;
 	}
 
@@ -140,6 +130,7 @@ class DefaultBulkOperations implements BulkOperations {
 		mongoOperations.getConverter().write(document, sink);
 
 		models.add(new InsertOneModel<>(sink));
+
 		return this;
 	}
 
@@ -152,9 +143,7 @@ class DefaultBulkOperations implements BulkOperations {
 
 		Assert.notNull(documents, "Documents must not be null!");
 
-		for (Object document : documents) {
+		documents.forEach(this::insert);
-			insert(document);
-		}
 
 		return this;
 	}
@@ -170,7 +159,7 @@ class DefaultBulkOperations implements BulkOperations {
 		Assert.notNull(query, "Query must not be null!");
 		Assert.notNull(update, "Update must not be null!");
 
-		return updateOne(Arrays.asList(Pair.of(query, update)));
+		return updateOne(Collections.singletonList(Pair.of(query, update)));
 	}
 
 	/*
@@ -200,7 +189,7 @@ class DefaultBulkOperations implements BulkOperations {
 		Assert.notNull(query, "Query must not be null!");
 		Assert.notNull(update, "Update must not be null!");
 
-		return updateMulti(Arrays.asList(Pair.of(query, update)));
+		return updateMulti(Collections.singletonList(Pair.of(query, update)));
 	}
 
 	/*
@@ -254,7 +243,8 @@ class DefaultBulkOperations implements BulkOperations {
 		DeleteOptions deleteOptions = new DeleteOptions();
 		query.getCollation().map(Collation::toMongoCollation).ifPresent(deleteOptions::collation);
 
-		models.add(new DeleteManyModel(query.getQueryObject(), deleteOptions));
+		models.add(new DeleteManyModel<>(query.getQueryObject(), deleteOptions));
+
 		return this;
 	}
 
@@ -296,13 +286,13 @@ class DefaultBulkOperations implements BulkOperations {
 			throw toThrow == null ? o_O : toThrow;
 
 		} finally {
-			this.bulkOptions = initBulkOperation();
+			this.bulkOptions = getBulkWriteOptions(bulkOperationContext.getBulkMode());
 		}
 	}
 
 	/**
 	 * Performs update and upsert bulk operations.
-	 * 
+	 *
 	 * @param query the {@link Query} to determine documents to update.
 	 * @param update the {@link Update} to perform, must not be {@literal null}.
 	 * @param upsert whether to upsert.
@@ -323,29 +313,17 @@ class DefaultBulkOperations implements BulkOperations {
 		} else {
 			models.add(new UpdateOneModel<>(query.getQueryObject(), update.getUpdateObject(), options));
 		}
+
 		return this;
 	}
 
-	private final BulkWriteOptions initBulkOperation() {
-
-		BulkWriteOptions options = new BulkWriteOptions();
-
-		switch (bulkOperationContext.getBulkMode()) {
-			case ORDERED:
-				return options.ordered(true);
-			case UNORDERED:
-				return options.ordered(false);
-		}
-		throw new IllegalStateException("BulkMode was null!");
-	}
-
 	private WriteModel<Document> mapWriteModel(WriteModel<Document> writeModel) {
 
 		if (writeModel instanceof UpdateOneModel) {
 
 			UpdateOneModel<Document> model = (UpdateOneModel<Document>) writeModel;
 
-			return new UpdateOneModel(getMappedQuery(model.getFilter()), getMappedUpdate(model.getUpdate()),
+			return new UpdateOneModel<>(getMappedQuery(model.getFilter()), getMappedUpdate(model.getUpdate()),
 					model.getOptions());
 		}
 
@@ -353,7 +331,7 @@ class DefaultBulkOperations implements BulkOperations {
 
 			UpdateManyModel<Document> model = (UpdateManyModel<Document>) writeModel;
 
-			return new UpdateManyModel(getMappedQuery(model.getFilter()), getMappedUpdate(model.getUpdate()),
+			return new UpdateManyModel<>(getMappedQuery(model.getFilter()), getMappedUpdate(model.getUpdate()),
 					model.getOptions());
 		}
 
@@ -361,14 +339,14 @@ class DefaultBulkOperations implements BulkOperations {
 
 			DeleteOneModel<Document> model = (DeleteOneModel<Document>) writeModel;
 
-			return new DeleteOneModel(getMappedQuery(model.getFilter()), model.getOptions());
+			return new DeleteOneModel<>(getMappedQuery(model.getFilter()), model.getOptions());
 		}
 
 		if (writeModel instanceof DeleteManyModel) {
 
 			DeleteManyModel<Document> model = (DeleteManyModel<Document>) writeModel;
 
-			return new DeleteManyModel(getMappedQuery(model.getFilter()), model.getOptions());
+			return new DeleteManyModel<>(getMappedQuery(model.getFilter()), model.getOptions());
 		}
 
 		return writeModel;
@@ -382,6 +360,20 @@ class DefaultBulkOperations implements BulkOperations {
 		return bulkOperationContext.getQueryMapper().getMappedObject(query, bulkOperationContext.getEntity());
 	}
 
+	private static BulkWriteOptions getBulkWriteOptions(BulkMode bulkMode) {
+
+		BulkWriteOptions options = new BulkWriteOptions();
+
+		switch (bulkMode) {
+			case ORDERED:
+				return options.ordered(true);
+			case UNORDERED:
+				return options.ordered(false);
+		}
+
+		throw new IllegalStateException("BulkMode was null!");
+	}
+
 	/**
 	 * {@link BulkOperationContext} holds information about
 	 * {@link org.springframework.data.mongodb.core.BulkOperations.BulkMode} the entity in use as well as references to
@@ -390,12 +382,12 @@ class DefaultBulkOperations implements BulkOperations {
 	 * @author Christoph Strobl
 	 * @since 2.0
 	 */
-	@Data
+	@Value
 	static class BulkOperationContext {
 
-		final BulkMode bulkMode;
+		@NonNull BulkMode bulkMode;
-		final Optional<? extends MongoPersistentEntity<?>> entity;
+		@NonNull Optional<? extends MongoPersistentEntity<?>> entity;
-		final QueryMapper queryMapper;
+		@NonNull QueryMapper queryMapper;
-		final UpdateMapper updateMapper;
+		@NonNull UpdateMapper updateMapper;
 	}
 }

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java

@@ -562,7 +562,6 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware,
 				new BulkOperationContext(mode, getPersistentEntity(entityType), queryMapper, updateMapper));
 
 		operations.setExceptionTranslator(exceptionTranslator);
-		operations.setWriteConcernResolver(writeConcernResolver);
 		operations.setDefaultWriteConcern(writeConcern);
 
 		return operations;

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/DefaultBulkOperationsUnitTests.java

@@ -29,6 +29,7 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
+import org.mockito.Captor;
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 import org.springframework.data.annotation.Id;
@@ -51,14 +52,18 @@ import com.mongodb.client.model.UpdateOneModel;
 import com.mongodb.client.model.WriteModel;
 
 /**
+ * Unit tests for {@link DefaultBulkOperations}.
+ *
  * @author Christoph Strobl
+ * @author Mark Paluch
  */
 @RunWith(MockitoJUnitRunner.class)
 public class DefaultBulkOperationsUnitTests {
 
 	@Mock MongoTemplate template;
-	@Mock MongoCollection collection;
+	@Mock MongoCollection<Document> collection;
 	@Mock DbRefResolver dbRefResolver;
+	@Captor ArgumentCaptor<List<WriteModel<Document>>> captor;
 	MongoConverter converter;
 	MongoMappingContext mappingContext;
 
@@ -85,27 +90,23 @@ public class DefaultBulkOperationsUnitTests {
 		ops.updateOne(new BasicQuery("{}").collation(Collation.of("de")), new Update().set("lastName", "targaryen"))
 				.execute();
 
-		ArgumentCaptor<List<WriteModel<Document>>> captor = ArgumentCaptor.forClass(List.class);
-
 		verify(collection).bulkWrite(captor.capture(), any());
 
 		assertThat(captor.getValue().get(0)).isInstanceOf(UpdateOneModel.class);
-		assertThat(((UpdateOneModel) captor.getValue().get(0)).getOptions().getCollation())
+		assertThat(((UpdateOneModel<Document>) captor.getValue().get(0)).getOptions().getCollation())
 				.isEqualTo(com.mongodb.client.model.Collation.builder().locale("de").build());
 	}
 
 	@Test // DATAMONGO-1518
-	public void updateMayShouldUseCollationWhenPresent() {
+	public void updateManyShouldUseCollationWhenPresent() {
 
 		ops.updateMulti(new BasicQuery("{}").collation(Collation.of("de")), new Update().set("lastName", "targaryen"))
 				.execute();
 
-		ArgumentCaptor<List<WriteModel<Document>>> captor = ArgumentCaptor.forClass(List.class);
-
 		verify(collection).bulkWrite(captor.capture(), any());
 
 		assertThat(captor.getValue().get(0)).isInstanceOf(UpdateManyModel.class);
-		assertThat(((UpdateManyModel) captor.getValue().get(0)).getOptions().getCollation())
+		assertThat(((UpdateManyModel<Document>) captor.getValue().get(0)).getOptions().getCollation())
 				.isEqualTo(com.mongodb.client.model.Collation.builder().locale("de").build());
 	}
 
@@ -114,12 +115,10 @@ public class DefaultBulkOperationsUnitTests {
 
 		ops.remove(new BasicQuery("{}").collation(Collation.of("de"))).execute();
 
-		ArgumentCaptor<List<WriteModel<Document>>> captor = ArgumentCaptor.forClass(List.class);
-
 		verify(collection).bulkWrite(captor.capture(), any());
 
 		assertThat(captor.getValue().get(0)).isInstanceOf(DeleteManyModel.class);
-		assertThat(((DeleteManyModel) captor.getValue().get(0)).getOptions().getCollation())
+		assertThat(((DeleteManyModel<Document>) captor.getValue().get(0)).getOptions().getCollation())
 				.isEqualTo(com.mongodb.client.model.Collation.builder().locale("de").build());
 	}
 
@@ -128,11 +127,9 @@ public class DefaultBulkOperationsUnitTests {
 
 		ops.updateOne(query(where("firstName").is("danerys")), Update.update("firstName", "queen danerys")).execute();
 
-		ArgumentCaptor<List<WriteModel<Document>>> captor = ArgumentCaptor.forClass(List.class);
-
 		verify(collection).bulkWrite(captor.capture(), any());
 
-		UpdateOneModel<Document> updateModel = (UpdateOneModel) captor.getValue().get(0);
+		UpdateOneModel<Document> updateModel = (UpdateOneModel<Document>) captor.getValue().get(0);
 		assertThat(updateModel.getFilter()).isEqualTo(new Document("first_name", "danerys"));
 		assertThat(updateModel.getUpdate()).isEqualTo(new Document("$set", new Document("first_name", "queen danerys")));
 	}
@@ -142,11 +139,9 @@ public class DefaultBulkOperationsUnitTests {
 
 		ops.remove(query(where("firstName").is("danerys"))).execute();
 
-		ArgumentCaptor<List<WriteModel<Document>>> captor = ArgumentCaptor.forClass(List.class);
-
 		verify(collection).bulkWrite(captor.capture(), any());
 
-		DeleteManyModel<Document> updateModel = (DeleteManyModel) captor.getValue().get(0);
+		DeleteManyModel<Document> updateModel = (DeleteManyModel<Document>) captor.getValue().get(0);
 		assertThat(updateModel.getFilter()).isEqualTo(new Document("first_name", "danerys"));
 	}