Store entities completely in the database

packages/backend-common/src/logging/index.ts

@@ -15,3 +15,4 @@
  */
 
 export * from './rootLogger';
+export * from './voidLogger';

packages/backend-common/src/logging/voidLogger.ts

@@ -14,15 +14,14 @@
  * limitations under the License.
  */
 
-import { ReaderOutput } from '../types';
+import { PassThrough } from 'stream';
+import winston, { Logger } from 'winston';
 
-export type LocationSource = {
-  /**
-   * Reads the contents of a single location.
-   *
-   * @param target The location target to read
-   * @returns The parsed contents, as an array of unverified descriptors
-   * @throws An error if the location target could not be read
-   */
-  read(target: string): Promise<ReaderOutput[]>;
-};
+/**
+ * A logger that just throws away all messages.
+ */
+export function getVoidLogger(): Logger {
+  return winston.createLogger({
+    transports: [new winston.transports.Stream({ stream: new PassThrough() })],
+  });
+}

packages/backend/src/plugins/catalog.ts

@@ -29,7 +29,7 @@ export default async function ({ logger, database }: PluginEnvironment) {
   const reader = LocationReaders.create();
   const parser = DescriptorParsers.create();
 
-  const db = await DatabaseManager.createDatabase(database);
+  const db = await DatabaseManager.createDatabase(database, logger);
   runPeriodically(
     () => DatabaseManager.refreshLocations(db, reader, parser, logger),
     10000,

plugins/catalog-backend/fixtures/two_components.yaml

@@ -11,4 +11,4 @@ kind: Component
 metadata:
   name: component2
 spec:
-  type: service
+  type: website

plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+import { NotFoundError } from '@backstage/backend-common';
 import { Database } from '../database';
 import { DescriptorEnvelope } from '../ingestion/types';
 import { EntitiesCatalog } from './types';
@@ -22,12 +23,23 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
   constructor(private readonly database: Database) {}
 
   async entities(): Promise<DescriptorEnvelope[]> {
-    const items = await this.database.entities();
+    const items = await this.database.transaction(tx =>
+      this.database.entities(tx),
+    );
     return items.map(i => i.entity);
   }
 
-  async entity(name: string): Promise<DescriptorEnvelope> {
-    const item = await this.database.entity(name);
+  async entity(
+    kind: string,
+    name: string,
+    namespace: string | undefined,
+  ): Promise<DescriptorEnvelope | undefined> {
+    const item = await this.database.transaction(tx =>
+      this.database.entity(tx, kind, name, namespace),
+    );
+    if (!item) {
+      throw new NotFoundError('Entity cannot be found');
+    }
     return item.entity;
   }
 }

plugins/catalog-backend/src/catalog/StaticEntitiesCatalog.ts

@@ -15,6 +15,7 @@
  */
 
 import { NotFoundError } from '@backstage/backend-common';
+import lodash from 'lodash';
 import { DescriptorEnvelope } from '../ingestion';
 import { EntitiesCatalog } from './types';
 
@@ -26,14 +27,23 @@ export class StaticEntitiesCatalog implements EntitiesCatalog {
   }
 
   async entities(): Promise<DescriptorEnvelope[]> {
-    return this._entities.slice();
+    return lodash.cloneDeep(this._entities);
   }
 
-  async entity(name: string): Promise<DescriptorEnvelope> {
-    const item = this._entities.find(e => e.metadata?.name === name);
+  async entity(
+    kind: string,
+    name: string,
+    namespace: string | undefined,
+  ): Promise<DescriptorEnvelope | undefined> {
+    const item = this._entities.find(
+      e =>
+        kind === e.kind &&
+        name === e.metadata?.name &&
+        namespace === e.metadata?.namespace,
+    );
     if (!item) {
-      throw new NotFoundError(`Found no entity with name ${name}`);
+      throw new NotFoundError('Entity cannot be found');
     }
-    return item;
+    return lodash.cloneDeep(item);
   }
 }

plugins/catalog-backend/src/catalog/types.ts

@@ -23,7 +23,11 @@ import { DescriptorEnvelope } from '../ingestion';
 
 export type EntitiesCatalog = {
   entities(): Promise<DescriptorEnvelope[]>;
-  entity(id: string): Promise<DescriptorEnvelope>;
+  entity(
+    kind: string,
+    name: string,
+    namespace: string | undefined,
+  ): Promise<DescriptorEnvelope | undefined>;
 };
 
 //

plugins/catalog-backend/src/database/Database.test.ts

@@ -14,30 +14,74 @@
  * limitations under the License.
  */
 
-import knex from 'knex';
+import {
+  ConflictError,
+  getVoidLogger,
+  NotFoundError,
+} from '@backstage/backend-common';
+import Knex from 'knex';
 import path from 'path';
 import { Database } from './Database';
-import { AddDatabaseLocation, DbLocationsRow } from './types';
+import {
+  AddDatabaseLocation,
+  DbEntityRequest,
+  DbEntityResponse,
+  DbLocationsRow,
+} from './types';
 
 describe('Database', () => {
-  const database = knex({
-    client: 'sqlite3',
-    connection: ':memory:',
-    useNullAsDefault: true,
-  });
-  database.client.pool.on('createSuccess', (_eventId: any, resource: any) => {
-    resource.run('PRAGMA foreign_keys = ON', () => {});
-  });
+  let database: Knex;
+  let entityRequest: DbEntityRequest;
+  let entityResponse: DbEntityResponse;
 
   beforeEach(async () => {
+    database = Knex({
+      client: 'sqlite3',
+      connection: ':memory:',
+      useNullAsDefault: true,
+    });
+
+    await database.raw('PRAGMA foreign_keys = ON');
     await database.migrate.latest({
       directory: path.resolve(__dirname, 'migrations'),
       loadExtensions: ['.ts'],
     });
+
+    entityRequest = {
+      entity: {
+        apiVersion: 'a',
+        kind: 'b',
+        metadata: {
+          name: 'c',
+          namespace: 'd',
+          labels: { e: 'f' },
+          annotations: { g: 'h' },
+        },
+        spec: { i: 'j' },
+      },
+    };
+
+    entityResponse = {
+      locationId: undefined,
+      entity: {
+        apiVersion: 'a',
+        kind: 'b',
+        metadata: {
+          uid: expect.anything(),
+          etag: expect.anything(),
+          generation: expect.anything(),
+          name: 'c',
+          namespace: 'd',
+          labels: { e: 'f' },
+          annotations: { g: 'h' },
+        },
+        spec: { i: 'j' },
+      },
+    };
   });
 
   it('manages locations', async () => {
-    const db = new Database(database);
+    const db = new Database(database, getVoidLogger());
     const input: AddDatabaseLocation = { type: 'a', target: 'b' };
     const output: DbLocationsRow = {
       id: expect.anything(),
@@ -62,7 +106,7 @@ describe('Database', () => {
 
   it('instead of adding second location with the same target, returns existing one', async () => {
     // Prepare
-    const catalog = new Database(database);
+    const catalog = new Database(database, getVoidLogger());
     const input: AddDatabaseLocation = { type: 'a', target: 'b' };
     const output1: DbLocationsRow = await catalog.addLocation(input);
 
@@ -75,4 +119,127 @@ describe('Database', () => {
     // Locations contain only one record
     expect(locations).toEqual([output1]);
   });
+
+  describe('addEntity', () => {
+    it('happy path: adds entity to empty database', async () => {
+      const catalog = new Database(database, getVoidLogger());
+      const added = await catalog.transaction(tx =>
+        catalog.addEntity(tx, entityRequest),
+      );
+      expect(added).toStrictEqual(entityResponse);
+      expect(added.entity.metadata!.generation).toBe(1);
+    });
+
+    it('rejects adding the same-named entity twice', async () => {
+      const catalog = new Database(database, getVoidLogger());
+      await catalog.transaction(tx => catalog.addEntity(tx, entityRequest));
+      await expect(
+        catalog.transaction(tx => catalog.addEntity(tx, entityRequest)),
+      ).rejects.toThrow(ConflictError);
+    });
+
+    it('accepts adding the same-named entity twice if on different namespaces', async () => {
+      const catalog = new Database(database, getVoidLogger());
+      entityRequest.entity.metadata!.namespace = 'namespace1';
+      await catalog.transaction(tx => catalog.addEntity(tx, entityRequest));
+      entityRequest.entity.metadata!.namespace = 'namespace2';
+      await expect(
+        catalog.transaction(tx => catalog.addEntity(tx, entityRequest)),
+      ).resolves.toBeDefined();
+    });
+  });
+
+  describe('updateEntity', () => {
+    it('can read and no-op-update an entity', async () => {
+      const catalog = new Database(database, getVoidLogger());
+      const added = await catalog.transaction(tx =>
+        catalog.addEntity(tx, entityRequest),
+      );
+      const updated = await catalog.transaction(tx =>
+        catalog.updateEntity(tx, { entity: added.entity }),
+      );
+      expect(updated.entity.apiVersion).toEqual(added.entity.apiVersion);
+      expect(updated.entity.kind).toEqual(added.entity.kind);
+      expect(updated.entity.metadata!.etag).not.toEqual(
+        added.entity.metadata!.etag,
+      );
+      expect(updated.entity.metadata!.generation).toEqual(
+        added.entity.metadata!.generation,
+      );
+      expect(updated.entity.metadata!.name).toEqual(
+        added.entity.metadata!.name,
+      );
+      expect(updated.entity.metadata!.namespace).toEqual(
+        added.entity.metadata!.namespace,
+      );
+    });
+
+    it('can update name if uid matches', async () => {
+      const catalog = new Database(database, getVoidLogger());
+      const added = await catalog.transaction(tx =>
+        catalog.addEntity(tx, entityRequest),
+      );
+      added.entity.metadata!.name! = 'new!';
+      const updated = await catalog.transaction(tx =>
+        catalog.updateEntity(tx, { entity: added.entity }),
+      );
+      expect(updated.entity.metadata!.name).toEqual('new!');
+    });
+
+    it('can update fields if kind, name, and namespace match', async () => {
+      const catalog = new Database(database, getVoidLogger());
+      const added = await catalog.transaction(tx =>
+        catalog.addEntity(tx, entityRequest),
+      );
+      added.entity.apiVersion = 'something.new';
+      delete added.entity.metadata!.uid;
+      delete added.entity.metadata!.generation;
+      const updated = await catalog.transaction(tx =>
+        catalog.updateEntity(tx, { entity: added.entity }),
+      );
+      expect(updated.entity.apiVersion).toEqual('something.new');
+    });
+
+    it('rejects if kind, name, but not namespace match', async () => {
+      const catalog = new Database(database, getVoidLogger());
+      const added = await catalog.transaction(tx =>
+        catalog.addEntity(tx, entityRequest),
+      );
+      added.entity.apiVersion = 'something.new';
+      delete added.entity.metadata!.uid;
+      delete added.entity.metadata!.generation;
+      added.entity.metadata!.namespace = 'something.wrong';
+      await expect(
+        catalog.transaction(tx =>
+          catalog.updateEntity(tx, { entity: added.entity }),
+        ),
+      ).rejects.toThrow(NotFoundError);
+    });
+
+    it('fails to update an entity if etag does not match', async () => {
+      const catalog = new Database(database, getVoidLogger());
+      const added = await catalog.transaction(tx =>
+        catalog.addEntity(tx, entityRequest),
+      );
+      added.entity.metadata!.etag = 'garbage';
+      await expect(
+        catalog.transaction(tx =>
+          catalog.updateEntity(tx, { entity: added.entity }),
+        ),
+      ).rejects.toThrow(ConflictError);
+    });
+
+    it('fails to update an entity if generation does not match', async () => {
+      const catalog = new Database(database, getVoidLogger());
+      const added = await catalog.transaction(tx =>
+        catalog.addEntity(tx, entityRequest),
+      );
+      added.entity.metadata!.generation! += 100;
+      await expect(
+        catalog.transaction(tx =>
+          catalog.updateEntity(tx, { entity: added.entity }),
+        ),
+      ).rejects.toThrow(ConflictError);
+    });
+  });
 });

plugins/catalog-backend/src/database/Database.ts

@@ -14,34 +14,43 @@
  * limitations under the License.
  */
 
-import { InputError, NotFoundError } from '@backstage/backend-common';
+import {
+  ConflictError,
+  InputError,
+  NotFoundError,
+} from '@backstage/backend-common';
 import Knex from 'knex';
+import lodash from 'lodash';
 import { v4 as uuidv4 } from 'uuid';
-import { DescriptorEnvelope } from '../ingestion';
+import { Logger } from 'winston';
+import { DescriptorEnvelope, EntityMeta } from '../ingestion';
+import { buildEntitySearch } from './search';
 import {
   AddDatabaseLocation,
   DatabaseLocationUpdateLogEvent,
   DatabaseLocationUpdateLogStatus,
   DbEntitiesRow,
+  DbEntitiesSearchRow,
   DbEntityRequest,
   DbEntityResponse,
   DbLocationsRow,
 } from './types';
-import { buildEntitySearch } from './search';
 
-function serializeMetadata(
-  metadata: DescriptorEnvelope['metadata'],
-): DbEntitiesRow['metadata'] {
-  if (!metadata) {
-    return null;
-  }
-
-  const output = { ...metadata };
+function getStrippedMetadata(metadata: EntityMeta): EntityMeta {
+  const output = lodash.cloneDeep(metadata);
   delete output.uid;
   delete output.etag;
   delete output.generation;
 
-  return JSON.stringify(output);
+  return output;
+}
+
+function serializeMetadata(metadata: EntityMeta | undefined): string | null {
+  if (!metadata) {
+    return null;
+  }
+
+  return JSON.stringify(getStrippedMetadata(metadata));
 }
 
 function serializeSpec(
@@ -54,29 +63,32 @@ function serializeSpec(
   return JSON.stringify(spec);
 }
 
-function entityRequestToDb(request: DbEntityRequest): DbEntitiesRow {
+function toEntityRow(
+  locationId: string | undefined,
+  entity: DescriptorEnvelope,
+): DbEntitiesRow {
   return {
-    id: '',
-    location_id: request.locationId || null,
-    etag: new Buffer(uuidv4()).toString('base64').replace(/[^\w]/g, ''), // TODO(freben): Atomicity isn't checked using these yet
-    generation: 1, // TODO(freben): These aren't updated yet
-    api_version: request.entity.apiVersion,
-    kind: request.entity.kind,
-    name: request.entity.metadata?.name || null,
-    namespace: request.entity.metadata?.namespace || null,
-    metadata: serializeMetadata(request.entity.metadata),
-    spec: serializeSpec(request.entity.spec),
+    id: entity.metadata!.uid!,
+    location_id: locationId || null,
+    etag: entity.metadata!.etag!,
+    generation: entity.metadata!.generation!,
+    api_version: entity.apiVersion,
+    kind: entity.kind,
+    name: entity.metadata!.name || null,
+    namespace: entity.metadata!.namespace || null,
+    metadata: serializeMetadata(entity.metadata),
+    spec: serializeSpec(entity.spec),
   };
 }
 
-function entityDbToResponse(row: DbEntitiesRow): DbEntityResponse {
+function toEntityResponse(row: DbEntitiesRow): DbEntityResponse {
   const entity: DescriptorEnvelope = {
     apiVersion: row.api_version,
     kind: row.kind,
     metadata: {
       uid: row.id,
       etag: row.etag,
-      generation: row.generation,
+      generation: Number(row.generation), // cast because of sqlite
     },
   };
 
@@ -96,49 +108,229 @@ function entityDbToResponse(row: DbEntitiesRow): DbEntityResponse {
   };
 }
 
-export class Database {
-  constructor(private readonly database: Knex) {}
-
-  async addOrUpdateEntity(request: DbEntityRequest): Promise<void> {
-    if (!request.entity.metadata?.name) {
-      throw new InputError(`Entities without names are not yet supported`);
-    }
-
-    const newRow = entityRequestToDb(request);
-
-    await this.database.transaction(async tx => {
-      // TODO(freben): Currently, several locations can compete for the same entity
-      // TODO(freben): If locationId is unset in the input, it won't be overwritten - should we instead replace with null?
-      const count = await tx<DbEntitiesRow>('entities')
-        .where({ name: request.entity.metadata?.name })
-        .update({
-          ...newRow,
-          id: undefined,
-        });
-      if (!count) {
-        await tx<DbEntitiesRow>('entities').insert({
-          ...newRow,
-          id: uuidv4(),
-        });
-      }
-    });
+function specsAreEqual(
+  first: string | null,
+  second: object | undefined,
+): boolean {
+  if (!first && !second) {
+    return true;
+  } else if (!first || !second) {
+    return false;
   }
 
-  async entities(): Promise<DbEntityResponse[]> {
-    const items = await this.database<DbEntitiesRow>('entities')
+  return lodash.isEqual(JSON.parse(first), second);
+}
+
+function generateUid(): string {
+  return uuidv4();
+}
+
+function generateEtag(): string {
+  return Buffer.from(uuidv4(), 'utf8').toString('base64').replace(/[^\w]/g, '');
+}
+
+/**
+ * An abstraction on top of the underlying database, wrapping the basic CRUD
+ * needs.
+ */
+export class Database {
+  constructor(
+    private readonly database: Knex,
+    private readonly logger: Logger,
+  ) {}
+
+  /**
+   * Runs a transaction.
+   *
+   * The callback is expected to make calls back into this class. When it
+   * completes, the transaction is closed.
+   *
+   * @param fn The callback that implements the transaction
+   */
+  async transaction<T>(
+    fn: (tx: Knex.Transaction<any, any>) => Promise<T>,
+  ): Promise<T> {
+    try {
+      return await this.database.transaction<T>(fn);
+    } catch (e) {
+      this.logger.debug(`Error during transaction, ${e}`);
+
+      if (
+        /SQLITE_CONSTRAINT: UNIQUE/.test(e.message) ||
+        /unique constraint/.test(e.message)
+      ) {
+        throw new ConflictError(`Rejected due to a conflicting entity`, e);
+      }
+
+      throw e;
+    }
+  }
+
+  /**
+   * Adds a new entity to the catalog.
+   *
+   * @param tx An ongoing transaction
+   * @param request The entity being added
+   * @returns The added entity, with uid, etag and generation set
+   */
+  async addEntity(
+    tx: Knex.Transaction<any, any>,
+    request: DbEntityRequest,
+  ): Promise<DbEntityResponse> {
+    if (request.entity.metadata?.uid !== undefined) {
+      throw new InputError('May not specify uid for new entities');
+    } else if (request.entity.metadata?.etag !== undefined) {
+      throw new InputError('May not specify etag for new entities');
+    } else if (request.entity.metadata?.generation !== undefined) {
+      throw new InputError('May not specify generation for new entities');
+    }
+
+    const newEntity = lodash.cloneDeep(request.entity);
+    newEntity.metadata = Object.assign({}, newEntity.metadata, {
+      uid: generateUid(),
+      etag: generateEtag(),
+      generation: 1,
+    });
+
+    const newRow = toEntityRow(request.locationId, newEntity);
+    await tx<DbEntitiesRow>('entities').insert(newRow);
+    await this.updateEntitiesSearch(tx, newRow.id, newEntity);
+
+    return { locationId: request.locationId, entity: newEntity };
+  }
+
+  /**
+   * Updates an existing entity in the catalog.
+   *
+   * The given entity must contain enough information to identify an already
+   * stored entity in the catalog - either by uid, or by kind + namespace +
+   * name. If no matching entity is found, the operation fails.
+   *
+   * If etag or generation are given, they are taken into account. Attempts to
+   * update a matching entity, but where the etag and/or generation are not
+   * equal to the passed values, will fail.
+   *
+   * @param tx An ongoing transaction
+   * @param request The entity being updated
+   * @returns The updated entity
+   */
+  async updateEntity(
+    tx: Knex.Transaction<any, any>,
+    request: DbEntityRequest,
+  ): Promise<DbEntityResponse> {
+    const { kind } = request.entity;
+    const {
+      uid,
+      etag: expectedOldEtag,
+      generation: expectedOldGeneration,
+      name,
+      namespace,
+    } = request.entity.metadata ?? {};
+
+    // Find existing entities that match the given metadata
+    let entitySelector: Partial<DbEntitiesRow>;
+    if (uid) {
+      entitySelector = { id: uid };
+    } else if (kind && name) {
+      entitySelector = {
+        kind,
+        name: name,
+        namespace: namespace || null,
+      };
+    } else {
+      throw new InputError(
+        'Must specify either uid, or kind + name + namespace to be able to identify an entity',
+      );
+    }
+    const oldRows = await tx<DbEntitiesRow>('entities')
+      .where(entitySelector)
+      .select();
+    if (oldRows.length !== 1) {
+      throw new NotFoundError('No matching entity found');
+    }
+
+    // Validate the old entity
+    const oldRow = oldRows[0];
+    // The Number cast is here because sqlite reads it as a string, no matter
+    // what the table actually says
+    oldRow.generation = Number(oldRow.generation);
+    if (expectedOldEtag) {
+      if (expectedOldEtag !== oldRow.etag) {
+        throw new ConflictError(
+          `Etag mismatch, expected="${expectedOldEtag}" found="${oldRow.etag}"`,
+        );
+      }
+    }
+    if (expectedOldGeneration) {
+      if (expectedOldGeneration !== oldRow.generation) {
+        throw new ConflictError(
+          `Generation mismatch, expected="${expectedOldGeneration}" found="${oldRow.generation}"`,
+        );
+      }
+    }
+
+    // Build the new shape of the entity
+    const newEtag = generateEtag();
+    const newGeneration = specsAreEqual(oldRow.spec, request.entity.spec)
+      ? oldRow.generation
+      : oldRow.generation + 1;
+    const newEntity = lodash.cloneDeep(request.entity);
+    newEntity.metadata = Object.assign({}, request.entity.metadata, {
+      uid: oldRow.id,
+      etag: newEtag,
+      generation: newGeneration,
+    });
+
+    // Preserve annotations that were set on the old version of the entity,
+    // unless the new version overwrites them
+    if (oldRow.metadata) {
+      const oldMetadata = JSON.parse(oldRow.metadata) as EntityMeta;
+      if (oldMetadata.annotations) {
+        newEntity.metadata!.annotations = {
+          ...oldMetadata.annotations,
+          ...newEntity.metadata!.annotations,
+        };
+      }
+    }
+
+    // Store the updated entity; select on the old etag to ensure that we do
+    // not lose to another writer
+    const newRow = toEntityRow(request.locationId, newEntity);
+    const updatedRows = await tx<DbEntitiesRow>('entities')
+      .where({ id: oldRow.id, etag: oldRow.etag })
+      .update(newRow);
+
+    // If this happens, somebody else changed the entity just now
+    if (updatedRows !== 1) {
+      throw new ConflictError(`Failed to update entity`);
+    }
+
+    await this.updateEntitiesSearch(tx, oldRow.id, newEntity);
+    return { locationId: request.locationId, entity: newEntity };
+  }
+
+  async entities(tx: Knex.Transaction<any, any>): Promise<DbEntityResponse[]> {
+    const rows = await tx<DbEntitiesRow>('entities')
       .orderBy('namespace', 'name')
       .select();
-    return items.map(entityDbToResponse);
+    return rows.map(row => toEntityResponse(row));
   }
 
-  async entity(name: string): Promise<DbEntityResponse> {
-    const items = await this.database<DbEntitiesRow>('entities')
-      .where({ name })
+  async entity(
+    tx: Knex.Transaction<any, any>,
+    kind: string,
+    name: string,
+    namespace?: string,
+  ): Promise<DbEntityResponse | undefined> {
+    const rows = await tx<DbEntitiesRow>('entities')
+      .where({ kind, name, namespace: namespace || null })
       .select();
-    if (!items.length) {
-      throw new NotFoundError(`Found no entity with name ${name}`);
+
+    if (rows.length !== 1) {
+      return undefined;
     }
-    return entityDbToResponse(items[0]);
+
+    return toEntityResponse(rows[0]);
   }
 
   async addLocation(location: AddDatabaseLocation): Promise<DbLocationsRow> {
@@ -206,15 +398,20 @@ export class Database {
     });
   }
 
-  /*
   private async updateEntitiesSearch(
     tx: Knex.Transaction<any, any>,
     entityId: string,
     data: DescriptorEnvelope,
   ): Promise<void> {
-    const entries = buildEntitySearch(entityId, data);
-    await tx('entities_search').where({ entity_id: entityId }).del();
-    await tx('entities_search').insert(entries);
+    try {
+      const entries = buildEntitySearch(entityId, data);
+      await tx<DbEntitiesSearchRow>('entities_search')
+        .where({ entity_id: entityId })
+        .del();
+      await tx<DbEntitiesSearchRow>('entities_search').insert(entries);
+    } catch {
+      // ignore intentionally - if this happens, the entity was deleted before
+      // we got around to writing the entries
+    }
   }
-  */
 }

plugins/catalog-backend/src/database/DatabaseManager.test.ts

@@ -14,8 +14,7 @@
  * limitations under the License.
  */
 
-import { PassThrough } from 'stream';
-import winston from 'winston';
+import { getVoidLogger } from '@backstage/backend-common';
 import {
   ComponentDescriptor,
   DescriptorParser,
@@ -25,16 +24,12 @@ import {
 import { Database } from './Database';
 import { DatabaseManager } from './DatabaseManager';
 import { DatabaseLocationUpdateLogStatus, DbLocationsRow } from './types';
+import Knex from 'knex';
 
 describe('DatabaseManager', () => {
-  const logger = winston.createLogger({
-    transports: [new winston.transports.Stream({ stream: new PassThrough() })],
-  });
-
   describe('refreshLocations', () => {
     it('works with no locations added', async () => {
       const db = ({
-        addOrUpdateEntity: jest.fn(),
         locations: jest.fn().mockResolvedValue([]),
       } as unknown) as Database;
       const reader: LocationReader = {
@@ -45,33 +40,34 @@ describe('DatabaseManager', () => {
       };
 
       await expect(
-        DatabaseManager.refreshLocations(db, reader, parser, logger),
+        DatabaseManager.refreshLocations(db, reader, parser, getVoidLogger()),
       ).resolves.toBeUndefined();
       expect(reader.read).not.toHaveBeenCalled();
       expect(parser.parse).not.toHaveBeenCalled();
     });
 
     it('can update a single location', async () => {
-      const db = ({
-        addOrUpdateEntity: jest.fn(),
-        locations: jest.fn(() =>
-          Promise.resolve([
-            {
-              id: '123',
-              type: 'some',
-              target: 'thing',
-            } as DbLocationsRow,
-          ]),
-        ),
-        addLocationUpdateLogEvent: jest.fn(),
-      } as unknown) as Database;
-
+      const location: DbLocationsRow = {
+        id: '123',
+        type: 'some',
+        target: 'thing',
+      };
       const desc: ComponentDescriptor = {
         apiVersion: 'backstage.io/v1beta1',
         kind: 'Component',
         metadata: { name: 'c1' },
         spec: { type: 'service' },
       };
+      const tx = (undefined as unknown) as Knex.Transaction<any, any>;
+
+      const db = ({
+        transaction: jest.fn(f => f(tx)),
+        entity: jest.fn(() => Promise.resolve(undefined)),
+        addEntity: jest.fn(),
+        locations: jest.fn(() => Promise.resolve([location])),
+        addLocationUpdateLogEvent: jest.fn(),
+      } as Partial<Database>) as Database;
+
       const reader: LocationReader = {
         read: jest.fn(() => Promise.resolve([{ type: 'data', data: desc }])),
       };
@@ -80,12 +76,12 @@ describe('DatabaseManager', () => {
       };
 
       await expect(
-        DatabaseManager.refreshLocations(db, reader, parser, logger),
+        DatabaseManager.refreshLocations(db, reader, parser, getVoidLogger()),
       ).resolves.toBeUndefined();
       expect(reader.read).toHaveBeenCalledTimes(1);
       expect(reader.read).toHaveBeenNthCalledWith(1, 'some', 'thing');
-      expect(db.addOrUpdateEntity).toHaveBeenCalledTimes(1);
-      expect(db.addOrUpdateEntity).toHaveBeenNthCalledWith(1, {
+      expect(db.addEntity).toHaveBeenCalledTimes(1);
+      expect(db.addEntity).toHaveBeenNthCalledWith(1, undefined, {
         locationId: '123',
         entity: expect.objectContaining({
           metadata: expect.objectContaining({ name: 'c1' }),
@@ -94,8 +90,12 @@ describe('DatabaseManager', () => {
     });
 
     it('logs successful updates', async () => {
+      const tx = (undefined as unknown) as Knex.Transaction<any, any>;
+
       const db = ({
-        addOrUpdateEntity: jest.fn(),
+        transaction: jest.fn(f => f(tx)),
+        addEntity: jest.fn(),
+        entity: jest.fn(() => Promise.resolve(undefined)),
         locations: jest.fn(() =>
           Promise.resolve([
             {
@@ -122,7 +122,7 @@ describe('DatabaseManager', () => {
       };
 
       await expect(
-        DatabaseManager.refreshLocations(db, reader, parser, logger),
+        DatabaseManager.refreshLocations(db, reader, parser, getVoidLogger()),
       ).resolves.toBeUndefined();
 
       expect(db.addLocationUpdateLogEvent).toHaveBeenNthCalledWith(
@@ -141,8 +141,11 @@ describe('DatabaseManager', () => {
     });
 
     it('logs unsuccessful updates when parser fails', async () => {
+      const tx = (undefined as unknown) as Knex.Transaction<any, any>;
+
       const db = ({
-        addOrUpdateEntity: jest.fn(),
+        transaction: jest.fn(f => f(tx)),
+        addEntity: jest.fn(),
         locations: jest.fn(() =>
           Promise.resolve([
             {
@@ -171,7 +174,7 @@ describe('DatabaseManager', () => {
       };
 
       await expect(
-        DatabaseManager.refreshLocations(db, reader, parser, logger),
+        DatabaseManager.refreshLocations(db, reader, parser, getVoidLogger()),
       ).resolves.toBeUndefined();
 
       expect(db.addLocationUpdateLogEvent).toHaveBeenNthCalledWith(
@@ -191,8 +194,11 @@ describe('DatabaseManager', () => {
     });
 
     it('logs unsuccessful updates when reader fails', async () => {
+      const tx = (undefined as unknown) as Knex.Transaction<any, any>;
+
       const db = ({
-        addOrUpdateEntity: jest.fn(),
+        transaction: jest.fn(f => f(tx)),
+        addEntity: jest.fn(),
         locations: jest.fn(() =>
           Promise.resolve([
             {
@@ -217,7 +223,7 @@ describe('DatabaseManager', () => {
       };
 
       await expect(
-        DatabaseManager.refreshLocations(db, reader, parser, logger),
+        DatabaseManager.refreshLocations(db, reader, parser, getVoidLogger()),
       ).resolves.toBeUndefined();
 
       expect(db.addLocationUpdateLogEvent).toHaveBeenNthCalledWith(

plugins/catalog-backend/src/database/DatabaseManager.ts

@@ -15,19 +15,28 @@
  */
 
 import Knex from 'knex';
+import lodash from 'lodash';
 import path from 'path';
 import { Logger } from 'winston';
-import { DescriptorParser, LocationReader, ParserError } from '../ingestion';
+import {
+  DescriptorEnvelope,
+  DescriptorParser,
+  LocationReader,
+  ParserError,
+} from '../ingestion';
 import { Database } from './Database';
 import { DatabaseLocationUpdateLogStatus, DbEntityRequest } from './types';
 
 export class DatabaseManager {
-  public static async createDatabase(database: Knex): Promise<Database> {
+  public static async createDatabase(
+    database: Knex,
+    logger: Logger,
+  ): Promise<Database> {
     await database.migrate.latest({
       directory: path.resolve(__dirname, 'migrations'),
       loadExtensions: ['.js'],
     });
-    return new Database(database);
+    return new Database(database, logger);
   }
 
   private static async logUpdateSuccess(
@@ -66,20 +75,25 @@ export class DatabaseManager {
     for (const location of locations) {
       try {
         logger.debug(
-          `Refreshing location ${location.id} type "${location.type}" target "${location.target}"`,
+          `Refreshing location id="${location.id}" type="${location.type}" target="${location.target}"`,
         );
 
         const readerOutput = await reader.read(location.type, location.target);
 
         for (const readerItem of readerOutput) {
           if (readerItem.type === 'error') {
-            logger.debug(readerItem.error);
+            logger.info(readerItem.error);
             continue;
           }
+
           try {
             const entity = await parser.parse(readerItem.data);
-            const dbc: DbEntityRequest = { locationId: location.id, entity };
-            await database.addOrUpdateEntity(dbc);
+            await DatabaseManager.refreshSingleEntity(
+              database,
+              location.id,
+              entity,
+              logger,
+            );
             await DatabaseManager.logUpdateSuccess(
               database,
               location.id,
@@ -98,11 +112,76 @@ export class DatabaseManager {
             );
           }
         }
-        await DatabaseManager.logUpdateSuccess(database, location.id);
+        await DatabaseManager.logUpdateSuccess(
+          database,
+          location.id,
+          undefined,
+        );
       } catch (error) {
-        logger.debug(`Failed to refresh location ${location.id}, ${error}`);
+        logger.debug(
+          `Failed to refresh location id="${location.id}", ${error}`,
+        );
         await DatabaseManager.logUpdateFailure(database, location.id, error);
       }
     }
   }
+
+  private static async refreshSingleEntity(
+    database: Database,
+    locationId: string,
+    entity: DescriptorEnvelope,
+    logger: Logger,
+  ): Promise<void> {
+    const { kind } = entity;
+    const { name, namespace } = entity.metadata || {};
+    if (!name) {
+      throw new Error('Entities without names are not yet supported');
+    }
+
+    const request: DbEntityRequest = {
+      locationId: locationId,
+      entity: entity,
+    };
+
+    logger.debug(
+      `Read entity kind="${kind}" name="${name}" namespace="${namespace}"`,
+    );
+
+    await database.transaction(async tx => {
+      const previous = await database.entity(tx, kind, name, namespace);
+      if (!previous) {
+        logger.debug(`No such entity found, adding`);
+        await database.addEntity(tx, request);
+      } else if (
+        !DatabaseManager.entitiesAreEqual(previous.entity, request.entity)
+      ) {
+        logger.debug(`Different from existing entity, updating`);
+        await database.updateEntity(tx, request);
+      } else {
+        logger.debug(`Equal to existing entity, skipping update`);
+      }
+    });
+  }
+
+  private static entitiesAreEqual(
+    first: DescriptorEnvelope,
+    second: DescriptorEnvelope,
+  ) {
+    const firstClone = lodash.cloneDeep(first);
+    const secondClone = lodash.cloneDeep(second);
+
+    // Remove generated fields
+    if (firstClone.metadata) {
+      delete firstClone.metadata.uid;
+      delete firstClone.metadata.etag;
+      delete firstClone.metadata.generation;
+    }
+    if (secondClone.metadata) {
+      delete secondClone.metadata.uid;
+      delete secondClone.metadata.etag;
+      delete secondClone.metadata.generation;
+    }
+
+    return lodash.isEqual(firstClone, secondClone);
+  }
 }

plugins/catalog-backend/src/database/migrations/20200520140700_location_update_log_table.ts

@@ -27,7 +27,7 @@ export async function up(knex: Knex): Promise<any> {
       .inTable('locations')
       .onUpdate('CASCADE')
       .onDelete('CASCADE');
-    table.string('entity_name').notNullable();
+    table.string('entity_name').nullable();
   });
 }
 

plugins/catalog-backend/src/database/search.test.ts

@@ -14,9 +14,9 @@
  * limitations under the License.
  */
 
+import { DescriptorEnvelope } from '../ingestion';
 import { buildEntitySearch, visitEntityPart } from './search';
 import { DbEntitiesSearchRow } from './types';
-import { DescriptorEnvelope } from '../ingestion';
 
 describe('search', () => {
   describe('visitEntityPart', () => {

plugins/catalog-backend/src/ingestion/LocationReaders.ts

@@ -16,8 +16,7 @@
 
 import { FileLocationSource } from './sources/FileLocationSource';
 import { GitHubLocationSource } from './sources/GitHubLocationSource';
-import { LocationSource } from './sources/types';
-import { LocationReader, ReaderOutput } from './types';
+import { LocationReader, LocationSource, ReaderOutput } from './types';
 
 export class LocationReaders implements LocationReader {
   static create(): LocationReader {

plugins/catalog-backend/src/ingestion/descriptors/ComponentDescriptorV1beta1Parser.ts

@@ -15,8 +15,7 @@
  */
 
 import * as yup from 'yup';
-import { DescriptorEnvelope, ParserError } from '../types';
-import { KindParser } from './types';
+import { DescriptorEnvelope, KindParser, ParserError } from '../types';
 
 export interface ComponentDescriptorV1beta1 extends DescriptorEnvelope {
   spec: {

plugins/catalog-backend/src/ingestion/descriptors/DescriptorEnvelopeParser.ts

@@ -89,7 +89,7 @@ export class DescriptorEnvelopeParser {
       );
 
     const labelsSchema = yup
-      .object()
+      .object<Record<string, string>>()
       .notRequired()
       .test({
         name: 'metadata.labels.keys',
@@ -113,7 +113,7 @@ export class DescriptorEnvelopeParser {
       });
 
     const annotationsSchema = yup
-      .object()
+      .object<Record<string, string>>()
       .notRequired()
       .test({
         name: 'metadata.annotations.keys',

plugins/catalog-backend/src/ingestion/descriptors/types.ts

@@ -1,38 +0,0 @@
-/*
- * Copyright 2020 Spotify AB
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import { DescriptorEnvelope } from '../types';
-
-/**
- * Parses and validates a single envelope into its materialized kind.
- *
- * These parsers may assume that the envelope is already validated and well
- * formed.
- */
-export type KindParser = {
-  /**
-   * Try to parse an envelope into a materialized kind.
-   *
-   * @param envelope A valid descriptor envelope
-   * @returns A materialized type, or undefined if the given version/kind is
-   *          not meant to be handled by this parser
-   * @throws An Error if the type was handled and found to not be properly
-   *         formatted
-   */
-  tryParse(
-    envelope: DescriptorEnvelope,
-  ): Promise<DescriptorEnvelope | undefined>;
-};

plugins/catalog-backend/src/ingestion/sources/FileLocationSource.ts

@@ -15,8 +15,7 @@
  */
 
 import fs from 'fs-extra';
-import { ReaderOutput } from '../types';
-import { LocationSource } from './types';
+import { LocationSource, ReaderOutput } from '../types';
 import { readDescriptorYaml } from './util';
 
 export class FileLocationSource implements LocationSource {

plugins/catalog-backend/src/ingestion/types.ts

@@ -18,6 +18,70 @@ import { ComponentDescriptorV1beta1 } from './descriptors/ComponentDescriptorV1b
 
 export type ComponentDescriptor = ComponentDescriptorV1beta1;
 
+/**
+ * Metadata fields common to all versions/kinds of entity.
+ *
+ * @see https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#objectmeta-v1-meta
+ */
+export type EntityMeta = {
+  /**
+   * A globally unique ID for the entity.
+   *
+   * This field can not be set by the user at creation time, and the server
+   * will reject an attempt to do so. The field will be populated in read
+   * operations. The field can (optionally) be specified when performing
+   * update or delete operations, but the server is free to reject requests
+   * that do so in such a way that it breaks semantics.
+   */
+  uid?: string;
+
+  /**
+   * An opaque string that changes for each update operation to any part of
+   * the entity, including metadata.
+   *
+   * This field can not be set by the user at creation time, and the server
+   * will reject an attempt to do so. The field will be populated in read
+   * operations. The field can (optionally) be specified when performing
+   * update or delete operations, and the server will then reject the
+   * operation if it does not match the current stored value.
+   */
+  etag?: string;
+
+  /**
+   * A positive nonzero number that indicates the current generation of data
+   * for this entity; the value is incremented each time the spec changes.
+   *
+   * This field can not be set by the user at creation time, and the server
+   * will reject an attempt to do so. The field will be populated in read
+   * operations.
+   */
+  generation?: number;
+
+  /**
+   * The name of the entity.
+   *
+   * Must be uniqe within the catalog at any given point in time, for any
+   * given namespace, for any given kind.
+   */
+  name?: string;
+
+  /**
+   * The namespace that the entity belongs to.
+   */
+  namespace?: string;
+
+  /**
+   * Key/value pairs of identifying information attached to the entity.
+   */
+  labels?: Record<string, string>;
+
+  /**
+   * Key/value pairs of non-identifying auxiliary information attached to the
+   * entity.
+   */
+  annotations?: Record<string, string>;
+};
+
 /**
  * The format envelope that's common to all versions/kinds.
  *
@@ -37,67 +101,8 @@ export type DescriptorEnvelope = {
 
   /**
    * Optional metadata related to the entity.
-   *
-   * @see https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#objectmeta-v1-meta
    */
-  metadata?: {
-    /**
-     * A globally unique ID for the entity.
-     *
-     * This field can not be set by the user at creation time, and the server
-     * will reject an attempt to do so. The field will be populated in read
-     * operations. The field can (optionally) be specified when performing
-     * update or delete operations, but the server is free to reject requests
-     * that do so in such a way that it breaks semantics.
-     */
-    uid?: string;
-
-    /**
-     * An opaque string that changes for each update operation to any part of
-     * the entity, including metadata.
-     *
-     * This field can not be set by the user at creation time, and the server
-     * will reject an attempt to do so. The field will be populated in read
-     * operations. The field can (optionally) be specified when performing
-     * update or delete operations, and the server will then reject the
-     * operation if it does not match the current stored value.
-     */
-    etag?: string;
-
-    /**
-     * A positive nonzero number that indicates the current generation of data
-     * for this entity; the value is incremented each time the spec changes.
-     *
-     * This field can not be set by the user at creation time, and the server
-     * will reject an attempt to do so. The field will be populated in read
-     * operations.
-     */
-    generation?: number;
-
-    /**
-     * The name of the entity.
-     *
-     * Must be uniqe within the catalog at any given point in time, for any
-     * given namespace, for any given kind.
-     */
-    name?: string;
-
-    /**
-     * The namespace that the entity belongs to.
-     */
-    namespace?: string;
-
-    /**
-     * Key/value pairs of identifying information attached to the entity.
-     */
-    labels?: object;
-
-    /**
-     * Key/value pairs of non-identifying auxiliary information attached to the
-     * entity.
-     */
-    annotations?: object;
-  };
+  metadata?: EntityMeta;
 
   /**
    * The specification data describing the entity itself.

plugins/catalog-backend/src/service/router.ts

@@ -38,16 +38,10 @@ export async function createRouter(
 
   if (entitiesCatalog) {
     // Entities
-    router
-      .get('/entities', async (_req, res) => {
-        const entities = await entitiesCatalog.entities();
-        res.status(200).send(entities);
-      })
-      .get('/entities/:id', async (req, res) => {
-        const { id } = req.params;
-        const entity = await entitiesCatalog.entity(id);
-        res.status(200).send(entity);
-      });
+    router.get('/entities', async (_req, res) => {
+      const entities = await entitiesCatalog.entities();
+      res.status(200).send(entities);
+    });
   }
 
   // Locations