Skip to content

Commit

Permalink
Nio Walk File Tree (#27253)
Browse files Browse the repository at this point in the history
* Wrote tests

* Added recordings

* CI fixes

* Pr feedback

* PR feedback

* Updated readme samples

* CI fix

* Ci fixes

* Pr feedback

* woops

* Updated recording file
  • Loading branch information
rickle-msft authored Mar 4, 2022
1 parent 18a60d9 commit 82193b6
Show file tree
Hide file tree
Showing 21 changed files with 3,290 additions and 424 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,16 @@
<Bug pattern="NM_CONFUSING"/>
</Match>

<!-- Returning null is correct in these cases as it represents the not-applicable case -->
<Match>
<Class name="com.azure.storage.blob.nio.AzureBlobFileAttributes"/>
<Or>
<Method name="isServerEncrypted"/>
<Method name="isAccessTierInferred"/>
</Or>
<Bug pattern="NP_BOOLEAN_RETURN_NULL"/>
</Match>

<!-- Incorrect flagging, there is already switchIfEmpty operator applied in the upstream to throw error on empty -->
<Match>
<Class name="com.azure.core.util.polling.DefaultSyncPoller"/>
Expand Down Expand Up @@ -2174,7 +2184,7 @@
UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR,
UWF_UNWRITTEN_FIELD"/>
</Match>

<!-- Exclude AAP source code types from spotbug -->
<Match>
<Class name="~com\.azure\.cosmos\.encryption\.implementation\.mdesrc(.+)"/>
Expand Down
4 changes: 4 additions & 0 deletions sdk/storage/azure-storage-blob-nio/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,16 @@
## 12.0.0-beta.17 (Unreleased)

### Features Added
- Enabled support for Files.exists()
- Enabled support for Files.walkFileTree()

### Breaking Changes
- `AzureFileSystemProvider.readAttributes()` no longer throws an IOException for virtual directories and instead returns a set of attributes that are all empty except for an `isVirtual` property set to true.

### Bugs Fixed

### Other Changes
- Enabling support for Files.exists() to support virtual directories required supporting virtual directories in reading file attributes. This required introducing a perf hit in the way of an extra getProps request

## 12.0.0-beta.16 (2022-02-11)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
package com.azure.storage.blob.nio;

import com.azure.core.util.logging.ClientLogger;
import com.azure.storage.blob.models.BlobProperties;
import com.azure.storage.blob.models.BlobStorageException;

import java.io.IOException;
import java.nio.file.Path;
Expand All @@ -24,6 +22,8 @@
* {@link AzureBlobFileAttributes} is generally preferred.
* <p>
* Some attributes are not supported. Refer to the javadocs on each method for more information.
* <p>
* If the target file is a virtual directory, most attributes will be set to null.
*/
public final class AzureBasicFileAttributes implements BasicFileAttributes {
private final ClientLogger logger = new ClientLogger(AzureBasicFileAttributes.class);
Expand All @@ -35,64 +35,60 @@ public final class AzureBasicFileAttributes implements BasicFileAttributes {
set.add("lastModifiedTime");
set.add("isRegularFile");
set.add("isDirectory");
set.add("isVirtualDirectory");
set.add("isSymbolicLink");
set.add("isOther");
set.add("size");
set.add("creationTime");
ATTRIBUTE_STRINGS = Collections.unmodifiableSet(set);
}

private final BlobProperties properties;
private final AzureResource resource;
private final AzureBlobFileAttributes internalAttributes;

/*
There are some work-arounds we could do to try to accommodate virtual directories such as making a checkDirStatus
call before or after getProperties to throw an appropriate error or adding an isVirtualDirectory method. However,
the former wastes network time only to throw a slightly more specific error when we will throw on 404 anyway. The
latter introduces virtual directories into the actual code path/api surface. While we are clear in our docs about
the possible pitfalls of virtual directories, and customers should be aware of it, they shouldn't have to code
against it. Therefore, we fall back to documenting that reading attributes on a virtual directory will throw.
In order to support Files.exist() and other methods like Files.walkFileTree() which depend on it, we have had to add
support for virtual directories. This is not ideal as customers will have to now perform null checks when inspecting
attributes (or at least check if it is a virtual directory before inspecting properties). It also incurs extra
network requests as we have to call a checkDirectoryExists() after receiving the initial 404. This is two
additional network requests, though they only happen in the case when a file doesn't exist or is virtual, so it
shouldn't happen in the majority of api calls.
*/
AzureBasicFileAttributes(Path path) throws IOException {
try {
this.resource = new AzureResource(path);
this.properties = resource.getBlobClient().getProperties();
} catch (BlobStorageException e) {
throw LoggingUtility.logError(logger, new IOException(e));
}
this.internalAttributes = new AzureBlobFileAttributes(path);
}

/**
* Returns the time of last modification.
* Returns the time of last modification or null if this is a virtual directory.
*
* @return the time of last modification.
* @return the time of last modification or null if this is a virtual directory
*/
@Override
public FileTime lastModifiedTime() {
return FileTime.from(properties.getLastModified().toInstant());
return this.internalAttributes.lastModifiedTime();
}

/**
* Returns the time of last modification.
* Returns the time of last modification or null if this is a virtual directory
* <p>
* Last access time is not supported by the blob service. In this case, it is typical for implementations to return
* the {@link #lastModifiedTime()}.
*
* @return the time of last modification.
* @return the time of last modification or null if this is a virtual directory
*/
@Override
public FileTime lastAccessTime() {
return this.lastModifiedTime();
return this.internalAttributes.lastAccessTime();
}

/**
* Returns the creation time. The creation time is the time that the file was created.
* Returns the creation time. The creation time is the time that the file was created. Returns null if this is a
* virtual directory.
*
* @return The creation time.
* @return The creation time or null if this is a virtual directory
*/
@Override
public FileTime creationTime() {
return FileTime.from(properties.getCreationTime().toInstant());
return this.internalAttributes.creationTime();
}

/**
Expand All @@ -102,7 +98,7 @@ public FileTime creationTime() {
*/
@Override
public boolean isRegularFile() {
return !this.properties.getMetadata().getOrDefault(AzureResource.DIR_METADATA_MARKER, "false").equals("true");
return this.internalAttributes.isRegularFile();
}

/**
Expand All @@ -116,7 +112,19 @@ public boolean isRegularFile() {
*/
@Override
public boolean isDirectory() {
return !this.isRegularFile();
return this.internalAttributes.isDirectory();
}

/**
* Tells whether the file is a virtual directory.
* <p>
* See {@link AzureFileSystemProvider#createDirectory(Path, FileAttribute[])} for more information on virtual and
* concrete directories.
*
* @return whether the file is a virtual directory
*/
public boolean isVirtualDirectory() {
return this.internalAttributes.isVirtualDirectory();
}

/**
Expand All @@ -126,7 +134,7 @@ public boolean isDirectory() {
*/
@Override
public boolean isSymbolicLink() {
return false;
return this.internalAttributes.isSymbolicLink();
}

/**
Expand All @@ -136,7 +144,7 @@ public boolean isSymbolicLink() {
*/
@Override
public boolean isOther() {
return false;
return this.internalAttributes.isOther();
}

/**
Expand All @@ -146,7 +154,7 @@ public boolean isOther() {
*/
@Override
public long size() {
return properties.getBlobSize();
return this.internalAttributes.size();
}

/**
Expand All @@ -156,6 +164,6 @@ public long size() {
*/
@Override
public Object fileKey() {
return this.resource.getBlobClient().getBlobUrl();
return this.internalAttributes.fileKey();
}
}
Loading

0 comments on commit 82193b6

Please sign in to comment.