Skip to content

Commit

Permalink
feat: delta rework (#9133)
Browse files Browse the repository at this point in the history
We are changing how the Delta API works, as discussed:

1. We have removed the `updated` and `removed` arrays and now keep
everything in the `events` array.
2. We decided to keep the hydration cache separate from the events array
internally. Since the hydration cache has a special structure and may
contain not just one feature but potentially 1,000 features, it behaved
differently, requiring a lot of special logic to handle it.
3. Implemented `nameprefix` filtering, which we were missing before.


Things still to implement:

1.  Segment hydration and updates to it.
  • Loading branch information
sjaanus authored Jan 22, 2025
1 parent 4bbff0c commit 280710f
Show file tree
Hide file tree
Showing 12 changed files with 659 additions and 469 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import {
} from '../../internals';
import isEqual from 'lodash.isequal';
import { diff } from 'json-diff';
import type { DeltaHydrationEvent } from './delta/client-feature-toggle-delta-types';

const version = 2;

Expand Down Expand Up @@ -191,22 +192,34 @@ export default class FeatureController extends Controller {
const sortedToggles = features.sort((a, b) =>
a.name.localeCompare(b.name),
);
const sortedNewToggles = delta?.updated.sort((a, b) =>
a.name.localeCompare(b.name),
);
if (delta?.events[0].type === 'hydration') {
const hydrationEvent: DeltaHydrationEvent =
delta?.events[0];
const sortedNewToggles = hydrationEvent.features.sort(
(a, b) => a.name.localeCompare(b.name),
);

if (
!this.deepEqualIgnoreOrder(sortedToggles, sortedNewToggles)
) {
this.logger.warn(
`old features and new features are different. Old count ${
features.length
}, new count ${delta?.updated.length}, query ${JSON.stringify(query)},
if (
!this.deepEqualIgnoreOrder(
sortedToggles,
sortedNewToggles,
)
) {
this.logger.warn(
`old features and new features are different. Old count ${
features.length
}, new count ${hydrationEvent.features.length}, query ${JSON.stringify(query)},
diff ${JSON.stringify(
diff(sortedToggles, sortedNewToggles),
)}`,
);
}
} else {
this.logger.warn(
`Delta diff should have only hydration event, query ${JSON.stringify(query)}`,
);
}

this.storeFootprint();
} catch (e) {
this.logger.error('Delta diff failed', e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ const setupFeatures = async (
{
name: 'default',
constraints: [
{ contextName: 'userId', operator: 'IN', values: ['123'] },
{
contextName: 'userId',
operator: 'IN',
values: ['123'],
},
],
parameters: {},
},
Expand Down Expand Up @@ -88,89 +92,122 @@ test('should match with /api/client/delta', async () => {
.expect('Content-Type', /json/)
.expect(200);

expect(body.features).toMatchObject(deltaBody.updated);
expect(body.features).toMatchObject(deltaBody.events[0].features);
});

test('should get 304 if asked for latest revision', async () => {
await setupFeatures(db, app);

const { body } = await app.request.get('/api/client/delta').expect(200);
const currentRevisionId = body.revisionId;
const { body, headers } = await app.request
.get('/api/client/delta')
.expect(200);
const etag = headers.etag;

await app.request
.set('If-None-Match', `"${currentRevisionId}"`)
.set('If-None-Match', etag)
.get('/api/client/delta')
.expect(304);
});

test('should return correct delta after feature created', async () => {
await app.createFeature('base_feature');
await syncRevisions();
const { body } = await app.request.get('/api/client/delta').expect(200);
const currentRevisionId = body.revisionId;
const { body, headers } = await app.request
.set('If-None-Match', null)
.get('/api/client/delta')
.expect(200);
const etag = headers.etag;

expect(body).toMatchObject({
updated: [
events: [
{
name: 'base_feature',
type: 'hydration',
features: [
{
name: 'base_feature',
},
],
},
],
});

await app.createFeature('new_feature');

await syncRevisions();
//@ts-ignore
await app.services.clientFeatureToggleService.clientFeatureToggleDelta.onUpdateRevisionEvent();

const { body: deltaBody } = await app.request
.get('/api/client/delta')
.set('If-None-Match', `"${currentRevisionId}"`)
.set('If-None-Match', etag)
.expect(200);

expect(deltaBody).toMatchObject({
updated: [
events: [
{
type: 'feature-updated',
feature: {
name: 'new_feature',
},
},
{
name: 'new_feature',
type: 'feature-updated',
feature: {
name: 'new_feature',
},
},
],
});
});

const syncRevisions = async () => {
await app.services.configurationRevisionService.updateMaxRevisionId();
// @ts-ignore
await app.services.clientFeatureToggleService.clientFeatureToggleDelta.onUpdateRevisionEvent();
};

test('archived features should not be returned as updated', async () => {
await app.createFeature('base_feature');
await syncRevisions();
const { body } = await app.request.get('/api/client/delta').expect(200);
const currentRevisionId = body.revisionId;
const { body, headers } = await app.request
.get('/api/client/delta')
.expect(200);
const etag = headers.etag;

expect(body).toMatchObject({
updated: [
events: [
{
name: 'base_feature',
features: [
{
name: 'base_feature',
},
],
},
],
});

await app.archiveFeature('base_feature');
await syncRevisions();
await app.createFeature('new_feature');

await syncRevisions();
await app.getProjectFeatures('new_feature'); // TODO: this is silly, but events syncing and tests do not work nicely. this is basically a setTimeout

const { body: deltaBody } = await app.request
.get('/api/client/delta')
.set('If-None-Match', `"${currentRevisionId}"`)
.set('If-None-Match', etag)
.expect(200);

expect(deltaBody).toMatchObject({
updated: [
events: [
{
type: 'feature-removed',
featureName: 'base_feature',
},
{
name: 'new_feature',
type: 'feature-updated',
feature: {
name: 'new_feature',
},
},
],
removed: ['base_feature'],
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -103,15 +103,16 @@ export default class ClientFeatureToggleDeltaController extends Controller {
res.end();
return;
}

if (changedFeatures.revisionId === currentSdkRevisionId) {
const lastEventId =
changedFeatures.events[changedFeatures.events.length - 1].eventId;
if (lastEventId === currentSdkRevisionId) {
res.status(304);
res.getHeaderNames().forEach((header) => res.removeHeader(header));
res.end();
return;
}

res.setHeader('ETag', `"${changedFeatures.revisionId}"`);
res.setHeader('ETag', `"${lastEventId}"`);
this.openApiService.respondWithValidation(
200,
res,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import type { ClientFeatureSchema } from '../../../openapi';
import type { IClientSegment } from '../../../types';

export type DeltaHydrationEvent = {
eventId: number;
type: 'hydration';
features: ClientFeatureSchema[];
};

export type DeltaEvent =
| {
eventId: number;
type: 'feature-updated';
feature: ClientFeatureSchema;
}
| {
eventId: number;
type: 'feature-removed';
featureName: string;
project: string;
}
| {
eventId: number;
type: 'segment-updated';
segment: IClientSegment;
}
| {
eventId: number;
type: 'segment-removed';
segmentId: number;
};

export const DELTA_EVENT_TYPES = {
FEATURE_UPDATED: 'feature-updated',
FEATURE_REMOVED: 'feature-removed',
SEGMENT_UPDATED: 'segment-updated',
SEGMENT_REMOVED: 'segment-removed',
HYDRATION: 'hydration',
} as const;

export const isDeltaFeatureUpdatedEvent = (
event: DeltaEvent,
): event is Extract<DeltaEvent, { type: 'feature-updated' }> => {
return event.type === DELTA_EVENT_TYPES.FEATURE_UPDATED;
};

export const isDeltaFeatureRemovedEvent = (
event: DeltaEvent,
): event is Extract<DeltaEvent, { type: 'feature-removed' }> => {
return event.type === DELTA_EVENT_TYPES.FEATURE_REMOVED;
};

export const isDeltaSegmentUpdatedEvent = (
event: DeltaEvent,
): event is Extract<DeltaEvent, { type: 'segment-updated' }> => {
return event.type === DELTA_EVENT_TYPES.SEGMENT_UPDATED;
};

export const isDeltaSegmentRemovedEvent = (
event: DeltaEvent,
): event is Extract<DeltaEvent, { type: 'segment-removed' }> => {
return event.type === DELTA_EVENT_TYPES.SEGMENT_REMOVED;
};
Loading

0 comments on commit 280710f

Please sign in to comment.