Mailing-List: contact commits-help@beam.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@beam.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: dhalperi@apache.org
To: commits@beam.apache.org
Date: Tue, 09 May 2017 20:16:13 -0000
Message-Id: <611e002f70594a8fb9dfadec9f8a471c@git.apache.org>
Subject: [1/2] beam git commit: [BEAM-2229] GcsFileSystem: handle empty files
archived-at: Tue, 09 May 2017 20:16:15 -0000

Repository: beam
Updated Branches:
  refs/heads/release-2.0.0 330931836 -> 61c5a42f8


[BEAM-2229] GcsFileSystem: handle empty files

And add tests


Project: http://git-wip-us.apache.org/repos/asf/beam/repo
Commit: http://git-wip-us.apache.org/repos/asf/beam/commit/c4687404
Tree: http://git-wip-us.apache.org/repos/asf/beam/tree/c4687404
Diff: http://git-wip-us.apache.org/repos/asf/beam/diff/c4687404

Branch: refs/heads/release-2.0.0
Commit: c46874045c4694d4dbd2aa85af261fb326e772eb
Parents: 3309318
Author: Dan Halperin <dhalperi@google.com>
Authored: Tue May 9 11:29:20 2017 -0700
Committer: Dan Halperin <dhalperi@google.com>
Committed: Tue May 9 13:16:03 2017 -0700

----------------------------------------------------------------------
 .../extensions/gcp/storage/GcsFileSystem.java   |  7 +++----
 .../gcp/storage/GcsFileSystemTest.java          | 22 +++++++++++++-------
 2 files changed, 18 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/beam/blob/c4687404/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/storage/GcsFileSystem.java
----------------------------------------------------------------------
diff --git a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/storage/GcsFileSystem.java b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/storage/GcsFileSystem.java
index 38b8347..9052a5a 100644
--- a/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/storage/GcsFileSystem.java
+++ b/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/storage/GcsFileSystem.java
@@ -17,6 +17,7 @@
  */
 package org.apache.beam.sdk.extensions.gcp.storage;
 
+import static com.google.common.base.MoreObjects.firstNonNull;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
@@ -241,10 +242,8 @@ class GcsFileSystem extends FileSystem<GcsResourceId> {
     Metadata.Builder ret = Metadata.builder()
         .setIsReadSeekEfficient(true)
         .setResourceId(GcsResourceId.fromGcsPath(GcsPath.fromObject(storageObject)));
-    BigInteger size = storageObject.getSize();
-    if (size != null) {
-      ret.setSizeBytes(size.longValue());
-    }
+    BigInteger size = firstNonNull(storageObject.getSize(), BigInteger.ZERO);
+    ret.setSizeBytes(size.longValue());
     return ret.build();
   }
 

http://git-wip-us.apache.org/repos/asf/beam/blob/c4687404/sdks/java/extensions/google-cloud-platform-core/src/test/java/org/apache/beam/sdk/extensions/gcp/storage/GcsFileSystemTest.java
----------------------------------------------------------------------
diff --git a/sdks/java/extensions/google-cloud-platform-core/src/test/java/org/apache/beam/sdk/extensions/gcp/storage/GcsFileSystemTest.java b/sdks/java/extensions/google-cloud-platform-core/src/test/java/org/apache/beam/sdk/extensions/gcp/storage/GcsFileSystemTest.java
index 37ff9c8..03194d2 100644
--- a/sdks/java/extensions/google-cloud-platform-core/src/test/java/org/apache/beam/sdk/extensions/gcp/storage/GcsFileSystemTest.java
+++ b/sdks/java/extensions/google-cloud-platform-core/src/test/java/org/apache/beam/sdk/extensions/gcp/storage/GcsFileSystemTest.java
@@ -35,6 +35,7 @@ import java.io.IOException;
 import java.math.BigInteger;
 import java.util.ArrayList;
 import java.util.List;
+import javax.annotation.Nullable;
 import org.apache.beam.sdk.extensions.gcp.options.GcsOptions;
 import org.apache.beam.sdk.io.fs.MatchResult;
 import org.apache.beam.sdk.io.fs.MatchResult.Metadata;
@@ -62,13 +63,12 @@ public class GcsFileSystemTest {
   public transient ExpectedException thrown = ExpectedException.none();
   @Mock
   private GcsUtil mockGcsUtil;
-  private GcsOptions gcsOptions;
   private GcsFileSystem gcsFileSystem;
 
   @Before
   public void setUp() {
     MockitoAnnotations.initMocks(this);
-    gcsOptions = PipelineOptionsFactory.as(GcsOptions.class);
+    GcsOptions gcsOptions = PipelineOptionsFactory.as(GcsOptions.class);
     gcsOptions.setGcsUtil(mockGcsUtil);
     gcsFileSystem = new GcsFileSystem(gcsOptions);
   }
@@ -228,6 +228,8 @@ public class GcsFileSystemTest {
     // Files within the directory
     items.add(StorageObjectOrIOException.create(
         createStorageObject("gs://testbucket/testdirectory/file1name", 1L /* fileSize */)));
+    items.add(StorageObjectOrIOException.create(
+        createStorageObject("gs://testbucket/testdirectory/dir2name/", 0L /* fileSize */)));
     items.add(StorageObjectOrIOException.create(new FileNotFoundException()));
     items.add(StorageObjectOrIOException.create(new IOException()));
     items.add(StorageObjectOrIOException.create(
@@ -235,6 +237,7 @@ public class GcsFileSystemTest {
 
     List<GcsPath> gcsPaths = ImmutableList.of(
         GcsPath.fromUri("gs://testbucket/testdirectory/file1name"),
+        GcsPath.fromUri("gs://testbucket/testdirectory/dir2name/"),
         GcsPath.fromUri("gs://testbucket/testdirectory/file2name"),
         GcsPath.fromUri("gs://testbucket/testdirectory/file3name"),
         GcsPath.fromUri("gs://testbucket/testdirectory/file4name"));
@@ -242,23 +245,28 @@ public class GcsFileSystemTest {
     when(mockGcsUtil.getObjects(eq(gcsPaths))).thenReturn(items);
     List<MatchResult> matchResults = gcsFileSystem.matchNonGlobs(gcsPaths);
 
-    assertEquals(4, matchResults.size());
+    assertEquals(5, matchResults.size());
     assertThat(
         ImmutableList.of("gs://testbucket/testdirectory/file1name"),
         contains(toFilenames(matchResults.get(0)).toArray()));
-    assertEquals(Status.NOT_FOUND, matchResults.get(1).status());
-    assertEquals(Status.ERROR, matchResults.get(2).status());
+    assertThat(
+        ImmutableList.of("gs://testbucket/testdirectory/dir2name/"),
+        contains(toFilenames(matchResults.get(1)).toArray()));
+    assertEquals(Status.NOT_FOUND, matchResults.get(2).status());
+    assertEquals(Status.ERROR, matchResults.get(3).status());
     assertThat(
         ImmutableList.of("gs://testbucket/testdirectory/file4name"),
-        contains(toFilenames(matchResults.get(3)).toArray()));
+        contains(toFilenames(matchResults.get(4)).toArray()));
   }
 
   private StorageObject createStorageObject(String gcsFilename, long fileSize) {
     GcsPath gcsPath = GcsPath.fromUri(gcsFilename);
+    // Google APIs will use null for empty files.
+    @Nullable BigInteger size = (fileSize == 0) ? null : BigInteger.valueOf(fileSize);
     return new StorageObject()
         .setBucket(gcsPath.getBucket())
         .setName(gcsPath.getObject())
-        .setSize(BigInteger.valueOf(fileSize));
+        .setSize(size);
   }
 
   private List<String> toFilenames(MatchResult matchResult) throws IOException {