diff --git a/src/lib/db/feature-toggle-store.ts b/src/lib/db/feature-toggle-store.ts index 2da29c7d75..2ec492a4b8 100644 --- a/src/lib/db/feature-toggle-store.ts +++ b/src/lib/db/feature-toggle-store.ts @@ -263,25 +263,7 @@ export default class FeatureToggleStore implements IFeatureToggleStore { .update(this.dtoToRow(project, data)) .returning(FEATURE_COLUMNS); - const feature = this.rowToFeature(row[0]); - // if a feature toggle's type or createdAt has changed, update its potentially stale status - if (!feature.stale && (data.type || data.createdAt)) { - await this.db(TABLE) - .where({ name: data.name }) - .update( - 'potentially_stale', - this.db.raw( - `(? > (features.created_at + (( - SELECT feature_types.lifetime_days - FROM feature_types - WHERE feature_types.id = features.type - ) * INTERVAL '1 day')))`, - this.db.fn.now(), - ), - ); - } - - return feature; + return this.rowToFeature(row[0]); } async archive(name: string): Promise { @@ -397,29 +379,50 @@ export default class FeatureToggleStore implements IFeatureToggleStore { return toggle; } - async markPotentiallyStaleFeatures( + async updatePotentiallyStaleFeatures( currentTime?: string, - ): Promise { - const query = this.db(TABLE) - .update('potentially_stale', true) - .whereRaw( - `? > (features.created_at + (( - SELECT feature_types.lifetime_days - FROM feature_types - WHERE feature_types.id = features.type - ) * INTERVAL '1 day'))`, - [currentTime || this.db.fn.now()], - ) - .andWhere(function () { - this.where('potentially_stale', null).orWhere( - 'potentially_stale', - false, - ); - }) - .andWhereNot('stale', true); + ): Promise<{ name: string; potentiallyStale: boolean }[]> { + const query = this.db.raw( + `SELECT name, potentially_stale, (? > (features.created_at + (( + SELECT feature_types.lifetime_days + FROM feature_types + WHERE feature_types.id = features.type + ) * INTERVAL '1 day'))) as current_staleness + FROM features + WHERE NOT stale = true`, + [currentTime || this.db.fn.now()], + ); - const updatedFeatures = await query.returning(FEATURE_COLUMNS); - return updatedFeatures.map(({ name }) => name); + const featuresToUpdate = (await query).rows + .filter( + ({ potentially_stale, current_staleness }) => + (potentially_stale ?? false) !== + (current_staleness ?? false), + ) + .map(({ current_staleness, name }) => ({ + potentiallyStale: current_staleness ?? false, + name, + })); + + await this.db(TABLE) + .update('potentially_stale', true) + .whereIn( + 'name', + featuresToUpdate + .filter((feature) => feature.potentiallyStale === true) + .map((feature) => feature.name), + ); + + await this.db(TABLE) + .update('potentially_stale', false) + .whereIn( + 'name', + featuresToUpdate + .filter((feature) => feature.potentiallyStale !== true) + .map((feature) => feature.name), + ); + + return featuresToUpdate; } async isPotentiallyStale(featureName: string): Promise { diff --git a/src/lib/services/feature-toggle-service.ts b/src/lib/services/feature-toggle-service.ts index 57eb4891a1..f369397d18 100644 --- a/src/lib/services/feature-toggle-service.ts +++ b/src/lib/services/feature-toggle-service.ts @@ -1983,8 +1983,8 @@ class FeatureToggleService { } } - async markPotentiallyStaleFeatures(): Promise { - await this.featureToggleStore.markPotentiallyStaleFeatures(); + async updatePotentiallyStaleFeatures(): Promise { + await this.featureToggleStore.updatePotentiallyStaleFeatures(); } } diff --git a/src/lib/services/index.ts b/src/lib/services/index.ts index d85842d050..6800a6c7f3 100644 --- a/src/lib/services/index.ts +++ b/src/lib/services/index.ts @@ -128,7 +128,7 @@ export const scheduleServices = async ( ); schedulerService.schedule( - featureToggleService.markPotentiallyStaleFeatures.bind( + featureToggleService.updatePotentiallyStaleFeatures.bind( featureToggleService, ), minutesToMilliseconds(1), diff --git a/src/lib/types/events.ts b/src/lib/types/events.ts index f8749511c9..d0bb6452a2 100644 --- a/src/lib/types/events.ts +++ b/src/lib/types/events.ts @@ -122,6 +122,9 @@ export const SERVICE_ACCOUNT_CREATED = 'service-account-created' as const; export const SERVICE_ACCOUNT_UPDATED = 'service-account-updated' as const; export const SERVICE_ACCOUNT_DELETED = 'service-account-deleted' as const; +export const FEATURE_POTENTIALLY_STALE_UPDATED = + 'feature-potentially-stale-updated' as const; + export const IEventTypes = [ APPLICATION_CREATED, FEATURE_CREATED, @@ -223,6 +226,7 @@ export const IEventTypes = [ SERVICE_ACCOUNT_CREATED, SERVICE_ACCOUNT_DELETED, SERVICE_ACCOUNT_UPDATED, + FEATURE_POTENTIALLY_STALE_UPDATED, ] as const; export type IEventType = typeof IEventTypes[number]; diff --git a/src/lib/types/stores/feature-toggle-store.ts b/src/lib/types/stores/feature-toggle-store.ts index ac0f0f8316..76c029bbfc 100644 --- a/src/lib/types/stores/feature-toggle-store.ts +++ b/src/lib/types/stores/feature-toggle-store.ts @@ -32,7 +32,9 @@ export interface IFeatureToggleStore extends Store { range?: string[]; dateAccessor: string; }): Promise; - markPotentiallyStaleFeatures(currentTime?: string): Promise; + updatePotentiallyStaleFeatures( + currentTime?: string, + ): Promise<{ name: string; potentiallyStale: boolean }[]>; isPotentiallyStale(featureName: string): Promise; /** diff --git a/src/test/e2e/stores/feature-toggle-store.e2e.test.ts b/src/test/e2e/stores/feature-toggle-store.e2e.test.ts index 0b927ea3ff..f25b30f17b 100644 --- a/src/test/e2e/stores/feature-toggle-store.e2e.test.ts +++ b/src/test/e2e/stores/feature-toggle-store.e2e.test.ts @@ -42,7 +42,7 @@ describe('potentially_stale marking', () => { ).toISOString(); }; - test('it returns an empty list of no toggles were updated', async () => { + test('it returns an empty list if no toggles were updated', async () => { const features: FeatureToggleDTO[] = [ { name: 'feature1', @@ -56,11 +56,12 @@ describe('potentially_stale marking', () => { ); const markedToggles = - await featureToggleStore.markPotentiallyStaleFeatures(); + await featureToggleStore.updatePotentiallyStaleFeatures(); expect(markedToggles).toStrictEqual([]); }); - test('it returns the names of only the marked toggles', async () => { + + test('it returns only updated toggles', async () => { const features: FeatureToggleDTO[] = [ { name: 'feature1', @@ -78,11 +79,13 @@ describe('potentially_stale marking', () => { ); const markedToggles = - await featureToggleStore.markPotentiallyStaleFeatures( + await featureToggleStore.updatePotentiallyStaleFeatures( getFutureTimestamp(41), ); - expect(markedToggles).toStrictEqual(['feature1']); + expect(markedToggles).toStrictEqual([ + { name: 'feature1', potentiallyStale: true }, + ]); }); test.each([ @@ -111,12 +114,17 @@ describe('potentially_stale marking', () => { } const markedToggles = - await featureToggleStore.markPotentiallyStaleFeatures( + await featureToggleStore.updatePotentiallyStaleFeatures( getFutureTimestamp(daysElapsed), ); expect(markedToggles).toEqual( - expect.arrayContaining(expectedMarkedFeatures), + expect.arrayContaining( + expectedMarkedFeatures.map((name: string) => ({ + name, + potentiallyStale: true, + })), + ), ); expect(markedToggles.length).toEqual(expectedMarkedFeatures.length); @@ -141,11 +149,12 @@ describe('potentially_stale marking', () => { ), ); const markedToggles = - await featureToggleStore.markPotentiallyStaleFeatures( + await featureToggleStore.updatePotentiallyStaleFeatures( getFutureTimestamp(1000), ); expect(markedToggles).toStrictEqual([]); }); + test('it does not return toggles previously marked as potentially_stale', async () => { const features: FeatureToggleDTO[] = [ { @@ -159,28 +168,27 @@ describe('potentially_stale marking', () => { ), ); const markedToggles = - await featureToggleStore.markPotentiallyStaleFeatures( + await featureToggleStore.updatePotentiallyStaleFeatures( getFutureTimestamp(50), ); - expect(markedToggles).toStrictEqual(['feature1']); + + expect(markedToggles).toStrictEqual([ + { name: 'feature1', potentiallyStale: true }, + ]); + + expect( + await featureToggleStore.isPotentiallyStale('feature1'), + ).toBeTruthy(); const secondPass = - await featureToggleStore.markPotentiallyStaleFeatures( + await featureToggleStore.updatePotentiallyStaleFeatures( getFutureTimestamp(100), ); + expect(secondPass).toStrictEqual([]); }); describe('changing feature types', () => { - const getPastDate = (days: number) => { - return new Date( - new Date().getTime() - - days * 24 * 60 * 60 * 1000 - - // subtract an extra second - 1000, - ); - }; - test("if a potentially stale feature changes to a type that shouldn't be stale, it's 'potentially_stale' marker is removed.", async () => { const features: FeatureToggleDTO[] = [ { @@ -194,11 +202,14 @@ describe('potentially_stale marking', () => { ), ); const markedToggles = - await featureToggleStore.markPotentiallyStaleFeatures( + await featureToggleStore.updatePotentiallyStaleFeatures( getFutureTimestamp(50), ); - expect(markedToggles).toStrictEqual(['feature1']); + expect(markedToggles).toStrictEqual([ + { name: 'feature1', potentiallyStale: true }, + ]); + expect( await featureToggleStore.isPotentiallyStale('feature1'), ).toBeTruthy(); @@ -208,6 +219,10 @@ describe('potentially_stale marking', () => { type: 'kill-switch', }); + expect( + await featureToggleStore.updatePotentiallyStaleFeatures(), + ).toStrictEqual([{ name: 'feature1', potentiallyStale: false }]); + const potentiallyStale = await featureToggleStore.isPotentiallyStale('feature1'); @@ -227,7 +242,7 @@ describe('potentially_stale marking', () => { ), ); const markedToggles = - await featureToggleStore.markPotentiallyStaleFeatures( + await featureToggleStore.updatePotentiallyStaleFeatures( getFutureTimestamp(50), ); @@ -236,9 +251,11 @@ describe('potentially_stale marking', () => { await featureToggleStore.update('default', { name: 'feature1', type: 'release', - createdAt: getPastDate(40), }); + await featureToggleStore.updatePotentiallyStaleFeatures( + getFutureTimestamp(40), + ); const potentiallyStale = await featureToggleStore.isPotentiallyStale('feature1'); @@ -262,9 +279,12 @@ describe('potentially_stale marking', () => { await featureToggleStore.update('default', { name: 'feature1', type: 'release', - createdAt: getPastDate(40), }); + await featureToggleStore.updatePotentiallyStaleFeatures( + getFutureTimestamp(40), + ); + const potentiallyStale = await featureToggleStore.isPotentiallyStale('feature1'); diff --git a/src/test/fixtures/fake-feature-toggle-store.ts b/src/test/fixtures/fake-feature-toggle-store.ts index e35d9d768e..e214a237e1 100644 --- a/src/test/fixtures/fake-feature-toggle-store.ts +++ b/src/test/fixtures/fake-feature-toggle-store.ts @@ -253,7 +253,9 @@ export default class FakeFeatureToggleStore implements IFeatureToggleStore { return Promise.resolve(); } - markPotentiallyStaleFeatures(): Promise { + updatePotentiallyStaleFeatures(): Promise< + { name: string; potentiallyStale: boolean }[] + > { throw new Error('Method not implemented.'); }