From 8d00dc427c4871364d3514471aa6787b6b17a903 Mon Sep 17 00:00:00 2001 From: goenning Date: Wed, 22 Dec 2021 16:58:40 +0000 Subject: [PATCH 1/6] ability to add custom operators Signed-off-by: goenning --- .changeset/spicy-moons-poke.md | 5 + .../JsonRulesEngineFactChecker.test.ts | 120 ++++++++++++++++-- .../src/service/JsonRulesEngineFactChecker.ts | 33 ++++- 3 files changed, 144 insertions(+), 14 deletions(-) create mode 100644 .changeset/spicy-moons-poke.md diff --git a/.changeset/spicy-moons-poke.md b/.changeset/spicy-moons-poke.md new file mode 100644 index 0000000000..fdf130c491 --- /dev/null +++ b/.changeset/spicy-moons-poke.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-tech-insights-backend-module-jsonfc': minor +--- + +ability to add custom operators diff --git a/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.test.ts b/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.test.ts index 5c33e9e369..923f1554b6 100644 --- a/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.test.ts +++ b/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.test.ts @@ -24,6 +24,7 @@ import { } from '../index'; import { getVoidLogger } from '@backstage/backend-common'; import { TechInsightJsonRuleCheck } from '../types'; +import { Operator } from 'json-rules-engine'; const testChecks: Record = { broken: [ @@ -127,6 +128,49 @@ const testChecks: Record = { }, }, ], + + customOperator: [ + { + id: 'customOperatorTestCheck', + name: 'customOperatorTestCheck', + type: JSON_RULE_ENGINE_CHECK_TYPE, + description: 'Check For Testing using Custom Operator', + factIds: ['test-factretriever'], + rule: { + conditions: { + all: [ + { + fact: 'testnumberfact', + operator: 'isDivisibleBy', + value: 2, + }, + ], + }, + }, + }, + ], + + invalidCustomOperator: [ + { + id: 'invalidCustomOperatorTestCheck', + name: 'invalidCustomOperatorTestCheck', + type: JSON_RULE_ENGINE_CHECK_TYPE, + description: + 'Check For Testing using a Custom Operator that is not registered', + factIds: ['test-factretriever'], + rule: { + conditions: { + all: [ + { + fact: 'testnumberfact', + operator: 'isOdd', + value: 2, + }, + ], + }, + }, + }, + ], }; const latestSchemasMock = jest.fn().mockImplementation(() => [ @@ -166,6 +210,9 @@ describe('JsonRulesEngineFactChecker', () => { const factChecker = new JsonRulesEngineFactCheckerFactory({ checkRegistry: mockCheckRegistry, checks: [], + operators: [ + new Operator('isDivisibleBy', (a, b) => a % b === 0), + ], logger: getVoidLogger(), }).construct(mockRepository); @@ -234,6 +281,52 @@ describe('JsonRulesEngineFactChecker', () => { }); }); + it('should use custom operators when defined', async () => { + latestFactsByIdsMock.mockImplementation(() => + Promise.resolve({ + ['test-factretriever']: { + id: 'test-factretriever', + facts: { + testnumberfact: 4, + }, + }, + }), + ); + const results = await factChecker.runChecks('a/a/a', ['customOperator']); + expect(results).toHaveLength(1); + expect(results[0]).toMatchObject({ + facts: { + testnumberfact: { + value: 4, + type: 'integer', + description: '', + }, + }, + result: true, + check: { + id: 'customOperatorTestCheck', + type: JSON_RULE_ENGINE_CHECK_TYPE, + name: 'customOperatorTestCheck', + description: 'Check For Testing using Custom Operator', + factIds: ['test-factretriever'], + rule: { + conditions: { + all: [ + { + fact: 'testnumberfact', + factResult: 4, + operator: 'isDivisibleBy', + result: true, + value: 2, + }, + ], + priority: 1, + }, + }, + }, + }); + }); + it('should gracefully handle multiple check at once', async () => { const results = await factChecker.runChecks('a/a/a', [ 'simple', @@ -307,17 +400,22 @@ describe('JsonRulesEngineFactChecker', () => { }); describe('when validating checks', () => { - it('should succeed on valid rules', async () => { - const validationResponse = await factChecker.validate( - testChecks.simple[0], - ); - expect(validationResponse.valid).toBeTruthy(); - }); - it('should fail on broken rules', async () => { - const validationResponse = await factChecker.validate( - testChecks.broken[0], - ); - expect(validationResponse.valid).toBeFalsy(); + [testChecks.simple[0], testChecks.customOperator[0]].forEach(check => { + it(`should succeed on valid rule: ${check.name}`, async () => { + const validationResponse = await factChecker.validate(check); + expect(validationResponse.valid).toBeTruthy(); + }); }); + + [testChecks.broken[0], testChecks.invalidCustomOperator[0]].forEach( + check => { + it(`should succeed on invalid rule: ${check.name}`, async () => { + const validationResponse = await factChecker.validate( + testChecks.broken[0], + ); + expect(validationResponse.valid).toBeFalsy(); + }); + }, + ); }); }); diff --git a/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.ts b/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.ts index 2623a5c2ee..aef09d5434 100644 --- a/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.ts +++ b/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.ts @@ -23,11 +23,16 @@ import { CheckValidationResponse, } from '@backstage/plugin-tech-insights-node'; import { FactResponse } from '@backstage/plugin-tech-insights-common'; -import { Engine, EngineResult, TopLevelCondition } from 'json-rules-engine'; +import { + Engine, + EngineResult, + Operator, + TopLevelCondition, +} from 'json-rules-engine'; import { DefaultCheckRegistry } from './CheckRegistry'; import { Logger } from 'winston'; import { pick } from 'lodash'; -import Ajv from 'ajv'; +import Ajv, { SchemaObject } from 'ajv'; import * as validationSchema from './validation-schema.json'; import { JSON_RULE_ENGINE_CHECK_TYPE } from '../constants'; @@ -46,6 +51,7 @@ export type JsonRulesEngineFactCheckerOptions = { repository: TechInsightsStore; logger: Logger; checkRegistry?: TechInsightCheckRegistry; + operators?: Operator[]; }; /** @@ -60,15 +66,27 @@ export class JsonRulesEngineFactChecker private readonly checkRegistry: TechInsightCheckRegistry; private repository: TechInsightsStore; private readonly logger: Logger; + private readonly validationSchema: SchemaObject; + private readonly operators: Operator[]; constructor({ checks, repository, logger, checkRegistry, + operators, }: JsonRulesEngineFactCheckerOptions) { this.repository = repository; this.logger = logger; + this.operators = operators || []; + this.validationSchema = JSON.parse(JSON.stringify(validationSchema)); + + this.operators.forEach(op => { + this.validationSchema.definitions.condition.properties.operator.anyOf.push( + { const: op.name }, + ); + }); + checks.forEach(check => this.validate(check)); this.checkRegistry = checkRegistry ?? @@ -80,6 +98,10 @@ export class JsonRulesEngineFactChecker checks?: string[], ): Promise { const engine = new Engine(); + this.operators.forEach(op => { + engine.addOperator(op); + }); + const techInsightChecks = checks ? await this.checkRegistry.getAll(checks) : await this.checkRegistry.list(); @@ -125,7 +147,7 @@ export class JsonRulesEngineFactChecker check: TechInsightJsonRuleCheck, ): Promise { const ajv = new Ajv({ verbose: true }); - const validator = ajv.compile(validationSchema); + const validator = ajv.compile(this.validationSchema); const isValidToSchema = validator(check.rule); if (check.type !== JSON_RULE_ENGINE_CHECK_TYPE) { const msg = `Only ${JSON_RULE_ENGINE_CHECK_TYPE} checks can be registered to this fact checker`; @@ -317,6 +339,7 @@ export type JsonRulesEngineFactCheckerFactoryOptions = { checks: TechInsightJsonRuleCheck[]; logger: Logger; checkRegistry?: TechInsightCheckRegistry; + operators?: Operator[]; }; /** @@ -330,15 +353,18 @@ export class JsonRulesEngineFactCheckerFactory { private readonly checks: TechInsightJsonRuleCheck[]; private readonly logger: Logger; private readonly checkRegistry?: TechInsightCheckRegistry; + private readonly operators?: Operator[]; constructor({ checks, logger, checkRegistry, + operators, }: JsonRulesEngineFactCheckerFactoryOptions) { this.logger = logger; this.checks = checks; this.checkRegistry = checkRegistry; + this.operators = operators; } /** @@ -352,6 +378,7 @@ export class JsonRulesEngineFactCheckerFactory { logger: this.logger, checkRegistry: this.checkRegistry, repository, + operators: this.operators, }); } } From d0db4a56f103b6fd0e790f6bbb9e00f5e51cef6b Mon Sep 17 00:00:00 2001 From: goenning Date: Wed, 22 Dec 2021 17:14:24 +0000 Subject: [PATCH 2/6] update api reports Signed-off-by: goenning --- plugins/tech-insights-backend-module-jsonfc/api-report.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugins/tech-insights-backend-module-jsonfc/api-report.md b/plugins/tech-insights-backend-module-jsonfc/api-report.md index 7093fbfc2b..2e62e7d6a0 100644 --- a/plugins/tech-insights-backend-module-jsonfc/api-report.md +++ b/plugins/tech-insights-backend-module-jsonfc/api-report.md @@ -8,6 +8,7 @@ import { CheckResponse } from '@backstage/plugin-tech-insights-common'; import { CheckValidationResponse } from '@backstage/plugin-tech-insights-node'; import { FactChecker } from '@backstage/plugin-tech-insights-node'; import { Logger as Logger_2 } from 'winston'; +import { Operator } from 'json-rules-engine'; import { TechInsightCheck } from '@backstage/plugin-tech-insights-node'; import { TechInsightCheckRegistry } from '@backstage/plugin-tech-insights-node'; import { TechInsightsStore } from '@backstage/plugin-tech-insights-node'; @@ -50,6 +51,7 @@ export class JsonRulesEngineFactChecker repository, logger, checkRegistry, + operators, }: JsonRulesEngineFactCheckerOptions); // (undocumented) getChecks(): Promise; @@ -68,6 +70,7 @@ export class JsonRulesEngineFactCheckerFactory { checks, logger, checkRegistry, + operators, }: JsonRulesEngineFactCheckerFactoryOptions); // (undocumented) construct(repository: TechInsightsStore): JsonRulesEngineFactChecker; @@ -78,6 +81,7 @@ export type JsonRulesEngineFactCheckerFactoryOptions = { checks: TechInsightJsonRuleCheck[]; logger: Logger_2; checkRegistry?: TechInsightCheckRegistry; + operators?: Operator[]; }; // @public @@ -86,6 +90,7 @@ export type JsonRulesEngineFactCheckerOptions = { repository: TechInsightsStore; logger: Logger_2; checkRegistry?: TechInsightCheckRegistry; + operators?: Operator[]; }; // @public (undocumented) From fc6530ea191ed62014691bc628e5a8e6f5e2e241 Mon Sep 17 00:00:00 2001 From: goenning Date: Wed, 22 Dec 2021 17:22:25 +0000 Subject: [PATCH 3/6] fix unit test name Signed-off-by: goenning --- .../src/service/JsonRulesEngineFactChecker.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.test.ts b/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.test.ts index 923f1554b6..28b5a776fa 100644 --- a/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.test.ts +++ b/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.test.ts @@ -409,7 +409,7 @@ describe('JsonRulesEngineFactChecker', () => { [testChecks.broken[0], testChecks.invalidCustomOperator[0]].forEach( check => { - it(`should succeed on invalid rule: ${check.name}`, async () => { + it(`should fail on broken rules: ${check.name}`, async () => { const validationResponse = await factChecker.validate( testChecks.broken[0], ); From 273c9aa1ca26a169dd61673ff907e6068f5cddd6 Mon Sep 17 00:00:00 2001 From: goenning Date: Thu, 23 Dec 2021 11:03:06 +0000 Subject: [PATCH 4/6] fix unit test Signed-off-by: goenning --- .../service/JsonRulesEngineFactChecker.test.ts | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.test.ts b/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.test.ts index 28b5a776fa..c808713c3b 100644 --- a/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.test.ts +++ b/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.test.ts @@ -282,27 +282,17 @@ describe('JsonRulesEngineFactChecker', () => { }); it('should use custom operators when defined', async () => { - latestFactsByIdsMock.mockImplementation(() => - Promise.resolve({ - ['test-factretriever']: { - id: 'test-factretriever', - facts: { - testnumberfact: 4, - }, - }, - }), - ); const results = await factChecker.runChecks('a/a/a', ['customOperator']); expect(results).toHaveLength(1); expect(results[0]).toMatchObject({ facts: { testnumberfact: { - value: 4, + value: 3, type: 'integer', description: '', }, }, - result: true, + result: false, check: { id: 'customOperatorTestCheck', type: JSON_RULE_ENGINE_CHECK_TYPE, @@ -314,9 +304,9 @@ describe('JsonRulesEngineFactChecker', () => { all: [ { fact: 'testnumberfact', - factResult: 4, + factResult: 3, operator: 'isDivisibleBy', - result: true, + result: false, value: 2, }, ], From e3b49153b76932a08b2a33b7149921efec4ee81b Mon Sep 17 00:00:00 2001 From: goenning Date: Mon, 27 Dec 2021 12:08:42 +0000 Subject: [PATCH 5/6] add example of custom operator Signed-off-by: goenning --- .changeset/spicy-moons-poke.md | 2 +- .../README.md | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/.changeset/spicy-moons-poke.md b/.changeset/spicy-moons-poke.md index fdf130c491..cc39d629bf 100644 --- a/.changeset/spicy-moons-poke.md +++ b/.changeset/spicy-moons-poke.md @@ -1,5 +1,5 @@ --- -'@backstage/plugin-tech-insights-backend-module-jsonfc': minor +'@backstage/plugin-tech-insights-backend-module-jsonfc': patch --- ability to add custom operators diff --git a/plugins/tech-insights-backend-module-jsonfc/README.md b/plugins/tech-insights-backend-module-jsonfc/README.md index 108b1c8564..6703f4b9f9 100644 --- a/plugins/tech-insights-backend-module-jsonfc/README.md +++ b/plugins/tech-insights-backend-module-jsonfc/README.md @@ -85,3 +85,32 @@ export const exampleCheck: TechInsightJsonRuleCheck = { }, }; ``` + +# Custom operators + +json-rules-engine supports a limited [number of built-in operators](https://github.com/CacheControl/json-rules-engine/blob/master/docs/rules.md#operators) that can be used in conditions. You can add your own operators by adding them to the `operators` array in the `JsonRulesEngineFactCheckerFactory` constructor. For example: + +```diff +const myFactCheckerFactory = new JsonRulesEngineFactCheckerFactory({ + checks: [], + logger, ++ operators: [ new Operator("startsWith", (a, b) => a.startsWith(b) ] +}) +``` + +And you can then use it in your checks like this: + +```js +... +rule: { + conditions: { + any: [ + { + fact: 'version', + operator: 'startsWith', + value: '12', + }, + ], + }, +} +``` From d088a484db72c18d22e4a68a4126a23d98cdbd36 Mon Sep 17 00:00:00 2001 From: goenning Date: Mon, 27 Dec 2021 12:10:12 +0000 Subject: [PATCH 6/6] add import to example Signed-off-by: goenning --- plugins/tech-insights-backend-module-jsonfc/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/tech-insights-backend-module-jsonfc/README.md b/plugins/tech-insights-backend-module-jsonfc/README.md index 6703f4b9f9..aab57fae91 100644 --- a/plugins/tech-insights-backend-module-jsonfc/README.md +++ b/plugins/tech-insights-backend-module-jsonfc/README.md @@ -91,6 +91,8 @@ export const exampleCheck: TechInsightJsonRuleCheck = { json-rules-engine supports a limited [number of built-in operators](https://github.com/CacheControl/json-rules-engine/blob/master/docs/rules.md#operators) that can be used in conditions. You can add your own operators by adding them to the `operators` array in the `JsonRulesEngineFactCheckerFactory` constructor. For example: ```diff ++ import { Operator } from 'json-rules-engine'; + const myFactCheckerFactory = new JsonRulesEngineFactCheckerFactory({ checks: [], logger,