diff --git a/.changeset/popular-radios-visit.md b/.changeset/popular-radios-visit.md new file mode 100644 index 0000000000..94d292d10a --- /dev/null +++ b/.changeset/popular-radios-visit.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-tech-insights-backend-module-jsonfc': patch +--- + +When multiple fact retrievers are used for a check, allow for cases where only one returns a given fact diff --git a/plugins/tech-insights-backend-module-jsonfc/README.md b/plugins/tech-insights-backend-module-jsonfc/README.md index f61666d671..5940d39b15 100644 --- a/plugins/tech-insights-backend-module-jsonfc/README.md +++ b/plugins/tech-insights-backend-module-jsonfc/README.md @@ -86,6 +86,11 @@ export const exampleCheck: TechInsightJsonRuleCheck = { }; ``` +### More than one `factIds` for a check. + +When more than one is supplied, the requested fact **MUST** be present in at least one of the fact retrievers. +The order of the fact retrievers defined in the `factIds` array has no bearing on the checks, the check will merge all facts from the various retrievers, and then check against latest fact . + # 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 --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 0d651be178..7fbc2e320b 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 @@ -129,6 +129,98 @@ const testChecks: Record = { }, ], + twoRetrieversSame: [ + { + id: 'twoRetrieversSameCheck', + name: 'twoRetrieversSameCheck', + type: JSON_RULE_ENGINE_CHECK_TYPE, + description: 'Two Retriever Check For Testing', + factIds: [ + 'test-factretriever', + 'test-factretriever-same-fact', + 'non-existing-factretriever', + ], + rule: { + conditions: { + all: [ + { + fact: 'testnumberfact', + operator: 'greaterThan', + value: 2, + }, + ], + }, + }, + }, + ], + + twoRetrieversDifferent: [ + { + id: 'twoRetrieversDifferentCheck', + name: 'twoRetrieversDifferentCheck', + type: JSON_RULE_ENGINE_CHECK_TYPE, + description: 'Two Retriever Check For Testing', + factIds: ['test-factretriever-different-fact', 'test-factretriever'], + rule: { + conditions: { + all: [ + { + fact: 'testnumberfact', + operator: 'greaterThan', + value: 2, + }, + ], + }, + }, + }, + ], + + twoRetrieversDifferentOrder: [ + { + id: 'twoRetrieversDifferentOrderCheck', + name: 'twoRetrieversDifferentOrderCheck', + type: JSON_RULE_ENGINE_CHECK_TYPE, + description: 'Two Retriever Check For Testing', + factIds: ['test-factretriever', 'test-factretriever-different-fact'], + rule: { + conditions: { + all: [ + { + fact: 'testnumberfact', + operator: 'greaterThan', + value: 2, + }, + ], + }, + }, + }, + ], + + twoRetrieversOnlyOneData: [ + { + id: 'twoRetrieversOnlyOneDataCheck', + name: 'twoRetrieversOnlyOneDataCheck', + type: JSON_RULE_ENGINE_CHECK_TYPE, + description: 'Two Retriever Check For Testing', + factIds: [ + 'test-factretriever', + 'test-factretriever-no-fact', + 'non-existing-factretriever', + ], + rule: { + conditions: { + all: [ + { + fact: 'testnumberfact', + operator: 'greaterThan', + value: 2, + }, + ], + }, + }, + }, + ], + customOperator: [ { id: 'customOperatorTestCheck', @@ -184,16 +276,75 @@ const latestSchemasMock = jest.fn().mockImplementation(() => [ description: '', }, }, -]); -const factsBetweenTimestampsByIdsMock = jest.fn(); -const latestFactsByIdsMock = jest.fn().mockResolvedValue({ - ['test-factretriever']: { - id: 'test-factretriever', - facts: { - testnumberfact: 3, + { + version: '0.0.1', + id: 3, + ref: 'test-factretriever-no-fact', + entityTypes: ['component'], + testnumberfact: { + type: 'integer', + description: '', }, }, -}); + { + version: '0.0.1', + id: 4, + ref: 'test-factretriever-same-fact', + entityTypes: ['component'], + testnumberfact: { + type: 'integer', + description: '', + }, + }, + { + version: '0.0.1', + id: 5, + ref: 'test-factretriever-different-fact', + entityTypes: ['users'], + testnumberfact: { + type: 'integer', + description: '', + }, + }, +]); +const factsBetweenTimestampsByIdsMock = jest.fn(); +const latestFactsByIdsMock = jest + .fn() + .mockImplementation((factIds: string[]) => { + const facts = [ + { + id: 'test-factretriever-different-fact', + facts: { + testnumberfact: 1, + }, + }, + { + id: 'test-factretriever-same-fact', + facts: { + testnumberfact: 3, + }, + }, + { + id: 'test-factretriever', + facts: { + testnumberfact: 3, + }, + }, + { + id: 'test-factretriever-no-fact', + facts: {}, + }, + ]; + + const factResults = Object.fromEntries( + factIds + .map(factId => facts.filter(fact => fact.id === factId)) + .flat() + .map(fact => [fact.id, fact]), + ); + + return factResults; + }); const mockCheckRegistry = { getAll(checks: string[]) { return checks.flatMap(check => testChecks[check]); @@ -281,6 +432,174 @@ describe('JsonRulesEngineFactChecker', () => { }); }); + it('should respond with result, facts, fact schemas and checks for multiple retrievers', async () => { + const results = await factChecker.runChecks('a/a/a', [ + 'twoRetrieversSame', + ]); + expect(results).toHaveLength(1); + expect(results[0]).toMatchObject({ + facts: { + testnumberfact: { + value: 3, + type: 'integer', + description: '', + }, + }, + result: true, + check: { + id: 'twoRetrieversSameCheck', + type: JSON_RULE_ENGINE_CHECK_TYPE, + name: 'twoRetrieversSameCheck', + description: 'Two Retriever Check For Testing', + factIds: [ + 'test-factretriever', + 'test-factretriever-same-fact', + 'non-existing-factretriever', + ], + rule: { + conditions: { + all: [ + { + fact: 'testnumberfact', + factResult: 3, + operator: 'greaterThan', + result: true, + value: 2, + }, + ], + priority: 1, + }, + }, + }, + }); + }); + + it('should respond with result, facts, fact schemas and checks for multiple retrievers with the last fact winning', async () => { + const results = await factChecker.runChecks('a/a/a', [ + 'twoRetrieversDifferent', + 'twoRetrieversDifferentOrder', + ]); + + expect(results).toHaveLength(2); + expect(results[0]).toMatchObject({ + facts: { + testnumberfact: { + value: 3, + type: 'integer', + description: '', + }, + }, + result: true, + check: { + id: 'twoRetrieversDifferentCheck', + type: JSON_RULE_ENGINE_CHECK_TYPE, + name: 'twoRetrieversDifferentCheck', + description: 'Two Retriever Check For Testing', + factIds: ['test-factretriever-different-fact', 'test-factretriever'], + rule: { + conditions: { + all: [ + { + fact: 'testnumberfact', + factResult: 3, + operator: 'greaterThan', + result: true, + value: 2, + }, + ], + priority: 1, + }, + }, + }, + }); + expect(results[1]).toMatchObject({ + facts: { + testnumberfact: { + value: 3, + type: 'integer', + description: '', + }, + }, + result: true, + check: { + id: 'twoRetrieversDifferentOrderCheck', + type: JSON_RULE_ENGINE_CHECK_TYPE, + name: 'twoRetrieversDifferentOrderCheck', + description: 'Two Retriever Check For Testing', + factIds: ['test-factretriever', 'test-factretriever-different-fact'], + rule: { + conditions: { + all: [ + { + fact: 'testnumberfact', + factResult: 3, + operator: 'greaterThan', + result: true, + value: 2, + }, + ], + priority: 1, + }, + }, + }, + }); + }); + + it('the ordering of the factIds do not effect the result, its just which fact is last.', async () => { + const resultsDIfferentFirst = await factChecker.runChecks('a/a/a', [ + 'twoRetrieversDifferent', + ]); + const resultsDIfferentSecond = await factChecker.runChecks('a/a/a', [ + 'twoRetrieversDifferentOrder', + ]); + + expect(resultsDIfferentFirst[0].result).not.toEqual( + resultsDIfferentSecond[0].result, + ); + }); + + it('should respond with result, facts, fact schemas and checks for multiple retrievers one without data', async () => { + const results = await factChecker.runChecks('a/a/a', [ + 'twoRetrieversOnlyOneData', + ]); + expect(results).toHaveLength(1); + expect(results[0]).toMatchObject({ + facts: { + testnumberfact: { + value: 3, + type: 'integer', + description: '', + }, + }, + result: true, + check: { + id: 'twoRetrieversOnlyOneDataCheck', + type: JSON_RULE_ENGINE_CHECK_TYPE, + name: 'twoRetrieversOnlyOneDataCheck', + description: 'Two Retriever Check For Testing', + factIds: [ + 'test-factretriever', + 'test-factretriever-no-fact', + 'non-existing-factretriever', + ], + rule: { + conditions: { + all: [ + { + fact: 'testnumberfact', + factResult: 3, + operator: 'greaterThan', + result: true, + value: 2, + }, + ], + priority: 1, + }, + }, + }, + }); + }); + it('should use custom operators when defined', async () => { const results = await factChecker.runChecks('a/a/a', ['customOperator']); expect(results).toHaveLength(1); 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 c36c0ae6de..721c9b3c62 100644 --- a/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.ts +++ b/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.ts @@ -104,14 +104,13 @@ export class JsonRulesEngineFactChecker : await this.checkRegistry.list(); const factIds = techInsightChecks.flatMap(it => it.factIds); const facts = await this.repository.getLatestFactsByIds(factIds, entity); + techInsightChecks.forEach(techInsightCheck => { const rule = techInsightCheck.rule; rule.name = techInsightCheck.id; // Only run checks that have all the facts available: - const hasAllFacts = techInsightCheck.factIds.every( - factId => facts[factId], - ); - if (hasAllFacts) { + const hasFacts = techInsightCheck.factIds.some(factId => facts[factId]); + if (hasFacts) { engine.addRule({ ...techInsightCheck.rule, event: noopEvent }); } else { this.logger.debug(