From 20efa465ef030f9608192aceaec46fadb69edc52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20DONNART?= Date: Tue, 29 Sep 2026 16:02:01 +0200 Subject: [PATCH 1/2] Honor transitionOneDayEarlier in lifecycle transitions The v1 eligibility pre-filter and the noncurrent version transition apply compared raw rule days, ignoring transitionOneDayEarlier, while arsenal's getApplicableRules honors it. Eligible objects were skipped by the v1 pre-filter, and noncurrent versions were not transitioned early, also on the v2 task which reuses the same apply check. Compute transition times with LifecycleDateTime, as getApplicableRules does. Issue: BB-867 --- extensions/lifecycle/tasks/LifecycleTask.js | 44 +++++++------ tests/unit/lifecycle/LifecycleTask.spec.js | 69 +++++++++++++++++++++ 2 files changed, 90 insertions(+), 23 deletions(-) diff --git a/extensions/lifecycle/tasks/LifecycleTask.js b/extensions/lifecycle/tasks/LifecycleTask.js index ee050633d..11ff203f9 100644 --- a/extensions/lifecycle/tasks/LifecycleTask.js +++ b/extensions/lifecycle/tasks/LifecycleTask.js @@ -813,18 +813,18 @@ class LifecycleTask extends BackbeatTask { /** * check if rule applies for a given date or calculed days. * @param {array} rule - bucket lifecycle rule - * @param {number} daysSinceInitiated - Days passed since entity (object or version) last modified + * @param {string} lastModified - entity (object or version) last modified date * NOTE: entity is not an in-progress MPU or a delete marker. - * @param {number} currentDate - current date * @return {boolean} true if rule applies - false otherwise. */ - _isRuleApplying(rule, daysSinceInitiated, currentDate) { + _isRuleApplying(rule, lastModified) { if (rule.Expiration && this._supportedRules.includes('Expiration')) { - if (rule.Expiration.Days !== undefined && daysSinceInitiated >= rule.Expiration.Days) { + if (rule.Expiration.Days !== undefined && + this._lifecycleDateTime.findDaysSince(new Date(lastModified)) >= rule.Expiration.Days) { return true; } - if (rule.Expiration.Date && rule.Expiration.Date < currentDate) { + if (rule.Expiration.Date && rule.Expiration.Date < this._lifecycleDateTime.getCurrentDate()) { return true; } // Expiration.ExpiredObjectDeleteMarker rule's action does not apply @@ -835,14 +835,11 @@ class LifecycleTask extends BackbeatTask { if (rule.Transitions && rule.Transitions.length > 0 && this._supportedRules.includes('Transition')) { + // Same computation as the apply stage, so that + // transitionOneDayEarlier is honored. return rule.Transitions.some(t => { - if (t.Days !== undefined && daysSinceInitiated >= t.Days) { - return true; - } - if (t.Date && t.Date < currentDate) { - return true; - } - return false; + const transitionTime = this._lifecycleDateTime.getTransitionTimestamp(t, lastModified); + return transitionTime !== null && transitionTime <= this._lifecycleDateTime.getCurrentDate(); }); } @@ -861,9 +858,6 @@ class LifecycleTask extends BackbeatTask { */ _isEntityEligible(rules, entity, versioningStatus) { const currentDate = this._lifecycleDateTime.getCurrentDate(); - const daysSinceInitiated = this._lifecycleDateTime.findDaysSince( - new Date(entity.LastModified) - ); const { staleDate } = entity; const daysSinceStaled = staleDate ? this._lifecycleDateTime.findDaysSince(new Date(staleDate)) : null; @@ -881,7 +875,7 @@ class LifecycleTask extends BackbeatTask { if (versioningStatus === 'Enabled' || versioningStatus === 'Suspended') { if (entity.IsLatest) { - return this._isRuleApplying(rule, daysSinceInitiated, currentDate); + return this._isRuleApplying(rule, entity.LastModified); } if (!staleDate) { @@ -900,14 +894,17 @@ class LifecycleTask extends BackbeatTask { if (rule.NoncurrentVersionTransitions && rule.NoncurrentVersionTransitions.length > 0 && this._supportedRules.includes('NoncurrentVersionTransition')) { - return rule.NoncurrentVersionTransitions.some(t => - (t.NoncurrentDays !== undefined && daysSinceInitiated >= t.NoncurrentDays)); + return rule.NoncurrentVersionTransitions.some(t => { + const transitionTime = this._lifecycleDateTime + .getNCVTransitionTimestamp(t, staleDate); + return transitionTime !== undefined && transitionTime <= currentDate; + }); } return false; } - return this._isRuleApplying(rule, daysSinceInitiated, currentDate); + return this._isRuleApplying(rule, entity.LastModified); }); } @@ -1356,12 +1353,13 @@ class LifecycleTask extends BackbeatTask { */ _checkAndApplyNCVTransitionRule(bucketData, version, rules, log, cb) { const staleDate = version.staleDate; - const daysSinceInitiated = this._lifecycleDateTime.findDaysSince(new Date(staleDate)); const ncvt = 'NoncurrentVersionTransition'; const ncd = 'NoncurrentDays'; - const doesNCVTransitionRuleApply = (rules[ncvt] && - rules[ncvt][ncd] !== undefined && - daysSinceInitiated >= rules[ncvt][ncd]); + const ncvTransitionTime = rules[ncvt] && rules[ncvt][ncd] !== undefined ? + this._lifecycleDateTime.getNCVTransitionTimestamp(rules[ncvt], staleDate) : + undefined; + const doesNCVTransitionRuleApply = ncvTransitionTime !== undefined && + ncvTransitionTime <= this._lifecycleDateTime.getCurrentDate(); if (doesNCVTransitionRuleApply) { this._applyTransitionRule({ diff --git a/tests/unit/lifecycle/LifecycleTask.spec.js b/tests/unit/lifecycle/LifecycleTask.spec.js index 8ae9f536d..45b0f290b 100644 --- a/tests/unit/lifecycle/LifecycleTask.spec.js +++ b/tests/unit/lifecycle/LifecycleTask.spec.js @@ -1488,6 +1488,75 @@ describe('lifecycle task helper methods', () => { }); }); + describe('transitions with transitionOneDayEarlier', () => { + const bucketData = { target: { owner: 'o', accountId: 'a', bucket: 'b' } }; + const transitionRules = [{ + ID: 'id1', + Prefix: '', + Status: 'Enabled', + Transitions: [{ Days: 1, StorageClass: 'cold' }], + NoncurrentVersionTransitions: [], + }]; + const ncvTransitionRules = [{ + ID: 'id1', + Prefix: '', + Status: 'Enabled', + Transitions: [], + NoncurrentVersionTransitions: [{ NoncurrentDays: 1, StorageClass: 'cold' }], + }]; + const applicableNCVRules = { + NoncurrentVersionTransition: { NoncurrentDays: 1, StorageClass: 'cold' }, + }; + + const makeTask = flags => new LifecycleTask({ + getStateVars: () => ({ + ncvHeap: new Map(), + lcOptions: { ...timeOptions, ...flags }, + log: fakeLogger, + supportedRules: ValidLifecycleRules, + }), + }); + + const isNCVTransitionApplied = (task, version) => { + const applyStub = sinon.stub(task, '_applyTransitionRule').callsFake((params, log, cb) => cb()); + task._checkAndApplyNCVTransitionRule(bucketData, version, applicableNCVRules, fakeLogger, () => {}); + return applyStub.calledOnce; + }; + + [ + { flags: {}, expected: false }, + { flags: { transitionOneDayEarlier: true }, expected: true }, + ].forEach(({ flags, expected }) => { + const desc = JSON.stringify(flags); + + it(`should ${expected ? '' : 'not '}find 1 day transition eligible on 1 hour old object with ${desc}`, + () => { + const object = { ...OBJECT, LastModified: new Date(Date.now() - HOUR).toISOString() }; + assert.strictEqual(makeTask(flags)._isEntityEligible(transitionRules, object, 'Disabled'), expected); + }); + + it(`should ${expected ? '' : 'not '}find 1 day ncv transition eligible on 1 hour old version with ${desc}`, + () => { + const version = { + ...NON_CURRENT_VERSION, + LastModified: new Date(Date.now() - 2 * DAY).toISOString(), + staleDate: new Date(Date.now() - HOUR).toISOString(), + }; + assert.strictEqual(makeTask(flags)._isEntityEligible(ncvTransitionRules, version, 'Enabled'), expected); + }); + + it(`should ${expected ? '' : 'not '}apply 1 day ncv transition on 1 hour stale version with ${desc}`, + () => { + const version = { + ...NON_CURRENT_VERSION, + LastModified: new Date(Date.now() - 2 * DAY).toISOString(), + staleDate: new Date(Date.now() - HOUR).toISOString(), + }; + assert.strictEqual(isNCVTransitionApplied(makeTask(flags), version), expected); + }); + }); + }); + describe('_checkAndApplyNCVExpirationRule', () => { let lct2; From 3a9910c274e382170e232df0d7d3171183308bde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20DONNART?= Date: Wed, 9 Sep 2026 08:54:58 +0200 Subject: [PATCH 2/2] Keep lifecycle transition deadlines on the real clock Transition times were compared with getCurrentDate(), which is shifted by expireOneDayEarlier, so that flag moved transitions one day earlier too: noncurrent version transitions on both v1 and v2 tasks, and the v1 eligibility pre-filter. With both flags set, the shifts stacked. Compare transition times with the real clock, as arsenal's getApplicableRules does. Issue: BB-867 --- extensions/lifecycle/tasks/LifecycleTask.js | 10 ++++------ tests/unit/lifecycle/LifecycleTask.spec.js | 3 ++- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/extensions/lifecycle/tasks/LifecycleTask.js b/extensions/lifecycle/tasks/LifecycleTask.js index 11ff203f9..7d25a2117 100644 --- a/extensions/lifecycle/tasks/LifecycleTask.js +++ b/extensions/lifecycle/tasks/LifecycleTask.js @@ -835,11 +835,10 @@ class LifecycleTask extends BackbeatTask { if (rule.Transitions && rule.Transitions.length > 0 && this._supportedRules.includes('Transition')) { - // Same computation as the apply stage, so that - // transitionOneDayEarlier is honored. + // getCurrentDate() is shifted by expireOneDayEarlier: transitions use the real clock. return rule.Transitions.some(t => { const transitionTime = this._lifecycleDateTime.getTransitionTimestamp(t, lastModified); - return transitionTime !== null && transitionTime <= this._lifecycleDateTime.getCurrentDate(); + return transitionTime !== null && transitionTime <= Date.now(); }); } @@ -857,7 +856,6 @@ class LifecycleTask extends BackbeatTask { * @return {boolean} true if eligible - false otherwise. */ _isEntityEligible(rules, entity, versioningStatus) { - const currentDate = this._lifecycleDateTime.getCurrentDate(); const { staleDate } = entity; const daysSinceStaled = staleDate ? this._lifecycleDateTime.findDaysSince(new Date(staleDate)) : null; @@ -897,7 +895,7 @@ class LifecycleTask extends BackbeatTask { return rule.NoncurrentVersionTransitions.some(t => { const transitionTime = this._lifecycleDateTime .getNCVTransitionTimestamp(t, staleDate); - return transitionTime !== undefined && transitionTime <= currentDate; + return transitionTime !== undefined && transitionTime <= Date.now(); }); } @@ -1359,7 +1357,7 @@ class LifecycleTask extends BackbeatTask { this._lifecycleDateTime.getNCVTransitionTimestamp(rules[ncvt], staleDate) : undefined; const doesNCVTransitionRuleApply = ncvTransitionTime !== undefined && - ncvTransitionTime <= this._lifecycleDateTime.getCurrentDate(); + ncvTransitionTime <= Date.now(); if (doesNCVTransitionRuleApply) { this._applyTransitionRule({ diff --git a/tests/unit/lifecycle/LifecycleTask.spec.js b/tests/unit/lifecycle/LifecycleTask.spec.js index 45b0f290b..b51818c45 100644 --- a/tests/unit/lifecycle/LifecycleTask.spec.js +++ b/tests/unit/lifecycle/LifecycleTask.spec.js @@ -1488,7 +1488,7 @@ describe('lifecycle task helper methods', () => { }); }); - describe('transitions with transitionOneDayEarlier', () => { + describe('transitions with one day earlier flags', () => { const bucketData = { target: { owner: 'o', accountId: 'a', bucket: 'b' } }; const transitionRules = [{ ID: 'id1', @@ -1526,6 +1526,7 @@ describe('lifecycle task helper methods', () => { [ { flags: {}, expected: false }, { flags: { transitionOneDayEarlier: true }, expected: true }, + { flags: { expireOneDayEarlier: true }, expected: false }, ].forEach(({ flags, expected }) => { const desc = JSON.stringify(flags);