fix: fix logic for buckets that do not have a generation (#1634)

* fix: fix logic for buckets that do not have a generation * add test for autoRetry remaining true
googleapis · Oct 1, 2021 · 9069bdc · 9069bdc
1 parent b841d5b
commit 9069bdc
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 9 deletions.
diff --git a/src/bucket.ts b/src/bucket.ts
@@ -4075,15 +4075,8 @@ class Bucket extends ServiceObject {
     if (
       typeof coreOpts === 'object' &&
       coreOpts?.reqOpts?.qs?.ifMetagenerationMatch === undefined &&
-      methodType === AvailableServiceObjectMethods.setMetadata &&
-      this.storage.retryOptions.idempotencyStrategy ===
-        IdempotencyStrategy.RetryConditional
-    ) {
-      this.storage.retryOptions.autoRetry = false;
-    } else if (
-      typeof coreOpts === 'object' &&
-      coreOpts?.reqOpts?.qs?.ifGenerationMatch === undefined &&
-      methodType === AvailableServiceObjectMethods.delete &&
+      (methodType === AvailableServiceObjectMethods.setMetadata ||
+        methodType === AvailableServiceObjectMethods.delete) &&
       this.storage.retryOptions.idempotencyStrategy ===
         IdempotencyStrategy.RetryConditional
     ) {

diff --git a/test/bucket.ts b/test/bucket.ts
@@ -47,6 +47,7 @@ import {
   SetBucketMetadataCallback,
   SetBucketMetadataResponse,
   GetBucketSignedUrlConfig,
+  AvailableServiceObjectMethods,
 } from '../src/bucket';
 import {AddAclOptions} from '../src/acl';
 import {Policy} from '../src/iam';
@@ -3237,4 +3238,58 @@ describe('Bucket', () => {
       );
     });
   });
+  describe('disableAutoRetryConditionallyIdempotent_', () => {
+    beforeEach(() => {
+      bucket.storage.retryOptions.autoRetry = true;
+      STORAGE.retryOptions.idempotencyStrategy =
+        IdempotencyStrategy.RetryConditional;
+    });
+
+    it('should set autoRetry to false when ifMetagenerationMatch is undefined (setMetadata)', done => {
+      bucket.disableAutoRetryConditionallyIdempotent_(
+        bucket.methods.setMetadata,
+        AvailableServiceObjectMethods.setMetadata
+      );
+      assert.strictEqual(bucket.storage.retryOptions.autoRetry, false);
+      done();
+    });
+
+    it('should set autoRetry to false when ifMetagenerationMatch is undefined (delete)', done => {
+      bucket.disableAutoRetryConditionallyIdempotent_(
+        bucket.methods.delete,
+        AvailableServiceObjectMethods.delete
+      );
+      assert.strictEqual(bucket.storage.retryOptions.autoRetry, false);
+      done();
+    });
+
+    it('should set autoRetry to false when IdempotencyStrategy is set to RetryNever', done => {
+      STORAGE.retryOptions.idempotencyStrategy = IdempotencyStrategy.RetryNever;
+      bucket = new Bucket(STORAGE, BUCKET_NAME, {
+        preconditionOpts: {
+          ifMetagenerationMatch: 100,
+        },
+      });
+      bucket.disableAutoRetryConditionallyIdempotent_(
+        bucket.methods.delete,
+        AvailableServiceObjectMethods.delete
+      );
+      assert.strictEqual(bucket.storage.retryOptions.autoRetry, false);
+      done();
+    });
+
+    it('autoRetry should remain true when ifMetagenerationMatch is not undefined', done => {
+      bucket = new Bucket(STORAGE, BUCKET_NAME, {
+        preconditionOpts: {
+          ifMetagenerationMatch: 100,
+        },
+      });
+      bucket.disableAutoRetryConditionallyIdempotent_(
+        bucket.methods.delete,
+        AvailableServiceObjectMethods.delete
+      );
+      assert.strictEqual(bucket.storage.retryOptions.autoRetry, true);
+      done();
+    });
+  });
 });