Skip to content

Commit c85704f

Browse files
frankynfrankyn
authored
fix(storage): allow to use age=0 in OLM conditions (#6204)
Storage Go doesn't allow setting AgeInDays=0 after discussing with @tritone. This PR is the agreed upon path to support Age=0 in OLM conditions. It also notes that other int fields in OLM Conditions do not support 0 values. Fixes: #6539, #6240
1 parent 1937ba4 commit c85704f

File tree

3 files changed

+68
-61
lines changed

3 files changed

+68
-61
lines changed

‎storage/bucket.go‎

Lines changed: 33 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -610,7 +610,12 @@ const (
610610
//
611611
// All configured conditions must be met for the associated action to be taken.
612612
type LifecycleCondition struct {
613+
// AllObjects is used to select all objects in a bucket by
614+
// setting AgeInDays to 0.
615+
AllObjects bool
616+
613617
// AgeInDays is the age of the object in days.
618+
// If you want to set AgeInDays to `0` use AllObjects set to `true`.
614619
AgeInDays int64
615620

616621
// CreatedBefore is the time the object was created.
@@ -628,10 +633,12 @@ type LifecycleCondition struct {
628633

629634
// DaysSinceCustomTime is the days elapsed since the CustomTime date of the
630635
// object. This condition can only be satisfied if CustomTime has been set.
636+
// Note: Using `0` as the value will be ignored by the library and not sent to the API.
631637
DaysSinceCustomTime int64
632638

633639
// DaysSinceNoncurrentTime is the days elapsed since the noncurrent timestamp
634640
// of the object. This condition is relevant only for versioned objects.
641+
// Note: Using `0` as the value will be ignored by the library and not sent to the API.
635642
DaysSinceNoncurrentTime int64
636643

637644
// Liveness specifies the object's liveness. Relevant only for versioned objects
@@ -663,6 +670,7 @@ type LifecycleCondition struct {
663670
// If the value is N, this condition is satisfied when there are at least N
664671
// versions (including the live version) newer than this version of the
665672
// object.
673+
// Note: Using `0` as the value will be ignored by the library and not sent to the API.
666674
NumNewerVersions int64
667675
}
668676

@@ -1421,19 +1429,6 @@ func toCORSFromProto(rc []*storagepb.Bucket_Cors) []CORS {
14211429
return out
14221430
}
14231431

1424-
// Used to handle breaking change in Autogen Storage client OLM Age field
1425-
// from int64 to *int64 gracefully in the manual client
1426-
// TODO(#6240): Method should be removed once breaking change is made and introduced to this client
1427-
func setAgeCondition(age int64, ageField interface{}) {
1428-
c := reflect.ValueOf(ageField).Elem()
1429-
switch c.Kind() {
1430-
case reflect.Int64:
1431-
c.SetInt(age)
1432-
case reflect.Ptr:
1433-
c.Set(reflect.ValueOf(&age))
1434-
}
1435-
}
1436-
14371432
func toRawLifecycle(l Lifecycle) *raw.BucketLifecycle {
14381433
var rl raw.BucketLifecycle
14391434
if len(l.Rules) == 0 {
@@ -1455,7 +1450,15 @@ func toRawLifecycle(l Lifecycle) *raw.BucketLifecycle {
14551450
},
14561451
}
14571452

1458-
setAgeCondition(r.Condition.AgeInDays, &rr.Condition.Age)
1453+
// AllObjects takes precedent when both AllObjects and AgeInDays are set
1454+
// Rationale: If you've opted into using AllObjects, it makes sense that you
1455+
// understand the implications of how this option works with AgeInDays.
1456+
if r.Condition.AllObjects {
1457+
rr.Condition.Age = googleapi.Int64(0)
1458+
rr.Condition.ForceSendFields = []string{"Age"}
1459+
} else if r.Condition.AgeInDays > 0 {
1460+
rr.Condition.Age = googleapi.Int64(r.Condition.AgeInDays)
1461+
}
14591462

14601463
switch r.Condition.Liveness {
14611464
case LiveAndArchived:
@@ -1504,6 +1507,11 @@ func toProtoLifecycle(l Lifecycle) *storagepb.Bucket_Lifecycle {
15041507
},
15051508
}
15061509

1510+
// TODO(#6205): This may not be needed for gRPC
1511+
if r.Condition.AllObjects {
1512+
rr.Condition.AgeDays = proto.Int32(0)
1513+
}
1514+
15071515
switch r.Condition.Liveness {
15081516
case LiveAndArchived:
15091517
rr.Condition.IsLive = nil
@@ -1527,21 +1535,6 @@ func toProtoLifecycle(l Lifecycle) *storagepb.Bucket_Lifecycle {
15271535
return &rl
15281536
}
15291537

1530-
// Used to handle breaking change in Autogen Storage client OLM Age field
1531-
// from int64 to *int64 gracefully in the manual client
1532-
// TODO(#6240): Method should be removed once breaking change is made and introduced to this client
1533-
func getAgeCondition(ageField interface{}) int64 {
1534-
v := reflect.ValueOf(ageField)
1535-
if v.Kind() == reflect.Int64 {
1536-
return v.Interface().(int64)
1537-
} else if v.Kind() == reflect.Ptr {
1538-
if val, ok := v.Interface().(*int64); ok {
1539-
return *val
1540-
}
1541-
}
1542-
return 0
1543-
}
1544-
15451538
func toLifecycle(rl *raw.BucketLifecycle) Lifecycle {
15461539
var l Lifecycle
15471540
if rl == nil {
@@ -1562,7 +1555,12 @@ func toLifecycle(rl *raw.BucketLifecycle) Lifecycle {
15621555
NumNewerVersions: rr.Condition.NumNewerVersions,
15631556
},
15641557
}
1565-
r.Condition.AgeInDays = getAgeCondition(rr.Condition.Age)
1558+
if rr.Condition.Age != nil {
1559+
r.Condition.AgeInDays = *rr.Condition.Age
1560+
if *rr.Condition.Age == 0 {
1561+
r.Condition.AllObjects = true
1562+
}
1563+
}
15661564

15671565
if rr.Condition.IsLive == nil {
15681566
r.Condition.Liveness = LiveAndArchived
@@ -1608,6 +1606,11 @@ func toLifecycleFromProto(rl *storagepb.Bucket_Lifecycle) Lifecycle {
16081606
},
16091607
}
16101608

1609+
// TODO(#6205): This may not be needed for gRPC
1610+
if rr.GetCondition().GetAgeDays() == 0 {
1611+
r.Condition.AllObjects = true
1612+
}
1613+
16111614
if rr.GetCondition().IsLive == nil {
16121615
r.Condition.Liveness = LiveAndArchived
16131616
} else if rr.GetCondition().GetIsLive() {

‎storage/bucket_test.go‎

Lines changed: 24 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,7 @@ import (
2525
raw "google.golang.org/api/storage/v1"
2626
)
2727

28-
// TODO(#6539): re-enable tests after breaking change is released and AgeInDays is a int64*
2928
func TestBucketAttrsToRawBucket(t *testing.T) {
30-
t.Skip("TestBucketAttrsToRawBucket skipped: https://github.com/googleapis/google-cloud-go/issues/6539")
3129
t.Parallel()
3230
attrs := &BucketAttrs{
3331
Name: "name",
@@ -119,6 +117,13 @@ func TestBucketAttrsToRawBucket(t *testing.T) {
119117
Condition: LifecycleCondition{
120118
AgeInDays: 20,
121119
},
120+
}, {
121+
Action: LifecycleAction{
122+
Type: DeleteAction,
123+
},
124+
Condition: LifecycleCondition{
125+
AllObjects: true,
126+
},
122127
}},
123128
},
124129
}
@@ -163,7 +168,7 @@ func TestBucketAttrsToRawBucket(t *testing.T) {
163168
StorageClass: "NEARLINE",
164169
},
165170
Condition: &raw.BucketLifecycleRuleCondition{
166-
//Age: 10,
171+
Age: googleapi.Int64(10),
167172
IsLive: googleapi.Bool(true),
168173
CreatedBefore: "2017-01-02",
169174
MatchesStorageClass: []string{"STANDARD"},
@@ -199,7 +204,7 @@ func TestBucketAttrsToRawBucket(t *testing.T) {
199204
Type: DeleteAction,
200205
},
201206
Condition: &raw.BucketLifecycleRuleCondition{
202-
//Age: 10,
207+
Age: googleapi.Int64(10),
203208
MatchesPrefix: []string{"testPrefix"},
204209
MatchesSuffix: []string{"testSuffix"},
205210
NumNewerVersions: 3,
@@ -218,7 +223,16 @@ func TestBucketAttrsToRawBucket(t *testing.T) {
218223
Type: AbortIncompleteMPUAction,
219224
},
220225
Condition: &raw.BucketLifecycleRuleCondition{
221-
//Age: 20,
226+
Age: googleapi.Int64(20),
227+
},
228+
},
229+
{
230+
Action: &raw.BucketLifecycleRuleAction{
231+
Type: DeleteAction,
232+
},
233+
Condition: &raw.BucketLifecycleRuleCondition{
234+
Age: googleapi.Int64(0),
235+
ForceSendFields: []string{"Age"},
222236
},
223237
},
224238
},
@@ -350,9 +364,7 @@ func TestBucketAttrsToRawBucket(t *testing.T) {
350364
}
351365
}
352366

353-
// TODO(#6539): re-enable tests after breaking change is released and AgeInDays is a int64*
354367
func TestBucketAttrsToUpdateToRawBucket(t *testing.T) {
355-
t.Skip("TestBucketAttrsToUpdateToRawBucket skipped: https://github.com/googleapis/google-cloud-go/issues/6539")
356368
t.Parallel()
357369
au := &BucketAttrsToUpdate{
358370
VersioningEnabled: false,
@@ -408,12 +420,12 @@ func TestBucketAttrsToUpdateToRawBucket(t *testing.T) {
408420
Lifecycle: &raw.BucketLifecycle{
409421
Rule: []*raw.BucketLifecycleRule{
410422
{
411-
Action: &raw.BucketLifecycleRuleAction{Type: "Delete"},
412-
//Condition: &raw.BucketLifecycleRuleCondition{Age: 30},
423+
Action: &raw.BucketLifecycleRuleAction{Type: "Delete"},
424+
Condition: &raw.BucketLifecycleRuleCondition{Age: googleapi.Int64(30)},
413425
},
414426
{
415-
Action: &raw.BucketLifecycleRuleAction{Type: AbortIncompleteMPUAction},
416-
//Condition: &raw.BucketLifecycleRuleCondition{Age: 13},
427+
Action: &raw.BucketLifecycleRuleAction{Type: AbortIncompleteMPUAction},
428+
Condition: &raw.BucketLifecycleRuleCondition{Age: googleapi.Int64(13)},
417429
},
418430
},
419431
},
@@ -564,26 +576,7 @@ func TestBucketAttrsToUpdateToRawBucket(t *testing.T) {
564576
}
565577
}
566578

567-
func TestAgeConditionBackwardCompat(t *testing.T) {
568-
var ti int64
569-
var want int64 = 100
570-
setAgeCondition(want, &ti)
571-
if getAgeCondition(ti) != want {
572-
t.Fatalf("got %v, want %v", getAgeCondition(ti), want)
573-
}
574-
575-
var tp *int64
576-
want = 10
577-
setAgeCondition(want, &tp)
578-
if getAgeCondition(tp) != want {
579-
t.Fatalf("got %v, want %v", getAgeCondition(tp), want)
580-
}
581-
582-
}
583-
584-
// TODO(#6539): re-enable tests after breaking change is released and AgeInDays is a int64*
585579
func TestNewBucket(t *testing.T) {
586-
t.Skip("TestNewBucket skipped: https://github.com/googleapis/google-cloud-go/issues/6539")
587580
labels := map[string]string{"a": "b"}
588581
matchClasses := []string{"STANDARD"}
589582
aTime := time.Date(2017, 1, 2, 0, 0, 0, 0, time.UTC)
@@ -605,7 +598,7 @@ func TestNewBucket(t *testing.T) {
605598
StorageClass: "NEARLINE",
606599
},
607600
Condition: &raw.BucketLifecycleRuleCondition{
608-
// Age: 10,
601+
Age: googleapi.Int64(10),
609602
IsLive: googleapi.Bool(true),
610603
CreatedBefore: "2017-01-02",
611604
MatchesStorageClass: matchClasses,

‎storage/integration_test.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,13 @@ func TestIntegration_BucketCreateDelete(t *testing.T) {
309309
MatchesSuffix: []string{"testSuffix"},
310310
NumNewerVersions: 3,
311311
},
312+
}, {
313+
Action: LifecycleAction{
314+
Type: DeleteAction,
315+
},
316+
Condition: LifecycleCondition{
317+
AllObjects: true,
318+
},
312319
}},
313320
}
314321

@@ -442,6 +449,10 @@ func TestIntegration_BucketLifecycle(t *testing.T) {
442449
Action: LifecycleAction{Type: AbortIncompleteMPUAction},
443450
Condition: LifecycleCondition{AgeInDays: 30},
444451
},
452+
{
453+
Action: LifecycleAction{Type: DeleteAction},
454+
Condition: LifecycleCondition{AllObjects: true},
455+
},
445456
},
446457
}
447458

0 commit comments

Comments
 (0)