diff --git a/lombok.config b/lombok.config new file mode 100644 index 000000000..e50c7ea43 --- /dev/null +++ b/lombok.config @@ -0,0 +1,2 @@ +lombok.nonNull.exceptionType = IllegalArgumentException +lombok.log.fieldName = LOG diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/BulkOperations.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/BulkOperations.java index d4762e738..12421eee2 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/BulkOperations.java +++ b/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(); } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/DefaultBulkOperations.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/DefaultBulkOperations.java index db196d3f2..76b24010a 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/DefaultBulkOperations.java +++ b/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; - final MongoPersistentEntity entity; - final QueryMapper queryMapper; - final UpdateMapper updateMapper; + @NonNull BulkMode bulkMode; + MongoPersistentEntity entity; + @NonNull QueryMapper queryMapper; + @NonNull UpdateMapper updateMapper; Class getEntityType() { return entity != null ? entity.getType() : null; 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 f8fb92445..8062e1102 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 @@ -564,7 +564,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; diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/DefaultBulkOperationsIntegrationTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/DefaultBulkOperationsIntegrationTests.java index bd394983f..a516215df 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/DefaultBulkOperationsIntegrationTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/DefaultBulkOperationsIntegrationTests.java @@ -334,7 +334,6 @@ public class DefaultBulkOperationsIntegrationTests { DefaultBulkOperations bulkOps = new DefaultBulkOperations(operations, COLLECTION_NAME, bulkOperationContext); bulkOps.setDefaultWriteConcern(WriteConcern.ACKNOWLEDGED); - bulkOps.setWriteConcernResolver(DefaultWriteConcernResolver.INSTANCE); return bulkOps; } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/DefaultBulkOperationsUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/DefaultBulkOperationsUnitTests.java new file mode 100644 index 000000000..a57f4074f --- /dev/null +++ b/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 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 + } +}