From df000b95969d74b8d264543110989283650e05d6 Mon Sep 17 00:00:00 2001 From: Jussi Hallila Date: Mon, 25 Oct 2021 17:04:33 +0200 Subject: [PATCH] Address various code review comments. Signed-off-by: Jussi Hallila --- .changeset/bright-pandas-rush.md | 17 --- packages/backend/src/index.ts | 2 +- packages/backend/src/plugins/techInsights.ts | 71 +++++++++- .../CHANGELOG.md | 7 - .../README.md | 2 +- .../api-report.md | 14 -- .../package.json | 4 +- .../src/index.ts | 1 - .../JsonRulesEngineFactChecker.test.ts | 99 +++++++------- .../src/service/JsonRulesEngineFactChecker.ts | 46 +++---- .../src/types.ts | 16 +-- plugins/tech-insights-backend/CHANGELOG.md | 7 - plugins/tech-insights-backend/README.md | 2 +- plugins/tech-insights-backend/api-report.md | 19 +-- .../migrations/202109061111_fact_schemas.js | 17 ++- .../migrations/202109061212_facts.js | 21 ++- plugins/tech-insights-backend/src/index.ts | 4 +- .../service/fact/FactRetrieverEngine.test.ts | 45 +++---- .../src/service/fact/FactRetrieverEngine.ts | 22 +-- .../src/service/fact/FactRetrieverRegistry.ts | 8 +- .../persistence/TechInsightsDatabase.test.ts | 127 ++++++++++++------ .../persistence/TechInsightsDatabase.ts | 94 ++++++------- .../src/service/router.test.ts | 51 +++---- .../src/service/router.ts | 44 ++++-- ...ilder.ts => techInsightsContextBuilder.ts} | 91 ++++++------- plugins/tech-insights-common/CHANGELOG.md | 7 - plugins/tech-insights-common/api-report.md | 55 ++++---- plugins/tech-insights-common/src/checks.ts | 2 +- plugins/tech-insights-common/src/facts.ts | 64 +++++---- .../tech-insights-common/src/persistence.ts | 32 +++-- plugins/tech-insights-common/src/responses.ts | 11 +- 31 files changed, 529 insertions(+), 473 deletions(-) delete mode 100644 .changeset/bright-pandas-rush.md delete mode 100644 plugins/tech-insights-backend-module-jsonfc/CHANGELOG.md delete mode 100644 plugins/tech-insights-backend/CHANGELOG.md rename plugins/tech-insights-backend/src/service/{DefaultTechInsightsBuilder.ts => techInsightsContextBuilder.ts} (63%) delete mode 100644 plugins/tech-insights-common/CHANGELOG.md diff --git a/.changeset/bright-pandas-rush.md b/.changeset/bright-pandas-rush.md deleted file mode 100644 index 86b3b819ee..0000000000 --- a/.changeset/bright-pandas-rush.md +++ /dev/null @@ -1,17 +0,0 @@ ---- -'@backstage/plugin-tech-insights-backend': minor -'@backstage/plugin-tech-insights-backend-module-jsonfc': minor -'@backstage/plugin-tech-insights-common': minor ---- - -## Add initial implementation of Tech Insights backend - -Add common types and interfaces for Tech Insights. Exposing components needed to use and modify tech-insights-backend module and to implement individual Fact Retrievers, Fact Checkers and their persistence options. - -Implement a framework to run fact retrievers and store fact data into the database. Add migration scripts to create a new database for `tech_insights`. - -Create a default implementation of a FactChecker enabling users to construct checks and run them and generate scorecards based on checks. - -To be able to use tech insights in your application you need to implement `FactRetriever`s to retrieve and return data for facts and register them to the tech-insights-backend. If you want to use fact checking functionality, you need to create `check`s and register them to an implementation of a `FactChecker`. - -For more information see documentation on the README.md files of the respective packages. diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index 6d40072f99..ffdce949b7 100644 --- a/packages/backend/src/index.ts +++ b/packages/backend/src/index.ts @@ -109,7 +109,7 @@ async function main() { const badgesEnv = useHotMemoize(module, () => createEnv('badges')); const jenkinsEnv = useHotMemoize(module, () => createEnv('jenkins')); const techInsightsEnv = useHotMemoize(module, () => - createEnv('tech_insights'), + createEnv('tech-insights'), ); const apiRouter = Router(); diff --git a/packages/backend/src/plugins/techInsights.ts b/packages/backend/src/plugins/techInsights.ts index c396f4b82b..d51c0c7575 100644 --- a/packages/backend/src/plugins/techInsights.ts +++ b/packages/backend/src/plugins/techInsights.ts @@ -15,11 +15,15 @@ */ import { createRouter, - DefaultTechInsightsBuilder, + buildTechInsightsContext, } from '@backstage/plugin-tech-insights-backend'; import { Router } from 'express'; import { PluginEnvironment } from '../types'; -import { JsonRulesEngineFactCheckerFactory } from '@backstage/plugin-tech-insights-backend-module-jsonfc'; +import { + JSON_RULE_ENGINE_CHECK_TYPE, + JsonRulesEngineFactCheckerFactory, +} from '@backstage/plugin-tech-insights-backend-module-jsonfc'; +import { CatalogClient } from '@backstage/catalog-client'; export default async function createPlugin({ logger, @@ -27,20 +31,75 @@ export default async function createPlugin({ discovery, database, }: PluginEnvironment): Promise { - const builder = new DefaultTechInsightsBuilder({ + const techInsightsContext = await buildTechInsightsContext({ logger, config, database, discovery, - factRetrievers: [], + factRetrievers: [ + { + cadence: '1 1 1 * *', // At 01:01 on day-of-month 1. + factRetriever: { + id: 'testRetriever', + version: '1.1.1', + entityTypes: ['component'], + schema: { + examplenumberfact: { + type: 'integer', + description: '', + }, + }, + handler: async _ctx => { + const catalogClient = new CatalogClient({ + discoveryApi: discovery, + }); + const entities = await catalogClient.getEntities(); + + return Promise.resolve( + entities.items.map(it => { + return { + entity: { + namespace: it.metadata.namespace!!, + kind: it.kind, + name: it.metadata.name, + }, + facts: { + examplenumberfact: 2, + }, + }; + }), + ); + }, + }, + }, + ], factCheckerFactory: new JsonRulesEngineFactCheckerFactory({ - checks: [], + checks: [ + { + id: 'simpleTestCheck', + type: JSON_RULE_ENGINE_CHECK_TYPE, + name: 'simpleTestCheck', + description: 'Simple Check For Testing', + factIds: ['testRetriever'], + rule: { + conditions: { + all: [ + { + fact: 'examplenumberfact', + operator: 'lessThan', + value: 5, + }, + ], + }, + }, + }, + ], logger, }), }); return await createRouter({ - ...(await builder.build()), + ...techInsightsContext, logger, config, }); diff --git a/plugins/tech-insights-backend-module-jsonfc/CHANGELOG.md b/plugins/tech-insights-backend-module-jsonfc/CHANGELOG.md deleted file mode 100644 index d0aced5bf5..0000000000 --- a/plugins/tech-insights-backend-module-jsonfc/CHANGELOG.md +++ /dev/null @@ -1,7 +0,0 @@ -# @backstage/plugin-tech-insights-backend-module-jsonfc - -## 0.0.1 - -### Patch Changes - -- Initial implementation diff --git a/plugins/tech-insights-backend-module-jsonfc/README.md b/plugins/tech-insights-backend-module-jsonfc/README.md index 99570f1c7a..1404bd4ea4 100644 --- a/plugins/tech-insights-backend-module-jsonfc/README.md +++ b/plugins/tech-insights-backend-module-jsonfc/README.md @@ -2,7 +2,7 @@ This is an extension to module to tech-insights-backend plugin, which provides basic framework and functionality to implement tech insights within Backstage. -This module provides functionality to run checks against a `json-rules-engine` and provide boolean logic by simply building checks using JSON conditions. +This module provides functionality to run checks against a [json-rules-engine](https://github.com/CacheControl/json-rules-engine) and provide boolean logic by simply building checks using JSON conditions. ## Getting started diff --git a/plugins/tech-insights-backend-module-jsonfc/api-report.md b/plugins/tech-insights-backend-module-jsonfc/api-report.md index 8f6bf0cd6c..3d214566c5 100644 --- a/plugins/tech-insights-backend-module-jsonfc/api-report.md +++ b/plugins/tech-insights-backend-module-jsonfc/api-report.md @@ -6,9 +6,7 @@ import { BooleanCheckResult } from '@backstage/plugin-tech-insights-common'; import { CheckResponse } from '@backstage/plugin-tech-insights-common'; import { CheckValidationResponse } from '@backstage/plugin-tech-insights-common'; -import { DynamicFactCallback } from 'json-rules-engine'; import { FactChecker } from '@backstage/plugin-tech-insights-common'; -import { FactOptions } from 'json-rules-engine'; import { Logger as Logger_2 } from 'winston'; import { TechInsightCheck } from '@backstage/plugin-tech-insights-common'; import { TechInsightCheckRegistry } from '@backstage/plugin-tech-insights-common'; @@ -24,16 +22,6 @@ export type CheckCondition = { result: boolean; }; -// @public (undocumented) -export interface DynamicFact { - // (undocumented) - calculationMethod: DynamicFactCallback | T; - // (undocumented) - id: string; - // (undocumented) - options?: FactOptions; -} - // @public (undocumented) export const JSON_RULE_ENGINE_CHECK_TYPE = 'json-rules-engine'; @@ -120,8 +108,6 @@ export type Rule = { // @public (undocumented) export interface TechInsightJsonRuleCheck extends TechInsightCheck { - // (undocumented) - dynamicFacts?: DynamicFact[]; // (undocumented) rule: Rule; } diff --git a/plugins/tech-insights-backend-module-jsonfc/package.json b/plugins/tech-insights-backend-module-jsonfc/package.json index 945947dd3b..c11f20ee38 100644 --- a/plugins/tech-insights-backend-module-jsonfc/package.json +++ b/plugins/tech-insights-backend-module-jsonfc/package.json @@ -1,8 +1,8 @@ { "name": "@backstage/plugin-tech-insights-backend-module-jsonfc", "version": "0.1.0", - "main": "src/index.ts", - "types": "src/index.ts", + "main": "dist/index.cjs.js", + "types": "dist/index.d.ts", "license": "Apache-2.0", "private": false, "publishConfig": { diff --git a/plugins/tech-insights-backend-module-jsonfc/src/index.ts b/plugins/tech-insights-backend-module-jsonfc/src/index.ts index 1a537b8257..1239211f73 100644 --- a/plugins/tech-insights-backend-module-jsonfc/src/index.ts +++ b/plugins/tech-insights-backend-module-jsonfc/src/index.ts @@ -28,6 +28,5 @@ export type { TechInsightJsonRuleCheck, ResponseTopLevelCondition, Rule, - DynamicFact, CheckCondition, } from './types'; 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 124030a545..a31ae90c70 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 @@ -32,7 +32,7 @@ const testChecks: Record = { name: 'brokenTestCheck', type: JSON_RULE_ENGINE_CHECK_TYPE, description: 'Broken Check For Testing', - factRefs: ['test-factretriever'], + factIds: ['test-factretriever'], rule: { conditions: { all: [ @@ -52,7 +52,7 @@ const testChecks: Record = { name: 'brokenTestCheck2', type: JSON_RULE_ENGINE_CHECK_TYPE, description: 'Second Broken Check For Testing', - factRefs: ['non-existing-factretriever'], + factIds: ['non-existing-factretriever'], rule: { conditions: { any: [ @@ -72,7 +72,7 @@ const testChecks: Record = { name: 'simpleTestCheck', type: JSON_RULE_ENGINE_CHECK_TYPE, description: 'Simple Check For Testing', - factRefs: ['test-factretriever'], + factIds: ['test-factretriever'], rule: { conditions: { all: [ @@ -93,7 +93,7 @@ const testChecks: Record = { name: 'simpleTestCheck2', type: JSON_RULE_ENGINE_CHECK_TYPE, description: 'Second Simple Check For Testing', - factRefs: ['test-factretriever'], + factIds: ['test-factretriever'], rule: { conditions: { all: [ @@ -114,17 +114,15 @@ const latestSchemasMock = jest.fn().mockImplementation(() => [ version: '0.0.1', id: 2, ref: 'test-factretriever', - schema: { - testnumberfact: { - type: 'integer', - description: '', - entityKinds: ['component'], - }, + entityTypes: ['component'], + testnumberfact: { + type: 'integer', + description: '', }, }, ]); -const factsBetweenTimestampsForRefsMock = jest.fn(); -const latestFactsForRefsMock = jest.fn().mockImplementation(() => ({})); +const factsBetweenTimestampsByIdsMock = jest.fn(); +const latestFactsByIdsMock = jest.fn().mockImplementation(() => ({})); const mockCheckRegistry = { getAll(checks: string[]) { return checks.flatMap(check => testChecks[check]); @@ -132,8 +130,8 @@ const mockCheckRegistry = { } as unknown as TechInsightCheckRegistry; const mockRepository: TechInsightsStore = { - getLatestFactsForRefs: latestFactsForRefsMock, - getFactsBetweenTimestampsForRefs: factsBetweenTimestampsForRefsMock, + getLatestFactsByIds: latestFactsByIdsMock, + getFactsBetweenTimestampsByIds: factsBetweenTimestampsByIdsMock, getLatestSchemas: latestSchemasMock, } as unknown as TechInsightsStore; @@ -159,10 +157,10 @@ describe('JsonRulesEngineFactChecker', () => { ); }); it('should respond with result, facts, fact schemas and checks', async () => { - latestFactsForRefsMock.mockImplementation(() => + latestFactsByIdsMock.mockImplementation(() => Promise.resolve({ ['test-factretriever']: { - ref: 'test-factretriever', + id: 'test-factretriever', facts: { testnumberfact: 3, }, @@ -170,46 +168,45 @@ describe('JsonRulesEngineFactChecker', () => { }), ); const results = await factChecker.runChecks('a/a/a', ['simple']); - expect(results).toMatchObject([ - { - facts: { - testnumberfact: { - value: 3, - type: 'integer', - description: '', - entityKinds: ['component'], - }, + expect(results).toHaveLength(1); + expect(results[0]).toMatchObject({ + facts: { + testnumberfact: { + value: 3, + type: 'integer', + description: '', }, - result: true, - check: { - id: 'simpleTestCheck', - name: 'simpleTestCheck', - description: 'Simple Check For Testing', - factRefs: ['test-factretriever'], - rule: { - conditions: { - all: [ - { - fact: 'testnumberfact', - factResult: 3, - operator: 'lessThan', - result: true, - value: 5, - }, - ], - priority: 1, - }, + }, + result: true, + check: { + id: 'simpleTestCheck', + type: JSON_RULE_ENGINE_CHECK_TYPE, + name: 'simpleTestCheck', + description: 'Simple Check For Testing', + factIds: ['test-factretriever'], + rule: { + conditions: { + all: [ + { + fact: 'testnumberfact', + factResult: 3, + operator: 'lessThan', + result: true, + value: 5, + }, + ], + priority: 1, }, }, }, - ]); + }); }); it('should gracefully handle multiple check at once', async () => { - latestFactsForRefsMock.mockImplementation(() => + latestFactsByIdsMock.mockImplementation(() => Promise.resolve({ ['test-factretriever']: { - ref: 'test-factretriever', + id: 'test-factretriever', facts: { testnumberfact: 3, }, @@ -227,15 +224,15 @@ describe('JsonRulesEngineFactChecker', () => { value: 3, type: 'integer', description: '', - entityKinds: ['component'], }, }, result: true, check: { id: 'simpleTestCheck', + type: JSON_RULE_ENGINE_CHECK_TYPE, name: 'simpleTestCheck', description: 'Simple Check For Testing', - factRefs: ['test-factretriever'], + factIds: ['test-factretriever'], rule: { conditions: { priority: 1, @@ -258,15 +255,15 @@ describe('JsonRulesEngineFactChecker', () => { value: 3, type: 'integer', description: '', - entityKinds: ['component'], }, }, result: true, check: { id: 'simpleTestCheck2', + type: JSON_RULE_ENGINE_CHECK_TYPE, name: 'simpleTestCheck2', description: 'Second Simple Check For Testing', - factRefs: ['test-factretriever'], + factIds: ['test-factretriever'], rule: { conditions: { priority: 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 7e8cab8b7f..ab90a3d828 100644 --- a/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.ts +++ b/plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.ts @@ -18,7 +18,6 @@ import { JsonRuleBooleanCheckResult, TechInsightJsonRuleCheck } from '../types'; import { FactChecker, FactResponse, - FactValueDefinitions, TechInsightCheckRegistry, FlatTechInsightFact, TechInsightsStore, @@ -81,20 +80,13 @@ export class JsonRulesEngineFactChecker ): Promise { const engine = new Engine(); const techInsightChecks = await this.checkRegistry.getAll(checks); - const factRefs = techInsightChecks.flatMap(it => it.factRefs); - const facts = await this.repository.getLatestFactsForRefs(factRefs, entity); + 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; engine.addRule({ ...techInsightCheck.rule, event: noopEvent }); - - if (techInsightCheck.dynamicFacts) { - techInsightCheck.dynamicFacts.forEach(it => - engine.addFact(it.id, it.calculationMethod, it.options), - ); - } }); - const factValues = Object.values(facts).reduce( (acc, it) => ({ ...acc, ...it.facts }), {}, @@ -140,21 +132,21 @@ export class JsonRulesEngineFactChecker } const existingSchemas = await this.repository.getLatestSchemas( - check.factRefs, + check.factIds, + ); + const references = this.retrieveIndividualFactReferences( + check.rule.conditions, ); - const references = this.retrieveFactReferences(check.rule.conditions); const results = references.map(ref => ({ ref, - result: existingSchemas.some(schema => schema.schema.hasOwnProperty(ref)), + result: existingSchemas.some(schema => schema.hasOwnProperty(ref)), })); const failedReferences = results.filter(it => !it.result); failedReferences.forEach(it => { this.logger.warn( `Validation failed for check ${check.name}. Reference to value ${ it.ref - } does not exists in referred fact schemas: ${check.factRefs.join( - ',', - )}`, + } does not exists in referred fact schemas: ${check.factIds.join(',')}`, ); }); const valid = failedReferences.length === 0; @@ -189,17 +181,21 @@ export class JsonRulesEngineFactChecker return await this.checkRegistry.register(check); } - private retrieveFactReferences( + private retrieveIndividualFactReferences( condition: TopLevelCondition | { fact: string }, ): string[] { let results: string[] = []; if ('all' in condition) { results = results.concat( - condition.all.flatMap(con => this.retrieveFactReferences(con)), + condition.all.flatMap(con => + this.retrieveIndividualFactReferences(con), + ), ); } else if ('any' in condition) { results = results.concat( - condition.any.flatMap(con => this.retrieveFactReferences(con)), + condition.any.flatMap(con => + this.retrieveIndividualFactReferences(con), + ), ); } else { results.push(condition.fact); @@ -251,7 +247,7 @@ export class JsonRulesEngineFactChecker type: techInsightCheck.type, name: techInsightCheck.name, description: techInsightCheck.description, - factRefs: techInsightCheck.factRefs, + factIds: techInsightCheck.factIds, metadata: result.result ? techInsightCheck.successMetadata : techInsightCheck.failureMetadata, @@ -272,18 +268,18 @@ export class JsonRulesEngineFactChecker techInsightCheck: TechInsightJsonRuleCheck, ): Promise { const factSchemas = await this.repository.getLatestSchemas( - techInsightCheck.factRefs, + techInsightCheck.factIds, ); - const schemas: FactValueDefinitions = factSchemas.reduce( - (acc, schema) => ({ ...acc, ...schema.schema }), + const schemas = factSchemas.reduce( + (acc, schema) => ({ ...acc, ...schema }), {}, ); - const individualFacts = this.retrieveFactReferences( + const individualFacts = this.retrieveIndividualFactReferences( techInsightCheck.rule.conditions, ); const factValues = facts .filter(factContainer => - techInsightCheck.factRefs.includes(factContainer.ref), + techInsightCheck.factIds.includes(factContainer.id), ) .reduce( (acc, factContainer) => ({ diff --git a/plugins/tech-insights-backend-module-jsonfc/src/types.ts b/plugins/tech-insights-backend-module-jsonfc/src/types.ts index 5f6210d110..23d5849eda 100644 --- a/plugins/tech-insights-backend-module-jsonfc/src/types.ts +++ b/plugins/tech-insights-backend-module-jsonfc/src/types.ts @@ -13,26 +13,13 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { - DynamicFactCallback, - FactOptions, - TopLevelCondition, -} from 'json-rules-engine'; +import { TopLevelCondition } from 'json-rules-engine'; import { BooleanCheckResult, CheckResponse, TechInsightCheck, } from '@backstage/plugin-tech-insights-common'; -/** - * @public - */ -export interface DynamicFact { - id: string; - calculationMethod: DynamicFactCallback | T; - options?: FactOptions; -} - /** * @public */ @@ -47,7 +34,6 @@ export type Rule = { */ export interface TechInsightJsonRuleCheck extends TechInsightCheck { rule: Rule; - dynamicFacts?: DynamicFact[]; } /** diff --git a/plugins/tech-insights-backend/CHANGELOG.md b/plugins/tech-insights-backend/CHANGELOG.md deleted file mode 100644 index 17e0211ab0..0000000000 --- a/plugins/tech-insights-backend/CHANGELOG.md +++ /dev/null @@ -1,7 +0,0 @@ -# @backstage/plugin-tech-insights-backend - -## 0.0.1 - -### Patch Changes - -- Initial implementation diff --git a/plugins/tech-insights-backend/README.md b/plugins/tech-insights-backend/README.md index 4e5b5ad1b3..32265c1ced 100644 --- a/plugins/tech-insights-backend/README.md +++ b/plugins/tech-insights-backend/README.md @@ -124,7 +124,7 @@ const myFactRetriever: FactRetriever = { examplenumberfact: { type: 'integer', // Type of the fact description: 'A fact of a number', // Description of the fact - entityKinds: ['component'], // An array of entity kinds that this fact is applicable to + entityTypes: ['component'], // An array of entity kinds that this fact is applicable to }, }, }, diff --git a/plugins/tech-insights-backend/api-report.md b/plugins/tech-insights-backend/api-report.md index fc1bd9217b..6bd161c092 100644 --- a/plugins/tech-insights-backend/api-report.md +++ b/plugins/tech-insights-backend/api-report.md @@ -15,21 +15,22 @@ import { PluginEndpointDiscovery } from '@backstage/backend-common'; import { TechInsightCheck } from '@backstage/plugin-tech-insights-common'; import { TechInsightsStore } from '@backstage/plugin-tech-insights-common'; +// Warning: (ae-missing-release-tag) "buildTechInsightsContext" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) +// +// @public +export const buildTechInsightsContext: < + CheckType extends TechInsightCheck, + CheckResultType extends CheckResult, +>( + options: TechInsightsOptions, +) => Promise>; + // @public export function createRouter< CheckType extends TechInsightCheck, CheckResultType extends CheckResult, >(options: RouterOptions): Promise; -// @public (undocumented) -export class DefaultTechInsightsBuilder< - CheckType extends TechInsightCheck, - CheckResultType extends CheckResult, -> { - constructor(options: TechInsightsOptions); - build(): Promise>; -} - // @public export type PersistenceContext = { techInsightsStore: TechInsightsStore; diff --git a/plugins/tech-insights-backend/migrations/202109061111_fact_schemas.js b/plugins/tech-insights-backend/migrations/202109061111_fact_schemas.js index 4afe47061f..64f6c11e83 100644 --- a/plugins/tech-insights-backend/migrations/202109061111_fact_schemas.js +++ b/plugins/tech-insights-backend/migrations/202109061111_fact_schemas.js @@ -24,24 +24,28 @@ exports.up = async function up(knex) { table.comment( 'The table for tech insight fact schemas. Containing a versioned data model definition for a collection of facts.', ); - table.increments('id').primary(); table - .text('ref') + .text('id') .notNullable() .comment('Identifier of the fact retriever plugin/package'); table .string('version') .notNullable() .comment('SemVer string defining the version of schema.'); + table + .string('entityTypes') + .nullable() + .comment( + 'A comma separated collection of entity kinds the fact retriever providing this schema affects. Defaults to null, which means all entity kinds.', + ); table .text('schema') .notNullable() .comment( 'Fact schema defining the values/types what this version of the fact would contain.', ); - - table.index('ref', 'fact_schema_ref_idx'); - table.index(['ref', 'version'], 'fact_schema_ref_version_idx'); + table.primary(['id', 'version']); + table.index('id', 'fact_schema_id_idx'); }); }; @@ -50,8 +54,7 @@ exports.up = async function up(knex) { */ exports.down = async function down(knex) { await knex.schema.alterTable('fact_schemas', table => { - table.dropIndex([], 'fact_schema_ref_idx'); - table.dropIndex([], 'fact_schema_ref_version_idx'); + table.dropIndex([], 'fact_schema_id_idx'); }); await knex.schema.dropTable('fact_schemas'); }; diff --git a/plugins/tech-insights-backend/migrations/202109061212_facts.js b/plugins/tech-insights-backend/migrations/202109061212_facts.js index f80883ef1d..0bdcdc0da1 100644 --- a/plugins/tech-insights-backend/migrations/202109061212_facts.js +++ b/plugins/tech-insights-backend/migrations/202109061212_facts.js @@ -25,11 +25,7 @@ exports.up = async function up(knex) { 'The table for tech insight fact collections. Contains facts for individual fact retriever namespace/ref.', ); table - .bigIncrements('index') - .notNullable() - .comment('An insert counter to ensure ordering'); - table - .text('ref') + .text('id') .notNullable() .comment('Unique identifier of the fact retriever plugin/package'); table @@ -54,9 +50,13 @@ exports.up = async function up(knex) { 'Values of the fact collection stored as key-value pairs in JSON format.', ); - table.index('index', 'fact_index_idx'); - table.index('ref', 'fact_ref_idx'); - table.index(['ref', 'entity'], 'fact_ref_entity_idx'); + table + .foreign(['id', 'version']) + .references(['id', 'version']) + .inTable('fact_schemas'); + + table.index(['id', 'entity'], 'fact_id_entity_idx'); + table.index('id', 'fact_id_idx'); }); }; @@ -65,9 +65,8 @@ exports.up = async function up(knex) { */ exports.down = async function down(knex) { await knex.schema.alterTable('facts', table => { - table.dropIndex([], 'facts_index_idx'); - table.dropIndex([], 'fact_ref_idx'); - table.dropIndex([], 'fact_ref_entity_idx'); + table.dropIndex([], 'fact_id_idx'); + table.dropIndex([], 'fact_id_entity_idx'); }); await knex.schema.dropTable('facts'); }; diff --git a/plugins/tech-insights-backend/src/index.ts b/plugins/tech-insights-backend/src/index.ts index eb54e8cfed..fd7f3ccd53 100644 --- a/plugins/tech-insights-backend/src/index.ts +++ b/plugins/tech-insights-backend/src/index.ts @@ -17,10 +17,10 @@ export * from './service/router'; export type { RouterOptions } from './service/router'; -export { DefaultTechInsightsBuilder } from './service/DefaultTechInsightsBuilder'; +export { buildTechInsightsContext } from './service/techInsightsContextBuilder'; export type { TechInsightsOptions, TechInsightsContext, -} from './service/DefaultTechInsightsBuilder'; +} from './service/techInsightsContextBuilder'; export type { PersistenceContext } from './service/persistence/DatabaseManager'; diff --git a/plugins/tech-insights-backend/src/service/fact/FactRetrieverEngine.test.ts b/plugins/tech-insights-backend/src/service/fact/FactRetrieverEngine.test.ts index 6b9b58e3b8..ef53ae8412 100644 --- a/plugins/tech-insights-backend/src/service/fact/FactRetrieverEngine.test.ts +++ b/plugins/tech-insights-backend/src/service/fact/FactRetrieverEngine.test.ts @@ -16,7 +16,7 @@ import { FactRetriever, FactRetrieverRegistration, - FactSchema, + FactSchemaDefinition, TechInsightFact, TechInsightsStore, } from '@backstage/plugin-tech-insights-common'; @@ -35,21 +35,18 @@ jest.mock('node-cron', () => { }); const testFactRetriever: FactRetriever = { - ref: 'test-factretriever', + id: 'test-factretriever', + version: '0.0.1', + entityTypes: ['component'], schema: { - version: '0.0.1', - schema: { - testnumberfact: { - type: 'integer', - description: '', - entityKinds: ['component'], - }, + testnumberfact: { + type: 'integer', + description: '', }, }, handler: async () => { return [ { - ref: 'test-factretriever', entity: { namespace: 'a', kind: 'a', @@ -65,7 +62,9 @@ const testFactRetriever: FactRetriever = { const cadence = '1 * * * *'; describe('FactRetrieverEngine', () => { let engine: FactRetrieverEngine; - let factSchemaAssertionCallback: (ref: string, schema: FactSchema) => void; + let factSchemaAssertionCallback: ( + factSchemaDefinition: FactSchemaDefinition, + ) => void; let factInsertionAssertionCallback: (facts: TechInsightFact[]) => void; const mockRepository: TechInsightsStore = { @@ -73,8 +72,8 @@ describe('FactRetrieverEngine', () => { factInsertionAssertionCallback(facts); return Promise.resolve(); }, - insertFactSchema: (ref: string, schema: FactSchema) => { - factSchemaAssertionCallback(ref, schema); + insertFactSchema: (def: FactSchemaDefinition) => { + factSchemaAssertionCallback(def); return Promise.resolve(); }, } as unknown as TechInsightsStore; @@ -102,21 +101,19 @@ describe('FactRetrieverEngine', () => { }; it('Should update fact retriever schemas on initialization', async () => { - factSchemaAssertionCallback = (ref, schema) => { - expect(ref).toEqual('test-factretriever'); + factSchemaAssertionCallback = ({ id, schema, version, entityTypes }) => { + expect(id).toEqual('test-factretriever'); + expect(version).toEqual('0.0.1'); + expect(entityTypes).toEqual(['component']); expect(schema).toEqual({ - version: '0.0.1', - schema: { - testnumberfact: { - type: 'integer', - description: '', - entityKinds: ['component'], - }, + testnumberfact: { + type: 'integer', + description: '', }, }); }; - engine = await FactRetrieverEngine.fromConfig(defaultEngineConfig); + engine = await FactRetrieverEngine.create(defaultEngineConfig); }); it('Should insert facts when scheduled step is run', async () => { (schedule as jest.Mock).mockImplementation( @@ -143,7 +140,7 @@ describe('FactRetrieverEngine', () => { }, }); }; - engine = await FactRetrieverEngine.fromConfig(defaultEngineConfig); + engine = await FactRetrieverEngine.create(defaultEngineConfig); engine.schedule(); const job: any = engine.getJob('test-factretriever'); job.triggerScheduledJobNow(); diff --git a/plugins/tech-insights-backend/src/service/fact/FactRetrieverEngine.ts b/plugins/tech-insights-backend/src/service/fact/FactRetrieverEngine.ts index 01e3c12370..6dc3396819 100644 --- a/plugins/tech-insights-backend/src/service/fact/FactRetrieverEngine.ts +++ b/plugins/tech-insights-backend/src/service/fact/FactRetrieverEngine.ts @@ -45,7 +45,7 @@ export class FactRetrieverEngine { private readonly defaultCadence?: string, ) {} - static async fromConfig({ + static async create({ repository, factRetrieverRegistry, factRetrieverContext, @@ -59,7 +59,7 @@ export class FactRetrieverEngine { await Promise.all( factRetrieverRegistry .listRetrievers() - .map(it => repository.insertFactSchema(it.ref, it.schema)), + .map(it => repository.insertFactSchema(it)), ); return new FactRetrieverEngine( @@ -76,12 +76,12 @@ export class FactRetrieverEngine { const newRegs: string[] = []; registrations.forEach(registration => { const { factRetriever, cadence } = registration; - if (!this.scheduledJobs.has(factRetriever.ref)) { + if (!this.scheduledJobs.has(factRetriever.id)) { const cronExpression = cadence || this.defaultCadence || randomDailyCron(); if (!validate(cronExpression)) { this.logger.warn( - `Validation failed for cron expression ${cronExpression} when trying to schedule fact retriever ${factRetriever.ref}`, + `Validation failed for cron expression ${cronExpression} when trying to schedule fact retriever ${factRetriever.id}`, ); return; } @@ -89,8 +89,8 @@ export class FactRetrieverEngine { cronExpression, this.createFactRetrieverHandler(factRetriever), ); - this.scheduledJobs.set(factRetriever.ref, job); - newRegs.push(factRetriever.ref); + this.scheduledJobs.set(factRetriever.id, job); + newRegs.push(factRetriever.id); } }); this.logger.info( @@ -106,27 +106,27 @@ export class FactRetrieverEngine { return async () => { const startTimestamp = process.hrtime(); this.logger.info( - `Retrieving facts for fact retriever ${factRetriever.ref}`, + `Retrieving facts for fact retriever ${factRetriever.id}`, ); const facts = await factRetriever.handler(this.factRetrieverContext); if (this.logger.isDebugEnabled()) { this.logger.debug( `Retrieved ${facts.length} facts for fact retriever ${ - factRetriever.ref + factRetriever.id } in ${duration(startTimestamp)}`, ); } try { - await this.repository.insertFacts(factRetriever.ref, facts); + await this.repository.insertFacts(factRetriever.id, facts); this.logger.info( `Stored ${facts.length} facts for fact retriever ${ - factRetriever.ref + factRetriever.id } in ${duration(startTimestamp)}`, ); } catch (e) { this.logger.warn( - `Failed to insert facts for fact retriever ${factRetriever.ref}`, + `Failed to insert facts for fact retriever ${factRetriever.id}`, e, ); } diff --git a/plugins/tech-insights-backend/src/service/fact/FactRetrieverRegistry.ts b/plugins/tech-insights-backend/src/service/fact/FactRetrieverRegistry.ts index c76bb6039d..59d1483720 100644 --- a/plugins/tech-insights-backend/src/service/fact/FactRetrieverRegistry.ts +++ b/plugins/tech-insights-backend/src/service/fact/FactRetrieverRegistry.ts @@ -31,19 +31,19 @@ export class FactRetrieverRegistry { } register(registration: FactRetrieverRegistration) { - if (this.retrievers.has(registration.factRetriever.ref)) { + if (this.retrievers.has(registration.factRetriever.id)) { throw new ConflictError( - `Tech insight fact retriever with reference '${registration.factRetriever.ref}' has already been registered`, + `Tech insight fact retriever with identifier '${registration.factRetriever.id}' has already been registered`, ); } - this.retrievers.set(registration.factRetriever.ref, registration); + this.retrievers.set(registration.factRetriever.id, registration); } get(retrieverReference: string): FactRetriever { const registration = this.retrievers.get(retrieverReference); if (!registration) { throw new NotFoundError( - `Tech insight fact retriever with reference '${retrieverReference}' is not registered.`, + `Tech insight fact retriever with identifier '${retrieverReference}' is not registered.`, ); } return registration.factRetriever; diff --git a/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.test.ts b/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.test.ts index ab54cf819d..44a376d5da 100644 --- a/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.test.ts +++ b/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.test.ts @@ -20,47 +20,58 @@ import { Knex } from 'knex'; const factSchemas = [ { - ref: 'test-schema', + id: 'test-fact', version: '0.0.1-test', + entityTypes: ['component'], schema: JSON.stringify({ testNumberFact: { type: 'integer', description: 'Test fact with a number type', - entityKinds: ['component'], }, }), }, ]; const additionalFactSchemas = [ { - ref: 'test-schema', + id: 'test-fact', version: '1.2.1-test', + entityTypes: ['component'], schema: JSON.stringify({ testNumberFact: { type: 'integer', description: 'Test fact with a number type', - entityKinds: ['component'], }, testStringFact: { type: 'string', description: 'Test fact with a string type', - entityKinds: ['service'], }, }), }, { - ref: 'test-schema', + id: 'test-fact', version: '1.1.1-test', + entityTypes: ['component'], schema: JSON.stringify({ testStringFact: { type: 'string', description: 'Test fact with a string type', - entityKinds: ['service'], }, }), }, ]; +const secondSchema = { + id: 'second-test-fact', + version: '0.0.1-test', + entityTypes: ['service'], + schema: JSON.stringify({ + testStringFact: { + type: 'string', + description: 'Test fact with a string type', + }, + }), +}; + const now = DateTime.now().toISO(); const shortlyInTheFuture = DateTime.now() .plus(Duration.fromMillis(555)) @@ -72,18 +83,18 @@ const farInTheFuture = DateTime.now() const facts = [ { timestamp: now, - ref: 'test-fact', + id: 'test-fact', version: '0.0.1-test', - entity: 'a/a/a', + entity: 'a:a/a', facts: JSON.stringify({ testNumberFact: 1, }), }, { timestamp: shortlyInTheFuture, - ref: 'test-fact', + id: 'test-fact', version: '0.0.1-test', - entity: 'a/a/a', + entity: 'a:a/a', facts: JSON.stringify({ testNumberFact: 2, }), @@ -93,9 +104,9 @@ const facts = [ const additionalFacts = [ { timestamp: farInTheFuture, - ref: 'test-fact', + id: 'test-fact', version: '0.0.1-test', - entity: 'a/a/a', + entity: 'a:a/a', facts: JSON.stringify({ testNumberFact: 3, }), @@ -114,7 +125,7 @@ describe('Tech Insights database', () => { }); const baseAssertionFact = { - ref: 'test-fact', + id: 'test-fact', entity: { namespace: 'a', kind: 'a', name: 'a' }, timestamp: DateTime.fromISO(shortlyInTheFuture), version: '0.0.1-test', @@ -124,14 +135,12 @@ describe('Tech Insights database', () => { it('should be able to return latest schema', async () => { const schemas = await store.getLatestSchemas(); expect(schemas[0]).toMatchObject({ - ref: 'test-schema', + id: 'test-fact', version: '0.0.1-test', - schema: { - testNumberFact: { - type: 'integer', - description: 'Test fact with a number type', - entityKinds: ['component'], - }, + entityTypes: ['component'], + testNumberFact: { + type: 'integer', + description: 'Test fact with a number type', }, }); }); @@ -141,43 +150,75 @@ describe('Tech Insights database', () => { const schemas = await store.getLatestSchemas(); expect(schemas[0]).toMatchObject({ - ref: 'test-schema', + id: 'test-fact', version: '1.2.1-test', - schema: { - testNumberFact: { - type: 'integer', - description: 'Test fact with a number type', - entityKinds: ['component'], - }, - testStringFact: { - type: 'string', - description: 'Test fact with a string type', - entityKinds: ['service'], - }, + entityTypes: ['component'], + testNumberFact: { + type: 'integer', + description: 'Test fact with a number type', + }, + testStringFact: { + type: 'string', + description: 'Test fact with a string type', }, }); }); - it('should return latest facts only for the correct ref', async () => { - const returnedFact = await store.getLatestFactsForRefs( + it('should return multiple schemas if those exists', async () => { + await testDbClient.batchInsert('fact_schemas', [ + { + ...secondSchema, + id: 'second', + }, + ]); + + const schemas = await store.getLatestSchemas(); + expect(schemas).toHaveLength(2); + expect(schemas[0]).toMatchObject({ + id: 'test-fact', + version: '1.2.1-test', + entityTypes: ['component'], + testNumberFact: { + type: 'integer', + description: 'Test fact with a number type', + }, + testStringFact: { + type: 'string', + description: 'Test fact with a string type', + }, + }); + expect(schemas[1]).toMatchObject({ + id: 'second', + version: '0.0.1-test', + entityTypes: ['service'], + testStringFact: { + type: 'string', + description: 'Test fact with a string type', + }, + }); + }); + + it('should return latest facts only for the correct id', async () => { + const returnedFact = await store.getLatestFactsByIds( ['test-fact'], - 'a/a/a', + 'a:a/a', ); expect(returnedFact['test-fact']).toMatchObject(baseAssertionFact); }); - it('should return latest facts for multiple refs', async () => { + it('should return latest facts for multiple ids', async () => { + await testDbClient.batchInsert('fact_schemas', [secondSchema]); await testDbClient.batchInsert( 'facts', additionalFacts.map(fact => ({ ...fact, - ref: 'second-test-fact', + id: 'second-test-fact', timestamp: farInTheFuture, })), ); - const returnedFacts = await store.getLatestFactsForRefs( + const returnedFacts = await store.getLatestFactsByIds( ['test-fact', 'second-test-fact'], - 'a/a/a', + 'a:a/a', ); expect(returnedFacts['test-fact']).toMatchObject({ @@ -185,7 +226,7 @@ describe('Tech Insights database', () => { }); expect(returnedFacts['second-test-fact']).toMatchObject({ ...baseAssertionFact, - ref: 'second-test-fact', + id: 'second-test-fact', timestamp: DateTime.fromISO(farInTheFuture), facts: { testNumberFact: 3 }, }); @@ -193,9 +234,9 @@ describe('Tech Insights database', () => { it('should return facts correctly between time range', async () => { await testDbClient.batchInsert('facts', additionalFacts); - const returnedFacts = await store.getFactsBetweenTimestampsForRefs( + const returnedFacts = await store.getFactsBetweenTimestampsByIds( ['test-fact'], - 'a/a/a', + 'a:a/a', DateTime.fromISO(now), DateTime.fromISO(shortlyInTheFuture).plus(Duration.fromMillis(10)), ); diff --git a/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts b/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts index 1bdf18249f..af45539b32 100644 --- a/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts +++ b/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts @@ -19,14 +19,16 @@ import { TechInsightFact, FlatTechInsightFact, TechInsightsStore, + FactSchemaDefinition, } from '@backstage/plugin-tech-insights-common'; import { rsort } from 'semver'; -import { groupBy } from 'lodash'; +import { groupBy, omit } from 'lodash'; import { DateTime } from 'luxon'; import { Logger } from 'winston'; +import { parseEntityName, stringifyEntityRef } from '@backstage/catalog-model'; export type RawDbFactRow = { - ref: string; + id: string; version: string; timestamp: Date | string; entity: string; @@ -34,10 +36,10 @@ export type RawDbFactRow = { }; type RawDbFactSchemaRow = { - id: number; - ref: string; + id: string; version: string; schema: string; + entityTypes?: string; }; export class TechInsightsDatabase implements TechInsightsStore { @@ -45,51 +47,51 @@ export class TechInsightsDatabase implements TechInsightsStore { constructor(private readonly db: Knex, private readonly logger: Logger) {} - async getLatestSchemas(refs?: string[]): Promise { + async getLatestSchemas(ids?: string[]): Promise { const queryBuilder = this.db('fact_schemas'); - if (refs) { - queryBuilder.whereIn('ref', refs); + if (ids) { + queryBuilder.whereIn('id', ids); } const existingSchemas = await queryBuilder.orderBy('id', 'desc').select(); - const groupedSchemas = groupBy(existingSchemas, 'ref'); + const groupedSchemas = groupBy(existingSchemas, 'id'); return Object.values(groupedSchemas) .map(schemas => { const sorted = rsort(schemas.map(it => it.version)); return schemas.find(it => it.version === sorted[0])!!; }) .map((it: RawDbFactSchemaRow) => ({ - ...it, - schema: JSON.parse(it.schema), + ...omit(it, 'schema'), + ...JSON.parse(it.schema), + entityTypes: it.entityTypes ? it.entityTypes.split(',') : [], })); } - async insertFactSchema(ref: string, schema: FactSchema) { + async insertFactSchema(schemaDefinition: FactSchemaDefinition) { + const { id, version, schema, entityTypes } = schemaDefinition; const existingSchemas = await this.db('fact_schemas') - .where({ ref }) + .where({ id }) + .and.where({ version }) .select(); - const exists = existingSchemas.some( - it => it.ref === ref && it.version === schema.version, - ); - if (!exists) { + if (!existingSchemas || existingSchemas.length === 0) { await this.db('fact_schemas').insert({ - ref, - version: schema.version, - schema: JSON.stringify(schema.schema), + id, + version, + entityTypes: entityTypes && entityTypes.join(','), + schema: JSON.stringify(schema), }); } } - async insertFacts(ref: string, facts: TechInsightFact[]): Promise { + async insertFacts(id: string, facts: TechInsightFact[]): Promise { if (facts.length === 0) return; - const currentSchema = await this.getLatestSchema(ref); + const currentSchema = await this.getLatestSchema(id); const factRows = facts.map(it => { - const { namespace, name, kind } = it.entity; return { - ref: ref, + id, version: currentSchema.version, - entity: `${namespace}/${kind}/${name}`.toLocaleLowerCase('en-US'), + entity: stringifyEntityRef(it.entity), facts: JSON.stringify(it.facts), ...(it.timestamp && { timestamp: it.timestamp.toJSDate() }), }; @@ -99,36 +101,36 @@ export class TechInsightsDatabase implements TechInsightsStore { }); } - async getLatestFactsForRefs( - refs: string[], + async getLatestFactsByIds( + ids: string[], entityTriplet: string, - ): Promise<{ [p: string]: FlatTechInsightFact }> { + ): Promise<{ [factId: string]: FlatTechInsightFact }> { const results = await this.db('facts') .where({ entity: entityTriplet }) - .and.whereIn('ref', refs) + .and.whereIn('id', ids) .join( this.db('facts') .max('timestamp') - .column('ref as subRef') - .groupBy('ref') + .column('id as subId') + .groupBy('id') .as('subQ'), - 'facts.ref', - 'subQ.subRef', + 'facts.id', + 'subQ.subId', ); return this.dbFactRowsToTechInsightFacts(results); } - async getFactsBetweenTimestampsForRefs( - refs: string[], + async getFactsBetweenTimestampsByIds( + ids: string[], entityTriplet: string, startDateTime: DateTime, endDateTime: DateTime, ): Promise<{ - [p: string]: FlatTechInsightFact[]; + [factId: string]: FlatTechInsightFact[]; }> { const results = await this.db('facts') .where({ entity: entityTriplet }) - .and.whereIn('ref', refs) + .and.whereIn('id', ids) .and.whereBetween('timestamp', [ startDateTime.toISO(), endDateTime.toISO(), @@ -136,31 +138,31 @@ export class TechInsightsDatabase implements TechInsightsStore { return groupBy( results.map(it => { - const [namespace, kind, name] = it.entity.split('/'); + const { namespace, kind, name } = parseEntityName(it.entity); const timestamp = typeof it.timestamp === 'string' ? DateTime.fromISO(it.timestamp) : DateTime.fromJSDate(it.timestamp); return { - ref: it.ref, + id: it.id, entity: { namespace, kind, name }, timestamp, version: it.version, facts: JSON.parse(it.facts), }; }), - 'ref', + 'id', ); } - private async getLatestSchema(ref: string): Promise { + private async getLatestSchema(id: string): Promise { const existingSchemas = await this.db('fact_schemas') - .where({ ref }) + .where({ id }) .orderBy('id', 'desc') .select(); if (existingSchemas.length < 1) { - this.logger.warn(`No schema found for ${ref}. `); - throw new Error(`No schema found for ${ref}. `); + this.logger.warn(`No schema found for ${id}. `); + throw new Error(`No schema found for ${id}. `); } const sorted = rsort(existingSchemas.map(it => it.version)); return existingSchemas.find(it => it.version === sorted[0])!!; @@ -168,15 +170,15 @@ export class TechInsightsDatabase implements TechInsightsStore { private dbFactRowsToTechInsightFacts(rows: RawDbFactRow[]) { return rows.reduce((acc, it) => { - const [namespace, kind, name] = it.entity.split('/'); + const { namespace, kind, name } = parseEntityName(it.entity); const timestamp = typeof it.timestamp === 'string' ? DateTime.fromISO(it.timestamp) : DateTime.fromJSDate(it.timestamp); return { ...acc, - [it.ref]: { - ref: it.ref, + [it.id]: { + id: it.id, entity: { namespace, kind, name }, timestamp, version: it.version, diff --git a/plugins/tech-insights-backend/src/service/router.test.ts b/plugins/tech-insights-backend/src/service/router.test.ts index 1433722e6c..075f67ccfe 100644 --- a/plugins/tech-insights-backend/src/service/router.test.ts +++ b/plugins/tech-insights-backend/src/service/router.test.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { DefaultTechInsightsBuilder } from './DefaultTechInsightsBuilder'; +import { buildTechInsightsContext } from './techInsightsContextBuilder'; import { createRouter } from './router'; import { getVoidLogger } from '@backstage/backend-common'; import { ConfigReader } from '@backstage/config'; @@ -27,14 +27,14 @@ import { Knex } from 'knex'; describe('Tech Insights router tests', () => { let app: express.Express; - const latestFactsForRefsMock = jest.fn(); - const factsBetweenTimestampsForRefsMock = jest.fn(); + const latestFactsByIdsMock = jest.fn(); + const factsBetweenTimestampsByIdsMock = jest.fn(); const latestSchemasMock = jest.fn(); const mockPersistenceContext: PersistenceContext = { techInsightsStore: { - getLatestFactsForRefs: latestFactsForRefsMock, - getFactsBetweenTimestampsForRefs: factsBetweenTimestampsForRefsMock, + getLatestFactsByIds: latestFactsByIdsMock, + getFactsBetweenTimestampsByIds: factsBetweenTimestampsByIdsMock, getLatestSchemas: latestSchemasMock, } as unknown as TechInsightsStore, }; @@ -44,7 +44,7 @@ describe('Tech Insights router tests', () => { }); beforeAll(async () => { - const techInsightsContext = await new DefaultTechInsightsBuilder({ + const techInsightsContext = await buildTechInsightsContext({ database: { getClient: () => { return Promise.resolve({ @@ -61,7 +61,7 @@ describe('Tech Insights router tests', () => { getBaseUrl: (_: string) => Promise.resolve('http://mock.url'), getExternalBaseUrl: (_: string) => Promise.resolve('http://mock.url'), }, - }).build(); + }); const router = await createRouter({ logger: getVoidLogger(), @@ -80,32 +80,36 @@ describe('Tech Insights router tests', () => { it('should not contain check endpoints when checker not present', async () => { await request(app).get('/checks').expect(404); - await request(app).get('/checks/a/a/a').expect(404); + await request(app).post('/checks/a/a/a').expect(404); }); - it('should parse be able to parse ref request params for fact retrieval', async () => { + it('should be able to parse id request params for fact retrieval', async () => { await request(app) - .get('/facts/latest/a/a/a') - .query({ refs: ['firstref', 'secondref'] }) + .get('/facts/latest') + .query({ + entity: 'a:a/a', + ids: ['firstId', 'secondId'], + }) .expect(200); - expect(latestFactsForRefsMock).toHaveBeenCalledWith( - ['firstref', 'secondref'], - 'a/a/a', + expect(latestFactsByIdsMock).toHaveBeenCalledWith( + ['firstId', 'secondId'], + 'a:a/a', ); }); - it('should parse be able to parse datetime request params for fact retrieval', async () => { + it('should be able to parse datetime request params for fact retrieval', async () => { await request(app) - .get('/facts/range/a/a/a') + .get('/facts/range') .query({ - refs: ['firstref', 'secondref'], + entity: 'a:a/a', + ids: ['firstId', 'secondId'], startDatetime: '2021-12-12T12:12:12', endDatetime: '2022-11-11T11:11:11', }) .expect(200); - expect(factsBetweenTimestampsForRefsMock).toHaveBeenCalledWith( - ['firstref', 'secondref'], - 'a/a/a', + expect(factsBetweenTimestampsByIdsMock).toHaveBeenCalledWith( + ['firstId', 'secondId'], + 'a:a/a', DateTime.fromISO('2021-12-12T12:12:12.000+00:00'), DateTime.fromISO('2022-11-11T11:11:11.000+00:00'), ); @@ -113,13 +117,14 @@ describe('Tech Insights router tests', () => { it('should respond gracefully on parsing errors', async () => { await request(app) - .get('/facts/range/a/a/a') + .get('/facts/range') .query({ - refs: ['firstref', 'secondref'], + entity: 'a:a/a', + ids: ['firstId', 'secondId'], startDatetime: '2021-12-1222T12:12:12', endDatetime: '2022-1122-11T11:11:11', }) .expect(422); - expect(latestFactsForRefsMock).toHaveBeenCalledTimes(0); + expect(latestFactsByIdsMock).toHaveBeenCalledTimes(0); }); }); diff --git a/plugins/tech-insights-backend/src/service/router.ts b/plugins/tech-insights-backend/src/service/router.ts index cc41868cd8..d02796117b 100644 --- a/plugins/tech-insights-backend/src/service/router.ts +++ b/plugins/tech-insights-backend/src/service/router.ts @@ -25,6 +25,11 @@ import { import { Logger } from 'winston'; import { DateTime } from 'luxon'; import { PersistenceContext } from './persistence/DatabaseManager'; +import { + EntityRef, + parseEntityName, + stringifyEntityRef, +} from '@backstage/catalog-model'; /** * @public @@ -92,7 +97,7 @@ export async function createRouter< }); } const { checks }: { checks: string[] } = req.body; - const entityTriplet = `${namespace.toLowerCase()}/${kind.toLowerCase()}/${name.toLowerCase()}`; + const entityTriplet = stringifyEntityRef({ namespace, kind, name }); const checkResult = await factChecker.runChecks(entityTriplet, checks); return res.send(checkResult); } catch (e) { @@ -106,22 +111,33 @@ export async function createRouter< } router.get('/fact-schemas', async (req, res) => { - const refs = req.query.refs as string[]; - return res.send(await techInsightsStore.getLatestSchemas(refs)); + const ids = req.query.ids as string[]; + return res.send(await techInsightsStore.getLatestSchemas(ids)); }); - router.get('/facts/latest/:namespace/:kind/:name', async (req, res) => { - const { namespace, kind, name } = req.params; - const refs = req.query.refs as string[]; - const entityTriplet = `${namespace.toLowerCase()}/${kind.toLowerCase()}/${name.toLowerCase()}`; + /** + * /facts/latest?entity=component:default/mycomponent&ids[]=factRetrieverId1&ids[]=factRetrieverId2 + */ + router.get('/facts/latest', async (req, res) => { + const { entity } = req.query; + const { namespace, kind, name } = parseEntityName(entity as EntityRef); + const ids = req.query.ids as string[]; return res.send( - await techInsightsStore.getLatestFactsForRefs(refs, entityTriplet), + await techInsightsStore.getLatestFactsByIds( + ids, + stringifyEntityRef({ namespace, kind, name }), + ), ); }); - router.get('/facts/range/:namespace/:kind/:name', async (req, res) => { - const { namespace, kind, name } = req.params; - const refs = req.query.refs as string[]; + /** + * /facts/latest?entity=component:default/mycomponent&startDateTime=2021-12-24T01:23:45&endDateTime=2021-12-31T23:59:59&ids[]=factRetrieverId1&ids[]=factRetrieverId2 + */ + router.get('/facts/range', async (req, res) => { + const { entity } = req.query; + const { namespace, kind, name } = parseEntityName(entity as EntityRef); + + const ids = req.query.ids as string[]; const startDatetime = DateTime.fromISO(req.query.startDatetime as string); const endDatetime = DateTime.fromISO(req.query.endDatetime as string); if (!startDatetime.isValid || !endDatetime.isValid) { @@ -131,10 +147,10 @@ export async function createRouter< value: !startDatetime.isValid ? startDatetime : endDatetime, }); } - const entityTriplet = `${namespace.toLowerCase()}/${kind.toLowerCase()}/${name.toLowerCase()}`; + const entityTriplet = stringifyEntityRef({ namespace, kind, name }); return res.send( - await techInsightsStore.getFactsBetweenTimestampsForRefs( - refs, + await techInsightsStore.getFactsBetweenTimestampsByIds( + ids, entityTriplet, startDatetime, endDatetime, diff --git a/plugins/tech-insights-backend/src/service/DefaultTechInsightsBuilder.ts b/plugins/tech-insights-backend/src/service/techInsightsContextBuilder.ts similarity index 63% rename from plugins/tech-insights-backend/src/service/DefaultTechInsightsBuilder.ts rename to plugins/tech-insights-backend/src/service/techInsightsContextBuilder.ts index 6c5b946c4c..bf30cdcf38 100644 --- a/plugins/tech-insights-backend/src/service/DefaultTechInsightsBuilder.ts +++ b/plugins/tech-insights-backend/src/service/techInsightsContextBuilder.ts @@ -80,70 +80,57 @@ export type TechInsightsContext< }; /** - * @public - * @typeParam CheckType - Type of the check for the fact checker this builder returns - * @typeParam CheckResultType - Type of the check result for the fact checker this builder returns + * Constructs needed persistence context, fact retriever engine + * and optionally fact checker implementations to be used in the tech insights module. * - * Default implementation of TechInsightsBuilder. + * @param options - Needed options to construct TechInsightsContext + * @returns TechInsightsContext with persistence implementations and optionally an implementation of a FactChecker */ -export class DefaultTechInsightsBuilder< +export const buildTechInsightsContext = async < CheckType extends TechInsightCheck, CheckResultType extends CheckResult, -> { - private readonly options: TechInsightsOptions; +>( + options: TechInsightsOptions, +): Promise> => { + const { + factRetrievers, + factCheckerFactory, + config, + discovery, + database, + logger, + } = options; - constructor(options: TechInsightsOptions) { - this.options = options; - } + const factRetrieverRegistry = new FactRetrieverRegistry(factRetrievers); - /** - * Constructs needed persistence context, fact retriever engine - * and optionally fact checker implementations to be used in the tech insights module. - * - * @returns TechInsightsContext with persistence implementations and optionally an implementation of a FactChecker - */ - async build(): Promise> { - const { - factRetrievers, - factCheckerFactory, + const persistenceContext = await DatabaseManager.initializePersistenceContext( + await database.getClient(), + { logger }, + ); + + const factRetrieverEngine = await FactRetrieverEngine.create({ + repository: persistenceContext.techInsightsStore, + factRetrieverRegistry, + factRetrieverContext: { config, discovery, - database, logger, - } = this.options; + }, + }); - const factRetrieverRegistry = new FactRetrieverRegistry(factRetrievers); - - const persistenceContext = - await DatabaseManager.initializePersistenceContext( - await database.getClient(), - { logger }, - ); - - const factRetrieverEngine = await FactRetrieverEngine.fromConfig({ - repository: persistenceContext.techInsightsStore, - factRetrieverRegistry, - factRetrieverContext: { - config, - discovery, - logger, - }, - }); - - factRetrieverEngine.schedule(); - - if (factCheckerFactory) { - const factChecker = factCheckerFactory.construct( - persistenceContext.techInsightsStore, - ); - return { - persistenceContext, - factChecker, - }; - } + factRetrieverEngine.schedule(); + if (factCheckerFactory) { + const factChecker = factCheckerFactory.construct( + persistenceContext.techInsightsStore, + ); return { persistenceContext, + factChecker, }; } -} + + return { + persistenceContext, + }; +}; diff --git a/plugins/tech-insights-common/CHANGELOG.md b/plugins/tech-insights-common/CHANGELOG.md deleted file mode 100644 index 17e0211ab0..0000000000 --- a/plugins/tech-insights-common/CHANGELOG.md +++ /dev/null @@ -1,7 +0,0 @@ -# @backstage/plugin-tech-insights-backend - -## 0.0.1 - -### Patch Changes - -- Initial implementation diff --git a/plugins/tech-insights-common/api-report.md b/plugins/tech-insights-common/api-report.md index db987546c3..e83ba18137 100644 --- a/plugins/tech-insights-common/api-report.md +++ b/plugins/tech-insights-common/api-report.md @@ -17,7 +17,7 @@ export interface BooleanCheckResult extends CheckResult { // @public export interface CheckResponse { description: string; - factRefs: string[]; + factIds: string[]; id: string; metadata?: Record; name: string; @@ -68,22 +68,23 @@ export interface FactCheckerFactory< // @public export type FactResponse = { - [key: string]: { - ref: string; + [id: string]: { + id: string; type: 'integer' | 'float' | 'string' | 'boolean' | 'datetime' | 'set'; description: string; value: number | string | boolean | DateTime | []; since?: string; metadata?: Record; - entityKinds: string[]; }; }; // @public export interface FactRetriever { + entityTypes?: string[]; handler: (ctx: FactRetrieverContext) => Promise; - ref: string; + id: string; schema: FactSchema; + version: string; } // @public @@ -101,30 +102,28 @@ export type FactRetrieverRegistration = { // @public export type FactSchema = { - version: string; - schema: FactValueDefinitions; -}; - -// @public -export type FactValueDefinitions = { - [key: string]: { + [name: string]: { type: 'integer' | 'float' | 'string' | 'boolean' | 'datetime' | 'set'; description: string; since?: string; metadata?: Record; - entityKinds: string[]; }; }; +// Warning: (ae-missing-release-tag) "FactSchemaDefinition" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) +// +// @public (undocumented) +export type FactSchemaDefinition = Omit; + // @public export type FlatTechInsightFact = TechInsightFact & { - ref: string; + id: string; }; // @public export interface TechInsightCheck { description: string; - factRefs: string[]; + factIds: string[]; failureMetadata?: Record; id: string; name: string; @@ -151,14 +150,24 @@ export type TechInsightFact = { kind: string; name: string; }; - facts: Record; + facts: Record< + string, + | number + | string + | boolean + | DateTime + | number[] + | string[] + | boolean[] + | DateTime[] + >; timestamp?: DateTime; }; // @public export interface TechInsightsStore { - getFactsBetweenTimestampsForRefs( - refs: string[], + getFactsBetweenTimestampsByIds( + ids: string[], entity: string, startDateTime: DateTime, endDateTime: DateTime, @@ -166,15 +175,15 @@ export interface TechInsightsStore { [factRef: string]: FlatTechInsightFact[]; }>; // (undocumented) - getLatestFactsForRefs( - refs: string[], + getLatestFactsByIds( + ids: string[], entity: string, ): Promise<{ [factRef: string]: FlatTechInsightFact; }>; - getLatestSchemas(refs?: string[]): Promise; - insertFacts(ref: string, facts: TechInsightFact[]): Promise; - insertFactSchema(ref: string, schema: FactSchema): Promise; + getLatestSchemas(ids?: string[]): Promise; + insertFacts(id: string, facts: TechInsightFact[]): Promise; + insertFactSchema(schemaDefinition: FactSchemaDefinition): Promise; } // (No @packageDocumentation comment for this package) diff --git a/plugins/tech-insights-common/src/checks.ts b/plugins/tech-insights-common/src/checks.ts index 7b2c4c6b8f..90fa7ee34f 100644 --- a/plugins/tech-insights-common/src/checks.ts +++ b/plugins/tech-insights-common/src/checks.ts @@ -157,7 +157,7 @@ export interface TechInsightCheck { * * References the fact container, aka fact retriever itself which may or may not contain multiple individual facts and values */ - factRefs: string[]; + factIds: string[]; /** * Metadata to be returned in case a check has been successfully evaluated diff --git a/plugins/tech-insights-common/src/facts.ts b/plugins/tech-insights-common/src/facts.ts index 1ae507adf8..a46e7bd95d 100644 --- a/plugins/tech-insights-common/src/facts.ts +++ b/plugins/tech-insights-common/src/facts.ts @@ -42,7 +42,17 @@ export type TechInsightFact = { * * Key indicates fact name as it is defined in FactSchema */ - facts: Record; + facts: Record< + string, + | number + | string + | boolean + | DateTime + | number[] + | string[] + | boolean[] + | DateTime[] + >; /** * Optional timestamp value which can be used to override retrieval time of the fact row. @@ -62,7 +72,7 @@ export type FlatTechInsightFact = TechInsightFact & { /** * Reference and unique identifier of the fact row */ - ref: string; + id: string; }; /** @@ -73,8 +83,11 @@ export type FlatTechInsightFact = TechInsightFact & { * Used as part of a schema to validate, identify and generically construct usage implementations * of individual fact values in the system. */ -export type FactValueDefinitions = { - [key: string]: { +export type FactSchema = { + /** + * Name of the fact + */ + [name: string]: { /** * Type of the individual fact value * @@ -109,30 +122,9 @@ export type FactValueDefinitions = { * ``` */ metadata?: Record; - - /** - * A list of entity kind descriptors to indicate if this fact is valid for an entity kind - */ - entityKinds: string[]; }; }; -/** - * @public - * - * Container for FactSchema - */ -export type FactSchema = { - /** - * Semver string indicating the version of this schema - */ - version: string; - /** - * Actual schema definitions for this schema - */ - schema: FactValueDefinitions; -}; - /** * @public * @@ -159,7 +151,15 @@ export interface FactRetriever { * Used to identify and store individual facts returned from this retriever * and schemas defined by this retriever. */ - ref: string; + id: string; + + /** + * Semver string indicating the version of this fact retriever + * This version is used to determine if the schema this fact retriever matches the data this fact retriever provides. + * + * Should be incremented on changes to returned data from the handler or if the schema changes. + */ + version: string; /** * Handler function that needs to be implemented to retrieve fact values for entities. @@ -173,8 +173,20 @@ export interface FactRetriever { * A fact schema defining the shape of data returned from the handler method for each entity */ schema: FactSchema; + + /** + * An optional list of entity type descriptors to indicate if this fact retriever is valid for an entity type. + * If omitted, the retriever should apply to all entities. + * + * Should be defined as: + * ['component', 'group', 'user'] for top level items + * ['component:service', 'component:website'] for component types. + */ + entityTypes?: string[]; } +export type FactSchemaDefinition = Omit; + /** * @public * diff --git a/plugins/tech-insights-common/src/persistence.ts b/plugins/tech-insights-common/src/persistence.ts index 9ffb6a9a28..444e961008 100644 --- a/plugins/tech-insights-common/src/persistence.ts +++ b/plugins/tech-insights-common/src/persistence.ts @@ -13,7 +13,12 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { FactSchema, TechInsightFact, FlatTechInsightFact } from './facts'; +import { + FactSchema, + TechInsightFact, + FlatTechInsightFact, + FactSchemaDefinition, +} from './facts'; import { DateTime } from 'luxon'; /** @@ -28,34 +33,34 @@ export interface TechInsightsStore { * * Each row may contain multiple individual facts and values * - * @param ref - Unique identifier of the fact retriever these facts relate to + * @param id - Unique identifier of the fact retriever these facts relate to * @param facts - A collection of TechInsightFacts */ - insertFacts(ref: string, facts: TechInsightFact[]): Promise; + insertFacts(id: string, facts: TechInsightFact[]): Promise; /** - * @param refs - A collection of reference string to a fact row + * @param ids - A collection of fact row identifiers * @param entity - A string identifying an entity. In a format namespace/kind/name * * @returns - An object keyed by a fact reference and containing an individual TechInsightFact */ - getLatestFactsForRefs( - refs: string[], + getLatestFactsByIds( + ids: string[], entity: string, ): Promise<{ [factRef: string]: FlatTechInsightFact }>; /** * Retrieves fact values identified by fact row references for an individual entity. * - * @param refs - A collection of reference string to a fact row + * @param ids - A collection of fact row identifiers * @param entity - A string identifying an entity. In a format namespace/kind/name * @param startDateTime - DateTime object indicating start of the time frame * @param endDateTime - DateTime object indicating start of the time frame * * @returns - An object keyed by a fact reference and containing a collection of TechInsightFacts matching the time frame */ - getFactsBetweenTimestampsForRefs( - refs: string[], + getFactsBetweenTimestampsByIds( + ids: string[], entity: string, startDateTime: DateTime, endDateTime: DateTime, @@ -64,16 +69,15 @@ export interface TechInsightsStore { /** * Stores versioned fact schemas into data store * - * @param ref - Identifier of the fact schema. Reference to a fact retriever. - * @param schema - The actual schema to store + * @param schemaDefinition - FactSchemaDefinition containing id, version, schema and entityTypes. */ - insertFactSchema(ref: string, schema: FactSchema): Promise; + insertFactSchema(schemaDefinition: FactSchemaDefinition): Promise; /** * Retrieves latest versions (as defined by semver) of fact schemas from the data store. * - * @param refs - Collection of refs to return. If omitted, all Schemas should be returned. + * @param ids - Collection of ids to return. If omitted, all Schemas should be returned. * @returns - A collection of schemas */ - getLatestSchemas(refs?: string[]): Promise; + getLatestSchemas(ids?: string[]): Promise; } diff --git a/plugins/tech-insights-common/src/responses.ts b/plugins/tech-insights-common/src/responses.ts index 0cd1d0a2e8..c3dee1df48 100644 --- a/plugins/tech-insights-common/src/responses.ts +++ b/plugins/tech-insights-common/src/responses.ts @@ -44,7 +44,7 @@ export interface CheckResponse { /** * A collection of references to fact rows used to run this checks against */ - factRefs: string[]; + factIds: string[]; /** * Metadata related to a check. @@ -62,11 +62,11 @@ export interface CheckResponse { * Keyed by the name of the fact */ export type FactResponse = { - [key: string]: { + [id: string]: { /** * Reference and unique identifier of the fact row */ - ref: string; + id: string; /** * Type of the individual fact value * @@ -97,10 +97,5 @@ export type FactResponse = { * Currently loosely typed, but in the future when patterns emerge, key shapes can be defined */ metadata?: Record; - - /** - * A list of entity kind descriptors to indicate if this fact is valid for an entity kind - */ - entityKinds: string[]; }; };