catalog-backend: relations db review comments and more tests

Co-authored-by: blam <ben@blam.sh>

plugins/catalog-backend/migrations/20201019130742_add_relations_table.js

@@ -50,5 +50,5 @@ exports.up = async function up(knex) {
  * @param {import('knex')} knex
  */
 exports.down = async function down(knex) {
-  return knex.schema.dropTable('entities_relations');
+  await knex.schema.dropTable('entities_relations');
 };

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

@@ -31,6 +31,7 @@ describe('DatabaseEntitiesCatalog', () => {
       entityByName: jest.fn(),
       entityByUid: jest.fn(),
       removeEntityByUid: jest.fn(),
+      setRelations: jest.fn(),
       addLocation: jest.fn(),
       removeLocation: jest.fn(),
       location: jest.fn(),

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

@@ -192,13 +192,13 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
     await Promise.all(tasks);
   }
 
-  // Set the relations on the entity using the DB layer
+  // Set the relations originating from an entity using the DB layer
   async setRelations(
-    entityUid: string,
+    originatingEntityId: string,
     relations: EntityRelationSpec[],
   ): Promise<void> {
     return await this.database.transaction(tx =>
-      this.database.setRelations(tx, entityUid, relations),
+      this.database.setRelations(tx, originatingEntityId, relations),
     );
   }
 

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

@@ -553,6 +553,24 @@ describe('CommonDatabase', () => {
       };
     }
 
+    it('should not allow setting relations on nonexistent entities', async () => {
+      await expect(
+        db.transaction(async tx => {
+          await db.setRelations(tx, 'nonexistent', [
+            makeRelation('a:b/c', 'rel1', 'x:y/z'),
+          ]);
+        }),
+      ).rejects.toThrow(/constraint failed/);
+    });
+
+    it('should allow setting relations on nonexistent entities without any relations', async () => {
+      await expect(
+        db.transaction(async tx => {
+          await db.setRelations(tx, 'nonexistent', []);
+        }),
+      ).resolves.toBeUndefined();
+    });
+
     it('adds multiple relations for entities', async () => {
       const entity1 = {
         apiVersion: 'v1',
@@ -579,9 +597,11 @@ describe('CommonDatabase', () => {
         makeRelation('a:b/c', 'rel4', 'x:y/z'),
         makeRelation('a:b/c', 'rel5', 'x:y/z'),
         makeRelation('x:y/z', 'rel6', 'a:b/c'),
+        // relations don't have to reference the originating entity, so this should be fine, but not show up
+        makeRelation('g:h/i', 'rel8', 'd:e/f'),
       ];
 
-      await db.transaction(async tx => {
+      const { id2: secondEntityId } = await db.transaction(async tx => {
         const [{ entity: e1 }, { entity: e2 }] = await db.addEntities(tx, [
           { entity: entity1 },
           { entity: entity2 },
@@ -637,6 +657,30 @@ describe('CommonDatabase', () => {
           ],
         },
       ]);
+
+      await db.transaction(tx => db.removeEntityByUid(tx, secondEntityId));
+
+      const res2 = await db.transaction(tx => db.entities(tx));
+      expect(
+        res2.map(r => ({
+          name: r.entity.metadata.name,
+          relations: r.entity.relations,
+        })),
+      ).toEqual([
+        {
+          name: 'c',
+          relations: [
+            {
+              type: 'rel1',
+              target: { kind: 'x', namespace: 'y', name: 'z' },
+            },
+            {
+              type: 'rel2',
+              target: { kind: 'x', namespace: 'y', name: 'z' },
+            },
+          ],
+        },
+      ]);
     });
   });
 

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

@@ -359,6 +359,7 @@ export class CommonDatabase implements Database {
       }),
     );
 
+    // TODO(blam): translate constraint failures to sane NotFoundError instead
     await tx.batchInsert('entities_relations', relationsRows, BATCH_SIZE);
   }
 

plugins/catalog-backend/src/ingestion/HigherOrderOperations.test.ts

@@ -34,6 +34,7 @@ describe('HigherOrderOperations', () => {
       addOrUpdateEntity: jest.fn(),
       addEntities: jest.fn(),
       removeEntityByUid: jest.fn(),
+      setRelations: jest.fn(),
       batchAddOrUpdateEntities: jest.fn(),
     };
     locationsCatalog = {

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

@@ -35,6 +35,7 @@ describe('createRouter', () => {
       addOrUpdateEntity: jest.fn(),
       addEntities: jest.fn(),
       removeEntityByUid: jest.fn(),
+      setRelations: jest.fn(),
       batchAddOrUpdateEntities: jest.fn(),
     };
     locationsCatalog = {