Revert "fix(server): wrap read-modify-write apis with distributed lock (

#5979)" This reverts commit 34f892b.
toeverything · Mar 15, 2024 · bc465f9 · bc465f9
1 parent a24320d
commit bc465f9
Show file tree

Hide file tree

Showing 20 changed files with 80 additions and 413 deletions.
diff --git a/packages/backend/server/src/app.module.ts b/packages/backend/server/src/app.module.ts
@@ -28,7 +28,6 @@ import { GqlModule } from './fundamentals/graphql';
 import { HelpersModule } from './fundamentals/helpers';
 import { MailModule } from './fundamentals/mailer';
 import { MetricsModule } from './fundamentals/metrics';
-import { MutexModule } from './fundamentals/mutex';
 import { PrismaModule } from './fundamentals/prisma';
 import { StorageProviderModule } from './fundamentals/storage';
 import { RateLimiterModule } from './fundamentals/throttler';
@@ -40,7 +39,6 @@ export const FunctionalityModules = [
   ScheduleModule.forRoot(),
   EventModule,
   CacheModule,
-  MutexModule,
   PrismaModule,
   MetricsModule,
   RateLimiterModule,

diff --git a/packages/backend/server/src/core/features/feature.ts b/packages/backend/server/src/core/features/feature.ts
@@ -1,4 +1,5 @@
-import { PrismaTransaction } from '../../fundamentals';
+import { PrismaClient } from '@prisma/client';
+
 import { Feature, FeatureSchema, FeatureType } from './types';
 
 class FeatureConfig {
@@ -66,7 +67,7 @@ export type FeatureConfigType<F extends FeatureType> = InstanceType<
 
 const FeatureCache = new Map<number, FeatureConfigType<FeatureType>>();
 
-export async function getFeature(prisma: PrismaTransaction, featureId: number) {
+export async function getFeature(prisma: PrismaClient, featureId: number) {
   const cachedQuota = FeatureCache.get(featureId);
 
   if (cachedQuota) {

diff --git a/packages/backend/server/src/core/quota/quota.ts b/packages/backend/server/src/core/quota/quota.ts
@@ -1,19 +1,20 @@
-import { PrismaTransaction } from '../../fundamentals';
+import { PrismaClient } from '@prisma/client';
+
 import { formatDate, formatSize, Quota, QuotaSchema } from './types';
 
 const QuotaCache = new Map<number, QuotaConfig>();
 
 export class QuotaConfig {
   readonly config: Quota;
 
-  static async get(tx: PrismaTransaction, featureId: number) {
+  static async get(prisma: PrismaClient, featureId: number) {
     const cachedQuota = QuotaCache.get(featureId);
 
     if (cachedQuota) {
       return cachedQuota;
     }
 
-    const quota = await tx.features.findFirst({
+    const quota = await prisma.features.findFirst({
       where: {
         id: featureId,
       },

diff --git a/packages/backend/server/src/core/quota/service.ts b/packages/backend/server/src/core/quota/service.ts
@@ -1,15 +1,13 @@
 import { Injectable } from '@nestjs/common';
 import { PrismaClient } from '@prisma/client';
 
-import {
-  type EventPayload,
-  OnEvent,
-  PrismaTransaction,
-} from '../../fundamentals';
+import { type EventPayload, OnEvent } from '../../fundamentals';
 import { FeatureKind } from '../features';
 import { QuotaConfig } from './quota';
 import { QuotaType } from './types';
 
+type Transaction = Parameters<Parameters<PrismaClient['$transaction']>[0]>[0];
+
 @Injectable()
 export class QuotaService {
   constructor(private readonly prisma: PrismaClient) {}
@@ -142,8 +140,8 @@ export class QuotaService {
     });
   }
 
-  async hasQuota(userId: string, quota: QuotaType, tx?: PrismaTransaction) {
-    const executor = tx ?? this.prisma;
+  async hasQuota(userId: string, quota: QuotaType, transaction?: Transaction) {
+    const executor = transaction ?? this.prisma;
 
     return executor.userFeatures
       .count({

diff --git a/packages/backend/server/src/core/user/service.ts b/packages/backend/server/src/core/user/service.ts
@@ -54,7 +54,7 @@ export class UserService {
 
     return this.createUser({
       email,
-      name: email.split('@')[0],
+      name: 'Unnamed',
       ...data,
     });
   }

diff --git a/packages/backend/server/src/core/workspaces/resolvers/workspace.ts b/packages/backend/server/src/core/workspaces/resolvers/workspace.ts
@@ -25,9 +25,7 @@ import {
   EventEmitter,
   type FileUpload,
   MailService,
-  MutexService,
   Throttle,
-  TooManyRequestsException,
 } from '../../../fundamentals';
 import { CurrentUser, Public } from '../../auth';
 import { QuotaManagementService, QuotaQueryType } from '../../quota';
@@ -60,8 +58,7 @@ export class WorkspaceResolver {
     private readonly quota: QuotaManagementService,
     private readonly users: UserService,
     private readonly event: EventEmitter,
-    private readonly blobStorage: WorkspaceBlobStorage,
-    private readonly mutex: MutexService
+    private readonly blobStorage: WorkspaceBlobStorage
   ) {}
 
   @ResolveField(() => Permission, {
@@ -339,87 +336,74 @@ export class WorkspaceResolver {
       throw new ForbiddenException('Cannot change owner');
     }
 
-    try {
-      // lock to prevent concurrent invite
-      const lockFlag = `invite:${workspaceId}`;
-      await using lock = await this.mutex.lock(lockFlag);
-      if (!lock) {
-        return new TooManyRequestsException('Server is busy');
-      }
+    // member limit check
+    const [memberCount, quota] = await Promise.all([
+      this.prisma.workspaceUserPermission.count({
+        where: { workspaceId },
+      }),
+      this.quota.getWorkspaceUsage(workspaceId),
+    ]);
+    if (memberCount >= quota.memberLimit) {
+      throw new PayloadTooLargeException('Workspace member limit reached.');
+    }
 
-      // member limit check
-      const [memberCount, quota] = await Promise.all([
-        this.prisma.workspaceUserPermission.count({
-          where: { workspaceId },
-        }),
-        this.quota.getWorkspaceUsage(workspaceId),
-      ]);
-      if (memberCount >= quota.memberLimit) {
-        return new PayloadTooLargeException('Workspace member limit reached.');
-      }
+    let target = await this.users.findUserByEmail(email);
+    if (target) {
+      const originRecord = await this.prisma.workspaceUserPermission.findFirst({
+        where: {
+          workspaceId,
+          userId: target.id,
+        },
+      });
+      // only invite if the user is not already in the workspace
+      if (originRecord) return originRecord.id;
+    } else {
+      target = await this.users.createAnonymousUser(email, {
+        registered: false,
+      });
+    }
 
-      let target = await this.users.findUserByEmail(email);
-      if (target) {
-        const originRecord =
-          await this.prisma.workspaceUserPermission.findFirst({
-            where: {
-              workspaceId,
-              userId: target.id,
-            },
-          });
-        // only invite if the user is not already in the workspace
-        if (originRecord) return originRecord.id;
-      } else {
-        target = await this.users.createAnonymousUser(email, {
-          registered: false,
+    const inviteId = await this.permissions.grant(
+      workspaceId,
+      target.id,
+      permission
+    );
+    if (sendInviteMail) {
+      const inviteInfo = await this.getInviteInfo(inviteId);
+
+      try {
+        await this.mailer.sendInviteEmail(email, inviteId, {
+          workspace: {
+            id: inviteInfo.workspace.id,
+            name: inviteInfo.workspace.name,
+            avatar: inviteInfo.workspace.avatar,
+          },
+          user: {
+            avatar: inviteInfo.user?.avatarUrl || '',
+            name: inviteInfo.user?.name || '',
+          },
         });
-      }
-
-      const inviteId = await this.permissions.grant(
-        workspaceId,
-        target.id,
-        permission
-      );
-      if (sendInviteMail) {
-        const inviteInfo = await this.getInviteInfo(inviteId);
-
-        try {
-          await this.mailer.sendInviteEmail(email, inviteId, {
-            workspace: {
-              id: inviteInfo.workspace.id,
-              name: inviteInfo.workspace.name,
-              avatar: inviteInfo.workspace.avatar,
-            },
-            user: {
-              avatar: inviteInfo.user?.avatarUrl || '',
-              name: inviteInfo.user?.name || '',
-            },
-          });
-        } catch (e) {
-          const ret = await this.permissions.revokeWorkspace(
-            workspaceId,
-            target.id
+      } catch (e) {
+        const ret = await this.permissions.revokeWorkspace(
+          workspaceId,
+          target.id
+        );
+
+        if (!ret) {
+          this.logger.fatal(
+            `failed to send ${workspaceId} invite email to ${email} and failed to revoke permission: ${inviteId}, ${e}`
           );
-
-          if (!ret) {
-            this.logger.fatal(
-              `failed to send ${workspaceId} invite email to ${email} and failed to revoke permission: ${inviteId}, ${e}`
-            );
-          } else {
-            this.logger.warn(
-              `failed to send ${workspaceId} invite email to ${email}, but successfully revoked permission: ${e}`
-            );
-          }
-          return new InternalServerErrorException(
-            'Failed to send invite email. Please try again.'
+        } else {
+          this.logger.warn(
+            `failed to send ${workspaceId} invite email to ${email}, but successfully revoked permission: ${e}`
           );
         }
+        return new InternalServerErrorException(
+          'Failed to send invite email. Please try again.'
+        );
       }
-      return inviteId;
-    } catch (e) {
-      this.logger.error('failed to invite user', e);
-      return new TooManyRequestsException('Server is busy');
     }
+    return inviteId;
   }
 
   @Throttle({

diff --git a/packages/backend/server/src/fundamentals/error/index.ts b/packages/backend/server/src/fundamentals/error/index.ts
@@ -1,2 +1 @@
 export * from './payment-required';
-export * from './too-many-requests';
diff --git a/packages/backend/server/src/fundamentals/error/too-many-requests.ts b/packages/backend/server/src/fundamentals/error/too-many-requests.ts
diff --git a/packages/backend/server/src/fundamentals/graphql/index.ts b/packages/backend/server/src/fundamentals/graphql/index.ts
@@ -11,12 +11,6 @@ import { GraphQLError } from 'graphql';
 import { Config } from '../config';
 import { GQLLoggerPlugin } from './logger-plugin';
 
-export type GraphqlContext = {
-  req: Request;
-  res: Response;
-  isAdminQuery: boolean;
-};
-
 @Global()
 @Module({
   imports: [
@@ -36,13 +30,7 @@ export type GraphqlContext = {
               : '../../../schema.gql'
           ),
           sortSchema: true,
-          context: ({
-            req,
-            res,
-          }: {
-            req: Request;
-            res: Response;
-          }): GraphqlContext => ({
+          context: ({ req, res }: { req: Request; res: Response }) => ({
             req,
             res,
             isAdminQuery: false,

diff --git a/packages/backend/server/src/fundamentals/index.ts b/packages/backend/server/src/fundamentals/index.ts
@@ -14,23 +14,14 @@ export {
 } from './config';
 export * from './error';
 export { EventEmitter, type EventPayload, OnEvent } from './event';
-export type { GraphqlContext } from './graphql';
 export { CryptoHelper, URLHelper } from './helpers';
 export { MailService } from './mailer';
 export { CallCounter, CallTimer, metrics } from './metrics';
-export {
-  BucketService,
-  LockGuard,
-  MUTEX_RETRY,
-  MUTEX_WAIT,
-  MutexService,
-} from './mutex';
 export {
   getOptionalModuleMetadata,
   GlobalExceptionFilter,
   OptionalModule,
 } from './nestjs';
-export type { PrismaTransaction } from './prisma';
 export * from './storage';
 export { type StorageProvider, StorageProviderFactory } from './storage';
 export { AuthThrottlerGuard, CloudThrottlerGuard, Throttle } from './throttler';

diff --git a/packages/backend/server/src/fundamentals/mutex/bucket.ts b/packages/backend/server/src/fundamentals/mutex/bucket.ts
diff --git a/packages/backend/server/src/fundamentals/mutex/index.ts b/packages/backend/server/src/fundamentals/mutex/index.ts