From 17d3eb1629256828d3162a46d078087dd6ef7b91 Mon Sep 17 00:00:00 2001 From: Timothy Jones Date: Sat, 16 Oct 2021 19:32:10 +1100 Subject: [PATCH] fix: Warn if broker arguments are set without a broker URL and ignore the argument (fixes pact-foundation/pact-js#760) --- src/ffi/argumentMapper/index.ts | 49 --------------- src/ffi/argumentMapper/types.ts | 17 ------ src/verifier/argumentMapper/index.ts | 89 ++++++++++++++++++++++++++++ src/verifier/argumentMapper/types.ts | 24 ++++++++ src/verifier/arguments.ts | 31 +++++----- src/verifier/nativeVerifier.ts | 20 +++++-- src/verifier/types.ts | 18 ++++++ tsconfig.json | 3 +- 8 files changed, 165 insertions(+), 86 deletions(-) delete mode 100644 src/ffi/argumentMapper/index.ts delete mode 100644 src/ffi/argumentMapper/types.ts create mode 100644 src/verifier/argumentMapper/index.ts create mode 100644 src/verifier/argumentMapper/types.ts diff --git a/src/ffi/argumentMapper/index.ts b/src/ffi/argumentMapper/index.ts deleted file mode 100644 index 1283b730..00000000 --- a/src/ffi/argumentMapper/index.ts +++ /dev/null @@ -1,49 +0,0 @@ -import { ArgMapping } from './types'; -import logger from '../../logger'; - -export const argumentMapper = ( - argMapping: ArgMapping, - options: PactOptions, - ignoredArguments: string[] -): string[] => - Object.keys(options) - .map((key: string) => { - if (!argMapping[key]) { - if (!ignoredArguments.includes(key)) { - logger.error(`Pact-core is ignoring unknown option '${key}'`); - } - return []; - } - if (options[key] === 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 (argMapping[key].warningMessage) { - logger.warn(argMapping[key].warningMessage); - return []; - } - if (argMapping[key].arg) { - switch (argMapping[key].mapper) { - case 'string': - return [argMapping[key].arg, `${options[key]}`]; - case 'flag': - return options[key] ? [argMapping[key].arg] : []; - default: - logger.pactCrash( - `Option mapper for '${key}' maps to '${argMapping[key].arg}' with unknown mapper type '${argMapping[key].mapper}'` - ); - return []; - } - } - if (typeof argMapping[key] === 'function') { - return argMapping[key](options[key]); - } - 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/ffi/argumentMapper/types.ts b/src/ffi/argumentMapper/types.ts deleted file mode 100644 index b050a971..00000000 --- a/src/ffi/argumentMapper/types.ts +++ /dev/null @@ -1,17 +0,0 @@ -type BasicMapping = { - arg: string; - mapper: 'string' | 'flag'; -}; - -type IgnoreAndWarn = { - warningMessage: string; -}; - -type FunctionMapping = (arg: NonNullable) => string[]; - -export type ArgMapping = { - [Key in keyof PactOptions]: - | BasicMapping - | IgnoreAndWarn - | FunctionMapping; -}; diff --git a/src/verifier/argumentMapper/index.ts b/src/verifier/argumentMapper/index.ts new file mode 100644 index 00000000..97a4e4fa --- /dev/null +++ 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, + options: InternalPactVerifierOptions, + ignoredArguments: Array, + ignoreOptionCombinations: IgnoreOptionCombinations +): string[] => + (Object.keys(options) as Array) + .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)(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 new file mode 100644 index 00000000..8191c91d --- /dev/null +++ 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 = (arg: NonNullable) => string[]; + +/** @internal */ +export type ArgMapping = { + [Key in keyof PactOptions]-?: + | BasicMapping + | IgnoreAndWarn + | FunctionMapping; +}; + +/** @internal */ +export type IgnoreOptionCombinations = { + [Key in keyof Partial]: { ifNotSet: keyof T }; +}; diff --git a/src/verifier/arguments.ts b/src/verifier/arguments.ts index e4c76c05..9842aa88 100644 --- 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 = [ 'requestFilter', 'stateHandlers', 'messageProviders', @@ -26,9 +21,15 @@ export const ignoredArguments = [ 'validateSSL', ]; -export const argMapping: ArgMapping< - VerifierOptions & DeprecatedVerifierOptions -> = { +export const ignoreOptionCombinations: IgnoreOptionCombinations = + { + enablePending: { ifNotSet: 'pactBrokerUrl' }, + consumerVersionSelectors: { ifNotSet: 'pactBrokerUrl' }, + consumerVersionTags: { ifNotSet: 'pactBrokerUrl' }, + publishVerificationResult: { ifNotSet: 'pactBrokerUrl' }, + }; + +export const argMapping: ArgMapping = { 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 index f5ab1367..685aba00 100644 --- 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 => { 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((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 index 457ef820..868fb179 100644 --- 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 index 092998a4..87c4558f 100644 --- 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__"],