From cee1d976ded5812655855cc078e99bdc7c04ec8b Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Tue, 23 Jun 2020 08:37:37 +0200 Subject: [PATCH] DATAMONGO-2285 - Polishing. Preserve existing logic translating com.mongodb.MongoBulkWriteException that might be thrown by calling MongoOperations.insertAll(Collection), and move bulk operation translation to DefaultBulkOperations. Along the lines remove the no longer used PersistenceExceptionTranslator from DefaultBulkOperations. Original Pull Request: #862 --- .../mongodb/core/DefaultBulkOperations.java | 41 +++++++++++-------- .../core/MongoExceptionTranslator.java | 20 ++++----- .../data/mongodb/core/MongoTemplate.java | 1 - ...DefaultBulkOperationsIntegrationTests.java | 9 ++-- .../core/DefaultBulkOperationsUnitTests.java | 31 ++++++++++++++ .../MongoExceptionTranslatorUnitTests.java | 24 ----------- 6 files changed, 70 insertions(+), 56 deletions(-) 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 10518301f..fb3d3fcf2 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 @@ -24,8 +24,9 @@ import java.util.stream.Collectors; import org.bson.Document; import org.bson.conversions.Bson; import org.springframework.context.ApplicationEventPublisher; -import org.springframework.dao.support.PersistenceExceptionTranslator; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.data.mapping.callback.EntityCallbacks; +import org.springframework.data.mongodb.BulkOperationException; import org.springframework.data.mongodb.core.convert.QueryMapper; import org.springframework.data.mongodb.core.convert.UpdateMapper; import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; @@ -46,6 +47,7 @@ import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; +import com.mongodb.MongoBulkWriteException; import com.mongodb.WriteConcern; import com.mongodb.bulk.BulkWriteResult; import com.mongodb.client.MongoCollection; @@ -62,6 +64,7 @@ import com.mongodb.client.model.*; * @author Jens Schauder * @author Michail Nikolaev * @author Roman Puchkovskiy + * @author Jacob Botuck * @since 1.9 */ class DefaultBulkOperations implements BulkOperations { @@ -71,7 +74,6 @@ class DefaultBulkOperations implements BulkOperations { private final BulkOperationContext bulkOperationContext; private final List models = new ArrayList<>(); - private PersistenceExceptionTranslator exceptionTranslator; private @Nullable WriteConcern defaultWriteConcern; private BulkWriteOptions bulkOptions; @@ -95,19 +97,9 @@ class DefaultBulkOperations implements BulkOperations { this.mongoOperations = mongoOperations; this.collectionName = collectionName; this.bulkOperationContext = bulkOperationContext; - this.exceptionTranslator = new MongoExceptionTranslator(); this.bulkOptions = getBulkWriteOptions(bulkOperationContext.getBulkMode()); } - /** - * Configures the {@link PersistenceExceptionTranslator} to be used. Defaults to {@link MongoExceptionTranslator}. - * - * @param exceptionTranslator can be {@literal null}. - */ - public void setExceptionTranslator(@Nullable PersistenceExceptionTranslator exceptionTranslator) { - this.exceptionTranslator = exceptionTranslator == null ? new MongoExceptionTranslator() : exceptionTranslator; - } - /** * Configures the default {@link WriteConcern} to be used. Defaults to {@literal null}. * @@ -314,11 +306,26 @@ class DefaultBulkOperations implements BulkOperations { collection = collection.withWriteConcern(defaultWriteConcern); } - return collection.bulkWrite( // - models.stream() // - .map(this::extractAndMapWriteModel) // - .collect(Collectors.toList()), // - bulkOptions); + try { + + return collection.bulkWrite( // + models.stream() // + .map(this::extractAndMapWriteModel) // + .collect(Collectors.toList()), // + bulkOptions); + } catch (RuntimeException ex) { + + if (ex instanceof MongoBulkWriteException) { + + MongoBulkWriteException mongoBulkWriteException = (MongoBulkWriteException) ex; + if (mongoBulkWriteException.getWriteConcernError() != null) { + throw new DataIntegrityViolationException(ex.getMessage(), ex); + } + throw new BulkOperationException(ex.getMessage(), mongoBulkWriteException); + } + + throw ex; + } } private WriteModel extractAndMapWriteModel(SourceAwareWriteModelHolder it) { diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoExceptionTranslator.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoExceptionTranslator.java index cf1a788c6..e34950375 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoExceptionTranslator.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoExceptionTranslator.java @@ -29,7 +29,6 @@ import org.springframework.dao.InvalidDataAccessApiUsageException; import org.springframework.dao.InvalidDataAccessResourceUsageException; import org.springframework.dao.PermissionDeniedDataAccessException; import org.springframework.dao.support.PersistenceExceptionTranslator; -import org.springframework.data.mongodb.BulkOperationException; import org.springframework.data.mongodb.ClientSessionException; import org.springframework.data.mongodb.MongoTransactionException; import org.springframework.data.mongodb.UncategorizedMongoDbException; @@ -40,6 +39,7 @@ import org.springframework.util.ClassUtils; import com.mongodb.MongoBulkWriteException; import com.mongodb.MongoException; import com.mongodb.MongoServerException; +import com.mongodb.bulk.BulkWriteError; /** * Simple {@link PersistenceExceptionTranslator} for Mongo. Convert the given runtime exception to an appropriate @@ -49,7 +49,6 @@ import com.mongodb.MongoServerException; * @author Oliver Gierke * @author Michal Vich * @author Christoph Strobl - * @author Jacob Botuck */ public class MongoExceptionTranslator implements PersistenceExceptionTranslator { @@ -64,7 +63,7 @@ public class MongoExceptionTranslator implements PersistenceExceptionTranslator Collections.singletonList("MongoInternalException")); private static final Set DATA_INTEGRITY_EXCEPTIONS = new HashSet<>( - Arrays.asList("WriteConcernException", "MongoWriteException")); + Arrays.asList("WriteConcernException", "MongoWriteException", "MongoBulkWriteException")); /* * (non-Javadoc) @@ -99,17 +98,18 @@ public class MongoExceptionTranslator implements PersistenceExceptionTranslator if (((MongoServerException) ex).getCode() == 11000) { return new DuplicateKeyException(ex.getMessage(), ex); } + if (ex instanceof MongoBulkWriteException) { + for (BulkWriteError x : ((MongoBulkWriteException) ex).getWriteErrors()) { + if (x.getCode() == 11000) { + return new DuplicateKeyException(ex.getMessage(), ex); + } + } + } } return new DataIntegrityViolationException(ex.getMessage(), ex); } - if (ex instanceof MongoBulkWriteException) { - MongoBulkWriteException mongoBulkWriteException = (MongoBulkWriteException) ex; - if (mongoBulkWriteException.getWriteConcernError() != null) { - return new DataIntegrityViolationException(ex.getMessage(), ex); - } - return new BulkOperationException(ex.getMessage(), mongoBulkWriteException); - } + // All other MongoExceptions if (ex instanceof MongoException) { 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 d37c9888a..015cbf25b 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 @@ -757,7 +757,6 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware, new BulkOperationContext(mode, Optional.ofNullable(getPersistentEntity(entityType)), queryMapper, updateMapper, eventPublisher, entityCallbacks)); - operations.setExceptionTranslator(exceptionTranslator); 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 3a04a67a0..ac0f6e509 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 @@ -27,6 +27,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.springframework.dao.DuplicateKeyException; +import org.springframework.data.mongodb.BulkOperationException; import org.springframework.data.mongodb.core.BulkOperations.BulkMode; import org.springframework.data.mongodb.core.DefaultBulkOperations.BulkOperationContext; import org.springframework.data.mongodb.core.convert.QueryMapper; @@ -91,13 +92,13 @@ public class DefaultBulkOperationsIntegrationTests { assertThat(createBulkOps(BulkMode.ORDERED).insert(documents).execute().getInsertedCount()).isEqualTo(2); } - @Test // DATAMONGO-934 + @Test // DATAMONGO-934, DATAMONGO-2285 public void insertOrderedFails() { List documents = Arrays.asList(newDoc("1"), newDoc("1"), newDoc("2")); assertThatThrownBy(() -> createBulkOps(BulkMode.ORDERED).insert(documents).execute()) // - .isInstanceOf(DuplicateKeyException.class) // + .isInstanceOf(BulkOperationException.class) // .hasCauseInstanceOf(MongoBulkWriteException.class) // .extracting(Throwable::getCause) // .satisfies(it -> { @@ -117,13 +118,13 @@ public class DefaultBulkOperationsIntegrationTests { assertThat(createBulkOps(BulkMode.UNORDERED).insert(documents).execute().getInsertedCount()).isEqualTo(2); } - @Test // DATAMONGO-934 + @Test // DATAMONGO-934, DATAMONGO-2285 public void insertUnOrderedContinuesOnError() { List documents = Arrays.asList(newDoc("1"), newDoc("1"), newDoc("2")); assertThatThrownBy(() -> createBulkOps(BulkMode.UNORDERED).insert(documents).execute()) // - .isInstanceOf(DuplicateKeyException.class) // + .isInstanceOf(BulkOperationException.class) // .hasCauseInstanceOf(MongoBulkWriteException.class) // .extracting(Throwable::getCause) // .satisfies(it -> { 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 index 4b9cd57fc..7e1356c39 100644 --- 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 @@ -24,6 +24,7 @@ import static org.mockito.Mockito.eq; import static org.springframework.data.mongodb.core.query.Criteria.*; import static org.springframework.data.mongodb.core.query.Query.*; +import java.util.Collections; import java.util.List; import java.util.Optional; @@ -40,9 +41,11 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.context.ApplicationEventPublisher; import org.springframework.dao.DataAccessException; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.dao.support.PersistenceExceptionTranslator; import org.springframework.data.annotation.Id; import org.springframework.data.mapping.callback.EntityCallbacks; +import org.springframework.data.mongodb.BulkOperationException; import org.springframework.data.mongodb.MongoDatabaseFactory; import org.springframework.data.mongodb.core.BulkOperations.BulkMode; import org.springframework.data.mongodb.core.DefaultBulkOperations.BulkOperationContext; @@ -64,9 +67,13 @@ import org.springframework.data.mongodb.core.query.Collation; import org.springframework.data.mongodb.core.query.Criteria; import org.springframework.data.mongodb.core.query.Update; +import com.mongodb.MongoBulkWriteException; import com.mongodb.MongoWriteException; +import com.mongodb.ServerAddress; import com.mongodb.WriteConcern; import com.mongodb.WriteError; +import com.mongodb.bulk.BulkWriteError; +import com.mongodb.bulk.WriteConcernError; import com.mongodb.client.MongoCollection; import com.mongodb.client.MongoDatabase; import com.mongodb.client.model.BulkWriteOptions; @@ -85,6 +92,7 @@ import com.mongodb.client.model.WriteModel; * @author Minsu Kim * @author Jens Schauder * @author Roman Puchkovskiy + * @author Jacob Botuck */ @ExtendWith(MockitoExtension.class) class DefaultBulkOperationsUnitTests { @@ -367,6 +375,29 @@ class DefaultBulkOperationsUnitTests { .isEqualTo(new Document("$set", new Document("items.$.documents.0.the_file_id", "file-id"))); } + @Test // DATAMONGO-2285 + public void translateMongoBulkOperationExceptionWithWriteConcernError() { + + when(collection.bulkWrite(anyList(), any(BulkWriteOptions.class))).thenThrow(new MongoBulkWriteException(null, + Collections.emptyList(), + new WriteConcernError(42, "codename", "writeconcern error happened", new BsonDocument()), new ServerAddress())); + + assertThatExceptionOfType(DataIntegrityViolationException.class) + .isThrownBy(() -> ops.insert(new SomeDomainType()).execute()); + + } + + @Test // DATAMONGO-2285 + public void translateMongoBulkOperationExceptionWithoutWriteConcernError() { + + when(collection.bulkWrite(anyList(), any(BulkWriteOptions.class))).thenThrow(new MongoBulkWriteException(null, + Collections.singletonList(new BulkWriteError(42, "a write error happened", new BsonDocument(), 49)), null, + new ServerAddress())); + + assertThatExceptionOfType(BulkOperationException.class) + .isThrownBy(() -> ops.insert(new SomeDomainType()).execute()); + } + static class OrderTest { String id; diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoExceptionTranslatorUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoExceptionTranslatorUnitTests.java index e9241b81c..04cd781ac 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoExceptionTranslatorUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoExceptionTranslatorUnitTests.java @@ -18,12 +18,7 @@ package org.springframework.data.mongodb.core; import static org.assertj.core.api.Assertions.*; import java.net.UnknownHostException; -import java.util.ArrayList; -import java.util.Arrays; -import com.mongodb.MongoBulkWriteException; -import com.mongodb.bulk.BulkWriteError; -import com.mongodb.bulk.WriteConcernError; import org.bson.BsonDocument; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -31,11 +26,9 @@ import org.junit.jupiter.api.Test; import org.springframework.core.NestedRuntimeException; import org.springframework.dao.DataAccessException; import org.springframework.dao.DataAccessResourceFailureException; -import org.springframework.dao.DataIntegrityViolationException; import org.springframework.dao.DuplicateKeyException; import org.springframework.dao.InvalidDataAccessApiUsageException; import org.springframework.dao.InvalidDataAccessResourceUsageException; -import org.springframework.data.mongodb.BulkOperationException; import org.springframework.data.mongodb.ClientSessionException; import org.springframework.data.mongodb.MongoTransactionException; import org.springframework.data.mongodb.UncategorizedMongoDbException; @@ -52,7 +45,6 @@ import com.mongodb.ServerAddress; * @author Michal Vich * @author Oliver Gierke * @author Christoph Strobl - * @author Jacob Botuck */ public class MongoExceptionTranslatorUnitTests { @@ -160,22 +152,6 @@ public class MongoExceptionTranslatorUnitTests { checkTranslatedMongoException(MongoTransactionException.class, 267); } - @Test - public void translateMongoBulkOperationExceptionWithWriteConcernError() { - - expectExceptionWithCauseMessage(translator.translateExceptionIfPossible(new MongoBulkWriteException(null, - new ArrayList<>(), new WriteConcernError(42, "codename", "writeconcern error happened", new BsonDocument()), - new ServerAddress())), DataIntegrityViolationException.class, null); - } - - @Test - public void translateMongoBulkOperationExceptionWithoutWriteConcernError() { - - expectExceptionWithCauseMessage(translator.translateExceptionIfPossible(new MongoBulkWriteException(null, - Arrays.asList(new BulkWriteError(42, "a write error happened", new BsonDocument(), 49)), null, - new ServerAddress())), BulkOperationException.class, null); - } - private void checkTranslatedMongoException(Class clazz, int code) { DataAccessException translated = translator.translateExceptionIfPossible(new MongoException(code, ""));