Skip to content

Commit

Permalink
Fix/clean last seen environments (#5402)
Browse files Browse the repository at this point in the history
This PR addresses some cleanup related to removing the
useLastSeenRefactor flag:

* Added fallback last seen to the feature table last_seen_at column 
* Remove foreign key on environment since we can not guarantee that we
will get valid data in this field
* Add environments to cleanup function
* Add test for cleanup environments
  • Loading branch information
FredrikOseberg committed Nov 23, 2023
1 parent 88a034d commit a8ea8d3
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 1 deletion.
Expand Up @@ -386,6 +386,7 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore {
acc.description = r.description;
acc.project = r.project;
acc.stale = r.stale;
acc.lastSeenAt = r.last_seen_at;

acc.createdAt = r.created_at;
acc.type = r.type;
Expand Down Expand Up @@ -638,6 +639,7 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore {
'features.type as type',
'features.created_at as created_at',
'features.stale as stale',
'features.last_seen_at as last_seen_at',
'features.impression_data as impression_data',
'feature_environments.enabled as enabled',
'feature_environments.environment as environment',
Expand Down Expand Up @@ -806,6 +808,7 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore {
'features.type as type',
'features.created_at as created_at',
'features.stale as stale',
'features.last_seen_at as last_seen_at',
'features.impression_data as impression_data',
'feature_environments.enabled as enabled',
'feature_environments.environment as environment',
Expand Down Expand Up @@ -888,6 +891,7 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore {
createdAt: row.created_at,
stale: row.stale,
impressionData: row.impression_data,
lastSeenAt: row.last_seen_at,
environments: [FeatureStrategiesStore.getEnvironment(row)],
};

Expand All @@ -898,7 +902,8 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore {
const featureRow = acc[row.feature_name];
if (
featureRow.lastSeenAt === undefined ||
new Date(row.env_last_seen_at) > new Date(featureRow.lastSeenAt)
new Date(row.env_last_seen_at) >
new Date(featureRow.last_seen_at)
) {
featureRow.lastSeenAt = row.env_last_seen_at;
}
Expand Down
4 changes: 4 additions & 0 deletions src/lib/services/client-metrics/last-seen/last-seen-store.ts
Expand Up @@ -74,6 +74,10 @@ export default class LastSeenStore implements ILastSeenStore {
async cleanLastSeen() {
await this.db(TABLE)
.whereNotIn('feature_name', this.db.select('name').from('features'))
.or.whereNotIn(
'environment',
this.db.select('name').from('environments'),
)
.del();
}
}
Expand Down
Expand Up @@ -24,6 +24,10 @@ beforeAll(async () => {
);
});

beforeEach(async () => {
await db.rawDatabase.raw('DELETE FROM last_seen_at_metrics;');
});

afterAll(async () => {
await app.destroy();
await db.destroy();
Expand Down Expand Up @@ -79,3 +83,51 @@ test('should clean unknown feature toggle names from last seen store', async ()

expect(notInDirty.length).toBe(4);
});

test('should clean unknown feature toggle environments from last seen store', async () => {
const { lastSeenService, featureToggleService } = app.services;

const clean = [
{ name: 'clean5', environment: 'default' },
{ name: 'clean6', environment: 'default' },
{ name: 'clean7', environment: 'nonexisting' },
{ name: 'clean8', environment: 'nonexisting' },
];

await Promise.all(
clean.map((feature) =>
featureToggleService.createFeatureToggle(
'default',
{ name: feature.name },
'user',
),
),
);

const inserts = [...clean].map((feature) => {
return {
featureName: feature.name,
environment: feature.environment,
yes: 1,
no: 0,
appName: 'test',
timestamp: new Date(),
};
});

lastSeenService.updateLastSeen(inserts);
await lastSeenService.store();

// We have no method to get these from the last seen service or any other service or store
let stored = await db.rawDatabase.raw(
'SELECT * FROM last_seen_at_metrics;',
);

expect(stored.rows.length).toBe(4);

await lastSeenService.cleanLastSeen();

stored = await db.rawDatabase.raw('SELECT * FROM last_seen_at_metrics;');

expect(stored.rows.length).toBe(2);
});
17 changes: 17 additions & 0 deletions src/migrations/20231123100052-drop-last-seen-foreign-key.js
@@ -0,0 +1,17 @@
exports.up = function (db, cb) {
db.runSql(
`
ALTER TABLE last_seen_at_metrics DROP CONSTRAINT last_seen_at_metrics_environment_fkey;
`,
cb
);
};

exports.down = function (db, cb) {
db.runSql(
`
ALTER TABLE last_seen_at_metrics ADD CONSTRAINT last_seen_at_metrics_environment_fkey FOREIGN KEY (environment) REFERENCES environments(name);
`,
cb
);
};

0 comments on commit a8ea8d3

Please sign in to comment.