feat(storage): add retry config to ACL handle methods (#5185)

storage/acl.go

@@ -73,6 +73,7 @@ type ACLHandle struct {
 	object      string
 	isDefault   bool
 	userProject string // for requester-pays buckets
+	retry       *retryConfig
 }
 
 // Delete permanently deletes the ACL entry for the given entity.
@@ -120,12 +121,12 @@ func (a *ACLHandle) List(ctx context.Context) (rules []ACLRule, err error) {
 func (a *ACLHandle) bucketDefaultList(ctx context.Context) ([]ACLRule, error) {
 	var acls *raw.ObjectAccessControls
 	var err error
-	err = runWithRetry(ctx, func() error {
+	err = run(ctx, func() error {
 		req := a.c.raw.DefaultObjectAccessControls.List(a.bucket)
 		a.configureCall(ctx, req)
 		acls, err = req.Do()
 		return err
-	})
+	}, a.retry, true)
 	if err != nil {
 		return nil, err
 	}
@@ -135,18 +136,21 @@ func (a *ACLHandle) bucketDefaultList(ctx context.Context) ([]ACLRule, error) {
 func (a *ACLHandle) bucketDefaultDelete(ctx context.Context, entity ACLEntity) error {
 	req := a.c.raw.DefaultObjectAccessControls.Delete(a.bucket, string(entity))
 	a.configureCall(ctx, req)
-	return req.Do()
+
+	return run(ctx, func() error {
+		return req.Do()
+	}, a.retry, false)
 }
 
 func (a *ACLHandle) bucketList(ctx context.Context) ([]ACLRule, error) {
 	var acls *raw.BucketAccessControls
 	var err error
-	err = runWithRetry(ctx, func() error {
+	err = run(ctx, func() error {
 		req := a.c.raw.BucketAccessControls.List(a.bucket)
 		a.configureCall(ctx, req)
 		acls, err = req.Do()
 		return err
-	})
+	}, a.retry, true)
 	if err != nil {
 		return nil, err
 	}
@@ -161,25 +165,29 @@ func (a *ACLHandle) bucketSet(ctx context.Context, entity ACLEntity, role ACLRol
 	}
 	req := a.c.raw.BucketAccessControls.Update(a.bucket, string(entity), acl)
 	a.configureCall(ctx, req)
-	_, err := req.Do()
-	return err
+	return run(ctx, func() error {
+		_, err := req.Do()
+		return err
+	}, a.retry, false)
 }
 
 func (a *ACLHandle) bucketDelete(ctx context.Context, entity ACLEntity) error {
 	req := a.c.raw.BucketAccessControls.Delete(a.bucket, string(entity))
 	a.configureCall(ctx, req)
-	return req.Do()
+	return run(ctx, func() error {
+		return req.Do()
+	}, a.retry, false)
 }
 
 func (a *ACLHandle) objectList(ctx context.Context) ([]ACLRule, error) {
 	var acls *raw.ObjectAccessControls
 	var err error
-	err = runWithRetry(ctx, func() error {
+	err = run(ctx, func() error {
 		req := a.c.raw.ObjectAccessControls.List(a.bucket, a.object)
 		a.configureCall(ctx, req)
 		acls, err = req.Do()
 		return err
-	})
+	}, a.retry, true)
 	if err != nil {
 		return nil, err
 	}
@@ -204,14 +212,18 @@ func (a *ACLHandle) objectSet(ctx context.Context, entity ACLEntity, role ACLRol
 		req = a.c.raw.ObjectAccessControls.Update(a.bucket, a.object, string(entity), acl)
 	}
 	a.configureCall(ctx, req)
-	_, err := req.Do()
-	return err
+	return run(ctx, func() error {
+		_, err := req.Do()
+		return err
+	}, a.retry, false)
 }
 
 func (a *ACLHandle) objectDelete(ctx context.Context, entity ACLEntity) error {
 	req := a.c.raw.ObjectAccessControls.Delete(a.bucket, a.object, string(entity))
 	a.configureCall(ctx, req)
-	return req.Do()
+	return run(ctx, func() error {
+		return req.Do()
+	}, a.retry, false)
 }
 
 func (a *ACLHandle) configureCall(ctx context.Context, call interface{ Header() http.Header }) {

storage/bucket.go

@@ -146,6 +146,7 @@ func (b *BucketHandle) DefaultObjectACL() *ACLHandle {
 // for valid object names can be found at:
 //   https://cloud.google.com/storage/docs/naming-objects
 func (b *BucketHandle) Object(name string) *ObjectHandle {
+	retry := b.retry.clone()
 	return &ObjectHandle{
 		c:      b.c,
 		bucket: b.name,
@@ -155,10 +156,11 @@ func (b *BucketHandle) Object(name string) *ObjectHandle {
 			bucket:      b.name,
 			object:      name,
 			userProject: b.userProject,
+			retry:       retry,
 		},
 		gen:         -1,
 		userProject: b.userProject,
-		retry:       b.retry.clone(),
+		retry:       retry,
 	}
 }
 
@@ -1436,6 +1438,8 @@ func (b *BucketHandle) Retryer(opts ...RetryOption) *BucketHandle {
 		opt.apply(retry)
 	}
 	b2.retry = retry
+	b2.acl.retry = retry
+	b2.defaultObjectACL.retry = retry
 	return &b2
 }
 

storage/storage.go

@@ -1806,6 +1806,7 @@ func (o *ObjectHandle) Retryer(opts ...RetryOption) *ObjectHandle {
 		opt.apply(retry)
 	}
 	o2.retry = retry
+	o2.acl.retry = retry
 	return &o2
 }
 

storage/storage_test.go

@@ -876,7 +876,7 @@ func TestObjectRetryer(t *testing.T) {
 }
 
 // Test the interactions between ObjectHandle and BucketHandle Retryers and that
-// they correctly configure the retry configuration
+// they correctly configure the retry configuration for objects and ACLs
 func TestRetryer(t *testing.T) {
 	testCases := []struct {
 		name          string
@@ -1007,17 +1007,47 @@ func TestRetryer(t *testing.T) {
 				o = o.Retryer(tc.objectOptions...)
 			}
 
-			if diff := cmp.Diff(
-				o.retry,
-				tc.want,
-				cmp.AllowUnexported(retryConfig{}, gax.Backoff{}),
-				// ErrorFunc cannot be compared directly, but we check if both are
-				// either nil or non-nil.
-				cmp.Comparer(func(a, b func(err error) bool) bool {
-					return (a == nil && b == nil) || (a != nil && b != nil)
-				}),
-			); diff != "" {
-				s.Fatalf("retry not configured correctly: %v", diff)
+			configHandleCases := []struct {
+				r    *retryConfig
+				name string
+				want *retryConfig
+			}{
+				{
+					name: "object.retry",
+					r:    o.retry,
+					want: tc.want,
+				},
+				{
+					name: "object.ACL()",
+					r:    o.ACL().retry,
+					want: tc.want,
+				},
+				{
+					name: "bucket.ACL()",
+					r:    b.ACL().retry,
+					want: b.retry,
+				},
+				{
+					name: "bucket.DefaultObjectACL()",
+					r:    b.DefaultObjectACL().retry,
+					want: b.retry,
+				},
+			}
+			for _, ac := range configHandleCases {
+				s.Run(ac.name, func(ss *testing.T) {
+					if diff := cmp.Diff(
+						ac.want,
+						ac.r,
+						cmp.AllowUnexported(retryConfig{}, gax.Backoff{}),
+						// ErrorFunc cannot be compared directly, but we check if both are
+						// either nil or non-nil.
+						cmp.Comparer(func(a, b func(err error) bool) bool {
+							return (a == nil && b == nil) || (a != nil && b != nil)
+						}),
+					); diff != "" {
+						ss.Fatalf("retry not configured correctly: %v", diff)
+					}
+				})
 			}
 		})
 	}