Skip to content

Commit 4e19460

Browse files
authored
Extract benchmark feature guard as middleware (#2094)
Centralizes benchmark access validation for better extensibility. Part of OPS-3873.
1 parent 486deaf commit 4e19460

3 files changed

Lines changed: 69 additions & 21 deletions

File tree

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
import { AppSystemProp, logger, system } from '@openops/server-shared';
2+
import { preHandlerAsyncHookHandler } from 'fastify';
23
import { throwFeatureDisabledError } from './errors';
34

4-
export async function assertBenchmarkFeatureEnabled(
5-
projectId: string,
6-
provider?: string,
7-
): Promise<void> {
5+
export const assertBenchmarkFeatureEnabled: preHandlerAsyncHookHandler = async (
6+
request,
7+
) => {
88
if (system.getBoolean(AppSystemProp.FINOPS_BENCHMARK_ENABLED) !== true) {
99
logger.info(
1010
'Benchmark access denied: FINOPS_BENCHMARK_ENABLED flag is not enabled',
11-
{ provider, projectId },
11+
{ projectId: request.principal.projectId },
1212
);
1313
throwFeatureDisabledError('Benchmark feature is not enabled');
1414
}
15-
}
15+
};

packages/server/api/src/app/benchmark/benchmark.controller.ts

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,12 @@ import { createBenchmark } from './create-benchmark.service';
1919
import { resolveWizardNavigation } from './wizard.service';
2020

2121
export const benchmarkController: FastifyPluginAsyncTypebox = async (app) => {
22+
app.addHook('preHandler', assertBenchmarkFeatureEnabled);
23+
2224
app.post(
2325
'/:provider/wizard',
2426
WizardStepRequestOptions,
2527
async (request, reply) => {
26-
await assertBenchmarkFeatureEnabled(
27-
request.principal.projectId,
28-
request.params.provider,
29-
);
30-
3128
const step = await resolveWizardNavigation(
3229
request.params.provider,
3330
{
@@ -44,11 +41,6 @@ export const benchmarkController: FastifyPluginAsyncTypebox = async (app) => {
4441
'/:provider',
4542
CreateBenchmarkRequestOptions,
4643
async (request, reply) => {
47-
await assertBenchmarkFeatureEnabled(
48-
request.principal.projectId,
49-
request.params.provider,
50-
);
51-
5244
const result = await createBenchmark({
5345
provider: request.params.provider,
5446
projectId: request.principal.projectId,
@@ -59,10 +51,6 @@ export const benchmarkController: FastifyPluginAsyncTypebox = async (app) => {
5951
},
6052
);
6153
app.get('/', ListBenchmarksRequestOptions, async (request, reply) => {
62-
await assertBenchmarkFeatureEnabled(
63-
request.principal.projectId,
64-
request.query.provider,
65-
);
6654
const items = await listBenchmarks({
6755
projectId: request.principal.projectId,
6856
provider: request.query.provider,
@@ -74,7 +62,6 @@ export const benchmarkController: FastifyPluginAsyncTypebox = async (app) => {
7462
'/:benchmarkId/status',
7563
BenchmarkStatusRequestOptions,
7664
async (request, reply) => {
77-
await assertBenchmarkFeatureEnabled(request.principal.projectId);
7865
const status = await getBenchmarkStatus({
7966
benchmarkId: request.params.benchmarkId,
8067
projectId: request.principal.projectId,
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
jest.mock('@openops/server-shared', () => ({
2+
system: {
3+
getBoolean: jest.fn(),
4+
},
5+
AppSystemProp: {
6+
FINOPS_BENCHMARK_ENABLED: 'FINOPS_BENCHMARK_ENABLED',
7+
},
8+
logger: {
9+
info: jest.fn(),
10+
},
11+
}));
12+
13+
import { logger, system } from '@openops/server-shared';
14+
import { FastifyInstance, FastifyReply, FastifyRequest } from 'fastify';
15+
16+
const mockSystem = system as jest.Mocked<typeof system>;
17+
18+
import { assertBenchmarkFeatureEnabled } from '../../../src/app/benchmark/benchmark-feature-guard';
19+
20+
const mockFastifyInstance = {} as FastifyInstance;
21+
22+
const mockRequest = (projectId: string) =>
23+
({ principal: { projectId } } as unknown as FastifyRequest);
24+
25+
const mockReply = {} as FastifyReply;
26+
27+
const callHook = (projectId: string) =>
28+
assertBenchmarkFeatureEnabled.call(
29+
mockFastifyInstance,
30+
mockRequest(projectId),
31+
mockReply,
32+
);
33+
34+
describe('assertBenchmarkFeatureEnabled', () => {
35+
const projectId = 'project-id';
36+
37+
beforeEach(() => {
38+
jest.clearAllMocks();
39+
});
40+
41+
it('should throw error if FINOPS_BENCHMARK_ENABLED is not enabled', async () => {
42+
mockSystem.getBoolean.mockReturnValue(false);
43+
44+
await expect(callHook(projectId)).rejects.toThrow(
45+
expect.objectContaining({
46+
message: 'FEATURE_DISABLED: Benchmark feature is not enabled',
47+
}),
48+
);
49+
50+
expect(logger.info).toHaveBeenCalledWith(
51+
'Benchmark access denied: FINOPS_BENCHMARK_ENABLED flag is not enabled',
52+
{ projectId },
53+
);
54+
});
55+
56+
it('should pass when FINOPS_BENCHMARK_ENABLED is true', async () => {
57+
mockSystem.getBoolean.mockReturnValue(true);
58+
59+
await expect(callHook(projectId)).resolves.toBeUndefined();
60+
});
61+
});

0 commit comments

Comments
 (0)