fix: allow hooks to run only once per interaction (#1243)

Current behaviour will run the beforeEach and afterEach hooks multiple times if there are several provider states defined in an interaction. This is due to the way the hook are run on every call to "/_pactSetup" via the proxy. This change will ensure each of those hooks is run only once per interaction, regardless of how many provider states are defined in that interaction.
pact-foundation · Nov 13, 2024 · b238fd0 · b238fd0
1 parent 7bf180f
commit b238fd0
Show file tree

Hide file tree

Showing 3 changed files with 179 additions and 27 deletions.
diff --git a/src/dsl/verifier/proxy/hooks.spec.ts b/src/dsl/verifier/proxy/hooks.spec.ts
@@ -0,0 +1,121 @@
+import { expect } from 'chai';
+import { stub } from 'sinon';
+import { RequestHandler } from 'express';
+
+import {
+  registerHookStateTracking,
+  registerBeforeHook,
+  registerAfterHook,
+  HooksState,
+} from './hooks';
+
+// This mimics the proxy setup (src/dsl/verifier/proxy/proxy.ts), whereby the
+// state handling middleware is run regardless of whether a hook is registered
+// or not.
+const doRequest = async (
+  action: string,
+  hooksState: HooksState,
+  hookHandler?: RequestHandler
+) => {
+  const hooksStateHandler = registerHookStateTracking(hooksState);
+  const hookRequestHandler = hookHandler || ((req, res, next) => next());
+
+  const request: any = {
+    body: {
+      action,
+    },
+  };
+
+  return new Promise((resolve) => {
+    hooksStateHandler(request, null as any, () => {
+      hookRequestHandler(request, null as any, resolve);
+    });
+  });
+};
+
+describe('Verifier', () => {
+  describe('#registerBeforeHook', () => {
+    describe('when the state setup routine is called multiple times before the next teardown', () => {
+      it('it executes the beforeEach hook only once', async () => {
+        const hooksState: HooksState = { setupCounter: 0 };
+        const hook = stub().resolves();
+        const hookHandler = registerBeforeHook(hook, hooksState);
+
+        await doRequest('setup', hooksState, hookHandler);
+        await doRequest('setup', hooksState, hookHandler);
+        await doRequest('teardown', hooksState);
+        await doRequest('teardown', hooksState);
+
+        expect(hook).to.be.calledOnce;
+      });
+    });
+  });
+
+  describe('#registerAfterHook', () => {
+    describe('when the state teardown routine is called multiple times before the next setup', () => {
+      it('it executes the afterEach hook only once', async () => {
+        const hooksState: HooksState = { setupCounter: 0 };
+        const hook = stub().resolves();
+        const hookHandler = registerAfterHook(hook, hooksState);
+
+        await doRequest('setup', hooksState);
+        await doRequest('setup', hooksState);
+        await doRequest('teardown', hooksState, hookHandler);
+        await doRequest('teardown', hooksState, hookHandler);
+
+        expect(hook).to.be.calledOnce;
+      });
+    });
+  });
+
+  describe('#registerBeforeHook and #registerAfterHook', () => {
+    describe('when the state teardown routine is called multiple times before the next setup', () => {
+      it('it executes the beforeEach and afterEach hooks only once', async () => {
+        const hooksState: HooksState = { setupCounter: 0 };
+        const beforeHook = stub().resolves();
+        const afterHook = stub().resolves();
+        const beforeHookHandler = registerBeforeHook(beforeHook, hooksState);
+        const afterHookHandler = registerAfterHook(afterHook, hooksState);
+
+        await doRequest('setup', hooksState, beforeHookHandler);
+        await doRequest('setup', hooksState, beforeHookHandler);
+        await doRequest('teardown', hooksState, afterHookHandler);
+        await doRequest('teardown', hooksState, afterHookHandler);
+
+        expect(beforeHook).to.be.calledOnce;
+        expect(afterHook).to.be.calledOnce;
+      });
+    });
+
+    describe('when multiple interactions are executed', () => {
+      it('it executes the beforeEach and afterEach hooks once for each interaction', async () => {
+        const hooksState: HooksState = { setupCounter: 0 };
+        const beforeHook = stub().resolves();
+        const afterHook = stub().resolves();
+        const beforeHookHandler = registerBeforeHook(beforeHook, hooksState);
+        const afterHookHandler = registerAfterHook(afterHook, hooksState);
+
+        // Interaction 1 (two "given" states)
+        await doRequest('setup', hooksState, beforeHookHandler);
+        await doRequest('setup', hooksState, beforeHookHandler);
+        await doRequest('teardown', hooksState, afterHookHandler);
+        await doRequest('teardown', hooksState, afterHookHandler);
+
+        // Interaction 2 (one "given" state)
+        await doRequest('setup', hooksState, beforeHookHandler);
+        await doRequest('teardown', hooksState, afterHookHandler);
+
+        // Interaction 3 (three "given" states)
+        await doRequest('setup', hooksState, beforeHookHandler);
+        await doRequest('setup', hooksState, beforeHookHandler);
+        await doRequest('setup', hooksState, beforeHookHandler);
+        await doRequest('teardown', hooksState, afterHookHandler);
+        await doRequest('teardown', hooksState, afterHookHandler);
+        await doRequest('teardown', hooksState, afterHookHandler);
+
+        expect(beforeHook).to.be.calledThrice;
+        expect(afterHook).to.be.calledThrice;
+      });
+    });
+  });
+});
diff --git a/src/dsl/verifier/proxy/hooks.ts b/src/dsl/verifier/proxy/hooks.ts
@@ -1,19 +1,39 @@
-import express from 'express';
+/* eslint-disable no-param-reassign */
+/**
+ * These handlers assume that the number of "setup" and "teardown" requests to
+ * `/_pactSetup` are always sequential and balanced, i.e. if 3 "setup" actions
+ * are received prior to an interaction being executed, then 3 "teardown"
+ * actions will be received after that interaction has ended.
+ */
+import { RequestHandler } from 'express';
 
 import logger from '../../../common/logger';
-import { ProxyOptions } from './types';
+import { Hook } from './types';
 
-export const registerBeforeHook = (
-  app: express.Express,
-  config: ProxyOptions,
-  stateSetupPath: string
-): void => {
-  if (config.beforeEach) logger.trace("registered 'beforeEach' hook");
-  app.use(async (req, res, next) => {
-    if (req.path === stateSetupPath && config.beforeEach) {
+export type HooksState = {
+  setupCounter: number;
+};
+
+export const registerHookStateTracking =
+  (hooksState: HooksState): RequestHandler =>
+  async ({ body }, res, next) => {
+    if (body?.action === 'setup') hooksState.setupCounter += 1;
+    if (body?.action === 'teardown') hooksState.setupCounter -= 1;
+
+    logger.debug(
+      `hooks state counter is ${hooksState.setupCounter} after receiving "${body?.action}" action`
+    );
+
+    next();
+  };
+
+export const registerBeforeHook =
+  (beforeEach: Hook, hooksState: HooksState): RequestHandler =>
+  async ({ body }, res, next) => {
+    if (body?.action === 'setup' && hooksState.setupCounter === 1) {
       logger.debug("executing 'beforeEach' hook");
       try {
-        await config.beforeEach();
+        await beforeEach();
         next();
       } catch (e) {
         logger.error(`error executing 'beforeEach' hook: ${e.message}`);
@@ -23,20 +43,15 @@ export const registerBeforeHook = (
     } else {
       next();
     }
-  });
-};
+  };
 
-export const registerAfterHook = (
-  app: express.Express,
-  config: ProxyOptions,
-  stateSetupPath: string
-): void => {
-  if (config.afterEach) logger.trace("registered 'afterEach' hook");
-  app.use(async (req, res, next) => {
-    if (req.path !== stateSetupPath && config.afterEach) {
+export const registerAfterHook =
+  (afterEach: Hook, hooksState: HooksState): RequestHandler =>
+  async ({ body }, res, next) => {
+    if (body?.action === 'teardown' && hooksState.setupCounter === 0) {
       logger.debug("executing 'afterEach' hook");
       try {
-        await config.afterEach();
+        await afterEach();
         next();
       } catch (e) {
         logger.error(`error executing 'afterEach' hook: ${e.message}`);
@@ -46,5 +61,4 @@ export const registerAfterHook = (
     } else {
       next();
     }
-  });
-};
+  };
diff --git a/src/dsl/verifier/proxy/proxy.ts b/src/dsl/verifier/proxy/proxy.ts
@@ -6,7 +6,12 @@ import * as http from 'http';
 import { ProxyOptions } from './types';
 import logger from '../../../common/logger';
 import { createProxyStateHandler } from './stateHandler/stateHandler';
-import { registerAfterHook, registerBeforeHook } from './hooks';
+import {
+  registerHookStateTracking,
+  registerAfterHook,
+  registerBeforeHook,
+  HooksState,
+} from './hooks';
 import { createRequestTracer, createResponseTracer } from './tracer';
 import { createProxyMessageHandler } from './messages';
 import { toServerOptions } from './proxyRequest';
@@ -43,8 +48,20 @@ export const createProxy = (
   );
   app.use(bodyParser.urlencoded({ extended: true }));
   app.use('/*', bodyParser.raw({ type: '*/*' }));
-  registerBeforeHook(app, config, stateSetupPath);
-  registerAfterHook(app, config, stateSetupPath);
+
+  // Hooks
+  const hooksState: HooksState = {
+    setupCounter: 0,
+  };
+  app.use(stateSetupPath, registerHookStateTracking(hooksState));
+  if (config.beforeEach) {
+    logger.trace("registered 'beforeEach' hook");
+    app.use(stateSetupPath, registerBeforeHook(config.beforeEach, hooksState));
+  }
+  if (config.afterEach) {
+    logger.trace("registered 'afterEach' hook");
+    app.use(stateSetupPath, registerAfterHook(config.afterEach, hooksState));
+  }
 
   // Trace req/res logging
   if (config.logLevel === 'debug' || config.logLevel === 'trace') {