From 6eef9b4a23b367e64400135a7983c8756e7da21a Mon Sep 17 00:00:00 2001 From: Krishnan Parthasarathi Date: Thu, 10 Nov 2022 07:17:45 -0800 Subject: [PATCH] lifecycle: simplify Eval and HasActiveRules (#16036) --- cmd/bucket-lifecycle.go | 2 +- cmd/data-scanner.go | 4 +- cmd/xl-storage.go | 2 +- internal/bucket/lifecycle/lifecycle.go | 43 ++++------- internal/bucket/lifecycle/lifecycle_test.go | 81 ++++++++++----------- 5 files changed, 58 insertions(+), 74 deletions(-) diff --git a/cmd/bucket-lifecycle.go b/cmd/bucket-lifecycle.go index e28c15d33..e1a4a372b 100644 --- a/cmd/bucket-lifecycle.go +++ b/cmd/bucket-lifecycle.go @@ -298,7 +298,7 @@ func validateTransitionTier(lc *lifecycle.Lifecycle) error { // This is to be called after a successful upload of an object (version). func enqueueTransitionImmediate(obj ObjectInfo) { if lc, err := globalLifecycleSys.Get(obj.Bucket); err == nil { - event := lc.Eval(obj.ToLifecycleOpts(), time.Now()) + event := lc.Eval(obj.ToLifecycleOpts()) switch event.Action { case lifecycle.TransitionAction, lifecycle.TransitionVersionAction: globalTransitionState.queueTransitionTask(obj, event.StorageClass) diff --git a/cmd/data-scanner.go b/cmd/data-scanner.go index c963608e0..a04aa6134 100644 --- a/cmd/data-scanner.go +++ b/cmd/data-scanner.go @@ -420,7 +420,7 @@ func (f *folderScanner) scanFolder(ctx context.Context, folder cachedFolder, int filter := f.withFilter _, prefix := path2BucketObjectWithBasePath(f.root, folder.name) var activeLifeCycle *lifecycle.Lifecycle - if f.oldCache.Info.lifeCycle != nil && f.oldCache.Info.lifeCycle.HasActiveRules(prefix, true) { + if f.oldCache.Info.lifeCycle != nil && f.oldCache.Info.lifeCycle.HasActiveRules(prefix) { if f.dataUsageScannerDebug { console.Debugf(scannerLogPrefix+" Prefix %q has active rules\n", prefix) } @@ -1101,7 +1101,7 @@ func (i *scannerItem) applyActions(ctx context.Context, o ObjectLayer, oi Object } func evalActionFromLifecycle(ctx context.Context, lc lifecycle.Lifecycle, lr lock.Retention, obj ObjectInfo) lifecycle.Event { - event := lc.Eval(obj.ToLifecycleOpts(), time.Now().UTC()) + event := lc.Eval(obj.ToLifecycleOpts()) if serverDebugLog { console.Debugf(applyActionsLogPrefix+" lifecycle: Secondary scan: %v\n", event.Action) } diff --git a/cmd/xl-storage.go b/cmd/xl-storage.go index 0b052410c..648ecdb9d 100644 --- a/cmd/xl-storage.go +++ b/cmd/xl-storage.go @@ -445,7 +445,7 @@ func (s *xlStorage) NSScanner(ctx context.Context, cache dataUsageCache, updates // Check if the current bucket has a configured lifecycle policy if globalLifecycleSys != nil { lc, err = globalLifecycleSys.Get(cache.Info.Name) - if err == nil && lc.HasActiveRules("", true) { + if err == nil && lc.HasActiveRules("") { cache.Info.lifeCycle = lc if intDataUpdateTracker.debug { console.Debugln(color.Green("scannerDisk:") + " lifecycle: Active rules found") diff --git a/internal/bucket/lifecycle/lifecycle.go b/internal/bucket/lifecycle/lifecycle.go index ef6d7b364..ab72e0709 100644 --- a/internal/bucket/lifecycle/lifecycle.go +++ b/internal/bucket/lifecycle/lifecycle.go @@ -121,11 +121,8 @@ func (lc *Lifecycle) UnmarshalXML(d *xml.Decoder, start xml.StartElement) (err e return nil } -// HasActiveRules - returns whether policy has active rules for. -// Optionally a prefix can be supplied. -// If recursive is specified the function will also return true if any level below the -// prefix has active rules. If no prefix is specified recursive is effectively true. -func (lc Lifecycle) HasActiveRules(prefix string, recursive bool) bool { +// HasActiveRules - returns whether lc has active rules at any level below or at prefix. +func (lc Lifecycle) HasActiveRules(prefix string) bool { if len(lc.Rules) == 0 { return false } @@ -135,17 +132,9 @@ func (lc Lifecycle) HasActiveRules(prefix string, recursive bool) bool { } if len(prefix) > 0 && len(rule.GetPrefix()) > 0 { - if !recursive { - // If not recursive, incoming prefix must be in rule prefix - if !strings.HasPrefix(prefix, rule.GetPrefix()) { - continue - } - } - if recursive { - // If recursive, we can skip this rule if it doesn't match the tested prefix. - if !strings.HasPrefix(prefix, rule.GetPrefix()) && !strings.HasPrefix(rule.GetPrefix(), prefix) { - continue - } + // If recursive, we can skip this rule if it doesn't match the tested prefix. + if !strings.HasPrefix(prefix, rule.GetPrefix()) && !strings.HasPrefix(rule.GetPrefix(), prefix) { + continue } } @@ -316,8 +305,14 @@ func (es lifecycleEvents) Less(i, j int) bool { return es[i].Due.Before(es[j].Due) } -// Eval returns the lifecycle event applicable at now. If now is the zero value of time.Time, it returns the upcoming lifecycle event. -func (lc Lifecycle) Eval(obj ObjectOpts, now time.Time) Event { +// Eval returns the lifecycle event applicable now. +func (lc Lifecycle) Eval(obj ObjectOpts) Event { + return lc.eval(obj, time.Now().UTC()) +} + +// eval returns the lifecycle event applicable at the given now. If now is the +// zero value of time.Time, it returns the upcoming lifecycle event. +func (lc Lifecycle) eval(obj ObjectOpts, now time.Time) Event { var events []Event if obj.ModTime.IsZero() { return Event{} @@ -371,8 +366,8 @@ func (lc Lifecycle) Eval(obj ObjectOpts, now time.Time) Event { } // Skip rules with newer noncurrent versions specified. These rules are - // not handled at an individual version level. ComputeAction applies - // only to a specific version. + // not handled at an individual version level. eval applies only to a + // specific version. if !obj.IsLatest && rule.NoncurrentVersionExpiration.NewerNoncurrentVersions > 0 { continue } @@ -448,12 +443,6 @@ func (lc Lifecycle) Eval(obj ObjectOpts, now time.Time) Event { } } -// ComputeAction returns the action to perform by evaluating all lifecycle rules -// against the object name and its modification time. -func (lc Lifecycle) ComputeAction(obj ObjectOpts) Action { - return lc.Eval(obj, time.Now().UTC()).Action -} - // ExpectedExpiryTime calculates the expiry, transition or restore date/time based on a object modtime. // The expected transition or restore time is always a midnight time following the object // modification time plus the number of transition/restore days. @@ -471,7 +460,7 @@ func ExpectedExpiryTime(modTime time.Time, days int) time.Time { // SetPredictionHeaders sets time to expiry and transition headers on w for a // given obj. func (lc Lifecycle) SetPredictionHeaders(w http.ResponseWriter, obj ObjectOpts) { - event := lc.Eval(obj, time.Time{}) + event := lc.eval(obj, time.Time{}) switch event.Action { case DeleteAction, DeleteVersionAction: w.Header()[xhttp.AmzExpiration] = []string{ diff --git a/internal/bucket/lifecycle/lifecycle_test.go b/internal/bucket/lifecycle/lifecycle_test.go index 67ef558a5..58c0fb48d 100644 --- a/internal/bucket/lifecycle/lifecycle_test.go +++ b/internal/bucket/lifecycle/lifecycle_test.go @@ -221,7 +221,7 @@ func TestExpectedExpiryTime(t *testing.T) { } } -func TestComputeActions(t *testing.T) { +func TestEval(t *testing.T) { testCases := []struct { inputConfig string objectName string @@ -538,7 +538,7 @@ func TestComputeActions(t *testing.T) { if err != nil { t.Fatalf("Got unexpected error: %v", err) } - if resultAction := lc.ComputeAction(ObjectOpts{ + if res := lc.Eval(ObjectOpts{ Name: tc.objectName, UserTags: tc.objectTags, ModTime: tc.objectModTime, @@ -547,8 +547,8 @@ func TestComputeActions(t *testing.T) { IsLatest: !tc.isNoncurrent, SuccessorModTime: tc.objectSuccessorModTime, VersionID: tc.versionID, - }); resultAction != tc.expectedAction { - t.Fatalf("Expected action: `%v`, got: `%v`", tc.expectedAction, resultAction) + }); res.Action != tc.expectedAction { + t.Fatalf("Expected action: `%v`, got: `%v`", tc.expectedAction, res.Action) } }) } @@ -556,50 +556,49 @@ func TestComputeActions(t *testing.T) { func TestHasActiveRules(t *testing.T) { testCases := []struct { - inputConfig string - prefix string - expectedNonRec bool - expectedRec bool + inputConfig string + prefix string + want bool }{ { - inputConfig: `foodir/Enabled5`, - prefix: "foodir/foobject", - expectedNonRec: true, expectedRec: true, + inputConfig: `foodir/Enabled5`, + prefix: "foodir/foobject", + want: true, }, { // empty prefix - inputConfig: `Enabled5`, - prefix: "foodir/foobject/foo.txt", - expectedNonRec: true, expectedRec: true, + inputConfig: `Enabled5`, + prefix: "foodir/foobject/foo.txt", + want: true, }, { - inputConfig: `foodir/Enabled5`, - prefix: "zdir/foobject", - expectedNonRec: false, expectedRec: false, + inputConfig: `foodir/Enabled5`, + prefix: "zdir/foobject", + want: false, }, { - inputConfig: `foodir/zdir/Enabled5`, - prefix: "foodir/", - expectedNonRec: false, expectedRec: true, + inputConfig: `foodir/zdir/Enabled5`, + prefix: "foodir/", + want: true, }, { - inputConfig: `Disabled5`, - prefix: "foodir/", - expectedNonRec: false, expectedRec: false, + inputConfig: `Disabled5`, + prefix: "foodir/", + want: false, }, { - inputConfig: `foodir/Enabled2999-01-01T00:00:00.000Z`, - prefix: "foodir/foobject", - expectedNonRec: false, expectedRec: false, + inputConfig: `foodir/Enabled2999-01-01T00:00:00.000Z`, + prefix: "foodir/foobject", + want: false, }, { - inputConfig: `EnabledS3TIER-1`, - prefix: "foodir/foobject/foo.txt", - expectedNonRec: true, expectedRec: true, + inputConfig: `EnabledS3TIER-1`, + prefix: "foodir/foobject/foo.txt", + want: true, }, { - inputConfig: `EnabledS3TIER-1`, - prefix: "foodir/foobject/foo.txt", - expectedNonRec: true, expectedRec: true, + inputConfig: `EnabledS3TIER-1`, + prefix: "foodir/foobject/foo.txt", + want: true, }, } @@ -610,14 +609,10 @@ func TestHasActiveRules(t *testing.T) { if err != nil { t.Fatalf("Got unexpected error: %v", err) } - if got := lc.HasActiveRules(tc.prefix, false); got != tc.expectedNonRec { - t.Fatalf("Expected result with recursive set to false: `%v`, got: `%v`", tc.expectedNonRec, got) - } - if got := lc.HasActiveRules(tc.prefix, true); got != tc.expectedRec { - t.Fatalf("Expected result with recursive set to true: `%v`, got: `%v`", tc.expectedRec, got) + if got := lc.HasActiveRules(tc.prefix); got != tc.want { + t.Fatalf("Expected result with recursive set to false: `%v`, got: `%v`", tc.want, got) } }) - } } @@ -741,7 +736,7 @@ func TestTransitionTier(t *testing.T) { // Go back seven days in the past now = now.Add(7 * 24 * time.Hour) - evt := lc.Eval(obj1, now) + evt := lc.eval(obj1, now) if evt.Action != TransitionAction { t.Fatalf("Expected action: %s but got %s", TransitionAction, evt.Action) } @@ -749,7 +744,7 @@ func TestTransitionTier(t *testing.T) { t.Fatalf("Expected TIER-1 but got %s", evt.StorageClass) } - evt = lc.Eval(obj2, now) + evt = lc.eval(obj2, now) if evt.Action != TransitionVersionAction { t.Fatalf("Expected action: %s but got %s", TransitionVersionAction, evt.Action) } @@ -818,13 +813,13 @@ func TestTransitionTierWithPrefixAndTags(t *testing.T) { now = now.Add(7 * 24 * time.Hour) // Eval object 1 - evt := lc.Eval(obj1, now) + evt := lc.eval(obj1, now) if evt.Action != NoneAction { t.Fatalf("Expected action: %s but got %s", NoneAction, evt.Action) } // Eval object 2 - evt = lc.Eval(obj2, now) + evt = lc.eval(obj2, now) if evt.Action != TransitionAction { t.Fatalf("Expected action: %s but got %s", TransitionAction, evt.Action) } @@ -833,7 +828,7 @@ func TestTransitionTierWithPrefixAndTags(t *testing.T) { } // Eval object 3 - evt = lc.Eval(obj3, now) + evt = lc.eval(obj3, now) if evt.Action != TransitionAction { t.Fatalf("Expected action: %s but got %s", TransitionAction, evt.Action) }