Mirrors/unleash.unleash
1
0
Fork 0
mirror of https://github.com/Unleash/unleash.git synced 2025-01-25 00:07:47 +01:00
Code Issues Projects Releases Wiki Activity

#4205: update to prepare for emitting potentially stale events (#4239)

Browse Source 
This PR adds updates the potentially stale status change events whenever
the potentially stale update function is run.

No events are emitted yet. While the emission is only a few lines of
code, I'd like to do that in a separate PR so that we can give it the
attention it deserves in the form of tests, etc.

This PR also moves the potentially stale update functionality from the
`update` method to only being done in the
`updatePotentiallyStaleFeatures` method. This keeps all functionality
related to marking `potentiallyStale` in one place.

The emission implementation was removed in
4fb7cbde03

## The update queries

While it would be possible to do the state updates in a single query
instead of three separate ones, wrangling this into knex proved to be
troublesome (and would also probably be harder to understand and reason
about). The current solution uses three smaller queries (one select, two
updates), as Jaanus suggested in a private slack thread.
This commit is contained in:
Thomas Heartman 2023-07-17 09:53:32 +02:00 committed by GitHub
parent f91c8a338a
commit 333c0c0db1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 101 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

83
src/lib/db/feature-toggle-store.ts
View File

@ -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<FeatureToggle> {
@ -397,29 +379,50 @@ export default class FeatureToggleStore implements IFeatureToggleStore {
return toggle;
}
async markPotentiallyStaleFeatures(
async updatePotentiallyStaleFeatures(
currentTime?: string,
): Promise<string[]> {
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<boolean> {

4
src/lib/services/feature-toggle-service.ts
View File

@ -1983,8 +1983,8 @@ class FeatureToggleService {
}
}
async markPotentiallyStaleFeatures(): Promise<void> {
await this.featureToggleStore.markPotentiallyStaleFeatures();
async updatePotentiallyStaleFeatures(): Promise<void> {
await this.featureToggleStore.updatePotentiallyStaleFeatures();
}
}

2
src/lib/services/index.ts
View File

@ -128,7 +128,7 @@ export const scheduleServices = async (
);
schedulerService.schedule(
featureToggleService.markPotentiallyStaleFeatures.bind(
featureToggleService.updatePotentiallyStaleFeatures.bind(
featureToggleService,
),
minutesToMilliseconds(1),

4
src/lib/types/events.ts
View File

@ -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];

4
src/lib/types/stores/feature-toggle-store.ts
View File

@ -32,7 +32,9 @@ export interface IFeatureToggleStore extends Store<FeatureToggle, string> {
range?: string[];
dateAccessor: string;
}): Promise<number>;
markPotentiallyStaleFeatures(currentTime?: string): Promise<string[]>;
updatePotentiallyStaleFeatures(
currentTime?: string,
): Promise<{ name: string; potentiallyStale: boolean }[]>;
isPotentiallyStale(featureName: string): Promise<boolean>;
/**

70
src/test/e2e/stores/feature-toggle-store.e2e.test.ts
View File

@ -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');

4
src/test/fixtures/fake-feature-toggle-store.ts vendored
View File

@ -253,7 +253,9 @@ export default class FakeFeatureToggleStore implements IFeatureToggleStore {
return Promise.resolve();
}
markPotentiallyStaleFeatures(): Promise<string[]> {
updatePotentiallyStaleFeatures(): Promise<
{ name: string; potentiallyStale: boolean }[]
> {
throw new Error('Method not implemented.');
}