-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Implemented ORS List Blobs #11756
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
Implemented ORS List Blobs #11756
Conversation
sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ContainerAPITest.groovy
Show resolved
Hide resolved
| this.objectReplicationSourcePolicies = null; | ||
| if (blobItemInternal.getObjectReplicationMetadata() != null) { | ||
| this.objectReplicationSourcePolicies = new HashMap<>(); | ||
| for (String orString : blobItemInternal.getObjectReplicationMetadata().keySet()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think CI will tell you this, but we should use entrySet instead of keySet so we don't have to index into the map
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
alzimmermsft
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to add another parameter to the BlobRequestConditions model for ifTags? There will need to be a Swagger transform to remove the following from the $.parameters.IfTags.
"x-ms-parameter-grouping": {
"name": "modified-access-conditions"
},
.../azure-storage-blob/src/main/java/com/azure/storage/blob/implementation/AppendBlobsImpl.java
Show resolved
Hide resolved
| String ifTags = null; | ||
| if (modifiedAccessConditions != null) { | ||
| ifTags = modifiedAccessConditions.getIfTags(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to add another property to the BlobRequestConditions class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ifTags generated code stuff wasnt meant to be part of this PR but someone had already changed swagger so I havent implemented it yet. That will come in a future PR.
Bute you're right, I think we need to remove the grouping and add it to BlobRequestCondiitons like the other params.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this PR, we want to just focus on the listBlobs changes
kasobol-msft
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Please sync data model with Azure/azure-sdk-for-net#12740
- Please hold with merge until current STG73 branch ships. So we don't creep APIViews.
| if (blobItemInternal.getObjectReplicationMetadata() != null) { | ||
| this.objectReplicationSourcePolicies = new HashMap<>(); | ||
| for (Map.Entry<String, String> p : blobItemInternal.getObjectReplicationMetadata().entrySet()) { | ||
| String orString = p.getKey(); | ||
| String str = orString.startsWith("or-") ? orString.substring(3) : orString; | ||
| String[] split = str.split("_"); | ||
| String policyId = split[0]; | ||
| String ruleId = split[1]; | ||
| if (objectReplicationSourcePolicies.containsKey(policyId)) { | ||
| objectReplicationSourcePolicies.get(policyId) | ||
| .putRuleAndStatus(ruleId, p.getValue()); | ||
| } else { | ||
| ObjectReplicationPolicy policy = new ObjectReplicationPolicy(policyId); | ||
| policy.putRuleAndStatus(ruleId, p.getValue()); | ||
| objectReplicationSourcePolicies.put(policyId, policy); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to find new home. We need to decouple model from "implementation model" per APIView feedback.
Anyway, it's a good practice to NOT place such logic inside constructors. BlobItem should ideally be plain data structure and this logic should move to some "transformer/mapper".
No description provided.