Refactor LocationStore to transaction from context

Co-authored-by: Ben Lambert <ben@blam.sh>
Signed-off-by: Johan Haals <johan.haals@gmail.com>

plugins/catalog-backend/src/next/DefaultLocationStore.test.ts

@@ -14,17 +14,25 @@
  * limitations under the License.
  */
 import { v4 as uuid } from 'uuid';
+import { BackgroundContext, Context, TransactionValue } from './Context';
 import { DatabaseManager } from './database/DatabaseManager';
 import { DefaultLocationStore } from './DefaultLocationStore';
 
-/* eslint-disable */
-xdescribe('DefaultLocationStore', () => {
+describe('DefaultLocationStore', () => {
   const createLocationStore = async () => {
-    const db = await DatabaseManager.createTestDatabase();
+    const knex = await DatabaseManager.createTestDatabaseConnection();
+    const db = await DatabaseManager.createDatabase(knex);
     const connection = { applyMutation: jest.fn() };
-    const store = new DefaultLocationStore(db);
+    const store = new DefaultLocationStore(knex);
     await store.connect(connection);
-    return { store, connection };
+
+    const withContext = async (handler: (ctx: Context) => Promise<any>) => {
+      return await db.transaction(async tx => {
+        const ctx = TransactionValue.in(new BackgroundContext(), tx as any);
+        return await handler(ctx);
+      });
+    };
+    return { store, connection, db, withContext };
   };
 
   it('should do a full sync with the locations on connect', async () => {
@@ -38,109 +46,128 @@ xdescribe('DefaultLocationStore', () => {
 
   describe('listLocations', () => {
     it('lists empty locations when there is no locations', async () => {
-      const { store } = await createLocationStore();
+      expect.assertions(1);
 
-      expect(await store.listLocations()).toEqual([]);
+      const { store, withContext } = await createLocationStore();
+      await withContext(async ctx => {
+        expect(await store.listLocations(ctx)).toEqual([]);
+      });
     });
 
     it('lists locations that are added to the db', async () => {
-      const { store } = await createLocationStore();
+      expect.assertions(2);
 
-      await store.createLocation({
-        target:
-          'https://github.com/backstage/demo/blob/master/catalog-info.yml',
-        type: 'url',
+      const { store, withContext } = await createLocationStore();
+
+      await withContext(async ctx => {
+        await store.createLocation(ctx, {
+          target:
+            'https://github.com/backstage/demo/blob/master/catalog-info.yml',
+          type: 'url',
+        });
+
+        const listLocations = await store.listLocations(ctx);
+        expect(listLocations).toHaveLength(1);
+        expect(listLocations).toEqual(
+          expect.arrayContaining([
+            expect.objectContaining({
+              target:
+                'https://github.com/backstage/demo/blob/master/catalog-info.yml',
+              type: 'url',
+            }),
+          ]),
+        );
       });
-
-      const listLocations = await store.listLocations();
-
-      expect(listLocations).toHaveLength(1);
-      expect(listLocations).toEqual(
-        expect.arrayContaining([
-          expect.objectContaining({
-            target:
-              'https://github.com/backstage/demo/blob/master/catalog-info.yml',
-            type: 'url',
-          }),
-        ]),
-      );
     });
   });
 
   describe('createLocation', () => {
     it('throws when the location already exists', async () => {
-      const { store } = await createLocationStore();
+      expect.assertions(1);
+
+      const { store, withContext } = await createLocationStore();
       const spec = {
         target:
           'https://github.com/backstage/demo/blob/master/catalog-info.yml',
         type: 'url',
       };
-      await store.createLocation(spec);
+      await withContext(async ctx => {
+        await store.createLocation(ctx, spec);
 
-      await expect(() => store.createLocation(spec)).rejects.toThrow(
-        new RegExp(`Location ${spec.type}:${spec.target} already exists`),
-      );
+        await expect(() => store.createLocation(ctx, spec)).rejects.toThrow(
+          new RegExp(`Location ${spec.type}:${spec.target} already exists`),
+        );
+      });
     });
 
     it('calls apply mutation when adding a new location', async () => {
-      const { store, connection } = await createLocationStore();
+      expect.assertions(1);
 
-      await store.createLocation({
-        target:
-          'https://github.com/backstage/demo/blob/master/catalog-info.yml',
-        type: 'url',
-      });
+      const { store, connection, withContext } = await createLocationStore();
 
-      expect(connection.applyMutation).toHaveBeenCalledWith({
-        type: 'delta',
-        removed: [],
-        added: expect.arrayContaining([
-          expect.objectContaining({
-            spec: {
-              target:
-                'https://github.com/backstage/demo/blob/master/catalog-info.yml',
-              type: 'url',
-            },
-          }),
-        ]),
+      await withContext(async ctx => {
+        await store.createLocation(ctx, {
+          target:
+            'https://github.com/backstage/demo/blob/master/catalog-info.yml',
+          type: 'url',
+        });
+
+        expect(connection.applyMutation).toHaveBeenCalledWith({
+          type: 'delta',
+          removed: [],
+          added: expect.arrayContaining([
+            expect.objectContaining({
+              spec: {
+                target:
+                  'https://github.com/backstage/demo/blob/master/catalog-info.yml',
+                type: 'url',
+              },
+            }),
+          ]),
+        });
       });
     });
   });
 
   describe('deleteLocation', () => {
     it('throws if the location does not exist', async () => {
-      const { store } = await createLocationStore();
-
-      const id = uuid();
-
-      await expect(() => store.deleteLocation(id)).rejects.toThrow(
-        new RegExp(`Found no location with ID ${id}`),
-      );
+      expect.assertions(1);
+      const { store, withContext } = await createLocationStore();
+      await withContext(async ctx => {
+        const id = uuid();
+        await expect(() => store.deleteLocation(ctx, id)).rejects.toThrow(
+          new RegExp(`Found no location with ID ${id}`),
+        );
+      });
     });
 
     it('calls apply mutation when adding a new location', async () => {
-      const { store, connection } = await createLocationStore();
+      expect.assertions(1);
 
-      const location = await store.createLocation({
-        target:
-          'https://github.com/backstage/demo/blob/master/catalog-info.yml',
-        type: 'url',
-      });
+      const { store, connection, withContext } = await createLocationStore();
 
-      await store.deleteLocation(location.id);
+      await withContext(async ctx => {
+        const location = await store.createLocation(ctx, {
+          target:
+            'https://github.com/backstage/demo/blob/master/catalog-info.yml',
+          type: 'url',
+        });
 
-      expect(connection.applyMutation).toHaveBeenCalledWith({
-        type: 'delta',
-        added: [],
-        removed: expect.arrayContaining([
-          expect.objectContaining({
-            spec: {
-              target:
-                'https://github.com/backstage/demo/blob/master/catalog-info.yml',
-              type: 'url',
-            },
-          }),
-        ]),
+        await store.deleteLocation(ctx, location.id);
+
+        expect(connection.applyMutation).toHaveBeenCalledWith({
+          type: 'delta',
+          added: [],
+          removed: expect.arrayContaining([
+            expect.objectContaining({
+              spec: {
+                target:
+                  'https://github.com/backstage/demo/blob/master/catalog-info.yml',
+                type: 'url',
+              },
+            }),
+          ]),
+        });
       });
     });
   });

plugins/catalog-backend/src/next/DefaultLocationStore.ts

@@ -15,7 +15,6 @@
  */
 
 import { LocationSpec, Location } from '@backstage/catalog-model';
-import { Database } from '../database';
 import {
   LocationStore,
   EntityProvider,
@@ -23,52 +22,61 @@ import {
 } from './types';
 import { v4 as uuid } from 'uuid';
 import { locationSpecToLocationEntity } from './util';
-import { ConflictError } from '@backstage/errors';
+import { ConflictError, NotFoundError } from '@backstage/errors';
+import { BackgroundContext, Context, TransactionValue } from './Context';
+import { Knex } from 'knex';
+
+type DbLocationsRow = {
+  id: string;
+  type: string;
+  target: string;
+};
 
 export class DefaultLocationStore implements LocationStore, EntityProvider {
   private _connection: EntityProviderConnection | undefined;
 
-  constructor(private readonly db: Database) {}
+  constructor(private readonly db: Knex) {}
 
   getProviderName(): string {
     return 'DefaultLocationStore';
   }
 
-  async createLocation(spec: LocationSpec): Promise<Location> {
-    return this.db.transaction(async tx => {
-      // Attempt to find a previous location matching the spec
-      const previousLocations = await this.listLocations();
-      const previousLocation = previousLocations.some(
-        l => spec.type === l.type && spec.target === l.target,
+  async createLocation(ctx: Context, spec: LocationSpec): Promise<Location> {
+    const tx = TransactionValue.from(ctx);
+
+    // Attempt to find a previous location matching the spec
+    const previousLocations = await this.listLocations(ctx);
+    const previousLocation = previousLocations.some(
+      l => spec.type === l.type && spec.target === l.target,
+    );
+
+    if (previousLocation) {
+      throw new ConflictError(
+        `Location ${spec.type}:${spec.target} already exists`,
       );
+    }
 
-      if (previousLocation) {
-        throw new ConflictError(
-          `Location ${spec.type}:${spec.target} already exists`,
-        );
-      }
+    const location: DbLocationsRow = {
+      id: uuid(),
+      type: spec.type,
+      target: spec.target,
+    };
+    await tx<DbLocationsRow>('locations').insert(location);
 
-      // TODO: id should really be type and target combined and not a uuid.
-      const location = await this.db.addLocation(tx, {
-        id: uuid(),
-        type: spec.type,
-        target: spec.target,
-      });
-
-      await this.connection.applyMutation({
-        type: 'delta',
-        added: [locationSpecToLocationEntity(location)],
-        removed: [],
-      });
-
-      return location;
+    await this.connection.applyMutation({
+      type: 'delta',
+      added: [locationSpecToLocationEntity(location)],
+      removed: [],
     });
+
+    return location;
   }
 
-  async listLocations(): Promise<Location[]> {
-    const dbLocations = await this.db.locations();
+  async listLocations(ctx: Context): Promise<Location[]> {
+    const tx = TransactionValue.from(ctx);
+    const locations = await tx<DbLocationsRow>('locations').select();
     return (
-      dbLocations
+      locations
         // TODO(blam): We should create a mutation to remove this location for everyone
         // eventually when it's all done and dusted
         .filter(({ type }) => type !== 'bootstrap')
@@ -80,23 +88,38 @@ export class DefaultLocationStore implements LocationStore, EntityProvider {
     );
   }
 
-  getLocation(id: string): Promise<Location> {
-    return this.db.location(id);
+  async getLocation(ctx: Context, id: string): Promise<Location> {
+    const tx = TransactionValue.from(ctx);
+    const items = await tx<DbLocationsRow>('locations').where({ id }).select();
+
+    if (!items.length) {
+      throw new NotFoundError(`Found no location with ID ${id}`);
+    }
+    return items[0];
   }
 
-  deleteLocation(id: string): Promise<void> {
+  async deleteLocation(ctx: Context, id: string): Promise<void> {
     if (!this.connection) {
       throw new Error('location store is not initialized');
     }
 
-    return this.db.transaction(async tx => {
-      const location = await this.db.location(id);
-      await this.db.removeLocation(tx, id);
-      await this.connection.applyMutation({
-        type: 'delta',
-        added: [],
-        removed: [locationSpecToLocationEntity(location)],
-      });
+    const tx = TransactionValue.from(ctx);
+    const location = await this.getLocation(ctx, id);
+
+    const locations = await tx<DbLocationsRow>('locations')
+      .where({ id })
+      .select();
+
+    if (!locations.length) {
+      throw new NotFoundError(`Found no location with ID ${id}`);
+    }
+
+    await tx<DbLocationsRow>('locations').where({ id }).del();
+
+    await this.connection.applyMutation({
+      type: 'delta',
+      added: [],
+      removed: [locationSpecToLocationEntity(location)],
     });
   }
 
@@ -110,10 +133,18 @@ export class DefaultLocationStore implements LocationStore, EntityProvider {
 
   async connect(connection: EntityProviderConnection): Promise<void> {
     this._connection = connection;
-    const locations = await this.listLocations();
+
+    // TODO: this feels a little weird creating the ctx in multiple places
+    // Let's work out a better way, maybe.
+    const locations = await this.db.transaction(tx => {
+      const ctx = TransactionValue.in(new BackgroundContext(), tx);
+      return this.listLocations(ctx);
+    });
+
     const entities = locations.map(location => {
       return locationSpecToLocationEntity(location);
     });
+
     await this.connection.applyMutation({
       type: 'full',
       entities,

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

@@ -21,6 +21,7 @@ import {
   EntityRelationSpec,
 } from '@backstage/catalog-model';
 import { JsonObject } from '@backstage/config';
+import { Context } from './Context';
 
 export interface LocationService {
   createLocation(
@@ -33,10 +34,10 @@ export interface LocationService {
 }
 
 export interface LocationStore {
-  createLocation(spec: LocationSpec): Promise<Location>;
-  listLocations(): Promise<Location[]>;
-  getLocation(id: string): Promise<Location>;
-  deleteLocation(id: string): Promise<void>;
+  createLocation(ctx: Context, spec: LocationSpec): Promise<Location>;
+  listLocations(ctx: Context): Promise<Location[]>;
+  getLocation(ctx: Context, id: string): Promise<Location>;
+  deleteLocation(ctx: Context, id: string): Promise<void>;
 }
 
 export interface CatalogProcessingEngine {