From 4711cd50514e0260251132762ddb4b749b6f7394 Mon Sep 17 00:00:00 2001 From: JP Martin Date: Tue, 23 Aug 2016 14:05:17 -0700 Subject: [PATCH 1/7] Support enumeration of filesytems before configuring gcloud --- .../nio/CloudStorageFileSystemProvider.java | 44 +++++-- .../CloudStorageLateInitializationTest.java | 110 ++++++++++++++++++ .../storage/contrib/nio/it/ITGcsNio.java | 5 + 3 files changed, 151 insertions(+), 8 deletions(-) create mode 100644 gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageLateInitializationTest.java diff --git a/gcloud-java-contrib/gcloud-java-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java b/gcloud-java-contrib/gcloud-java-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java index 624c410ddb5c..1c36ee2386ac 100644 --- a/gcloud-java-contrib/gcloud-java-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java +++ b/gcloud-java-contrib/gcloud-java-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java @@ -85,10 +85,11 @@ @AutoService(FileSystemProvider.class) public final class CloudStorageFileSystemProvider extends FileSystemProvider { - private final Storage storage; + private Storage storage; + private StorageOptions storageOptions; // used only when we create a new instance of CloudStorageFileSystemProvider. - private static StorageOptions storageOptions; + private static StorageOptions futureStorageOptions; private static class LazyPathIterator extends AbstractIterator { private final Iterator blobIterator; @@ -123,7 +124,7 @@ protected Path computeNext() { */ @VisibleForTesting public static void setGCloudOptions(StorageOptions newStorageOptions) { - storageOptions = newStorageOptions; + futureStorageOptions = newStorageOptions; } /** @@ -133,14 +134,26 @@ public static void setGCloudOptions(StorageOptions newStorageOptions) { * @see CloudStorageFileSystem#forBucket(String) */ public CloudStorageFileSystemProvider() { - this(storageOptions); + this(futureStorageOptions); } CloudStorageFileSystemProvider(@Nullable StorageOptions gcsStorageOptions) { - if (gcsStorageOptions == null) { + this.storageOptions = gcsStorageOptions; + + } + + // Initialize this.storage, once. This may throw an exception if the environment variable + // GOOGLE_APPLICATION_CREDENTIALS is not set. + // We don't do this in the constructor because the ctor is called even when just enumerating + // available filesystems, and this should work without configuring gcloud. + private void initStorage() { + if (this.storage != null) { + return; + } + if (storageOptions == null) { this.storage = StorageOptions.defaultInstance().service(); } else { - this.storage = gcsStorageOptions.service(); + this.storage = storageOptions.service(); } } @@ -154,6 +167,7 @@ public String getScheme() { */ @Override public CloudStorageFileSystem getFileSystem(URI uri) { + initStorage(); return newFileSystem(uri, Collections.emptyMap()); } @@ -186,11 +200,13 @@ && isNullOrEmpty(uri.getUserInfo()), "GCS FileSystem URIs mustn't have: port, userinfo, path, query, or fragment: %s", uri); CloudStorageUtil.checkBucket(uri.getHost()); + initStorage(); return new CloudStorageFileSystem(this, uri.getHost(), CloudStorageConfiguration.fromMap(env)); } @Override public CloudStoragePath getPath(URI uri) { + initStorage(); return CloudStoragePath.getPath( getFileSystem(CloudStorageUtil.stripPathFromUri(uri)), uri.getPath()); } @@ -199,6 +215,7 @@ public CloudStoragePath getPath(URI uri) { public SeekableByteChannel newByteChannel( Path path, Set options, FileAttribute... attrs) throws IOException { checkNotNull(path); + initStorage(); CloudStorageUtil.checkNotNullArray(attrs); if (options.contains(StandardOpenOption.WRITE)) { // TODO: Make our OpenOptions implement FileAttribute. Also remove buffer option. @@ -210,6 +227,7 @@ public SeekableByteChannel newByteChannel( private SeekableByteChannel newReadChannel(Path path, Set options) throws IOException { + initStorage(); for (OpenOption option : options) { if (option instanceof StandardOpenOption) { switch ((StandardOpenOption) option) { @@ -244,7 +262,7 @@ private SeekableByteChannel newReadChannel(Path path, Set private SeekableByteChannel newWriteChannel(Path path, Set options) throws IOException { - + initStorage(); CloudStoragePath cloudPath = CloudStorageUtil.checkPath(path); if (cloudPath.seemsLikeADirectoryAndUsePseudoDirectories()) { throw new CloudStoragePseudoDirectoryException(cloudPath); @@ -318,6 +336,7 @@ private SeekableByteChannel newWriteChannel(Path path, Set @Override public InputStream newInputStream(Path path, OpenOption... options) throws IOException { + initStorage(); InputStream result = super.newInputStream(path, options); CloudStoragePath cloudPath = CloudStorageUtil.checkPath(path); int blockSize = cloudPath.getFileSystem().config().blockSize(); @@ -331,6 +350,7 @@ public InputStream newInputStream(Path path, OpenOption... options) throws IOExc @Override public boolean deleteIfExists(Path path) throws IOException { + initStorage(); CloudStoragePath cloudPath = CloudStorageUtil.checkPath(path); if (cloudPath.seemsLikeADirectoryAndUsePseudoDirectories()) { throw new CloudStoragePseudoDirectoryException(cloudPath); @@ -340,6 +360,7 @@ public boolean deleteIfExists(Path path) throws IOException { @Override public void delete(Path path) throws IOException { + initStorage(); CloudStoragePath cloudPath = CloudStorageUtil.checkPath(path); if (!deleteIfExists(cloudPath)) { throw new NoSuchFileException(cloudPath.toString()); @@ -348,6 +369,7 @@ public void delete(Path path) throws IOException { @Override public void move(Path source, Path target, CopyOption... options) throws IOException { + initStorage(); for (CopyOption option : options) { if (option == StandardCopyOption.ATOMIC_MOVE) { throw new AtomicMoveNotSupportedException( @@ -362,6 +384,7 @@ public void move(Path source, Path target, CopyOption... options) throws IOExcep @Override public void copy(Path source, Path target, CopyOption... options) throws IOException { + initStorage(); boolean wantCopyAttributes = false; boolean wantReplaceExisting = false; boolean setContentType = false; @@ -492,6 +515,7 @@ public boolean isHidden(Path path) { @Override public void checkAccess(Path path, AccessMode... modes) throws IOException { + initStorage(); for (AccessMode mode : modes) { switch (mode) { case READ: @@ -520,6 +544,7 @@ public A readAttributes( if (type != CloudStorageFileAttributes.class && type != BasicFileAttributes.class) { throw new UnsupportedOperationException(type.getSimpleName()); } + initStorage(); CloudStoragePath cloudPath = CloudStorageUtil.checkPath(path); if (cloudPath.seemsLikeADirectoryAndUsePseudoDirectories()) { @SuppressWarnings("unchecked") @@ -574,6 +599,7 @@ public void createDirectory(Path dir, FileAttribute... attrs) { public DirectoryStream newDirectoryStream(Path dir, final Filter filter) { final CloudStoragePath cloudPath = CloudStorageUtil.checkPath(dir); checkNotNull(filter); + initStorage(); String prefix = cloudPath.toString(); final Iterator blobIterator = storage.list(cloudPath.bucket(), Storage.BlobListOption.prefix(prefix), Storage.BlobListOption.currentDirectory(), @@ -621,7 +647,9 @@ public int hashCode() { @Override public String toString() { - return MoreObjects.toStringHelper(this).add("storage", storage).toString(); + return "CloudStorageFileSystemProvider"; + //initStorage(); + //return MoreObjects.toStringHelper(this).add("storage", storage).toString(); } private IOException asIoException(StorageException oops) { diff --git a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageLateInitializationTest.java b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageLateInitializationTest.java new file mode 100644 index 000000000000..597851670dd7 --- /dev/null +++ b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageLateInitializationTest.java @@ -0,0 +1,110 @@ +/* + * Copyright 2016 Google Inc. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * 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 com.google.cloud.storage.contrib.nio; + +import com.google.common.collect.ImmutableList; +import com.google.common.testing.NullPointerTester; +import org.junit.Before; +import org.junit.Ignore; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.net.URI; +import java.nio.ByteBuffer; +import java.nio.channels.ReadableByteChannel; +import java.nio.channels.SeekableByteChannel; +import java.nio.file.AtomicMoveNotSupportedException; +import java.nio.file.CopyOption; +import java.nio.file.FileAlreadyExistsException; +import java.nio.file.FileSystem; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.NoSuchFileException; +import java.nio.file.OpenOption; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.spi.FileSystemProvider; +import java.util.List; + +import static com.google.cloud.storage.contrib.nio.CloudStorageFileSystem.forBucket; +import static com.google.common.truth.Truth.assertThat; +import static java.nio.charset.StandardCharsets.UTF_8; +import static java.nio.file.StandardCopyOption.ATOMIC_MOVE; +import static java.nio.file.StandardCopyOption.COPY_ATTRIBUTES; +import static java.nio.file.StandardCopyOption.REPLACE_EXISTING; +import static java.nio.file.StandardOpenOption.CREATE; +import static java.nio.file.StandardOpenOption.CREATE_NEW; +import static java.nio.file.StandardOpenOption.TRUNCATE_EXISTING; +import static java.nio.file.StandardOpenOption.WRITE; + +/** + * Unit tests for {@link CloudStorageFileSystemProvider} when gcloud is not configured. + */ +//@RunWith(JUnit4.class) +@Ignore // these tests must be run manually because they require gcloud to NOT be configured. +public class CloudStorageLateInitializationTest { + + @Rule public final ExpectedException thrown = ExpectedException.none(); + + @Before + public void before() { + if (System.getenv().containsKey("GOOGLE_APPLICATION_CREDENTIALS")) { + throw new RuntimeException("CloudStorageLateInitializationTest can only be run if gcloud is not configured. This means no GOOGLE_APPLICATION_CREDENTIALS environment variable."); + } + // actually that one's OK so long as it doesn't point to a valid configuration, but it's simpler + // to check that it isn't there. + if (System.getenv().containsKey("CLOUDSDK_CONFIG")) { + throw new RuntimeException("CloudStorageLateInitializationTest can only be run if gcloud is" + + " not configured. This means no CLOUDSDK_CONFIG environment variable."); + } + if (Files.exists(Paths.get(System.getenv("HOME"), ".config/gcloud/properties"))) { + throw new RuntimeException("CloudStorageLateInitializationTest can only be run if gcloud is" + + " not configured. This means no ~/.config/gcloud/properties."); + } + if (Files.exists(Paths.get(System.getenv("HOME"), ".config/gcloud/active_config"))) { + throw new RuntimeException("CloudStorageLateInitializationTest can only be run if gcloud is" + + " not configured. This means no ~/.config/gcloud/active_config."); + } + } + + @Test(expected = java.lang.IllegalArgumentException.class) + public void pathFailsIfNoEnvVariable() throws IOException { + // this should fail if we haven't set GOOGLE_APPLICATION_CREDENTIALS nor CLOUDSDK_CONFIG + // *and* we don't have a ~/.config/gcloud/properties file (or %APPDATA%/gcloud in Windows) + // and we don't have a ~/.config/gcloud/active-config. + // (since we're also not providing credentials in any other way) + Path path = Paths.get(URI.create("gs://bucket/wat")); + } + + @Test + public void enumerateFilesystemsIfNoEnvVariable() throws IOException { + // listing available filesystem providers should work even if we can't initialize + // CloudStorageFilesystemProvider. This makes it possible to use other filesystems + // when gcloud-java is in the classpath but gcloud isn't configured. + System.out.println("Installed filesystem providers:"); + for (FileSystemProvider p : FileSystemProvider.installedProviders()) { + System.out.println(" " + p.getScheme()); + } + } + +} diff --git a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java index dad6fc35ba2d..cf68017b7892 100644 --- a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java +++ b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java @@ -96,6 +96,11 @@ public class ITGcsNio { @BeforeClass public static void beforeClass() throws IOException { + if (!System.getenv().containsKey("GOOGLE_APPLICATION_CREDENTIALS")) { + throw new RuntimeException("ITGcsNio can only be run if gcloud is configured. Please set " + + " the GOOGLE_APPLICATION_CREDENTIALS environment variable."); + } + // loads the credentials from local disk as par README RemoteStorageHelper gcsHelper = RemoteStorageHelper.create(); storageOptions = gcsHelper.options(); From 189cd0549665a1efa711344ab280f0c5c3dc9564 Mon Sep 17 00:00:00 2001 From: JP Martin Date: Tue, 23 Aug 2016 14:17:19 -0700 Subject: [PATCH 2/7] Remove unused imports, before->beforeClass --- .../CloudStorageLateInitializationTest.java | 35 +++---------------- 1 file changed, 4 insertions(+), 31 deletions(-) diff --git a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageLateInitializationTest.java b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageLateInitializationTest.java index 597851670dd7..34202494d483 100644 --- a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageLateInitializationTest.java +++ b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageLateInitializationTest.java @@ -16,9 +16,7 @@ package com.google.cloud.storage.contrib.nio; -import com.google.common.collect.ImmutableList; -import com.google.common.testing.NullPointerTester; -import org.junit.Before; +import org.junit.BeforeClass; import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; @@ -27,35 +25,10 @@ import org.junit.runners.JUnit4; import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; import java.net.URI; -import java.nio.ByteBuffer; -import java.nio.channels.ReadableByteChannel; -import java.nio.channels.SeekableByteChannel; -import java.nio.file.AtomicMoveNotSupportedException; -import java.nio.file.CopyOption; -import java.nio.file.FileAlreadyExistsException; -import java.nio.file.FileSystem; -import java.nio.file.FileSystems; import java.nio.file.Files; -import java.nio.file.NoSuchFileException; -import java.nio.file.OpenOption; -import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.spi.FileSystemProvider; -import java.util.List; - -import static com.google.cloud.storage.contrib.nio.CloudStorageFileSystem.forBucket; -import static com.google.common.truth.Truth.assertThat; -import static java.nio.charset.StandardCharsets.UTF_8; -import static java.nio.file.StandardCopyOption.ATOMIC_MOVE; -import static java.nio.file.StandardCopyOption.COPY_ATTRIBUTES; -import static java.nio.file.StandardCopyOption.REPLACE_EXISTING; -import static java.nio.file.StandardOpenOption.CREATE; -import static java.nio.file.StandardOpenOption.CREATE_NEW; -import static java.nio.file.StandardOpenOption.TRUNCATE_EXISTING; -import static java.nio.file.StandardOpenOption.WRITE; /** * Unit tests for {@link CloudStorageFileSystemProvider} when gcloud is not configured. @@ -66,8 +39,8 @@ public class CloudStorageLateInitializationTest { @Rule public final ExpectedException thrown = ExpectedException.none(); - @Before - public void before() { + @BeforeClass + public static void beforeClass() { if (System.getenv().containsKey("GOOGLE_APPLICATION_CREDENTIALS")) { throw new RuntimeException("CloudStorageLateInitializationTest can only be run if gcloud is not configured. This means no GOOGLE_APPLICATION_CREDENTIALS environment variable."); } @@ -93,7 +66,7 @@ public void pathFailsIfNoEnvVariable() throws IOException { // *and* we don't have a ~/.config/gcloud/properties file (or %APPDATA%/gcloud in Windows) // and we don't have a ~/.config/gcloud/active-config. // (since we're also not providing credentials in any other way) - Path path = Paths.get(URI.create("gs://bucket/wat")); + Paths.get(URI.create("gs://bucket/wat")); } @Test From 3ebee208800942b03f7677d34bf04bfb571cb544 Mon Sep 17 00:00:00 2001 From: JP Martin Date: Wed, 24 Aug 2016 10:15:17 -0700 Subject: [PATCH 3/7] Change test to use mocks --- .../nio/CloudStorageFileSystemProvider.java | 8 +- .../CloudStorageFileAttributeViewTest.java | 2 +- .../nio/CloudStorageFileAttributesTest.java | 2 +- .../CloudStorageFileSystemProviderTest.java | 2 +- .../nio/CloudStorageFileSystemTest.java | 2 +- .../CloudStorageLateInitializationTest.java | 77 ++++++++----------- .../contrib/nio/CloudStorageOptionsTest.java | 2 +- .../contrib/nio/CloudStoragePathTest.java | 2 +- 8 files changed, 43 insertions(+), 54 deletions(-) diff --git a/gcloud-java-contrib/gcloud-java-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java b/gcloud-java-contrib/gcloud-java-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java index 1c36ee2386ac..7cf236edc397 100644 --- a/gcloud-java-contrib/gcloud-java-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java +++ b/gcloud-java-contrib/gcloud-java-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java @@ -123,7 +123,7 @@ protected Path computeNext() { * Sets options that are only used by the constructor. */ @VisibleForTesting - public static void setGCloudOptions(StorageOptions newStorageOptions) { + public static void setStorageOptions(StorageOptions newStorageOptions) { futureStorageOptions = newStorageOptions; } @@ -142,10 +142,8 @@ public CloudStorageFileSystemProvider() { } - // Initialize this.storage, once. This may throw an exception if the environment variable - // GOOGLE_APPLICATION_CREDENTIALS is not set. - // We don't do this in the constructor because the ctor is called even when just enumerating - // available filesystems, and this should work without configuring gcloud. + // Initialize this.storage, once. This may throw an exception if default authentication + // credentials are not available (hence not doing it in the ctor). private void initStorage() { if (this.storage != null) { return; diff --git a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileAttributeViewTest.java b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileAttributeViewTest.java index 4a437a288f5f..cfdae1330689 100644 --- a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileAttributeViewTest.java +++ b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileAttributeViewTest.java @@ -51,7 +51,7 @@ public class CloudStorageFileAttributeViewTest { @Before public void before() { - CloudStorageFileSystemProvider.setGCloudOptions(LocalStorageHelper.options()); + CloudStorageFileSystemProvider.setStorageOptions(LocalStorageHelper.options()); path = Paths.get(URI.create("gs://red/water")); } diff --git a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileAttributesTest.java b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileAttributesTest.java index c5001114c14e..e0196b0d674f 100644 --- a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileAttributesTest.java +++ b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileAttributesTest.java @@ -47,7 +47,7 @@ public class CloudStorageFileAttributesTest { @Before public void before() { - CloudStorageFileSystemProvider.setGCloudOptions(LocalStorageHelper.options()); + CloudStorageFileSystemProvider.setStorageOptions(LocalStorageHelper.options()); path = Paths.get(URI.create("gs://bucket/randompath")); dir = Paths.get(URI.create("gs://bucket/randompath/")); } diff --git a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProviderTest.java b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProviderTest.java index 5475459d9376..8aca4439098a 100644 --- a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProviderTest.java +++ b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProviderTest.java @@ -89,7 +89,7 @@ public class CloudStorageFileSystemProviderTest { @Before public void before() { - CloudStorageFileSystemProvider.setGCloudOptions(LocalStorageHelper.options()); + CloudStorageFileSystemProvider.setStorageOptions(LocalStorageHelper.options()); } @Test diff --git a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemTest.java b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemTest.java index 90e61ded721a..66d2c730696c 100644 --- a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemTest.java +++ b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemTest.java @@ -56,7 +56,7 @@ public class CloudStorageFileSystemTest { @Before public void before() { - CloudStorageFileSystemProvider.setGCloudOptions(LocalStorageHelper.options()); + CloudStorageFileSystemProvider.setStorageOptions(LocalStorageHelper.options()); } @Test diff --git a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageLateInitializationTest.java b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageLateInitializationTest.java index 34202494d483..cafb88a3202b 100644 --- a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageLateInitializationTest.java +++ b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageLateInitializationTest.java @@ -16,68 +16,59 @@ package com.google.cloud.storage.contrib.nio; -import org.junit.BeforeClass; -import org.junit.Ignore; +import com.google.cloud.storage.StorageOptions; +import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -import java.io.IOException; import java.net.URI; -import java.nio.file.Files; -import java.nio.file.Paths; -import java.nio.file.spi.FileSystemProvider; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; /** - * Unit tests for {@link CloudStorageFileSystemProvider} when gcloud is not configured. + * Unit tests for {@link CloudStorageFileSystemProvider} late initialization. */ -//@RunWith(JUnit4.class) -@Ignore // these tests must be run manually because they require gcloud to NOT be configured. +@RunWith(JUnit4.class) public class CloudStorageLateInitializationTest { @Rule public final ExpectedException thrown = ExpectedException.none(); - @BeforeClass - public static void beforeClass() { - if (System.getenv().containsKey("GOOGLE_APPLICATION_CREDENTIALS")) { - throw new RuntimeException("CloudStorageLateInitializationTest can only be run if gcloud is not configured. This means no GOOGLE_APPLICATION_CREDENTIALS environment variable."); - } - // actually that one's OK so long as it doesn't point to a valid configuration, but it's simpler - // to check that it isn't there. - if (System.getenv().containsKey("CLOUDSDK_CONFIG")) { - throw new RuntimeException("CloudStorageLateInitializationTest can only be run if gcloud is" + - " not configured. This means no CLOUDSDK_CONFIG environment variable."); - } - if (Files.exists(Paths.get(System.getenv("HOME"), ".config/gcloud/properties"))) { - throw new RuntimeException("CloudStorageLateInitializationTest can only be run if gcloud is" + - " not configured. This means no ~/.config/gcloud/properties."); - } - if (Files.exists(Paths.get(System.getenv("HOME"), ".config/gcloud/active_config"))) { - throw new RuntimeException("CloudStorageLateInitializationTest can only be run if gcloud is" + - " not configured. This means no ~/.config/gcloud/active_config."); - } + private StorageOptions mockOptions; + + @Before + public void before() { + mockOptions = mock(StorageOptions.class); + when(mockOptions.service()).thenReturn(StorageOptions.defaultInstance().service()); + CloudStorageFileSystemProvider.setStorageOptions(mockOptions); + } + + @Test + public void ctorDoesNotCreateStorage() { + new CloudStorageFileSystemProvider(); + verify(mockOptions, never()).service(); } - @Test(expected = java.lang.IllegalArgumentException.class) - public void pathFailsIfNoEnvVariable() throws IOException { - // this should fail if we haven't set GOOGLE_APPLICATION_CREDENTIALS nor CLOUDSDK_CONFIG - // *and* we don't have a ~/.config/gcloud/properties file (or %APPDATA%/gcloud in Windows) - // and we don't have a ~/.config/gcloud/active-config. - // (since we're also not providing credentials in any other way) - Paths.get(URI.create("gs://bucket/wat")); + @Test + public void getPathCreatesStorageOnce() { + CloudStorageFileSystemProvider provider = new CloudStorageFileSystemProvider(); + provider.getPath(URI.create("gs://bucket/wat")); + provider.getPath(URI.create("gs://bucket2/wat")); + verify(mockOptions, times(1)).service(); } @Test - public void enumerateFilesystemsIfNoEnvVariable() throws IOException { - // listing available filesystem providers should work even if we can't initialize - // CloudStorageFilesystemProvider. This makes it possible to use other filesystems - // when gcloud-java is in the classpath but gcloud isn't configured. - System.out.println("Installed filesystem providers:"); - for (FileSystemProvider p : FileSystemProvider.installedProviders()) { - System.out.println(" " + p.getScheme()); - } + public void getFileSystemCreatesStorageOnce() { + CloudStorageFileSystemProvider provider = new CloudStorageFileSystemProvider(); + provider.getFileSystem(URI.create("gs://bucket1")); + provider.getFileSystem(URI.create("gs://bucket2")); + verify(mockOptions, times(1)).service(); } } diff --git a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageOptionsTest.java b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageOptionsTest.java index e23ba5055511..28e23c4e85c9 100644 --- a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageOptionsTest.java +++ b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageOptionsTest.java @@ -41,7 +41,7 @@ public class CloudStorageOptionsTest { @Before public void before() { - CloudStorageFileSystemProvider.setGCloudOptions(LocalStorageHelper.options()); + CloudStorageFileSystemProvider.setStorageOptions(LocalStorageHelper.options()); } @Test diff --git a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStoragePathTest.java b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStoragePathTest.java index 7ba37211cabb..95d5df729ce5 100644 --- a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStoragePathTest.java +++ b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStoragePathTest.java @@ -46,7 +46,7 @@ public class CloudStoragePathTest { @Before public void before() { - CloudStorageFileSystemProvider.setGCloudOptions(LocalStorageHelper.options()); + CloudStorageFileSystemProvider.setStorageOptions(LocalStorageHelper.options()); } @Test From 44cd26805f1465a6b1a2f469d1e0fd407ec10a22 Mon Sep 17 00:00:00 2001 From: JP Martin Date: Wed, 24 Aug 2016 11:16:10 -0700 Subject: [PATCH 4/7] Revert ITGcsNio.java --- .../com/google/cloud/storage/contrib/nio/it/ITGcsNio.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java index cf68017b7892..dad6fc35ba2d 100644 --- a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java +++ b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java @@ -96,11 +96,6 @@ public class ITGcsNio { @BeforeClass public static void beforeClass() throws IOException { - if (!System.getenv().containsKey("GOOGLE_APPLICATION_CREDENTIALS")) { - throw new RuntimeException("ITGcsNio can only be run if gcloud is configured. Please set " + - " the GOOGLE_APPLICATION_CREDENTIALS environment variable."); - } - // loads the credentials from local disk as par README RemoteStorageHelper gcsHelper = RemoteStorageHelper.create(); storageOptions = gcsHelper.options(); From 356da96bf6dc86ffa4aec30a43dde4df6a0b07e8 Mon Sep 17 00:00:00 2001 From: JP Martin Date: Wed, 24 Aug 2016 13:11:41 -0700 Subject: [PATCH 5/7] Use mock storage --- .../contrib/nio/CloudStorageLateInitializationTest.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageLateInitializationTest.java b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageLateInitializationTest.java index cafb88a3202b..c52db94df1e9 100644 --- a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageLateInitializationTest.java +++ b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageLateInitializationTest.java @@ -16,6 +16,7 @@ package com.google.cloud.storage.contrib.nio; +import com.google.cloud.storage.Storage; import com.google.cloud.storage.StorageOptions; import org.junit.Before; import org.junit.Rule; @@ -45,7 +46,8 @@ public class CloudStorageLateInitializationTest { @Before public void before() { mockOptions = mock(StorageOptions.class); - when(mockOptions.service()).thenReturn(StorageOptions.defaultInstance().service()); + Storage mockStorage = mock(Storage.class); + when(mockOptions.service()).thenReturn(mockStorage); CloudStorageFileSystemProvider.setStorageOptions(mockOptions); } @@ -58,7 +60,7 @@ public void ctorDoesNotCreateStorage() { @Test public void getPathCreatesStorageOnce() { CloudStorageFileSystemProvider provider = new CloudStorageFileSystemProvider(); - provider.getPath(URI.create("gs://bucket/wat")); + provider.getPath(URI.create("gs://bucket1/wat")); provider.getPath(URI.create("gs://bucket2/wat")); verify(mockOptions, times(1)).service(); } From ed0239834e587519240e8f99015815954f2bf694 Mon Sep 17 00:00:00 2001 From: JP Martin Date: Wed, 24 Aug 2016 15:16:38 -0700 Subject: [PATCH 6/7] Revert CloudStorageFileSystemProvider.toString --- .../storage/contrib/nio/CloudStorageFileSystemProvider.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/gcloud-java-contrib/gcloud-java-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java b/gcloud-java-contrib/gcloud-java-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java index 7cf236edc397..ceb3428ea24a 100644 --- a/gcloud-java-contrib/gcloud-java-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java +++ b/gcloud-java-contrib/gcloud-java-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java @@ -645,9 +645,8 @@ public int hashCode() { @Override public String toString() { - return "CloudStorageFileSystemProvider"; - //initStorage(); - //return MoreObjects.toStringHelper(this).add("storage", storage).toString(); + initStorage(); + return MoreObjects.toStringHelper(this).add("storage", storage).toString(); } private IOException asIoException(StorageException oops) { From 32e779b9daea9861aa8b01163fe94e5e6273d49e Mon Sep 17 00:00:00 2001 From: JP Martin Date: Wed, 24 Aug 2016 15:21:26 -0700 Subject: [PATCH 7/7] Reviewer comments --- .../contrib/nio/CloudStorageFileSystemProviderTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProviderTest.java b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProviderTest.java index 8aca4439098a..c66de5dabb9e 100644 --- a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProviderTest.java +++ b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProviderTest.java @@ -628,7 +628,7 @@ public void testNullness() throws IOException, NoSuchMethodException, SecurityEx tester.setDefault(Path.class, fs.getPath("and/one")); tester.setDefault(OpenOption.class, CREATE); tester.setDefault(CopyOption.class, COPY_ATTRIBUTES); - // can't do that, setGCloudOptions accepts a null argument. + // can't do that, setStorageOptions accepts a null argument. // TODO(jart): Figure out how to re-enable this. // tester.testAllPublicStaticMethods(CloudStorageFileSystemProvider.class); tester.testAllPublicInstanceMethods(new CloudStorageFileSystemProvider()); @@ -651,4 +651,4 @@ private static CloudStorageConfiguration permitEmptyPathComponents(boolean value private static CloudStorageConfiguration usePseudoDirectories(boolean value) { return CloudStorageConfiguration.builder().usePseudoDirectories(value).build(); } -} +} \ No newline at end of file