feat: ignore events in log when nothing has changed (#9364)

## About the changes
Some automation may keep some data up-to-date (e.g. segments). These
updates sometimes don't generate changes but we're still storing these
events in the event log and triggering reactions to those events.


Arguably, this could be done in each service domain logic, but it seems
to be a pretty straightforward solution: if preData and data are
provided, it means some change happened. Other events that don't have
preData or don't have data are treated as before.

Tests were added to validate we don't break other events.

src/lib/features/events/event-service.test.ts

@@ -1,5 +1,15 @@
+import { getLogger } from 'log4js';
+import {
+    type IBaseEvent,
+    type IEventStore,
+    type IFeatureTagStore,
+    type IUnleashConfig,
+    USER_UPDATED,
+} from '../../internals';
+import type { IAccessReadModel } from '../access/access-read-model-type';
+import type { IPrivateProjectChecker } from '../private-project/privateProjectCheckerType';
 import type { ProjectAccess } from '../private-project/privateProjectStore';
-import { filterAccessibleProjects } from './event-service';
+import EventService, { filterAccessibleProjects } from './event-service';
 
 describe('filterPrivateProjectsFromParams', () => {
     it('should return IS_ANY_OF with allowed projects when projectParam is undefined and mode is limited', () => {
@@ -91,3 +101,89 @@ describe('filterPrivateProjectsFromParams', () => {
         ).toThrow('No accessible projects in the search parameters');
     });
 });
+
+describe('storeEvents', () => {
+    test.each([
+        {},
+        {
+            data: {
+                name: 'test',
+            },
+        },
+        {
+            predata: {
+                name: 'pretest',
+            },
+            data: {
+                name: 'test',
+            },
+        },
+    ])(
+        'should store the event %s',
+        async (preDataAndData: Pick<IBaseEvent, 'preData' | 'data'>) => {
+            const eventStore = {
+                batchStore: jest.fn(),
+            } as unknown as IEventStore;
+            const eventService = new EventService(
+                {
+                    eventStore,
+                    featureTagStore: {
+                        getAllByFeatures: jest.fn().mockReturnValue([]),
+                    } as unknown as IFeatureTagStore,
+                },
+                { getLogger, eventBus: undefined } as unknown as IUnleashConfig,
+                undefined as unknown as IPrivateProjectChecker,
+                undefined as unknown as IAccessReadModel,
+            );
+
+            const event = {
+                type: USER_UPDATED,
+                createdBy: 'test',
+                createdByUserId: 1,
+                ip: '127.0.0.1',
+                ...preDataAndData,
+            };
+
+            await eventService.storeEvent(event);
+            expect(eventStore.batchStore).toHaveBeenCalledWith([event]);
+        },
+    );
+    test('should not store the event when predata and data are the same', async () => {
+        const eventStore = {
+            batchStore: jest.fn(),
+        } as unknown as IEventStore;
+        const eventService = new EventService(
+            {
+                eventStore,
+                featureTagStore: {
+                    getAllByFeatures: jest.fn().mockReturnValue([]),
+                } as unknown as IFeatureTagStore,
+            },
+            { getLogger, eventBus: undefined } as unknown as IUnleashConfig,
+            undefined as unknown as IPrivateProjectChecker,
+            undefined as unknown as IAccessReadModel,
+        );
+
+        const event = {
+            type: USER_UPDATED,
+            createdBy: 'test',
+            createdByUserId: 1,
+            ip: '127.0.0.1',
+            preData: {
+                name: 'test',
+                nest: {
+                    this: 'object',
+                },
+            },
+            data: {
+                name: 'test',
+                nest: {
+                    this: 'object',
+                },
+            },
+        };
+
+        await eventService.storeEvent(event);
+        expect(eventStore.batchStore).not.toHaveBeenCalled();
+    });
+});

src/lib/features/events/event-service.ts

@@ -17,6 +17,7 @@ import { addDays, formatISO } from 'date-fns';
 import type { IPrivateProjectChecker } from '../private-project/privateProjectCheckerType';
 import type { ProjectAccess } from '../private-project/privateProjectStore';
 import type { IAccessReadModel } from '../access/access-read-model-type';
+import { isEqual } from 'lodash';
 
 export default class EventService {
     private logger: Logger;
@@ -153,7 +154,16 @@ export default class EventService {
     }
 
     async storeEvents(events: IBaseEvent[]): Promise<void> {
-        let enhancedEvents = events;
+        // if the event comes with both preData and data, we need to check if they are different before storing, otherwise we discard the event
+        let enhancedEvents = events.filter(
+            (event) =>
+                !event.preData ||
+                !event.data ||
+                !isEqual(event.preData, event.data),
+        );
+        if (enhancedEvents.length === 0) {
+            return;
+        }
         for (const enhancer of [this.enhanceEventsWithTags.bind(this)]) {
             enhancedEvents = await enhancer(enhancedEvents);
         }

src/lib/features/feature-toggle/feature-toggle-service.ts

@@ -603,12 +603,9 @@ class FeatureToggleService {
         const eventPreData: StrategyIds = { strategyIds: existingOrder };
 
         await Promise.all(
-            sortOrders.map(async ({ id, sortOrder }) => {
-                await this.featureStrategiesStore.updateSortOrder(
-                    id,
-                    sortOrder,
-                );
-            }),
+            sortOrders.map(({ id, sortOrder }) =>
+                this.featureStrategiesStore.updateSortOrder(id, sortOrder),
+            ),
         );
         const newOrder = (
             await this.getStrategiesForEnvironment(

src/lib/features/feature-toggle/tests/feature-toggles.e2e.test.ts

@@ -3609,11 +3609,16 @@ test('Updating feature strategy sort-order should trigger a an event', async ()
     );
 
     const strategies: FeatureStrategySchema[] = body;
-    let order = 1;
     const sortOrders: SetStrategySortOrderSchema = [];
 
-    strategies.forEach((strategy) => {
-        sortOrders.push({ id: strategy.id!, sortOrder: order++ });
+    // swap two strategies with different sort orders (note: first and second have the same sort order)
+    sortOrders.push({
+        id: strategies[0].id!,
+        sortOrder: strategies[2].sortOrder ?? 0,
+    });
+    sortOrders.push({
+        id: strategies[2].id!,
+        sortOrder: strategies[0].sortOrder ?? 0,
     });
 
     await app.request

src/lib/features/segment/segment-service.ts

@@ -25,7 +25,7 @@ import type { IChangeRequestAccessReadModel } from '../change-request-access-ser
 import type { IPrivateProjectChecker } from '../private-project/privateProjectCheckerType';
 import type EventService from '../events/event-service';
 import type { IChangeRequestSegmentUsageReadModel } from '../change-request-segment-usage-service/change-request-segment-usage-read-model';
-import type { ResourceLimitsSchema } from '../../openapi';
+import type { ResourceLimitsSchema, UpsertSegmentSchema } from '../../openapi';
 import { throwExceedsLimitError } from '../../error/exceeds-limit-error';
 
 export class SegmentService implements ISegmentService {
@@ -162,7 +162,7 @@ export class SegmentService implements ISegmentService {
 
     async update(
         id: number,
-        data: unknown,
+        data: UpsertSegmentSchema,
         user: User,
         auditUser: IAuditUser,
     ): Promise<void> {
@@ -173,7 +173,7 @@ export class SegmentService implements ISegmentService {
 
     async unprotectedUpdate(
         id: number,
-        data: unknown,
+        data: UpsertSegmentSchema,
         auditUser: IAuditUser,
     ): Promise<void> {
         const input = await segmentSchema.validateAsync(data);

src/lib/services/api-token-service.test.ts

@@ -111,7 +111,7 @@ test('Api token operations should all have events attached', async () => {
         (e) => e.type === API_TOKEN_UPDATED,
     );
     expect(updatedApiTokenEvents).toHaveLength(1);
-    expect(updatedApiTokenEvents[0].preData.expiresAt).toBeDefined();
+    expect(updatedApiTokenEvents[0].preData.expiresAt).toBeUndefined();
     expect(updatedApiTokenEvents[0].preData.secret).toBeUndefined();
     expect(updatedApiTokenEvents[0].data.secret).toBeUndefined();
     expect(updatedApiTokenEvents[0].data.expiresAt).toBe(newExpiry);

src/lib/services/api-token-service.ts

@@ -244,7 +244,7 @@ export class ApiTokenService {
         expiresAt: Date,
         auditUser: IAuditUser,
     ): Promise<IApiToken> {
-        const previous = await this.store.get(secret);
+        const previous = (await this.store.get(secret))!;
         const token = await this.store.setExpiry(secret, expiresAt);
         await this.eventService.storeEvent(
             new ApiTokenUpdatedEvent({

src/lib/types/stores/store.ts

@@ -1,5 +1,5 @@
 export interface Store<T, K> {
-    get(key: K): Promise<T>;
+    get(key: K): Promise<T | undefined>;
     getAll(query?: Object): Promise<T[]>;
     exists(key: K): Promise<boolean>;
     delete(key: K): Promise<void>;

src/test/fixtures/fake-api-token-store.ts

@@ -37,9 +37,10 @@ export default class FakeApiTokenStore
         return this.tokens.some((token) => token.secret === key);
     }
 
-    async get(key: string): Promise<IApiToken> {
-        // get can return undefined. See api-token-store.e2e.test.ts
-        return this.tokens.find((t) => t.secret === key);
+    async get(key: string): Promise<IApiToken | undefined> {
+        const found = this.tokens.find((t) => t.secret === key);
+        // clone the object to get a copy
+        return found ? { ...found } : undefined;
     }
 
     async getAll(): Promise<IApiToken[]> {
@@ -74,9 +75,12 @@ export default class FakeApiTokenStore
     }
 
     async setExpiry(secret: string, expiresAt: Date): Promise<IApiToken> {
-        const t = await this.get(secret);
-        t.expiresAt = expiresAt;
-        return t;
+        const found = this.tokens.find((t) => t.secret === secret);
+        if (!found) {
+            return undefined;
+        }
+        found.expiresAt = expiresAt;
+        return found;
     }
 
     async countDeprecatedTokens(): Promise<{