diff --git a/src/lib/db/feature-toggle-store.ts b/src/lib/db/feature-toggle-store.ts index d43e62ed86..2da29c7d75 100644 --- a/src/lib/db/feature-toggle-store.ts +++ b/src/lib/db/feature-toggle-store.ts @@ -262,7 +262,26 @@ export default class FeatureToggleStore implements IFeatureToggleStore { .where({ name: data.name }) .update(this.dtoToRow(project, data)) .returning(FEATURE_COLUMNS); - return this.rowToFeature(row[0]); + + 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; } async archive(name: string): Promise { @@ -377,6 +396,40 @@ export default class FeatureToggleStore implements IFeatureToggleStore { return toggle; } + + async markPotentiallyStaleFeatures( + 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); + + const updatedFeatures = await query.returning(FEATURE_COLUMNS); + return updatedFeatures.map(({ name }) => name); + } + + async isPotentiallyStale(featureName: string): Promise { + const result = await this.db(TABLE) + .first(['potentially_stale']) + .from(TABLE) + .where({ name: featureName }); + + return result?.potentially_stale ?? false; + } } module.exports = FeatureToggleStore; diff --git a/src/lib/services/feature-toggle-service.ts b/src/lib/services/feature-toggle-service.ts index 648bd15b85..3807e1b7e4 100644 --- a/src/lib/services/feature-toggle-service.ts +++ b/src/lib/services/feature-toggle-service.ts @@ -1965,6 +1965,10 @@ class FeatureToggleService { } } } + + async markPotentiallyStaleFeatures(): Promise { + await this.featureToggleStore.markPotentiallyStaleFeatures(); + } } export default FeatureToggleService; diff --git a/src/lib/services/index.ts b/src/lib/services/index.ts index 57a1093ef3..d85842d050 100644 --- a/src/lib/services/index.ts +++ b/src/lib/services/index.ts @@ -74,6 +74,7 @@ export const scheduleServices = async ( configurationRevisionService, maintenanceService, eventAnnouncerService, + featureToggleService, } = services; if (await maintenanceService.isMaintenanceMode()) { @@ -125,6 +126,13 @@ export const scheduleServices = async ( ), secondsToMilliseconds(1), ); + + schedulerService.schedule( + featureToggleService.markPotentiallyStaleFeatures.bind( + featureToggleService, + ), + minutesToMilliseconds(1), + ); }; export const createServices = ( diff --git a/src/lib/types/stores/feature-toggle-store.ts b/src/lib/types/stores/feature-toggle-store.ts index 2fa195513d..ac0f0f8316 100644 --- a/src/lib/types/stores/feature-toggle-store.ts +++ b/src/lib/types/stores/feature-toggle-store.ts @@ -32,6 +32,9 @@ export interface IFeatureToggleStore extends Store { range?: string[]; dateAccessor: string; }): Promise; + markPotentiallyStaleFeatures(currentTime?: string): Promise; + isPotentiallyStale(featureName: string): Promise; + /** * @deprecated - Variants should be fetched from FeatureEnvironmentStore (since variants are now; since 4.18, connected to environments) * @param featureName diff --git a/src/migrations/20230711094214-add-potentially-stale-flag.js b/src/migrations/20230711094214-add-potentially-stale-flag.js new file mode 100644 index 0000000000..2bb2eaf6a8 --- /dev/null +++ b/src/migrations/20230711094214-add-potentially-stale-flag.js @@ -0,0 +1,25 @@ +'use strict'; + +exports.up = function (db, cb) { + db.runSql( + ` + ALTER TABLE features ADD COLUMN IF NOT EXISTS potentially_stale boolean; + UPDATE features + SET potentially_stale = TRUE + FROM feature_types + WHERE feature_types.id = features.type + AND CURRENT_TIMESTAMP > (features.created_at + (feature_types.lifetime_days * INTERVAL '1 day')) + AND features.stale IS NOT TRUE; + `, + cb, + ); +}; + +exports.down = function (db, cb) { + db.runSql( + ` + ALTER table features DROP COLUMN IF EXISTS potentially_stale; + `, + cb, + ); +}; 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 f74c008823..0b927ea3ff 100644 --- a/src/test/e2e/stores/feature-toggle-store.e2e.test.ts +++ b/src/test/e2e/stores/feature-toggle-store.e2e.test.ts @@ -1,9 +1,10 @@ import dbInit from '../helpers/database-init'; import getLogger from '../../fixtures/no-logger'; +import { FeatureToggleDTO, IFeatureToggleStore } from '../../../lib/types'; let stores; let db; -let featureToggleStore; +let featureToggleStore: IFeatureToggleStore; beforeAll(async () => { getLogger.setMuteError(true); @@ -27,3 +28,247 @@ test('should not crash for undefined toggle name', async () => { const project = await featureToggleStore.getProjectId(undefined); expect(project).toBe(undefined); }); + +describe('potentially_stale marking', () => { + afterEach(async () => { + await featureToggleStore.deleteAll(); + }); + const getFutureTimestamp = (days: number) => { + return new Date( + new Date().getTime() + + days * 24 * 60 * 60 * 1000 + + // add an extra second + 1000, + ).toISOString(); + }; + + test('it returns an empty list of no toggles were updated', async () => { + const features: FeatureToggleDTO[] = [ + { + name: 'feature1', + type: 'release', + }, + ]; + await Promise.all( + features.map((feature) => + featureToggleStore.create('default', feature), + ), + ); + + const markedToggles = + await featureToggleStore.markPotentiallyStaleFeatures(); + + expect(markedToggles).toStrictEqual([]); + }); + test('it returns the names of only the marked toggles', async () => { + const features: FeatureToggleDTO[] = [ + { + name: 'feature1', + type: 'release', + }, + { + name: 'feature2', + type: 'kill-switch', + }, + ]; + await Promise.all( + features.map((feature) => + featureToggleStore.create('default', feature), + ), + ); + + const markedToggles = + await featureToggleStore.markPotentiallyStaleFeatures( + getFutureTimestamp(41), + ); + + expect(markedToggles).toStrictEqual(['feature1']); + }); + + test.each([ + [0, []], + [7, ['operational']], + [40, ['operational', 'release', 'experiment']], + [10000, ['operational', 'release', 'experiment']], + ])( + 'it marks toggles based on their type (days elapsed: %s)', + async (daysElapsed, expectedMarkedFeatures) => { + const features: FeatureToggleDTO[] = [ + 'release', + 'experiment', + 'operational', + 'kill-switch', + 'permission', + ].map((type) => ({ name: type, type })); + await Promise.all( + features.map((feature) => + featureToggleStore.create('default', feature), + ), + ); + + for (const feature of expectedMarkedFeatures) { + expect(await featureToggleStore.get(feature)).toBeTruthy(); + } + + const markedToggles = + await featureToggleStore.markPotentiallyStaleFeatures( + getFutureTimestamp(daysElapsed), + ); + + expect(markedToggles).toEqual( + expect.arrayContaining(expectedMarkedFeatures), + ); + expect(markedToggles.length).toEqual(expectedMarkedFeatures.length); + + for (const feature of expectedMarkedFeatures) { + expect( + await featureToggleStore.isPotentiallyStale(feature), + ).toBeTruthy(); + } + }, + ); + test('it does not mark toggles already flagged as stale', async () => { + const features: FeatureToggleDTO[] = [ + { + name: 'feature1', + type: 'release', + stale: true, + }, + ]; + await Promise.all( + features.map((feature) => + featureToggleStore.create('default', feature), + ), + ); + const markedToggles = + await featureToggleStore.markPotentiallyStaleFeatures( + getFutureTimestamp(1000), + ); + expect(markedToggles).toStrictEqual([]); + }); + test('it does not return toggles previously marked as potentially_stale', async () => { + const features: FeatureToggleDTO[] = [ + { + name: 'feature1', + type: 'release', + }, + ]; + await Promise.all( + features.map((feature) => + featureToggleStore.create('default', feature), + ), + ); + const markedToggles = + await featureToggleStore.markPotentiallyStaleFeatures( + getFutureTimestamp(50), + ); + expect(markedToggles).toStrictEqual(['feature1']); + + const secondPass = + await featureToggleStore.markPotentiallyStaleFeatures( + 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[] = [ + { + name: 'feature1', + type: 'release', + }, + ]; + await Promise.all( + features.map((feature) => + featureToggleStore.create('default', feature), + ), + ); + const markedToggles = + await featureToggleStore.markPotentiallyStaleFeatures( + getFutureTimestamp(50), + ); + + expect(markedToggles).toStrictEqual(['feature1']); + expect( + await featureToggleStore.isPotentiallyStale('feature1'), + ).toBeTruthy(); + + await featureToggleStore.update('default', { + name: 'feature1', + type: 'kill-switch', + }); + + const potentiallyStale = + await featureToggleStore.isPotentiallyStale('feature1'); + + expect(potentiallyStale).toBeFalsy(); + }); + + test('if a fresh feature changes to a type that should be stale, it gets marked as potentially stale', async () => { + const features: FeatureToggleDTO[] = [ + { + name: 'feature1', + type: 'kill-switch', + }, + ]; + await Promise.all( + features.map((feature) => + featureToggleStore.create('default', feature), + ), + ); + const markedToggles = + await featureToggleStore.markPotentiallyStaleFeatures( + getFutureTimestamp(50), + ); + + expect(markedToggles).toStrictEqual([]); + + await featureToggleStore.update('default', { + name: 'feature1', + type: 'release', + createdAt: getPastDate(40), + }); + + const potentiallyStale = + await featureToggleStore.isPotentiallyStale('feature1'); + + expect(potentiallyStale).toBeTruthy(); + }); + + test('if a stale feature changes to a type that should be stale, it does not get marked as potentially stale', async () => { + const features: FeatureToggleDTO[] = [ + { + name: 'feature1', + type: 'kill-switch', + stale: true, + }, + ]; + await Promise.all( + features.map((feature) => + featureToggleStore.create('default', feature), + ), + ); + + await featureToggleStore.update('default', { + name: 'feature1', + type: 'release', + createdAt: getPastDate(40), + }); + + const potentiallyStale = + await featureToggleStore.isPotentiallyStale('feature1'); + + expect(potentiallyStale).toBeFalsy(); + }); + }); +}); diff --git a/src/test/fixtures/fake-feature-toggle-store.ts b/src/test/fixtures/fake-feature-toggle-store.ts index b4fb17a7eb..e35d9d768e 100644 --- a/src/test/fixtures/fake-feature-toggle-store.ts +++ b/src/test/fixtures/fake-feature-toggle-store.ts @@ -252,4 +252,12 @@ export default class FakeFeatureToggleStore implements IFeatureToggleStore { this.features.forEach((feature) => (feature.variants = [])); return Promise.resolve(); } + + markPotentiallyStaleFeatures(): Promise { + throw new Error('Method not implemented.'); + } + + isPotentiallyStale(): Promise { + throw new Error('Method not implemented.'); + } }