From 368920a86576da79c0c6c8457e065972316dbaab Mon Sep 17 00:00:00 2001 From: Peter-Josef Meisch Date: Fri, 21 Jul 2023 14:54:18 +0200 Subject: [PATCH] Add VersionConflictException. Closes #2467 --- .../asciidoc/reference/elasticsearch-new.adoc | 3 +- .../VersionConflictException.java | 34 +++++++++++++++++++ .../elc/ElasticsearchExceptionTranslator.java | 24 ++++++++----- .../core/ElasticsearchIntegrationTests.java | 34 +++++++++++++++---- 4 files changed, 78 insertions(+), 17 deletions(-) create mode 100644 src/main/java/org/springframework/data/elasticsearch/VersionConflictException.java diff --git a/src/main/asciidoc/reference/elasticsearch-new.adoc b/src/main/asciidoc/reference/elasticsearch-new.adoc index e4fd7a3bd3..332cd4528b 100644 --- a/src/main/asciidoc/reference/elasticsearch-new.adoc +++ b/src/main/asciidoc/reference/elasticsearch-new.adoc @@ -8,12 +8,13 @@ * The `JsonpMapper` for Elasticsearch is now configurable and provided as bean. * Improved AOT runtime hints for Elasticsearch client library classes. * Add Kotlin extensions and repository coroutine support. +* Introducing `VersionConflictException` class thrown in case thatElasticsearch reports an 409 error with a version conflict. [[new-features.5-1-0]] == New in Spring Data Elasticsearch 5.1 * Upgrade to Elasticsearch 8.7.1 -* Allow specification of the TLS certificate when connecting to an Elasticsearch 8 cluster +* Allow specification of the TLS certificate when connecting to an Elasticsearch 8 cluster [[new-features.5-0-0]] == New in Spring Data Elasticsearch 5.0 diff --git a/src/main/java/org/springframework/data/elasticsearch/VersionConflictException.java b/src/main/java/org/springframework/data/elasticsearch/VersionConflictException.java new file mode 100644 index 0000000000..d9c2e83dff --- /dev/null +++ b/src/main/java/org/springframework/data/elasticsearch/VersionConflictException.java @@ -0,0 +1,34 @@ +/* + * Copyright 2023 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 + * + * https://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.elasticsearch; + +import org.springframework.dao.DataIntegrityViolationException; + +/** + * Exception that is thrown when a version conflict from the server is detected. + * + * @author Peter-Josef Meisch + * @since 5.2 + */ +public class VersionConflictException extends DataIntegrityViolationException { + public VersionConflictException(String msg) { + super(msg); + } + + public VersionConflictException(String msg, Throwable cause) { + super(msg, cause); + } +} diff --git a/src/main/java/org/springframework/data/elasticsearch/client/elc/ElasticsearchExceptionTranslator.java b/src/main/java/org/springframework/data/elasticsearch/client/elc/ElasticsearchExceptionTranslator.java index 3cdf391762..2851190d93 100644 --- a/src/main/java/org/springframework/data/elasticsearch/client/elc/ElasticsearchExceptionTranslator.java +++ b/src/main/java/org/springframework/data/elasticsearch/client/elc/ElasticsearchExceptionTranslator.java @@ -32,6 +32,7 @@ import org.springframework.data.elasticsearch.NoSuchIndexException; import org.springframework.data.elasticsearch.ResourceNotFoundException; import org.springframework.data.elasticsearch.UncategorizedElasticsearchException; +import org.springframework.data.elasticsearch.VersionConflictException; /** * Simple {@link PersistenceExceptionTranslator} for Elasticsearch. Convert the given runtime exception to an @@ -68,9 +69,7 @@ public RuntimeException translateException(Throwable throwable) { @Override public DataAccessException translateExceptionIfPossible(RuntimeException ex) { - if (isSeqNoConflict(ex)) { - return new OptimisticLockingFailureException("Cannot index a document due to seq_no+primary_term conflict", ex); - } + checkForConflictException(ex); if (ex instanceof ElasticsearchException elasticsearchException) { @@ -93,6 +92,10 @@ public DataAccessException translateExceptionIfPossible(RuntimeException ex) { return new ResourceNotFoundException(errorReason); } + + if (response.status() == 409) { + + } String body = JsonUtils.toJson(response, jsonpMapper); if (errorType != null && errorType.contains("validation_exception")) { @@ -110,7 +113,7 @@ public DataAccessException translateExceptionIfPossible(RuntimeException ex) { return null; } - private boolean isSeqNoConflict(Throwable exception) { + private void checkForConflictException(Throwable exception) { Integer status = null; String message = null; @@ -118,14 +121,17 @@ private boolean isSeqNoConflict(Throwable exception) { status = responseException.getResponse().getStatusLine().getStatusCode(); message = responseException.getMessage(); } else if (exception.getCause() != null) { - return isSeqNoConflict(exception.getCause()); + checkForConflictException(exception.getCause()); } if (status != null && message != null) { - return status == 409 && message.contains("type\":\"version_conflict_engine_exception") - && message.contains("version conflict, required seqNo"); + if (status == 409 && message.contains("type\":\"version_conflict_engine_exception")) + if (message.contains("version conflict, required seqNo")) { + throw new OptimisticLockingFailureException("Cannot index a document due to seq_no+primary_term conflict", + exception); + } else if (message.contains("version conflict, current version [")) { + throw new VersionConflictException("Version conflict", exception); + } } - - return false; } } diff --git a/src/test/java/org/springframework/data/elasticsearch/core/ElasticsearchIntegrationTests.java b/src/test/java/org/springframework/data/elasticsearch/core/ElasticsearchIntegrationTests.java index 5ef782e330..ab184ada24 100755 --- a/src/test/java/org/springframework/data/elasticsearch/core/ElasticsearchIntegrationTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/ElasticsearchIntegrationTests.java @@ -15,15 +15,15 @@ */ package org.springframework.data.elasticsearch.core; -import static java.util.Collections.singletonList; +import static java.util.Collections.*; import static org.assertj.core.api.Assertions.*; -import static org.springframework.data.elasticsearch.annotations.Document.VersionType.EXTERNAL_GTE; +import static org.springframework.data.elasticsearch.annotations.Document.VersionType.*; import static org.springframework.data.elasticsearch.annotations.FieldType.*; import static org.springframework.data.elasticsearch.annotations.FieldType.Integer; -import static org.springframework.data.elasticsearch.core.document.Document.create; -import static org.springframework.data.elasticsearch.core.query.StringQuery.MATCH_ALL; -import static org.springframework.data.elasticsearch.utils.IdGenerator.nextIdAsString; -import static org.springframework.data.elasticsearch.utils.IndexBuilder.buildIndex; +import static org.springframework.data.elasticsearch.core.document.Document.*; +import static org.springframework.data.elasticsearch.core.query.StringQuery.*; +import static org.springframework.data.elasticsearch.utils.IdGenerator.*; +import static org.springframework.data.elasticsearch.utils.IndexBuilder.*; import java.lang.Double; import java.lang.Integer; @@ -50,6 +50,7 @@ import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; import org.springframework.data.elasticsearch.BulkFailureException; +import org.springframework.data.elasticsearch.VersionConflictException; import org.springframework.data.elasticsearch.annotations.*; import org.springframework.data.elasticsearch.annotations.Field; import org.springframework.data.elasticsearch.annotations.ScriptedField; @@ -2008,7 +2009,7 @@ public void shouldIndexGteEntityWithVersionType() { // reindex with version one below assertThatThrownBy(() -> operations.index(indexQueryBuilder.withVersion(entity.getVersion() - 1).build(), index)) - .hasMessageContaining("version").hasMessageContaining("conflict"); + .isInstanceOf(VersionConflictException.class); } @Test @@ -3639,6 +3640,18 @@ void shouldFailWithConflictOnAttemptToSaveWithSameVersion() { .allMatch(failureStatus -> failureStatus.status().equals(409)); } + @Test // #2467 + @DisplayName("should throw VersionConflictException when saving invalid version") + void shouldThrowVersionConflictExceptionWhenSavingInvalidVersion() { + + var entity = new VersionedEntity("42", 1L); + operations.save(entity); + + assertThatThrownBy(() -> { + operations.save(entity); + }).isInstanceOf(VersionConflictException.class); + } + // region entities @Document(indexName = "#{@indexNameProvider.indexName()}") @Setting(shards = 1, replicas = 0, refreshInterval = "-1") @@ -4431,6 +4444,13 @@ static class VersionedEntity { @Nullable @Version private Long version; + public VersionedEntity() {} + + public VersionedEntity(@Nullable String id, @Nullable java.lang.Long version) { + this.id = id; + this.version = version; + } + @Nullable public String getId() { return id;