[db, ts] Introduce a generic db.transaction() (#17980)

* [ts] Upgrade inversify 5.0.1 -> 6.0.1

* [server, db] Introduce TransactionalDB for User, Workspace, Projects

* Fix tests

Author: geropl

components/content-service-api/typescript/package.json

@@ -13,7 +13,7 @@
   "dependencies": {
     "@grpc/grpc-js": "^1.3.7",
     "google-protobuf": "^3.19.1",
-    "inversify": "^5.0.1",
+    "inversify": "^6.0.1",
     "node-pre-gyp": "^0.17.0",
     "opentracing": "^0.14.4"
   },

components/gitpod-db/src/container-module.ts

@@ -7,7 +7,7 @@
 import { ContainerModule } from "inversify";
 
 import { WorkspaceDB } from "./workspace-db";
-import { TypeORMWorkspaceDBImpl, TransactionalWorkspaceDbImpl } from "./typeorm/workspace-db-impl";
+import { TypeORMWorkspaceDBImpl } from "./typeorm/workspace-db-impl";
 import { TypeORMUserDBImpl } from "./typeorm/user-db-impl";
 import { UserDB } from "./user-db";
 import { Config } from "./config";
@@ -48,12 +48,11 @@ import { DataCache, DataCacheNoop } from "./data-cache";
 
 // THE DB container module that contains all DB implementations
 export const dbContainerModule = (cacheClass = DataCacheNoop) =>
-    new ContainerModule((bind, unbind, isBound, rebind) => {
+    new ContainerModule((bind, unbind, isBound, rebind, unbindAsync, onActivation, onDeactivation) => {
         bind(Config).toSelf().inSingletonScope();
         bind(TypeORM).toSelf().inSingletonScope();
         bind(DBWithTracing).toSelf().inSingletonScope();
         bind(DataCache).to(cacheClass).inSingletonScope();
-        bind(TransactionalWorkspaceDbImpl).toSelf().inSingletonScope();
 
         bind(TypeORMBlockedRepositoryDBImpl).toSelf().inSingletonScope();
         bind(BlockedRepositoryDB).toService(TypeORMBlockedRepositoryDBImpl);
@@ -78,7 +77,7 @@ export const dbContainerModule = (cacheClass = DataCacheNoop) =>
         bind(OneTimeSecretDB).toService(TypeORMOneTimeSecretDBImpl);
         bindDbWithTracing(TracedOneTimeSecretDB, bind, OneTimeSecretDB).inSingletonScope();
 
-        encryptionModule(bind, unbind, isBound, rebind);
+        encryptionModule(bind, unbind, isBound, rebind, unbindAsync, onActivation, onDeactivation);
         bind(KeyProviderConfig)
             .toDynamicValue((ctx) => {
                 const config = ctx.container.get<Config>(Config);

components/gitpod-db/src/project-db.ts

@@ -5,9 +5,10 @@
  */
 
 import { PartialProject, Project, ProjectEnvVar, ProjectEnvVarWithValue, ProjectUsage } from "@gitpod/gitpod-protocol";
+import { TransactionalDB } from "./typeorm/transactional-db-impl";
 
 export const ProjectDB = Symbol("ProjectDB");
-export interface ProjectDB {
+export interface ProjectDB extends TransactionalDB<ProjectDB> {
     findProjectById(projectId: string): Promise<Project | undefined>;
     findProjectByCloneUrl(cloneUrl: string): Promise<Project | undefined>;
     findProjectsByCloneUrls(cloneUrls: string[]): Promise<(Project & { teamOwners?: string[] })[]>;

components/gitpod-db/src/typeorm/project-db-impl.ts

@@ -4,9 +4,8 @@
  * See License.AGPL.txt in the project root for license information.
  */
 
-import { inject, injectable } from "inversify";
-import { TypeORM } from "./typeorm";
-import { Repository } from "typeorm";
+import { inject, injectable, optional } from "inversify";
+import { EntityManager, Repository } from "typeorm";
 import { v4 as uuidv4 } from "uuid";
 import { PartialProject, Project, ProjectEnvVar, ProjectEnvVarWithValue, ProjectUsage } from "@gitpod/gitpod-protocol";
 import { EncryptionService } from "@gitpod/gitpod-protocol/lib/encryption/encryption-service";
@@ -15,6 +14,7 @@ import { DBProject } from "./entity/db-project";
 import { DBProjectEnvVar } from "./entity/db-project-env-vars";
 import { DBProjectInfo } from "./entity/db-project-info";
 import { DBProjectUsage } from "./entity/db-project-usage";
+import { TransactionalDBImpl, UndefinedEntityManager } from "./transactional-db-impl";
 
 function toProjectEnvVar(envVarWithValue: ProjectEnvVarWithValue): ProjectEnvVar {
     const envVar = { ...envVarWithValue };
@@ -23,12 +23,16 @@ function toProjectEnvVar(envVarWithValue: ProjectEnvVarWithValue): ProjectEnvVar
 }
 
 @injectable()
-export class ProjectDBImpl implements ProjectDB {
-    @inject(TypeORM) typeORM: TypeORM;
-    @inject(EncryptionService) protected readonly encryptionService: EncryptionService;
+export class ProjectDBImpl extends TransactionalDBImpl<ProjectDB> implements ProjectDB {
+    constructor(
+        @inject(EncryptionService) protected readonly encryptionService: EncryptionService,
+        @inject(UndefinedEntityManager) @optional() transactionalEM: EntityManager | undefined,
+    ) {
+        super(transactionalEM);
+    }
 
-    protected async getEntityManager() {
-        return (await this.typeORM.getConnection()).manager;
+    protected createTransactionalDB(transactionalEM: EntityManager): ProjectDB {
+        return new ProjectDBImpl(this.encryptionService, transactionalEM);
     }
 
     protected async getRepo(): Promise<Repository<DBProject>> {

components/gitpod-db/src/typeorm/transactional-db-impl.ts

@@ -0,0 +1,44 @@
+/**
+ * Copyright (c) 2023 Gitpod GmbH. All rights reserved.
+ * Licensed under the GNU Affero General Public License (AGPL).
+ * See License.AGPL.txt in the project root for license information.
+ */
+
+import { EntityManager } from "typeorm";
+import { TypeORM } from "./typeorm";
+import { inject, injectable, optional } from "inversify";
+
+export interface TransactionalDB<DB> {
+    transaction<R>(code: (db: DB) => Promise<R>): Promise<R>;
+}
+
+// A symbol we never bind to
+export const UndefinedEntityManager = Symbol("UndefinedEntityManager");
+
+@injectable()
+export abstract class TransactionalDBImpl<DB> implements TransactionalDB<DB> {
+    @inject(TypeORM) protected readonly typeorm: TypeORM;
+
+    constructor(
+        @optional() protected transactionalEM: EntityManager | undefined, // will be undefined when constructed with inversify, which is inteded
+    ) {}
+
+    protected async getEntityManager(): Promise<EntityManager> {
+        if (this.transactionalEM) {
+            return this.transactionalEM;
+        }
+        return (await this.typeorm.getConnection()).manager;
+    }
+
+    async transaction<R>(code: (db: DB) => Promise<R>): Promise<R> {
+        const manager = await this.getEntityManager();
+        return await manager.transaction(async (manager) => {
+            return await code(this.createTransactionalDB(manager));
+        });
+    }
+
+    // TODO(gpl) This feels unnecessary because we should already be able to create a new instance using inversify.
+    // E.g. it could look like: transaction<R>(code: (db1: ..., db2...) => Promise<R>, ...serviceIdentifier: interfaces.ServiceIdentifier<DB>
+    // But bc this requires some type shennenigans, and we're not sure we need it, we'll stick with this for now
+    protected abstract createTransactionalDB(transactionalEM: EntityManager): DB;
+}

components/gitpod-db/src/typeorm/user-db-impl.ts

@@ -27,7 +27,7 @@ import {
     OAuthToken,
     OAuthUser,
 } from "@jmondi/oauth2-server";
-import { inject, injectable, postConstruct } from "inversify";
+import { inject, injectable, postConstruct, optional } from "inversify";
 import { EntityManager, Repository } from "typeorm";
 import { v4 as uuidv4 } from "uuid";
 import {
@@ -45,9 +45,9 @@ import { DBUser } from "./entity/db-user";
 import { DBUserEnvVar } from "./entity/db-user-env-vars";
 import { DBWorkspace } from "./entity/db-workspace";
 import { DBUserSshPublicKey } from "./entity/db-user-ssh-public-key";
-import { TypeORM } from "./typeorm";
 import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
 import { DataCache } from "../data-cache";
+import { TransactionalDBImpl } from "./transactional-db-impl";
 
 // OAuth token expiry
 const tokenExpiryInFuture = new DateInterval("7d");
@@ -61,19 +61,23 @@ function getUserCacheKey(id: string): string {
 }
 
 @injectable()
-export class TypeORMUserDBImpl implements UserDB {
-    @inject(TypeORM) protected readonly typeorm: TypeORM;
-    @inject(EncryptionService) protected readonly encryptionService: EncryptionService;
-    @inject(DataCache) protected readonly cache: DataCache;
+export class TypeORMUserDBImpl extends TransactionalDBImpl<UserDB> implements UserDB {
+    constructor(
+        @inject(EncryptionService) protected readonly encryptionService: EncryptionService,
+        @inject(DataCache) protected readonly cache: DataCache,
+        @optional() transactionalEM: EntityManager,
+    ) {
+        super(transactionalEM);
+    }
 
     @postConstruct()
     init() {
         /** Publish the instance of EncryptionService our entities should use */
         encryptionService = this.encryptionService;
     }
 
-    protected async getEntityManager(): Promise<EntityManager> {
-        return (await this.typeorm.getConnection()).manager;
+    protected createTransactionalDB(transactionalEM: EntityManager): UserDB {
+        return new TypeORMUserDBImpl(this.encryptionService, this.cache, transactionalEM);
     }
 
     async getUserRepo(): Promise<Repository<DBUser>> {
@@ -83,13 +87,6 @@ export class TypeORMUserDBImpl implements UserDB {
         return (await this.getEntityManager()).getRepository<DBWorkspace>(DBWorkspace);
     }
 
-    async transaction<T>(code: (db: UserDB) => Promise<T>): Promise<T> {
-        const manager = await this.getEntityManager();
-        return await manager.transaction(async (manager) => {
-            return await code(new TransactionalUserDBImpl(manager, this.cache, this.encryptionService));
-        });
-    }
-
     protected async getTokenRepo(): Promise<Repository<DBTokenEntry>> {
         return (await this.getEntityManager()).getRepository<DBTokenEntry>(DBTokenEntry);
     }
@@ -655,17 +652,3 @@ export class TypeORMUserDBImpl implements UserDB {
         return this.mapDBUserToUser(result);
     }
 }
-
-export class TransactionalUserDBImpl extends TypeORMUserDBImpl {
-    constructor(
-        protected readonly manager: EntityManager,
-        protected readonly cache: DataCache,
-        protected readonly encryptionService: EncryptionService,
-    ) {
-        super();
-    }
-
-    async getEntityManager(): Promise<EntityManager> {
-        return this.manager;
-    }
-}

components/gitpod-db/src/typeorm/workspace-db-impl.ts

@@ -5,7 +5,7 @@
  */
 
 import * as crypto from "crypto";
-import { injectable, inject } from "inversify";
+import { injectable, inject, optional } from "inversify";
 import { Repository, EntityManager, DeepPartial, UpdateQueryBuilder, Brackets } from "typeorm";
 import {
     MaybeWorkspace,
@@ -37,7 +37,6 @@ import {
     SnapshotState,
     PrebuiltWorkspaceState,
 } from "@gitpod/gitpod-protocol";
-import { TypeORM } from "./typeorm";
 import { DBWorkspace } from "./entity/db-workspace";
 import { DBWorkspaceInstance } from "./entity/db-workspace-instance";
 import { DBSnapshot } from "./entity/db-snapshot";
@@ -56,6 +55,7 @@ import {
     reportWorkspaceInstancePurged,
     reportWorkspacePurged,
 } from "./metrics";
+import { TransactionalDBImpl, UndefinedEntityManager } from "./transactional-db-impl";
 
 type RawTo<T> = (instance: WorkspaceInstance, ws: Workspace) => T;
 interface OrderBy {
@@ -64,46 +64,54 @@ interface OrderBy {
 }
 
 @injectable()
-export abstract class AbstractTypeORMWorkspaceDBImpl implements WorkspaceDB {
-    protected abstract getManager(): Promise<EntityManager>;
+export class TypeORMWorkspaceDBImpl extends TransactionalDBImpl<WorkspaceDB> implements WorkspaceDB {
+    constructor(@inject(UndefinedEntityManager) @optional() transactionalEM: EntityManager | undefined) {
+        super(transactionalEM);
+    }
+
+    protected createTransactionalDB(transactionalEM: EntityManager): WorkspaceDB {
+        return new TypeORMWorkspaceDBImpl(transactionalEM);
+    }
 
     protected async getWorkspaceRepo(): Promise<Repository<DBWorkspace>> {
-        return (await this.getManager()).getRepository<DBWorkspace>(DBWorkspace);
+        return (await this.getEntityManager()).getRepository<DBWorkspace>(DBWorkspace);
     }
 
     protected async getWorkspaceInstanceRepo(): Promise<Repository<DBWorkspaceInstance>> {
-        return (await this.getManager()).getRepository<DBWorkspaceInstance>(DBWorkspaceInstance);
+        return (await this.getEntityManager()).getRepository<DBWorkspaceInstance>(DBWorkspaceInstance);
     }
 
     protected async getWorkspaceInstanceUserRepo(): Promise<Repository<DBWorkspaceInstanceUser>> {
-        return (await this.getManager()).getRepository<DBWorkspaceInstanceUser>(DBWorkspaceInstanceUser);
+        return (await this.getEntityManager()).getRepository<DBWorkspaceInstanceUser>(DBWorkspaceInstanceUser);
     }
 
     protected async getRepositoryWhitelist(): Promise<Repository<DBRepositoryWhiteList>> {
-        return (await this.getManager()).getRepository<DBRepositoryWhiteList>(DBRepositoryWhiteList);
+        return (await this.getEntityManager()).getRepository<DBRepositoryWhiteList>(DBRepositoryWhiteList);
     }
 
     protected async getSnapshotRepo(): Promise<Repository<DBSnapshot>> {
-        return (await this.getManager()).getRepository<DBSnapshot>(DBSnapshot);
+        return (await this.getEntityManager()).getRepository<DBSnapshot>(DBSnapshot);
     }
 
     protected async getPrebuiltWorkspaceRepo(): Promise<Repository<DBPrebuiltWorkspace>> {
-        return (await this.getManager()).getRepository<DBPrebuiltWorkspace>(DBPrebuiltWorkspace);
+        return (await this.getEntityManager()).getRepository<DBPrebuiltWorkspace>(DBPrebuiltWorkspace);
     }
 
     protected async getPrebuildInfoRepo(): Promise<Repository<DBPrebuildInfo>> {
-        return (await this.getManager()).getRepository<DBPrebuildInfo>(DBPrebuildInfo);
+        return (await this.getEntityManager()).getRepository<DBPrebuildInfo>(DBPrebuildInfo);
     }
 
     protected async getPrebuiltWorkspaceUpdatableRepo(): Promise<Repository<DBPrebuiltWorkspaceUpdatable>> {
-        return (await this.getManager()).getRepository<DBPrebuiltWorkspaceUpdatable>(DBPrebuiltWorkspaceUpdatable);
+        return (await this.getEntityManager()).getRepository<DBPrebuiltWorkspaceUpdatable>(
+            DBPrebuiltWorkspaceUpdatable,
+        );
     }
 
     public async connect(maxTries: number = 3, timeout: number = 2000): Promise<void> {
         let tries = 1;
         while (tries <= maxTries) {
             try {
-                await this.getManager();
+                await this.getEntityManager();
                 return;
             } catch (err) {
                 log.error(`DB connection error (attempt ${tries} of ${maxTries})`, err);
@@ -114,10 +122,6 @@ export abstract class AbstractTypeORMWorkspaceDBImpl implements WorkspaceDB {
         throw new Error("Could not establish connection to database!");
     }
 
-    public async transaction<T>(code: (db: WorkspaceDB) => Promise<T>): Promise<T> {
-        return code(this);
-    }
-
     async storeInstance(instance: WorkspaceInstance): Promise<WorkspaceInstance> {
         const inst = await this.internalStoreInstance(instance);
         return inst;
@@ -1197,34 +1201,4 @@ export abstract class AbstractTypeORMWorkspaceDBImpl implements WorkspaceDB {
     }
 }
 
-@injectable()
-export class TypeORMWorkspaceDBImpl extends AbstractTypeORMWorkspaceDBImpl {
-    @inject(TypeORM) protected readonly typeorm: TypeORM;
-
-    protected async getManager() {
-        return (await this.typeorm.getConnection()).manager;
-    }
-
-    public async transaction<T>(code: (db: WorkspaceDB) => Promise<T>): Promise<T> {
-        const connection = await this.typeorm.getConnection();
-        return connection.transaction((manager) => {
-            return code(new TransactionalWorkspaceDbImpl(manager));
-        });
-    }
-}
-
-export class TransactionalWorkspaceDbImpl extends AbstractTypeORMWorkspaceDBImpl {
-    constructor(protected readonly manager: EntityManager) {
-        super();
-    }
-
-    protected async getManager() {
-        return this.manager;
-    }
-
-    public async transaction<T>(code: (sb: WorkspaceDB) => Promise<T>): Promise<T> {
-        return await code(this);
-    }
-}
-
 type InstanceJoinResult = DBWorkspace & { instance: WorkspaceInstance };

components/gitpod-db/src/user-db.ts

@@ -19,13 +19,12 @@ import {
 import { OAuthTokenRepository, OAuthUserRepository } from "@jmondi/oauth2-server";
 import { Repository } from "typeorm";
 import { DBUser } from "./typeorm/entity/db-user";
+import { TransactionalDB } from "./typeorm/transactional-db-impl";
 
 export type MaybeUser = User | undefined;
 
 export const UserDB = Symbol("UserDB");
-export interface UserDB extends OAuthUserRepository, OAuthTokenRepository {
-    transaction<T>(code: (db: UserDB) => Promise<T>): Promise<T>;
-
+export interface UserDB extends OAuthUserRepository, OAuthTokenRepository, TransactionalDB<UserDB> {
     newUser(): Promise<User>;
     storeUser(newUser: User): Promise<User>;
     updateUserPartial(partial: PartialUserUpdate): Promise<void>;

components/gitpod-protocol/package.json

@@ -50,7 +50,7 @@
         "cookie": "^0.4.2",
         "express": "^4.17.3",
         "google-protobuf": "^3.19.1",
-        "inversify": "^5.1.1",
+        "inversify": "^6.0.1",
         "jaeger-client": "^3.18.1",
         "js-yaml": "^3.10.0",
         "nice-grpc-common": "^2.0.0",

components/image-builder-api/typescript/package.json

@@ -15,7 +15,7 @@
     "@gitpod/gitpod-protocol": "0.1.5",
     "@grpc/grpc-js": "^1.3.7",
     "google-protobuf": "^3.19.1",
-    "inversify": "^5.0.1",
+    "inversify": "^6.0.1",
     "opentracing": "^0.14.4"
   },
   "devDependencies": {