Skip to content

Commit

Permalink
@W-14689342@: (Part 8) Introduce ResultsProcessorFactory and inject i…
Browse files Browse the repository at this point in the history
…t as dependency to run actions
  • Loading branch information
stephen-carter-at-sf committed Jan 4, 2024
1 parent 8c8e4bd commit 5d920d1
Show file tree
Hide file tree
Showing 8 changed files with 77 additions and 34 deletions.
5 changes: 4 additions & 1 deletion src/commands/scanner/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {Action} from "../../lib/ScannerCommand";
import {Display} from "../../lib/Display";
import {RunAction} from "../../lib/actions/RunAction";
import {RuleFilterFactory, RuleFilterFactoryImpl} from "../../lib/RuleFilterFactory";
import {ResultsProcessorFactory, ResultsProcessorFactoryImpl} from "../../lib/output/ResultsProcessorFactory";

/**
* Defines the "run" command for the "scanner" cli.
Expand Down Expand Up @@ -91,6 +92,8 @@ export default class Run extends ScannerRunCommand {
const inputProcessor: InputProcessor = new InputProcessorImpl(this.config.version);
const ruleFilterFactory: RuleFilterFactory = new RuleFilterFactoryImpl();
const engineOptionsFactory: EngineOptionsFactory = new RunEngineOptionsFactory(inputProcessor);
return new RunAction(logger, display, inputProcessor, ruleFilterFactory, engineOptionsFactory);
const resultsProcessorFactory: ResultsProcessorFactory = new ResultsProcessorFactoryImpl();
return new RunAction(logger, display, inputProcessor, ruleFilterFactory, engineOptionsFactory,
resultsProcessorFactory);
}
}
5 changes: 4 additions & 1 deletion src/commands/scanner/run/dfa.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {Display} from "../../../lib/Display";
import {Action} from "../../../lib/ScannerCommand";
import {RunDfaAction} from "../../../lib/actions/RunDfaAction";
import {RuleFilterFactory, RuleFilterFactoryImpl} from "../../../lib/RuleFilterFactory";
import {ResultsProcessorFactory, ResultsProcessorFactoryImpl} from "../../../lib/output/ResultsProcessorFactory";

/**
* Defines the "run dfa" command for the "scanner" cli.
Expand Down Expand Up @@ -79,6 +80,8 @@ export default class Dfa extends ScannerRunCommand {
const inputProcessor: InputProcessor = new InputProcessorImpl(this.config.version);
const ruleFilterFactory: RuleFilterFactory = new RuleFilterFactoryImpl();
const engineOptionsFactory: EngineOptionsFactory = new RunDfaEngineOptionsFactory(inputProcessor);
return new RunDfaAction(logger, display, inputProcessor, ruleFilterFactory, engineOptionsFactory);
const resultsProcessorFactory: ResultsProcessorFactory = new ResultsProcessorFactoryImpl();
return new RunDfaAction(logger, display, inputProcessor, ruleFilterFactory, engineOptionsFactory,
resultsProcessorFactory);
}
}
29 changes: 13 additions & 16 deletions src/lib/actions/AbstractRunAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ import {Display} from "../Display";
import {RuleFilter} from "../RuleFilter";
import {RuleFilterFactory} from "../RuleFilterFactory";
import {Controller} from "../../Controller";
import {RunOutputOptions, RunResultsProcessor} from "../output/RunResultsProcessor";
import {RunOutputOptions} from "../output/RunResultsProcessor";
import {InputProcessor} from "../InputProcessor";
import {EngineOptionsFactory} from "../EngineOptionsFactory";
import {ENV_VAR_NAMES, INTERNAL_ERROR_CODE} from "../../Constants";
import {INTERNAL_ERROR_CODE} from "../../Constants";
import {Results} from "../output/Results";
import {inferFormatFromInternalOutfile, inferFormatFromOutfile, OutputFormat} from "../output/OutputFormat";
import {CompositeResultsProcessor, ResultsProcessor} from "../output/ResultsProcessor";
import {OutfileResultsProcessor} from "../output/OutfileResultsProcessor";
import {inferFormatFromOutfile, OutputFormat} from "../output/OutputFormat";
import {ResultsProcessor} from "../output/ResultsProcessor";
import {ResultsProcessorFactory} from "../output/ResultsProcessorFactory";
import {JsonReturnValueHolder} from "../output/JsonReturnValueHolder";

/**
Expand All @@ -29,14 +29,17 @@ export abstract class AbstractRunAction implements Action {
private readonly inputProcessor: InputProcessor;
private readonly ruleFilterFactory: RuleFilterFactory;
private readonly engineOptionsFactory: EngineOptionsFactory;
private readonly resultsProcessorFactory: ResultsProcessorFactory;

protected constructor(logger: Logger, display: Display, inputProcessor: InputProcessor,
ruleFilterFactory: RuleFilterFactory, engineOptionsFactory: EngineOptionsFactory) {
ruleFilterFactory: RuleFilterFactory, engineOptionsFactory: EngineOptionsFactory,
resultsProcessorFactory: ResultsProcessorFactory) {
this.logger = logger;
this.display = display;
this.inputProcessor = inputProcessor;
this.ruleFilterFactory = ruleFilterFactory;
this.engineOptionsFactory = engineOptionsFactory;
this.resultsProcessorFactory = resultsProcessorFactory;
}

protected abstract isDfa(): boolean;
Expand Down Expand Up @@ -80,24 +83,18 @@ export abstract class AbstractRunAction implements Action {
const runOptions: RunOptions = this.inputProcessor.createRunOptions(inputs, this.isDfa());
const engineOptions: EngineOptions = this.engineOptionsFactory.createEngineOptions(inputs);
const outputOptions: RunOutputOptions = this.inputProcessor.createRunOutputOptions(inputs);
const jsonReturnValueHolder: JsonReturnValueHolder = new JsonReturnValueHolder();

const runResultsProcessor: RunResultsProcessor = new RunResultsProcessor(this.display, outputOptions, jsonReturnValueHolder);
const resultsProcessors: ResultsProcessor[] = [runResultsProcessor];
const internalOutfile: string = process.env[ENV_VAR_NAMES.SCANNER_INTERNAL_OUTFILE];
if (internalOutfile && internalOutfile.length > 0) {
const internalOutputFormat: OutputFormat = inferFormatFromInternalOutfile(internalOutfile);
resultsProcessors.push(new OutfileResultsProcessor(internalOutputFormat, internalOutfile, outputOptions.verboseViolations));
}
const compositeResultsProcessor: ResultsProcessor = new CompositeResultsProcessor(resultsProcessors);
const jsonReturnValueHolder: JsonReturnValueHolder = new JsonReturnValueHolder();
const resultsProcessor: ResultsProcessor = this.resultsProcessorFactory.createResultsProcessor(
this.display, outputOptions, jsonReturnValueHolder);

// TODO: Inject RuleManager as a dependency to improve testability by removing coupling to runtime implementation
const ruleManager: RuleManager = await Controller.createRuleManager();

try {
const results: Results = await ruleManager.runRulesMatchingCriteria(filters, targetPaths, runOptions, engineOptions);
this.logger.trace(`Processing output with format ${outputOptions.format}`);
await compositeResultsProcessor.processResults(results);
await resultsProcessor.processResults(results);
return jsonReturnValueHolder.get();

} catch (e) {
Expand Down
6 changes: 4 additions & 2 deletions src/lib/actions/RunAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,16 @@ import {Inputs} from "../../types";
import {Logger, SfError} from "@salesforce/core";
import {BundleName, getMessage} from "../../MessageCatalog";
import {RuleFilterFactory} from "../RuleFilterFactory";
import {ResultsProcessorFactory} from "../output/ResultsProcessorFactory";

/**
* The Action behind the "run" command
*/
export class RunAction extends AbstractRunAction {
public constructor(logger: Logger, display: Display, inputProcessor: InputProcessor,
ruleFilterFactory: RuleFilterFactory, engineOptionsFactory: EngineOptionsFactory) {
super(logger, display, inputProcessor, ruleFilterFactory, engineOptionsFactory);
ruleFilterFactory: RuleFilterFactory, engineOptionsFactory: EngineOptionsFactory,
resultsProcessorFactory: ResultsProcessorFactory) {
super(logger, display, inputProcessor, ruleFilterFactory, engineOptionsFactory, resultsProcessorFactory);
}

public override async validateInputs(inputs: Inputs): Promise<void> {
Expand Down
6 changes: 4 additions & 2 deletions src/lib/actions/RunDfaAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,16 @@ import {FileHandler} from "../util/FileHandler";
import {Logger, SfError} from "@salesforce/core";
import {BundleName, getMessage} from "../../MessageCatalog";
import * as globby from "globby";
import {ResultsProcessorFactory} from "../output/ResultsProcessorFactory";

/**
* The Action behind the "run dfa" command
*/
export class RunDfaAction extends AbstractRunAction {
public constructor(logger: Logger, display: Display, inputProcessor: InputProcessor,
ruleFilterFactory: RuleFilterFactory, engineOptionsFactory: EngineOptionsFactory) {
super(logger, display, inputProcessor, ruleFilterFactory, engineOptionsFactory);
ruleFilterFactory: RuleFilterFactory, engineOptionsFactory: EngineOptionsFactory,
resultsProcessorFactory: ResultsProcessorFactory) {
super(logger, display, inputProcessor, ruleFilterFactory, engineOptionsFactory, resultsProcessorFactory);
}

public override async validateInputs(inputs: Inputs): Promise<void> {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/output/Results.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export class RunResults implements Results {
}

public async toFormattedOutput(format: OutputFormat, verboseViolations: boolean): Promise<FormattedOutput> {
const cacheKey: string = String(OutputFormat) + '_' + String(verboseViolations);
const cacheKey: string = String(format) + '_' + String(verboseViolations);
if (this.formattedResultsCache.has(cacheKey)) {
return Promise.resolve(this.formattedResultsCache.get(cacheKey));
}
Expand Down
28 changes: 28 additions & 0 deletions src/lib/output/ResultsProcessorFactory.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import {Display} from "../Display";
import {JsonReturnValueHolder} from "./JsonReturnValueHolder";
import {RunOutputOptions, RunResultsProcessor} from "./RunResultsProcessor";
import {CompositeResultsProcessor, ResultsProcessor} from "./ResultsProcessor";
import {ENV_VAR_NAMES} from "../../Constants";
import {inferFormatFromInternalOutfile, OutputFormat} from "./OutputFormat";
import {OutfileResultsProcessor} from "./OutfileResultsProcessor";

export interface ResultsProcessorFactory {
createResultsProcessor(display: Display, runOutputOptions: RunOutputOptions,
jsonReturnValueHolder: JsonReturnValueHolder): ResultsProcessor
}

export class ResultsProcessorFactoryImpl implements ResultsProcessorFactory {
public createResultsProcessor(display: Display, runOutputOptions: RunOutputOptions,
jsonReturnValueHolder: JsonReturnValueHolder): ResultsProcessor {

const resultsProcessors: ResultsProcessor[] = [new RunResultsProcessor(display, runOutputOptions, jsonReturnValueHolder)];

const internalOutfile: string = process.env[ENV_VAR_NAMES.SCANNER_INTERNAL_OUTFILE];
if (internalOutfile && internalOutfile.length > 0) {
const internalOutputFormat: OutputFormat = inferFormatFromInternalOutfile(internalOutfile);
resultsProcessors.push(new OutfileResultsProcessor(internalOutputFormat, internalOutfile, runOutputOptions.verboseViolations));
}

return new CompositeResultsProcessor(resultsProcessors);
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {expect} from 'chai';

import {RunOutputOptions, RunResultsProcessor} from '../../../src/lib/output/RunResultsProcessor';
import {RunOutputOptions} from '../../../src/lib/output/RunResultsProcessor';
import {EngineExecutionSummary} from '../../../src/types';
import {AnyJson} from '@salesforce/ts-types';
import Sinon = require('sinon');
Expand All @@ -12,6 +12,9 @@ import {OutputFormat} from "../../../src/lib/output/OutputFormat";
import {Results} from "../../../src/lib/output/Results";
import {FakeResults} from "./FakeResults";
import {JsonReturnValueHolder} from "../../../src/lib/output/JsonReturnValueHolder";
import {ResultsProcessor} from "../../../src/lib/output/ResultsProcessor";
import {ResultsProcessorFactoryImpl} from "../../../src/lib/output/ResultsProcessorFactory";
import {Display} from "../../../src/lib/Display";

const FAKE_SUMMARY_MAP: Map<string, EngineExecutionSummary> = new Map();
FAKE_SUMMARY_MAP.set('pmd', {fileCount: 1, violationCount: 1});
Expand Down Expand Up @@ -108,7 +111,7 @@ describe('RunOutputProcessor', () => {
verboseViolations: false
};
const jsonHolder: JsonReturnValueHolder = new JsonReturnValueHolder();
const rrp = new RunResultsProcessor(display, opts, jsonHolder);
const rrp = createResultsProcessor(display, opts, jsonHolder);
const summaryMap: Map<string, EngineExecutionSummary> = new Map();
summaryMap.set('pmd', {fileCount: 0, violationCount: 0});
summaryMap.set('eslint', {fileCount: 0, violationCount: 0});
Expand All @@ -135,7 +138,7 @@ describe('RunOutputProcessor', () => {
verboseViolations: false
};
const jsonHolder: JsonReturnValueHolder = new JsonReturnValueHolder();
const rrp = new RunResultsProcessor(display, opts, jsonHolder);
const rrp = createResultsProcessor(display, opts, jsonHolder);

// THIS IS THE PART BEING TESTED.
await rrp.processResults(fakeTableResults);
Expand All @@ -160,7 +163,7 @@ ${getMessage(BundleName.RunOutputProcessor, 'output.writtenToConsole')}`;
severityForError: 1
};
const jsonHolder: JsonReturnValueHolder = new JsonReturnValueHolder();
const rrp = new RunResultsProcessor(display, opts, jsonHolder);
const rrp = createResultsProcessor(display, opts, jsonHolder);

// THIS IS THE PART BEING TESTED.
try {
Expand Down Expand Up @@ -194,7 +197,7 @@ ${getMessage(BundleName.RunOutputProcessor, 'output.writtenToConsole')}`;
};

const jsonHolder: JsonReturnValueHolder = new JsonReturnValueHolder();
const rrp = new RunResultsProcessor(display, opts, jsonHolder);
const rrp = createResultsProcessor(display, opts, jsonHolder);

// THIS IS THE PART BEING TESTED.
await rrp.processResults(fakeCsvResults);
Expand All @@ -211,7 +214,7 @@ ${getMessage(BundleName.RunOutputProcessor, 'output.writtenToConsole')}`;
};

const jsonHolder: JsonReturnValueHolder = new JsonReturnValueHolder();
const rrp = new RunResultsProcessor(display, opts, jsonHolder);
const rrp = createResultsProcessor(display, opts, jsonHolder);

// THIS IS THE PART BEING TESTED.
try {
Expand Down Expand Up @@ -240,7 +243,7 @@ ${getMessage(BundleName.RunOutputProcessor, 'output.writtenToConsole')}`;
};

const jsonHolder: JsonReturnValueHolder = new JsonReturnValueHolder();
const rrp = new RunResultsProcessor(display, opts, jsonHolder);
const rrp = createResultsProcessor(display, opts, jsonHolder);

// THIS IS THE PART BEING TESTED
await rrp.processResults(fakeJsonResults);
Expand All @@ -258,7 +261,7 @@ ${getMessage(BundleName.RunOutputProcessor, 'output.writtenToConsole')}`;
};

const jsonHolder: JsonReturnValueHolder = new JsonReturnValueHolder();
const rrp = new RunResultsProcessor(display, opts, jsonHolder);
const rrp = createResultsProcessor(display, opts, jsonHolder);

// THIS IS THE PART BEING TESTED
try {
Expand All @@ -282,7 +285,7 @@ ${getMessage(BundleName.RunOutputProcessor, 'output.writtenToConsole')}`;
outfile: fakeFilePath
};
const jsonHolder: JsonReturnValueHolder = new JsonReturnValueHolder();
const rrp = new RunResultsProcessor(display, opts, jsonHolder);
const rrp = createResultsProcessor(display, opts, jsonHolder);
const summaryMap: Map<string, EngineExecutionSummary> = new Map();
summaryMap.set('pmd', {fileCount: 0, violationCount: 0});
summaryMap.set('eslint', {fileCount: 0, violationCount: 0});
Expand Down Expand Up @@ -314,7 +317,7 @@ ${getMessage(BundleName.RunOutputProcessor, 'output.writtenToOutFile', [fakeFile
outfile: fakeFilePath
};
const jsonHolder: JsonReturnValueHolder = new JsonReturnValueHolder();
const rrp = new RunResultsProcessor(display, opts, jsonHolder);
const rrp = createResultsProcessor(display, opts, jsonHolder);

// THIS IS THE PART BEING TESTED.
await rrp.processResults(fakeCsvResults);
Expand All @@ -337,7 +340,7 @@ ${getMessage(BundleName.RunOutputProcessor, 'output.writtenToOutFile', [fakeFile
outfile: fakeFilePath
};
const jsonHolder: JsonReturnValueHolder = new JsonReturnValueHolder();
const rrp = new RunResultsProcessor(display, opts, jsonHolder);
const rrp = createResultsProcessor(display, opts, jsonHolder);

// THIS IS THE PART BEING TESTED.
try {
Expand All @@ -359,3 +362,8 @@ ${getMessage(BundleName.RunOutputProcessor, 'output.writtenToOutFile', [fakeFile
});
});
});

function createResultsProcessor(display: Display, runOutputOptions: RunOutputOptions,
jsonReturnValueHolder: JsonReturnValueHolder): ResultsProcessor {
return (new ResultsProcessorFactoryImpl()).createResultsProcessor(display, runOutputOptions, jsonReturnValueHolder);
}

0 comments on commit 5d920d1

Please sign in to comment.