diff --git a/CHANGELOG.md b/CHANGELOG.md index a329bcef79a3e..4c219ca69fcc2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,6 +66,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Bump `opentelemetry-semconv` from 1.27.0-alpha to 1.29.0-alpha ([#16700](https://github.com/opensearch-project/OpenSearch/pull/16700)) - Bump `com.google.re2j:re2j` from 1.7 to 1.8 ([#17012](https://github.com/opensearch-project/OpenSearch/pull/17012)) - Bump `com.squareup.okio:okio` from 3.9.1 to 3.10.2 ([#17060](https://github.com/opensearch-project/OpenSearch/pull/17060)) +- Bump `com.diffplug.spotless` from 6.25.0 to 7.0.2 ([#17058](https://github.com/opensearch-project/OpenSearch/pull/17058)) - Bump `org.jruby.jcodings:jcodings` from 1.0.58 to 1.0.61 ([#17061](https://github.com/opensearch-project/OpenSearch/pull/17061)) ### Changed diff --git a/build.gradle b/build.gradle index 679f7b9299248..fde086b3bd79e 100644 --- a/build.gradle +++ b/build.gradle @@ -54,7 +54,7 @@ plugins { id 'lifecycle-base' id 'opensearch.docker-support' id 'opensearch.global-build-info' - id "com.diffplug.spotless" version "6.25.0" apply false + id "com.diffplug.spotless" version "7.0.2" apply false id "test-report-aggregation" id 'jacoco-report-aggregation' } diff --git a/libs/common/src/test/java/org/opensearch/common/annotation/processor/ApiAnnotationProcessorTests.java b/libs/common/src/test/java/org/opensearch/common/annotation/processor/ApiAnnotationProcessorTests.java index 716dcc3b9015f..944b29c139160 100644 --- a/libs/common/src/test/java/org/opensearch/common/annotation/processor/ApiAnnotationProcessorTests.java +++ b/libs/common/src/test/java/org/opensearch/common/annotation/processor/ApiAnnotationProcessorTests.java @@ -8,8 +8,11 @@ package org.opensearch.common.annotation.processor; +import org.opensearch.common.SuppressForbidden; import org.opensearch.common.annotation.InternalApi; import org.opensearch.test.OpenSearchTestCase; +import org.junit.Rule; +import org.junit.rules.TemporaryFolder; import javax.tools.Diagnostic; @@ -20,10 +23,14 @@ import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; +@SuppressForbidden(reason = "TemporaryFolder does not support Path-based APIs") @SuppressWarnings("deprecation") public class ApiAnnotationProcessorTests extends OpenSearchTestCase implements CompilerSupport { + @Rule + public final TemporaryFolder folder = new TemporaryFolder(); + public void testPublicApiMethodArgumentNotAnnotated() { - final CompilerResult result = compile("PublicApiMethodArgumentNotAnnotated.java", "NotAnnotated.java"); + final CompilerResult result = compile(folder.getRoot().toPath(), "PublicApiMethodArgumentNotAnnotated.java", "NotAnnotated.java"); assertThat(result, instanceOf(Failure.class)); final Failure failure = (Failure) result; @@ -44,7 +51,11 @@ public void testPublicApiMethodArgumentNotAnnotated() { } public void testPublicApiMethodArgumentNotAnnotatedGenerics() { - final CompilerResult result = compile("PublicApiMethodArgumentNotAnnotatedGenerics.java", "NotAnnotated.java"); + final CompilerResult result = compile( + folder.getRoot().toPath(), + "PublicApiMethodArgumentNotAnnotatedGenerics.java", + "NotAnnotated.java" + ); assertThat(result, instanceOf(Failure.class)); final Failure failure = (Failure) result; @@ -65,7 +76,11 @@ public void testPublicApiMethodArgumentNotAnnotatedGenerics() { } public void testPublicApiMethodThrowsNotAnnotated() { - final CompilerResult result = compile("PublicApiMethodThrowsNotAnnotated.java", "PublicApiAnnotated.java"); + final CompilerResult result = compile( + folder.getRoot().toPath(), + "PublicApiMethodThrowsNotAnnotated.java", + "PublicApiAnnotated.java" + ); assertThat(result, instanceOf(Failure.class)); final Failure failure = (Failure) result; @@ -86,7 +101,7 @@ public void testPublicApiMethodThrowsNotAnnotated() { } public void testPublicApiMethodArgumentNotAnnotatedPackagePrivate() { - final CompilerResult result = compile("PublicApiMethodArgumentNotAnnotatedPackagePrivate.java"); + final CompilerResult result = compile(folder.getRoot().toPath(), "PublicApiMethodArgumentNotAnnotatedPackagePrivate.java"); assertThat(result, instanceOf(Failure.class)); final Failure failure = (Failure) result; @@ -120,7 +135,7 @@ public void testPublicApiMethodArgumentNotAnnotatedPackagePrivate() { } public void testPublicApiMethodArgumentAnnotatedPackagePrivate() { - final CompilerResult result = compile("PublicApiMethodArgumentAnnotatedPackagePrivate.java"); + final CompilerResult result = compile(folder.getRoot().toPath(), "PublicApiMethodArgumentAnnotatedPackagePrivate.java"); assertThat(result, instanceOf(Failure.class)); final Failure failure = (Failure) result; @@ -141,7 +156,7 @@ public void testPublicApiMethodArgumentAnnotatedPackagePrivate() { } public void testPublicApiWithInternalApiMethod() { - final CompilerResult result = compile("PublicApiWithInternalApiMethod.java"); + final CompilerResult result = compile(folder.getRoot().toPath(), "PublicApiWithInternalApiMethod.java"); assertThat(result, instanceOf(Failure.class)); final Failure failure = (Failure) result; @@ -164,40 +179,48 @@ public void testPublicApiWithInternalApiMethod() { * The constructor arguments have relaxed semantics at the moment: those could be not annotated or be annotated as {@link InternalApi} */ public void testPublicApiConstructorArgumentNotAnnotated() { - final CompilerResult result = compile("PublicApiConstructorArgumentNotAnnotated.java", "NotAnnotated.java"); - assertThat(result, instanceOf(Failure.class)); + final CompilerResult result = compile( + folder.getRoot().toPath(), + "PublicApiConstructorArgumentNotAnnotated.java", + "NotAnnotated.java" + ); + assertThat(result, instanceOf(Success.class)); - final Failure failure = (Failure) result; - assertThat(failure.diagnotics(), hasSize(2)); + final Success success = (Success) result; + assertThat(success.diagnotics(), hasSize(2)); - assertThat(failure.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); + assertThat(success.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); } /** * The constructor arguments have relaxed semantics at the moment: those could be not annotated or be annotated as {@link InternalApi} */ public void testPublicApiConstructorArgumentAnnotatedInternalApi() { - final CompilerResult result = compile("PublicApiConstructorArgumentAnnotatedInternalApi.java", "InternalApiAnnotated.java"); - assertThat(result, instanceOf(Failure.class)); + final CompilerResult result = compile( + folder.getRoot().toPath(), + "PublicApiConstructorArgumentAnnotatedInternalApi.java", + "InternalApiAnnotated.java" + ); + assertThat(result, instanceOf(Success.class)); - final Failure failure = (Failure) result; - assertThat(failure.diagnotics(), hasSize(2)); + final Success success = (Success) result; + assertThat(success.diagnotics(), hasSize(2)); - assertThat(failure.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); + assertThat(success.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); } public void testPublicApiWithExperimentalApiMethod() { - final CompilerResult result = compile("PublicApiWithExperimentalApiMethod.java"); - assertThat(result, instanceOf(Failure.class)); + final CompilerResult result = compile(folder.getRoot().toPath(), "PublicApiWithExperimentalApiMethod.java"); + assertThat(result, instanceOf(Success.class)); - final Failure failure = (Failure) result; - assertThat(failure.diagnotics(), hasSize(2)); + final Success success = (Success) result; + assertThat(success.diagnotics(), hasSize(2)); - assertThat(failure.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); + assertThat(success.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); } public void testPublicApiMethodReturnNotAnnotated() { - final CompilerResult result = compile("PublicApiMethodReturnNotAnnotated.java", "NotAnnotated.java"); + final CompilerResult result = compile(folder.getRoot().toPath(), "PublicApiMethodReturnNotAnnotated.java", "NotAnnotated.java"); assertThat(result, instanceOf(Failure.class)); final Failure failure = (Failure) result; @@ -218,7 +241,11 @@ public void testPublicApiMethodReturnNotAnnotated() { } public void testPublicApiMethodReturnNotAnnotatedGenerics() { - final CompilerResult result = compile("PublicApiMethodReturnNotAnnotatedGenerics.java", "NotAnnotated.java"); + final CompilerResult result = compile( + folder.getRoot().toPath(), + "PublicApiMethodReturnNotAnnotatedGenerics.java", + "NotAnnotated.java" + ); assertThat(result, instanceOf(Failure.class)); final Failure failure = (Failure) result; @@ -239,7 +266,11 @@ public void testPublicApiMethodReturnNotAnnotatedGenerics() { } public void testPublicApiMethodReturnNotAnnotatedArray() { - final CompilerResult result = compile("PublicApiMethodReturnNotAnnotatedArray.java", "NotAnnotated.java"); + final CompilerResult result = compile( + folder.getRoot().toPath(), + "PublicApiMethodReturnNotAnnotatedArray.java", + "NotAnnotated.java" + ); assertThat(result, instanceOf(Failure.class)); final Failure failure = (Failure) result; @@ -260,7 +291,11 @@ public void testPublicApiMethodReturnNotAnnotatedArray() { } public void testPublicApiMethodReturnNotAnnotatedBoundedGenerics() { - final CompilerResult result = compile("PublicApiMethodReturnNotAnnotatedBoundedGenerics.java", "NotAnnotated.java"); + final CompilerResult result = compile( + folder.getRoot().toPath(), + "PublicApiMethodReturnNotAnnotatedBoundedGenerics.java", + "NotAnnotated.java" + ); assertThat(result, instanceOf(Failure.class)); final Failure failure = (Failure) result; @@ -282,6 +317,7 @@ public void testPublicApiMethodReturnNotAnnotatedBoundedGenerics() { public void testPublicApiMethodReturnNotAnnotatedAnnotation() { final CompilerResult result = compile( + folder.getRoot().toPath(), "PublicApiMethodReturnNotAnnotatedAnnotation.java", "PublicApiAnnotated.java", "NotAnnotatedAnnotation.java" @@ -306,57 +342,57 @@ public void testPublicApiMethodReturnNotAnnotatedAnnotation() { } public void testPublicApiMethodReturnNotAnnotatedWildcardGenerics() { - final CompilerResult result = compile("PublicApiMethodReturnNotAnnotatedWildcardGenerics.java"); - assertThat(result, instanceOf(Failure.class)); + final CompilerResult result = compile(folder.getRoot().toPath(), "PublicApiMethodReturnNotAnnotatedWildcardGenerics.java"); + assertThat(result, instanceOf(Success.class)); - final Failure failure = (Failure) result; - assertThat(failure.diagnotics(), hasSize(2)); + final Success success = (Success) result; + assertThat(success.diagnotics(), hasSize(2)); - assertThat(failure.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); + assertThat(success.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); } public void testPublicApiWithPackagePrivateMethod() { - final CompilerResult result = compile("PublicApiWithPackagePrivateMethod.java", "NotAnnotated.java"); - assertThat(result, instanceOf(Failure.class)); + final CompilerResult result = compile(folder.getRoot().toPath(), "PublicApiWithPackagePrivateMethod.java", "NotAnnotated.java"); + assertThat(result, instanceOf(Success.class)); - final Failure failure = (Failure) result; - assertThat(failure.diagnotics(), hasSize(2)); + final Success success = (Success) result; + assertThat(success.diagnotics(), hasSize(2)); - assertThat(failure.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); + assertThat(success.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); } public void testPublicApiMethodReturnSelf() { - final CompilerResult result = compile("PublicApiMethodReturnSelf.java"); - assertThat(result, instanceOf(Failure.class)); + final CompilerResult result = compile(folder.getRoot().toPath(), "PublicApiMethodReturnSelf.java"); + assertThat(result, instanceOf(Success.class)); - final Failure failure = (Failure) result; - assertThat(failure.diagnotics(), hasSize(2)); + final Success success = (Success) result; + assertThat(success.diagnotics(), hasSize(2)); - assertThat(failure.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); + assertThat(success.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); } public void testExperimentalApiMethodReturnSelf() { - final CompilerResult result = compile("ExperimentalApiMethodReturnSelf.java"); - assertThat(result, instanceOf(Failure.class)); + final CompilerResult result = compile(folder.getRoot().toPath(), "ExperimentalApiMethodReturnSelf.java"); + assertThat(result, instanceOf(Success.class)); - final Failure failure = (Failure) result; - assertThat(failure.diagnotics(), hasSize(2)); + final Success success = (Success) result; + assertThat(success.diagnotics(), hasSize(2)); - assertThat(failure.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); + assertThat(success.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); } public void testDeprecatedApiMethodReturnSelf() { - final CompilerResult result = compile("DeprecatedApiMethodReturnSelf.java"); - assertThat(result, instanceOf(Failure.class)); + final CompilerResult result = compile(folder.getRoot().toPath(), "DeprecatedApiMethodReturnSelf.java"); + assertThat(result, instanceOf(Success.class)); - final Failure failure = (Failure) result; - assertThat(failure.diagnotics(), hasSize(2)); + final Success success = (Success) result; + assertThat(success.diagnotics(), hasSize(2)); - assertThat(failure.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); + assertThat(success.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); } public void testPublicApiPackagePrivate() { - final CompilerResult result = compile("PublicApiPackagePrivate.java"); + final CompilerResult result = compile(folder.getRoot().toPath(), "PublicApiPackagePrivate.java"); assertThat(result, instanceOf(Failure.class)); final Failure failure = (Failure) result; @@ -376,7 +412,11 @@ public void testPublicApiPackagePrivate() { } public void testPublicApiMethodGenericsArgumentNotAnnotated() { - final CompilerResult result = compile("PublicApiMethodGenericsArgumentNotAnnotated.java", "NotAnnotated.java"); + final CompilerResult result = compile( + folder.getRoot().toPath(), + "PublicApiMethodGenericsArgumentNotAnnotated.java", + "NotAnnotated.java" + ); assertThat(result, instanceOf(Failure.class)); final Failure failure = (Failure) result; @@ -397,27 +437,35 @@ public void testPublicApiMethodGenericsArgumentNotAnnotated() { } public void testPublicApiMethodReturnAnnotatedArray() { - final CompilerResult result = compile("PublicApiMethodReturnAnnotatedArray.java", "PublicApiAnnotated.java"); - assertThat(result, instanceOf(Failure.class)); + final CompilerResult result = compile( + folder.getRoot().toPath(), + "PublicApiMethodReturnAnnotatedArray.java", + "PublicApiAnnotated.java" + ); + assertThat(result, instanceOf(Success.class)); - final Failure failure = (Failure) result; - assertThat(failure.diagnotics(), hasSize(2)); + final Success success = (Success) result; + assertThat(success.diagnotics(), hasSize(2)); - assertThat(failure.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); + assertThat(success.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); } public void testPublicApiMethodGenericsArgumentAnnotated() { - final CompilerResult result = compile("PublicApiMethodGenericsArgumentAnnotated.java", "PublicApiAnnotated.java"); - assertThat(result, instanceOf(Failure.class)); + final CompilerResult result = compile( + folder.getRoot().toPath(), + "PublicApiMethodGenericsArgumentAnnotated.java", + "PublicApiAnnotated.java" + ); + assertThat(result, instanceOf(Success.class)); - final Failure failure = (Failure) result; - assertThat(failure.diagnotics(), hasSize(2)); + final Success success = (Success) result; + assertThat(success.diagnotics(), hasSize(2)); - assertThat(failure.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); + assertThat(success.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); } public void testPublicApiAnnotatedNotOpensearch() { - final CompilerResult result = compileWithPackage("org.acme", "PublicApiAnnotated.java"); + final CompilerResult result = compileWithPackage(folder.getRoot().toPath(), "org.acme", "PublicApiAnnotated.java"); assertThat(result, instanceOf(Failure.class)); final Failure failure = (Failure) result; @@ -438,6 +486,7 @@ public void testPublicApiAnnotatedNotOpensearch() { public void testPublicApiMethodReturnAnnotatedGenerics() { final CompilerResult result = compile( + folder.getRoot().toPath(), "PublicApiMethodReturnAnnotatedGenerics.java", "PublicApiAnnotated.java", "NotAnnotatedAnnotation.java" @@ -465,30 +514,34 @@ public void testPublicApiMethodReturnAnnotatedGenerics() { * The type could expose protected inner types which are still considered to be a public API when used */ public void testPublicApiWithProtectedInterface() { - final CompilerResult result = compile("PublicApiWithProtectedInterface.java"); - assertThat(result, instanceOf(Failure.class)); + final CompilerResult result = compile(folder.getRoot().toPath(), "PublicApiWithProtectedInterface.java"); + assertThat(result, instanceOf(Success.class)); - final Failure failure = (Failure) result; - assertThat(failure.diagnotics(), hasSize(2)); + final Success success = (Success) result; + assertThat(success.diagnotics(), hasSize(2)); - assertThat(failure.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); + assertThat(success.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); } /** * The constructor arguments have relaxed semantics at the moment: those could be not annotated or be annotated as {@link InternalApi} */ public void testPublicApiConstructorAnnotatedInternalApi() { - final CompilerResult result = compile("PublicApiConstructorAnnotatedInternalApi.java", "NotAnnotated.java"); - assertThat(result, instanceOf(Failure.class)); + final CompilerResult result = compile( + folder.getRoot().toPath(), + "PublicApiConstructorAnnotatedInternalApi.java", + "NotAnnotated.java" + ); + assertThat(result, instanceOf(Success.class)); - final Failure failure = (Failure) result; - assertThat(failure.diagnotics(), hasSize(2)); + final Success success = (Success) result; + assertThat(success.diagnotics(), hasSize(2)); - assertThat(failure.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); + assertThat(success.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); } public void testPublicApiUnparseableVersion() { - final CompilerResult result = compile("PublicApiAnnotatedUnparseable.java"); + final CompilerResult result = compile(folder.getRoot().toPath(), "PublicApiAnnotatedUnparseable.java"); assertThat(result, instanceOf(Failure.class)); final Failure failure = (Failure) result; @@ -508,13 +561,13 @@ public void testPublicApiUnparseableVersion() { } public void testPublicApiWithDeprecatedApiMethod() { - final CompilerResult result = compile("PublicApiWithDeprecatedApiMethod.java"); - assertThat(result, instanceOf(Failure.class)); + final CompilerResult result = compile(folder.getRoot().toPath(), "PublicApiWithDeprecatedApiMethod.java"); + assertThat(result, instanceOf(Success.class)); - final Failure failure = (Failure) result; - assertThat(failure.diagnotics(), hasSize(2)); + final Success success = (Success) result; + assertThat(success.diagnotics(), hasSize(2)); - assertThat(failure.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); + assertThat(success.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR)))); } } diff --git a/libs/common/src/test/java/org/opensearch/common/annotation/processor/CompilerSupport.java b/libs/common/src/test/java/org/opensearch/common/annotation/processor/CompilerSupport.java index c8fdb3333a714..e6bde87ec9348 100644 --- a/libs/common/src/test/java/org/opensearch/common/annotation/processor/CompilerSupport.java +++ b/libs/common/src/test/java/org/opensearch/common/annotation/processor/CompilerSupport.java @@ -29,6 +29,7 @@ import java.net.URI; import java.net.URL; import java.nio.charset.StandardCharsets; +import java.nio.file.Path; import java.security.AccessController; import java.security.PrivilegedAction; import java.util.Arrays; @@ -39,12 +40,12 @@ import java.util.stream.Stream; interface CompilerSupport { - default CompilerResult compile(String name, String... names) { - return compileWithPackage(ApiAnnotationProcessorTests.class.getPackageName(), name, names); + default CompilerResult compile(Path outputDirectory, String name, String... names) { + return compileWithPackage(outputDirectory, ApiAnnotationProcessorTests.class.getPackageName(), name, names); } @SuppressWarnings("removal") - default CompilerResult compileWithPackage(String pck, String name, String... names) { + default CompilerResult compileWithPackage(Path outputDirectory, String pck, String name, String... names) { final JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); final DiagnosticCollector collector = new DiagnosticCollector<>(); @@ -54,11 +55,18 @@ default CompilerResult compileWithPackage(String pck, String name, String... nam .map(f -> asSource(pck, f)) .collect(Collectors.toList()); - final CompilationTask task = compiler.getTask(out, fileManager, collector, null, null, files); + final CompilationTask task = compiler.getTask( + out, + fileManager, + collector, + List.of("-d", outputDirectory.toString()), + null, + files + ); task.setProcessors(Collections.singleton(new ApiAnnotationProcessor())); if (AccessController.doPrivileged((PrivilegedAction) () -> task.call())) { - return new Success(); + return new Success(collector.getDiagnostics()); } else { return new Failure(collector.getDiagnostics()); } @@ -81,16 +89,10 @@ public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOExcept }; } - class CompilerResult {} - - class Success extends CompilerResult { - - } - - class Failure extends CompilerResult { + class CompilerResult { private final List> diagnotics; - Failure(List> diagnotics) { + CompilerResult(List> diagnotics) { this.diagnotics = diagnotics; } @@ -99,6 +101,18 @@ List> diagnotics() { } } + class Success extends CompilerResult { + Success(List> diagnotics) { + super(diagnotics); + } + } + + class Failure extends CompilerResult { + Failure(List> diagnotics) { + super(diagnotics); + } + } + class HasDiagnostic extends TypeSafeMatcher> { private final Diagnostic.Kind kind; private final Matcher matcher; diff --git a/server/src/main/java/org/opensearch/index/translog/transfer/TranslogTransferManager.java b/server/src/main/java/org/opensearch/index/translog/transfer/TranslogTransferManager.java index 924669d0e46a9..1e621d6cb7688 100644 --- a/server/src/main/java/org/opensearch/index/translog/transfer/TranslogTransferManager.java +++ b/server/src/main/java/org/opensearch/index/translog/transfer/TranslogTransferManager.java @@ -206,7 +206,8 @@ public boolean transferSnapshot(TransferSnapshot transferSnapshot, TranslogTrans } catch (Exception ex) { logger.error(() -> new ParameterizedMessage("Transfer failed for snapshot {}", transferSnapshot), ex); captureStatsOnUploadFailure(); - translogTransferListener.onUploadFailed(transferSnapshot, ex); + Exception exWithoutSuppressed = new TranslogUploadFailedException(ex.getMessage()); + translogTransferListener.onUploadFailed(transferSnapshot, exWithoutSuppressed); return false; } } diff --git a/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java b/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java index 63501f878d55d..fa6bcc3372fb7 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java @@ -800,9 +800,7 @@ public void testDeleteStaleIndexRoutingPathsThrowsIOException() throws IOExcepti doThrow(new IOException("test exception")).when(blobContainer).deleteBlobsIgnoringIfNotExists(Mockito.anyList()); remoteRoutingTableService.doStart(); - IOException thrown = assertThrows(IOException.class, () -> { - remoteRoutingTableService.deleteStaleIndexRoutingPaths(stalePaths); - }); + IOException thrown = assertThrows(IOException.class, () -> { remoteRoutingTableService.deleteStaleIndexRoutingPaths(stalePaths); }); assertEquals("test exception", thrown.getMessage()); verify(blobContainer).deleteBlobsIgnoringIfNotExists(stalePaths); } @@ -823,9 +821,10 @@ public void testDeleteStaleIndexRoutingDiffPathsThrowsIOException() throws IOExc doThrow(new IOException("test exception")).when(blobContainer).deleteBlobsIgnoringIfNotExists(Mockito.anyList()); remoteRoutingTableService.doStart(); - IOException thrown = assertThrows(IOException.class, () -> { - remoteRoutingTableService.deleteStaleIndexRoutingDiffPaths(stalePaths); - }); + IOException thrown = assertThrows( + IOException.class, + () -> { remoteRoutingTableService.deleteStaleIndexRoutingDiffPaths(stalePaths); } + ); assertEquals("test exception", thrown.getMessage()); verify(blobContainer).deleteBlobsIgnoringIfNotExists(stalePaths); } diff --git a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/builder/StarTreesBuilderTests.java b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/builder/StarTreesBuilderTests.java index 4ab21dbce059f..337f948d6da97 100644 --- a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/builder/StarTreesBuilderTests.java +++ b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/builder/StarTreesBuilderTests.java @@ -104,16 +104,32 @@ public void test_buildWithNoStarTreeFields() throws IOException { public void test_getStarTreeBuilder() throws IOException { when(mapperService.getCompositeFieldTypes()).thenReturn(Set.of(starTreeFieldType)); StarTreesBuilder starTreesBuilder = new StarTreesBuilder(segmentWriteState, mapperService, new AtomicInteger()); - StarTreeBuilder starTreeBuilder = starTreesBuilder.getStarTreeBuilder(metaOut, dataOut, starTreeField, segmentWriteState, mapperService); + StarTreeBuilder starTreeBuilder = starTreesBuilder.getStarTreeBuilder( + metaOut, + dataOut, + starTreeField, + segmentWriteState, + mapperService + ); assertTrue(starTreeBuilder instanceof OnHeapStarTreeBuilder); } public void test_getStarTreeBuilder_illegalArgument() throws IOException { when(mapperService.getCompositeFieldTypes()).thenReturn(Set.of(starTreeFieldType)); - StarTreeFieldConfiguration starTreeFieldConfiguration = new StarTreeFieldConfiguration(1, new HashSet<>(), StarTreeFieldConfiguration.StarTreeBuildMode.OFF_HEAP); + StarTreeFieldConfiguration starTreeFieldConfiguration = new StarTreeFieldConfiguration( + 1, + new HashSet<>(), + StarTreeFieldConfiguration.StarTreeBuildMode.OFF_HEAP + ); StarTreeField starTreeField = new StarTreeField("star_tree", new ArrayList<>(), new ArrayList<>(), starTreeFieldConfiguration); StarTreesBuilder starTreesBuilder = new StarTreesBuilder(segmentWriteState, mapperService, new AtomicInteger()); - StarTreeBuilder starTreeBuilder = starTreesBuilder.getStarTreeBuilder(metaOut, dataOut, starTreeField, segmentWriteState, mapperService); + StarTreeBuilder starTreeBuilder = starTreesBuilder.getStarTreeBuilder( + metaOut, + dataOut, + starTreeField, + segmentWriteState, + mapperService + ); assertTrue(starTreeBuilder instanceof OffHeapStarTreeBuilder); starTreeBuilder.close(); } diff --git a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java index df3df81361a12..cd2f4cc1eb079 100644 --- a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java +++ b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java @@ -135,9 +135,12 @@ public void testGetPrimaryTermGenerationUuid() { } public void testInitException() throws IOException { - when(remoteMetadataDirectory.listFilesByPrefixInLexicographicOrder(RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX, METADATA_FILES_TO_FETCH)).thenThrow( - new IOException("Error") - ); + when( + remoteMetadataDirectory.listFilesByPrefixInLexicographicOrder( + RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX, + METADATA_FILES_TO_FETCH + ) + ).thenThrow(new IOException("Error")); assertThrows(IOException.class, () -> remoteSegmentStoreDirectory.init()); } @@ -155,9 +158,12 @@ public void testInitNoMetadataFile() throws IOException { } public void testInitMultipleMetadataFile() throws IOException { - when(remoteMetadataDirectory.listFilesByPrefixInLexicographicOrder(RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX, METADATA_FILES_TO_FETCH)).thenReturn( - List.of(metadataFilename, metadataFilenameDup) - ); + when( + remoteMetadataDirectory.listFilesByPrefixInLexicographicOrder( + RemoteSegmentStoreDirectory.MetadataFilenameUtils.METADATA_PREFIX, + METADATA_FILES_TO_FETCH + ) + ).thenReturn(List.of(metadataFilename, metadataFilenameDup)); assertThrows(IllegalStateException.class, () -> remoteSegmentStoreDirectory.init()); } diff --git a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryWithPinnedTimestampTests.java b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryWithPinnedTimestampTests.java index e71023125d4cd..2531462d21d40 100644 --- a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryWithPinnedTimestampTests.java +++ b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryWithPinnedTimestampTests.java @@ -187,7 +187,8 @@ public void testDeleteStaleCommitsPinnedTimestampMdFile() throws Exception { ) ).thenReturn(List.of(metadataFilename, metadataFilename2, metadataFilename3)); - long pinnedTimestampMatchingMetadataFilename2 = RemoteSegmentStoreDirectory.MetadataFilenameUtils.getTimestamp(metadataFilename2) + 10; + long pinnedTimestampMatchingMetadataFilename2 = RemoteSegmentStoreDirectory.MetadataFilenameUtils.getTimestamp(metadataFilename2) + + 10; String blobName = "snapshot1__" + pinnedTimestampMatchingMetadataFilename2; when(blobContainer.listBlobs()).thenReturn(Map.of(blobName, new PlainBlobMetadata(blobName, 100))); diff --git a/server/src/test/java/org/opensearch/index/translog/transfer/TranslogTransferManagerTests.java b/server/src/test/java/org/opensearch/index/translog/transfer/TranslogTransferManagerTests.java index ed0d6b7d50706..77dfd5b27581d 100644 --- a/server/src/test/java/org/opensearch/index/translog/transfer/TranslogTransferManagerTests.java +++ b/server/src/test/java/org/opensearch/index/translog/transfer/TranslogTransferManagerTests.java @@ -206,6 +206,80 @@ public void onUploadFailed(TransferSnapshot transferSnapshot, Exception ex) { assertEquals(4, fileTransferTracker.allUploaded().size()); } + public void testTransferSnapshotOnFileTransferUploadFail() throws Exception { + AtomicInteger fileTransferSucceeded = new AtomicInteger(); + AtomicInteger fileTransferFailed = new AtomicInteger(); + AtomicInteger translogTransferSucceeded = new AtomicInteger(); + AtomicInteger translogTransferFailed = new AtomicInteger(); + + doAnswer(invocationOnMock -> { + ActionListener listener = (ActionListener) invocationOnMock.getArguments()[2]; + Set transferFileSnapshots = (Set) invocationOnMock.getArguments()[0]; + + TransferFileSnapshot actualFileSnapshot = transferFileSnapshots.iterator().next(); + FileTransferException testException = new FileTransferException( + actualFileSnapshot, + new RuntimeException("FileTransferUploadNeedsToFail-Exception") + ); + + listener.onFailure(testException); + transferFileSnapshots.stream().skip(1).forEach(listener::onResponse); + return null; + }).when(transferService).uploadBlobs(anySet(), anyMap(), any(ActionListener.class), any(WritePriority.class)); + + FileTransferTracker fileTransferTracker = new FileTransferTracker( + new ShardId("index", "indexUUid", 0), + remoteTranslogTransferTracker + ) { + @Override + public void onSuccess(TransferFileSnapshot fileSnapshot) { + fileTransferSucceeded.incrementAndGet(); + super.onSuccess(fileSnapshot); + } + + @Override + public void onFailure(TransferFileSnapshot fileSnapshot, Exception e) { + fileTransferFailed.incrementAndGet(); + super.onFailure(fileSnapshot, e); + } + }; + + TranslogTransferManager translogTransferManager = new TranslogTransferManager( + shardId, + transferService, + remoteBaseTransferPath.add(TRANSLOG.getName()), + remoteBaseTransferPath.add(METADATA.getName()), + fileTransferTracker, + remoteTranslogTransferTracker, + DefaultRemoteStoreSettings.INSTANCE, + isTranslogMetadataEnabled + ); + + SetOnce exception = new SetOnce<>(); + assertFalse(translogTransferManager.transferSnapshot(createTransferSnapshot(), new TranslogTransferListener() { + @Override + public void onUploadComplete(TransferSnapshot transferSnapshot) { + translogTransferSucceeded.incrementAndGet(); + } + + @Override + public void onUploadFailed(TransferSnapshot transferSnapshot, Exception ex) { + translogTransferFailed.incrementAndGet(); + exception.set(ex); + } + })); + + assertNotNull(exception.get()); + assertTrue(exception.get() instanceof TranslogUploadFailedException); + assertEquals("Failed to upload 1 files during transfer", exception.get().getMessage()); + assertEquals(0, exception.get().getSuppressed().length); + assertEquals(3, fileTransferSucceeded.get()); + assertEquals(1, fileTransferFailed.get()); + assertEquals(0, translogTransferSucceeded.get()); + assertEquals(1, translogTransferFailed.get()); + assertEquals(3, fileTransferTracker.allUploaded().size()); + } + public void testTransferSnapshotOnUploadTimeout() throws Exception { doAnswer(invocationOnMock -> { Set transferFileSnapshots = invocationOnMock.getArgument(0); diff --git a/server/src/test/java/org/opensearch/indices/replication/RemoteStoreReplicationSourceTests.java b/server/src/test/java/org/opensearch/indices/replication/RemoteStoreReplicationSourceTests.java index 287962b158c79..b41c8718ec23d 100644 --- a/server/src/test/java/org/opensearch/indices/replication/RemoteStoreReplicationSourceTests.java +++ b/server/src/test/java/org/opensearch/indices/replication/RemoteStoreReplicationSourceTests.java @@ -166,8 +166,12 @@ private void buildIndexShardBehavior(IndexShard mockShard, IndexShard indexShard when(mockShard.getSegmentInfosSnapshot()).thenReturn(indexShard.getSegmentInfosSnapshot()); Store remoteStore = mock(Store.class); when(mockShard.remoteStore()).thenReturn(remoteStore); - RemoteSegmentStoreDirectory remoteSegmentStoreDirectory = (RemoteSegmentStoreDirectory) ((FilterDirectory) ((FilterDirectory) indexShard.remoteStore().directory()).getDelegate()).getDelegate(); - FilterDirectory remoteStoreFilterDirectory = new RemoteStoreRefreshListenerTests.TestFilterDirectory(new RemoteStoreRefreshListenerTests.TestFilterDirectory(remoteSegmentStoreDirectory)); + RemoteSegmentStoreDirectory remoteSegmentStoreDirectory = + (RemoteSegmentStoreDirectory) ((FilterDirectory) ((FilterDirectory) indexShard.remoteStore().directory()).getDelegate()) + .getDelegate(); + FilterDirectory remoteStoreFilterDirectory = new RemoteStoreRefreshListenerTests.TestFilterDirectory( + new RemoteStoreRefreshListenerTests.TestFilterDirectory(remoteSegmentStoreDirectory) + ); when(remoteStore.directory()).thenReturn(remoteStoreFilterDirectory); } }