From 969a0535648386275b1ad40ec7ef1f6a518a0c85 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Fri, 30 Mar 2018 10:15:19 -0600 Subject: [PATCH 1/7] Move Streams.copy into elasticsearch-core and make a multi-release jar This moves the method `Streams.copy(InputStream in, OutputStream out)` into the `elasticsearch-core` project (inside the `o.e.core.internal.io` package). It also makes this class into a multi-release class where the Java 9 equivalent uses `InputStream#transferTo`. This is a followup from https://github.com/elastic/elasticsearch/pull/29300#discussion_r178147495 --- libs/elasticsearch-core/build.gradle | 43 +++++++++--- .../core/internal/io/Streams.java | 69 +++++++++++++++++++ .../core/internal/io/Streams.java | 48 +++++++++++++ .../core/internal/io/StreamsTests.java | 42 +++++++++++ .../azure/AzureStorageServiceMock.java | 2 +- .../gcs/GoogleCloudStorageFixture.java | 2 +- .../common/blobstore/fs/FsBlobContainer.java | 4 +- .../common/compress/CompressorFactory.java | 2 +- .../xcontent/json/JsonXContentGenerator.java | 2 +- .../recovery/RecoverySourceHandler.java | 2 +- .../blobstore/BlobStoreRepository.java | 2 +- .../blobstore/ChecksumBlobStoreFormat.java | 2 +- .../elasticsearch/rest/RestController.java | 3 +- .../tasks/TaskResultsService.java | 6 +- .../elasticsearch/common/io/StreamsTests.java | 17 +---- .../elasticsearch/search/geo/GeoFilterIT.java | 2 +- .../org/elasticsearch/test/StreamsUtils.java | 6 +- 17 files changed, 211 insertions(+), 43 deletions(-) create mode 100644 libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io/Streams.java create mode 100644 libs/elasticsearch-core/src/main/java9/org/elasticsearch/core/internal/io/Streams.java create mode 100644 libs/elasticsearch-core/src/test/java/org/elasticsearch/core/internal/io/StreamsTests.java diff --git a/libs/elasticsearch-core/build.gradle b/libs/elasticsearch-core/build.gradle index dea5664a14fd1..96926e1d32035 100644 --- a/libs/elasticsearch-core/build.gradle +++ b/libs/elasticsearch-core/build.gradle @@ -52,6 +52,29 @@ forbiddenApisMain { signaturesURLs = [PrecommitTasks.getResource('/forbidden/jdk-signatures.txt')] } +// we want to keep the JDKs in our IDEs set to JDK 8 until minimum JDK is bumped to 9 so we do not include this source set in our IDEs +if (!isEclipse && !isIdea) { + sourceSets { + java9 { + java { + srcDirs = ['src/main/java9'] + } + } + } + + compileJava9Java { + sourceCompatibility = 9 + targetCompatibility = 9 + } + + jar { + into('META-INF/versions/9') { + from sourceSets.java9.output + } + manifest.attributes('Multi-Release': 'true') + } +} + if (isEclipse) { // in eclipse the project is under a fake root, we need to change around the source sets sourceSets { @@ -66,14 +89,14 @@ if (isEclipse) { } thirdPartyAudit.excludes = [ - // from log4j - 'org/osgi/framework/AdaptPermission', - 'org/osgi/framework/AdminPermission', - 'org/osgi/framework/Bundle', - 'org/osgi/framework/BundleActivator', - 'org/osgi/framework/BundleContext', - 'org/osgi/framework/BundleEvent', - 'org/osgi/framework/SynchronousBundleListener', - 'org/osgi/framework/wiring/BundleWire', - 'org/osgi/framework/wiring/BundleWiring' + // from log4j + 'org/osgi/framework/AdaptPermission', + 'org/osgi/framework/AdminPermission', + 'org/osgi/framework/Bundle', + 'org/osgi/framework/BundleActivator', + 'org/osgi/framework/BundleContext', + 'org/osgi/framework/BundleEvent', + 'org/osgi/framework/SynchronousBundleListener', + 'org/osgi/framework/wiring/BundleWire', + 'org/osgi/framework/wiring/BundleWiring' ] diff --git a/libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io/Streams.java b/libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io/Streams.java new file mode 100644 index 0000000000000..f84acb55fcc63 --- /dev/null +++ b/libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io/Streams.java @@ -0,0 +1,69 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you 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.elasticsearch.core.internal.io; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.util.Objects; + +/** + * Simple utility methods for file and stream copying. + * All copy methods use a block size of 4096 bytes, + * and close all affected streams when done. + *

+ * Mainly for use within the framework, + * but also useful for application code. + */ +public class Streams { + + /** + * Copy the contents of the given InputStream to the given OutputStream. + * Closes both streams when done. + * + * @param in the stream to copy from + * @param out the stream to copy to + * @return the number of bytes copied + * @throws IOException in case of I/O errors + */ + public static long copy(InputStream in, OutputStream out) throws IOException { + Objects.requireNonNull(in, "No InputStream specified"); + Objects.requireNonNull(out, "No OutputStream specified"); + final byte[] buffer = new byte[8192]; + boolean success = false; + try { + long byteCount = 0; + int bytesRead; + while ((bytesRead = in.read(buffer)) != -1) { + out.write(buffer, 0, bytesRead); + byteCount += bytesRead; + } + out.flush(); + success = true; + return byteCount; + } finally { + if (success) { + IOUtils.close(in, out); + } else { + IOUtils.closeWhileHandlingException(in, out); + } + } + } +} diff --git a/libs/elasticsearch-core/src/main/java9/org/elasticsearch/core/internal/io/Streams.java b/libs/elasticsearch-core/src/main/java9/org/elasticsearch/core/internal/io/Streams.java new file mode 100644 index 0000000000000..56a8734838662 --- /dev/null +++ b/libs/elasticsearch-core/src/main/java9/org/elasticsearch/core/internal/io/Streams.java @@ -0,0 +1,48 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you 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.elasticsearch.core.internal.io; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; + +/** + * Simple utility methods for file and stream copying. + * All copy methods use a block size of 4096 bytes, + * and close all affected streams when done. + *

+ * Mainly for use within the framework, + * but also useful for application code. + */ +public abstract class Streams { + + /** + * Copy the contents of the given InputStream to the given OutputStream. + * Closes both streams when done. + * + * @param in the stream to copy from + * @param out the stream to copy to + * @return the number of bytes copied + * @throws IOException in case of I/O errors + */ + public static long copy(InputStream in, OutputStream out) throws IOException { + return in.transferTo(out); + } +} diff --git a/libs/elasticsearch-core/src/test/java/org/elasticsearch/core/internal/io/StreamsTests.java b/libs/elasticsearch-core/src/test/java/org/elasticsearch/core/internal/io/StreamsTests.java new file mode 100644 index 0000000000000..3908ef83500c4 --- /dev/null +++ b/libs/elasticsearch-core/src/test/java/org/elasticsearch/core/internal/io/StreamsTests.java @@ -0,0 +1,42 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you 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.elasticsearch.core.internal.io; + +import org.elasticsearch.test.ESTestCase; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; + +import static org.hamcrest.Matchers.equalTo; + +public class StreamsTests extends ESTestCase { + public void testCopyFromInputStream() throws IOException { + byte[] content = "content".getBytes(StandardCharsets.UTF_8); + ByteArrayInputStream in = new ByteArrayInputStream(content); + ByteArrayOutputStream out = new ByteArrayOutputStream(content.length); + long count = Streams.copy(in, out); + + assertThat(count, equalTo((long) content.length)); + assertThat(Arrays.equals(content, out.toByteArray()), equalTo(true)); + } +} diff --git a/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureStorageServiceMock.java b/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureStorageServiceMock.java index 68b84594d62ca..7b360f41100ee 100644 --- a/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureStorageServiceMock.java +++ b/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureStorageServiceMock.java @@ -25,8 +25,8 @@ import org.elasticsearch.common.blobstore.support.PlainBlobMetaData; import org.elasticsearch.common.collect.MapBuilder; import org.elasticsearch.common.component.AbstractComponent; -import org.elasticsearch.common.io.Streams; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.core.internal.io.Streams; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; diff --git a/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageFixture.java b/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageFixture.java index cddcab870de34..1f5ec2f2dd819 100644 --- a/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageFixture.java +++ b/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageFixture.java @@ -22,7 +22,7 @@ import com.sun.net.httpserver.HttpHandler; import com.sun.net.httpserver.HttpServer; import org.elasticsearch.common.SuppressForbidden; -import org.elasticsearch.common.io.Streams; +import org.elasticsearch.core.internal.io.Streams; import org.elasticsearch.mocksocket.MockHttpServer; import org.elasticsearch.repositories.gcs.GoogleCloudStorageTestServer.Response; diff --git a/server/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobContainer.java b/server/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobContainer.java index df2e7a123a3ae..f960664306f08 100644 --- a/server/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobContainer.java +++ b/server/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobContainer.java @@ -24,7 +24,7 @@ import org.elasticsearch.common.blobstore.BlobPath; import org.elasticsearch.common.blobstore.support.AbstractBlobContainer; import org.elasticsearch.common.blobstore.support.PlainBlobMetaData; -import org.elasticsearch.common.io.Streams; +import org.elasticsearch.core.internal.io.Streams; import java.io.BufferedInputStream; import java.io.FileNotFoundException; @@ -128,7 +128,7 @@ public void writeBlob(String blobName, InputStream inputStream, long blobSize) t } final Path file = path.resolve(blobName); try (OutputStream outputStream = Files.newOutputStream(file, StandardOpenOption.CREATE_NEW)) { - Streams.copy(inputStream, outputStream, new byte[blobStore.bufferSizeInBytes()]); + Streams.copy(inputStream, outputStream); } IOUtils.fsync(file, false); IOUtils.fsync(path, true); diff --git a/server/src/main/java/org/elasticsearch/common/compress/CompressorFactory.java b/server/src/main/java/org/elasticsearch/common/compress/CompressorFactory.java index 332d9024e997f..3b1202fe66f42 100644 --- a/server/src/main/java/org/elasticsearch/common/compress/CompressorFactory.java +++ b/server/src/main/java/org/elasticsearch/common/compress/CompressorFactory.java @@ -21,11 +21,11 @@ import org.elasticsearch.common.Nullable; import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.common.io.Streams; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.core.internal.io.Streams; import java.io.IOException; import java.util.Objects; diff --git a/server/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContentGenerator.java b/server/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContentGenerator.java index 667a399096fd4..8bf495c0a1a47 100644 --- a/server/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContentGenerator.java +++ b/server/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContentGenerator.java @@ -28,7 +28,6 @@ import com.fasterxml.jackson.core.util.DefaultIndenter; import com.fasterxml.jackson.core.util.DefaultPrettyPrinter; import com.fasterxml.jackson.core.util.JsonGeneratorDelegate; -import org.elasticsearch.common.io.Streams; import org.elasticsearch.common.xcontent.DeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContent; @@ -38,6 +37,7 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.common.xcontent.support.filtering.FilterPathBasedFilter; +import org.elasticsearch.core.internal.io.Streams; import java.io.BufferedInputStream; import java.io.IOException; diff --git a/server/src/main/java/org/elasticsearch/indices/recovery/RecoverySourceHandler.java b/server/src/main/java/org/elasticsearch/indices/recovery/RecoverySourceHandler.java index 2189e6b2fb2a8..710b4bc46e235 100644 --- a/server/src/main/java/org/elasticsearch/indices/recovery/RecoverySourceHandler.java +++ b/server/src/main/java/org/elasticsearch/indices/recovery/RecoverySourceHandler.java @@ -38,13 +38,13 @@ import org.elasticsearch.common.Nullable; import org.elasticsearch.common.StopWatch; import org.elasticsearch.common.bytes.BytesArray; -import org.elasticsearch.common.io.Streams; import org.elasticsearch.common.lease.Releasable; import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.lucene.store.InputStreamIndexInput; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.util.CancellableThreads; +import org.elasticsearch.core.internal.io.Streams; import org.elasticsearch.index.engine.Engine; import org.elasticsearch.index.engine.RecoveryEngineException; import org.elasticsearch.index.seqno.LocalCheckpointTracker; diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index 0f8e29d7f3835..f1adf9273ffde 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -55,7 +55,6 @@ import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.component.AbstractLifecycleComponent; import org.elasticsearch.common.compress.NotXContentException; -import org.elasticsearch.common.io.Streams; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.OutputStreamStreamOutput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -75,6 +74,7 @@ import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.core.internal.io.Streams; import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.index.snapshots.IndexShardRestoreFailedException; diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/ChecksumBlobStoreFormat.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/ChecksumBlobStoreFormat.java index a959cd0efb87e..8c8139d5abd6a 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/ChecksumBlobStoreFormat.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/ChecksumBlobStoreFormat.java @@ -28,7 +28,6 @@ import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.compress.CompressorFactory; -import org.elasticsearch.common.io.Streams; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.lucene.store.ByteArrayIndexInput; @@ -39,6 +38,7 @@ import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.core.internal.io.Streams; import org.elasticsearch.gateway.CorruptStateException; import java.io.ByteArrayOutputStream; diff --git a/server/src/main/java/org/elasticsearch/rest/RestController.java b/server/src/main/java/org/elasticsearch/rest/RestController.java index d6aba28ce27eb..111663497d766 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestController.java +++ b/server/src/main/java/org/elasticsearch/rest/RestController.java @@ -27,7 +27,6 @@ import org.elasticsearch.common.breaker.CircuitBreaker; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.component.AbstractComponent; -import org.elasticsearch.common.io.Streams; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.path.PathTrie; @@ -35,6 +34,7 @@ import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.core.internal.io.Streams; import org.elasticsearch.http.HttpServerTransport; import org.elasticsearch.indices.breaker.CircuitBreakerService; import org.elasticsearch.usage.UsageService; @@ -51,7 +51,6 @@ import java.util.Optional; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.function.Supplier; import java.util.function.UnaryOperator; import static org.elasticsearch.rest.RestStatus.BAD_REQUEST; diff --git a/server/src/main/java/org/elasticsearch/tasks/TaskResultsService.java b/server/src/main/java/org/elasticsearch/tasks/TaskResultsService.java index de63994457a1f..6ec949a0c918b 100644 --- a/server/src/main/java/org/elasticsearch/tasks/TaskResultsService.java +++ b/server/src/main/java/org/elasticsearch/tasks/TaskResultsService.java @@ -19,10 +19,9 @@ package org.elasticsearch.tasks; import org.apache.logging.log4j.message.ParameterizedMessage; -import org.apache.logging.log4j.util.Supplier; -import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ExceptionsHelper; +import org.elasticsearch.ResourceAlreadyExistsException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.indices.create.CreateIndexRequest; import org.elasticsearch.action.admin.indices.create.CreateIndexResponse; @@ -38,13 +37,12 @@ import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.common.io.Streams; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; -import org.elasticsearch.ResourceAlreadyExistsException; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.core.internal.io.Streams; import java.io.ByteArrayOutputStream; import java.io.IOException; diff --git a/server/src/test/java/org/elasticsearch/common/io/StreamsTests.java b/server/src/test/java/org/elasticsearch/common/io/StreamsTests.java index 76b52c08a854f..ee1933e3a1043 100644 --- a/server/src/test/java/org/elasticsearch/common/io/StreamsTests.java +++ b/server/src/test/java/org/elasticsearch/common/io/StreamsTests.java @@ -24,7 +24,6 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.test.ESTestCase; -import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.StringReader; @@ -32,7 +31,6 @@ import java.nio.charset.StandardCharsets; import java.util.Arrays; -import static org.elasticsearch.common.io.Streams.copy; import static org.elasticsearch.common.io.Streams.copyToString; import static org.hamcrest.Matchers.equalTo; @@ -40,20 +38,11 @@ * Unit tests for {@link org.elasticsearch.common.io.Streams}. */ public class StreamsTests extends ESTestCase { - public void testCopyFromInputStream() throws IOException { - byte[] content = "content".getBytes(StandardCharsets.UTF_8); - ByteArrayInputStream in = new ByteArrayInputStream(content); - ByteArrayOutputStream out = new ByteArrayOutputStream(content.length); - long count = copy(in, out); - - assertThat(count, equalTo((long) content.length)); - assertThat(Arrays.equals(content, out.toByteArray()), equalTo(true)); - } public void testCopyFromByteArray() throws IOException { byte[] content = "content".getBytes(StandardCharsets.UTF_8); ByteArrayOutputStream out = new ByteArrayOutputStream(content.length); - copy(content, out); + Streams.copy(content, out); assertThat(Arrays.equals(content, out.toByteArray()), equalTo(true)); } @@ -61,7 +50,7 @@ public void testCopyFromReader() throws IOException { String content = "content"; StringReader in = new StringReader(content); StringWriter out = new StringWriter(); - int count = copy(in, out); + int count = Streams.copy(in, out); assertThat(content.length(), equalTo(count)); assertThat(out.toString(), equalTo(content)); } @@ -69,7 +58,7 @@ public void testCopyFromReader() throws IOException { public void testCopyFromString() throws IOException { String content = "content"; StringWriter out = new StringWriter(); - copy(content, out); + Streams.copy(content, out); assertThat(out.toString(), equalTo(content)); } diff --git a/server/src/test/java/org/elasticsearch/search/geo/GeoFilterIT.java b/server/src/test/java/org/elasticsearch/search/geo/GeoFilterIT.java index 3b1002a6f68c4..fa0531262bb1d 100644 --- a/server/src/test/java/org/elasticsearch/search/geo/GeoFilterIT.java +++ b/server/src/test/java/org/elasticsearch/search/geo/GeoFilterIT.java @@ -42,12 +42,12 @@ import org.elasticsearch.common.geo.builders.MultiPolygonBuilder; import org.elasticsearch.common.geo.builders.PointBuilder; import org.elasticsearch.common.geo.builders.PolygonBuilder; -import org.elasticsearch.common.io.Streams; import org.elasticsearch.common.logging.ESLoggerFactory; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.core.internal.io.Streams; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.search.SearchHit; diff --git a/test/framework/src/main/java/org/elasticsearch/test/StreamsUtils.java b/test/framework/src/main/java/org/elasticsearch/test/StreamsUtils.java index 1d0eaa7ce5154..767b74e447230 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/StreamsUtils.java +++ b/test/framework/src/main/java/org/elasticsearch/test/StreamsUtils.java @@ -20,8 +20,8 @@ package org.elasticsearch.test; import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.common.io.Streams; import org.elasticsearch.common.io.stream.BytesStreamOutput; +import org.elasticsearch.core.internal.io.Streams; import java.io.FileNotFoundException; import java.io.IOException; @@ -36,7 +36,7 @@ public static String copyToStringFromClasspath(ClassLoader classLoader, String p if (is == null) { throw new FileNotFoundException("Resource [" + path + "] not found in classpath with class loader [" + classLoader + "]"); } - return Streams.copyToString(new InputStreamReader(is, StandardCharsets.UTF_8)); + return org.elasticsearch.common.io.Streams.copyToString(new InputStreamReader(is, StandardCharsets.UTF_8)); } public static String copyToStringFromClasspath(String path) throws IOException { @@ -44,7 +44,7 @@ public static String copyToStringFromClasspath(String path) throws IOException { if (is == null) { throw new FileNotFoundException("Resource [" + path + "] not found in classpath"); } - return Streams.copyToString(new InputStreamReader(is, StandardCharsets.UTF_8)); + return org.elasticsearch.common.io.Streams.copyToString(new InputStreamReader(is, StandardCharsets.UTF_8)); } public static byte[] copyToBytesFromClasspath(String path) throws IOException { From 7753e01ffd791f6feb95b37d344dafedc1fb85f4 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Mon, 2 Apr 2018 11:08:59 -0600 Subject: [PATCH 2/7] Bring feature parity to the Java9 version of Streams --- .../core/internal/io/Streams.java | 51 +++++++++++++++++-- 1 file changed, 48 insertions(+), 3 deletions(-) diff --git a/libs/elasticsearch-core/src/main/java9/org/elasticsearch/core/internal/io/Streams.java b/libs/elasticsearch-core/src/main/java9/org/elasticsearch/core/internal/io/Streams.java index 56a8734838662..a63c351283bd6 100644 --- a/libs/elasticsearch-core/src/main/java9/org/elasticsearch/core/internal/io/Streams.java +++ b/libs/elasticsearch-core/src/main/java9/org/elasticsearch/core/internal/io/Streams.java @@ -25,8 +25,7 @@ /** * Simple utility methods for file and stream copying. - * All copy methods use a block size of 4096 bytes, - * and close all affected streams when done. + * All copy methods close all affected streams when done. *

* Mainly for use within the framework, * but also useful for application code. @@ -43,6 +42,52 @@ public abstract class Streams { * @throws IOException in case of I/O errors */ public static long copy(InputStream in, OutputStream out) throws IOException { - return in.transferTo(out); + boolean success = false; + try { + final long byteCount = in.transferTo(out); + out.flush(); + success = true; + return byteCount; + } finally { + if (success) { + Exception ex = null; + try { + in.close(); + } catch (IOException | RuntimeException e) { + ex = e; + } + + try { + out.close(); + } catch (IOException | RuntimeException e) { + if (ex == null) { + ex = e; + } else { + ex.addSuppressed(e); + } + } + + if (ex != null) { + if (ex instanceof IOException) { + throw (IOException) ex; + } else { + throw (RuntimeException) ex; + } + } + } else { + try { + in.close(); + } catch (IOException | RuntimeException e) { + // empty + } + + try { + out.close(); + } catch (IOException | RuntimeException e) { + // empty + } + } + } + } } From 8d188af5a14c24b4f121b9b3d5c832db459a90d2 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Wed, 4 Apr 2018 10:20:05 -0600 Subject: [PATCH 3/7] Fix es-core build.gradle for the 9 MR jar --- libs/elasticsearch-core/build.gradle | 66 ++++++++++++++++++---------- 1 file changed, 43 insertions(+), 23 deletions(-) diff --git a/libs/elasticsearch-core/build.gradle b/libs/elasticsearch-core/build.gradle index 96926e1d32035..26a232664ad33 100644 --- a/libs/elasticsearch-core/build.gradle +++ b/libs/elasticsearch-core/build.gradle @@ -26,6 +26,45 @@ apply plugin: 'nebula.maven-scm' archivesBaseName = 'elasticsearch-core' +// we want to keep the JDKs in our IDEs set to JDK 8 until minimum JDK is bumped to 9 so we do not include this source set in our IDEs +if (!isEclipse && !isIdea) { + sourceSets { + java9 { + java { + srcDirs = ['src/main/java9'] + } + } + } + + configurations { + java9Compile.extendsFrom(compile) + } + + dependencies { + java9Compile sourceSets.main.output + } + + compileJava9Java { + sourceCompatibility = 9 + targetCompatibility = 9 + } + + /* Enable this when forbiddenapis was updated to 2.6. + * See: https://github.com/elastic/elasticsearch/issues/29292 + forbiddenApisJava9 { + targetCompatibility = 9 + } + */ + + jar { + metaInf { + into 'versions/9' + from sourceSets.java9.output + } + manifest.attributes('Multi-Release': 'true') + } +} + publishing { publications { nebula { @@ -39,6 +78,10 @@ dependencies { testCompile "junit:junit:${versions.junit}" testCompile "org.hamcrest:hamcrest-all:${versions.hamcrest}" + if (!isEclipse && !isIdea) { + java9Compile sourceSets.main.output + } + if (isEclipse == false || project.path == ":libs:elasticsearch-core-tests") { testCompile("org.elasticsearch.test:framework:${version}") { exclude group: 'org.elasticsearch', module: 'elasticsearch-core' @@ -52,29 +95,6 @@ forbiddenApisMain { signaturesURLs = [PrecommitTasks.getResource('/forbidden/jdk-signatures.txt')] } -// we want to keep the JDKs in our IDEs set to JDK 8 until minimum JDK is bumped to 9 so we do not include this source set in our IDEs -if (!isEclipse && !isIdea) { - sourceSets { - java9 { - java { - srcDirs = ['src/main/java9'] - } - } - } - - compileJava9Java { - sourceCompatibility = 9 - targetCompatibility = 9 - } - - jar { - into('META-INF/versions/9') { - from sourceSets.java9.output - } - manifest.attributes('Multi-Release': 'true') - } -} - if (isEclipse) { // in eclipse the project is under a fake root, we need to change around the source sets sourceSets { From 5242f9e2b403f704c0e31282a8355dbd874b467d Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Wed, 4 Apr 2018 10:34:12 -0600 Subject: [PATCH 4/7] Allow IOUtils.close() to rethrow exception if passed in --- .../core/internal/io/IOUtils.java | 39 ++++++++++++--- .../core/internal/io/Streams.java | 14 +++--- .../core/internal/io/Streams.java | 48 +++---------------- 3 files changed, 45 insertions(+), 56 deletions(-) diff --git a/libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io/IOUtils.java b/libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io/IOUtils.java index eaa4df768cd71..4b469b0d7835d 100644 --- a/libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io/IOUtils.java +++ b/libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io/IOUtils.java @@ -41,25 +41,52 @@ private IOUtils() { } /** - * Closes all given Closeables. Some of the Closeables may be null; they are ignored. After everything is closed, the - * method either throws the first exception it hit while closing, or completes normally if there were no exceptions. + * Closes all given Closeables. Some of the Closeables may be null; they are + * ignored. After everything is closed, the method either throws the first exception it hit + * while closing with other exceptions added as suppressed, or completes normally if there were + * no exceptions. * * @param objects objects to close */ public static void close(final Closeable... objects) throws IOException { - close(Arrays.asList(objects)); + close(null, Arrays.asList(objects)); } /** - * Closes all given {@link Closeable}s. + * Closes all given Closeables. Some of the Closeables may be null; they are + * ignored. After everything is closed, the method adds any exceptions as suppressed to the + * original exception, or throws the first exception it hit if {@code Exception} is null. If + * no exceptions are encountered and the passed in exception is null, it completes normally. * * @param objects objects to close + */ + public static void close(final Exception e, final Closeable... objects) throws IOException { + close(e, Arrays.asList(objects)); + } + + /** + * Closes all given Closeables. Some of the Closeables may be null; they are + * ignored. After everything is closed, the method either throws the first exception it hit + * while closing with other exceptions added as suppressed, or completes normally if there were + * no exceptions. * - * @see #close(Closeable...) + * @param objects objects to close */ public static void close(final Iterable objects) throws IOException { - Exception ex = null; + close(null, objects); + } + /** + * Closes all given {@link Closeable}s. If a non-null exception is passed in, or closing a + * stream causes an exception, throws the exception with other {@link RuntimeException} or + * {@link IOException} exceptions added as suppressed. + * + * @param ex existing Exception to add exceptions occurring during close to + * @param objects objects to close + * + * @see #close(Closeable...) + */ + public static void close(Exception ex, final Iterable objects) throws IOException { for (final Closeable object : objects) { try { if (object != null) { diff --git a/libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io/Streams.java b/libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io/Streams.java index f84acb55fcc63..a006028b90556 100644 --- a/libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io/Streams.java +++ b/libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io/Streams.java @@ -43,11 +43,11 @@ public class Streams { * @return the number of bytes copied * @throws IOException in case of I/O errors */ - public static long copy(InputStream in, OutputStream out) throws IOException { + public static long copy(final InputStream in, final OutputStream out) throws IOException { Objects.requireNonNull(in, "No InputStream specified"); Objects.requireNonNull(out, "No OutputStream specified"); final byte[] buffer = new byte[8192]; - boolean success = false; + Exception err = null; try { long byteCount = 0; int bytesRead; @@ -56,14 +56,12 @@ public static long copy(InputStream in, OutputStream out) throws IOException { byteCount += bytesRead; } out.flush(); - success = true; return byteCount; + } catch (IOException | RuntimeException e) { + err = e; + throw e; } finally { - if (success) { - IOUtils.close(in, out); - } else { - IOUtils.closeWhileHandlingException(in, out); - } + IOUtils.close(err, in, out); } } } diff --git a/libs/elasticsearch-core/src/main/java9/org/elasticsearch/core/internal/io/Streams.java b/libs/elasticsearch-core/src/main/java9/org/elasticsearch/core/internal/io/Streams.java index a63c351283bd6..34b3785765d87 100644 --- a/libs/elasticsearch-core/src/main/java9/org/elasticsearch/core/internal/io/Streams.java +++ b/libs/elasticsearch-core/src/main/java9/org/elasticsearch/core/internal/io/Streams.java @@ -41,53 +41,17 @@ public abstract class Streams { * @return the number of bytes copied * @throws IOException in case of I/O errors */ - public static long copy(InputStream in, OutputStream out) throws IOException { - boolean success = false; + public static long copy(final InputStream in, final OutputStream out) throws IOException { + Exception err = null; try { final long byteCount = in.transferTo(out); out.flush(); - success = true; return byteCount; + } catch (IOException | RuntimeException e) { + err = e; + throw e; } finally { - if (success) { - Exception ex = null; - try { - in.close(); - } catch (IOException | RuntimeException e) { - ex = e; - } - - try { - out.close(); - } catch (IOException | RuntimeException e) { - if (ex == null) { - ex = e; - } else { - ex.addSuppressed(e); - } - } - - if (ex != null) { - if (ex instanceof IOException) { - throw (IOException) ex; - } else { - throw (RuntimeException) ex; - } - } - } else { - try { - in.close(); - } catch (IOException | RuntimeException e) { - // empty - } - - try { - out.close(); - } catch (IOException | RuntimeException e) { - // empty - } - } + IOUtils.close(err, in, out); } - } } From 85fe2feb90a35848e41df019f708df4344aac91b Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Fri, 6 Apr 2018 08:34:28 -0600 Subject: [PATCH 5/7] Make Exception passed to close final --- .../main/java/org/elasticsearch/core/internal/io/IOUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io/IOUtils.java b/libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io/IOUtils.java index 4b469b0d7835d..10268ac477a6c 100644 --- a/libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io/IOUtils.java +++ b/libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io/IOUtils.java @@ -86,7 +86,7 @@ public static void close(final Iterable objects) throws IOE * * @see #close(Closeable...) */ - public static void close(Exception ex, final Iterable objects) throws IOException { + public static void close(final Exception ex, final Iterable objects) throws IOException { for (final Closeable object : objects) { try { if (object != null) { From 87e44854b2a2e1a4eb36be3ef58ab608397980c3 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Fri, 6 Apr 2018 08:57:36 -0600 Subject: [PATCH 6/7] Fix assignment to final var --- .../elasticsearch/core/internal/io/IOUtils.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io/IOUtils.java b/libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io/IOUtils.java index 10268ac477a6c..aafc4eec9d6c1 100644 --- a/libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io/IOUtils.java +++ b/libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io/IOUtils.java @@ -87,26 +87,27 @@ public static void close(final Iterable objects) throws IOE * @see #close(Closeable...) */ public static void close(final Exception ex, final Iterable objects) throws IOException { + Exception error = ex; for (final Closeable object : objects) { try { if (object != null) { object.close(); } } catch (final IOException | RuntimeException e) { - if (ex == null) { - ex = e; + if (error == null) { + error = e; } else { - ex.addSuppressed(e); + error.addSuppressed(e); } } } - if (ex != null) { - if (ex instanceof IOException) { - throw (IOException) ex; + if (error != null) { + if (error instanceof IOException) { + throw (IOException) error; } else { // since we only assigned an IOException or a RuntimeException to ex above, in this case ex must be a RuntimeException - throw (RuntimeException) ex; + throw (RuntimeException) error; } } } From 81a402067aa3fe46b5db7f68a51d5cae9c3c6ec2 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Fri, 6 Apr 2018 09:28:01 -0600 Subject: [PATCH 7/7] Rename first exception to a better name (firstException) --- .../elasticsearch/core/internal/io/IOUtils.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io/IOUtils.java b/libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io/IOUtils.java index aafc4eec9d6c1..4108992fb1f59 100644 --- a/libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io/IOUtils.java +++ b/libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io/IOUtils.java @@ -87,27 +87,27 @@ public static void close(final Iterable objects) throws IOE * @see #close(Closeable...) */ public static void close(final Exception ex, final Iterable objects) throws IOException { - Exception error = ex; + Exception firstException = ex; for (final Closeable object : objects) { try { if (object != null) { object.close(); } } catch (final IOException | RuntimeException e) { - if (error == null) { - error = e; + if (firstException == null) { + firstException = e; } else { - error.addSuppressed(e); + firstException.addSuppressed(e); } } } - if (error != null) { - if (error instanceof IOException) { - throw (IOException) error; + if (firstException != null) { + if (firstException instanceof IOException) { + throw (IOException) firstException; } else { // since we only assigned an IOException or a RuntimeException to ex above, in this case ex must be a RuntimeException - throw (RuntimeException) error; + throw (RuntimeException) firstException; } } }