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

feat: integrate disable Lifecycle rule api for to remove lifecycle rule of bucket #28

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
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
13 changes: 12 additions & 1 deletion google-cloud-storage/clirr-ignored-differences.xml
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,15 @@
<method>com.google.cloud.storage.PostPolicyV4 generateSignedPostPolicyV4(com.google.cloud.storage.BlobInfo, long, java.util.concurrent.TimeUnit, com.google.cloud.storage.Storage$PostPolicyV4Option[])</method>
<differenceType>7012</differenceType>
</difference>
</differences>
<difference>
<className>com/google/cloud/storage/Storage</className>
<method>boolean deleteLifecycleRules(java.lang.String)</method>
<differenceType>7012</differenceType>
</difference>
<difference>
<className>com/google/cloud/storage/spi/v1/StorageRpc</className>
<method>boolean deleteLifecycleRules(com.google.api.services.storage.model.Bucket)</method>
<differenceType>7012</differenceType>
</difference>
</differences>

Original file line number Diff line number Diff line change
Expand Up @@ -1114,6 +1114,37 @@ public boolean deleteDefaultAcl(Entity entity) {
return storage.deleteDefaultAcl(getName(), entity);
}

/**
* Delete the lifecycle rules of this bucket.
*
* <p>Example of deleting the lifecycle rules of this bucket.
*
* <pre>{@code
* String bucketName = "my-unique-bucket";
* Bucket bucket =
* storage.create(
* BucketInfo.newBuilder(bucketName)
* .setStorageClass(StorageClass.COLDLINE)
* .setLocation("us-central1")
* .setLifecycleRules(
* ImmutableList.of(
* new LifecycleRule(
* LifecycleAction.newDeleteAction(),
* LifecycleCondition.newBuilder().setAge(2).build())))
* .build());
* boolean rulesDeleted = bucket.deleteLifecycleRules();
athakor marked this conversation as resolved.
Show resolved Hide resolved
* if (rulesDeleted) {
* // the lifecycle rules were deleted
* }
* }</pre>
*
* @return {@code true} if the bucket lifecycle rules were deleted.
* @throws StorageException upon failure
*/
public boolean deleteLifecycleRules() {
return storage.deleteLifecycleRules(getName());
}

/**
* Creates a new default blob ACL entry on this bucket.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1999,7 +1999,6 @@ Blob create(
* only if supplied Decrpytion Key decrypts the blob successfully, otherwise a {@link
* StorageException} is thrown. For more information review
*
* @throws StorageException upon failure
* @see <a
* href="https://cloud.google.com/storage/docs/encryption/customer-supplied-keys#encrypted-elements">Encrypted
* Elements</a>
Expand Down Expand Up @@ -3076,6 +3075,36 @@ PostPolicyV4 generateSignedPostPolicyV4(
*/
boolean deleteDefaultAcl(String bucket, Entity entity);

/**
* Delete the lifecycle rules of the requested bucket.
*
* <p>Example of deleting the lifecycle rules of the requested bucket.
*
* <pre>{@code
* String bucketName = "my-unique-bucket";
* Bucket bucket =
* storage.create(
* BucketInfo.newBuilder(bucketName)
* .setStorageClass(StorageClass.COLDLINE)
* .setLocation("us-central1")
* .setLifecycleRules(
* ImmutableList.of(
* new LifecycleRule(
* LifecycleAction.newDeleteAction(),
* LifecycleCondition.newBuilder().setAge(2).build())))
* .build());
* boolean rulesDeleted = storage.deleteLifecycleRules(bucketName);
* if (rulesDeleted) {
* // the lifecycle rules were deleted
* }
* }</pre>
*
* @param bucket name of the bucket.
* @return {@code true} if the bucket lifecycle rules were deleted.
* @throws StorageException upon failure
*/
boolean deleteLifecycleRules(String bucket);

/**
* Creates a new default blob ACL entry on the specified bucket.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1158,6 +1158,25 @@ public Boolean call() {
}
}

@Override
public boolean deleteLifecycleRules(final String bucket) {
final com.google.api.services.storage.model.Bucket bucketPb = BucketInfo.of(bucket).toPb();
try {
return runWithRetries(
new Callable<Boolean>() {
@Override
public Boolean call() {
return storageRpc.deleteLifecycleRules(bucketPb);
}
},
getOptions().getRetrySettings(),
EXCEPTION_HANDLER,
getOptions().getClock());
} catch (RetryHelperException e) {
throw StorageException.translateAndThrow(e);
}
}

@Override
public boolean deleteAcl(final String bucket, final Entity entity) {
return deleteAcl(bucket, entity, new BucketSourceOption[0]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.google.api.client.googleapis.json.GoogleJsonError;
import com.google.api.client.http.ByteArrayContent;
import com.google.api.client.http.GenericUrl;
import com.google.api.client.http.HttpContent;
import com.google.api.client.http.HttpHeaders;
import com.google.api.client.http.HttpRequest;
import com.google.api.client.http.HttpRequestFactory;
Expand Down Expand Up @@ -87,7 +88,8 @@ public class HttpStorageRpc implements StorageRpc {
public static final String NO_ACL_PROJECTION = "noAcl";
private static final String ENCRYPTION_KEY_PREFIX = "x-goog-encryption-";
private static final String SOURCE_ENCRYPTION_KEY_PREFIX = "x-goog-copy-source-encryption-";

private static final String GOOGLE_STORAGE_API_ENDPOINT =
"https://storage.googleapis.com/storage/v1/b/";
// declare this HttpStatus code here as it's not included in java.net.HttpURLConnection
private static final int SC_REQUESTED_RANGE_NOT_SATISFIABLE = 416;

Expand Down Expand Up @@ -1000,6 +1002,32 @@ public boolean deleteAcl(String bucket, String entity, Map<Option, ?> options) {
}
}

@Override
public boolean deleteLifecycleRules(Bucket bucket) {
Span span = startSpan(HttpStorageRpcSpans.SPAN_NAME_DELETE_BUCKET_LIFECYCLE_RULE);
Scope scope = tracer.withSpan(span);
try {
HttpRequestFactory requestFactory = storage.getRequestFactory();
HttpContent httpContent = new JsonHttpContent(new JacksonFactory(), "");
HttpRequest httpRequest =
requestFactory.buildPutRequest(
new GenericUrl(GOOGLE_STORAGE_API_ENDPOINT + bucket.getName() + "?fields=lifecycle"),
httpContent);
httpRequest.execute();
return true;
} catch (IOException ex) {
span.setStatus(Status.UNKNOWN.withDescription(ex.getMessage()));
StorageException serviceException = translate(ex);
if (serviceException.getCode() == HTTP_NOT_FOUND) {
return false;
}
throw serviceException;
} finally {
scope.close();
span.end();
}
}

@Override
public BucketAccessControl createAcl(BucketAccessControl acl, Map<Option, ?> options) {
Span span = startSpan(HttpStorageRpcSpans.SPAN_NAME_CREATE_BUCKET_ACL);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ class HttpStorageRpcSpans {
static final String SPAN_NAME_PATCH_BUCKET = getTraceSpanName("patch(Bucket,Map)");
static final String SPAN_NAME_PATCH_OBJECT = getTraceSpanName("patch(StorageObject,Map)");
static final String SPAN_NAME_DELETE_BUCKET = getTraceSpanName("delete(Bucket,Map)");
static final String SPAN_NAME_DELETE_BUCKET_LIFECYCLE_RULE =
getTraceSpanName("deleteLifecycleRules(String)");
static final String SPAN_NAME_DELETE_OBJECT = getTraceSpanName("delete(StorageObject,Map)");
static final String SPAN_NAME_CREATE_BATCH = getTraceSpanName("createBatch()");
static final String SPAN_NAME_COMPOSE = getTraceSpanName("compose(Iterable,StorageObject,Map)");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,14 @@ public int hashCode() {
*/
boolean delete(Bucket bucket, Map<Option, ?> options);

/**
* Delete the lifecycle rules of the requested bucket.
*
* @return {@code true} if the bucket lifecycle rules were deleted.
* @throws StorageException upon failure
*/
boolean deleteLifecycleRules(Bucket bucket);

/**
* Deletes the requested storage object.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -703,6 +703,15 @@ public void testDeleteDefaultAcl() throws Exception {
assertTrue(bucket.deleteDefaultAcl(User.ofAllAuthenticatedUsers()));
}

@Test
public void testDeleteLifecycleRules() {
expect(storage.getOptions()).andReturn(mockOptions).times(1);
expect(storage.deleteLifecycleRules(BUCKET_INFO.getName())).andReturn(true);
replay(storage);
initializeBucket();
assertTrue(bucket.deleteLifecycleRules());
}

@Test
public void testCreateDefaultAcl() throws Exception {
initializeExpectedBucket(4);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2737,6 +2737,17 @@ public void testDeleteDefaultBucketAcl() {
assertTrue(storage.deleteDefaultAcl(BUCKET_NAME1, User.ofAllAuthenticatedUsers()));
}

@Test
public void testDeleteLifecyclesRulesOfBucket() {
EasyMock.expect(
storageRpcMock.deleteLifecycleRules(
EasyMock.isA(com.google.api.services.storage.model.Bucket.class)))
.andReturn(true);
EasyMock.replay(storageRpcMock);
initializeService();
assertTrue(storage.deleteLifecycleRules(BUCKET_NAME1));
}

@Test
public void testCreateDefaultBucketAcl() {
Acl returnedAcl = ACL.toBuilder().setEtag("ETAG").setId("ID").build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
import com.google.cloud.storage.Storage.BlobField;
import com.google.cloud.storage.Storage.BlobWriteOption;
import com.google.cloud.storage.Storage.BucketField;
import com.google.cloud.storage.Storage.BucketGetOption;
import com.google.cloud.storage.StorageBatch;
import com.google.cloud.storage.StorageBatchResult;
import com.google.cloud.storage.StorageClass;
Expand Down Expand Up @@ -3273,4 +3274,36 @@ public void testBlobReload() throws Exception {
updated.delete();
assertNull(updated.reload());
}

@Test
public void testDeleteLifecycleRules() throws ExecutionException, InterruptedException {
Copy link
Member

Choose a reason for hiding this comment

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

Following up, did you try deleting a single lifecycle rule?

Copy link
Contributor Author

@athakor athakor Apr 22, 2020

Choose a reason for hiding this comment

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

@frankyn

I have tried to find the feasible way to delete a single lifecycle rule but its look like there is no possible way to do that and also found that across all the languages have same behavior which we have currently implemented.

Suggestion

  • I think we should have to update the method name like clearLifecycleRules or disableLifecycleRules instead of deleteLifecycleRules to be more clear, deleteLifecycleRules might confused to user.

  • we can also place a Note there like delete individual rules through the console because currently this library have limited support to delete rule.

Copy link
Member

Choose a reason for hiding this comment

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

Hi @athakor, apologies for the delay.

    String lifecycleTestBucketName = RemoteStorageHelper.generateBucketName();
    storage.create(
            BucketInfo.newBuilder(lifecycleTestBucketName)
                    .setLocation("us")
                    .setLifecycleRules(
                            ImmutableList.of(
                                    new LifecycleRule(
                                            LifecycleAction.newSetStorageClassAction(StorageClass.COLDLINE),
                                            LifecycleCondition.newBuilder()
                                                    .setAge(1)
                                                    .setNumberOfNewerVersions(3)
                                                    .setIsLive(false)
                                                    .setCreatedBefore(new DateTime(System.currentTimeMillis()))
                                                    .setMatchesStorageClass(ImmutableList.of(StorageClass.COLDLINE))
                                                    .build()),
                                    new LifecycleRule(
                                            LifecycleAction.newDeleteAction(),
                                            LifecycleCondition.newBuilder()
                                                    .setAge(1)
                                                    .build()
                                    )))
                    .build());
    // Delete OLM rule.
    Bucket remoteBucket =
            storage.get(lifecycleTestBucketName, Storage.BucketGetOption.fields(BucketField.LIFECYCLE));
    int priorSize = remoteBucket.getLifecycleRules().size();
    ArrayList<LifecycleRule> lifecycleRules = new ArrayList(remoteBucket.getLifecycleRules());
    Iterator<LifecycleRule> iterator = lifecycleRules.iterator();
    while(iterator.hasNext()) {
      LifecycleRule rule = iterator.next();
      if (rule.getAction().getActionType().equals(LifecycleRule.DeleteLifecycleAction.TYPE)) {
        iterator.remove();
      }
    }
    remoteBucket.toBuilder().setLifecycleRules(lifecycleRules).build().update();

Copy link
Member

Choose a reason for hiding this comment

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

Did this help? I think we can merge this change once you add a helper to Storage interface but no change is required for StorageRpc.java.

Copy link
Contributor Author

@athakor athakor May 15, 2020

Choose a reason for hiding this comment

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

@frankyn thanks for this, I tried your sample code but its not working please check the below response.

[
  LifecycleRule{
    lifecycleAction=DeleteLifecycleAction{
      actionType=Delete
    },
    lifecycleCondition=LifecycleCondition{
      age=1,
      createBefore=null,
      numberofNewerVersions=null,
      isLive=null,
      matchesStorageClass=null
    }
  },
  LifecycleRule{
    lifecycleAction=SetStorageClassLifecycleAction{
      actionType=SetStorageClass,
      storageClass=COLDLINE
    },
    lifecycleCondition=LifecycleCondition{
      age=1,
      createBefore=2020-05-15,
      numberofNewerVersions=3,
      isLive=false,
      matchesStorageClass=[
        COLDLINE
      ]
    }
  }
]

does it works on your end? i think bucket lifecycle rule's not updated properly.

Copy link
Member

Choose a reason for hiding this comment

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

storage.delete(bucketName) is clean up for the example and not required by remove lifecycle rule.

Please let me know if there is still confusion with the workaround. In short, the library should not require the workaround.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@frankyn thanks for the clarification. It's works i will raise separate PR by adding these helper to Storage interface.

Thank you for your help.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you! I appreciate your patience.

Copy link
Member

@frankyn frankyn Jun 2, 2020

Choose a reason for hiding this comment

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

@athakor I might be confused, you're going to close this PR and make another right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, will close this once newly created PR gets Approved

String lifeCycleRuleBucket = RemoteStorageHelper.generateBucketName();
List<LifecycleRule> lifecycleRules =
ImmutableList.of(
new LifecycleRule(
LifecycleAction.newDeleteAction(),
LifecycleCondition.newBuilder().setAge(2).setIsLive(Boolean.TRUE).build()));
try {
Bucket bucket =
storage.create(
BucketInfo.newBuilder(lifeCycleRuleBucket)
.setStorageClass(StorageClass.COLDLINE)
.setLocation("us-central1")
.setLifecycleRules(lifecycleRules)
.build());
assertEquals(lifeCycleRuleBucket, bucket.getName());
assertEquals(StorageClass.COLDLINE, bucket.getStorageClass());
assertEquals(lifecycleRules, bucket.getLifecycleRules());
assertNotNull(bucket.getLifecycleRules());
boolean rulesDeleted = bucket.deleteLifecycleRules();
assertTrue(rulesDeleted);
bucket =
storage.get(lifeCycleRuleBucket, BucketGetOption.fields(Storage.BucketField.values()));
assertNull(bucket.getLifecycleRules());
} catch (StorageException ex) {
throw ex;
} finally {
RemoteStorageHelper.forceDelete(storage, lifeCycleRuleBucket, 5, TimeUnit.SECONDS);
}
}
}