From ac326befa4b6e81a60f977c1f22058049d948fcb Mon Sep 17 00:00:00 2001 From: Jean-Philippe Martin Date: Mon, 20 May 2019 15:41:28 -0700 Subject: [PATCH 1/4] Update NIO to understand directory stand-ins The web interface has a "create folder" button, and that creates a file ending in '/' that is used to pretend the folder exists. The web interface maintains the illusion but of course this is still only a file, so all clients (this one included) has to play along and pretend the directory exists (or users get confused). Those fake folders used to be 0-byte, but no longer. This pull request updates our code so it just looks at the trailing slash, ignoring the size of the file. It also adds a test, and in order to write the test we had to add the ability to create files whose name ends in '/', for test purpose only. --- .../nio/CloudStorageFileSystemProvider.java | 13 ++++++-- .../contrib/nio/CloudStorageOptions.java | 8 +++++ .../contrib/nio/OptionAllowTrailingSlash.java | 32 +++++++++++++++++++ .../CloudStorageFileSystemProviderTest.java | 14 ++++++++ 4 files changed, 64 insertions(+), 3 deletions(-) create mode 100644 google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/OptionAllowTrailingSlash.java diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java index 4f22c9aeb89e..b55753e4c266 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java @@ -353,7 +353,8 @@ private SeekableByteChannel newWriteChannel(Path path, Set throws IOException { initStorage(); CloudStoragePath cloudPath = CloudStorageUtil.checkPath(path); - if (cloudPath.seemsLikeADirectoryAndUsePseudoDirectories(null)) { + boolean allowSlash = options.contains(OptionAllowTrailingSlash.getInstance()); + if (!allowSlash && cloudPath.seemsLikeADirectoryAndUsePseudoDirectories(null)) { throw new CloudStoragePseudoDirectoryException(cloudPath); } BlobId file = cloudPath.getBlobId(); @@ -768,8 +769,14 @@ public A readAttributes( } CloudStorageObjectAttributes ret; ret = new CloudStorageObjectAttributes(blobInfo); - // if size is 0 it could be a folder - if (ret.size() == 0 && cloudPath.seemsLikeADirectoryAndUsePseudoDirectories(storage)) { + // There exists a file with this name, yes. But should we pretend it's a directory? + // Because the web UI will allow the user to "create directories" by creating files + // whose name ends in slash (and these files aren't always zero-size). + // If we're set to use pseudo directories and the file name looks like a path, + // then say it's a directory. We pass null to avoid trying to actually list files; + // if the path doesn't end in "/" we'll truthfully say it's a file. Yes it's also + // a directory but we don't want to do a prefix search every time the user stats a file. + if (cloudPath.seemsLikeADirectoryAndUsePseudoDirectories(null)) { @SuppressWarnings("unchecked") A result = (A) new CloudStoragePseudoDirectoryAttributes(cloudPath); return result; diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageOptions.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageOptions.java index db7d45be8397..8863bf50174e 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageOptions.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageOptions.java @@ -95,5 +95,13 @@ public static CloudStorageOption.OpenCopy withChannelReopen(int count) { return OptionMaxChannelReopens.create(count); } + /** + * Allows one to use trailing slashes in file names. You really shouldn't + * (this is here for tests only). + */ + static CloudStorageOption.Open allowTrailingSlash() { + return OptionAllowTrailingSlash.getInstance(); + } + private CloudStorageOptions() {} } diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/OptionAllowTrailingSlash.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/OptionAllowTrailingSlash.java new file mode 100644 index 000000000000..c7a194415e8b --- /dev/null +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/OptionAllowTrailingSlash.java @@ -0,0 +1,32 @@ +/* + * Copyright 2016 Google LLC + * + * 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; + +class OptionAllowTrailingSlash implements CloudStorageOption.Open { + + static OptionAllowTrailingSlash instance; + + private OptionAllowTrailingSlash() {}; + + public synchronized static OptionAllowTrailingSlash getInstance() { + if (null==instance) { + instance = new OptionAllowTrailingSlash(); + } + return instance; + } + +} diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProviderTest.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProviderTest.java index 8056acd52b54..5b7401b881a9 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProviderTest.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProviderTest.java @@ -104,6 +104,13 @@ public void testSize_trailingSlash_returnsFakePseudoDirectorySize() throws Excep assertThat(Files.size(Paths.get(URI.create("gs://bucket/wat/")))).isEqualTo(1); } + @Test + public void test_trailingSlash_isFolder() throws Exception { + Path path = Paths.get(URI.create("gs://bucket/wat/")); + Files.write(path, SINGULARITY.getBytes(UTF_8), CloudStorageOptions.allowTrailingSlash()); + assertThat(Files.isDirectory(path)).isTrue(); + } + @Test public void testSize_trailingSlash_disablePseudoDirectories() throws Exception { try (CloudStorageFileSystem fs = forBucket("doodle", usePseudoDirectories(false))) { @@ -111,6 +118,7 @@ public void testSize_trailingSlash_disablePseudoDirectories() throws Exception { byte[] rapture = SINGULARITY.getBytes(UTF_8); Files.write(path, rapture); assertThat(Files.size(path)).isEqualTo(rapture.length); + Files.delete(path); } } @@ -119,6 +127,7 @@ public void testReadAllBytes() throws Exception { Path path = Paths.get(URI.create("gs://bucket/wat")); Files.write(path, SINGULARITY.getBytes(UTF_8)); assertThat(new String(Files.readAllBytes(path), UTF_8)).isEqualTo(SINGULARITY); + Files.delete(path); } @Test @@ -139,6 +148,7 @@ public void testNewByteChannelRead() throws Exception { buffer.rewind(); assertThat(input.read(buffer)).isEqualTo(-1); } + Files.delete(path); } @Test @@ -160,6 +170,7 @@ public void testNewByteChannelRead_seeking() throws Exception { assertThat(input.position()).isEqualTo(5); assertThat(new String(buffer.array(), UTF_8)).isEqualTo("hello"); } + Files.delete(path); } @Test @@ -667,6 +678,9 @@ public void testCopy_overwriteAttributes() throws Exception { attributes = Files.readAttributes(target2, CloudStorageFileAttributes.class); assertThat(attributes.mimeType()).hasValue("text/palfun"); assertThat(attributes.cacheControl()).hasValue("public; max-age=666"); + Files.delete(source); + Files.delete(target1); + Files.delete(target2); } @Test From c1fd96658abc83debc6d3a54432a859fb738a7ee Mon Sep 17 00:00:00 2001 From: Jean-Philippe Martin Date: Tue, 21 May 2019 10:56:49 -0700 Subject: [PATCH 2/4] formatting changes --- .../nio/CloudStorageFileSystemProvider.java | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java index b55753e4c266..fbb611bfd66a 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java @@ -1,5 +1,5 @@ /* - * Copyright 2016 Google LLC + * Copyright 2019 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -769,13 +769,15 @@ public A readAttributes( } CloudStorageObjectAttributes ret; ret = new CloudStorageObjectAttributes(blobInfo); - // There exists a file with this name, yes. But should we pretend it's a directory? - // Because the web UI will allow the user to "create directories" by creating files - // whose name ends in slash (and these files aren't always zero-size). - // If we're set to use pseudo directories and the file name looks like a path, - // then say it's a directory. We pass null to avoid trying to actually list files; - // if the path doesn't end in "/" we'll truthfully say it's a file. Yes it's also - // a directory but we don't want to do a prefix search every time the user stats a file. + /* + There exists a file with this name, yes. But should we pretend it's a directory? + The web UI will allow the user to "create directories" by creating files + whose name ends in slash (and these files aren't always zero-size). + If we're set to use pseudo directories and the file name looks like a path, + then say it's a directory. We pass null to avoid trying to actually list files; + if the path doesn't end in "/" we'll truthfully say it's a file. Yes it may also be + a directory but we don't want to do a prefix search every time the user stats a file. + */ if (cloudPath.seemsLikeADirectoryAndUsePseudoDirectories(null)) { @SuppressWarnings("unchecked") A result = (A) new CloudStoragePseudoDirectoryAttributes(cloudPath); From d4d78f1319428c7e0e7cbdba172d0bb5c00891c8 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Martin Date: Wed, 22 May 2019 13:06:53 -0700 Subject: [PATCH 3/4] formatting --- .../cloud/storage/contrib/nio/CloudStorageOptions.java | 4 ++-- .../cloud/storage/contrib/nio/OptionAllowTrailingSlash.java | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageOptions.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageOptions.java index 8863bf50174e..2d0548a112e2 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageOptions.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageOptions.java @@ -96,8 +96,8 @@ public static CloudStorageOption.OpenCopy withChannelReopen(int count) { } /** - * Allows one to use trailing slashes in file names. You really shouldn't - * (this is here for tests only). + * Allows one to use trailing slashes in file names. You really shouldn't (this is here for tests + * only). */ static CloudStorageOption.Open allowTrailingSlash() { return OptionAllowTrailingSlash.getInstance(); diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/OptionAllowTrailingSlash.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/OptionAllowTrailingSlash.java index c7a194415e8b..7c0e92033d0f 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/OptionAllowTrailingSlash.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/OptionAllowTrailingSlash.java @@ -22,11 +22,10 @@ class OptionAllowTrailingSlash implements CloudStorageOption.Open { private OptionAllowTrailingSlash() {}; - public synchronized static OptionAllowTrailingSlash getInstance() { - if (null==instance) { + public static synchronized OptionAllowTrailingSlash getInstance() { + if (null == instance) { instance = new OptionAllowTrailingSlash(); } return instance; } - } From 5615f46f15a596cb9396ff5747971d2dccc0967b Mon Sep 17 00:00:00 2001 From: Jean-Philippe Martin Date: Thu, 23 May 2019 09:58:35 -0700 Subject: [PATCH 4/4] Update (C) year --- .../storage/contrib/nio/CloudStorageFileSystemProvider.java | 2 +- .../cloud/storage/contrib/nio/OptionAllowTrailingSlash.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java index fbb611bfd66a..f1e35eed067f 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java @@ -1,5 +1,5 @@ /* - * Copyright 2019 Google LLC + * Copyright 2016 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/OptionAllowTrailingSlash.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/OptionAllowTrailingSlash.java index 7c0e92033d0f..751222dd8cdb 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/OptionAllowTrailingSlash.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/OptionAllowTrailingSlash.java @@ -1,5 +1,5 @@ /* - * Copyright 2016 Google LLC + * Copyright 2019 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License.