From 15278fae30d2948011de44d66660617f0920b707 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 22 Oct 2020 17:36:56 +0200 Subject: [PATCH] catalog-backend: review fixes --- packages/catalog-model/src/kinds/relations.ts | 4 +-- .../catalog/DatabaseEntitiesCatalog.test.ts | 12 +++---- .../src/catalog/DatabaseEntitiesCatalog.ts | 36 +++++++++---------- plugins/catalog-backend/src/catalog/types.ts | 8 ++--- .../src/ingestion/HigherOrderOperations.ts | 4 --- .../processors/OwnerRelationProcessor.ts | 11 +++--- 6 files changed, 34 insertions(+), 41 deletions(-) diff --git a/packages/catalog-model/src/kinds/relations.ts b/packages/catalog-model/src/kinds/relations.ts index 3964da99f9..846313ec8c 100644 --- a/packages/catalog-model/src/kinds/relations.ts +++ b/packages/catalog-model/src/kinds/relations.ts @@ -32,8 +32,8 @@ export const RELATION_OWNER_OF = 'ownerOf'; /** * A relation with an API entity, typically from a component or system */ -export const RELATION_IMPLEMENTED_BY = 'implementedBy'; -export const RELATION_IMPLEMENTS = 'implements'; +export const RELATION_CONSUMES_API = 'consumesApi'; +export const RELATION_PROVIDES_API = 'providesApi'; /** * A relation denoting a dependency on another entity. diff --git a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts index 3e3a5a1972..a801015915 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts @@ -18,7 +18,7 @@ import { getVoidLogger } from '@backstage/backend-common'; import type { Entity } from '@backstage/catalog-model'; import { Database, DatabaseManager } from '../database'; import { DatabaseEntitiesCatalog } from './DatabaseEntitiesCatalog'; -import { EntityMutationRequest } from './types'; +import { EntityUpsertRequest } from './types'; describe('DatabaseEntitiesCatalog', () => { let db: jest.Mocked; @@ -272,8 +272,8 @@ describe('DatabaseEntitiesCatalog', () => { await DatabaseManager.createTestDatabase(), getVoidLogger(), ); - const entities: EntityMutationRequest[] = []; - for (let i = 0; i < 200; ++i) { + const entities: EntityUpsertRequest[] = []; + for (let i = 0; i < 300; ++i) { entities.push({ entity: { apiVersion: 'a', @@ -286,21 +286,21 @@ describe('DatabaseEntitiesCatalog', () => { await catalog.batchAddOrUpdateEntities(entities); const afterFirst = await catalog.entities(); - expect(afterFirst.length).toBe(200); + expect(afterFirst.length).toBe(300); entities[40].entity.metadata.op = 'changed'; entities.push({ entity: { apiVersion: 'a', kind: 'k', - metadata: { name: `n200`, op: 'added' }, + metadata: { name: `n300`, op: 'added' }, }, relations: [], }); await catalog.batchAddOrUpdateEntities(entities); const afterSecond = await catalog.entities(); - expect(afterSecond.length).toBe(201); + expect(afterSecond.length).toBe(301); expect(afterSecond.find(e => e.metadata.op === 'changed')).toBeDefined(); expect(afterSecond.find(e => e.metadata.op === 'added')).toBeDefined(); }, 10000); diff --git a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts index 18769cfdf7..6c3065aeaa 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts @@ -31,8 +31,8 @@ import type { Database, DbEntityResponse, EntityFilters } from '../database'; import { durationText } from '../util/timing'; import type { EntitiesCatalog, - EntityMutationRequest, - EntityMutationResponse, + EntityUpsertRequest, + EntityUpsertResponse, } from './types'; type BatchContext = { @@ -134,9 +134,9 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { * @param locationId The location that they all belong to */ async batchAddOrUpdateEntities( - requests: EntityMutationRequest[], + requests: EntityUpsertRequest[], locationId?: string, - ): Promise { + ): Promise { // Group the entities by unique kind+namespace combinations const entitiesByKindAndNamespace = groupBy(requests, ({ entity }) => { const name = getEntityName(entity); @@ -144,7 +144,7 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { }); const limiter = limiterFactory(BATCH_CONCURRENCY); - const tasks: Promise[] = []; + const tasks: Promise[] = []; for (const groupRequests of Object.values(entitiesByKindAndNamespace)) { const { kind, namespace } = getEntityName(groupRequests[0].entity); @@ -157,7 +157,7 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { limiter(async () => { const first = serializeEntityRef(batch[0].entity); const last = serializeEntityRef(batch[batch.length - 1].entity); - const modifiedEntityIds: EntityMutationResponse[] = []; + const modifiedEntityIds: EntityUpsertResponse[] = []; this.logger.debug( `Considering batch ${first}-${last} (${batch.length} entries)`, ); @@ -224,12 +224,12 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { // produce the list of entities to be added, and the list of entities to be // updated private async analyzeBatch( - requests: EntityMutationRequest[], + requests: EntityUpsertRequest[], { kind, namespace }: BatchContext, ): Promise<{ - toAdd: EntityMutationRequest[]; - toUpdate: EntityMutationRequest[]; - toIgnore: EntityMutationRequest[]; + toAdd: EntityUpsertRequest[]; + toUpdate: EntityUpsertRequest[]; + toIgnore: EntityUpsertRequest[]; }> { const markTimestamp = process.hrtime(); @@ -244,9 +244,9 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { oldEntities.map(e => [e.metadata.name, e]), ); - const toAdd: EntityMutationRequest[] = []; - const toUpdate: EntityMutationRequest[] = []; - const toIgnore: EntityMutationRequest[] = []; + const toAdd: EntityUpsertRequest[] = []; + const toUpdate: EntityUpsertRequest[] = []; + const toIgnore: EntityUpsertRequest[] = []; for (const request of requests) { const newEntity = request.entity; @@ -275,9 +275,9 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { // Efficiently adds the given entities to storage, under the assumption that // they do not conflict with any existing entities private async batchAdd( - requests: EntityMutationRequest[], + requests: EntityUpsertRequest[], { locationId }: BatchContext, - ): Promise { + ): Promise { const markTimestamp = process.hrtime(); const res = await this.database.transaction( @@ -306,11 +306,11 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { // Efficiently updates the given entities into storage, under the assumption // that there already exist entities with the same names private async batchUpdate( - requests: EntityMutationRequest[], + requests: EntityUpsertRequest[], { locationId }: BatchContext, - ): Promise { + ): Promise { const markTimestamp = process.hrtime(); - const responseIds: EntityMutationResponse[] = []; + const responseIds: EntityUpsertResponse[] = []; // TODO(freben): Still not batched for (const entity of requests) { const res = await this.addOrUpdateEntity(entity.entity, locationId); diff --git a/plugins/catalog-backend/src/catalog/types.ts b/plugins/catalog-backend/src/catalog/types.ts index c9c514c42f..4d821c428a 100644 --- a/plugins/catalog-backend/src/catalog/types.ts +++ b/plugins/catalog-backend/src/catalog/types.ts @@ -21,12 +21,12 @@ import type { EntityFilters } from '../database'; // Entities // -export type EntityMutationRequest = { +export type EntityUpsertRequest = { entity: Entity; relations: EntityRelationSpec[]; }; -export type EntityMutationResponse = { +export type EntityUpsertResponse = { entityId: string; }; @@ -41,9 +41,9 @@ export type EntitiesCatalog = { * @param locationId The location that they all belong to */ batchAddOrUpdateEntities( - entities: EntityMutationRequest[], + entities: EntityUpsertRequest[], locationId?: string, - ): Promise; + ): Promise; }; // diff --git a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts index 314b91cb13..44e11c551e 100644 --- a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts +++ b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts @@ -103,10 +103,6 @@ export class HigherOrderOperations implements HigherOrderOperation { location.id, ); - if (writtenEntities.length === 0) { - return { location, entities: [] }; - } - const entities = await this.entitiesCatalog.entities({ 'metadata.uid': writtenEntities.map(e => e.entityId), }); diff --git a/plugins/catalog-backend/src/ingestion/processors/OwnerRelationProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/OwnerRelationProcessor.ts index 62a02207f1..4c67589789 100644 --- a/plugins/catalog-backend/src/ingestion/processors/OwnerRelationProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/OwnerRelationProcessor.ts @@ -23,6 +23,7 @@ import { ComponentEntityV1alpha1, RELATION_OWNED_BY, RELATION_OWNER_OF, + getEntityName, } from '@backstage/catalog-model'; import { CatalogProcessor, CatalogProcessorEmit } from './types'; import * as result from './results'; @@ -46,11 +47,7 @@ export class OwnerRelationProcessor implements CatalogProcessor { if (owner) { const namespace = entity.metadata.namespace ?? ENTITY_DEFAULT_NAMESPACE; - const selfRef = { - kind: entity.kind, - name: entity.metadata.name, - namespace, - }; + const selfRef = getEntityName(entity); const ownerRef = parseEntityRef(owner, { defaultKind: 'group', defaultNamespace: namespace, @@ -58,15 +55,15 @@ export class OwnerRelationProcessor implements CatalogProcessor { emit( result.relation({ - type: RELATION_OWNED_BY, source: selfRef, + type: RELATION_OWNED_BY, target: ownerRef, }), ); emit( result.relation({ - type: RELATION_OWNER_OF, source: ownerRef, + type: RELATION_OWNER_OF, target: selfRef, }), );