Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: combine services enterprise #6307

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

feat: combine services enterprise #6307

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 9 additions & 3 deletions src/lib/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ export default async function getApp(
services: IUnleashServices,
unleashSession?: RequestHandler,
db?: Knex,
): Promise<Application> {
): Promise<{ app: Application; services: unknown; stores: unknown }> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have these as parametric types? Something like <EServices extends IUnleashServices, EStores extends IUnleashStores>

Suggested change
): Promise<{ app: Application; services: unknown; stores: unknown }> {
): Promise<{ app: Application; services: EServices; stores: EStores }> {

let combinedServices = services;
let combinedStores = stores;
Comment on lines +38 to +40
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Getting worse: Complex Method
getApp already has high cyclomatic complexity, and now it increases in Lines of Code from 158 to 163

Why does this problem occur?

This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring. Read more.

To ignore this warning click here.

const app = express();

const baseUriPath = config.server.baseUriPath || '';
Expand Down Expand Up @@ -179,7 +181,11 @@ export default async function getApp(
);

if (typeof config.preRouterHook === 'function') {
config.preRouterHook(app, config, services, stores, db);
const { services: enterpriseServices, stores: enterpriseStores } =
config.preRouterHook(app, config, services, stores, db);

combinedServices = enterpriseServices;
combinedStores = enterpriseStores;
Comment on lines +184 to +188
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to do this? I think the "beauty" of the getApp method is that it just return an app, and the preHook just modifies it

If we have to do that, I wouldn't call them enterpriseServices, I'd do something like:

const { services: extendedServices, stores: extendedStores } =
            config.preRouterHook(app, config, services, stores, db);

}

// Setup API routes
Expand Down Expand Up @@ -212,5 +218,5 @@ export default async function getApp(
res.send(indexHTML);
});

return app;
return { app, stores: combinedStores, services: combinedServices };
}
2 changes: 1 addition & 1 deletion src/lib/features/client-feature-toggles/tests/client-feature-toggle.e2e.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ async function getSetup() {
});
const services = createServices(stores, config);

app = await getApp(config, stores, services);
const { app } = await getApp(config, stores, services);

return {
base,
Expand Down
2 changes: 1 addition & 1 deletion src/lib/features/metrics/instance/metrics.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ async function getSetup(opts?: IUnleashOptions) {
db = await dbInit('metrics', config.getLogger);

const services = createServices(db.stores, config, db.rawDatabase);
const app = await getApp(config, db.stores, services);
const { app } = await getApp(config, db.stores, services);
return {
request: supertest(app),
stores: db.stores,
Expand Down
2 changes: 1 addition & 1 deletion src/lib/features/metrics/instance/register.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ async function getSetup() {
const stores = createStores();
const config = createTestConfig();
const services = createServices(stores, config);
const app = await getApp(config, stores, services);
const { app } = await getApp(config, stores, services);

return {
request: supertest(app),
Expand Down
2 changes: 1 addition & 1 deletion src/lib/features/playground/playground.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ async function getSetup() {
experimental: { flags: { strictSchemaValidation: true } },
});
const services = createServices(stores, config);
const app = await getApp(config, stores, services);
const { app } = await getApp(config, stores, services);
return { base, request: supertest(app) };
}
describe('toggle generator', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/middleware/oss-authentication.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ async function getSetup(preRouterHook) {
const stores = createStores();
const services = createServices(stores, config);
const unleashSession = sessionDb(config, {} as Knex);
const app = await getApp(config, stores, services, unleashSession);
const { app } = await getApp(config, stores, services, unleashSession);

return {
base,
Expand Down
2 changes: 1 addition & 1 deletion src/lib/routes/admin-api/api-token.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ async function getSetup(adminTokenKillSwitchEnabled: boolean) {
enabled: true,
});
const services = createServices(stores, config);
const app = await getApp(config, stores, services);
const { app } = await getApp(config, stores, services);

return {
base,
Expand Down
2 changes: 1 addition & 1 deletion src/lib/routes/admin-api/config.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ async function getSetup() {
const stores = createStores();
const services = createServices(stores, config);

const app = await getApp(config, stores, services);
const { app } = await getApp(config, stores, services);

return {
base,
Expand Down
2 changes: 1 addition & 1 deletion src/lib/routes/admin-api/context.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ async function getSetup() {
const stores = createStores();

const services = createServices(stores, config);
const app = await getApp(config, stores, services);
const { app } = await getApp(config, stores, services);

return {
base,
Expand Down
2 changes: 1 addition & 1 deletion src/lib/routes/admin-api/email.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ async function getSetup() {
});

const services = createServices(stores, config);
const app = await getApp(config, stores, services);
const { app } = await getApp(config, stores, services);

return {
base,
Expand Down
2 changes: 1 addition & 1 deletion src/lib/routes/admin-api/events.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ async function getSetup(anonymise: boolean = false) {
experimental: { flags: { anonymiseEventLog: anonymise } },
});
const services = createServices(stores, config);
const app = await getApp(config, stores, services);
const { app } = await getApp(config, stores, services);

return {
base,
Expand Down
2 changes: 1 addition & 1 deletion src/lib/routes/admin-api/metrics.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ async function getSetup() {
preRouterHook: perms.hook,
});
const services = createServices(stores, config);
const app = await getApp(config, stores, services);
const { app } = await getApp(config, stores, services);

return {
request: supertest(app),
Expand Down
2 changes: 1 addition & 1 deletion src/lib/routes/admin-api/public-signup.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe('Public Signup API', () => {
};

const services = createServices(stores, config);
const app = await getApp(config, stores, services);
const { app } = await getApp(config, stores, services);

await stores.roleStore.create({
name: RoleName.VIEWER,
Expand Down
2 changes: 1 addition & 1 deletion src/lib/routes/admin-api/strategy.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ async function getSetup() {
preRouterHook: perms.hook,
});
const services = createServices(stores, config);
const app = await getApp(config, stores, services);
const { app } = await getApp(config, stores, services);

return {
base: randomBase,
Expand Down
2 changes: 1 addition & 1 deletion src/lib/routes/admin-api/tag.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ async function getSetup() {
preRouterHook: perms.hook,
});
const services = createServices(stores, config);
const app = await getApp(config, stores, services);
const { app } = await getApp(config, stores, services);

return {
base,
Expand Down
2 changes: 1 addition & 1 deletion src/lib/routes/admin-api/user/user.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ async function getSetup() {
server: { baseUriPath: base },
});
const services = createServices(stores, config);
const app = await getApp(config, stores, services);
const { app } = await getApp(config, stores, services);
return {
base,
userStore: stores.userStore,
Expand Down
2 changes: 1 addition & 1 deletion src/lib/routes/backstage.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ test('should enable prometheus', async () => {
const config = createTestConfig();
const services = createServices(stores, config);

const app = await getApp(config, stores, services);
const { app } = await getApp(config, stores, services);

const request = supertest(app);

Expand Down
2 changes: 1 addition & 1 deletion src/lib/routes/health-check.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ async function getSetup() {
const stores = createStores();
const config = createTestConfig();
const services = createServices(stores, config);
const app = await getApp(config, stores, services);
const { app } = await getApp(config, stores, services);

return {
request: supertest(app),
Expand Down
2 changes: 1 addition & 1 deletion src/lib/routes/public-invite.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ describe('Public Signup API', () => {
};

const services = createServices(stores, config);
const app = await getApp(config, stores, services);
const { app } = await getApp(config, stores, services);

await stores.roleStore.create({
name: RoleName.VIEWER,
Expand Down
11 changes: 8 additions & 3 deletions src/lib/server-impl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
RoleName,
CustomAuthHandler,
SYSTEM_USER,
IUnleashStores,
} from './types';

import User, { IUser } from './types/user';
Expand Down Expand Up @@ -68,7 +69,11 @@ async function createApp(
const secret = await stores.settingStore.get<string>('unleash.secret');
config.server.secret = secret!;
}
const app = await getApp(config, stores, services, unleashSession, db);
const {
app,
services: combinedServices,
stores: combinedStores,
} = await getApp(config, stores, services, unleashSession, db);
Comment on lines +72 to +76
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Getting worse: Complex Method
createApp already has high cyclomatic complexity, and now it increases in Lines of Code from 86 to 90

Why does this problem occur?

This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring. Read more.

To ignore this warning click here.


await metricsMonitor.startMonitoring(
config,
Expand All @@ -80,9 +85,9 @@ async function createApp(
db,
);
const unleash: Omit<IUnleash, 'stop'> = {
stores,
stores: combinedStores as IUnleashStores,
eventBus: config.eventBus,
services,
services: combinedServices as IUnleashServices,
app,
config,
version: serverVersion,
Expand Down
2 changes: 1 addition & 1 deletion src/test/e2e/helpers/test-helper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ async function createApp(
const unleashSession = sessionDb(config, undefined);
const emitter = new EventEmitter();
emitter.setMaxListeners(0);
const app = await getApp(config, stores, services, unleashSession, db);
const { app } = await getApp(config, stores, services, unleashSession, db);
const request = supertest.agent(app);

const destroy = async () => {
Expand Down