Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Try harder to make believable directories #3775

Merged
merged 10 commits into from
Oct 10, 2018
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@ public abstract class CloudStorageConfiguration {
public abstract boolean stripPrefixSlash();

/**
* Returns {@code true} if paths with a trailing slash should be treated as fake directories.
* Returns {@code true} if directories and paths with a trailing slash should be treated as fake
* directories.
*
* <p>With this feature, if file "foo/bar.txt" exists then both "foo" and "foo/" will be
* treated as if they were existing directories.
*/
public abstract boolean usePseudoDirectories();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ private SeekableByteChannel newReadChannel(Path path, Set<? extends OpenOption>
}
}
CloudStoragePath cloudPath = CloudStorageUtil.checkPath(path);
if (cloudPath.seemsLikeADirectoryAndUsePseudoDirectories()) {
if (cloudPath.seemsLikeADirectoryAndUsePseudoDirectories(storage)) {
throw new CloudStoragePseudoDirectoryException(cloudPath);
}
return CloudStorageReadChannel.create(
Expand All @@ -348,7 +348,7 @@ private SeekableByteChannel newWriteChannel(Path path, Set<? extends OpenOption>
throws IOException {
initStorage();
CloudStoragePath cloudPath = CloudStorageUtil.checkPath(path);
if (cloudPath.seemsLikeADirectoryAndUsePseudoDirectories()) {
if (cloudPath.seemsLikeADirectoryAndUsePseudoDirectories(storage)) {
throw new CloudStoragePseudoDirectoryException(cloudPath);
}
BlobId file = cloudPath.getBlobId();
Expand Down Expand Up @@ -439,7 +439,7 @@ public InputStream newInputStream(Path path, OpenOption... options) throws IOExc
public boolean deleteIfExists(Path path) throws IOException {
initStorage();
CloudStoragePath cloudPath = CloudStorageUtil.checkPath(path);
if (cloudPath.seemsLikeADirectoryAndUsePseudoDirectories()) {
if (cloudPath.seemsLikeADirectoryAndUsePseudoDirectories(storage)) {
// if the "folder" is empty then we're fine, otherwise complain
// that we cannot act on folders.
try (DirectoryStream<Path> paths = Files.newDirectoryStream(path)) {
Expand Down Expand Up @@ -567,10 +567,10 @@ public void copy(Path source, Path target, CopyOption... options) throws IOExcep
"File systems associated with paths don't agree on pseudo-directories.");
}
}
if (fromPath.seemsLikeADirectoryAndUsePseudoDirectories()) {
if (fromPath.seemsLikeADirectoryAndUsePseudoDirectories(storage)) {
throw new CloudStoragePseudoDirectoryException(fromPath);
}
if (toPath.seemsLikeADirectoryAndUsePseudoDirectories()) {
if (toPath.seemsLikeADirectoryAndUsePseudoDirectories(storage)) {
throw new CloudStoragePseudoDirectoryException(toPath);
}

Expand Down Expand Up @@ -665,7 +665,7 @@ public void checkAccess(Path path, AccessMode... modes) throws IOException {
while (true) {
try {
CloudStoragePath cloudPath = CloudStorageUtil.checkPath(path);
if ( cloudPath.seemsLikeADirectoryAndUsePseudoDirectories() ) {
if ( cloudPath.seemsLikeADirectoryAndUsePseudoDirectories(storage) ) {
return;
}
boolean nullId;
Expand Down Expand Up @@ -708,7 +708,7 @@ public <A extends BasicFileAttributes> A readAttributes(
while (true) {
try {
CloudStoragePath cloudPath = CloudStorageUtil.checkPath(path);
if ( cloudPath.seemsLikeADirectoryAndUsePseudoDirectories() ) {
if ( cloudPath.seemsLikeADirectoryAndUsePseudoDirectories(storage) ) {
@SuppressWarnings("unchecked")
A result = (A) new CloudStoragePseudoDirectoryAttributes(cloudPath);
return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;

import com.google.api.gax.paging.Page;
import com.google.cloud.storage.Blob;
import com.google.cloud.storage.BlobId;
import com.google.cloud.storage.Storage;
import com.google.common.collect.UnmodifiableIterator;

import java.io.File;
Expand Down Expand Up @@ -83,8 +86,39 @@ boolean seemsLikeADirectory() {
return path.seemsLikeADirectory();
}

boolean seemsLikeADirectoryAndUsePseudoDirectories() {
return path.seemsLikeADirectory() && fileSystem.config().usePseudoDirectories();
boolean seemsLikeADirectoryAndUsePseudoDirectories(Storage storage) {
if (!fileSystem.config().usePseudoDirectories()) {
chingor13 marked this conversation as resolved.
Show resolved Hide resolved
return false;
}
if (path.seemsLikeADirectory()) {
return true;
}
// fancy case: the file name doesn't end in slash, but we've been asked to have pseudo dirs.
// Let's see if there are any files with this prefix.
if (storage == null) {
// we are in a context where we don't want to access the storage, so we conservatively
// say this isn't a directory.
return false;
}
Page<Blob> list = storage.list(
this.bucket(),
Storage.BlobListOption.prefix(path.removeBeginningSeparator().toString()),
Storage.BlobListOption.pageSize(1));
jean-philippe-martin marked this conversation as resolved.
Show resolved Hide resolved
for (Blob b : list.getValues()) {
// if this blob starts with our prefix and then a slash, then prefix is indeed a folder!
if (b.getBlobId() == null) {
continue;
}
String name = b.getBlobId().getName();
if (name == null) {
continue;
}
if (("/" + name).startsWith(this.path.toAbsolutePath() + "/")) {
return true;
}
}
// no match, so it's not a folder
return false;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ public Tuple<String, Iterable<StorageObject>> list(String bucket, Map<Option, ?>
throws StorageException {
String delimiter = null;
String preprefix = "";
long maxResults = Long.MAX_VALUE;
for (Map.Entry<Option, ?> e : options.entrySet()) {
switch (e.getKey()) {
case PREFIX:
Expand All @@ -138,6 +139,9 @@ public Tuple<String, Iterable<StorageObject>> list(String bucket, Map<Option, ?>
case FIELDS:
// ignore and return all the fields
break;
case MAX_RESULTS:
maxResults = (Long) e.getValue();
break;
default:
throw new UnsupportedOperationException("Unknown option: " + e.getKey());
}
Expand All @@ -156,6 +160,16 @@ public Tuple<String, Iterable<StorageObject>> list(String bucket, Map<Option, ?>
values.add(so);
}
values.addAll(folders.values());

// truncate to max allowed length
if (values.size() > maxResults) {
List<StorageObject> newValues = new ArrayList<>();
for (int i=0; i < maxResults; i++) {
newValues.add(values.get(i));
}
values = newValues;
}

// null cursor to indicate there is no more data (empty string would cause us to be called again).
// The type cast seems to be necessary to help Java's typesystem remember that collections are iterable.
return Tuple.of(null, (Iterable<StorageObject>) values);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,46 @@ public void testListFilesInRootDirectory() throws IOException {
assertThat(objectNames).containsExactly(BIG_FILE, SML_FILE);
}

@Test
public void testFakeDirectories() throws IOException {
try (FileSystem fs = getTestBucket()) {
List<Path> paths = new ArrayList<>();
paths.add(fs.getPath("dir/angel"));
paths.add(fs.getPath("dir/deeper/fish"));
for (Path path : paths) {
fillFile(storage, BUCKET, path.toString(), SML_SIZE);
}

// ends with slash, must be a directory
assertThat(Files.isDirectory(fs.getPath("dir/"))).isTrue();
// files are not directories
assertThat(Files.isDirectory(fs.getPath("dir/angel"))).isFalse();
// directories are recognized even without the trailing "/"
assertThat(Files.isDirectory(fs.getPath("dir"))).isTrue();
// also works for absolute paths
assertThat(Files.isDirectory(fs.getPath("/dir"))).isTrue();
// non-existent files are not directories (but they don't make us crash)
assertThat(Files.isDirectory(fs.getPath("di"))).isFalse();
assertThat(Files.isDirectory(fs.getPath("dirs"))).isFalse();
assertThat(Files.isDirectory(fs.getPath("dir/deep"))).isFalse();
assertThat(Files.isDirectory(fs.getPath("dir/deeper/fi"))).isFalse();
assertThat(Files.isDirectory(fs.getPath("/dir/deeper/fi"))).isFalse();
// also works for subdirectories
assertThat(Files.isDirectory(fs.getPath("dir/deeper/"))).isTrue();
assertThat(Files.isDirectory(fs.getPath("dir/deeper"))).isTrue();
assertThat(Files.isDirectory(fs.getPath("/dir/deeper/"))).isTrue();
assertThat(Files.isDirectory(fs.getPath("/dir/deeper"))).isTrue();
// the root folder is a directory
assertThat(Files.isDirectory(fs.getPath("/"))).isTrue();
assertThat(Files.isDirectory(fs.getPath(""))).isTrue();

// clean up
for (Path path : paths) {
Files.delete(path);
}
}
}

/**
* Delete the given directory and all of its contents if non-empty.
* @param directory the directory to delete
Expand Down