diff --git a/src/lib/features/events/event-service.test.ts b/src/lib/features/events/event-service.test.ts index 79f93d5109..35787b3d83 100644 --- a/src/lib/features/events/event-service.test.ts +++ b/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) => { + 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(); + }); +}); diff --git a/src/lib/features/events/event-service.ts b/src/lib/features/events/event-service.ts index 885c2888e7..310b546514 100644 --- a/src/lib/features/events/event-service.ts +++ b/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 { - 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); } diff --git a/src/lib/features/feature-toggle/feature-toggle-service.ts b/src/lib/features/feature-toggle/feature-toggle-service.ts index 75d9b80c1b..a8c1429f48 100644 --- a/src/lib/features/feature-toggle/feature-toggle-service.ts +++ b/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( diff --git a/src/lib/features/feature-toggle/tests/feature-toggles.e2e.test.ts b/src/lib/features/feature-toggle/tests/feature-toggles.e2e.test.ts index 2af3c45eda..c1ca02194b 100644 --- a/src/lib/features/feature-toggle/tests/feature-toggles.e2e.test.ts +++ b/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 diff --git a/src/lib/features/segment/segment-service.ts b/src/lib/features/segment/segment-service.ts index 38e12e6b6f..8fa3a90172 100644 --- a/src/lib/features/segment/segment-service.ts +++ b/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 { @@ -173,7 +173,7 @@ export class SegmentService implements ISegmentService { async unprotectedUpdate( id: number, - data: unknown, + data: UpsertSegmentSchema, auditUser: IAuditUser, ): Promise { const input = await segmentSchema.validateAsync(data); diff --git a/src/lib/services/api-token-service.test.ts b/src/lib/services/api-token-service.test.ts index 85f6008d3c..132593c386 100644 --- a/src/lib/services/api-token-service.test.ts +++ b/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); diff --git a/src/lib/services/api-token-service.ts b/src/lib/services/api-token-service.ts index 9cac272dad..2b47b37717 100644 --- a/src/lib/services/api-token-service.ts +++ b/src/lib/services/api-token-service.ts @@ -244,7 +244,7 @@ export class ApiTokenService { expiresAt: Date, auditUser: IAuditUser, ): Promise { - 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({ diff --git a/src/lib/types/stores/store.ts b/src/lib/types/stores/store.ts index db9b5b9e7b..262d60bfc1 100644 --- a/src/lib/types/stores/store.ts +++ b/src/lib/types/stores/store.ts @@ -1,5 +1,5 @@ export interface Store { - get(key: K): Promise; + get(key: K): Promise; getAll(query?: Object): Promise; exists(key: K): Promise; delete(key: K): Promise; diff --git a/src/test/fixtures/fake-api-token-store.ts b/src/test/fixtures/fake-api-token-store.ts index d24f920e83..6c7624c820 100644 --- a/src/test/fixtures/fake-api-token-store.ts +++ b/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 { - // get can return undefined. See api-token-store.e2e.test.ts - return this.tokens.find((t) => t.secret === key); + async get(key: string): Promise { + const found = this.tokens.find((t) => t.secret === key); + // clone the object to get a copy + return found ? { ...found } : undefined; } async getAll(): Promise { @@ -74,9 +75,12 @@ export default class FakeApiTokenStore } async setExpiry(secret: string, expiresAt: Date): Promise { - 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<{