DATAMONGO-1678 - Polishing.

Use Lombok's Value annotation for immutable value objects. Use IllegalArgumentException for NonNull validation exceptions. 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.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,9 +15,10 @@
  */
 package org.springframework.data.mongodb.core;
 
-import lombok.Data;
+import lombok.NonNull;
+import lombok.Value;
 
-import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 
 import org.springframework.dao.DataAccessException;
@@ -45,6 +46,7 @@ import com.mongodb.WriteConcern;
  * @author Tobias Trelle
  * @author Oliver Gierke
  * @author Christoph Strobl
+ * @author Mark Paluch
  * @since 1.9
  */
 class DefaultBulkOperations implements BulkOperations {
@@ -53,8 +55,8 @@ class DefaultBulkOperations implements BulkOperations {
 	private final String collectionName;
 	private final BulkOperationContext bulkOperationContext;
 
-	private WriteConcernResolver writeConcernResolver;
 	private PersistenceExceptionTranslator exceptionTranslator;
+	private WriteConcernResolver writeConcernResolver;
 	private WriteConcern defaultWriteConcern;
 
 	private BulkWriteOperation bulk;
@@ -79,7 +81,8 @@ class DefaultBulkOperations implements BulkOperations {
 		this.collectionName = collectionName;
 		this.bulkOperationContext = bulkOperationContext;
 		this.exceptionTranslator = new MongoExceptionTranslator();
-		this.bulk = initBulkOperation();
+		this.writeConcernResolver = DefaultWriteConcernResolver.INSTANCE;
+		this.bulk = getBulkWriteOptions(bulkOperationContext.getBulkMode());
 	}
 
 	/**
@@ -106,7 +109,7 @@ class DefaultBulkOperations implements BulkOperations {
 	 *
 	 * @param defaultWriteConcern can be {@literal null}.
 	 */
-	public void setDefaultWriteConcern(WriteConcern defaultWriteConcern) {
+	void setDefaultWriteConcern(WriteConcern defaultWriteConcern) {
 		this.defaultWriteConcern = defaultWriteConcern;
 	}
 
@@ -158,7 +161,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)));
 	}
 
 	/*
@@ -188,7 +191,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)));
 	}
 
 	/*
@@ -279,7 +282,7 @@ class DefaultBulkOperations implements BulkOperations {
 			throw toThrow == null ? o_O : toThrow;
 
 		} finally {
-			this.bulk = initBulkOperation();
+			this.bulk = getBulkWriteOptions(bulkOperationContext.getBulkMode());
 		}
 	}
 
@@ -297,7 +300,7 @@ class DefaultBulkOperations implements BulkOperations {
 		Assert.notNull(query, "Query must not be null!");
 		Assert.notNull(update, "Update must not be null!");
 
-		BulkWriteRequestBuilder builder = bulk.find(query.getQueryObject());
+		BulkWriteRequestBuilder builder = bulk.find(getMappedQuery(query.getQueryObject()));
 
 		if (upsert) {
 
@@ -319,11 +322,19 @@ class DefaultBulkOperations implements BulkOperations {
 		return this;
 	}
 
-	private final BulkWriteOperation initBulkOperation() {
+	private DBObject getMappedUpdate(DBObject update) {
+		return bulkOperationContext.getUpdateMapper().getMappedObject(update, bulkOperationContext.getEntity());
+	}
+
+	private DBObject getMappedQuery(DBObject query) {
+		return bulkOperationContext.getQueryMapper().getMappedObject(query, bulkOperationContext.getEntity());
+	}
+
+	private BulkWriteOperation getBulkWriteOptions(BulkMode bulkMode) {
 
 		DBCollection collection = mongoOperations.getCollection(collectionName);
 
-		switch (bulkOperationContext.getBulkMode()) {
+		switch (bulkMode) {
 			case ORDERED:
 				return collection.initializeOrderedBulkOperation();
 			case UNORDERED:
@@ -333,14 +344,6 @@ class DefaultBulkOperations implements BulkOperations {
 		throw new IllegalStateException("BulkMode was null!");
 	}
 
-	private DBObject getMappedUpdate(DBObject update) {
-		return bulkOperationContext.getUpdateMapper().getMappedObject(update, bulkOperationContext.getEntity());
-	}
-
-	private DBObject getMappedQuery(DBObject query) {
-		return bulkOperationContext.getQueryMapper().getMappedObject(query, bulkOperationContext.getEntity());
-	}
-
 	/**
 	 * {@link BulkOperationContext} holds information about
 	 * {@link org.springframework.data.mongodb.core.BulkOperations.BulkMode} the entity in use as well as references to
@@ -349,13 +352,13 @@ class DefaultBulkOperations implements BulkOperations {
 	 * @author Christoph Strobl
 	 * @since 2.0
 	 */
-	@Data
+	@Value
 	static class BulkOperationContext {
 
-		final BulkMode bulkMode;
+		@NonNull BulkMode bulkMode;
-		final MongoPersistentEntity<?> entity;
+		MongoPersistentEntity<?> entity;
-		final QueryMapper queryMapper;
+		@NonNull QueryMapper queryMapper;
-		final UpdateMapper updateMapper;
+		@NonNull UpdateMapper updateMapper;
 
 		Class<?> getEntityType() {
 			return entity != null ? entity.getType() : null;

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

@@ -580,7 +580,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/DefaultBulkOperationsIntegrationTests.java

@@ -284,7 +284,6 @@ public class DefaultBulkOperationsIntegrationTests {
 
 		DefaultBulkOperations bulkOps = new DefaultBulkOperations(operations, COLLECTION_NAME, bulkOperationContext);
 		bulkOps.setDefaultWriteConcern(WriteConcern.ACKNOWLEDGED);
-		bulkOps.setWriteConcernResolver(DefaultWriteConcernResolver.INSTANCE);
 
 		return bulkOps;
 	}

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

@@ -0,0 +1,159 @@
+/*
+ * 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.
+ * 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;
+
+import static org.hamcrest.MatcherAssert.*;
+import static org.mockito.Mockito.*;
+import static org.springframework.data.mongodb.core.query.Criteria.*;
+import static org.springframework.data.mongodb.core.query.Query.*;
+import static org.springframework.data.mongodb.test.util.IsBsonObject.*;
+
+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.runners.MockitoJUnitRunner;
+import org.springframework.data.annotation.Id;
+import org.springframework.data.mongodb.core.BulkOperations.BulkMode;
+import org.springframework.data.mongodb.core.DefaultBulkOperations.BulkOperationContext;
+import org.springframework.data.mongodb.core.convert.DbRefResolver;
+import org.springframework.data.mongodb.core.convert.MappingMongoConverter;
+import org.springframework.data.mongodb.core.convert.MongoConverter;
+import org.springframework.data.mongodb.core.convert.QueryMapper;
+import org.springframework.data.mongodb.core.convert.UpdateMapper;
+import org.springframework.data.mongodb.core.mapping.Field;
+import org.springframework.data.mongodb.core.mapping.MongoMappingContext;
+import org.springframework.data.mongodb.core.query.BasicQuery;
+import org.springframework.data.mongodb.core.query.Update;
+
+import com.mongodb.BulkUpdateRequestBuilder;
+import com.mongodb.BulkWriteOperation;
+import com.mongodb.BulkWriteRequestBuilder;
+import com.mongodb.DBCollection;
+import com.mongodb.DBObject;
+
+/**
+ * Unit tests for {@link DefaultBulkOperations}.
+ *
+ * @author Christoph Strobl
+ * @author Mark Paluch
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class DefaultBulkOperationsUnitTests {
+
+	@Mock MongoTemplate template;
+	@Mock DBCollection collection;
+	@Mock DbRefResolver dbRefResolver;
+	@Captor ArgumentCaptor<DBObject> captor;
+	@Mock BulkWriteOperation bulk;
+	@Mock BulkWriteRequestBuilder bulkWriteRequestBuilder;
+	@Mock BulkUpdateRequestBuilder bulkUpdateRequestBuilder;
+	MongoConverter converter;
+	MongoMappingContext mappingContext;
+
+	DefaultBulkOperations ops;
+
+	@Before
+	public void before() throws Exception {
+
+		when(bulk.find(any(DBObject.class))).thenReturn(bulkWriteRequestBuilder);
+		when(bulkWriteRequestBuilder.upsert()).thenReturn(bulkUpdateRequestBuilder);
+		when(collection.initializeOrderedBulkOperation()).thenReturn(bulk);
+
+		mappingContext = new MongoMappingContext();
+		mappingContext.afterPropertiesSet();
+
+		converter = new MappingMongoConverter(dbRefResolver, mappingContext);
+
+		when(template.getCollection(anyString())).thenReturn(collection);
+
+		ops = new DefaultBulkOperations(template, "collection-1",
+				new BulkOperationContext(BulkMode.ORDERED, mappingContext.getPersistentEntity(SomeDomainType.class),
+						new QueryMapper(converter), new UpdateMapper(converter)));
+	}
+
+	@Test // DATAMONGO-1678
+	public void updateOneShouldMapQueryAndUpdate() {
+
+		ops.updateOne(new BasicQuery("{firstName:1}"), new Update().set("lastName", "targaryen")).execute();
+
+		verify(bulk).find(captor.capture());
+		verify(bulkWriteRequestBuilder).updateOne(captor.capture());
+
+		assertThat(captor.getAllValues().get(0), isBsonObject().containing("first_name", 1));
+		assertThat(captor.getAllValues().get(1), isBsonObject().containing("$set.last_name", "targaryen"));
+	}
+
+	@Test // DATAMONGO-1678
+	public void bulkUpdateShouldMapQueryAndUpdateCorrectly() {
+
+		ops.updateMulti(query(where("firstName").is("danerys")), Update.update("firstName", "queen danerys")).execute();
+
+		verify(bulk).find(captor.capture());
+		verify(bulkWriteRequestBuilder).update(captor.capture());
+
+		assertThat(captor.getAllValues().get(0), isBsonObject().containing("first_name", "danerys"));
+		assertThat(captor.getAllValues().get(1), isBsonObject().containing("$set.first_name", "queen danerys"));
+	}
+
+	@Test // DATAMONGO-1678
+	public void bulkUpdateManyShouldMapQueryAndUpdateCorrectly() {
+
+		ops.updateOne(query(where("firstName").is("danerys")), Update.update("firstName", "queen danerys")).execute();
+
+		verify(bulk).find(captor.capture());
+		verify(bulkWriteRequestBuilder).updateOne(captor.capture());
+
+		assertThat(captor.getAllValues().get(0), isBsonObject().containing("first_name", "danerys"));
+		assertThat(captor.getAllValues().get(1), isBsonObject().containing("$set.first_name", "queen danerys"));
+	}
+
+	@Test // DATAMONGO-1678
+	public void bulkUpsertShouldMapQueryAndUpdateCorrectly() {
+
+		ops.upsert(query(where("firstName").is("danerys")), Update.update("firstName", "queen danerys")).execute();
+
+		verify(bulk).find(captor.capture());
+		verify(bulkUpdateRequestBuilder).update(captor.capture());
+
+		assertThat(captor.getAllValues().get(0), isBsonObject().containing("first_name", "danerys"));
+		assertThat(captor.getAllValues().get(1), isBsonObject().containing("$set.first_name", "queen danerys"));
+	}
+
+	@Test // DATAMONGO-1678
+	public void bulkRemoveShouldMapQueryCorrectly() {
+
+		ops.remove(query(where("firstName").is("danerys"))).execute();
+
+		verify(bulk).find(captor.capture());
+
+		assertThat(captor.getValue(), isBsonObject().containing("first_name", "danerys"));
+	}
+
+	class SomeDomainType {
+
+		@Id String id;
+		Gender gender;
+		@Field("first_name") String firstName;
+		@Field("last_name") String lastName;
+	}
+
+	enum Gender {
+		M, F
+	}
+}