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

Addressed comments for immutable storage with versioning #22388

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

package com.azure.storage.blob.implementation.models;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a concern with putting handwritten models in a package where generated code lives. Any thoughts about that? @kasobol-msft

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ideally we'd have com.azure.storage.blob.implementation.generated.* for everything generated.
or have "hadwriten.models" :-). For this PR we can mix them, but it would be good to sit down and do some cleanup in implementation pacakge.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


import com.azure.storage.blob.models.BlobLegalHoldResult;

/**
* The blob legal hold result.
*/
public class InternalBlobLegalHoldResult implements BlobLegalHoldResult {

private final boolean hasLegalHold;

/**
* Creates a new BlobLegalHoldResult
* @param hasLegalHold whether or not a legal hold is enabled on the blob.
*/
public InternalBlobLegalHoldResult(boolean hasLegalHold) {
this.hasLegalHold = hasLegalHold;
}

/**
* @return whether or not a legal hold is enabled on the blob.
*/
public boolean hasLegalHold() {
return hasLegalHold;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

package com.azure.storage.blob.implementation.util;

public enum BlobRequestConditionProperty {
LEASE_ID,
TAGS_CONDITIONS,
IF_MODIFIED_SINCE,
IF_UNMODIFIED_SINCE,
IF_MATCH,
IF_NONE_MATCH;
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.azure.storage.blob.models.BlobItemProperties;
import com.azure.storage.blob.models.BlobLeaseRequestConditions;
import com.azure.storage.blob.models.BlobQueryHeaders;
import com.azure.storage.blob.models.BlobRequestConditions;
import com.azure.storage.blob.models.ObjectReplicationPolicy;
import com.azure.storage.blob.models.ObjectReplicationRule;
import com.azure.storage.blob.models.ObjectReplicationStatus;
Expand All @@ -37,6 +38,7 @@
import java.net.URL;
import java.util.ArrayList;
import java.util.Collections;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -498,4 +500,39 @@ public static BlobQueryHeaders transformQueryHeaders(HttpHeaders headers) {
throw LOGGER.logExceptionAsError(new RuntimeException(e));
}
}

public static void validateConditionsNotPresent(BlobRequestConditions requestConditions,
EnumSet<BlobRequestConditionProperty> invalidConditions) {
if (requestConditions == null) {
return;
}
if (invalidConditions.contains(BlobRequestConditionProperty.LEASE_ID)
&& requestConditions.getLeaseId() != null) {
throw LOGGER.logExceptionAsError(new IllegalArgumentException("'leaseId' is not applicable to this API."));
}
if (invalidConditions.contains(BlobRequestConditionProperty.TAGS_CONDITIONS)
&& requestConditions.getTagsConditions() != null) {
throw LOGGER.logExceptionAsError(new IllegalArgumentException("'tagsConditions' is not applicable to "
+ "this API."));
}
if (invalidConditions.contains(BlobRequestConditionProperty.IF_MODIFIED_SINCE)
&& requestConditions.getIfModifiedSince() != null) {
throw LOGGER.logExceptionAsError(new IllegalArgumentException("'ifModifiedSince' is not applicable to "
+ "this API."));
}
if (invalidConditions.contains(BlobRequestConditionProperty.IF_UNMODIFIED_SINCE)
&& requestConditions.getIfUnmodifiedSince() != null) {
throw LOGGER.logExceptionAsError(new IllegalArgumentException("'ifUnmodifiedSince' is not applicable to "
+ "this API."));
}
if (invalidConditions.contains(BlobRequestConditionProperty.IF_MATCH)
&& requestConditions.getIfMatch() != null) {
throw LOGGER.logExceptionAsError(new IllegalArgumentException("'ifMatch' is not applicable to this API."));
}
if (invalidConditions.contains(BlobRequestConditionProperty.IF_NONE_MATCH)
&& requestConditions.getIfNoneMatch() != null) {
throw LOGGER.logExceptionAsError(new IllegalArgumentException("'ifNoneMatch' is not applicable to this "
+ "API."));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,6 @@
/**
* The blob legal hold result.
*/
public class BlobLegalHoldResult {

private final boolean hasLegalHold;

/**
* Creates a new BlobLegalHoldResult
* @param hasLegalHold whether or not a legal hold is enabled on the blob.
*/
public BlobLegalHoldResult(boolean hasLegalHold) {
this.hasLegalHold = hasLegalHold;
}

/**
* @return whether or not a legal hold is enabled on the blob.
*/
public boolean hasLegalHold() {
return hasLegalHold;
}
public interface BlobLegalHoldResult {
boolean hasLegalHold();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this have javadoc?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah. Let me add that back

}
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,21 @@
import com.azure.storage.blob.implementation.models.BlobsSetImmutabilityPolicyHeaders;
import com.azure.storage.blob.implementation.models.BlobsStartCopyFromURLHeaders;
import com.azure.storage.blob.implementation.models.EncryptionScope;
import com.azure.storage.blob.implementation.models.InternalBlobLegalHoldResult;
import com.azure.storage.blob.implementation.models.QueryRequest;
import com.azure.storage.blob.implementation.models.QuerySerialization;
import com.azure.storage.blob.implementation.util.BlobQueryReader;
import com.azure.storage.blob.implementation.util.BlobRequestConditionProperty;
import com.azure.storage.blob.implementation.util.BlobSasImplUtil;
import com.azure.storage.blob.implementation.util.ChunkedDownloadUtils;
import com.azure.storage.blob.implementation.util.ModelHelper;
import com.azure.storage.blob.models.AccessTier;
import com.azure.storage.blob.models.ArchiveStatus;
import com.azure.storage.blob.models.BlobDownloadContentAsyncResponse;
import com.azure.storage.blob.models.BlobDownloadHeaders;
import com.azure.storage.blob.models.BlobBeginCopySourceRequestConditions;
import com.azure.storage.blob.models.BlobCopyInfo;
import com.azure.storage.blob.models.BlobDownloadAsyncResponse;
import com.azure.storage.blob.models.BlobDownloadContentAsyncResponse;
import com.azure.storage.blob.models.BlobDownloadHeaders;
import com.azure.storage.blob.models.BlobHttpHeaders;
import com.azure.storage.blob.models.BlobImmutabilityPolicy;
import com.azure.storage.blob.models.BlobImmutabilityPolicyMode;
Expand Down Expand Up @@ -98,6 +100,7 @@
import java.time.Duration;
import java.time.OffsetDateTime;
import java.util.ArrayList;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -2156,7 +2159,12 @@ Mono<Response<BlobImmutabilityPolicy>> setImmutabilityPolicyWithResponse(

BlobRequestConditions finalRequestConditions = requestConditions == null
? new BlobRequestConditions() : requestConditions;
// TODO (gapra) : Discuss, should we just expose ifUnmodifiedSince here?

ModelHelper.validateConditionsNotPresent(finalRequestConditions,
EnumSet.of(BlobRequestConditionProperty.LEASE_ID, BlobRequestConditionProperty.TAGS_CONDITIONS,
BlobRequestConditionProperty.IF_MATCH, BlobRequestConditionProperty.IF_NONE_MATCH,
BlobRequestConditionProperty.IF_MODIFIED_SINCE));

return this.azureBlobStorage.getBlobs().setImmutabilityPolicyWithResponseAsync(containerName, blobName, null,
null, finalRequestConditions.getIfUnmodifiedSince(), finalImmutabilityPolicy.getExpiryTime(),
finalImmutabilityPolicy.getPolicyMode(),
Expand Down Expand Up @@ -2265,6 +2273,6 @@ Mono<Response<BlobLegalHoldResult>> setLegalHoldWithResponse(boolean legalHold,
legalHold, null, null,
context.addData(AZ_TRACING_NAMESPACE_KEY, STORAGE_TRACING_NAMESPACE_VALUE))
.map(response -> new SimpleResponse<>(response,
new BlobLegalHoldResult(response.getDeserializedHeaders().isXMsLegalHold())));
new InternalBlobLegalHoldResult(response.getDeserializedHeaders().isXMsLegalHold())));
}
}
Loading