From 8a9137cc54c0b188f0a42cbf2e0ffda2538f4ede Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Tue, 21 Jun 2022 14:29:11 -0700 Subject: [PATCH 1/7] fix: allow for pointer based types --- storage/bucket.go | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/storage/bucket.go b/storage/bucket.go index ee5e4b24f9f2..8d4388ffbe0c 100644 --- a/storage/bucket.go +++ b/storage/bucket.go @@ -1503,6 +1503,17 @@ func toCORSFromProto(rc []*storagepb.Bucket_Cors) []CORS { return out } +// Handle ptr or int64 +func setAgeCondition(age int64, cond *raw.BucketLifecycleRuleCondition) { + c := reflect.ValueOf(cond).Elem().FieldByName("Age") + switch c.Kind() { + case reflect.Int64: + c.SetInt(age) + case reflect.Ptr: + c.Set(reflect.ValueOf(&age)) + } +} + func toRawLifecycle(l Lifecycle) *raw.BucketLifecycle { var rl raw.BucketLifecycle if len(l.Rules) == 0 { @@ -1525,6 +1536,8 @@ func toRawLifecycle(l Lifecycle) *raw.BucketLifecycle { }, } + setAgeCondition(r.Condition.AgeInDays, rr.Condition) + switch r.Condition.Liveness { case LiveAndArchived: rr.Condition.IsLive = nil @@ -1595,6 +1608,19 @@ func toProtoLifecycle(l Lifecycle) *storagepb.Bucket_Lifecycle { return &rl } +// Handle ptr or int64 +func getAgeCondition(cond *raw.BucketLifecycleRuleCondition) int64 { + v := reflect.ValueOf(cond).Elem().FieldByName("Age") + if v.Kind() == reflect.Int64 { + return v.Interface().(int64) + } else if v.Kind() == reflect.Pointer { + if v.Interface() != nil { + return *(v.Interface().(*int64)) + } + } + return 0 +} + func toLifecycle(rl *raw.BucketLifecycle) Lifecycle { var l Lifecycle if rl == nil { @@ -1607,7 +1633,6 @@ func toLifecycle(rl *raw.BucketLifecycle) Lifecycle { StorageClass: rr.Action.StorageClass, }, Condition: LifecycleCondition{ - AgeInDays: rr.Condition.Age, DaysSinceCustomTime: rr.Condition.DaysSinceCustomTime, DaysSinceNoncurrentTime: rr.Condition.DaysSinceNoncurrentTime, MatchesPrefix: rr.Condition.MatchesPrefix, @@ -1616,6 +1641,7 @@ func toLifecycle(rl *raw.BucketLifecycle) Lifecycle { NumNewerVersions: rr.Condition.NumNewerVersions, }, } + r.Condition.AgeInDays = getAgeCondition(rr.Condition) if rr.Condition.IsLive == nil { r.Condition.Liveness = LiveAndArchived From 52def100f0ed7f41ee236ee75100f56c92f3a57e Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Tue, 21 Jun 2022 21:16:25 -0700 Subject: [PATCH 2/7] add tests --- storage/bucket.go | 4 ++-- storage/bucket_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/storage/bucket.go b/storage/bucket.go index 8d4388ffbe0c..7fb70e27b087 100644 --- a/storage/bucket.go +++ b/storage/bucket.go @@ -1504,7 +1504,7 @@ func toCORSFromProto(rc []*storagepb.Bucket_Cors) []CORS { } // Handle ptr or int64 -func setAgeCondition(age int64, cond *raw.BucketLifecycleRuleCondition) { +func setAgeCondition(age int64, cond interface{}) { c := reflect.ValueOf(cond).Elem().FieldByName("Age") switch c.Kind() { case reflect.Int64: @@ -1609,7 +1609,7 @@ func toProtoLifecycle(l Lifecycle) *storagepb.Bucket_Lifecycle { } // Handle ptr or int64 -func getAgeCondition(cond *raw.BucketLifecycleRuleCondition) int64 { +func getAgeCondition(cond interface{}) int64 { v := reflect.ValueOf(cond).Elem().FieldByName("Age") if v.Kind() == reflect.Int64 { return v.Interface().(int64) diff --git a/storage/bucket_test.go b/storage/bucket_test.go index 57051f4e1316..053a0362b33f 100644 --- a/storage/bucket_test.go +++ b/storage/bucket_test.go @@ -561,6 +561,30 @@ func TestBucketAttrsToUpdateToRawBucket(t *testing.T) { } } +func TestAgeConditionBackwardCompat(t *testing.T) { + type testPointer struct { + Age *int64 + } + + type testInt64 struct { + Age int64 + } + + var tp testPointer + var want int64 = 10 + setAgeCondition(want, &tp) + if getAgeCondition(&tp) != want { + t.Fatalf("got %v, want 10", *tp.Age) + } + + var ti testInt64 + want = 100 + setAgeCondition(100, &ti) + if getAgeCondition(&ti) != want { + t.Fatalf("got %v, want %v", ti.Age, want) + } +} + func TestCallBuilders(t *testing.T) { rc, err := raw.NewService(context.Background()) if err != nil { From 42c83071d133d310d28dceb2c4906ab9a72912c6 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Wed, 22 Jun 2022 09:40:12 -0700 Subject: [PATCH 3/7] Pointer -> Ptr and update test constant to use want --- storage/bucket.go | 2 +- storage/bucket_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/storage/bucket.go b/storage/bucket.go index 7fb70e27b087..ccfddcabd7fb 100644 --- a/storage/bucket.go +++ b/storage/bucket.go @@ -1613,7 +1613,7 @@ func getAgeCondition(cond interface{}) int64 { v := reflect.ValueOf(cond).Elem().FieldByName("Age") if v.Kind() == reflect.Int64 { return v.Interface().(int64) - } else if v.Kind() == reflect.Pointer { + } else if v.Kind() == reflect.Ptr { if v.Interface() != nil { return *(v.Interface().(*int64)) } diff --git a/storage/bucket_test.go b/storage/bucket_test.go index 053a0362b33f..93c546d1316c 100644 --- a/storage/bucket_test.go +++ b/storage/bucket_test.go @@ -579,7 +579,7 @@ func TestAgeConditionBackwardCompat(t *testing.T) { var ti testInt64 want = 100 - setAgeCondition(100, &ti) + setAgeCondition(want, &ti) if getAgeCondition(&ti) != want { t.Fatalf("got %v, want %v", ti.Age, want) } From 074312d42e000c76f2fd64630d585208256ffd37 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Wed, 22 Jun 2022 10:31:01 -0700 Subject: [PATCH 4/7] address feedback --- storage/bucket.go | 26 +++++++++++++++----------- storage/bucket_test.go | 27 ++++++++++----------------- 2 files changed, 25 insertions(+), 28 deletions(-) diff --git a/storage/bucket.go b/storage/bucket.go index ccfddcabd7fb..9947879f7e25 100644 --- a/storage/bucket.go +++ b/storage/bucket.go @@ -1503,14 +1503,16 @@ func toCORSFromProto(rc []*storagepb.Bucket_Cors) []CORS { return out } -// Handle ptr or int64 -func setAgeCondition(age int64, cond interface{}) { - c := reflect.ValueOf(cond).Elem().FieldByName("Age") +// Used to handle breaking change in Autogen Storage client OLM Age field +// from int64 to *int64 gracefully in the manual client +// Method should be removed once breaking change is made and introduced to this client +func setAgeCondition(age *int64, ageField interface{}) { + c := reflect.Indirect(reflect.ValueOf(ageField)) switch c.Kind() { case reflect.Int64: - c.SetInt(age) + c.SetInt(*age) case reflect.Ptr: - c.Set(reflect.ValueOf(&age)) + c.Set(reflect.ValueOf(age)) } } @@ -1536,7 +1538,7 @@ func toRawLifecycle(l Lifecycle) *raw.BucketLifecycle { }, } - setAgeCondition(r.Condition.AgeInDays, rr.Condition) + setAgeCondition(&r.Condition.AgeInDays, &rr.Condition.Age) switch r.Condition.Liveness { case LiveAndArchived: @@ -1608,13 +1610,15 @@ func toProtoLifecycle(l Lifecycle) *storagepb.Bucket_Lifecycle { return &rl } -// Handle ptr or int64 -func getAgeCondition(cond interface{}) int64 { - v := reflect.ValueOf(cond).Elem().FieldByName("Age") +// Used to handle breaking change in Autogen Storage client OLM Age field +// from int64 to *int64 gracefully in the manual client +// Method should be removed once breaking change is made and introduced to this client +func getAgeCondition(ageField interface{}) int64 { + v := reflect.ValueOf(ageField) if v.Kind() == reflect.Int64 { return v.Interface().(int64) } else if v.Kind() == reflect.Ptr { - if v.Interface() != nil { + if v.Interface().(*int64) != nil { return *(v.Interface().(*int64)) } } @@ -1641,7 +1645,7 @@ func toLifecycle(rl *raw.BucketLifecycle) Lifecycle { NumNewerVersions: rr.Condition.NumNewerVersions, }, } - r.Condition.AgeInDays = getAgeCondition(rr.Condition) + r.Condition.AgeInDays = getAgeCondition(rr.Condition.Age) if rr.Condition.IsLive == nil { r.Condition.Liveness = LiveAndArchived diff --git a/storage/bucket_test.go b/storage/bucket_test.go index 93c546d1316c..33661c344653 100644 --- a/storage/bucket_test.go +++ b/storage/bucket_test.go @@ -562,27 +562,20 @@ func TestBucketAttrsToUpdateToRawBucket(t *testing.T) { } func TestAgeConditionBackwardCompat(t *testing.T) { - type testPointer struct { - Age *int64 + var ti int64 + var want int64 = 100 + setAgeCondition(&want, &ti) + if getAgeCondition(ti) != want { + t.Fatalf("got %v, want %v", getAgeCondition(ti), want) } - type testInt64 struct { - Age int64 + var tp *int64 + want = 10 + setAgeCondition(&want, &tp) + if getAgeCondition(tp) != want { + t.Fatalf("got %v, want %v", getAgeCondition(tp), want) } - var tp testPointer - var want int64 = 10 - setAgeCondition(want, &tp) - if getAgeCondition(&tp) != want { - t.Fatalf("got %v, want 10", *tp.Age) - } - - var ti testInt64 - want = 100 - setAgeCondition(want, &ti) - if getAgeCondition(&ti) != want { - t.Fatalf("got %v, want %v", ti.Age, want) - } } func TestCallBuilders(t *testing.T) { From df7ace9e42a061646e393d88f8ec8e0262a9d059 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Wed, 22 Jun 2022 10:43:00 -0700 Subject: [PATCH 5/7] remove AgeInDays set by autogen client --- storage/bucket.go | 1 - 1 file changed, 1 deletion(-) diff --git a/storage/bucket.go b/storage/bucket.go index 9947879f7e25..f04c461a7869 100644 --- a/storage/bucket.go +++ b/storage/bucket.go @@ -1528,7 +1528,6 @@ func toRawLifecycle(l Lifecycle) *raw.BucketLifecycle { StorageClass: r.Action.StorageClass, }, Condition: &raw.BucketLifecycleRuleCondition{ - Age: r.Condition.AgeInDays, DaysSinceCustomTime: r.Condition.DaysSinceCustomTime, DaysSinceNoncurrentTime: r.Condition.DaysSinceNoncurrentTime, MatchesPrefix: r.Condition.MatchesPrefix, From a0b26ba3352356e00d74953b0ab0ccd1a6524f15 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Wed, 22 Jun 2022 11:47:19 -0700 Subject: [PATCH 6/7] address todo feedback --- storage/bucket.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/storage/bucket.go b/storage/bucket.go index f04c461a7869..ae00829e6506 100644 --- a/storage/bucket.go +++ b/storage/bucket.go @@ -1505,7 +1505,7 @@ func toCORSFromProto(rc []*storagepb.Bucket_Cors) []CORS { // Used to handle breaking change in Autogen Storage client OLM Age field // from int64 to *int64 gracefully in the manual client -// Method should be removed once breaking change is made and introduced to this client +// TODO(#6240): Method should be removed once breaking change is made and introduced to this client func setAgeCondition(age *int64, ageField interface{}) { c := reflect.Indirect(reflect.ValueOf(ageField)) switch c.Kind() { @@ -1611,7 +1611,7 @@ func toProtoLifecycle(l Lifecycle) *storagepb.Bucket_Lifecycle { // Used to handle breaking change in Autogen Storage client OLM Age field // from int64 to *int64 gracefully in the manual client -// Method should be removed once breaking change is made and introduced to this client +// TODO(#6240): Method should be removed once breaking change is made and introduced to this client func getAgeCondition(ageField interface{}) int64 { v := reflect.ValueOf(ageField) if v.Kind() == reflect.Int64 { From f49b7f4b4a2ea1f88f65fc08494a618a678dbdda Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Wed, 22 Jun 2022 12:21:05 -0700 Subject: [PATCH 7/7] address feedback --- storage/bucket.go | 14 +++++++------- storage/bucket_test.go | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/storage/bucket.go b/storage/bucket.go index ae00829e6506..2148e6896539 100644 --- a/storage/bucket.go +++ b/storage/bucket.go @@ -1506,13 +1506,13 @@ func toCORSFromProto(rc []*storagepb.Bucket_Cors) []CORS { // Used to handle breaking change in Autogen Storage client OLM Age field // from int64 to *int64 gracefully in the manual client // TODO(#6240): Method should be removed once breaking change is made and introduced to this client -func setAgeCondition(age *int64, ageField interface{}) { - c := reflect.Indirect(reflect.ValueOf(ageField)) +func setAgeCondition(age int64, ageField interface{}) { + c := reflect.ValueOf(ageField).Elem() switch c.Kind() { case reflect.Int64: - c.SetInt(*age) + c.SetInt(age) case reflect.Ptr: - c.Set(reflect.ValueOf(age)) + c.Set(reflect.ValueOf(&age)) } } @@ -1537,7 +1537,7 @@ func toRawLifecycle(l Lifecycle) *raw.BucketLifecycle { }, } - setAgeCondition(&r.Condition.AgeInDays, &rr.Condition.Age) + setAgeCondition(r.Condition.AgeInDays, &rr.Condition.Age) switch r.Condition.Liveness { case LiveAndArchived: @@ -1617,8 +1617,8 @@ func getAgeCondition(ageField interface{}) int64 { if v.Kind() == reflect.Int64 { return v.Interface().(int64) } else if v.Kind() == reflect.Ptr { - if v.Interface().(*int64) != nil { - return *(v.Interface().(*int64)) + if val, ok := v.Interface().(*int64); ok { + return *val } } return 0 diff --git a/storage/bucket_test.go b/storage/bucket_test.go index 33661c344653..1eaa2f5ecf6b 100644 --- a/storage/bucket_test.go +++ b/storage/bucket_test.go @@ -564,14 +564,14 @@ func TestBucketAttrsToUpdateToRawBucket(t *testing.T) { func TestAgeConditionBackwardCompat(t *testing.T) { var ti int64 var want int64 = 100 - setAgeCondition(&want, &ti) + setAgeCondition(want, &ti) if getAgeCondition(ti) != want { t.Fatalf("got %v, want %v", getAgeCondition(ti), want) } var tp *int64 want = 10 - setAgeCondition(&want, &tp) + setAgeCondition(want, &tp) if getAgeCondition(tp) != want { t.Fatalf("got %v, want %v", getAgeCondition(tp), want) }