Skip to content

Commit

Permalink
Refactor/separate client and admin store (#5006)
Browse files Browse the repository at this point in the history
This PR is the first step in separating the client and admin stores.
Currently our feature toggle services uses the client store to serve
multiple purposes. 

Admin API uses the feature toggle service to serve both the feature
toggle list and playground features, while the client API uses the
feature toggle service to serve client features. The admin API can
change often and have very different requirements than the client API,
which changes infrequently and generally keeps the same stable structure
for long periods of time. This architecture is error prone, because when
you need to make changes to the admin API, you can very easily affect
the client API.

I aim to put up a stone wall between the two APIs. Complete separation
between the two APIs, at the cost of some duplication.

In this PR I have created a feature oriented architecture for client
features and disconnected the client API from the feature toggle
service. It now goes through it's own service to it's own store. For
feature toggle service I have duplicated and replaced the functionality
that serves /api/admin/features, I have kept a lot of the ugliness in
the code and haven't removed anything in order to avoid breaking
changes.

Next steps: 
* Move playground to admin API
* Remove client-feature-toggle-store from feature-toggle-service
  • Loading branch information
FredrikOseberg committed Oct 12, 2023
1 parent 7b7a2a7 commit f34d187
Show file tree
Hide file tree
Showing 28 changed files with 627 additions and 136 deletions.
2 changes: 2 additions & 0 deletions src/lib/__snapshots__/create-config.test.ts.snap
Expand Up @@ -110,6 +110,7 @@ exports[`should create default config 1`] = `
"privateProjects": false,
"proPlanAutoCharge": false,
"responseTimeWithAppNameKillSwitch": false,
"separateAdminClientApi": false,
"strictSchemaValidation": false,
"transactionalDecorator": false,
"useLastSeenRefactor": false,
Expand Down Expand Up @@ -153,6 +154,7 @@ exports[`should create default config 1`] = `
"privateProjects": false,
"proPlanAutoCharge": false,
"responseTimeWithAppNameKillSwitch": false,
"separateAdminClientApi": false,
"strictSchemaValidation": false,
"transactionalDecorator": false,
"useLastSeenRefactor": false,
Expand Down
4 changes: 2 additions & 2 deletions src/lib/db/index.ts
Expand Up @@ -19,7 +19,7 @@ import { AccessStore } from './access-store';
import { ResetTokenStore } from './reset-token-store';
import UserFeedbackStore from './user-feedback-store';
import FeatureStrategyStore from '../features/feature-toggle/feature-toggle-strategies-store';
import FeatureToggleClientStore from './feature-toggle-client-store';
import FeatureToggleClientStore from '../features/client-feature-toggles/client-feature-toggle-store';
import EnvironmentStore from './environment-store';
import FeatureTagStore from './feature-tag-store';
import { FeatureEnvironmentStore } from './feature-environment-store';
Expand Down Expand Up @@ -91,7 +91,7 @@ export const createStores = (
getLogger,
config.flagResolver,
),
featureToggleClientStore: new FeatureToggleClientStore(
clientFeatureToggleStore: new FeatureToggleClientStore(
db,
eventBus,
getLogger,
Expand Down
@@ -0,0 +1,61 @@
import {
IFeatureNaming,
IFeatureToggleClientStore,
IFeatureToggleQuery,
IUnleashConfig,
IUnleashStores,
} from '../../types';

import { Logger } from '../../logger';

import { FeatureConfigurationClient } from '../feature-toggle/types/feature-toggle-strategies-store-type';

export class ClientFeatureToggleService {
private logger: Logger;

private clientFeatureToggleStore: IFeatureToggleClientStore;

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

async getClientFeatures(
query?: IFeatureToggleQuery,
): Promise<FeatureConfigurationClient[]> {
const result = await this.clientFeatureToggleStore.getClient(
query || {},
);

return result.map(
({
name,
type,
enabled,
project,
stale,
strategies,
variants,
description,
impressionData,
dependencies,
}) => ({
name,
type,
enabled,
project,
stale,
strategies,
variants,
description,
impressionData,
dependencies,
}),
);
}
}
@@ -1,7 +1,7 @@
import { Knex } from 'knex';
import metricsHelper from '../util/metrics-helper';
import { DB_TIME } from '../metric-events';
import { Logger, LogProvider } from '../logger';
import metricsHelper from '../../util/metrics-helper';
import { DB_TIME } from '../../metric-events';
import { Logger, LogProvider } from '../../logger';
import {
IFeatureToggleClient,
IFeatureToggleClientStore,
Expand All @@ -10,11 +10,11 @@ import {
IStrategyConfig,
ITag,
PartialDeep,
} from '../types';
import { DEFAULT_ENV, ensureStringValue, mapValues } from '../util';
} from '../../types';
import { DEFAULT_ENV, ensureStringValue, mapValues } from '../../util';
import EventEmitter from 'events';
import FeatureToggleStore from '../features/feature-toggle/feature-toggle-store';
import { Db } from './db';
import FeatureToggleStore from '../feature-toggle/feature-toggle-store';
import { Db } from '../../db/db';
import Raw = Knex.Raw;

export interface IGetAllFeatures {
Expand Down
Expand Up @@ -2,17 +2,23 @@ import memoizee from 'memoizee';
import { Response } from 'express';
// eslint-disable-next-line import/no-extraneous-dependencies
import hashSum from 'hash-sum';
import Controller from '../controller';
import { IClientSegment, IUnleashConfig, IUnleashServices } from '../../types';
import FeatureToggleService from '../../features/feature-toggle/feature-toggle-service';
import Controller from '../../routes/controller';
import {
IClientSegment,
IFeatureToggleStore,
IFlagResolver,
IUnleashConfig,
IUnleashServices,
} from '../../types';
import FeatureToggleService from '../feature-toggle/feature-toggle-service';
import { Logger } from '../../logger';
import { querySchema } from '../../schema/feature-schema';
import { IFeatureToggleQuery } from '../../types/model';
import NotFoundError from '../../error/notfound-error';
import { IAuthRequest } from '../unleash-types';
import { IAuthRequest } from '../../routes/unleash-types';
import ApiUser from '../../types/api-user';
import { ALL, isAllProjects } from '../../types/models/api-token';
import { FeatureConfigurationClient } from '../../features/feature-toggle/types/feature-toggle-strategies-store-type';
import { FeatureConfigurationClient } from '../feature-toggle/types/feature-toggle-strategies-store-type';
import { ClientSpecService } from '../../services/client-spec-service';
import { OpenApiService } from '../../services/openapi-service';
import { NONE } from '../../types/permissions';
Expand All @@ -27,7 +33,8 @@ import {
ClientFeaturesSchema,
} from '../../openapi/spec/client-features-schema';
import { ISegmentService } from '../../segments/segment-service-interface';
import ConfigurationRevisionService from '../../features/feature-toggle/configuration-revision-service';
import ConfigurationRevisionService from '../feature-toggle/configuration-revision-service';
import { ClientFeatureToggleService } from './client-feature-toggle-service';

const version = 2;

Expand All @@ -45,7 +52,7 @@ interface IMeta {
export default class FeatureController extends Controller {
private readonly logger: Logger;

private featureToggleServiceV2: FeatureToggleService;
private clientFeatureToggleService: ClientFeatureToggleService;

private segmentService: ISegmentService;

Expand All @@ -55,35 +62,43 @@ export default class FeatureController extends Controller {

private configurationRevisionService: ConfigurationRevisionService;

private featureToggleService: FeatureToggleService;

private flagResolver: IFlagResolver;

private featuresAndSegments: (
query: IFeatureToggleQuery,
etag: string,
) => Promise<[FeatureConfigurationClient[], IClientSegment[]]>;

constructor(
{
featureToggleServiceV2,
clientFeatureToggleService,
segmentService,
clientSpecService,
openApiService,
configurationRevisionService,
featureToggleService,
}: Pick<
IUnleashServices,
| 'featureToggleServiceV2'
| 'clientFeatureToggleService'
| 'segmentService'
| 'clientSpecService'
| 'openApiService'
| 'configurationRevisionService'
| 'featureToggleService'
>,
config: IUnleashConfig,
) {
super(config);
const { clientFeatureCaching } = config;
this.featureToggleServiceV2 = featureToggleServiceV2;
this.clientFeatureToggleService = clientFeatureToggleService;
this.segmentService = segmentService;
this.clientSpecService = clientSpecService;
this.openApiService = openApiService;
this.configurationRevisionService = configurationRevisionService;
this.featureToggleService = featureToggleService;
this.flagResolver = config.flagResolver;
this.logger = config.getLogger('client-api/feature.js');

this.route({
Expand Down Expand Up @@ -146,8 +161,15 @@ export default class FeatureController extends Controller {
private async resolveFeaturesAndSegments(
query?: IFeatureToggleQuery,
): Promise<[FeatureConfigurationClient[], IClientSegment[]]> {
if (this.flagResolver.isEnabled('separateAdminClientApi')) {
return Promise.all([
this.clientFeatureToggleService.getClientFeatures(query),
this.segmentService.getActiveForClient(),
]);
}

return Promise.all([
this.featureToggleServiceV2.getClientFeatures(query),
this.featureToggleService.getClientFeatures(query),
this.segmentService.getActiveForClient(),
]);
}
Expand Down Expand Up @@ -287,7 +309,15 @@ export default class FeatureController extends Controller {
const name = req.params.featureName;
const featureQuery = await this.resolveQuery(req);
const q = { ...featureQuery, namePrefix: name };
const toggles = await this.featureToggleServiceV2.getClientFeatures(q);

let toggles = await this.featureToggleService.getClientFeatures(q);

if (this.flagResolver.isEnabled('separateAdminClientApi')) {
toggles = await this.clientFeatureToggleService.getClientFeatures(
q,
);
}

const toggle = toggles.find((t) => t.name === name);
if (!toggle) {
throw new NotFoundError(`Could not find feature toggle ${name}`);
Expand Down
@@ -0,0 +1,45 @@
import FeatureToggleClientStore from '../client-feature-toggles/client-feature-toggle-store';
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';

export const createClientFeatureToggleService = (
db: Db,
config: IUnleashConfig,
): ClientFeatureToggleService => {
const { getLogger, eventBus, flagResolver } = config;

const featureToggleClientStore = new FeatureToggleClientStore(
db,
eventBus,
getLogger,
flagResolver,
);

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

return clientFeatureToggleService;
};

export const createFakeClientFeatureToggleService = (
config: IUnleashConfig,
): ClientFeatureToggleService => {
const { getLogger, flagResolver } = config;

const fakeClientFeatureToggleStore = new FakeClientFeatureToggleStore();

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

return clientFeatureToggleService;
};
Expand Up @@ -2,11 +2,11 @@ import {
FeatureToggle,
IFeatureToggleClient,
IFeatureToggleQuery,
} from '../../lib/types/model';
import { IFeatureToggleClientStore } from '../../lib/types/stores/feature-toggle-client-store';
import { IGetAdminFeatures } from '../../lib/db/feature-toggle-client-store';
} from '../../../types/model';
import { IFeatureToggleClientStore } from '../types/client-feature-toggle-store-type';
import { IGetAdminFeatures } from '../client-feature-toggle-store';

export default class FakeFeatureToggleClientStore
export default class FakeClientFeatureToggleStore
implements IFeatureToggleClientStore
{
featureToggles: FeatureToggle[] = [];
Expand Down Expand Up @@ -34,6 +34,7 @@ export default class FakeFeatureToggleClientStore
}
return toggle.archived === archived;
});

const clientRows: IFeatureToggleClient[] = rows.map((t) => ({
...t,
enabled: true,
Expand Down Expand Up @@ -81,6 +82,7 @@ export default class FakeFeatureToggleClientStore
archived: false,
...feature,
});

return Promise.resolve();
}
}

0 comments on commit f34d187

Please sign in to comment.