From 2879ce9dd65b4bde0fff1dde2d3da3e204e70965 Mon Sep 17 00:00:00 2001 From: Jaanus Sellin Date: Tue, 27 May 2025 16:16:26 +0300 Subject: [PATCH] fix: make revision id not be so reactive (#10032) Unleash is being too reactive to events inside Unleash. We should not update etag if feature is created or tag is added to feature. This PR adds this condition and adds test for it. --- .../client-feature-toggles.e2e.test.ts.snap | 4 +- src/lib/features/events/event-store.ts | 20 +++++- .../api/client/feature.optimal304.e2e.test.ts | 16 ++--- src/test/e2e/stores/event-store.e2e.test.ts | 71 +++++++++++++++++++ 4 files changed, 99 insertions(+), 12 deletions(-) diff --git a/src/lib/features/client-feature-toggles/tests/__snapshots__/client-feature-toggles.e2e.test.ts.snap b/src/lib/features/client-feature-toggles/tests/__snapshots__/client-feature-toggles.e2e.test.ts.snap index 7ce6ce6020..34d8d7b451 100644 --- a/src/lib/features/client-feature-toggles/tests/__snapshots__/client-feature-toggles.e2e.test.ts.snap +++ b/src/lib/features/client-feature-toggles/tests/__snapshots__/client-feature-toggles.e2e.test.ts.snap @@ -63,9 +63,9 @@ exports[`should match snapshot from /api/client/features 1`] = ` }, ], "meta": { - "etag": ""61824cd0:11"", + "etag": ""61824cd0:20"", "queryHash": "61824cd0", - "revisionId": 11, + "revisionId": 20, }, "query": { "environment": "default", diff --git a/src/lib/features/events/event-store.ts b/src/lib/features/events/event-store.ts index 598ec2f864..3491ff7891 100644 --- a/src/lib/features/events/event-store.ts +++ b/src/lib/features/events/event-store.ts @@ -1,5 +1,7 @@ import { + FEATURE_CREATED, FEATURE_IMPORT, + FEATURE_TAGGED, FEATURES_IMPORTED, type IBaseEvent, type IEvent, @@ -196,7 +198,14 @@ class EventStore implements IEventStore { .max('id') .where((builder) => builder - .whereNotNull('feature_name') + .andWhere((inner) => + inner + .whereNotNull('feature_name') + .whereNotIn('type', [ + FEATURE_CREATED, + FEATURE_TAGGED, + ]), + ) .orWhereIn('type', [ SEGMENT_UPDATED, FEATURE_IMPORT, @@ -216,7 +225,14 @@ class EventStore implements IEventStore { .andWhere('id', '<=', end) .andWhere((builder) => builder - .whereNotNull('feature_name') + .andWhere((inner) => + inner + .whereNotNull('feature_name') + .whereNotIn('type', [ + FEATURE_CREATED, + FEATURE_TAGGED, + ]), + ) .orWhereIn('type', [ SEGMENT_UPDATED, FEATURE_IMPORT, diff --git a/src/test/e2e/api/client/feature.optimal304.e2e.test.ts b/src/test/e2e/api/client/feature.optimal304.e2e.test.ts index 13a1ec715a..f852106ff1 100644 --- a/src/test/e2e/api/client/feature.optimal304.e2e.test.ts +++ b/src/test/e2e/api/client/feature.optimal304.e2e.test.ts @@ -152,9 +152,9 @@ describe.each([ .expect(200); if (etagVariant.feature_enabled) { - expect(res.headers.etag).toBe(`"61824cd0:17:${etagVariant.name}"`); + expect(res.headers.etag).toBe(`"61824cd0:16:${etagVariant.name}"`); expect(res.body.meta.etag).toBe( - `"61824cd0:17:${etagVariant.name}"`, + `"61824cd0:16:${etagVariant.name}"`, ); } else { expect(res.headers.etag).toBe('"61824cd0:16"'); @@ -169,9 +169,9 @@ describe.each([ .expect(etagVariant.feature_enabled ? 200 : 304); if (etagVariant.feature_enabled) { - expect(res.headers.etag).toBe(`"61824cd0:17:${etagVariant.name}"`); + expect(res.headers.etag).toBe(`"61824cd0:16:${etagVariant.name}"`); expect(res.body.meta.etag).toBe( - `"61824cd0:17:${etagVariant.name}"`, + `"61824cd0:16:${etagVariant.name}"`, ); } }); @@ -193,13 +193,13 @@ describe.each([ .expect(200); if (etagVariant.feature_enabled) { - expect(res.headers.etag).toBe(`"61824cd0:17:${etagVariant.name}"`); + expect(res.headers.etag).toBe(`"61824cd0:16:${etagVariant.name}"`); expect(res.body.meta.etag).toBe( - `"61824cd0:17:${etagVariant.name}"`, + `"61824cd0:16:${etagVariant.name}"`, ); } else { - expect(res.headers.etag).toBe('"61824cd0:17"'); - expect(res.body.meta.etag).toBe('"61824cd0:17"'); + expect(res.headers.etag).toBe('"61824cd0:16"'); + expect(res.body.meta.etag).toBe('"61824cd0:16"'); } }); }); diff --git a/src/test/e2e/stores/event-store.e2e.test.ts b/src/test/e2e/stores/event-store.e2e.test.ts index c3948950eb..d54169d6aa 100644 --- a/src/test/e2e/stores/event-store.e2e.test.ts +++ b/src/test/e2e/stores/event-store.e2e.test.ts @@ -2,11 +2,16 @@ import { APPLICATION_CREATED, FEATURE_CREATED, FEATURE_DELETED, + FEATURE_TAGGED, + FEATURE_UPDATED, + SEGMENT_UPDATED, type IEvent, } from '../../../lib/events/index.js'; import { FeatureCreatedEvent, FeatureDeletedEvent, + FeatureTaggedEvent, + FeatureUpdatedEvent, } from '../../../lib/types/index.js'; import dbInit, { type ITestDb } from '../helpers/database-init.js'; @@ -247,3 +252,69 @@ test('Should get all events of type', async () => { }); expect(featureDeletedEvents).toHaveLength(3); }); + +test('getMaxRevisionId should exclude FEATURE_CREATED and FEATURE_TAGGED events', async () => { + const featureName = 'test-feature'; + const project = 'test-project'; + + const featureCreatedEvent = new FeatureCreatedEvent({ + project, + featureName, + auditUser: testAudit, + data: { name: featureName, project }, + }); + + const featureTaggedEvent = new FeatureTaggedEvent({ + project, + featureName, + auditUser: testAudit, + data: { type: 'simple', value: 'test-tag' }, + }); + + const featureUpdatedEvent = new FeatureUpdatedEvent({ + project, + featureName, + auditUser: testAudit, + data: { name: featureName, enabled: false }, + }); + + const segmentUpdatedEvent = { + type: SEGMENT_UPDATED, + createdBy: testAudit.username, + createdByUserId: testAudit.id, + ip: testAudit.ip, + data: { id: 1, name: 'test-segment' }, + }; + + await eventStore.store(featureCreatedEvent); + const maxRevisionAfterCreated = await eventStore.getMaxRevisionId(); + + await eventStore.store(featureTaggedEvent); + const maxRevisionAfterTagged = await eventStore.getMaxRevisionId(); + + await eventStore.store(featureUpdatedEvent); + const maxRevisionAfterUpdated = await eventStore.getMaxRevisionId(); + + await eventStore.store(segmentUpdatedEvent); + const maxRevisionAfterSegment = await eventStore.getMaxRevisionId(); + + const allEvents = await eventStore.getAll(); + const createdEvent = allEvents.find((e) => e.type === FEATURE_CREATED); + const taggedEvent = allEvents.find((e) => e.type === FEATURE_TAGGED); + const updatedEvent = allEvents.find((e) => e.type === FEATURE_UPDATED); + const segmentEvent = allEvents.find((e) => e.type === SEGMENT_UPDATED); + + expect(maxRevisionAfterCreated).toBe(0); + expect(maxRevisionAfterTagged).toBe(0); + expect(maxRevisionAfterUpdated).toBe(updatedEvent!.id); + expect(maxRevisionAfterSegment).toBe(segmentEvent!.id); + + expect(createdEvent).toBeDefined(); + expect(taggedEvent).toBeDefined(); + expect(updatedEvent).toBeDefined(); + expect(segmentEvent).toBeDefined(); + + expect(updatedEvent!.id).toBeGreaterThan(createdEvent!.id); + expect(updatedEvent!.id).toBeGreaterThan(taggedEvent!.id); + expect(segmentEvent!.id).toBeGreaterThan(updatedEvent!.id); +});