Skip to content

Commit

Permalink
refactor: Switch client feature toggles to segment read model (#6425)
Browse files Browse the repository at this point in the history
  • Loading branch information
kwasniew authored Mar 5, 2024
1 parent 6236184 commit 454f44d
Show file tree
Hide file tree
Showing 9 changed files with 32 additions and 75 deletions.
31 changes: 1 addition & 30 deletions src/lib/db/segment-store.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
import { ISegmentStore } from '../types/stores/segment-store';
import {
IClientSegment,
IConstraint,
IFeatureStrategySegment,
ISegment,
} from '../types/model';
import { IConstraint, IFeatureStrategySegment, ISegment } from '../types/model';
import { Logger, LogProvider } from '../logger';
import EventEmitter from 'events';
import NotFoundError from '../error/notfound-error';
Expand Down Expand Up @@ -284,30 +279,6 @@ export default class SegmentStore implements ISegmentStore {
);
}

async getActive(): Promise<ISegment[]> {
const rows: ISegmentRow[] = await this.db
.distinct(this.prefixColumns())
.from(T.segments)
.orderBy('name', 'asc')
.join(
T.featureStrategySegment,
`${T.featureStrategySegment}.segment_id`,
`${T.segments}.id`,
);

return rows.map(this.mapRow);
}

async getActiveForClient(): Promise<IClientSegment[]> {
const fullSegments = await this.getActive();

return fullSegments.map((segments) => ({
id: segments.id,
name: segments.name,
constraints: segments.constraints,
}));
}

async getByStrategy(strategyId: string): Promise<ISegment[]> {
const rows = await this.db
.select(this.prefixColumns())
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
IFeatureToggleClientStore,
IFeatureToggleQuery,
ISegmentReadModel,
IUnleashConfig,
IUnleashStores,
} from '../../types';
Expand All @@ -14,16 +15,24 @@ export class ClientFeatureToggleService {

private clientFeatureToggleStore: IFeatureToggleClientStore;

private segmentReadModel: ISegmentReadModel;

constructor(
{
clientFeatureToggleStore,
}: Pick<IUnleashStores, 'clientFeatureToggleStore'>,
segmentReadModel: ISegmentReadModel,
{ getLogger }: Pick<IUnleashConfig, 'getLogger' | 'flagResolver'>,
) {
this.logger = getLogger('services/client-feature-toggle-service.ts');
this.segmentReadModel = segmentReadModel;
this.clientFeatureToggleStore = clientFeatureToggleStore;
}

async getActiveSegmentsForClient() {
return this.segmentReadModel.getActiveForClient();
}

async getClientFeatures(
query?: IFeatureToggleQuery,
): Promise<FeatureConfigurationClient[]> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import {
clientFeaturesSchema,
ClientFeaturesSchema,
} from '../../openapi/spec/client-features-schema';
import { ISegmentService } from '../../segments/segment-service-interface';
import ConfigurationRevisionService from '../feature-toggle/configuration-revision-service';
import { ClientFeatureToggleService } from './client-feature-toggle-service';

Expand All @@ -53,8 +52,6 @@ export default class FeatureController extends Controller {

private clientFeatureToggleService: ClientFeatureToggleService;

private segmentService: ISegmentService;

private clientSpecService: ClientSpecService;

private openApiService: OpenApiService;
Expand All @@ -73,15 +70,13 @@ export default class FeatureController extends Controller {
constructor(
{
clientFeatureToggleService,
segmentService,
clientSpecService,
openApiService,
configurationRevisionService,
featureToggleService,
}: Pick<
IUnleashServices,
| 'clientFeatureToggleService'
| 'segmentService'
| 'clientSpecService'
| 'openApiService'
| 'configurationRevisionService'
Expand All @@ -92,7 +87,6 @@ export default class FeatureController extends Controller {
super(config);
const { clientFeatureCaching } = config;
this.clientFeatureToggleService = clientFeatureToggleService;
this.segmentService = segmentService;
this.clientSpecService = clientSpecService;
this.openApiService = openApiService;
this.configurationRevisionService = configurationRevisionService;
Expand Down Expand Up @@ -162,7 +156,7 @@ export default class FeatureController extends Controller {
): Promise<[FeatureConfigurationClient[], IClientSegment[]]> {
return Promise.all([
this.clientFeatureToggleService.getClientFeatures(query),
this.segmentService.getActiveForClient(),
this.clientFeatureToggleService.getActiveSegmentsForClient(),
]);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { Db } from '../../db/db';
import { IUnleashConfig } from '../../types';
import FakeClientFeatureToggleStore from './fakes/fake-client-feature-toggle-store';
import { ClientFeatureToggleService } from './client-feature-toggle-service';
import { SegmentReadModel } from '../segment/segment-read-model';
import { FakeSegmentReadModel } from '../segment/fake-segment-read-model';

export const createClientFeatureToggleService = (
db: Db,
Expand All @@ -17,10 +19,13 @@ export const createClientFeatureToggleService = (
flagResolver,
);

const segmentReadModel = new SegmentReadModel(db);

const clientFeatureToggleService = new ClientFeatureToggleService(
{
clientFeatureToggleStore: featureToggleClientStore,
},
segmentReadModel,
{ getLogger, flagResolver },
);

Expand All @@ -34,10 +39,13 @@ export const createFakeClientFeatureToggleService = (

const fakeClientFeatureToggleStore = new FakeClientFeatureToggleStore();

const fakeSegmentReadModel = new FakeSegmentReadModel();

const clientFeatureToggleService = new ClientFeatureToggleService(
{
clientFeatureToggleStore: fakeClientFeatureToggleStore,
},
fakeSegmentReadModel,
{ getLogger, flagResolver },
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,16 @@ test('should get empty getFeatures via client', () => {

test('if caching is enabled should memoize', async () => {
const getClientFeatures = jest.fn().mockReturnValue([]);
const getActive = jest.fn().mockReturnValue([]);
const getActiveForClient = jest.fn().mockReturnValue([]);
const getActiveSegmentsForClient = jest.fn().mockReturnValue([]);
const respondWithValidation = jest.fn().mockReturnValue({});
const validPath = jest.fn().mockReturnValue(jest.fn());
const clientSpecService = new ClientSpecService({ getLogger });
const openApiService = { respondWithValidation, validPath };
const clientFeatureToggleService = { getClientFeatures };
const clientFeatureToggleService = {
getClientFeatures,
getActiveSegmentsForClient,
};
const featureToggleService = { getClientFeatures };
const segmentService = { getActive, getActiveForClient };
const configurationRevisionService = { getMaxRevisionId: () => 1 };

const controller = new FeatureController(
Expand All @@ -91,8 +92,6 @@ test('if caching is enabled should memoize', async () => {
// @ts-expect-error due to partial implementation
featureToggleService,
// @ts-expect-error due to partial implementation
segmentService,
// @ts-expect-error due to partial implementation
configurationRevisionService,
},
{
Expand All @@ -112,13 +111,14 @@ test('if caching is enabled should memoize', async () => {

test('if caching is not enabled all calls goes to service', async () => {
const getClientFeatures = jest.fn().mockReturnValue([]);
const getActive = jest.fn().mockReturnValue([]);
const getActiveForClient = jest.fn().mockReturnValue([]);
const getActiveSegmentsForClient = jest.fn().mockReturnValue([]);
const respondWithValidation = jest.fn().mockReturnValue({});
const validPath = jest.fn().mockReturnValue(jest.fn());
const clientSpecService = new ClientSpecService({ getLogger });
const clientFeatureToggleService = { getClientFeatures };
const segmentService = { getActive, getActiveForClient };
const clientFeatureToggleService = {
getClientFeatures,
getActiveSegmentsForClient,
};
const featureToggleService = { getClientFeatures };
const openApiService = { respondWithValidation, validPath };
const configurationRevisionService = { getMaxRevisionId: () => 1 };
Expand All @@ -133,8 +133,6 @@ test('if caching is not enabled all calls goes to service', async () => {
// @ts-expect-error due to partial implementation
featureToggleService,
// @ts-expect-error due to partial implementation
segmentService,
// @ts-expect-error due to partial implementation
configurationRevisionService,
},
{
Expand Down
4 changes: 1 addition & 3 deletions src/lib/segments/segment-service-interface.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { ChangeRequestStrategy } from '../features/change-request-segment-usage-service/change-request-segment-usage-read-model';
import { UpsertSegmentSchema } from '../openapi';
import { IClientSegment, IFeatureStrategy, ISegment, IUser } from '../types';
import { IFeatureStrategy, ISegment, IUser } from '../types';

export type StrategiesUsingSegment = {
strategies: IFeatureStrategy[];
Expand Down Expand Up @@ -33,8 +33,6 @@ export interface ISegmentService {

validateName(name: string): Promise<void>;

getActiveForClient(): Promise<IClientSegment[]>;

getAll(): Promise<ISegment[]>;

create(
Expand Down
5 changes: 0 additions & 5 deletions src/lib/services/segment-service.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { IUnleashConfig } from '../types/option';
import {
IClientSegment,
IFlagResolver,
IUnleashStores,
SKIP_CHANGE_REQUEST,
Expand Down Expand Up @@ -79,10 +78,6 @@ export class SegmentService implements ISegmentService {
return this.segmentStore.getAll(this.config.isEnterprise);
}

async getActiveForClient(): Promise<IClientSegment[]> {
return this.segmentStore.getActiveForClient();
}

async getByStrategy(strategyId: string): Promise<ISegment[]> {
return this.segmentStore.getByStrategy(strategyId);
}
Expand Down
6 changes: 1 addition & 5 deletions src/lib/types/stores/segment-store.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
import { IClientSegment, IFeatureStrategySegment, ISegment } from '../model';
import { IFeatureStrategySegment, ISegment } from '../model';
import { Store } from './store';
import User from '../user';

export interface ISegmentStore extends Store<ISegment, number> {
getAll(includeChangeRequestUsageData?: boolean): Promise<ISegment[]>;

getActive(): Promise<ISegment[]>;

getActiveForClient(): Promise<IClientSegment[]>;

getByStrategy(strategyId: string): Promise<ISegment[]>;

create(
Expand Down
14 changes: 1 addition & 13 deletions src/test/fixtures/fake-segment-store.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
import { ISegmentStore } from '../../lib/types/stores/segment-store';
import {
IClientSegment,
IFeatureStrategySegment,
ISegment,
} from '../../lib/types/model';
import { IFeatureStrategySegment, ISegment } from '../../lib/types/model';

export default class FakeSegmentStore implements ISegmentStore {
count(): Promise<number> {
Expand Down Expand Up @@ -34,14 +30,6 @@ export default class FakeSegmentStore implements ISegmentStore {
return [];
}

async getActive(): Promise<ISegment[]> {
return [];
}

async getActiveForClient(): Promise<IClientSegment[]> {
return [];
}

async getByStrategy(): Promise<ISegment[]> {
return [];
}
Expand Down

0 comments on commit 454f44d

Please sign in to comment.