From 3e051faab5998964db9474c9d16622ac74a1dd45 Mon Sep 17 00:00:00 2001 From: JP Martin Date: Fri, 16 Jun 2017 14:37:24 -0700 Subject: [PATCH 1/4] Add setDefaultCloudStorageConfiguration This allows the main program to set default options that will be used by all subsequent filesystem objects. This is useful for a main program that has libraries that create their own Path objects via the recommended way (Paths.get(URI.create("gs://..."))). --- .../nio/CloudStorageConfiguration.java | 19 ++++++++++++++- .../contrib/nio/CloudStorageFileSystem.java | 23 ++++++++++++++++++- .../nio/CloudStorageFileSystemProvider.java | 23 ++++++++++++++++++- .../contrib/nio/CloudStorageOptions.java | 2 +- .../nio/CloudStorageFileSystemTest.java | 22 ++++++++++++++++++ 5 files changed, 85 insertions(+), 4 deletions(-) diff --git a/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageConfiguration.java b/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageConfiguration.java index 2af5fe2e341e..8b6b3c0db141 100644 --- a/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageConfiguration.java +++ b/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageConfiguration.java @@ -72,6 +72,7 @@ public abstract class CloudStorageConfiguration { *
  • Performing I/O on paths with extra slashes, e.g. {@code a//b} will throw an error. *
  • The prefix slash on absolute paths will be removed when converting to an object name. *
  • Pseudo-directories are enabled, so any path with a trailing slash is a fake directory. + *
  • Channel re-opens are disabled. * */ public static Builder builder() { @@ -159,11 +160,27 @@ public CloudStorageConfiguration build() { maxChannelReopens); } + Builder(CloudStorageConfiguration toModify) { + workingDirectory = toModify.workingDirectory(); + permitEmptyPathComponents = toModify.permitEmptyPathComponents(); + stripPrefixSlash = toModify.stripPrefixSlash(); + usePseudoDirectories = toModify.usePseudoDirectories(); + blockSize = toModify.blockSize(); + maxChannelReopens = toModify.maxChannelReopens(); + } + Builder() {} } static CloudStorageConfiguration fromMap(Map env) { - Builder builder = builder(); + return fromMap(builder(), env); + } + + static CloudStorageConfiguration fromMap(CloudStorageConfiguration defaultValues, Map env) { + return fromMap(new Builder(defaultValues), env); + } + + static private CloudStorageConfiguration fromMap(Builder builder, Map env) { for (Map.Entry entry : env.entrySet()) { switch (entry.getKey()) { case "workingDirectory": diff --git a/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystem.java b/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystem.java index 49f1772e90b7..771e0f6c7470 100644 --- a/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystem.java +++ b/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystem.java @@ -62,6 +62,27 @@ public final class CloudStorageFileSystem extends FileSystem { private final String bucket; private final CloudStorageConfiguration config; + // Users can change this: then this affects every filesystem object created + // later, including via SPI. This is meant to be done only once, at the beginning + // of some main program, in order to force all libraries to use some settings we like. + // Libraries should never call this. It'll cause surprise to the writers of the main + // program and they'll be unhappy. Instead, create your own filesystem object with the + // right configuration and pass it along. + private static CloudStorageConfiguration userSpecifiedDefault = CloudStorageConfiguration.DEFAULT; + + // Don't call this one, call the one in CloudStorageFileSystemProvider. + static void setDefaultCloudStorageConfiguration(CloudStorageConfiguration config) { + if (null == config) { + userSpecifiedDefault = CloudStorageConfiguration.DEFAULT; + } else { + userSpecifiedDefault = config; + } + } + + static CloudStorageConfiguration getDefaultCloudStorageConfiguration() { + return userSpecifiedDefault; + } + /** * Returns Google Cloud Storage {@link FileSystem} object for {@code bucket}. * @@ -79,7 +100,7 @@ public final class CloudStorageFileSystem extends FileSystem { */ @CheckReturnValue public static CloudStorageFileSystem forBucket(String bucket) { - return forBucket(bucket, CloudStorageConfiguration.DEFAULT); + return forBucket(bucket, userSpecifiedDefault); } /** diff --git a/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java b/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java index 71b45c8a282f..3036223ccffa 100644 --- a/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java +++ b/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java @@ -129,6 +129,23 @@ public static void setStorageOptions(StorageOptions newStorageOptions) { futureStorageOptions = newStorageOptions; } + /** + * Changes the default configuration for every filesystem object created + * from here on, including via SPI. If null then future filesystem objects + * will have the factory default configuration. + * + *

    This is meant to be done only once, at the beginning + * of some main program, in order to force all libraries to use some settings we like. + * + *

    Libraries should never call this. If you're a library then, instead, create your own + * filesystem object with the right configuration and pass it along. + * + * @param newDefault new default CloudStorageConfiguration + */ + public static void setDefaultCloudStorageConfiguration(CloudStorageConfiguration newDefault) { + CloudStorageFileSystem.setDefaultCloudStorageConfiguration(newDefault); + } + /** * Default constructor which should only be called by Java SPI. * @@ -200,7 +217,11 @@ && isNullOrEmpty(uri.getUserInfo()), uri); CloudStorageUtil.checkBucket(uri.getHost()); initStorage(); - return new CloudStorageFileSystem(this, uri.getHost(), CloudStorageConfiguration.fromMap(env)); + return new CloudStorageFileSystem( + this, + uri.getHost(), + CloudStorageConfiguration.fromMap( + CloudStorageFileSystem.getDefaultCloudStorageConfiguration(), env)); } @Override diff --git a/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageOptions.java b/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageOptions.java index 9a568883e3a4..282c1b0d810a 100644 --- a/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageOptions.java +++ b/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageOptions.java @@ -92,7 +92,7 @@ public static CloudStorageOption.OpenCopy withBlockSize(int size) { } /** - * Sets the max number of times that the channel can be reopen if reading + * Sets the max number of times that the channel can be reopened if reading * fails because the channel unexpectedly closes. * *

    The default is 0. diff --git a/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemTest.java b/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemTest.java index 9d9e864968ab..e55e9da52e86 100644 --- a/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemTest.java +++ b/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemTest.java @@ -69,6 +69,28 @@ public void before() { CloudStorageFileSystemProvider.setStorageOptions(LocalStorageHelper.getOptions()); } + @Test + public void checkDefaultOptions() throws IOException { + // 1. We get the normal default if we don't do anything special. + Path path = Paths.get(URI.create("gs://bucket/file")); + CloudStorageFileSystem gcs = (CloudStorageFileSystem)path.getFileSystem(); + assertThat(gcs.config().maxChannelReopens()).isEqualTo(0); + + // 2. Override the default, and see it reflected + CloudStorageFileSystemProvider.setDefaultCloudStorageConfiguration( + CloudStorageConfiguration.builder() + .maxChannelReopens(123).build()); + Path path2 = Paths.get(URI.create("gs://newbucket/file")); + CloudStorageFileSystem gcs2 = (CloudStorageFileSystem)path2.getFileSystem(); + assertThat(gcs2.config().maxChannelReopens()).isEqualTo(123); + + // 3. Clean up + CloudStorageFileSystemProvider.setDefaultCloudStorageConfiguration(null); + Path path3 = Paths.get(URI.create("gs://newbucket/file")); + CloudStorageFileSystem gcs3 = (CloudStorageFileSystem)path3.getFileSystem(); + assertThat(gcs3.config().maxChannelReopens()).isEqualTo(0); + } + @Test public void testGetPath() throws IOException { try (FileSystem fs = CloudStorageFileSystem.forBucket("bucket")) { From 18e4574e8f9e3c3316fffc8c9cea46558c49c22d Mon Sep 17 00:00:00 2001 From: JP Martin Date: Fri, 16 Jun 2017 15:17:32 -0700 Subject: [PATCH 2/4] Check default propagates to open channel --- .../contrib/nio/CloudStorageReadChannel.java | 4 +++- .../contrib/nio/CloudStorageFileSystemTest.java | 15 +++++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadChannel.java b/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadChannel.java index 8fa65f475be5..515871ea61d6 100644 --- a/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadChannel.java +++ b/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadChannel.java @@ -21,6 +21,7 @@ import com.google.cloud.storage.BlobInfo; import com.google.cloud.storage.Storage; import com.google.cloud.storage.StorageException; +import com.google.common.annotations.VisibleForTesting; import javax.annotation.CheckReturnValue; import javax.annotation.concurrent.ThreadSafe; @@ -49,7 +50,8 @@ final class CloudStorageReadChannel implements SeekableByteChannel { private final Storage gcsStorage; private final BlobId file; // max # of times we may reopen the file - private final int maxChannelReopens; + @VisibleForTesting + final int maxChannelReopens; // how many times we re-opened the file private int reopens; private ReadChannel channel; diff --git a/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemTest.java b/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemTest.java index e55e9da52e86..64f14b410147 100644 --- a/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemTest.java +++ b/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemTest.java @@ -33,6 +33,7 @@ import java.io.IOException; import java.net.URI; +import java.nio.channels.SeekableByteChannel; import java.nio.file.FileSystem; import java.nio.file.FileSystems; import java.nio.file.Files; @@ -76,7 +77,7 @@ public void checkDefaultOptions() throws IOException { CloudStorageFileSystem gcs = (CloudStorageFileSystem)path.getFileSystem(); assertThat(gcs.config().maxChannelReopens()).isEqualTo(0); - // 2. Override the default, and see it reflected + // 2(a). Override the default, and see it reflected CloudStorageFileSystemProvider.setDefaultCloudStorageConfiguration( CloudStorageConfiguration.builder() .maxChannelReopens(123).build()); @@ -84,7 +85,17 @@ public void checkDefaultOptions() throws IOException { CloudStorageFileSystem gcs2 = (CloudStorageFileSystem)path2.getFileSystem(); assertThat(gcs2.config().maxChannelReopens()).isEqualTo(123); - // 3. Clean up + // 2(b) ...even reflected if we try to open a file. + try (FileSystem fs = CloudStorageFileSystem.forBucket("bucket")) { + Files.write(fs.getPath("/angel"), ALONE.getBytes(UTF_8)); + path2 = Paths.get(URI.create("gs://bucket/angel")); + try (SeekableByteChannel seekableByteChannel = Files.newByteChannel(path2)) { + CloudStorageReadChannel cloudChannel = (CloudStorageReadChannel) seekableByteChannel; + assertThat(cloudChannel.maxChannelReopens).isEqualTo(123); + } + } + + // 4. Clean up CloudStorageFileSystemProvider.setDefaultCloudStorageConfiguration(null); Path path3 = Paths.get(URI.create("gs://newbucket/file")); CloudStorageFileSystem gcs3 = (CloudStorageFileSystem)path3.getFileSystem(); From 22a0f2a8fdba1f0a14cd6ee598156ab5e5c26240 Mon Sep 17 00:00:00 2001 From: JP Martin Date: Fri, 16 Jun 2017 15:23:43 -0700 Subject: [PATCH 3/4] Test NIO reopen defaults are overridable --- .../nio/CloudStorageFileSystemTest.java | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemTest.java b/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemTest.java index 64f14b410147..a4e231bb0cbd 100644 --- a/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemTest.java +++ b/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemTest.java @@ -77,7 +77,7 @@ public void checkDefaultOptions() throws IOException { CloudStorageFileSystem gcs = (CloudStorageFileSystem)path.getFileSystem(); assertThat(gcs.config().maxChannelReopens()).isEqualTo(0); - // 2(a). Override the default, and see it reflected + // 2(a). Override the default, and see it reflected. CloudStorageFileSystemProvider.setDefaultCloudStorageConfiguration( CloudStorageConfiguration.builder() .maxChannelReopens(123).build()); @@ -87,6 +87,8 @@ public void checkDefaultOptions() throws IOException { // 2(b) ...even reflected if we try to open a file. try (FileSystem fs = CloudStorageFileSystem.forBucket("bucket")) { + CloudStorageFileSystem csfs = (CloudStorageFileSystem)fs; + assertThat(csfs.config().maxChannelReopens()).isEqualTo(123); Files.write(fs.getPath("/angel"), ALONE.getBytes(UTF_8)); path2 = Paths.get(URI.create("gs://bucket/angel")); try (SeekableByteChannel seekableByteChannel = Files.newByteChannel(path2)) { @@ -95,13 +97,30 @@ public void checkDefaultOptions() throws IOException { } } - // 4. Clean up + // 4. Clean up. CloudStorageFileSystemProvider.setDefaultCloudStorageConfiguration(null); Path path3 = Paths.get(URI.create("gs://newbucket/file")); CloudStorageFileSystem gcs3 = (CloudStorageFileSystem)path3.getFileSystem(); assertThat(gcs3.config().maxChannelReopens()).isEqualTo(0); } + @Test + public void canOverrideDefaultOptions() throws IOException { + // Set a new default. + CloudStorageFileSystemProvider.setDefaultCloudStorageConfiguration( + CloudStorageConfiguration.builder() + .maxChannelReopens(123).build()); + + // This code wants its own value. + try (FileSystem fs = CloudStorageFileSystem.forBucket("bucket", CloudStorageConfiguration.builder().maxChannelReopens(7).build())) { + CloudStorageFileSystem csfs = (CloudStorageFileSystem)fs; + assertThat(csfs.config().maxChannelReopens()).isEqualTo(7); + } + + // Clean up. + CloudStorageFileSystemProvider.setDefaultCloudStorageConfiguration(null); + } + @Test public void testGetPath() throws IOException { try (FileSystem fs = CloudStorageFileSystem.forBucket("bucket")) { From c099a026b70242471c4db0b20b68466a019e7cc8 Mon Sep 17 00:00:00 2001 From: JP Martin Date: Mon, 26 Jun 2017 10:24:23 -0700 Subject: [PATCH 4/4] Improve description of default overriding --- .../contrib/nio/CloudStorageFileSystemProvider.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java b/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java index 3036223ccffa..c3d49746d6cd 100644 --- a/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java +++ b/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java @@ -134,8 +134,15 @@ public static void setStorageOptions(StorageOptions newStorageOptions) { * from here on, including via SPI. If null then future filesystem objects * will have the factory default configuration. * - *

    This is meant to be done only once, at the beginning - * of some main program, in order to force all libraries to use some settings we like. + *

    If options are specified later then they override the defaults. + * Methods that take a whole CloudStorageConfiguration (eg. + * CloudStorageFileSystem.forBucket) will completely override the defaults. + * Methods that take individual options (eg. + * CloudStorageFileSystemProvier.newFileSystem) will override only these options; + * the rest will be taken from the defaults specified here. + * + *

    This is meant to be done only once, at the beginning of some main program, + * in order to force all libraries to use some settings we like. * *

    Libraries should never call this. If you're a library then, instead, create your own * filesystem object with the right configuration and pass it along.