Refactor benchmark feature guard to use preHandler hook

Replace per-handler assertBenchmarkFeatureEnabled calls with a single
app.addHook('preHandler', ...) registered at the plugin scope, following
the established hook pattern used across the codebase.

Made-with: Cursor

Author: cezudas

packages/server/api/src/app/benchmark/benchmark-feature-guard-factory.ts

@@ -1,8 +1,6 @@
-import {
-  benchmarkFeatureGuard,
-  BenchmarkFeatureGuard,
-} from './benchmark-feature-guard';
+import { preHandlerAsyncHookHandler } from 'fastify';
+import { assertBenchmarkFeatureEnabled } from './benchmark-feature-guard';
 
-export const getBenchmarkFeatureGuard = (): BenchmarkFeatureGuard => {
-  return benchmarkFeatureGuard;
+export const getBenchmarkFeatureGuard = (): preHandlerAsyncHookHandler => {
+  return assertBenchmarkFeatureEnabled;
 };

packages/server/api/src/app/benchmark/benchmark-feature-guard.ts

@@ -1,26 +1,13 @@
+import { preHandlerAsyncHookHandler } from 'fastify';
 import { AppSystemProp, logger, system } from '@openops/server-shared';
 import { throwFeatureDisabledError } from './errors';
 
-export type BenchmarkFeatureGuard = {
-  assertBenchmarkFeatureEnabled(
-    projectId: string,
-    organizationId: string,
-    provider?: string,
-  ): Promise<void>;
-};
-
-export const benchmarkFeatureGuard: BenchmarkFeatureGuard = {
-  async assertBenchmarkFeatureEnabled(
-    projectId: string,
-    _organizationId: string,
-    provider?: string,
-  ): Promise<void> {
-    if (system.getBoolean(AppSystemProp.FINOPS_BENCHMARK_ENABLED) !== true) {
-      logger.info(
-        'Benchmark access denied: FINOPS_BENCHMARK_ENABLED flag is not enabled',
-        { provider, projectId },
-      );
-      throwFeatureDisabledError('Benchmark feature is not enabled');
-    }
-  },
+export const assertBenchmarkFeatureEnabled: preHandlerAsyncHookHandler = async (request) => {
+  if (system.getBoolean(AppSystemProp.FINOPS_BENCHMARK_ENABLED) !== true) {
+    logger.info(
+      'Benchmark access denied: FINOPS_BENCHMARK_ENABLED flag is not enabled',
+      { projectId: request.principal.projectId },
+    );
+    throwFeatureDisabledError('Benchmark feature is not enabled');
+  }
 };

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

@@ -19,16 +19,12 @@ import { createBenchmark } from './create-benchmark.service';
 import { resolveWizardNavigation } from './wizard.service';
 
 export const benchmarkController: FastifyPluginAsyncTypebox = async (app) => {
+  app.addHook('preHandler', getBenchmarkFeatureGuard());
+
   app.post(
     '/:provider/wizard',
     WizardStepRequestOptions,
     async (request, reply) => {
-      await getBenchmarkFeatureGuard().assertBenchmarkFeatureEnabled(
-        request.principal.projectId,
-        request.principal.organization.id,
-        request.params.provider,
-      );
-
       const step = await resolveWizardNavigation(
         request.params.provider,
         {
@@ -45,12 +41,6 @@ export const benchmarkController: FastifyPluginAsyncTypebox = async (app) => {
     '/:provider',
     CreateBenchmarkRequestOptions,
     async (request, reply) => {
-      await getBenchmarkFeatureGuard().assertBenchmarkFeatureEnabled(
-        request.principal.projectId,
-        request.principal.organization.id,
-        request.params.provider,
-      );
-
       const result = await createBenchmark({
         provider: request.params.provider,
         projectId: request.principal.projectId,
@@ -61,11 +51,6 @@ export const benchmarkController: FastifyPluginAsyncTypebox = async (app) => {
     },
   );
   app.get('/', ListBenchmarksRequestOptions, async (request, reply) => {
-    await getBenchmarkFeatureGuard().assertBenchmarkFeatureEnabled(
-      request.principal.projectId,
-      request.principal.organization.id,
-      request.query.provider,
-    );
     const items = await listBenchmarks({
       projectId: request.principal.projectId,
       provider: request.query.provider,
@@ -77,10 +62,6 @@ export const benchmarkController: FastifyPluginAsyncTypebox = async (app) => {
     '/:benchmarkId/status',
     BenchmarkStatusRequestOptions,
     async (request, reply) => {
-      await getBenchmarkFeatureGuard().assertBenchmarkFeatureEnabled(
-        request.principal.projectId,
-        request.principal.organization.id,
-      );
       const status = await getBenchmarkStatus({
         benchmarkId: request.params.benchmarkId,
         projectId: request.principal.projectId,

packages/server/api/test/unit/benchmark/benchmark-feature-guard.test.ts

@@ -11,14 +11,24 @@ jest.mock('@openops/server-shared', () => ({
 }));
 
 import { logger, system } from '@openops/server-shared';
+import { FastifyInstance, FastifyReply, FastifyRequest } from 'fastify';
 
 const mockSystem = system as jest.Mocked<typeof system>;
 
-import { benchmarkFeatureGuard } from '../../../src/app/benchmark/benchmark-feature-guard';
+import { assertBenchmarkFeatureEnabled } from '../../../src/app/benchmark/benchmark-feature-guard';
 
-describe('benchmarkFeatureGuard', () => {
+const mockFastifyInstance = {} as FastifyInstance;
+
+const mockRequest = (projectId: string) =>
+  ({ principal: { projectId } }) as unknown as FastifyRequest;
+
+const mockReply = {} as FastifyReply;
+
+const callHook = (projectId: string) =>
+  assertBenchmarkFeatureEnabled.call(mockFastifyInstance, mockRequest(projectId), mockReply);
+
+describe('assertBenchmarkFeatureEnabled', () => {
   const projectId = 'project-id';
-  const organizationId = 'org-id';
 
   beforeEach(() => {
     jest.clearAllMocks();
@@ -27,31 +37,21 @@ describe('benchmarkFeatureGuard', () => {
   it('should throw error if FINOPS_BENCHMARK_ENABLED is not enabled', async () => {
     mockSystem.getBoolean.mockReturnValue(false);
 
-    await expect(
-      benchmarkFeatureGuard.assertBenchmarkFeatureEnabled(
-        projectId,
-        organizationId,
-      ),
-    ).rejects.toThrow(
+    await expect(callHook(projectId)).rejects.toThrow(
       expect.objectContaining({
         message: 'FEATURE_DISABLED: Benchmark feature is not enabled',
       }),
     );
 
     expect(logger.info).toHaveBeenCalledWith(
       'Benchmark access denied: FINOPS_BENCHMARK_ENABLED flag is not enabled',
-      { provider: undefined, projectId },
+      { projectId },
     );
   });
 
   it('should pass when FINOPS_BENCHMARK_ENABLED is true', async () => {
     mockSystem.getBoolean.mockReturnValue(true);
 
-    await expect(
-      benchmarkFeatureGuard.assertBenchmarkFeatureEnabled(
-        projectId,
-        organizationId,
-      ),
-    ).resolves.toBeUndefined();
+    await expect(callHook(projectId)).resolves.toBeUndefined();
   });
 });