fix: Warn if broker arguments are set without a broker URL and ignore…

… the argument (fixes pact-foundation/pact-js#760)
pact-foundation · Oct 16, 2021 · 17d3eb1 · 17d3eb1
1 parent 9f73355
commit 17d3eb1
Show file tree

Hide file tree

Showing 8 changed files with 165 additions and 86 deletions.
diff --git a/src/ffi/argumentMapper/index.ts b/src/ffi/argumentMapper/index.ts
diff --git a/src/ffi/argumentMapper/types.ts b/src/ffi/argumentMapper/types.ts
diff --git a/src/verifier/argumentMapper/index.ts b/src/verifier/argumentMapper/index.ts
@@ -0,0 +1,89 @@
+import { ArgMapping, FunctionMapping, IgnoreOptionCombinations } from './types';
+import logger from '../../logger';
+import { InternalPactVerifierOptions } from '../types';
+
+/**
+ * This function maps arguments from the Verifier to the Rust core's verifier arguments
+ *
+ * @internal
+ *
+ * @param argMapping The argument mapping for the internal verifier
+ * @param options The actual options passed by the user
+ * @param ignoredArguments An array of strings for options to ignore (this is strings, because pact-js might put options on the object that we don't need)
+ * @param ignoreOptionCombinations Describes some options that we might want to ignore if certain other options are not set
+ * @returns An array of strings to past to the Rust core verifier
+ */
+export const argumentMapper = (
+  argMapping: ArgMapping<InternalPactVerifierOptions>,
+  options: InternalPactVerifierOptions,
+  ignoredArguments: Array<string>,
+  ignoreOptionCombinations: IgnoreOptionCombinations<InternalPactVerifierOptions>
+): string[] =>
+  (Object.keys(options) as Array<keyof InternalPactVerifierOptions>)
+    .filter((k) => {
+      const ignoreCondition = ignoreOptionCombinations[k];
+      if (ignoreCondition !== undefined) {
+        logger.trace(
+          `The argument mapper has an ignored combination for '${k}'`
+        );
+        // We might have multiple ways to ignore option combinations in the future
+        if (
+          ignoreCondition.ifNotSet &&
+          options[ignoreCondition.ifNotSet] === undefined
+        ) {
+          logger.warn(
+            `Ignoring option '${k}' because it is invalid without '${ignoreCondition.ifNotSet}' also being set. This may indicate an error in your configuration`
+          );
+          return false;
+        }
+        logger.trace(`But it was not ignored: '${k}'`);
+      }
+      return true;
+    })
+    .map((key: keyof InternalPactVerifierOptions) => {
+      // We pull these out, because TypeScript doesn't like to
+      // reason that argMapping[key] is a constant value.
+      // So, we need to pull it out to be able to say
+      // `if('someKey' in thisMapping)` and retain type checking
+      const thisMapping = argMapping[key];
+      const thisValue = options[key];
+
+      if (!thisMapping) {
+        if (!ignoredArguments.includes(key)) {
+          logger.error(`Pact-core is ignoring unknown option '${key}'`);
+        }
+        return [];
+      }
+      if (thisValue === undefined) {
+        logger.warn(
+          `The Verifier option '${key}' was was explicitly set to undefined and will be ignored. This may indicate an error in your config. Remove the option entirely to prevent this warning`
+        );
+        return [];
+      }
+      if ('warningMessage' in thisMapping) {
+        logger.warn(thisMapping.warningMessage);
+        return [];
+      }
+      if ('arg' in thisMapping) {
+        switch (thisMapping.mapper) {
+          case 'string':
+            return [thisMapping.arg, `${thisValue}`];
+          case 'flag':
+            return thisValue ? [thisMapping.arg] : [];
+          default:
+            logger.pactCrash(
+              `Option mapper for '${key}' maps to '${thisMapping.arg}' with unknown mapper type '${thisMapping.mapper}'`
+            );
+            return [];
+        }
+      }
+      if (typeof thisMapping === 'function') {
+        return (thisMapping as FunctionMapping<unknown>)(thisValue);
+      }
+      logger.pactCrash(
+        `The option mapper completely failed to find a mapping for '${key}'.`
+      );
+      return [];
+    })
+    // This can be replaced with .flat() when node 10 is EOL
+    .reduce((acc: string[], current: string[]) => [...acc, ...current], []);
diff --git a/src/verifier/argumentMapper/types.ts b/src/verifier/argumentMapper/types.ts
@@ -0,0 +1,24 @@
+type BasicMapping = {
+  arg: string;
+  mapper: 'string' | 'flag';
+};
+
+type IgnoreAndWarn = {
+  warningMessage: string;
+};
+
+/** @internal */
+export type FunctionMapping<T> = (arg: NonNullable<T>) => string[];
+
+/** @internal */
+export type ArgMapping<PactOptions> = {
+  [Key in keyof PactOptions]-?:
+    | BasicMapping
+    | IgnoreAndWarn
+    | FunctionMapping<PactOptions[Key]>;
+};
+
+/** @internal */
+export type IgnoreOptionCombinations<T> = {
+  [Key in keyof Partial<T>]: { ifNotSet: keyof T };
+};
diff --git a/src/verifier/arguments.ts b/src/verifier/arguments.ts
@@ -1,22 +1,17 @@
 import url = require('url');
 
-import { ArgMapping } from '../ffi/argumentMapper/types';
-import { ConsumerVersionSelector, VerifierOptions } from './types';
+import { ArgMapping, IgnoreOptionCombinations } from './argumentMapper/types';
+import {
+  ConsumerVersionSelector,
+  InternalPactVerifierOptions,
+  VerifierOptions,
+} from './types';
 
 import { getUriType } from './filesystem';
 import { LogLevel } from '../logger/types';
 
-type DeprecatedVerifierOptions = {
-  format?: 'json' | 'xml' | 'progress' | 'RspecJunitFormatter';
-  out?: string;
-  customProviderHeaders?: string[];
-  verbose?: boolean;
-  monkeypatch?: string;
-  logDir?: string;
-};
-
 // These are arguments that are on the PactJS object that we don't need to use
-export const ignoredArguments = [
+export const ignoredArguments: Array<string> = [
   'requestFilter',
   'stateHandlers',
   'messageProviders',
@@ -26,9 +21,15 @@ export const ignoredArguments = [
   'validateSSL',
 ];
 
-export const argMapping: ArgMapping<
-  VerifierOptions & DeprecatedVerifierOptions
-> = {
+export const ignoreOptionCombinations: IgnoreOptionCombinations<VerifierOptions> =
+  {
+    enablePending: { ifNotSet: 'pactBrokerUrl' },
+    consumerVersionSelectors: { ifNotSet: 'pactBrokerUrl' },
+    consumerVersionTags: { ifNotSet: 'pactBrokerUrl' },
+    publishVerificationResult: { ifNotSet: 'pactBrokerUrl' },
+  };
+
+export const argMapping: ArgMapping<InternalPactVerifierOptions> = {
   providerBaseUrl: (providerBaseUrl: string) => {
     const u = url.parse(providerBaseUrl);
     return u && u.port && u.hostname

diff --git a/src/verifier/nativeVerifier.ts b/src/verifier/nativeVerifier.ts
@@ -1,8 +1,12 @@
 import { VerifierOptions } from './types';
 import { getFfiLib } from '../ffi';
-import logger from '../logger';
-import { argMapping, ignoredArguments } from './arguments';
-import { argumentMapper } from '../ffi/argumentMapper';
+import logger, { setLogLevel } from '../logger';
+import {
+  argMapping,
+  ignoredArguments,
+  ignoreOptionCombinations,
+} from './arguments';
+import { argumentMapper } from './argumentMapper';
 
 const VERIFICATION_SUCCESSFUL = 0;
 const VERIFICATION_FAILED = 1;
@@ -12,9 +16,17 @@ const INVALID_ARGUMENTS = 4;
 
 export const verify = (opts: VerifierOptions): Promise<string> => {
   const verifierLib = getFfiLib(opts.logLevel);
+  if (opts.logLevel) {
+    setLogLevel(opts.logLevel);
+  }
   // Todo: probably separate out the sections of this logic into separate promises
   return new Promise<string>((resolve, reject) => {
-    const request = argumentMapper(argMapping, opts, ignoredArguments)
+    const request = argumentMapper(
+      argMapping,
+      opts,
+      ignoredArguments,
+      ignoreOptionCombinations
+    )
       .map((s) => s.replace('\n', ''))
       .join('\n');
 

diff --git a/src/verifier/types.ts b/src/verifier/types.ts
@@ -31,3 +31,21 @@ export interface VerifierOptions {
   logLevel?: LogLevel;
   disableSslVerification?: boolean;
 }
+
+/** These are the deprecated verifier options, removed prior to this verison,
+ * but it's useful to know what they were so we can potentially map or warn.
+ */
+type DeprecatedVerifierOptions = {
+  format?: 'json' | 'xml' | 'progress' | 'RspecJunitFormatter';
+  out?: string;
+  customProviderHeaders?: string[];
+  verbose?: boolean;
+  monkeypatch?: string;
+  logDir?: string;
+};
+
+/** Helper type for the mapper to reason about the options we want to be able to handle.
+ * Not exposed, because we only want to expose the current VerifierOptions to the user
+ * @internal */
+export type InternalPactVerifierOptions = VerifierOptions &
+  DeprecatedVerifierOptions;
diff --git a/tsconfig.json b/tsconfig.json
@@ -16,7 +16,8 @@
     "skipLibCheck": true,
     "strictNullChecks": true,
     "strict": true,
-    "noUnusedLocals": true
+    "noUnusedLocals": true,
+    "stripInternal": true
   },
   "include": ["**/*.ts"],
   "exclude": ["node_modules", "**/*.spec.ts", "**/__mocks__"],