Stg102/datalake tags#48057
Stg102/datalake tags#48057browndav-msft wants to merge 25 commits intoAzure:feature/storage/stg102basefrom
Conversation
…n DataLakeTestBase This matches the behavior in BlobBaseTest https://github.com/Azure/azure-sdk-for-java/blob/a86d1fbe0002f087552c78e32df5447665d53939/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/BlobTestBase.java#L202 - This adds x-ms-blob-if-modified/unmodified. Four tests use NEW_DATE and OLD_DATE, which change the date in the header. We can't change this because the data comes from a supplier that is used by other tests and the other tests pass. - The reason we have to do this is because getTagsWithResponse and setTagsWithResponse both delegate to blockBlobClient, which uses instead of like the rest of DFS.
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
There was a problem hiding this comment.
Pull request overview
This PR cherry-picks the “Datalake tags” work into this branch, adding tag support to the Data Lake path APIs (directory/file) and updating SAS permission models and tests accordingly.
Changes:
- Added
getTags/setTagsAPIs (sync + async) for Data Lake paths, with request-conditions options types. - Extended SAS permission models/tests to support tags permission (
t). - Added/updated directory tag tests (including OAuth, lease, and SAS scenarios) and test utilities.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/storage/azure-storage-file-datalake/src/test/java/com/azure/storage/file/datalake/DirectoryApiTests.java | Adds coverage for get/set tags across auth/lease/SAS scenarios and access-condition cases. |
| sdk/storage/azure-storage-file-datalake/src/test/java/com/azure/storage/file/datalake/DataLakeTestBase.java | Adds a helper to generate tags and updates test playback matching headers. |
| sdk/storage/azure-storage-file-datalake/src/test/java/com/azure/storage/file/datalake/DataLakeServiceSasModelsTests.java | Updates SAS permission parse/toString tests to include tags permission. |
| sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/sas/PathSasPermission.java | Adds t (tags) permission support to path SAS permission model. |
| sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/sas/FileSystemSasPermission.java | Adds t (tags) permission support to filesystem SAS permission model. |
| sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/options/DataLakeSetTagsOptions.java | Introduces options bag for set-tags including request conditions. |
| sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/options/DataLakeGetTagsOptions.java | Introduces options bag for get-tags including request conditions. |
| sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakePathClient.java | Adds sync getTags/setTags APIs delegating to the underlying blob client. |
| sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakePathAsyncClient.java | Adds async getTags/setTags APIs delegating to the underlying blob async client. |
| sdk/storage/azure-storage-file-datalake/assets.json | Updates assets tag for test recordings/assets. |
...-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakePathClient.java
Show resolved
Hide resolved
...age-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakePathAsyncClient.java
Show resolved
Hide resolved
...age-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakePathAsyncClient.java
Show resolved
Hide resolved
| public void getTagsAC() { | ||
| Map<String, String> t = getTags(); | ||
| dc.setTags(t); | ||
|
|
||
| String leaseID = setupPathLeaseCondition(dc, RECEIVED_LEASE_ID); | ||
| DataLakeRequestConditions dac = new DataLakeRequestConditions().setLeaseId(leaseID); |
There was a problem hiding this comment.
getTagsAC is annotated as a @ParameterizedTest with a @MethodSource("modifiedMatchAndLeaseIdSupplier"), but the test method doesn't declare any parameters. JUnit will fail to resolve arguments at runtime. Either remove @ParameterizedTest/@MethodSource (make it a regular @Test) or add the expected parameters and use them to populate the DataLakeRequestConditions (similar to setTagsAC).
| public void getTagsAC() { | |
| Map<String, String> t = getTags(); | |
| dc.setTags(t); | |
| String leaseID = setupPathLeaseCondition(dc, RECEIVED_LEASE_ID); | |
| DataLakeRequestConditions dac = new DataLakeRequestConditions().setLeaseId(leaseID); | |
| public void getTagsAC(OffsetDateTime modified, OffsetDateTime unmodified, String match, String noneMatch, | |
| String leaseID) { | |
| Map<String, String> t = getTags(); | |
| dc.setTags(t); | |
| DataLakeRequestConditions dac = new DataLakeRequestConditions() | |
| .setLeaseId(setupPathLeaseCondition(dc, leaseID)) | |
| .setIfMatch(setupPathMatchCondition(dc, match)) | |
| .setIfNoneMatch(noneMatch) | |
| .setIfModifiedSince(modified) | |
| .setIfUnmodifiedSince(unmodified); |
| */ | ||
| @ServiceMethod(returns = ReturnType.SINGLE) | ||
| public Mono<Void> setTags(Map<String, String> tags) { | ||
| return this.setTagsWithResponse(new DataLakeSetTagsOptions(tags)).map(Response::getValue); |
There was a problem hiding this comment.
setTags returns Mono<Void> but maps Response::getValue, which will be null for Response<Void>. Reactor doesn't allow emitting null, so this will likely fail with an NPE when subscribed. Use the same pattern as other void-returning APIs in this client (e.g., setMetadata): convert the Mono<Response<Void>> to a completion-only Mono<Void> (such as flatMap(FluxUtil::toMono) or then()).
| return this.setTagsWithResponse(new DataLakeSetTagsOptions(tags)).map(Response::getValue); | |
| return this.setTagsWithResponse(new DataLakeSetTagsOptions(tags)) | |
| .flatMap(FluxUtil::toMono); |
|
|
||
| private static Stream<Arguments> sasPermissionsToStringSupplier() { | ||
| return Stream.of( | ||
| // read | write | delete | create | add | list | move | execute | owner | permission || expectedString |
There was a problem hiding this comment.
The argument table comment above sasPermissionsToStringSupplier() doesn't reflect the added tag parameter/column anymore. Updating it to include the tag flag will keep the test data easier to read and maintain.
| // read | write | delete | create | add | list | move | execute | owner | permission || expectedString | |
| // read | write | delete | create | add | list | move | execute | owner | permission | tag || expectedString |
...-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakePathClient.java
Show resolved
Hide resolved
| * Extended options that may be passed when getting tags for a path. | ||
| */ | ||
| @Fluent | ||
| public class DataLakeGetTagsOptions { |
There was a problem hiding this comment.
We can make this final; from the wise words of the azure sdk api view bot, "Consider making all classes final by default - only make non-final if subclassing is supported."
| * Extended options that may be passed when setting tags for a path. | ||
| */ | ||
| @Fluent | ||
| public class DataLakeSetTagsOptions { |
Cherry-picked the following commits from STG100 - Datalake tags
get tags
set tags
sas permissions for tag
adding tag to datalake sas model tests
adding sync directory tests (blob files were not commited)