From dfd5e81721960d8d70ac5a5459b2aff72a4a2dd6 Mon Sep 17 00:00:00 2001 From: Jussi Hallila Date: Mon, 10 Jan 2022 15:38:16 +0100 Subject: [PATCH 1/5] Adds a configuration option to fact retrievers to define lifecycle for facts the retriever persists. Possible values are either 'items-to-live' or 'time-to-live'. The former will only n number of items in to the database for each fact per entity. The latter will remove all facts that are older than the TTL value. Possible values: * { itl: 5 } // Deletes all facts for the retriever/entity pair, apart from the last five * { ttl: 1209600000 } // (2 weeks) Deletes all facts older than 2 weeks for the retriever/entity pair * { ttl: { weeks: 2 } } // Deletes all facts older than 2 weeks for the retriever/entity pair Signed-off-by: Jussi Hallila --- .changeset/six-coins-admire.md | 12 ++ plugins/tech-insights-backend/README.md | 10 +- plugins/tech-insights-backend/api-report.md | 2 + .../src/service/fact/FactRetrieverEngine.ts | 12 +- .../src/service/fact/createFactRetriever.ts | 8 ++ .../src/service/fact/factRetrievers/utils.ts | 9 ++ .../persistence/TechInsightsDatabase.test.ts | 105 +++++++++++++++++- .../persistence/TechInsightsDatabase.ts | 79 ++++++++++++- plugins/tech-insights-node/api-report.md | 21 +++- plugins/tech-insights-node/src/facts.ts | 39 ++++++- plugins/tech-insights-node/src/persistence.ts | 8 +- 11 files changed, 286 insertions(+), 19 deletions(-) create mode 100644 .changeset/six-coins-admire.md diff --git a/.changeset/six-coins-admire.md b/.changeset/six-coins-admire.md new file mode 100644 index 0000000000..539c6e470d --- /dev/null +++ b/.changeset/six-coins-admire.md @@ -0,0 +1,12 @@ +--- +'@backstage/plugin-tech-insights-backend': patch +'@backstage/plugin-tech-insights-node': patch +--- + +Adds a configuration option to fact retrievers to define lifecycle for facts the retriever persists. Possible values are either 'items-to-live' or 'time-to-live'. The former will only n number of items in to the database for each fact per entity. The latter will remove all facts that are older than the TTL value. + +Possible values: + +- { itl: 5 } // Deletes all facts for the retriever/entity pair, apart from the last five +- { ttl: 1209600000 } // (2 weeks) Deletes all facts older than 2 weeks for the retriever/entity pair +- { ttl: { weeks: 2 } } // Deletes all facts older than 2 weeks for the retriever/entity pair diff --git a/plugins/tech-insights-backend/README.md b/plugins/tech-insights-backend/README.md index 31f5e402b0..ea76cf2541 100644 --- a/plugins/tech-insights-backend/README.md +++ b/plugins/tech-insights-backend/README.md @@ -91,7 +91,15 @@ const myFactRetrieverRegistration = createFactRetrieverRegistration( ); ``` -Then you can modify the example `techInsights.ts` file shown above like this: +FactRetrieverRegistration also accepts an optional `lifecycle` configuration value. This can be either ITL (items to live) or TTL (time to live). Valid options for this value are either a number for itl or a Luxon duration like object for ttl. For example: + +```ts +const itl = { itl: 7 }; // Deletes all but 7 latest facts for each id/entity pair +const ttl = { ttl: 1209600000 }; // (2 weeks) Deletes items older than 2 weeks +const ttlWithAHumanReadableValue = { ttl: { weeks: 2 } }; // Deletes items older than 2 weeks +``` + +To register these fact retrievers to your application you can modify the example `techInsights.ts` file shown above like this: ```diff const builder = new DefaultTechInsightsBuilder({ diff --git a/plugins/tech-insights-backend/api-report.md b/plugins/tech-insights-backend/api-report.md index ac2d1d1170..c5cefd2619 100644 --- a/plugins/tech-insights-backend/api-report.md +++ b/plugins/tech-insights-backend/api-report.md @@ -8,6 +8,7 @@ import { Config } from '@backstage/config'; import express from 'express'; import { FactChecker } from '@backstage/plugin-tech-insights-node'; import { FactCheckerFactory } from '@backstage/plugin-tech-insights-node'; +import { FactLifecycle } from '@backstage/plugin-tech-insights-node'; import { FactRetriever } from '@backstage/plugin-tech-insights-node'; import { FactRetrieverRegistration } from '@backstage/plugin-tech-insights-node'; import { Logger as Logger_2 } from 'winston'; @@ -28,6 +29,7 @@ export const buildTechInsightsContext: < export function createFactRetrieverRegistration( cadence: string, factRetriever: FactRetriever, + lifecycle?: FactLifecycle, ): FactRetrieverRegistration; // @public diff --git a/plugins/tech-insights-backend/src/service/fact/FactRetrieverEngine.ts b/plugins/tech-insights-backend/src/service/fact/FactRetrieverEngine.ts index 11fd0d29b8..51a68635b1 100644 --- a/plugins/tech-insights-backend/src/service/fact/FactRetrieverEngine.ts +++ b/plugins/tech-insights-backend/src/service/fact/FactRetrieverEngine.ts @@ -14,6 +14,7 @@ * limitations under the License. */ import { + FactLifecycle, FactRetriever, FactRetrieverContext, TechInsightsStore, @@ -75,7 +76,7 @@ export class FactRetrieverEngine { const registrations = this.factRetrieverRegistry.listRegistrations(); const newRegs: string[] = []; registrations.forEach(registration => { - const { factRetriever, cadence } = registration; + const { factRetriever, cadence, lifecycle } = registration; if (!this.scheduledJobs.has(factRetriever.id)) { const cronExpression = cadence || this.defaultCadence || randomDailyCron(); @@ -87,7 +88,7 @@ export class FactRetrieverEngine { } const job = schedule( cronExpression, - this.createFactRetrieverHandler(factRetriever), + this.createFactRetrieverHandler(factRetriever, lifecycle), ); this.scheduledJobs.set(factRetriever.id, job); newRegs.push(factRetriever.id); @@ -102,7 +103,10 @@ export class FactRetrieverEngine { return this.scheduledJobs.get(ref); } - private createFactRetrieverHandler(factRetriever: FactRetriever) { + private createFactRetrieverHandler( + factRetriever: FactRetriever, + lifecycle?: FactLifecycle, + ) { return async () => { const startTimestamp = process.hrtime(); this.logger.info( @@ -121,7 +125,7 @@ export class FactRetrieverEngine { } try { - await this.repository.insertFacts(factRetriever.id, facts); + await this.repository.insertFacts(factRetriever.id, facts, lifecycle); this.logger.info( `Stored ${facts.length} facts for fact retriever ${ factRetriever.id diff --git a/plugins/tech-insights-backend/src/service/fact/createFactRetriever.ts b/plugins/tech-insights-backend/src/service/fact/createFactRetriever.ts index 655736400a..1e3f7f8904 100644 --- a/plugins/tech-insights-backend/src/service/fact/createFactRetriever.ts +++ b/plugins/tech-insights-backend/src/service/fact/createFactRetriever.ts @@ -14,6 +14,7 @@ * limitations under the License. */ import { + FactLifecycle, FactRetriever, FactRetrieverRegistration, } from '@backstage/plugin-tech-insights-node'; @@ -25,6 +26,7 @@ import { * * @param cadence - cron expression to indicate when the fact retriever should be triggered * @param factRetriever - Implementation of fact retriever consisting of at least id, version, schema and handler + * @param lifecycle - Optional lifecycle definition indicating the cleanup logic of facts when this retriever is run * * * @remarks @@ -40,13 +42,19 @@ import { # │ │ │ │ │ │ # * * * * * * * + * Valid lifecycle values: + * \{ ttl: \{ weeks: 2 \} \} -- This fact retriever will remove items that are older than 2 weeks when it is run + * \{ itl: 7 \} -- This fact retriever will leave 7 newest items in the database when it is run + * */ export function createFactRetrieverRegistration( cadence: string, factRetriever: FactRetriever, + lifecycle?: FactLifecycle, ): FactRetrieverRegistration { return { cadence, factRetriever, + lifecycle, }; } diff --git a/plugins/tech-insights-backend/src/service/fact/factRetrievers/utils.ts b/plugins/tech-insights-backend/src/service/fact/factRetrievers/utils.ts index 7967502a63..b01bbe6248 100644 --- a/plugins/tech-insights-backend/src/service/fact/factRetrievers/utils.ts +++ b/plugins/tech-insights-backend/src/service/fact/factRetrievers/utils.ts @@ -16,9 +16,18 @@ import camelCase from 'lodash/camelCase'; import { Entity } from '@backstage/catalog-model'; import { get } from 'lodash'; +import { FactLifecycle, ITL, TTL } from '@backstage/plugin-tech-insights-node'; export const generateAnnotationFactName = (annotation: string) => camelCase(`hasAnnotation-${annotation}`); export const entityHasAnnotation = (entity: Entity, annotation: string) => Boolean(get(entity, ['metadata', 'annotations', annotation])); + +export const isTtl = (lifecycle: FactLifecycle): lifecycle is TTL => { + return !!(lifecycle as TTL).ttl; +}; + +export const isItl = (lifecycle: FactLifecycle): lifecycle is ITL => { + return !!(lifecycle as ITL).itl; +}; 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 43d00e2425..a1bb2dc299 100644 --- a/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.test.ts +++ b/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.test.ts @@ -126,10 +126,15 @@ describe('Tech Insights database', () => { logger: getVoidLogger(), }) ).techInsightsStore; - + }); + beforeEach(async () => { await testDbClient.batchInsert('fact_schemas', factSchemas); await testDbClient.batchInsert('facts', facts); }); + afterEach(async () => { + await testDbClient('facts').delete(); + await testDbClient('fact_schemas').delete(); + }); const baseAssertionFact = { id: 'test-fact', @@ -183,16 +188,12 @@ describe('Tech Insights database', () => { expect(schemas).toHaveLength(2); expect(schemas[0]).toMatchObject({ id: 'test-fact', - version: '1.2.1-test', + version: '0.0.1-test', entityFilter: [{ kind: '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', @@ -263,4 +264,96 @@ describe('Tech Insights database', () => { facts: { testNumberFact: 3 }, }); }); + + it('should delete extraneous rows when ITL is defined. Should leave only n latest', async () => { + const deviledFact = (it: {}) => ({ + ...it, + facts: JSON.stringify({ + testNumberFact: 666, + }), + }); + await testDbClient.batchInsert('facts', additionalFacts.map(deviledFact)); + + const preInsertionFacts = await testDbClient('facts').select(); + expect(preInsertionFacts).toHaveLength(3); + + const timestamp = DateTime.now().plus(Duration.fromMillis(1111)); + const factToBeInserted = { + timestamp: timestamp, + entity: { + namespace: 'a', + kind: 'a', + name: 'a', + }, + facts: { + testNumberFact: 555, + }, + }; + const itl = 2; + await store.insertFacts('test-fact', [factToBeInserted], { itl }); + + const afterInsertionFacts = await testDbClient('facts').select(); + expect(afterInsertionFacts).toHaveLength(itl); + expect(afterInsertionFacts[0]).toMatchObject( + deviledFact(additionalFacts[0]), + ); + expect(afterInsertionFacts[1]).toMatchObject({ + id: 'test-fact', + version: '0.0.1-test', + timestamp: timestamp.toISO(), + entity: 'a:a/a', + facts: JSON.stringify({ testNumberFact: 555 }), + }); + }); + + it('should delete extraneous rows when TTL is defined. Should leave only items with timestamp greater than TTL', async () => { + const oldStaledOutFact = (it: {}) => ({ + ...it, + facts: JSON.stringify({ + testNumberFact: 666, + }), + timestamp: DateTime.now().minus({ weeks: 3 }).toISO(), + }); + await testDbClient.batchInsert( + 'facts', + additionalFacts.map(oldStaledOutFact), + ); + + const preInsertionFacts = await testDbClient('facts').select(); + expect(preInsertionFacts).toHaveLength(3); + + const timestamp = DateTime.now().plus(Duration.fromMillis(1111)); + const factToBeInserted = { + timestamp: timestamp, + entity: { + namespace: 'a', + kind: 'a', + name: 'a', + }, + facts: { + testNumberFact: 555, + }, + }; + await store.insertFacts('test-fact', [factToBeInserted], { + ttl: { weeks: 2 }, + }); + + const afterInsertionFacts = await testDbClient('facts') + .select() + .orderBy('timestamp', 'desc'); + expect(afterInsertionFacts).toHaveLength(3); + expect(afterInsertionFacts[0]).toMatchObject({ + id: 'test-fact', + version: '0.0.1-test', + timestamp: timestamp.toISO(), + entity: 'a:a/a', + facts: JSON.stringify({ testNumberFact: 555 }), + }); + expect(afterInsertionFacts[1]).toMatchObject(facts[1]); + expect(afterInsertionFacts[2]).toMatchObject(facts[0]); + + expect(afterInsertionFacts).not.toContainEqual( + oldStaledOutFact(additionalFacts[0]), + ); + }); }); diff --git a/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts b/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts index 7cbf5671c2..bb29657c07 100644 --- a/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts +++ b/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts @@ -15,17 +15,20 @@ */ import { Knex } from 'knex'; import { + FactLifecycle, FactSchema, - TechInsightFact, - FlatTechInsightFact, - TechInsightsStore, FactSchemaDefinition, + FlatTechInsightFact, + TechInsightFact, + TechInsightsStore, } from '@backstage/plugin-tech-insights-node'; import { rsort } from 'semver'; import { groupBy, omit } from 'lodash'; import { DateTime } from 'luxon'; import { Logger } from 'winston'; import { parseEntityName, stringifyEntityRef } from '@backstage/catalog-model'; +import { isItl, isTtl } from '../fact/factRetrievers/utils'; +import Transaction = Knex.Transaction; export type RawDbFactRow = { id: string; @@ -84,7 +87,11 @@ export class TechInsightsDatabase implements TechInsightsStore { } } - async insertFacts(id: string, facts: TechInsightFact[]): Promise { + async insertFacts( + id: string, + facts: TechInsightFact[], + lifecycle?: FactLifecycle, + ): Promise { if (facts.length === 0) return; const currentSchema = await this.getLatestSchema(id); const factRows = facts.map(it => { @@ -93,11 +100,21 @@ export class TechInsightsDatabase implements TechInsightsStore { version: currentSchema.version, entity: stringifyEntityRef(it.entity), facts: JSON.stringify(it.facts), - ...(it.timestamp && { timestamp: it.timestamp.toJSDate() }), + ...(it.timestamp && { timestamp: it.timestamp.toISO() }), }; }); + await this.db.transaction(async tx => { await tx.batchInsert('facts', factRows, this.CHUNK_SIZE); + + if (lifecycle && isTtl(lifecycle)) { + const expiration = DateTime.now().minus(lifecycle.ttl); + await this.deleteExpiredFactsByDate(tx, factRows, expiration); + } + if (lifecycle && isItl(lifecycle)) { + const items = lifecycle.itl; + await this.deleteExpiredFactsByNumber(tx, factRows, items); + } }); } @@ -170,6 +187,58 @@ export class TechInsightsDatabase implements TechInsightsStore { return existingSchemas.find(it => it.version === sorted[0])!!; } + private async deleteExpiredFactsByDate( + tx: Transaction, + factRows: { id: string; entity: string }[], + timestamp: DateTime, + ) { + await tx('facts') + .whereIn( + ['id', 'entity'], + factRows.map(it => [it.id, it.entity]), + ) + .and.where('timestamp', '<', timestamp.toISO()) + .delete(); + } + + private async deleteExpiredFactsByNumber( + tx: Transaction, + factRows: { id: string; entity: string }[], + items: number, + ) { + const deletables = await tx('facts') + .whereIn( + ['id', 'entity'], + factRows.map(it => [it.id, it.entity]), + ) + .and.leftJoin( + this.db.raw( + `(select * + from (select id fid, + entity fentity, + timestamp ftimestamp, + row_number() over (partition by id, entity order by timestamp desc) as fact_rank + from facts) ranks + where fact_rank <= ?? ) as filterjoin`, + items, + ), + joinClause => { + joinClause + .on('filterjoin.fid', 'facts.id') + .on('filterjoin.fentity', 'facts.entity') + .on('filterjoin.ftimestamp', 'facts.timestamp'); + }, + ) + .whereNull('filterjoin.fid'); + + await tx('facts') + .whereIn( + ['id', 'entity', 'timestamp'], + deletables.map(it => [it.id, it.entity, it.timestamp]), + ) + .delete(); + } + private dbFactRowsToTechInsightFacts(rows: RawDbFactRow[]) { return rows.reduce((acc, it) => { const { namespace, kind, name } = parseEntityName(it.entity); diff --git a/plugins/tech-insights-node/api-report.md b/plugins/tech-insights-node/api-report.md index fc137da4bc..ec822bf5e2 100644 --- a/plugins/tech-insights-node/api-report.md +++ b/plugins/tech-insights-node/api-report.md @@ -6,6 +6,7 @@ import { CheckResult } from '@backstage/plugin-tech-insights-common'; import { Config } from '@backstage/config'; import { DateTime } from 'luxon'; +import { DurationLike } from 'luxon'; import { Logger as Logger_2 } from 'winston'; import { PluginEndpointDiscovery } from '@backstage/backend-common'; @@ -37,6 +38,9 @@ export interface FactCheckerFactory< ): FactChecker; } +// @public +export type FactLifecycle = TTL | ITL; + // @public export interface FactRetriever { entityFilter?: @@ -62,6 +66,7 @@ export type FactRetrieverContext = { export type FactRetrieverRegistration = { factRetriever: FactRetriever; cadence?: string; + lifecycle?: FactLifecycle; }; // @public @@ -82,6 +87,11 @@ export type FlatTechInsightFact = TechInsightFact & { id: string; }; +// @public +export type ITL = { + itl: number; +}; + // @public export interface TechInsightCheck { description: string; @@ -144,9 +154,18 @@ export interface TechInsightsStore { [factRef: string]: FlatTechInsightFact; }>; getLatestSchemas(ids?: string[]): Promise; - insertFacts(id: string, facts: TechInsightFact[]): Promise; + insertFacts( + id: string, + facts: TechInsightFact[], + lifecycle?: FactLifecycle, + ): Promise; insertFactSchema(schemaDefinition: FactSchemaDefinition): Promise; } +// @public +export type TTL = { + ttl: DurationLike; +}; + // (No @packageDocumentation comment for this package) ``` diff --git a/plugins/tech-insights-node/src/facts.ts b/plugins/tech-insights-node/src/facts.ts index 4a3157d391..34c753486a 100644 --- a/plugins/tech-insights-node/src/facts.ts +++ b/plugins/tech-insights-node/src/facts.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { DateTime } from 'luxon'; +import { DateTime, DurationLike } from 'luxon'; import { Config } from '@backstage/config'; import { PluginEndpointDiscovery } from '@backstage/backend-common'; import { Logger } from 'winston'; @@ -190,6 +190,36 @@ export interface FactRetriever { | Record; } +/** + * @public + * + * A Luxon duration like object for time to live value + * + * @example + * \{ ttl: 1209600000 \} + * \{ ttl: \{ weeks: 4 \} \} + * + **/ +export type TTL = { ttl: DurationLike }; + +/** + * @public + * + * A number for items to live value + * + * @example + * \{ itl: 10 \} + * + **/ +export type ITL = { itl: number }; + +/** + * @public + * + * A fact lifecycle definition. Determines which strategy to use to purge expired facts from the database. + */ +export type FactLifecycle = TTL | ITL; + /** * @public * @@ -216,4 +246,11 @@ export type FactRetrieverRegistration = { * */ cadence?: string; + + /** + * Fact lifecycle definition + * + * If defined this value will be used to determine expired items which will deleted when this fact retriever is run + */ + lifecycle?: FactLifecycle; }; diff --git a/plugins/tech-insights-node/src/persistence.ts b/plugins/tech-insights-node/src/persistence.ts index 444e961008..17a3867347 100644 --- a/plugins/tech-insights-node/src/persistence.ts +++ b/plugins/tech-insights-node/src/persistence.ts @@ -18,6 +18,7 @@ import { TechInsightFact, FlatTechInsightFact, FactSchemaDefinition, + FactLifecycle, } from './facts'; import { DateTime } from 'luxon'; @@ -35,8 +36,13 @@ export interface TechInsightsStore { * * @param id - Unique identifier of the fact retriever these facts relate to * @param facts - A collection of TechInsightFacts + * @param lifecycle - (Optional) Fact lifecycle object indicating the expiration logic for these items */ - insertFacts(id: string, facts: TechInsightFact[]): Promise; + insertFacts( + id: string, + facts: TechInsightFact[], + lifecycle?: FactLifecycle, + ): Promise; /** * @param ids - A collection of fact row identifiers From b567ee28186358cac3a4c2b57d78374ad1dd625f Mon Sep 17 00:00:00 2001 From: Jussi Hallila Date: Mon, 10 Jan 2022 16:13:34 +0100 Subject: [PATCH 2/5] Make Vale happier. Signed-off-by: Jussi Hallila --- .changeset/six-coins-admire.md | 8 ++++---- plugins/tech-insights-backend/README.md | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.changeset/six-coins-admire.md b/.changeset/six-coins-admire.md index 539c6e470d..42d0ecbb8f 100644 --- a/.changeset/six-coins-admire.md +++ b/.changeset/six-coins-admire.md @@ -3,10 +3,10 @@ '@backstage/plugin-tech-insights-node': patch --- -Adds a configuration option to fact retrievers to define lifecycle for facts the retriever persists. Possible values are either 'items-to-live' or 'time-to-live'. The former will only n number of items in to the database for each fact per entity. The latter will remove all facts that are older than the TTL value. +Adds a configuration option to fact retrievers to define lifecycle for facts the retriever persists. Possible values are either 'items-to-live' or 'time-to-live'. The former will keep only n number of items in to the database for each fact per entity. The latter will remove all facts that are older than the TTL value. Possible values: -- { itl: 5 } // Deletes all facts for the retriever/entity pair, apart from the last five -- { ttl: 1209600000 } // (2 weeks) Deletes all facts older than 2 weeks for the retriever/entity pair -- { ttl: { weeks: 2 } } // Deletes all facts older than 2 weeks for the retriever/entity pair +- `{ itl: 5 }` // Deletes all facts for the retriever/entity pair, apart from the last five +- `{ ttl: 1209600000 }` // (2 weeks) Deletes all facts older than 2 weeks for the retriever/entity pair +- `{ ttl: { weeks: 2 } }` // Deletes all facts older than 2 weeks for the retriever/entity pair diff --git a/plugins/tech-insights-backend/README.md b/plugins/tech-insights-backend/README.md index ea76cf2541..c0d903b2eb 100644 --- a/plugins/tech-insights-backend/README.md +++ b/plugins/tech-insights-backend/README.md @@ -91,7 +91,7 @@ const myFactRetrieverRegistration = createFactRetrieverRegistration( ); ``` -FactRetrieverRegistration also accepts an optional `lifecycle` configuration value. This can be either ITL (items to live) or TTL (time to live). Valid options for this value are either a number for itl or a Luxon duration like object for ttl. For example: +FactRetrieverRegistration also accepts an optional `lifecycle` configuration value. This can be either ITL (items to live) or TTL (time to live). Valid options for this value are either a number for ITL or a Luxon duration like object for TTL. For example: ```ts const itl = { itl: 7 }; // Deletes all but 7 latest facts for each id/entity pair From 28fd9bc6039e907c4a0fa57188d23f099ef5a7f3 Mon Sep 17 00:00:00 2001 From: Jussi Hallila Date: Thu, 13 Jan 2022 16:04:30 +0100 Subject: [PATCH 3/5] Modify based on review comments. Make it breaking change. Signed-off-by: Jussi Hallila --- .changeset/six-coins-admire.md | 25 ++++++++++++++++--- packages/backend/src/plugins/techInsights.ts | 18 ++++++++----- plugins/tech-insights-backend/README.md | 4 +-- .../src/service/fact/createFactRetriever.ts | 16 ++++++++---- .../src/service/fact/factRetrievers/utils.ts | 12 ++++++--- .../persistence/TechInsightsDatabase.test.ts | 10 ++++---- .../persistence/TechInsightsDatabase.ts | 13 +++++----- plugins/tech-insights-node/src/facts.ts | 14 +++++------ 8 files changed, 72 insertions(+), 40 deletions(-) diff --git a/.changeset/six-coins-admire.md b/.changeset/six-coins-admire.md index 42d0ecbb8f..f2ef03fe01 100644 --- a/.changeset/six-coins-admire.md +++ b/.changeset/six-coins-admire.md @@ -1,12 +1,29 @@ --- -'@backstage/plugin-tech-insights-backend': patch -'@backstage/plugin-tech-insights-node': patch +'@backstage/plugin-tech-insights-backend': minor +'@backstage/plugin-tech-insights-node': minor --- -Adds a configuration option to fact retrievers to define lifecycle for facts the retriever persists. Possible values are either 'items-to-live' or 'time-to-live'. The former will keep only n number of items in to the database for each fact per entity. The latter will remove all facts that are older than the TTL value. +BREAKING CHANGES: + +- The helper function to create a fact retriever registration is now expecting an object of configuration items instead of individual arguments. + Modify your techInsights.ts plugin configuration in `packages/backend/src/plugins/techInsights.ts` (or equivalent) the following way: + +```diff +-createFactRetrieverRegistration( +- '1 1 1 * *', // Example cron, At 01:01 on day-of-month 1. +- entityOwnershipFactRetriever, +-), ++createFactRetrieverRegistration({ ++ cadende: '1 1 1 * *', // Example cron, At 01:01 on day-of-month 1. ++ factRetriever: entityOwnershipFactRetriever, ++}), + +``` + +Adds a configuration option to fact retrievers to define lifecycle for facts the retriever persists. Possible values are either 'max items' or 'time-to-live'. The former will keep only n number of items in the database for each fact per entity. The latter will remove all facts that are older than the TTL value. Possible values: -- `{ itl: 5 }` // Deletes all facts for the retriever/entity pair, apart from the last five +- `{ maxItems: 5 }` // Deletes all facts for the retriever/entity pair, apart from the last five - `{ ttl: 1209600000 }` // (2 weeks) Deletes all facts older than 2 weeks for the retriever/entity pair - `{ ttl: { weeks: 2 } }` // Deletes all facts older than 2 weeks for the retriever/entity pair diff --git a/packages/backend/src/plugins/techInsights.ts b/packages/backend/src/plugins/techInsights.ts index a05ddc339a..7b0ded2240 100644 --- a/packages/backend/src/plugins/techInsights.ts +++ b/packages/backend/src/plugins/techInsights.ts @@ -40,12 +40,18 @@ export default async function createPlugin({ database, discovery, factRetrievers: [ - createFactRetrieverRegistration( - '1 1 1 * *', // Example cron, At 01:01 on day-of-month 1. - entityOwnershipFactRetriever, - ), - createFactRetrieverRegistration('1 1 1 * *', entityMetadataFactRetriever), - createFactRetrieverRegistration('1 1 1 * *', techdocsFactRetriever), + createFactRetrieverRegistration({ + cadence: '1 1 1 * *', // Example cron, At 01:01 on day-of-month 1. + factRetriever: entityOwnershipFactRetriever, + }), + createFactRetrieverRegistration({ + cadence: '1 1 1 * *', + factRetriever: entityMetadataFactRetriever, + }), + createFactRetrieverRegistration({ + cadence: '1 1 1 * *', + factRetriever: techdocsFactRetriever, + }), ], factCheckerFactory: new JsonRulesEngineFactCheckerFactory({ checks: [ diff --git a/plugins/tech-insights-backend/README.md b/plugins/tech-insights-backend/README.md index c0d903b2eb..d78048ef68 100644 --- a/plugins/tech-insights-backend/README.md +++ b/plugins/tech-insights-backend/README.md @@ -91,10 +91,10 @@ const myFactRetrieverRegistration = createFactRetrieverRegistration( ); ``` -FactRetrieverRegistration also accepts an optional `lifecycle` configuration value. This can be either ITL (items to live) or TTL (time to live). Valid options for this value are either a number for ITL or a Luxon duration like object for TTL. For example: +FactRetrieverRegistration also accepts an optional `lifecycle` configuration value. This can be either MaxItems or TTL (time to live). Valid options for this value are either a number for MaxItems or a Luxon duration like object for TTL. For example: ```ts -const itl = { itl: 7 }; // Deletes all but 7 latest facts for each id/entity pair +const maxItems = { maxItems: 7 }; // Deletes all but 7 latest facts for each id/entity pair const ttl = { ttl: 1209600000 }; // (2 weeks) Deletes items older than 2 weeks const ttlWithAHumanReadableValue = { ttl: { weeks: 2 } }; // Deletes items older than 2 weeks ``` diff --git a/plugins/tech-insights-backend/src/service/fact/createFactRetriever.ts b/plugins/tech-insights-backend/src/service/fact/createFactRetriever.ts index 1e3f7f8904..eae579b49f 100644 --- a/plugins/tech-insights-backend/src/service/fact/createFactRetriever.ts +++ b/plugins/tech-insights-backend/src/service/fact/createFactRetriever.ts @@ -19,6 +19,12 @@ import { FactRetrieverRegistration, } from '@backstage/plugin-tech-insights-node'; +export type FactRetrieverRegistrationOptions = { + cadence: string; + factRetriever: FactRetriever; + lifecycle?: FactLifecycle; +}; + /** * @public * @@ -47,11 +53,11 @@ import { * \{ itl: 7 \} -- This fact retriever will leave 7 newest items in the database when it is run * */ -export function createFactRetrieverRegistration( - cadence: string, - factRetriever: FactRetriever, - lifecycle?: FactLifecycle, -): FactRetrieverRegistration { +export function createFactRetrieverRegistration({ + cadence, + factRetriever, + lifecycle, +}: FactRetrieverRegistrationOptions): FactRetrieverRegistration { return { cadence, factRetriever, diff --git a/plugins/tech-insights-backend/src/service/fact/factRetrievers/utils.ts b/plugins/tech-insights-backend/src/service/fact/factRetrievers/utils.ts index b01bbe6248..0238721e99 100644 --- a/plugins/tech-insights-backend/src/service/fact/factRetrievers/utils.ts +++ b/plugins/tech-insights-backend/src/service/fact/factRetrievers/utils.ts @@ -16,7 +16,11 @@ import camelCase from 'lodash/camelCase'; import { Entity } from '@backstage/catalog-model'; import { get } from 'lodash'; -import { FactLifecycle, ITL, TTL } from '@backstage/plugin-tech-insights-node'; +import { + FactLifecycle, + MaxItems, + TTL, +} from '@backstage/plugin-tech-insights-node'; export const generateAnnotationFactName = (annotation: string) => camelCase(`hasAnnotation-${annotation}`); @@ -25,9 +29,9 @@ export const entityHasAnnotation = (entity: Entity, annotation: string) => Boolean(get(entity, ['metadata', 'annotations', annotation])); export const isTtl = (lifecycle: FactLifecycle): lifecycle is TTL => { - return !!(lifecycle as TTL).ttl; + return !!(lifecycle as TTL).timeToLive; }; -export const isItl = (lifecycle: FactLifecycle): lifecycle is ITL => { - return !!(lifecycle as ITL).itl; +export const isMaxItems = (lifecycle: FactLifecycle): lifecycle is MaxItems => { + return !!(lifecycle as MaxItems).maxItems; }; 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 a1bb2dc299..07c01eb5b0 100644 --- a/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.test.ts +++ b/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.test.ts @@ -265,7 +265,7 @@ describe('Tech Insights database', () => { }); }); - it('should delete extraneous rows when ITL is defined. Should leave only n latest', async () => { + it('should delete extraneous rows when MaxItems is defined. Should leave only n latest', async () => { const deviledFact = (it: {}) => ({ ...it, facts: JSON.stringify({ @@ -289,11 +289,11 @@ describe('Tech Insights database', () => { testNumberFact: 555, }, }; - const itl = 2; - await store.insertFacts('test-fact', [factToBeInserted], { itl }); + const maxItems = 2; + await store.insertFacts('test-fact', [factToBeInserted], { maxItems }); const afterInsertionFacts = await testDbClient('facts').select(); - expect(afterInsertionFacts).toHaveLength(itl); + expect(afterInsertionFacts).toHaveLength(maxItems); expect(afterInsertionFacts[0]).toMatchObject( deviledFact(additionalFacts[0]), ); @@ -335,7 +335,7 @@ describe('Tech Insights database', () => { }, }; await store.insertFacts('test-fact', [factToBeInserted], { - ttl: { weeks: 2 }, + timeToLive: { weeks: 2 }, }); const afterInsertionFacts = await testDbClient('facts') diff --git a/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts b/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts index bb29657c07..682b7967bb 100644 --- a/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts +++ b/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts @@ -27,7 +27,7 @@ import { groupBy, omit } from 'lodash'; import { DateTime } from 'luxon'; import { Logger } from 'winston'; import { parseEntityName, stringifyEntityRef } from '@backstage/catalog-model'; -import { isItl, isTtl } from '../fact/factRetrievers/utils'; +import { isMaxItems, isTtl } from '../fact/factRetrievers/utils'; import Transaction = Knex.Transaction; export type RawDbFactRow = { @@ -108,12 +108,11 @@ export class TechInsightsDatabase implements TechInsightsStore { await tx.batchInsert('facts', factRows, this.CHUNK_SIZE); if (lifecycle && isTtl(lifecycle)) { - const expiration = DateTime.now().minus(lifecycle.ttl); + const expiration = DateTime.now().minus(lifecycle.timeToLive); await this.deleteExpiredFactsByDate(tx, factRows, expiration); } - if (lifecycle && isItl(lifecycle)) { - const items = lifecycle.itl; - await this.deleteExpiredFactsByNumber(tx, factRows, items); + if (lifecycle && isMaxItems(lifecycle)) { + await this.deleteExpiredFactsByNumber(tx, factRows, lifecycle.maxItems); } }); } @@ -204,7 +203,7 @@ export class TechInsightsDatabase implements TechInsightsStore { private async deleteExpiredFactsByNumber( tx: Transaction, factRows: { id: string; entity: string }[], - items: number, + maxItems: number, ) { const deletables = await tx('facts') .whereIn( @@ -220,7 +219,7 @@ export class TechInsightsDatabase implements TechInsightsStore { row_number() over (partition by id, entity order by timestamp desc) as fact_rank from facts) ranks where fact_rank <= ?? ) as filterjoin`, - items, + maxItems, ), joinClause => { joinClause diff --git a/plugins/tech-insights-node/src/facts.ts b/plugins/tech-insights-node/src/facts.ts index 34c753486a..7a70f92f92 100644 --- a/plugins/tech-insights-node/src/facts.ts +++ b/plugins/tech-insights-node/src/facts.ts @@ -196,29 +196,29 @@ export interface FactRetriever { * A Luxon duration like object for time to live value * * @example - * \{ ttl: 1209600000 \} - * \{ ttl: \{ weeks: 4 \} \} + * \{ timeToLive: 1209600000 \} + * \{ timeToLive: \{ weeks: 4 \} \} * **/ -export type TTL = { ttl: DurationLike }; +export type TTL = { timeToLive: DurationLike }; /** * @public * - * A number for items to live value + * A maximum number for items to be kept in the database for each fact retriever/entity pair * * @example - * \{ itl: 10 \} + * \{ maxItems: 10 \} * **/ -export type ITL = { itl: number }; +export type MaxItems = { maxItems: number }; /** * @public * * A fact lifecycle definition. Determines which strategy to use to purge expired facts from the database. */ -export type FactLifecycle = TTL | ITL; +export type FactLifecycle = TTL | MaxItems; /** * @public From cd8bf7a4abb9a586e1d9eba70d5771458b96ffec Mon Sep 17 00:00:00 2001 From: Jussi Hallila Date: Thu, 13 Jan 2022 16:34:22 +0100 Subject: [PATCH 4/5] Add more breaking changes. Update API reports. Signed-off-by: Jussi Hallila --- .changeset/six-coins-admire.md | 13 ++++++++++- plugins/tech-insights-backend/api-report.md | 17 +++++++++----- plugins/tech-insights-backend/src/index.ts | 1 + .../src/service/fact/FactRetrieverEngine.ts | 6 ++++- .../src/service/fact/createFactRetriever.ts | 10 ++++++++- .../persistence/TechInsightsDatabase.test.ts | 12 +++++++--- .../persistence/TechInsightsDatabase.ts | 14 +++++++----- plugins/tech-insights-node/api-report.md | 22 +++++++++++-------- plugins/tech-insights-node/src/persistence.ts | 14 +++++++----- 9 files changed, 79 insertions(+), 30 deletions(-) diff --git a/.changeset/six-coins-admire.md b/.changeset/six-coins-admire.md index f2ef03fe01..658672d1c4 100644 --- a/.changeset/six-coins-admire.md +++ b/.changeset/six-coins-admire.md @@ -6,7 +6,7 @@ BREAKING CHANGES: - The helper function to create a fact retriever registration is now expecting an object of configuration items instead of individual arguments. - Modify your techInsights.ts plugin configuration in `packages/backend/src/plugins/techInsights.ts` (or equivalent) the following way: + Modify your `techInsights.ts` plugin configuration in `packages/backend/src/plugins/techInsights.ts` (or equivalent) the following way: ```diff -createFactRetrieverRegistration( @@ -20,6 +20,17 @@ BREAKING CHANGES: ``` +- `TechInsightsStore` interface has changed its signature of `insertFacts` method. If you have created your own implementation of either `TechInsightsDatabase` or `FactRetrieverEngine` you need to modify the implementation/call to this method to accept/pass-in an object instead if individual arguments. The interface now accepts an additional `lifecycle` argument which is optional (defined below). An example modification to fact retriever engine: + +```diff +-await this.repository.insertFacts(factRetriever.id, facts); ++await this.repository.insertFacts({ ++ id: factRetriever.id, ++ facts, ++ lifecycle, ++}); +``` + Adds a configuration option to fact retrievers to define lifecycle for facts the retriever persists. Possible values are either 'max items' or 'time-to-live'. The former will keep only n number of items in the database for each fact per entity. The latter will remove all facts that are older than the TTL value. Possible values: diff --git a/plugins/tech-insights-backend/api-report.md b/plugins/tech-insights-backend/api-report.md index c5cefd2619..1d19123a00 100644 --- a/plugins/tech-insights-backend/api-report.md +++ b/plugins/tech-insights-backend/api-report.md @@ -26,11 +26,11 @@ export const buildTechInsightsContext: < ) => Promise>; // @public -export function createFactRetrieverRegistration( - cadence: string, - factRetriever: FactRetriever, - lifecycle?: FactLifecycle, -): FactRetrieverRegistration; +export function createFactRetrieverRegistration({ + cadence, + factRetriever, + lifecycle, +}: FactRetrieverRegistrationOptions): FactRetrieverRegistration; // @public export function createRouter< @@ -44,6 +44,13 @@ export const entityMetadataFactRetriever: FactRetriever; // @public export const entityOwnershipFactRetriever: FactRetriever; +// @public (undocumented) +export type FactRetrieverRegistrationOptions = { + cadence: string; + factRetriever: FactRetriever; + lifecycle?: FactLifecycle; +}; + // @public export type PersistenceContext = { techInsightsStore: TechInsightsStore; diff --git a/plugins/tech-insights-backend/src/index.ts b/plugins/tech-insights-backend/src/index.ts index 763c565108..82fda47367 100644 --- a/plugins/tech-insights-backend/src/index.ts +++ b/plugins/tech-insights-backend/src/index.ts @@ -25,4 +25,5 @@ export type { export type { PersistenceContext } from './service/persistence/persistenceContext'; export { createFactRetrieverRegistration } from './service/fact/createFactRetriever'; +export type { FactRetrieverRegistrationOptions } from './service/fact/createFactRetriever'; export * from './service/fact/factRetrievers'; diff --git a/plugins/tech-insights-backend/src/service/fact/FactRetrieverEngine.ts b/plugins/tech-insights-backend/src/service/fact/FactRetrieverEngine.ts index 51a68635b1..0334599b96 100644 --- a/plugins/tech-insights-backend/src/service/fact/FactRetrieverEngine.ts +++ b/plugins/tech-insights-backend/src/service/fact/FactRetrieverEngine.ts @@ -125,7 +125,11 @@ export class FactRetrieverEngine { } try { - await this.repository.insertFacts(factRetriever.id, facts, lifecycle); + await this.repository.insertFacts({ + id: factRetriever.id, + facts, + lifecycle, + }); this.logger.info( `Stored ${facts.length} facts for fact retriever ${ factRetriever.id diff --git a/plugins/tech-insights-backend/src/service/fact/createFactRetriever.ts b/plugins/tech-insights-backend/src/service/fact/createFactRetriever.ts index eae579b49f..bd85b3c873 100644 --- a/plugins/tech-insights-backend/src/service/fact/createFactRetriever.ts +++ b/plugins/tech-insights-backend/src/service/fact/createFactRetriever.ts @@ -19,6 +19,14 @@ import { FactRetrieverRegistration, } from '@backstage/plugin-tech-insights-node'; +/** + * @public + * + * @param cadence - cron expression to indicate when the fact retriever should be triggered + * @param factRetriever - Implementation of fact retriever consisting of at least id, version, schema and handler + * @param lifecycle - Optional lifecycle definition indicating the cleanup logic of facts when this retriever is run + * + */ export type FactRetrieverRegistrationOptions = { cadence: string; factRetriever: FactRetriever; @@ -50,7 +58,7 @@ export type FactRetrieverRegistrationOptions = { * * Valid lifecycle values: * \{ ttl: \{ weeks: 2 \} \} -- This fact retriever will remove items that are older than 2 weeks when it is run - * \{ itl: 7 \} -- This fact retriever will leave 7 newest items in the database when it is run + * \{ maxItems: 7 \} -- This fact retriever will leave 7 newest items in the database when it is run * */ export function createFactRetrieverRegistration({ 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 07c01eb5b0..bd330bd378 100644 --- a/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.test.ts +++ b/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.test.ts @@ -290,7 +290,11 @@ describe('Tech Insights database', () => { }, }; const maxItems = 2; - await store.insertFacts('test-fact', [factToBeInserted], { maxItems }); + await store.insertFacts({ + id: 'test-fact', + facts: [factToBeInserted], + lifecycle: { maxItems }, + }); const afterInsertionFacts = await testDbClient('facts').select(); expect(afterInsertionFacts).toHaveLength(maxItems); @@ -334,8 +338,10 @@ describe('Tech Insights database', () => { testNumberFact: 555, }, }; - await store.insertFacts('test-fact', [factToBeInserted], { - timeToLive: { weeks: 2 }, + await store.insertFacts({ + id: 'test-fact', + facts: [factToBeInserted], + lifecycle: { timeToLive: { weeks: 2 } }, }); const afterInsertionFacts = await testDbClient('facts') diff --git a/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts b/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts index 682b7967bb..be761ff555 100644 --- a/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts +++ b/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts @@ -87,11 +87,15 @@ export class TechInsightsDatabase implements TechInsightsStore { } } - async insertFacts( - id: string, - facts: TechInsightFact[], - lifecycle?: FactLifecycle, - ): Promise { + async insertFacts({ + id, + facts, + lifecycle, + }: { + id: string; + facts: TechInsightFact[]; + lifecycle?: FactLifecycle; + }): Promise { if (facts.length === 0) return; const currentSchema = await this.getLatestSchema(id); const factRows = facts.map(it => { diff --git a/plugins/tech-insights-node/api-report.md b/plugins/tech-insights-node/api-report.md index ec822bf5e2..77a9d77871 100644 --- a/plugins/tech-insights-node/api-report.md +++ b/plugins/tech-insights-node/api-report.md @@ -39,7 +39,7 @@ export interface FactCheckerFactory< } // @public -export type FactLifecycle = TTL | ITL; +export type FactLifecycle = TTL | MaxItems; // @public export interface FactRetriever { @@ -88,8 +88,8 @@ export type FlatTechInsightFact = TechInsightFact & { }; // @public -export type ITL = { - itl: number; +export type MaxItems = { + maxItems: number; }; // @public @@ -154,17 +154,21 @@ export interface TechInsightsStore { [factRef: string]: FlatTechInsightFact; }>; getLatestSchemas(ids?: string[]): Promise; - insertFacts( - id: string, - facts: TechInsightFact[], - lifecycle?: FactLifecycle, - ): Promise; + insertFacts({ + id, + facts, + lifecycle, + }: { + id: string; + facts: TechInsightFact[]; + lifecycle?: FactLifecycle; + }): Promise; insertFactSchema(schemaDefinition: FactSchemaDefinition): Promise; } // @public export type TTL = { - ttl: DurationLike; + timeToLive: DurationLike; }; // (No @packageDocumentation comment for this package) diff --git a/plugins/tech-insights-node/src/persistence.ts b/plugins/tech-insights-node/src/persistence.ts index 17a3867347..4324c24c42 100644 --- a/plugins/tech-insights-node/src/persistence.ts +++ b/plugins/tech-insights-node/src/persistence.ts @@ -38,11 +38,15 @@ export interface TechInsightsStore { * @param facts - A collection of TechInsightFacts * @param lifecycle - (Optional) Fact lifecycle object indicating the expiration logic for these items */ - insertFacts( - id: string, - facts: TechInsightFact[], - lifecycle?: FactLifecycle, - ): Promise; + insertFacts({ + id, + facts, + lifecycle, + }: { + id: string; + facts: TechInsightFact[]; + lifecycle?: FactLifecycle; + }): Promise; /** * @param ids - A collection of fact row identifiers From 87f37b167499fbaf54adc21ef49863a4ad5ce5fa Mon Sep 17 00:00:00 2001 From: Jussi Hallila Date: Fri, 14 Jan 2022 13:20:35 +0100 Subject: [PATCH 5/5] Knexify queries. Add additional test cases for cases where multiple entities are inserted. Signed-off-by: Jussi Hallila --- .../persistence/TechInsightsDatabase.test.ts | 166 ++++++++++++++++++ .../persistence/TechInsightsDatabase.ts | 30 ++-- 2 files changed, 185 insertions(+), 11 deletions(-) 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 bd330bd378..0b81ad490f 100644 --- a/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.test.ts +++ b/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.test.ts @@ -310,6 +310,81 @@ describe('Tech Insights database', () => { }); }); + it('should delete extraneous rows for each entity when MaxItems is defined. Should leave only n latest', async () => { + const deviledFact = (it: {}) => ({ + ...it, + facts: JSON.stringify({ + testNumberFact: 666, + }), + }); + await testDbClient.batchInsert('facts', additionalFacts.map(deviledFact)); + + const preInsertionFacts = await testDbClient('facts').select(); + + expect(preInsertionFacts).toHaveLength(3); + + await testDbClient.batchInsert( + 'facts', + preInsertionFacts.map(it => ({ ...it, entity: 'b:b/b' })), + ); + + const timestamp = DateTime.now().plus(Duration.fromMillis(1111)); + const factsToBeInserted = [ + { + timestamp: timestamp, + entity: { + namespace: 'a', + kind: 'a', + name: 'a', + }, + facts: { + testNumberFact: 555, + }, + }, + { + timestamp: timestamp, + entity: { + namespace: 'b', + kind: 'b', + name: 'b', + }, + facts: { + testNumberFact: 555, + }, + }, + ]; + const maxItems = 2; + await store.insertFacts({ + id: 'test-fact', + facts: factsToBeInserted, + lifecycle: { maxItems }, + }); + + const afterInsertionFacts = await testDbClient('facts').select(); + + const inserted = { + id: 'test-fact', + version: '0.0.1-test', + timestamp: timestamp.toISO(), + entity: 'a:a/a', + facts: JSON.stringify({ testNumberFact: 555 }), + }; + expect(afterInsertionFacts).toHaveLength(maxItems * 2); + expect(afterInsertionFacts[0]).toMatchObject( + deviledFact(additionalFacts[0]), + ); + + expect(afterInsertionFacts[1]).toMatchObject({ + ...deviledFact(additionalFacts[0]), + entity: 'b:b/b', + }); + expect(afterInsertionFacts[2]).toMatchObject(inserted); + expect(afterInsertionFacts[3]).toMatchObject({ + ...inserted, + entity: 'b:b/b', + }); + }); + it('should delete extraneous rows when TTL is defined. Should leave only items with timestamp greater than TTL', async () => { const oldStaledOutFact = (it: {}) => ({ ...it, @@ -362,4 +437,95 @@ describe('Tech Insights database', () => { oldStaledOutFact(additionalFacts[0]), ); }); + + it('should delete extraneous rows for each entity when TTL expired', async () => { + const oldStaledOutFact = (it: {}) => ({ + ...it, + facts: JSON.stringify({ + testNumberFact: 666, + }), + timestamp: DateTime.now().minus({ weeks: 3 }).toISO(), + }); + await testDbClient.batchInsert( + 'facts', + additionalFacts.map(oldStaledOutFact), + ); + + const preInsertionFacts = await testDbClient('facts').select(); + expect(preInsertionFacts).toHaveLength(3); + + await testDbClient.batchInsert( + 'facts', + preInsertionFacts.map(it => ({ ...it, entity: 'b:b/b' })), + ); + + const timestamp = DateTime.now().plus(Duration.fromMillis(1111)); + const factsToBeInserted = [ + { + timestamp: timestamp, + entity: { + namespace: 'a', + kind: 'a', + name: 'a', + }, + facts: { + testNumberFact: 555, + }, + }, + { + timestamp: timestamp, + entity: { + namespace: 'b', + kind: 'b', + name: 'b', + }, + facts: { + testNumberFact: 555, + }, + }, + ]; + await store.insertFacts({ + id: 'test-fact', + facts: factsToBeInserted, + lifecycle: { timeToLive: { weeks: 2 } }, + }); + + const afterInsertionFacts = await testDbClient('facts') + .select() + .orderBy('timestamp', 'desc'); + + expect(afterInsertionFacts).toHaveLength(6); + expect(afterInsertionFacts[0]).toMatchObject({ + id: 'test-fact', + version: '0.0.1-test', + timestamp: timestamp.toISO(), + entity: 'a:a/a', + facts: JSON.stringify({ testNumberFact: 555 }), + }); + expect(afterInsertionFacts[1]).toMatchObject({ + id: 'test-fact', + version: '0.0.1-test', + timestamp: timestamp.toISO(), + entity: 'b:b/b', + facts: JSON.stringify({ testNumberFact: 555 }), + }); + expect(afterInsertionFacts[2]).toMatchObject(facts[1]); + expect(afterInsertionFacts[3]).toMatchObject({ + ...facts[1], + entity: 'b:b/b', + }); + expect(afterInsertionFacts[4]).toMatchObject(facts[0]); + expect(afterInsertionFacts[5]).toMatchObject({ + ...facts[0], + entity: 'b:b/b', + }); + + expect(afterInsertionFacts).not.toContainEqual( + oldStaledOutFact(additionalFacts[0]), + ); + expect(afterInsertionFacts).not.toContainEqual({ + ...oldStaledOutFact(additionalFacts[0]), + entity: 'b:b/b', + }); + }); }); diff --git a/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts b/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts index be761ff555..c6dda440fb 100644 --- a/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts +++ b/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts @@ -215,16 +215,25 @@ export class TechInsightsDatabase implements TechInsightsStore { factRows.map(it => [it.id, it.entity]), ) .and.leftJoin( - this.db.raw( - `(select * - from (select id fid, - entity fentity, - timestamp ftimestamp, - row_number() over (partition by id, entity order by timestamp desc) as fact_rank - from facts) ranks - where fact_rank <= ?? ) as filterjoin`, - maxItems, - ), + joinTable => + joinTable + .select('*') + .from( + this.db('facts') + .column( + { fid: 'id' }, + { fentity: 'entity' }, + { ftimestamp: 'timestamp' }, + ) + .column( + this.db.raw( + 'row_number() over (partition by id, entity order by timestamp desc) as fact_rank', + ), + ) + .as('ranks'), + ) + .where('fact_rank', '<=', maxItems) + .as('filterjoin'), joinClause => { joinClause .on('filterjoin.fid', 'facts.id') @@ -233,7 +242,6 @@ export class TechInsightsDatabase implements TechInsightsStore { }, ) .whereNull('filterjoin.fid'); - await tx('facts') .whereIn( ['id', 'entity', 'timestamp'],