From ed251790d443c7ca15a06e1c94f906649eb687de Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Tue, 29 Sep 2020 17:06:07 +0530 Subject: [PATCH 1/5] Fix the offset in get of GCP object --- extensions-core/google-extensions/pom.xml | 2 +- .../druid/storage/google/GoogleStorage.java | 8 +- .../druid/storage/google/GoogleTaskLogs.java | 5 +- .../storage/google/GoogleStorageTest.java | 90 +++++++++++++++++++ .../storage/google/GoogleTaskLogsTest.java | 19 ++-- .../druid/storage/google/GoogleTestUtils.java | 10 +++ pom.xml | 2 +- 7 files changed, 118 insertions(+), 18 deletions(-) create mode 100644 extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleStorageTest.java diff --git a/extensions-core/google-extensions/pom.xml b/extensions-core/google-extensions/pom.xml index 2105153ab929..028bc8feec9a 100644 --- a/extensions-core/google-extensions/pom.xml +++ b/extensions-core/google-extensions/pom.xml @@ -34,7 +34,7 @@ - v1-rev158-${com.google.apis.client.version} + v1-rev20190523-${com.google.apis.client.version} diff --git a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorage.java b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorage.java index ee5796678603..5a63183090c6 100644 --- a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorage.java +++ b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorage.java @@ -51,11 +51,9 @@ public InputStream get(final String bucket, final String path) throws IOExceptio public InputStream get(final String bucket, final String path, long start) throws IOException { final Get get = storage.objects().get(bucket, path); - if (start > 0) { - get.getMediaHttpDownloader().setBytesDownloaded(start); - } - get.getMediaHttpDownloader().setDirectDownloadEnabled(false); - return get.executeMediaAsInputStream(); + InputStream inputStream = get.executeMediaAsInputStream(); + inputStream.skip(start); + return inputStream; } public void delete(final String bucket, final String path) throws IOException diff --git a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleTaskLogs.java b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleTaskLogs.java index 1b3c5319ff21..d6d9ff569bee 100644 --- a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleTaskLogs.java +++ b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleTaskLogs.java @@ -142,10 +142,7 @@ public InputStream openStream() throws IOException start = 0; } - InputStream stream = new GoogleByteSource(storage, config.getBucket(), taskKey).openStream(); - stream.skip(start); - - return stream; + return new GoogleByteSource(storage, config.getBucket(), taskKey).openStream(start); } catch (Exception e) { throw new IOException(e); diff --git a/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleStorageTest.java b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleStorageTest.java new file mode 100644 index 000000000000..c9157480eb88 --- /dev/null +++ b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleStorageTest.java @@ -0,0 +1,90 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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.apache.druid.storage.google; + +import com.google.api.client.googleapis.testing.auth.oauth2.MockGoogleCredential; +import com.google.api.client.http.ByteArrayContent; +import com.google.api.client.http.HttpRequestInitializer; +import com.google.api.client.json.jackson2.JacksonFactory; +import com.google.api.client.testing.http.MockHttpTransport; +import com.google.api.client.testing.http.MockLowLevelHttpRequest; +import com.google.api.client.testing.http.MockLowLevelHttpResponse; +import com.google.api.services.storage.Storage; +import org.apache.druid.java.util.common.StringUtils; +import org.junit.Assert; +import org.junit.Test; + +import java.io.IOException; +import java.io.InputStream; + +public class GoogleStorageTest +{ + @Test + public void testGet() throws IOException + { + String content = "abcdefghij"; + MockLowLevelHttpResponse response = new MockLowLevelHttpResponse(); + response.setContent(content); + GoogleStorage googleStorage = makeGoogleStorage(response); + InputStream is = googleStorage.get("bucket", "path"); + String actual = GoogleTestUtils.readAsString(is); + Assert.assertEquals(content, actual); + } + + @Test + public void testGetWithOffset() throws IOException + { + String content = "abcdefghij"; + MockLowLevelHttpResponse response = new MockLowLevelHttpResponse(); + response.setContent(content); + GoogleStorage googleStorage = makeGoogleStorage(response); + InputStream is = googleStorage.get("bucket", "path", 2); + String actual = GoogleTestUtils.readAsString(is); + Assert.assertEquals(content.substring(2), actual); + } + + @Test + public void testInsert() throws IOException + { + String content = "abcdefghij"; + MockLowLevelHttpResponse response = new MockLowLevelHttpResponse(); + response.addHeader("Location", "http://random-path"); + response.setContent("{}"); + MockHttpTransport transport = new MockHttpTransport.Builder().setLowLevelHttpResponse(response).build(); + GoogleStorage googleStorage = makeGoogleStorage(transport); + googleStorage.insert("bucket", "path", new ByteArrayContent("text/html", StringUtils.toUtf8(content))); + MockLowLevelHttpRequest request = transport.getLowLevelHttpRequest(); + String actual = request.getContentAsString(); + Assert.assertEquals(content, actual); + } + + private GoogleStorage makeGoogleStorage(MockLowLevelHttpResponse response) + { + MockHttpTransport transport = new MockHttpTransport.Builder().setLowLevelHttpResponse(response).build(); + return makeGoogleStorage(transport); + } + + private GoogleStorage makeGoogleStorage(MockHttpTransport transport) + { + HttpRequestInitializer initializer = new MockGoogleCredential.Builder().build(); + Storage storage = new Storage(transport, JacksonFactory.getDefaultInstance(), initializer); + return new GoogleStorage(storage); + } +} diff --git a/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleTaskLogsTest.java b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleTaskLogsTest.java index b399e7f5eba2..6e5885ee6deb 100644 --- a/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleTaskLogsTest.java +++ b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleTaskLogsTest.java @@ -117,7 +117,7 @@ public void testStreamTaskLogWithoutOffset() throws Exception final String logPath = PREFIX + "/" + TASKID; EasyMock.expect(storage.exists(BUCKET, logPath)).andReturn(true); EasyMock.expect(storage.size(BUCKET, logPath)).andReturn((long) testLog.length()); - EasyMock.expect(storage.get(BUCKET, logPath)).andReturn(new ByteArrayInputStream(StringUtils.toUtf8(testLog))); + EasyMock.expect(storage.get(BUCKET, logPath, 0)).andReturn(new ByteArrayInputStream(StringUtils.toUtf8(testLog))); replayAll(); @@ -134,11 +134,13 @@ public void testStreamTaskLogWithoutOffset() throws Exception public void testStreamTaskLogWithPositiveOffset() throws Exception { final String testLog = "hello this is a log"; + final String expectedLog = testLog.substring(5); final String logPath = PREFIX + "/" + TASKID; EasyMock.expect(storage.exists(BUCKET, logPath)).andReturn(true); EasyMock.expect(storage.size(BUCKET, logPath)).andReturn((long) testLog.length()); - EasyMock.expect(storage.get(BUCKET, logPath)).andReturn(new ByteArrayInputStream(StringUtils.toUtf8(testLog))); + EasyMock.expect(storage.get(BUCKET, logPath, 5)) + .andReturn(new ByteArrayInputStream(StringUtils.toUtf8(expectedLog))); replayAll(); @@ -146,7 +148,7 @@ public void testStreamTaskLogWithPositiveOffset() throws Exception final StringWriter writer = new StringWriter(); IOUtils.copy(byteSource.get().openStream(), writer, "UTF-8"); - Assert.assertEquals(writer.toString(), testLog.substring(5)); + Assert.assertEquals(writer.toString(), expectedLog); verifyAll(); } @@ -155,19 +157,22 @@ public void testStreamTaskLogWithPositiveOffset() throws Exception public void testStreamTaskLogWithNegative() throws Exception { final String testLog = "hello this is a log"; - + final int offset = -3; + final int internalOffset = testLog.length() + offset; + final String expectedLog = testLog.substring(internalOffset); final String logPath = PREFIX + "/" + TASKID; EasyMock.expect(storage.exists(BUCKET, logPath)).andReturn(true); EasyMock.expect(storage.size(BUCKET, logPath)).andReturn((long) testLog.length()); - EasyMock.expect(storage.get(BUCKET, logPath)).andReturn(new ByteArrayInputStream(StringUtils.toUtf8(testLog))); + EasyMock.expect(storage.get(BUCKET, logPath, internalOffset)) + .andReturn(new ByteArrayInputStream(StringUtils.toUtf8(expectedLog))); replayAll(); - final Optional byteSource = googleTaskLogs.streamTaskLog(TASKID, -3); + final Optional byteSource = googleTaskLogs.streamTaskLog(TASKID, offset); final StringWriter writer = new StringWriter(); IOUtils.copy(byteSource.get().openStream(), writer, "UTF-8"); - Assert.assertEquals(writer.toString(), testLog.substring(testLog.length() - 3)); + Assert.assertEquals(writer.toString(), expectedLog); verifyAll(); } diff --git a/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleTestUtils.java b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleTestUtils.java index d95bbb4f6844..219d96c21662 100644 --- a/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleTestUtils.java +++ b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleTestUtils.java @@ -23,6 +23,7 @@ import com.google.api.services.storage.Storage; import com.google.api.services.storage.model.Objects; import com.google.api.services.storage.model.StorageObject; +import org.apache.commons.io.IOUtils; import org.apache.druid.java.util.common.DateTimes; import org.apache.druid.java.util.common.StringUtils; import org.easymock.EasyMock; @@ -30,6 +31,8 @@ import org.easymock.IExpectationSetters; import java.io.IOException; +import java.io.InputStream; +import java.io.StringWriter; import java.math.BigInteger; import java.net.URI; import java.util.HashMap; @@ -119,4 +122,11 @@ public static void expectDeleteObjects( resultExpectationSetter.andVoid(); } } + + public static String readAsString(InputStream is) throws IOException + { + final StringWriter writer = new StringWriter(); + IOUtils.copy(is, writer, "UTF-8"); + return writer.toString(); + } } diff --git a/pom.xml b/pom.xml index b8b36877e5e5..ca7befd36a9c 100644 --- a/pom.xml +++ b/pom.xml @@ -118,7 +118,7 @@ 3.4.14 2.5.7 - 1.25.0 + 1.26.0 v1-rev214-1.25.0 apache.snapshots Apache Snapshot Repository From 0e1a713967fed0bc0d63c58c6e44a905495c0246 Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Tue, 29 Sep 2020 17:20:22 +0530 Subject: [PATCH 2/5] upgrade compute dependency --- extensions-contrib/gce-extensions/pom.xml | 2 +- extensions-core/google-extensions/pom.xml | 4 ---- pom.xml | 3 ++- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/extensions-contrib/gce-extensions/pom.xml b/extensions-contrib/gce-extensions/pom.xml index 0d05bfcea57c..db64e27ecafe 100644 --- a/extensions-contrib/gce-extensions/pom.xml +++ b/extensions-contrib/gce-extensions/pom.xml @@ -88,7 +88,7 @@ com.google.apis google-api-services-compute - v1-rev214-1.25.0 + ${com.google.apis.compute.version} compile diff --git a/extensions-core/google-extensions/pom.xml b/extensions-core/google-extensions/pom.xml index 028bc8feec9a..b14e916ff232 100644 --- a/extensions-core/google-extensions/pom.xml +++ b/extensions-core/google-extensions/pom.xml @@ -33,10 +33,6 @@ ../../pom.xml - - v1-rev20190523-${com.google.apis.client.version} - - org.apache.druid diff --git a/pom.xml b/pom.xml index ca7befd36a9c..1c9e82735983 100644 --- a/pom.xml +++ b/pom.xml @@ -119,7 +119,8 @@ 3.4.14 2.5.7 1.26.0 - v1-rev214-1.25.0 + v1-rev20190523-1.26.0 + v1-rev20190523-1.26.0 apache.snapshots Apache Snapshot Repository https://repository.apache.org/snapshots From d9652a121ca3434d96d1b1a88ad8a61bb8d7c6f3 Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Tue, 29 Sep 2020 18:37:52 +0530 Subject: [PATCH 3/5] fix version --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 1c9e82735983..967c33713a3b 100644 --- a/pom.xml +++ b/pom.xml @@ -119,7 +119,7 @@ 3.4.14 2.5.7 1.26.0 - v1-rev20190523-1.26.0 + v1-rev20190607-1.26.0 v1-rev20190523-1.26.0 apache.snapshots Apache Snapshot Repository From 137065056c0c461235ac1cbf50965d411e75a7a7 Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Tue, 29 Sep 2020 23:06:50 +0530 Subject: [PATCH 4/5] review comments --- .../apache/druid/storage/google/GoogleTaskLogsTest.java | 6 +++--- licenses.yaml | 8 ++++---- pom.xml | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleTaskLogsTest.java b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleTaskLogsTest.java index 6e5885ee6deb..29f6153659e5 100644 --- a/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleTaskLogsTest.java +++ b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleTaskLogsTest.java @@ -135,16 +135,16 @@ public void testStreamTaskLogWithPositiveOffset() throws Exception { final String testLog = "hello this is a log"; final String expectedLog = testLog.substring(5); - + final int offset = 5; final String logPath = PREFIX + "/" + TASKID; EasyMock.expect(storage.exists(BUCKET, logPath)).andReturn(true); EasyMock.expect(storage.size(BUCKET, logPath)).andReturn((long) testLog.length()); - EasyMock.expect(storage.get(BUCKET, logPath, 5)) + EasyMock.expect(storage.get(BUCKET, logPath, offset)) .andReturn(new ByteArrayInputStream(StringUtils.toUtf8(expectedLog))); replayAll(); - final Optional byteSource = googleTaskLogs.streamTaskLog(TASKID, 5); + final Optional byteSource = googleTaskLogs.streamTaskLog(TASKID, offset); final StringWriter writer = new StringWriter(); IOUtils.copy(byteSource.get().openStream(), writer, "UTF-8"); diff --git a/licenses.yaml b/licenses.yaml index 03c0d7948291..4410ecc4d645 100644 --- a/licenses.yaml +++ b/licenses.yaml @@ -4100,7 +4100,7 @@ name: Google Cloud Storage JSON API license_category: binary module: extensions/druid-google-extensions license_name: Apache License version 2.0 -version: v1-rev158-1.25.0 +version: v1-rev20190523-1.26.0 libraries: - com.google.apis: google-api-services-storage @@ -4110,7 +4110,7 @@ name: Google Compute Engine API license_category: binary module: extensions/gce-extensions license_name: Apache License version 2.0 -version: v1-rev214-1.25.0 +version: v1-rev20190607-1.26.0 libraries: - com.google.apis: google-api-services-compute @@ -4130,7 +4130,7 @@ name: Google APIs Client Library For Java license_category: binary module: java-core license_name: Apache License version 2.0 -version: 1.25.0 +version: 1.26.0 libraries: - com.google.api-client: google-api-client @@ -4140,7 +4140,7 @@ name: Google HTTP Client Library For Java license_category: binary module: java-core license_name: Apache License version 2.0 -version: 1.25.0 +version: 1.26.0 libraries: - com.google.http-client: google-http-client - com.google.http-client: google-http-client-jackson2 diff --git a/pom.xml b/pom.xml index 967c33713a3b..bef016128bcd 100644 --- a/pom.xml +++ b/pom.xml @@ -119,8 +119,8 @@ 3.4.14 2.5.7 1.26.0 - v1-rev20190607-1.26.0 - v1-rev20190523-1.26.0 + v1-rev20190607-${com.google.apis.client.version} + v1-rev20190523-${com.google.apis.client.version} apache.snapshots Apache Snapshot Repository https://repository.apache.org/snapshots From 99679fe32177fd3478ff9f08646d1fe83a18e565 Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Tue, 29 Sep 2020 23:13:35 +0530 Subject: [PATCH 5/5] missed --- .../org/apache/druid/storage/google/GoogleTaskLogsTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleTaskLogsTest.java b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleTaskLogsTest.java index 29f6153659e5..c5807723b66f 100644 --- a/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleTaskLogsTest.java +++ b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleTaskLogsTest.java @@ -134,8 +134,8 @@ public void testStreamTaskLogWithoutOffset() throws Exception public void testStreamTaskLogWithPositiveOffset() throws Exception { final String testLog = "hello this is a log"; - final String expectedLog = testLog.substring(5); final int offset = 5; + final String expectedLog = testLog.substring(offset); final String logPath = PREFIX + "/" + TASKID; EasyMock.expect(storage.exists(BUCKET, logPath)).andReturn(true); EasyMock.expect(storage.size(BUCKET, logPath)).andReturn((long) testLog.length());