diff --git a/.env.example b/.env.example index 4156c5e7..66196da8 100644 --- a/.env.example +++ b/.env.example @@ -3,7 +3,7 @@ DATABASE_URL=postgresql://postgres:testpass@hoppscotch-db:5432/hoppscotch # Sensitive Data Encryption Key while storing in Database (32 character) -DATA_ENCRYPTION_KEY="data encryption key with 32 char" +DATA_ENCRYPTION_KEY=data encryption key with 32 char # Whitelisted origins for the Hoppscotch App. # This list controls which origins can interact with the app through cross-origin comms. @@ -12,7 +12,7 @@ DATA_ENCRYPTION_KEY="data encryption key with 32 char" # NOTE: `3200` here refers to the bundle server (port 3200) that provides the bundles, # NOT where the app runs. The app itself uses the `app://` protocol with dynamic # bundle names like `app://{bundle-name}/` -WHITELISTED_ORIGINS="http://localhost:3170,http://localhost:3000,http://localhost:3100,app://localhost_3200,app://hoppscotch" +WHITELISTED_ORIGINS=http://localhost:3170,http://localhost:3000,http://localhost:3100,app://localhost_3200,app://hoppscotch # If true, the client’s IP address is understood as the left-most entry in the X-Forwarded-For header TRUST_PROXY=false diff --git a/packages/hoppscotch-backend/prisma/migrations/20250818154928_fix_order_index/migration.sql b/packages/hoppscotch-backend/prisma/migrations/20250818154928_fix_order_index/migration.sql new file mode 100644 index 00000000..715e6067 --- /dev/null +++ b/packages/hoppscotch-backend/prisma/migrations/20250818154928_fix_order_index/migration.sql @@ -0,0 +1,59 @@ +-- Recalculate orderIndex for UserCollection per (parentID) +WITH ordered AS ( + SELECT + id, + ROW_NUMBER() OVER ( + PARTITION BY "parentID" + ORDER BY "orderIndex" ASC, "createdOn" ASC, id ASC + ) AS new_index + FROM "UserCollection" +) +UPDATE "UserCollection" uc +SET "orderIndex" = o.new_index +FROM ordered o +WHERE uc.id = o.id; + +-- Recalculate orderIndex for UserRequest per (collectionID) +WITH ordered AS ( + SELECT + id, + ROW_NUMBER() OVER ( + PARTITION BY "collectionID" + ORDER BY "orderIndex" ASC, "createdOn" ASC, id ASC + ) AS new_index + FROM "UserRequest" +) +UPDATE "UserRequest" ur +SET "orderIndex" = o.new_index +FROM ordered o +WHERE ur.id = o.id; + +-- Recalculate orderIndex for TeamCollection per (teamID, parentID) +WITH ordered AS ( + SELECT + id, + ROW_NUMBER() OVER ( + PARTITION BY "teamID", "parentID" + ORDER BY "orderIndex" ASC, "createdOn" ASC, id ASC + ) AS new_index + FROM "TeamCollection" +) +UPDATE "TeamCollection" tc +SET "orderIndex" = o.new_index +FROM ordered o +WHERE tc.id = o.id; + +-- Recalculate orderIndex for TeamRequest per (teamID, collectionID) +WITH ordered AS ( + SELECT + id, + ROW_NUMBER() OVER ( + PARTITION BY "teamID", "collectionID" + ORDER BY "orderIndex" ASC, "createdOn" ASC, id ASC + ) AS new_index + FROM "TeamRequest" +) +UPDATE "TeamRequest" tr +SET "orderIndex" = o.new_index +FROM ordered o +WHERE tr.id = o.id; diff --git a/packages/hoppscotch-backend/prisma/migrations/20250822044741_order_index_unique_constraint/migration.sql b/packages/hoppscotch-backend/prisma/migrations/20250822044741_order_index_unique_constraint/migration.sql new file mode 100644 index 00000000..8f3f7565 --- /dev/null +++ b/packages/hoppscotch-backend/prisma/migrations/20250822044741_order_index_unique_constraint/migration.sql @@ -0,0 +1,21 @@ +-- Recreate as DEFERRABLE UNIQUE CONSTRAINTS + +ALTER TABLE "TeamCollection" +ADD CONSTRAINT "TeamCollection_teamID_parentID_orderIndex_key" + UNIQUE ("teamID", "parentID", "orderIndex") + DEFERRABLE INITIALLY DEFERRED; + +ALTER TABLE "TeamRequest" +ADD CONSTRAINT "TeamRequest_teamID_collectionID_orderIndex_key" + UNIQUE ("teamID", "collectionID", "orderIndex") + DEFERRABLE INITIALLY DEFERRED; + +ALTER TABLE "UserCollection" +ADD CONSTRAINT "UserCollection_userUid_parentID_orderIndex_key" + UNIQUE ("userUid", "parentID", "orderIndex") + DEFERRABLE INITIALLY DEFERRED; + +ALTER TABLE "UserRequest" +ADD CONSTRAINT "UserRequest_userUid_collectionID_orderIndex_key" + UNIQUE ("userUid", "collectionID", "orderIndex") + DEFERRABLE INITIALLY DEFERRED; diff --git a/packages/hoppscotch-backend/prisma/schema.prisma b/packages/hoppscotch-backend/prisma/schema.prisma index 6a29be10..dfc8df01 100644 --- a/packages/hoppscotch-backend/prisma/schema.prisma +++ b/packages/hoppscotch-backend/prisma/schema.prisma @@ -53,6 +53,8 @@ model TeamCollection { orderIndex Int createdOn DateTime @default(now()) @db.Timestamptz(3) updatedOn DateTime @updatedAt @db.Timestamptz(3) + + @@unique([teamID, parentID, orderIndex]) } model TeamRequest { @@ -66,6 +68,8 @@ model TeamRequest { orderIndex Int createdOn DateTime @default(now()) @db.Timestamptz(3) updatedOn DateTime @updatedAt @db.Timestamptz(3) + + @@unique([teamID, collectionID, orderIndex]) } model Shortcode { @@ -189,6 +193,8 @@ model UserRequest { orderIndex Int createdOn DateTime @default(now()) @db.Timestamptz(3) updatedOn DateTime @updatedAt @db.Timestamptz(3) + + @@unique([userUid, collectionID, orderIndex]) } model UserCollection { @@ -205,6 +211,8 @@ model UserCollection { type ReqType createdOn DateTime @default(now()) @db.Timestamptz(3) updatedOn DateTime @updatedAt @db.Timestamptz(3) + + @@unique([userUid, parentID, orderIndex]) } enum TeamAccessRole { diff --git a/packages/hoppscotch-backend/src/errors.ts b/packages/hoppscotch-backend/src/errors.ts index e06f85e4..435cc42a 100644 --- a/packages/hoppscotch-backend/src/errors.ts +++ b/packages/hoppscotch-backend/src/errors.ts @@ -188,6 +188,12 @@ export const TEAM_FB_COLL_PATH_RESOLVE_FAIL = 'team/fb_coll_path_resolve_fail'; */ export const TEAM_COLL_NOT_FOUND = 'team_coll/collection_not_found'; +/** + * Could not find the team in the database + * (TeamCollectionService) + */ +export const TEAM_COLL_CREATION_FAILED = 'team_coll/creation_failed'; + /** * Cannot make parent collection a child of a collection that a child of itself * (TeamCollectionService) @@ -650,6 +656,13 @@ export const USER_COLL_NOT_SAME_TYPE = 'user_coll/type_mismatch' as const; export const USER_COLL_IS_PARENT_COLL = 'user_coll/user_collection_is_parent_coll' as const; +/** + * User Collection Creation Failed + * (UserCollectionService) + */ +export const USER_COLLECTION_CREATION_FAILED = + 'user_collection/creation_failed' as const; + /** * User Collection Re-Ordering Failed * (UserCollectionService) diff --git a/packages/hoppscotch-backend/src/infra-config/helper.ts b/packages/hoppscotch-backend/src/infra-config/helper.ts index ce0b05d0..c07b696f 100644 --- a/packages/hoppscotch-backend/src/infra-config/helper.ts +++ b/packages/hoppscotch-backend/src/infra-config/helper.ts @@ -87,7 +87,7 @@ export async function loadInfraConfiguration() { // Prisma throw error if 'Can't reach at database server' OR 'Table does not exist' // Reason for not throwing error is, we want successful build during 'postinstall' and generate dist files - console.log('Error from loadInfraConfiguration', error); + console.error('Error from loadInfraConfiguration', error); return { INFRA: {} }; } } diff --git a/packages/hoppscotch-backend/src/infra-config/infra-config.service.ts b/packages/hoppscotch-backend/src/infra-config/infra-config.service.ts index 52dca33d..5a78bb33 100644 --- a/packages/hoppscotch-backend/src/infra-config/infra-config.service.ts +++ b/packages/hoppscotch-backend/src/infra-config/infra-config.service.ts @@ -42,6 +42,7 @@ import { SaveOnboardingConfigResponse, } from './dto/onboarding.dto'; import * as crypto from 'crypto'; +import { PrismaError } from 'src/prisma/prisma-error-codes'; @Injectable() export class InfraConfigService implements OnModuleInit { @@ -125,14 +126,14 @@ export class InfraConfigService implements OnModuleInit { stopApp(); } } catch (error) { - if (error.code === 'P1001') { + if (error.code === PrismaError.DATABASE_UNREACHABLE) { // Prisma error code for 'Can't reach at database server' // We're not throwing error here because we want to allow the app to run 'pnpm install' - } else if (error.code === 'P2021') { + } else if (error.code === PrismaError.TABLE_DOES_NOT_EXIST) { // Prisma error code for 'Table does not exist' throwErr(DATABASE_TABLE_NOT_EXIST); } else { - console.log(error); + console.error(error); throwErr(error); } } diff --git a/packages/hoppscotch-backend/src/mailer/mailer.service.ts b/packages/hoppscotch-backend/src/mailer/mailer.service.ts index ab2abd1f..89eaa692 100644 --- a/packages/hoppscotch-backend/src/mailer/mailer.service.ts +++ b/packages/hoppscotch-backend/src/mailer/mailer.service.ts @@ -56,7 +56,7 @@ export class MailerService { context: mailDesc.variables, }); } catch (error) { - console.log('Error from sendEmail:', error); + console.error('Error from sendEmail:', error); return throwErr(EMAIL_FAILED); } } @@ -82,7 +82,7 @@ export class MailerService { }); return res; } catch (error) { - console.log('Error from sendUserInvitationEmail:', error); + console.error('Error from sendUserInvitationEmail:', error); return throwErr(EMAIL_FAILED); } } diff --git a/packages/hoppscotch-backend/src/main.ts b/packages/hoppscotch-backend/src/main.ts index 09ec0655..6b2b056c 100644 --- a/packages/hoppscotch-backend/src/main.ts +++ b/packages/hoppscotch-backend/src/main.ts @@ -88,7 +88,6 @@ async function bootstrap() { if (configService.get('TRUST_PROXY') === 'true') { console.log('Enabling trust proxy'); - app.set('trust proxy', true); } diff --git a/packages/hoppscotch-backend/src/prisma/prisma-error-codes.ts b/packages/hoppscotch-backend/src/prisma/prisma-error-codes.ts new file mode 100644 index 00000000..0ae4819c --- /dev/null +++ b/packages/hoppscotch-backend/src/prisma/prisma-error-codes.ts @@ -0,0 +1,7 @@ +export enum PrismaError { + DATABASE_UNREACHABLE = 'P1001', + TABLE_DOES_NOT_EXIST = 'P2021', + UNIQUE_CONSTRAINT_VIOLATION = 'P2002', + TRANSACTION_TIMEOUT = 'P2028', + TRANSACTION_DEADLOCK = 'P2034', // write conflict or a deadlock +} diff --git a/packages/hoppscotch-backend/src/prisma/prisma.service.ts b/packages/hoppscotch-backend/src/prisma/prisma.service.ts index 8febf1b5..029e59eb 100644 --- a/packages/hoppscotch-backend/src/prisma/prisma.service.ts +++ b/packages/hoppscotch-backend/src/prisma/prisma.service.ts @@ -1,5 +1,5 @@ import { Injectable, OnModuleInit, OnModuleDestroy } from '@nestjs/common'; -import { PrismaClient } from '@prisma/client'; +import { PrismaClient, Prisma } from '@prisma/client'; @Injectable() export class PrismaService @@ -7,7 +7,12 @@ export class PrismaService implements OnModuleInit, OnModuleDestroy { constructor() { - super(); + super({ + transactionOptions: { + maxWait: 5000, // 5 seconds + timeout: 10000, // 10 seconds + }, + }); } async onModuleInit() { await this.$connect(); @@ -16,4 +21,63 @@ export class PrismaService async onModuleDestroy() { await this.$disconnect(); } + + /** + * Centralized Lock Manager + * Locks UserCollections first, then UserRequests and same goes for TeamCollections and then TeamRequests + * + * For UserCollection -> userUid, parentID (nullable) + * For UserRequest -> collectionIDs + * For TeamCollection -> parentID (nullable) + * For TeamRequest -> collectionIDs + */ + async acquireLocks( + tx: Prisma.TransactionClient, + table: 'UserCollection' | 'UserRequest' | 'TeamCollection' | 'TeamRequest', + userUid: string, + parentID: string | null, + collectionIDs: string[] = [], + ) { + if (table === 'UserCollection' && userUid) { + const lockQuery = parentID + ? Prisma.sql`SELECT "orderIndex" FROM "UserCollection" WHERE "userUid" = ${userUid} AND "parentID" = ${parentID} FOR UPDATE` + : Prisma.sql`SELECT "orderIndex" FROM "UserCollection" WHERE "userUid" = ${userUid} AND "parentID" IS NULL FOR UPDATE`; + return tx.$executeRaw(lockQuery); + } + + if (table === 'UserRequest' && collectionIDs.length > 0) { + collectionIDs = collectionIDs.filter(Boolean).sort(); + const lockQuery = Prisma.sql`SELECT "orderIndex" FROM "UserRequest" WHERE "collectionID" IN (${Prisma.join(collectionIDs)}) FOR UPDATE`; + return tx.$executeRaw(lockQuery); + } + + if (table === 'TeamCollection') { + const lockQuery = parentID + ? Prisma.sql`SELECT "orderIndex" FROM "TeamCollection" WHERE "parentID" = ${parentID} FOR UPDATE` + : Prisma.sql`SELECT "orderIndex" FROM "TeamCollection" WHERE "parentID" IS NULL FOR UPDATE`; + return tx.$executeRaw(lockQuery); + } + + if (table === 'TeamRequest' && collectionIDs.length > 0) { + collectionIDs = collectionIDs.filter(Boolean).sort(); + const lockQuery = Prisma.sql`SELECT "orderIndex" FROM "TeamRequest" WHERE "collectionID" IN (${Prisma.join(collectionIDs)}) FOR UPDATE`; + return tx.$executeRaw(lockQuery); + } + } + + /** + * Table-level lock + */ + async lockTableExclusive( + tx: Prisma.TransactionClient, + tableName: + | 'UserCollection' + | 'UserRequest' + | 'TeamCollection' + | 'TeamRequest', + ) { + return tx.$executeRaw( + Prisma.sql`LOCK TABLE ${Prisma.raw(`"${tableName}"`)} IN EXCLUSIVE MODE`, + ); + } } diff --git a/packages/hoppscotch-backend/src/team-collection/input-type.args.ts b/packages/hoppscotch-backend/src/team-collection/input-type.args.ts index 6e999f18..24e70c50 100644 --- a/packages/hoppscotch-backend/src/team-collection/input-type.args.ts +++ b/packages/hoppscotch-backend/src/team-collection/input-type.args.ts @@ -90,29 +90,6 @@ export class UpdateTeamCollectionOrderArgs { destCollID: string; } -@ArgsType() -export class ReplaceTeamCollectionArgs { - @Field(() => ID, { - name: 'teamID', - description: 'Id of the team to add to', - }) - teamID: string; - - @Field({ - name: 'jsonString', - description: 'JSON string to replace with', - }) - jsonString: string; - - @Field(() => ID, { - name: 'parentCollectionID', - description: - 'ID to the collection to which to import to (null if to import to the root of team)', - nullable: true, - }) - parentCollectionID?: string; -} - @ArgsType() export class UpdateTeamCollectionArgs { @Field(() => ID, { diff --git a/packages/hoppscotch-backend/src/team-collection/team-collection.resolver.ts b/packages/hoppscotch-backend/src/team-collection/team-collection.resolver.ts index 07e322d5..3b42c7d4 100644 --- a/packages/hoppscotch-backend/src/team-collection/team-collection.resolver.ts +++ b/packages/hoppscotch-backend/src/team-collection/team-collection.resolver.ts @@ -24,7 +24,6 @@ import { GetRootTeamCollectionsArgs, MoveTeamCollectionArgs, RenameTeamCollectionArgs, - ReplaceTeamCollectionArgs, UpdateTeamCollectionArgs, UpdateTeamCollectionOrderArgs, } from './input-type.args'; @@ -235,24 +234,6 @@ export class TeamCollectionResolver { return importedCollection.right; } - @Mutation(() => Boolean, { - description: - 'Replace existing collections of a specific team with collections in JSON string', - }) - @UseGuards(GqlAuthGuard, GqlTeamMemberGuard) - @RequiresTeamRole(TeamAccessRole.OWNER, TeamAccessRole.EDITOR) - async replaceCollectionsWithJSON(@Args() args: ReplaceTeamCollectionArgs) { - const teamCollection = - await this.teamCollectionService.replaceCollectionsWithJSON( - args.jsonString, - args.teamID, - args.parentCollectionID ?? null, - ); - - if (E.isLeft(teamCollection)) throwErr(teamCollection.left); - return teamCollection.right; - } - @Mutation(() => TeamCollection, { description: 'Create a collection that has a parent collection', }) diff --git a/packages/hoppscotch-backend/src/team-collection/team-collection.service.spec.ts b/packages/hoppscotch-backend/src/team-collection/team-collection.service.spec.ts index fbfeb752..b4829396 100644 --- a/packages/hoppscotch-backend/src/team-collection/team-collection.service.spec.ts +++ b/packages/hoppscotch-backend/src/team-collection/team-collection.service.spec.ts @@ -15,6 +15,8 @@ import { TEAM_MEMBER_NOT_FOUND, TEAM_NOT_OWNER, } from 'src/errors'; +import * as E from 'fp-ts/Either'; +import * as O from 'fp-ts/Option'; import { PrismaService } from 'src/prisma/prisma.service'; import { PubSubService } from 'src/pubsub/pubsub.service'; import { AuthUser } from 'src/types/AuthUser'; @@ -26,8 +28,6 @@ const mockPrisma = mockDeep(); const mockPubSub = mockDeep(); const mockTeamService = mockDeep(); -// eslint-disable-next-line @typescript-eslint/ban-ts-comment -// @ts-ignore const teamCollectionService = new TeamCollectionService( mockPrisma, mockPubSub as any, @@ -671,10 +671,9 @@ describe('createCollection', () => { }); test('should throw TEAM_NOT_OWNER when parent TeamCollection does not belong to the team', async () => { - // isOwnerCheck - mockPrisma.teamCollection.findFirstOrThrow.mockRejectedValueOnce( - 'NotFoundError', - ); + jest + .spyOn(teamCollectionService as any, 'isOwnerCheck') + .mockResolvedValueOnce(O.none); const result = await teamCollectionService.createCollection( rootTeamCollection.teamID, @@ -686,10 +685,9 @@ describe('createCollection', () => { }); test('should throw TEAM_COLL_DATA_INVALID when parent TeamCollection does not belong to the team', async () => { - // isOwnerCheck - mockPrisma.teamCollection.findFirstOrThrow.mockResolvedValueOnce( - rootTeamCollection, - ); + jest + .spyOn(teamCollectionService as any, 'isOwnerCheck') + .mockResolvedValueOnce(O.some(true)); const result = await teamCollectionService.createCollection( rootTeamCollection.teamID, @@ -701,59 +699,58 @@ describe('createCollection', () => { }); test('should successfully create a new root TeamCollection with valid inputs', async () => { - // isOwnerCheck - mockPrisma.teamCollection.findFirstOrThrow.mockResolvedValueOnce( - rootTeamCollection, + mockPrisma.$transaction.mockImplementationOnce(async (fn) => + fn(mockPrisma), ); - - //getRootCollectionsCount - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([]); + mockPrisma.$executeRaw.mockResolvedValueOnce(null); + mockPrisma.teamCollection.findFirst.mockResolvedValueOnce(null); mockPrisma.teamCollection.create.mockResolvedValueOnce(rootTeamCollection); const result = await teamCollectionService.createCollection( rootTeamCollection.teamID, - 'abcdefg', + rootTeamCollection.title, JSON.stringify(rootTeamCollection.data), - rootTeamCollection.id, + null, ); expect(result).toEqualRight(rootTeamCollectionsCasted); }); test('should successfully create a new child TeamCollection with valid inputs', async () => { - // isOwnerCheck - mockPrisma.teamCollection.findFirstOrThrow.mockResolvedValueOnce( - rootTeamCollection, + jest + .spyOn(teamCollectionService as any, 'isOwnerCheck') + .mockResolvedValueOnce(O.some(true)); + mockPrisma.$transaction.mockImplementationOnce(async (fn) => + fn(mockPrisma), ); - - //getChildCollectionsCount - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([]); + mockPrisma.$executeRaw.mockResolvedValueOnce(null); + mockPrisma.teamCollection.findFirst.mockResolvedValueOnce(null); mockPrisma.teamCollection.create.mockResolvedValueOnce(childTeamCollection); const result = await teamCollectionService.createCollection( childTeamCollection.teamID, childTeamCollection.title, - JSON.stringify(rootTeamCollection.data), - rootTeamCollection.id, + JSON.stringify(childTeamCollection.data), + childTeamCollection.parentID, ); expect(result).toEqualRight(childTeamCollectionCasted); }); test('should send pubsub message to "team_coll//coll_added" if child TeamCollection is created successfully', async () => { - // isOwnerCheck - mockPrisma.teamCollection.findFirstOrThrow.mockResolvedValueOnce( - rootTeamCollection, + jest + .spyOn(teamCollectionService as any, 'isOwnerCheck') + .mockResolvedValueOnce(O.some(true)); + mockPrisma.$transaction.mockImplementationOnce(async (fn) => + fn(mockPrisma), ); - - //getChildCollectionsCount - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([]); + mockPrisma.$executeRaw.mockResolvedValueOnce(null); + mockPrisma.teamCollection.findFirst.mockResolvedValueOnce(null); mockPrisma.teamCollection.create.mockResolvedValueOnce(childTeamCollection); await teamCollectionService.createCollection( childTeamCollection.teamID, childTeamCollection.title, - JSON.stringify(rootTeamCollection.data), - - rootTeamCollection.id, + JSON.stringify(childTeamCollection.data), + childTeamCollection.parentID, ); expect(mockPubSub.publish).toHaveBeenCalledWith( `team_coll/${childTeamCollection.teamID}/coll_added`, @@ -762,21 +759,18 @@ describe('createCollection', () => { }); test('should send pubsub message to "team_coll//coll_added" if root TeamCollection is created successfully', async () => { - // isOwnerCheck - mockPrisma.teamCollection.findFirstOrThrow.mockResolvedValueOnce( - rootTeamCollection, + mockPrisma.$transaction.mockImplementationOnce(async (fn) => + fn(mockPrisma), ); - - //getRootCollectionsCount - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([]); + mockPrisma.$executeRaw.mockResolvedValueOnce(null); + mockPrisma.teamCollection.findFirst.mockResolvedValueOnce(null); mockPrisma.teamCollection.create.mockResolvedValueOnce(rootTeamCollection); await teamCollectionService.createCollection( rootTeamCollection.teamID, - 'abcdefg', + rootTeamCollection.title, JSON.stringify(rootTeamCollection.data), - - rootTeamCollection.id, + null, ); expect(mockPubSub.publish).toHaveBeenCalledWith( `team_coll/${rootTeamCollection.teamID}/coll_added`, @@ -891,26 +885,17 @@ describe('deleteCollection', () => { }); test('should throw TEAM_COLL_NOT_FOUND when collectionID is invalid when deleting TeamCollection from UserCollectionTable ', async () => { - // getCollection - mockPrisma.teamCollection.findUniqueOrThrow.mockResolvedValueOnce( - rootTeamCollection, - ); - // deleteCollectionData - // deleteCollectionData --> FindMany query 1st time - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([]); - // deleteCollectionData --> FindMany query 2nd time - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([]); - // deleteCollectionData --> DeleteMany query - mockPrisma.userRequest.deleteMany.mockResolvedValueOnce({ count: 0 }); - // deleteCollectionData --> updateOrderIndex - mockPrisma.teamCollection.updateMany.mockResolvedValueOnce({ count: 0 }); - // deleteCollectionData --> removeUserCollection - mockPrisma.teamCollection.delete.mockRejectedValueOnce('RecordNotFound'); + jest + .spyOn(teamCollectionService, 'getCollection') + .mockResolvedValueOnce(E.right(rootTeamCollection)); + jest + .spyOn(teamCollectionService as any, 'deleteCollectionData') + .mockResolvedValueOnce(E.left(TEAM_COL_REORDERING_FAILED)); const result = await teamCollectionService.deleteCollection( rootTeamCollection.id, ); - expect(result).toEqualLeft(TEAM_COLL_NOT_FOUND); + expect(result).toEqualLeft(TEAM_COL_REORDERING_FAILED); }); test('should send pubsub message to "team_coll//coll_removed" if TeamCollection is deleted successfully', async () => { @@ -940,20 +925,18 @@ describe('deleteCollection', () => { describe('moveCollection', () => { test('should throw TEAM_COLL_NOT_FOUND if collectionID is invalid', async () => { - // getCollection - mockPrisma.teamCollection.findUniqueOrThrow.mockRejectedValueOnce( - 'NotFoundError', - ); + jest + .spyOn(teamCollectionService, 'getCollection') + .mockResolvedValueOnce(E.left(TEAM_COLL_NOT_FOUND)); const result = await teamCollectionService.moveCollection('234', '009'); expect(result).toEqualLeft(TEAM_COLL_NOT_FOUND); }); test('should throw TEAM_COLL_DEST_SAME if collectionID and destCollectionID is the same', async () => { - // getCollection - mockPrisma.teamCollection.findUniqueOrThrow.mockResolvedValueOnce( - rootTeamCollection, - ); + jest + .spyOn(teamCollectionService, 'getCollection') + .mockResolvedValueOnce(E.right(rootTeamCollection)); const result = await teamCollectionService.moveCollection( rootTeamCollection.id, @@ -963,14 +946,12 @@ describe('moveCollection', () => { }); test('should throw TEAM_COLL_NOT_FOUND if destCollectionID is invalid', async () => { - // getCollection - mockPrisma.teamCollection.findUniqueOrThrow.mockResolvedValueOnce( - rootTeamCollection, - ); - // getCollection for destCollection - mockPrisma.teamCollection.findUniqueOrThrow.mockRejectedValueOnce( - 'NotFoundError', - ); + jest + .spyOn(teamCollectionService, 'getCollection') + .mockResolvedValueOnce(E.right(rootTeamCollection)); + jest + .spyOn(teamCollectionService, 'getCollection') + .mockResolvedValueOnce(E.left(TEAM_COLL_NOT_FOUND)); const result = await teamCollectionService.moveCollection( 'invalidID', @@ -980,15 +961,14 @@ describe('moveCollection', () => { }); test('should throw TEAM_COLL_NOT_SAME_TEAM if collectionID and destCollectionID are not from the same team', async () => { - // getCollection - mockPrisma.teamCollection.findUniqueOrThrow.mockResolvedValueOnce( - rootTeamCollection, - ); - // getCollection for destCollection - mockPrisma.teamCollection.findUniqueOrThrow.mockResolvedValueOnce({ - ...childTeamCollection_2, - teamID: 'differentTeamID', - }); + jest + .spyOn(teamCollectionService, 'getCollection') + .mockResolvedValueOnce(E.right(rootTeamCollection)); + jest + .spyOn(teamCollectionService, 'getCollection') + .mockResolvedValueOnce( + E.right({ ...childTeamCollection_2, teamID: 'anotherTeamID' }), + ); const result = await teamCollectionService.moveCollection( rootTeamCollection.id, @@ -998,14 +978,12 @@ describe('moveCollection', () => { }); test('should throw TEAM_COLL_IS_PARENT_COLL if collectionID is parent of destCollectionID ', async () => { - // getCollection - mockPrisma.teamCollection.findUniqueOrThrow.mockResolvedValueOnce( - rootTeamCollection, - ); - // getCollection for destCollection - mockPrisma.teamCollection.findUniqueOrThrow.mockResolvedValueOnce( - childTeamCollection, - ); + jest + .spyOn(teamCollectionService, 'getCollection') + .mockResolvedValueOnce(E.right(rootTeamCollection)); + jest + .spyOn(teamCollectionService, 'getCollection') + .mockResolvedValueOnce(E.right(childTeamCollection)); const result = await teamCollectionService.moveCollection( rootTeamCollection.id, @@ -1015,10 +993,9 @@ describe('moveCollection', () => { }); test('should throw TEAM_COL_ALREADY_ROOT when moving root TeamCollection to root', async () => { - // getCollection - mockPrisma.teamCollection.findUniqueOrThrow.mockResolvedValueOnce( - rootTeamCollection, - ); + jest + .spyOn(teamCollectionService, 'getCollection') + .mockResolvedValueOnce(E.right(rootTeamCollection)); const result = await teamCollectionService.moveCollection( rootTeamCollection.id, @@ -1028,25 +1005,14 @@ describe('moveCollection', () => { }); test('should successfully move a child TeamCollection into root', async () => { - // getCollection - mockPrisma.teamCollection.findUniqueOrThrow.mockResolvedValueOnce( - childTeamCollection, - ); - // updateOrderIndex - mockPrisma.teamCollection.updateMany.mockResolvedValueOnce({ count: 0 }); - // changeParent - // changeParent --> getRootCollectionsCount - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([ - rootTeamCollection, - ]); - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([ - rootTeamCollection, - ]); - mockPrisma.teamCollection.update.mockResolvedValue({ - ...childTeamCollection, - parentID: null, - orderIndex: 2, - }); + jest + .spyOn(teamCollectionService, 'getCollection') + .mockResolvedValueOnce(E.right(childTeamCollection)); + jest + .spyOn(teamCollectionService as any, 'changeParentAndUpdateOrderIndex') + .mockResolvedValueOnce( + E.right({ ...childTeamCollectionCasted, parentID: null }), + ); const result = await teamCollectionService.moveCollection( childTeamCollection.id, @@ -1059,21 +1025,12 @@ describe('moveCollection', () => { }); test('should throw TEAM_COLL_NOT_FOUND when trying to change parent of collection with invalid collectionID', async () => { - // getCollection - mockPrisma.teamCollection.findUniqueOrThrow.mockResolvedValueOnce( - childTeamCollection, - ); - // updateOrderIndex - mockPrisma.teamCollection.updateMany.mockResolvedValueOnce({ count: 0 }); - // changeParent - // changeParent --> getRootCollectionsCount - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([ - rootTeamCollection, - ]); - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([ - rootTeamCollection, - ]); - mockPrisma.teamCollection.update.mockRejectedValueOnce('RecordNotFound'); + jest + .spyOn(teamCollectionService, 'getCollection') + .mockResolvedValueOnce(E.right(childTeamCollection)); + jest + .spyOn(teamCollectionService as any, 'changeParentAndUpdateOrderIndex') + .mockResolvedValueOnce(E.left(TEAM_COLL_NOT_FOUND)); const result = await teamCollectionService.moveCollection( childTeamCollection.id, @@ -1083,25 +1040,14 @@ describe('moveCollection', () => { }); test('should send pubsub message to "team_coll//coll_moved" when a child TeamCollection is moved to root successfully', async () => { - // getCollection - mockPrisma.teamCollection.findUniqueOrThrow.mockResolvedValueOnce( - childTeamCollection, - ); - // updateOrderIndex - mockPrisma.teamCollection.updateMany.mockResolvedValueOnce({ count: 0 }); - // changeParent - // changeParent --> getRootCollectionsCount - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([ - rootTeamCollection, - ]); - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([ - rootTeamCollection, - ]); - mockPrisma.teamCollection.update.mockResolvedValue({ - ...childTeamCollection, - parentID: null, - orderIndex: 2, - }); + jest + .spyOn(teamCollectionService, 'getCollection') + .mockResolvedValueOnce(E.right(childTeamCollection)); + jest + .spyOn(teamCollectionService as any, 'changeParentAndUpdateOrderIndex') + .mockResolvedValueOnce( + E.right({ ...childTeamCollectionCasted, parentID: null }), + ); await teamCollectionService.moveCollection(childTeamCollection.id, null); expect(mockPubSub.publish).toHaveBeenCalledWith( @@ -1114,114 +1060,85 @@ describe('moveCollection', () => { }); test('should successfully move a root TeamCollection into a child TeamCollection', async () => { - // getCollection - mockPrisma.teamCollection.findUniqueOrThrow.mockResolvedValueOnce( - rootTeamCollection, - ); - // getCollection for destCollection - mockPrisma.teamCollection.findUniqueOrThrow - .mockResolvedValueOnce(rootTeamCollection_2) - .mockResolvedValueOnce(null); - // isParent --> getCollection - mockPrisma.teamCollection.findUnique.mockResolvedValueOnce( - childTeamCollection_2, - ); - // updateOrderIndex - mockPrisma.teamCollection.updateMany.mockResolvedValueOnce({ count: 0 }); - // changeParent - // changeParent --> getRootCollectionsCount - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([ - rootTeamCollection, - ]); - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([ - rootTeamCollection, - ]); - mockPrisma.teamCollection.update.mockResolvedValue({ - ...rootTeamCollection, - parentID: childTeamCollection_2.id, - orderIndex: 1, - }); + jest + .spyOn(teamCollectionService, 'getCollection') + .mockResolvedValueOnce(E.right(rootTeamCollection)); + jest + .spyOn(teamCollectionService, 'getCollection') + .mockResolvedValueOnce(E.right(childTeamCollection)); + jest + .spyOn(teamCollectionService as any, 'isParent') + .mockResolvedValueOnce(O.some(true)); + jest + .spyOn(teamCollectionService as any, 'changeParentAndUpdateOrderIndex') + .mockResolvedValueOnce( + E.right({ + ...rootTeamCollectionsCasted, + parentID: childTeamCollection.id, + }), + ); const result = await teamCollectionService.moveCollection( rootTeamCollection.id, - childTeamCollection_2.id, + childTeamCollection.id, ); expect(result).toEqualRight({ ...rootTeamCollectionsCasted, - parentID: childTeamCollection_2Casted.id, + parentID: childTeamCollection.id, }); }); test('should send pubsub message to "team_coll//coll_moved" when root TeamCollection is moved into another child TeamCollection successfully', async () => { - // getCollection - mockPrisma.teamCollection.findUniqueOrThrow.mockResolvedValueOnce( - rootTeamCollection, - ); - // getCollection for destCollection - mockPrisma.teamCollection.findUniqueOrThrow - .mockResolvedValueOnce(rootTeamCollection_2) - .mockResolvedValueOnce(null); - // isParent --> getCollection - mockPrisma.teamCollection.findUnique.mockResolvedValueOnce( - childTeamCollection_2, - ); - // updateOrderIndex - mockPrisma.teamCollection.updateMany.mockResolvedValueOnce({ count: 0 }); - // changeParent - // changeParent --> getRootCollectionsCount - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([ - rootTeamCollection, - ]); - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([ - rootTeamCollection, - ]); - mockPrisma.teamCollection.update.mockResolvedValue({ - ...rootTeamCollection, - parentID: childTeamCollection_2.id, - orderIndex: 1, - }); + jest + .spyOn(teamCollectionService, 'getCollection') + .mockResolvedValueOnce(E.right(rootTeamCollection)); + jest + .spyOn(teamCollectionService, 'getCollection') + .mockResolvedValueOnce(E.right(childTeamCollection)); + jest + .spyOn(teamCollectionService as any, 'isParent') + .mockResolvedValueOnce(O.some(true)); + jest + .spyOn(teamCollectionService as any, 'changeParentAndUpdateOrderIndex') + .mockResolvedValueOnce( + E.right({ + ...rootTeamCollectionsCasted, + parentID: childTeamCollection.id, + }), + ); await teamCollectionService.moveCollection( rootTeamCollection.id, - childTeamCollection_2.id, + childTeamCollection.id, ); + expect(mockPubSub.publish).toHaveBeenCalledWith( - `team_coll/${childTeamCollection_2.teamID}/coll_moved`, + `team_coll/${childTeamCollection.teamID}/coll_moved`, { ...rootTeamCollectionsCasted, - parentID: childTeamCollection_2Casted.id, + parentID: childTeamCollectionCasted.id, }, ); }); test('should successfully move a child TeamCollection into another child TeamCollection', async () => { - // getCollection - mockPrisma.teamCollection.findUniqueOrThrow.mockResolvedValueOnce( - childTeamCollection, - ); - // getCollection for destCollection - mockPrisma.teamCollection.findUniqueOrThrow - .mockResolvedValueOnce(rootTeamCollection_2) - .mockResolvedValueOnce(null); - // isParent --> getCollection - mockPrisma.teamCollection.findUnique.mockResolvedValueOnce( - childTeamCollection_2, - ); - // updateOrderIndex - mockPrisma.teamCollection.updateMany.mockResolvedValueOnce({ count: 0 }); - // changeParent - // changeParent --> getRootCollectionsCount - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([ - childTeamCollection, - ]); - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([ - childTeamCollection_2, - ]); - mockPrisma.teamCollection.update.mockResolvedValue({ - ...childTeamCollection, - parentID: childTeamCollection_2.id, - orderIndex: 1, - }); + jest + .spyOn(teamCollectionService, 'getCollection') + .mockResolvedValueOnce(E.right(childTeamCollection)); + jest + .spyOn(teamCollectionService, 'getCollection') + .mockResolvedValueOnce(E.right(childTeamCollection_2)); + jest + .spyOn(teamCollectionService as any, 'isParent') + .mockResolvedValueOnce(O.some(true)); + jest + .spyOn(teamCollectionService as any, 'changeParentAndUpdateOrderIndex') + .mockResolvedValueOnce( + E.right({ + ...childTeamCollectionCasted, + parentID: childTeamCollection_2.id, + }), + ); const result = await teamCollectionService.moveCollection( childTeamCollection.id, @@ -1234,33 +1151,23 @@ describe('moveCollection', () => { }); test('should send pubsub message to "team_coll//coll_moved" when child TeamCollection is moved into another child TeamCollection successfully', async () => { - // getCollection - mockPrisma.teamCollection.findUniqueOrThrow.mockResolvedValueOnce( - childTeamCollection, - ); - // getCollection for destCollection - mockPrisma.teamCollection.findUniqueOrThrow - .mockResolvedValueOnce(rootTeamCollection_2) - .mockResolvedValueOnce(null); - // isParent --> getCollection - mockPrisma.teamCollection.findUnique.mockResolvedValueOnce( - childTeamCollection_2, - ); - // updateOrderIndex - mockPrisma.teamCollection.updateMany.mockResolvedValueOnce({ count: 0 }); - // changeParent - // changeParent --> getRootCollectionsCount - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([ - childTeamCollection, - ]); - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([ - childTeamCollection_2, - ]); - mockPrisma.teamCollection.update.mockResolvedValue({ - ...childTeamCollection, - parentID: childTeamCollection_2.id, - orderIndex: 1, - }); + jest + .spyOn(teamCollectionService, 'getCollection') + .mockResolvedValueOnce(E.right(childTeamCollection)); + jest + .spyOn(teamCollectionService, 'getCollection') + .mockResolvedValueOnce(E.right(childTeamCollection_2)); + jest + .spyOn(teamCollectionService as any, 'isParent') + .mockResolvedValueOnce(O.some(true)); + jest + .spyOn(teamCollectionService as any, 'changeParentAndUpdateOrderIndex') + .mockResolvedValueOnce( + E.right({ + ...childTeamCollectionCasted, + parentID: childTeamCollection_2.id, + }), + ); await teamCollectionService.moveCollection( childTeamCollection.id, @@ -1477,9 +1384,9 @@ describe('importCollectionsFromJSON', () => { }); test('should successfully create new TeamCollections in root and TeamRequests with valid inputs', async () => { - //getRootCollectionsCount - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([]); - mockPrisma.$transaction.mockResolvedValueOnce([rootTeamCollection]); + mockPrisma.$transaction.mockImplementation(async (fn) => fn(mockPrisma)); + mockPrisma.teamCollection.findFirst.mockResolvedValueOnce(null); + mockPrisma.teamCollection.create.mockResolvedValueOnce(rootTeamCollection); const result = await teamCollectionService.importCollectionsFromJSON( jsonString, @@ -1490,9 +1397,9 @@ describe('importCollectionsFromJSON', () => { }); test('should successfully create new TeamCollections in a child collection and TeamRequests with valid inputs', async () => { - //getChildCollectionsCount - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([]); - mockPrisma.$transaction.mockResolvedValueOnce([rootTeamCollection]); + mockPrisma.$transaction.mockImplementation(async (fn) => fn(mockPrisma)); + mockPrisma.teamCollection.findFirst.mockResolvedValueOnce(null); + mockPrisma.teamCollection.create.mockResolvedValueOnce(rootTeamCollection); const result = await teamCollectionService.importCollectionsFromJSON( jsonString, @@ -1503,9 +1410,9 @@ describe('importCollectionsFromJSON', () => { }); test('should send pubsub message to "team_coll//coll_added" on successful creation from jsonString', async () => { - //getRootCollectionsCount - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([]); - mockPrisma.$transaction.mockResolvedValueOnce([rootTeamCollection]); + mockPrisma.$transaction.mockImplementation(async (fn) => fn(mockPrisma)); + mockPrisma.teamCollection.findFirst.mockResolvedValueOnce(null); + mockPrisma.teamCollection.create.mockResolvedValueOnce(rootTeamCollection); await teamCollectionService.importCollectionsFromJSON( jsonString, @@ -1519,125 +1426,6 @@ describe('importCollectionsFromJSON', () => { }); }); -describe('replaceCollectionsWithJSON', () => { - test('should throw TEAM_COLL_INVALID_JSON when the jsonString is invalid', async () => { - const result = await teamCollectionService.replaceCollectionsWithJSON( - 'invalidString', - rootTeamCollection.teamID, - null, - ); - expect(result).toEqualLeft(TEAM_COLL_INVALID_JSON); - }); - - test('should throw TEAM_COLL_INVALID_JSON when the parsed jsonString is not an array', async () => { - const result = await teamCollectionService.replaceCollectionsWithJSON( - '{}', - rootTeamCollection.teamID, - null, - ); - expect(result).toEqualLeft(TEAM_COLL_INVALID_JSON); - }); - - test('should successfully replace TeamCollections in root with new TeamCollections and TeamRequests with valid inputs', async () => { - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([ - rootTeamCollection, - ]); - // deleteCollection - // getCollection - mockPrisma.teamCollection.findUniqueOrThrow.mockResolvedValueOnce( - rootTeamCollection, - ); - // deleteCollectionData - // deleteCollectionData --> FindMany query 1st time - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([]); - // deleteCollectionData --> FindMany query 2nd time - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([]); - // deleteCollectionData --> DeleteMany query - mockPrisma.teamRequest.deleteMany.mockResolvedValueOnce({ count: 0 }); - // deleteCollectionData --> updateOrderIndex - mockPrisma.teamCollection.updateMany.mockResolvedValueOnce({ count: 0 }); - // deleteCollectionData --> removeUserCollection - mockPrisma.teamCollection.delete.mockResolvedValueOnce(rootTeamCollection); - //getRootCollectionsCount - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([]); - mockPrisma.$transaction.mockResolvedValueOnce([rootTeamCollection]); - - const result = await teamCollectionService.replaceCollectionsWithJSON( - jsonString, - rootTeamCollection.teamID, - null, - ); - expect(result).toEqualRight(true); - }); - - test('should successfully create new TeamCollections in a child collection and TeamRequests with valid inputs', async () => { - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([ - childTeamCollection, - ]); - // deleteCollection - // getCollection - mockPrisma.teamCollection.findUniqueOrThrow.mockResolvedValueOnce( - childTeamCollection, - ); - // deleteCollectionData - // deleteCollectionData --> FindMany query 1st time - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([]); - // deleteCollectionData --> FindMany query 2nd time - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([]); - // deleteCollectionData --> DeleteMany query - mockPrisma.teamRequest.deleteMany.mockResolvedValueOnce({ count: 0 }); - // deleteCollectionData --> updateOrderIndex - mockPrisma.teamCollection.updateMany.mockResolvedValueOnce({ count: 0 }); - // deleteCollectionData --> removeUserCollection - mockPrisma.teamCollection.delete.mockResolvedValueOnce(childTeamCollection); - //getRootCollectionsCount - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([]); - mockPrisma.$transaction.mockResolvedValueOnce([rootTeamCollection]); - - const result = await teamCollectionService.replaceCollectionsWithJSON( - jsonString, - rootTeamCollection.teamID, - rootTeamCollection.id, - ); - expect(result).toEqualRight(true); - }); - - test('should send pubsub message to "team_coll//coll_added" on successful creation from jsonString', async () => { - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([ - rootTeamCollection, - ]); - // deleteCollection - // getCollection - mockPrisma.teamCollection.findUniqueOrThrow.mockResolvedValueOnce( - rootTeamCollection, - ); - // deleteCollectionData - // deleteCollectionData --> FindMany query 1st time - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([]); - // deleteCollectionData --> FindMany query 2nd time - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([]); - // deleteCollectionData --> DeleteMany query - mockPrisma.teamRequest.deleteMany.mockResolvedValueOnce({ count: 0 }); - // deleteCollectionData --> updateOrderIndex - mockPrisma.teamCollection.updateMany.mockResolvedValueOnce({ count: 0 }); - // deleteCollectionData --> removeUserCollection - mockPrisma.teamCollection.delete.mockResolvedValueOnce(rootTeamCollection); - //getRootCollectionsCount - mockPrisma.teamCollection.findMany.mockResolvedValueOnce([]); - mockPrisma.$transaction.mockResolvedValueOnce([rootTeamCollection]); - - await teamCollectionService.replaceCollectionsWithJSON( - jsonString, - rootTeamCollection.teamID, - null, - ); - expect(mockPubSub.publish).toHaveBeenCalledWith( - `team_coll/${rootTeamCollection.teamID}/coll_added`, - rootTeamCollectionsCasted, - ); - }); -}); - describe('totalCollectionsInTeam', () => { test('should resolve right and return a total team colls count ', async () => { mockPrisma.teamCollection.count.mockResolvedValueOnce(2); diff --git a/packages/hoppscotch-backend/src/team-collection/team-collection.service.ts b/packages/hoppscotch-backend/src/team-collection/team-collection.service.ts index b00acd21..f1699028 100644 --- a/packages/hoppscotch-backend/src/team-collection/team-collection.service.ts +++ b/packages/hoppscotch-backend/src/team-collection/team-collection.service.ts @@ -1,4 +1,4 @@ -import { HttpStatus, Injectable } from '@nestjs/common'; +import { ConflictException, HttpStatus, Injectable } from '@nestjs/common'; import { PrismaService } from '../prisma/prisma.service'; import { TeamCollection } from './team-collection.model'; import { @@ -19,9 +19,11 @@ import { TEAM_REQ_PARENT_TREE_GEN_FAILED, TEAM_COLL_PARENT_TREE_GEN_FAILED, TEAM_MEMBER_NOT_FOUND, + TEAM_COLL_CREATION_FAILED, } from '../errors'; import { PubSubService } from '../pubsub/pubsub.service'; import { + delay, escapeSqlLikeString, isValidLength, transformCollectionData, @@ -43,6 +45,7 @@ import { } from './helper'; import { RESTError } from 'src/types/RESTError'; import { TeamService } from 'src/team/team.service'; +import { PrismaError } from 'src/prisma/prisma-error-codes'; @Injectable() export class TeamCollectionService { @@ -53,6 +56,7 @@ export class TeamCollectionService { ) {} TITLE_LENGTH = 3; + MAX_RETRIES = 5; // Maximum number of retries for database transactions /** * Generate a Prisma query object representation of a collection and its child collections and requests @@ -179,14 +183,14 @@ export class TeamCollectionService { * Create new TeamCollections and TeamRequests from JSON string * * @param jsonString The JSON string of the content - * @param destTeamID The Team ID - * @param destCollectionID The Collection ID + * @param teamID Team ID, where the collections will be created + * @param parentID Collection ID, where the collections will be created under * @returns An Either of a Boolean if the creation operation was successful */ async importCollectionsFromJSON( jsonString: string, - destTeamID: string, - destCollectionID: string | null, + teamID: string, + parentID: string | null, ) { // Check to see if jsonString is valid const collectionsList = stringToJson(jsonString); @@ -196,38 +200,55 @@ export class TeamCollectionService { if (!Array.isArray(collectionsList.right)) return E.left(TEAM_COLL_INVALID_JSON); - // Get number of root or child collections for destCollectionID(if destcollectionID != null) or destTeamID(if destcollectionID == null) - const count = !destCollectionID - ? await this.getRootCollectionsCount(destTeamID) - : await this.getChildCollectionsCount(destCollectionID); + let teamCollections: DBTeamCollection[] = []; + let queryList: Prisma.TeamCollectionCreateInput[] = []; + try { + await this.prisma.$transaction(async (tx) => { + try { + // lock the rows + await this.prisma.lockTableExclusive(tx, 'TeamCollection'); - // Generate Prisma Query Object for all child collections in collectionsList - const queryList = collectionsList.right.map((x) => - this.generatePrismaQueryObjForFBCollFolder(x, destTeamID, count + 1), - ); + // Get the last order index + const lastEntry = await tx.teamCollection.findFirst({ + where: { teamID, parentID }, + orderBy: { orderIndex: 'desc' }, + select: { orderIndex: true }, + }); + let lastOrderIndex = lastEntry ? lastEntry.orderIndex : 0; - const parent = destCollectionID - ? { - connect: { - id: destCollectionID, - }, + // Generate Prisma Query Object for all child collections in collectionsList + queryList = collectionsList.right.map((x) => + this.generatePrismaQueryObjForFBCollFolder( + x, + teamID, + ++lastOrderIndex, + ), + ); + + const promises = queryList.map((query) => + tx.teamCollection.create({ + data: { + ...query, + parent: parentID ? { connect: { id: parentID } } : undefined, + }, + }), + ); + teamCollections = await Promise.all(promises); + } catch (error) { + throw new ConflictException(error); } - : undefined; - - const teamCollections = await this.prisma.$transaction( - queryList.map((x) => - this.prisma.teamCollection.create({ - data: { - ...x, - parent, - }, - }), - ), - ); + }); + } catch (error) { + console.error( + 'Error from TeamCollectionService.importCollectionsFromJSON', + error, + ); + return E.left(TEAM_COLL_CREATION_FAILED); + } teamCollections.forEach((collection) => this.pubsub.publish( - `team_coll/${destTeamID}/coll_added`, + `team_coll/${teamID}/coll_added`, this.cast(collection), ), ); @@ -235,79 +256,6 @@ export class TeamCollectionService { return E.right(true); } - /** - * Replace all the existing contents of a collection (or root collections) with data from JSON String - * - * @param jsonString The JSON string of the content - * @param destTeamID The Team ID - * @param destCollectionID The Collection ID - * @returns An Either of a Boolean if the operation was successful - */ - async replaceCollectionsWithJSON( - jsonString: string, - destTeamID: string, - destCollectionID: string | null, - ) { - // Check to see if jsonString is valid - const collectionsList = stringToJson(jsonString); - if (E.isLeft(collectionsList)) return E.left(TEAM_COLL_INVALID_JSON); - - // Check to see if parsed jsonString is an array - if (!Array.isArray(collectionsList.right)) - return E.left(TEAM_COLL_INVALID_JSON); - - // Fetch all child collections of destCollectionID - const childrenCollection = await this.prisma.teamCollection.findMany({ - where: { - teamID: destTeamID, - parentID: destCollectionID, - }, - }); - - for (const coll of childrenCollection) { - const deletedTeamCollection = await this.deleteCollection(coll.id); - if (E.isLeft(deletedTeamCollection)) - return E.left(deletedTeamCollection.left); - } - - // Get number of root or child collections for destCollectionID(if destcollectionID != null) or destTeamID(if destcollectionID == null) - const count = !destCollectionID - ? await this.getRootCollectionsCount(destTeamID) - : await this.getChildCollectionsCount(destCollectionID); - - const queryList = collectionsList.right.map((x) => - this.generatePrismaQueryObjForFBCollFolder(x, destTeamID, count + 1), - ); - - const parent = destCollectionID - ? { - connect: { - id: destCollectionID, - }, - } - : undefined; - - const teamCollections = await this.prisma.$transaction( - queryList.map((x) => - this.prisma.teamCollection.create({ - data: { - ...x, - parent, - }, - }), - ), - ); - - teamCollections.forEach((collections) => - this.pubsub.publish( - `team_coll/${destTeamID}/coll_added`, - this.cast(collections), - ), - ); - - return E.right(true); - } - /** * Typecast a database TeamCollection to a TeamCollection model * @@ -476,62 +424,26 @@ export class TeamCollectionService { } } - /** - * Returns the count of child collections present for a given collectionID - * * The count returned is highest OrderIndex + 1 - * - * @param collectionID The Collection ID - * @returns Number of Child Collections - */ - private async getChildCollectionsCount(collectionID: string) { - const childCollectionCount = await this.prisma.teamCollection.findMany({ - where: { parentID: collectionID }, - orderBy: { - orderIndex: 'desc', - }, - }); - if (!childCollectionCount.length) return 0; - return childCollectionCount[0].orderIndex; - } - - /** - * Returns the count of root collections present for a given teamID - * * The count returned is highest OrderIndex + 1 - * - * @param teamID The Team ID - * @returns Number of Root Collections - */ - private async getRootCollectionsCount(teamID: string) { - const rootCollectionCount = await this.prisma.teamCollection.findMany({ - where: { teamID, parentID: null }, - orderBy: { - orderIndex: 'desc', - }, - }); - if (!rootCollectionCount.length) return 0; - return rootCollectionCount[0].orderIndex; - } - /** * Create a new TeamCollection * * @param teamID The Team ID * @param title The title of new TeamCollection - * @param parentTeamCollectionID The parent collectionID (null if root collection) + * @param parentID The parent collectionID (null if root collection) * @returns An Either of TeamCollection */ async createCollection( teamID: string, title: string, data: string | null = null, - parentTeamCollectionID: string | null, + parentID: string | null, ) { const isTitleValid = isValidLength(title, this.TITLE_LENGTH); if (!isTitleValid) return E.left(TEAM_COLL_SHORT_TITLE); // Check to see if parentTeamCollectionID belongs to this Team - if (parentTeamCollectionID !== null) { - const isOwner = await this.isOwnerCheck(parentTeamCollectionID, teamID); + if (parentID !== null) { + const isOwner = await this.isOwnerCheck(parentID, teamID); if (O.isNone(isOwner)) return E.left(TEAM_NOT_OWNER); } @@ -542,29 +454,38 @@ export class TeamCollectionService { data = jsonReq.right; } - const isParent = parentTeamCollectionID - ? { - connect: { - id: parentTeamCollectionID, - }, - } - : undefined; + let teamCollection: DBTeamCollection | null = null; + try { + teamCollection = await this.prisma.$transaction(async (tx) => { + try { + // lock the rows + await this.prisma.lockTableExclusive(tx, 'TeamCollection'); - const teamCollection = await this.prisma.teamCollection.create({ - data: { - title: title, - team: { - connect: { - id: teamID, - }, - }, - parent: isParent, - data: data ?? undefined, - orderIndex: !parentTeamCollectionID - ? (await this.getRootCollectionsCount(teamID)) + 1 - : (await this.getChildCollectionsCount(parentTeamCollectionID)) + 1, - }, - }); + // fetch last collection + const lastCollection = await tx.teamCollection.findFirst({ + where: { teamID, parentID }, + orderBy: { orderIndex: 'desc' }, + select: { orderIndex: true }, + }); + + // create new collection + return tx.teamCollection.create({ + data: { + title, + teamID, + parentID: parentID ? parentID : undefined, + data: data ?? undefined, + orderIndex: lastCollection ? lastCollection.orderIndex + 1 : 1, + }, + }); + } catch (error) { + throw new ConflictException(error); + } + }); + } catch (error) { + console.error('Error from TeamCollectionService.createCollection', error); + return E.left(TEAM_COLL_CREATION_FAILED); + } this.pubsub.publish( `team_coll/${teamID}/coll_added`, @@ -588,12 +509,8 @@ export class TeamCollectionService { try { const updatedTeamCollection = await this.prisma.teamCollection.update({ - where: { - id: collectionID, - }, - data: { - title: newTitle, - }, + where: { id: collectionID }, + data: { title: newTitle }, }); this.pubsub.publish( @@ -610,45 +527,61 @@ export class TeamCollectionService { /** * Update the OrderIndex of all collections in given parentID * - * @param parentID The Parent collectionID + * @param collection The collection to delete * @param orderIndexCondition Condition to decide what collections will be updated * @param dataCondition Increment/Decrement OrderIndex condition - * @returns A Collection with updated OrderIndexes */ - private async updateOrderIndex( - parentID: string, + private async deleteCollectionAndUpdateSiblingsOrderIndex( + collection: DBTeamCollection, orderIndexCondition: Prisma.IntFilter, dataCondition: Prisma.IntFieldUpdateOperationsInput, ) { - const updatedTeamCollection = await this.prisma.teamCollection.updateMany({ - where: { - parentID: parentID, - orderIndex: orderIndexCondition, - }, - data: { orderIndex: dataCondition }, - }); + let retryCount = 0; + while (retryCount < this.MAX_RETRIES) { + try { + await this.prisma.$transaction(async (tx) => { + try { + // lock the rows + await this.prisma.lockTableExclusive(tx, 'TeamCollection'); - return updatedTeamCollection; - } + await tx.teamCollection.delete({ + where: { id: collection.id }, + }); - /** - * Delete a TeamCollection from the DB - * - * @param collectionID The Collection Id - * @returns The deleted TeamCollection - */ - private async removeTeamCollection(collectionID: string) { - try { - const deletedTeamCollection = await this.prisma.teamCollection.delete({ - where: { - id: collectionID, - }, - }); + // update siblings orderIndexes + await tx.teamCollection.updateMany({ + where: { + parentID: collection.parentID, + orderIndex: orderIndexCondition, + }, + data: { orderIndex: dataCondition }, + }); + } catch (error) { + throw new ConflictException(error); + } + }); - return E.right(deletedTeamCollection); - } catch (error) { - return E.left(TEAM_COLL_NOT_FOUND); + break; + } catch (error) { + console.error( + 'Error from TeamCollectionService.updateOrderIndex', + error, + ); + retryCount++; + if ( + retryCount >= this.MAX_RETRIES || + (error.code !== PrismaError.UNIQUE_CONSTRAINT_VIOLATION && + error.code !== PrismaError.TRANSACTION_DEADLOCK && + error.code !== PrismaError.TRANSACTION_TIMEOUT) // return for all DB error except deadlocks, unique constraint violations, transaction timeouts + ) + return E.left(TEAM_COL_REORDERING_FAILED); + + await delay(retryCount * 100); + console.debug(`Retrying updateOrderIndex... (${retryCount})`); + } } + + return E.right(true); } /** @@ -677,19 +610,20 @@ export class TeamCollectionService { }, }); - // Delete collection from TeamCollection table - const deletedTeamCollection = await this.removeTeamCollection( - collection.id, + // Update orderIndexes in TeamCollection table for user + const isDeleted = await this.deleteCollectionAndUpdateSiblingsOrderIndex( + collection, + { gt: collection.orderIndex }, + { decrement: 1 }, ); - if (E.isLeft(deletedTeamCollection)) - return E.left(deletedTeamCollection.left); + if (E.isLeft(isDeleted)) return E.left(isDeleted.left); this.pubsub.publish( - `team_coll/${deletedTeamCollection.right.teamID}/coll_removed`, - deletedTeamCollection.right.id, + `team_coll/${collection.teamID}/coll_removed`, + collection.id, ); - return E.right(deletedTeamCollection.right); + return E.right(collection); } /** @@ -706,48 +640,67 @@ export class TeamCollectionService { const collectionData = await this.deleteCollectionData(collection.right); if (E.isLeft(collectionData)) return E.left(collectionData.left); - // Update orderIndexes in TeamCollection table for user - await this.updateOrderIndex( - collectionData.right.parentID, - { gt: collectionData.right.orderIndex }, - { decrement: 1 }, - ); - return E.right(true); } /** * Change parentID of TeamCollection's * - * @param collectionID The collection ID - * @param parentCollectionID The new parent's collection ID or change to root collection - * @returns If successful return an Either of true + * @param collection The collection that is being moved + * @param newParentID The new parent's collection ID or change to root collection + * @returns If successful return an Either of collection or error message */ - private async changeParent( + private async changeParentAndUpdateOrderIndex( collection: DBTeamCollection, - parentCollectionID: string | null, + newParentID: string | null, ) { + let updatedCollection: DBTeamCollection = null; + try { - let collectionCount: number; + await this.prisma.$transaction(async (tx) => { + try { + // fetch last collection + const lastCollectionUnderNewParent = + await tx.teamCollection.findFirst({ + where: { teamID: collection.teamID, parentID: newParentID }, + orderBy: { orderIndex: 'desc' }, + }); - if (!parentCollectionID) - collectionCount = await this.getRootCollectionsCount(collection.teamID); - collectionCount = await this.getChildCollectionsCount(parentCollectionID); + // decrement orderIndex of all next sibling collections from original collection + await tx.teamCollection.updateMany({ + where: { + teamID: collection.teamID, + parentID: collection.parentID, + orderIndex: { gt: collection.orderIndex }, + }, + data: { + orderIndex: { decrement: 1 }, + }, + }); - const updatedCollection = await this.prisma.teamCollection.update({ - where: { - id: collection.id, - }, - data: { - // if parentCollectionID == null, collection becomes root collection - // if parentCollectionID != null, collection becomes child collection - parentID: parentCollectionID, - orderIndex: collectionCount + 1, - }, + // update collection's parentID and orderIndex + updatedCollection = await tx.teamCollection.update({ + where: { id: collection.id }, + data: { + // if parentCollectionID == null, collection becomes root collection + // if parentCollectionID != null, collection becomes child collection + parentID: newParentID, + orderIndex: lastCollectionUnderNewParent + ? lastCollectionUnderNewParent.orderIndex + 1 + : 1, + }, + }); + } catch (error) { + throw new ConflictException(error); + } }); return E.right(this.cast(updatedCollection)); } catch (error) { + console.error( + 'Error from TeamCollectionService.changeParentAndUpdateOrderIndex', + error, + ); return E.left(TEAM_COLL_NOT_FOUND); } } @@ -819,15 +772,13 @@ export class TeamCollectionService { // Throw error if collection is already a root collection return E.left(TEAM_COL_ALREADY_ROOT); } - // Move child collection into root and update orderIndexes for root teamCollections - await this.updateOrderIndex( - collection.right.parentID, - { gt: collection.right.orderIndex }, - { decrement: 1 }, - ); // Change parent from child to root i.e child collection becomes a root collection - const updatedCollection = await this.changeParent(collection.right, null); + // Move child collection into root and update orderIndexes for root teamCollections + const updatedCollection = await this.changeParentAndUpdateOrderIndex( + collection.right, + null, + ); if (E.isLeft(updatedCollection)) return E.left(updatedCollection.left); this.pubsub.publish( @@ -862,15 +813,9 @@ export class TeamCollectionService { return E.left(TEAM_COLL_IS_PARENT_COLL); } - // Move root/child collection into another child collection and update orderIndexes of the previous parent - await this.updateOrderIndex( - collection.right.parentID, - { gt: collection.right.orderIndex }, - { decrement: 1 }, - ); - // Change parent from null to teamCollection i.e collection becomes a child collection - const updatedCollection = await this.changeParent( + // Move root/child collection into another child collection and update orderIndexes of the previous parent + const updatedCollection = await this.changeParentAndUpdateOrderIndex( collection.right, destCollection.right.id, ); @@ -919,27 +864,44 @@ export class TeamCollectionService { // nextCollectionID == null i.e move collection to the end of the list try { await this.prisma.$transaction(async (tx) => { - // Step 1: Decrement orderIndex of all items that come after collection.orderIndex till end of list of items - await tx.teamCollection.updateMany({ - where: { - parentID: collection.right.parentID, - orderIndex: { - gte: collection.right.orderIndex + 1, + try { + // Step 0: lock the rows + await this.prisma.acquireLocks( + tx, + 'TeamCollection', + null, + collectionID, + ); + + // Step 1: Decrement orderIndex of all items that come after collection.orderIndex till end of list of items + const collectionInTx = await tx.teamCollection.findFirst({ + where: { id: collection.right.id }, + select: { orderIndex: true }, + }); + await tx.teamCollection.updateMany({ + where: { + parentID: collection.right.parentID, + orderIndex: { + gte: collectionInTx.orderIndex + 1, + }, }, - }, - data: { - orderIndex: { decrement: 1 }, - }, - }); - // Step 2: Update orderIndex of collection to length of list - await tx.teamCollection.update({ - where: { id: collection.right.id }, - data: { - orderIndex: await this.getCollectionCount( - collection.right.parentID, - ), - }, - }); + data: { + orderIndex: { decrement: 1 }, + }, + }); + + // Step 2: Update orderIndex of collection to length of list + await tx.teamCollection.update({ + where: { id: collection.right.id }, + data: { + orderIndex: await this.getCollectionCount( + collection.right.parentID, + ), + }, + }); + } catch (error) { + throw new ConflictException(error); + } }); this.pubsub.publish( @@ -967,36 +929,58 @@ export class TeamCollectionService { try { await this.prisma.$transaction(async (tx) => { - // Step 1: Determine if we are moving collection up or down the list - const isMovingUp = - subsequentCollection.right.orderIndex < collection.right.orderIndex; - // Step 2: Update OrderIndex of items in list depending on moving up or down - const updateFrom = isMovingUp - ? subsequentCollection.right.orderIndex - : collection.right.orderIndex + 1; + try { + // Step 0: lock the rows + await this.prisma.acquireLocks( + tx, + 'TeamCollection', + null, + collection.right.parentID, + ); - const updateTo = isMovingUp - ? collection.right.orderIndex - 1 - : subsequentCollection.right.orderIndex - 1; + // Step 1: Determine if we are moving collection up or down the list + const collectionInTx = await tx.teamCollection.findFirst({ + where: { id: collectionID }, + select: { orderIndex: true }, + }); + const subsequentCollectionInTx = await tx.teamCollection.findFirst({ + where: { id: nextCollectionID }, + select: { orderIndex: true }, + }); + const isMovingUp = + subsequentCollectionInTx.orderIndex < collectionInTx.orderIndex; - await tx.teamCollection.updateMany({ - where: { - parentID: collection.right.parentID, - orderIndex: { gte: updateFrom, lte: updateTo }, - }, - data: { - orderIndex: isMovingUp ? { increment: 1 } : { decrement: 1 }, - }, - }); - // Step 3: Update OrderIndex of collection - await tx.teamCollection.update({ - where: { id: collection.right.id }, - data: { - orderIndex: isMovingUp - ? subsequentCollection.right.orderIndex - : subsequentCollection.right.orderIndex - 1, - }, - }); + // Step 2: Update OrderIndex of items in list depending on moving up or down + const updateFrom = isMovingUp + ? subsequentCollectionInTx.orderIndex + : collectionInTx.orderIndex + 1; + + const updateTo = isMovingUp + ? collectionInTx.orderIndex - 1 + : subsequentCollectionInTx.orderIndex - 1; + + await tx.teamCollection.updateMany({ + where: { + parentID: collection.right.parentID, + orderIndex: { gte: updateFrom, lte: updateTo }, + }, + data: { + orderIndex: isMovingUp ? { increment: 1 } : { decrement: 1 }, + }, + }); + + // Step 3: Update OrderIndex of collection + await tx.teamCollection.update({ + where: { id: collection.right.id }, + data: { + orderIndex: isMovingUp + ? subsequentCollectionInTx.orderIndex + : subsequentCollectionInTx.orderIndex - 1, + }, + }); + } catch (error) { + throw new ConflictException(error); + } }); this.pubsub.publish( diff --git a/packages/hoppscotch-backend/src/team-request/team-request.service.spec.ts b/packages/hoppscotch-backend/src/team-request/team-request.service.spec.ts index bc9dc64f..54df1fa0 100644 --- a/packages/hoppscotch-backend/src/team-request/team-request.service.spec.ts +++ b/packages/hoppscotch-backend/src/team-request/team-request.service.spec.ts @@ -19,19 +19,18 @@ import { Team as DbTeam, TeamCollection as DbTeamCollection, } from '@prisma/client'; +import { PubSubService } from 'src/pubsub/pubsub.service'; const mockPrisma = mockDeep(); const mockTeamService = mockDeep(); const mockTeamCollectionService = mockDeep(); -const mockPubSub = { publish: jest.fn().mockResolvedValue(null) }; +const mockPubSub = mockDeep(); -// eslint-disable-next-line @typescript-eslint/ban-ts-comment -// @ts-ignore const teamRequestService = new TeamRequestService( mockPrisma, - mockTeamService as any, - mockTeamCollectionService as any, - mockPubSub as any, + mockTeamService, + mockTeamCollectionService, + mockPubSub, ); const team: DbTeam = { @@ -241,9 +240,9 @@ describe('deleteTeamRequest', () => { describe('createTeamRequest', () => { test('rejects for invalid collection id', async () => { - mockTeamCollectionService.getTeamOfCollection.mockResolvedValue( - E.left(TEAM_INVALID_COLL_ID), - ); + jest + .spyOn(mockTeamCollectionService, 'getTeamOfCollection') + .mockResolvedValue(E.left(TEAM_INVALID_COLL_ID)); const response = await teamRequestService.createTeamRequest( 'invalidcollid', @@ -260,13 +259,17 @@ describe('createTeamRequest', () => { const dbRequest = dbTeamRequests[0]; const teamRequest = teamRequests[0]; - mockTeamCollectionService.getTeamOfCollection.mockResolvedValue( - E.right(team), - ); + jest + .spyOn(mockTeamCollectionService, 'getTeamOfCollection') + .mockResolvedValue(E.right(team)); + mockPrisma.$transaction.mockImplementation(async (fn) => { + return fn(mockPrisma); + }); + mockPrisma.teamRequest.findFirst.mockResolvedValue(null); mockPrisma.teamRequest.create.mockResolvedValue(dbRequest); const response = teamRequestService.createTeamRequest( - 'testcoll', + teamRequest.title, team.id, teamRequest.title, teamRequest.request, @@ -279,18 +282,21 @@ describe('createTeamRequest', () => { const dbRequest = dbTeamRequests[0]; const teamRequest = teamRequests[0]; - mockTeamCollectionService.getTeamOfCollection.mockResolvedValue( - E.right(team), - ); + jest + .spyOn(mockTeamCollectionService, 'getTeamOfCollection') + .mockResolvedValue(E.right(team)); + mockPrisma.$transaction.mockImplementation(async (fn) => { + return fn(mockPrisma); + }); + mockPrisma.teamRequest.findFirst.mockResolvedValue(null); mockPrisma.teamRequest.create.mockResolvedValue(dbRequest); await teamRequestService.createTeamRequest( - 'testcoll', + teamRequest.title, team.id, - 'Test Request', - '{}', + teamRequest.title, + teamRequest.request, ); - expect(mockPubSub.publish).toHaveBeenCalledWith( `team_req/${dbRequest.teamID}/req_created`, teamRequest, @@ -416,7 +422,7 @@ describe('reorderRequests', () => { const nextRequest = dbTeamRequests[4]; mockPrisma.$transaction.mockRejectedValueOnce(new Error()); - const result = await teamRequestService.reorderRequests( + const result = await (teamRequestService as any).reorderRequests( request, srcCollID, nextRequest, @@ -437,7 +443,7 @@ describe('reorderRequests', () => { }; mockPrisma.$transaction.mockResolvedValueOnce(E.right(updatedReq)); - const result = await teamRequestService.reorderRequests( + const result = await (teamRequestService as any).reorderRequests( request, srcCollID, nextRequest, @@ -461,7 +467,7 @@ describe('findRequestAndNextRequest', () => { .mockResolvedValueOnce(dbTeamRequests[0]) .mockResolvedValueOnce(dbTeamRequests[4]); - const result = await teamRequestService.findRequestAndNextRequest( + const result = await (teamRequestService as any).findRequestAndNextRequest( args.srcCollID, args.requestID, args.destCollID, @@ -485,7 +491,7 @@ describe('findRequestAndNextRequest', () => { .mockResolvedValueOnce(dbTeamRequests[0]) .mockResolvedValueOnce(null); - const result = teamRequestService.findRequestAndNextRequest( + const result = (teamRequestService as any).findRequestAndNextRequest( args.srcCollID, args.requestID, args.destCollID, @@ -507,7 +513,7 @@ describe('findRequestAndNextRequest', () => { mockPrisma.teamRequest.findFirst.mockResolvedValueOnce(null); - const result = teamRequestService.findRequestAndNextRequest( + const result = (teamRequestService as any).findRequestAndNextRequest( args.srcCollID, args.requestID, args.destCollID, @@ -528,7 +534,7 @@ describe('findRequestAndNextRequest', () => { .mockResolvedValueOnce(dbTeamRequests[0]) .mockResolvedValueOnce(null); - const result = teamRequestService.findRequestAndNextRequest( + const result = (teamRequestService as any).findRequestAndNextRequest( args.srcCollID, args.requestID, args.destCollID, @@ -549,12 +555,12 @@ describe('moveRequest', () => { }; jest - .spyOn(teamRequestService, 'findRequestAndNextRequest') + .spyOn(teamRequestService as any, 'findRequestAndNextRequest') .mockResolvedValue( E.right({ request: dbTeamRequests[0], nextRequest: null }), ); jest - .spyOn(teamRequestService, 'reorderRequests') + .spyOn(teamRequestService as any, 'reorderRequests') .mockResolvedValue(E.right(dbTeamRequests[0])); const result = teamRequestService.moveRequest( @@ -577,12 +583,12 @@ describe('moveRequest', () => { }; jest - .spyOn(teamRequestService, 'findRequestAndNextRequest') + .spyOn(teamRequestService as any, 'findRequestAndNextRequest') .mockResolvedValue( E.right({ request: dbTeamRequests[0], nextRequest: null }), ); jest - .spyOn(teamRequestService, 'reorderRequests') + .spyOn(teamRequestService as any, 'reorderRequests') .mockResolvedValue(E.right(dbTeamRequests[0])); await teamRequestService.moveRequest( @@ -608,12 +614,12 @@ describe('moveRequest', () => { }; jest - .spyOn(teamRequestService, 'findRequestAndNextRequest') + .spyOn(teamRequestService as any, 'findRequestAndNextRequest') .mockResolvedValue( E.right({ request: dbTeamRequests[0], nextRequest: null }), ); jest - .spyOn(teamRequestService, 'reorderRequests') + .spyOn(teamRequestService as any, 'reorderRequests') .mockResolvedValue(E.right(dbTeamRequests[0])); await teamRequestService.moveRequest( @@ -639,7 +645,7 @@ describe('moveRequest', () => { }; jest - .spyOn(teamRequestService, 'findRequestAndNextRequest') + .spyOn(teamRequestService as any, 'findRequestAndNextRequest') .mockResolvedValue(E.left(TEAM_REQ_NOT_FOUND)); expect( @@ -662,7 +668,7 @@ describe('moveRequest', () => { }; jest - .spyOn(teamRequestService, 'findRequestAndNextRequest') + .spyOn(teamRequestService as any, 'findRequestAndNextRequest') .mockResolvedValue(E.left(TEAM_REQ_INVALID_TARGET_COLL_ID)); expect( @@ -685,13 +691,13 @@ describe('moveRequest', () => { }; jest - .spyOn(teamRequestService, 'findRequestAndNextRequest') + .spyOn(teamRequestService as any, 'findRequestAndNextRequest') .mockResolvedValue( E.right({ request: dbTeamRequests[0], nextRequest: null }), ); jest - .spyOn(teamRequestService, 'reorderRequests') + .spyOn(teamRequestService as any, 'reorderRequests') .mockResolvedValue(E.left(TEAM_REQ_REORDERING_FAILED)); expect( diff --git a/packages/hoppscotch-backend/src/team-request/team-request.service.ts b/packages/hoppscotch-backend/src/team-request/team-request.service.ts index 5b639ecd..1c80d457 100644 --- a/packages/hoppscotch-backend/src/team-request/team-request.service.ts +++ b/packages/hoppscotch-backend/src/team-request/team-request.service.ts @@ -1,4 +1,4 @@ -import { Injectable } from '@nestjs/common'; +import { ConflictException, Injectable } from '@nestjs/common'; import { TeamService } from '../team/team.service'; import { PrismaService } from '../prisma/prisma.service'; import { TeamRequest } from './team-request.model'; @@ -9,6 +9,7 @@ import { TEAM_INVALID_ID, TEAM_REQ_NOT_FOUND, TEAM_REQ_REORDERING_FAILED, + TEAM_COLL_CREATION_FAILED, } from 'src/errors'; import { PubSubService } from 'src/pubsub/pubsub.service'; import { stringToJson } from 'src/utils'; @@ -111,13 +112,28 @@ export class TeamRequestService { }); if (!dbTeamReq) return E.left(TEAM_REQ_NOT_FOUND); - await this.prisma.teamRequest.updateMany({ - where: { orderIndex: { gte: dbTeamReq.orderIndex } }, - data: { orderIndex: { decrement: 1 } }, - }); - await this.prisma.teamRequest.delete({ - where: { id: requestID }, - }); + try { + await this.prisma.$transaction(async (tx) => { + try { + // lock the rows + await this.prisma.lockTableExclusive(tx, 'TeamRequest'); + + await tx.teamRequest.updateMany({ + where: { orderIndex: { gte: dbTeamReq.orderIndex } }, + data: { orderIndex: { decrement: 1 } }, + }); + + await tx.teamRequest.delete({ + where: { id: requestID }, + }); + } catch (error) { + throw new ConflictException(error); + } + }); + } catch (error) { + console.error('Error from TeamRequestService.deleteTeamRequest', error); + return E.left(TEAM_REQ_NOT_FOUND); + } this.pubsub.publish(`team_req/${dbTeamReq.teamID}/req_deleted`, requestID); @@ -142,26 +158,45 @@ export class TeamRequestService { if (E.isLeft(team)) return E.left(team.left); if (team.right.id !== teamID) return E.left(TEAM_INVALID_ID); - const reqCountInColl = - await this.getRequestsCountInCollection(collectionID); - - const createInput: Prisma.TeamRequestCreateInput = { - request: request, - title: title, - orderIndex: reqCountInColl + 1, - team: { connect: { id: team.right.id } }, - collection: { connect: { id: collectionID } }, - }; - + let jsonReq = null; if (request) { - const jsonReq = stringToJson(request); - if (E.isLeft(jsonReq)) return E.left(jsonReq.left); - createInput.request = jsonReq.right; + const parsedReq = stringToJson(request); + if (E.isLeft(parsedReq)) return E.left(parsedReq.left); + jsonReq = parsedReq.right; + } + + let dbTeamRequest: DbTeamRequest = null; + try { + dbTeamRequest = await this.prisma.$transaction(async (tx) => { + try { + // lock the rows + await this.prisma.lockTableExclusive(tx, 'TeamRequest'); + + // fetch last team request + const lastTeamRequest = await tx.teamRequest.findFirst({ + where: { collectionID }, + orderBy: { orderIndex: 'desc' }, + select: { orderIndex: true }, + }); + + // create the team request + return tx.teamRequest.create({ + data: { + request: jsonReq, + title, + orderIndex: lastTeamRequest ? lastTeamRequest.orderIndex + 1 : 1, + team: { connect: { id: team.right.id } }, + collection: { connect: { id: collectionID } }, + }, + }); + } catch (error) { + throw new ConflictException(error); + } + }); + } catch (error) { + return E.left(TEAM_COLL_CREATION_FAILED); } - const dbTeamRequest = await this.prisma.teamRequest.create({ - data: createInput, - }); const teamRequest = this.cast(dbTeamRequest); this.pubsub.publish( `team_req/${teamRequest.teamID}/req_created`, @@ -306,7 +341,7 @@ export class TeamRequestService { * @param destCollID Collection ID, where the request is to be moved to * @param nextRequestID ID of the request, which is after the request to be moved. If the request is to be moved to the end of the collection, nextRequestID should be null */ - async findRequestAndNextRequest( + private async findRequestAndNextRequest( srcCollID: string, requestID: string, destCollID: string, @@ -339,7 +374,7 @@ export class TeamRequestService { * A helper function to get the number of requests in a collection * @param collectionID Collection ID to fetch */ - async getRequestsCountInCollection(collectionID: string) { + private async getRequestsCountInCollection(collectionID: string) { return this.prisma.teamRequest.count({ where: { collectionID }, }); @@ -352,7 +387,7 @@ export class TeamRequestService { * @param nextRequest The request, which is after the request to be moved. If the request is to be moved to the end of the collection, nextRequest should be null * @param destCollID Collection ID, where the request is to be moved to */ - async reorderRequests( + private async reorderRequests( request: DbTeamRequest, srcCollID: string, nextRequest: DbTeamRequest, @@ -362,6 +397,21 @@ export class TeamRequestService { return await this.prisma.$transaction< E.Left | E.Right >(async (tx) => { + // lock the rows + await this.prisma.acquireLocks(tx, 'TeamRequest', null, null, [ + srcCollID, + destCollID, + ]); + + request = await tx.teamRequest.findUnique({ + where: { id: request.id }, + }); + nextRequest = nextRequest + ? await tx.teamRequest.findUnique({ + where: { id: nextRequest.id }, + }) + : null; + const isSameCollection = srcCollID === destCollID; const isMovingUp = nextRequest?.orderIndex < request.orderIndex; // false, if nextRequest is null diff --git a/packages/hoppscotch-backend/src/user-collection/user-collection.service.spec.ts b/packages/hoppscotch-backend/src/user-collection/user-collection.service.spec.ts index 00153c64..aea631d1 100644 --- a/packages/hoppscotch-backend/src/user-collection/user-collection.service.spec.ts +++ b/packages/hoppscotch-backend/src/user-collection/user-collection.service.spec.ts @@ -13,6 +13,8 @@ import { USER_NOT_OWNER, USER_COLL_DATA_INVALID, } from 'src/errors'; +import * as E from 'fp-ts/Either'; +import * as O from 'fp-ts/Option'; import { PrismaService } from 'src/prisma/prisma.service'; import { PubSubService } from 'src/pubsub/pubsub.service'; import { AuthUser } from 'src/types/AuthUser'; @@ -119,7 +121,7 @@ const childRESTUserCollectionCasted: UserCollection = { const childGQLUserCollection: DBUserCollection = { id: '234', orderIndex: 1, - parentID: rootRESTUserCollection.id, + parentID: rootGQLUserCollection.id, title: 'Child Collection 1', userUid: user.uid, type: ReqType.GQL, @@ -727,11 +729,14 @@ describe('createUserCollection', () => { }); test('should throw USER_NOT_OWNER when user is not the owner of the collection', async () => { - // isOwnerCheck - mockPrisma.userCollection.findUniqueOrThrow.mockResolvedValueOnce({ - ...rootRESTUserCollection, - userUid: 'othser-user-uid', - }); + jest + .spyOn(userCollectionService, 'getUserCollection') + .mockResolvedValueOnce( + E.right({ + ...rootRESTUserCollection, + userUid: 'other-user-uid', + }), + ); const result = await userCollectionService.createUserCollection( user, @@ -744,13 +749,8 @@ describe('createUserCollection', () => { }); test('should successfully create a new root REST user-collection with valid inputs', async () => { - // isOwnerCheck - mockPrisma.userCollection.findUniqueOrThrow.mockResolvedValueOnce( - rootRESTUserCollection, - ); - - //getRootCollectionsCount - mockPrisma.userCollection.findMany.mockResolvedValueOnce([]); + mockPrisma.$transaction.mockImplementation(async (fn) => fn(mockPrisma)); + mockPrisma.userCollection.findFirst.mockResolvedValueOnce(null); mockPrisma.userCollection.create.mockResolvedValueOnce( rootRESTUserCollection, ); @@ -759,20 +759,15 @@ describe('createUserCollection', () => { user, rootRESTUserCollection.title, JSON.stringify(rootRESTUserCollection.data), - rootRESTUserCollection.id, + null, ReqType.REST, ); expect(result).toEqualRight(rootRESTUserCollectionCasted); }); test('should successfully create a new root GQL user-collection with valid inputs', async () => { - // isOwnerCheck - mockPrisma.userCollection.findUniqueOrThrow.mockResolvedValueOnce( - rootGQLUserCollection, - ); - - //getRootCollectionsCount - mockPrisma.userCollection.findMany.mockResolvedValueOnce([]); + mockPrisma.$transaction.mockImplementation(async (fn) => fn(mockPrisma)); + mockPrisma.userCollection.findFirst.mockResolvedValueOnce(null); mockPrisma.userCollection.create.mockResolvedValueOnce( rootGQLUserCollection, ); @@ -781,20 +776,18 @@ describe('createUserCollection', () => { user, rootGQLUserCollection.title, JSON.stringify(rootGQLUserCollection.data), - rootGQLUserCollection.id, + null, ReqType.GQL, ); expect(result).toEqualRight(rootGQLUserCollectionCasted); }); test('should successfully create a new child REST user-collection with valid inputs', async () => { - // isOwnerCheck - mockPrisma.userCollection.findUniqueOrThrow.mockResolvedValueOnce( - childRESTUserCollection, - ); - - //getChildCollectionsCount - mockPrisma.userCollection.findMany.mockResolvedValueOnce([]); + jest + .spyOn(userCollectionService, 'getUserCollection') + .mockResolvedValueOnce(E.right(rootRESTUserCollection)); + mockPrisma.$transaction.mockImplementation(async (fn) => fn(mockPrisma)); + mockPrisma.userCollection.findFirst.mockResolvedValueOnce(null); mockPrisma.userCollection.create.mockResolvedValueOnce( childRESTUserCollection, ); @@ -803,20 +796,18 @@ describe('createUserCollection', () => { user, childRESTUserCollection.title, JSON.stringify(childRESTUserCollection.data), - childRESTUserCollection.id, + childRESTUserCollection.parentID, ReqType.REST, ); expect(result).toEqualRight(childRESTUserCollectionCasted); }); test('should successfully create a new child GQL user-collection with valid inputs', async () => { - // isOwnerCheck - mockPrisma.userCollection.findUniqueOrThrow.mockResolvedValueOnce( - childGQLUserCollection, - ); - - //getChildCollectionsCount - mockPrisma.userCollection.findMany.mockResolvedValueOnce([]); + jest + .spyOn(userCollectionService, 'getUserCollection') + .mockResolvedValueOnce(E.right(rootGQLUserCollection)); + mockPrisma.$transaction.mockImplementation(async (fn) => fn(mockPrisma)); + mockPrisma.userCollection.findFirst.mockResolvedValueOnce(null); mockPrisma.userCollection.create.mockResolvedValueOnce( childGQLUserCollection, ); @@ -825,20 +816,18 @@ describe('createUserCollection', () => { user, childGQLUserCollection.title, JSON.stringify(childGQLUserCollection.data), - childGQLUserCollection.id, + childGQLUserCollection.parentID, ReqType.GQL, ); expect(result).toEqualRight(childGQLUserCollectionCasted); }); test('should send pubsub message to "user_coll//created" if child REST user-collection is created successfully', async () => { - // isOwnerCheck - mockPrisma.userCollection.findUniqueOrThrow.mockResolvedValueOnce( - childRESTUserCollection, - ); - - //getChildCollectionsCount - mockPrisma.userCollection.findMany.mockResolvedValueOnce([]); + jest + .spyOn(userCollectionService, 'getUserCollection') + .mockResolvedValueOnce(E.right(rootRESTUserCollection)); + mockPrisma.$transaction.mockImplementation(async (fn) => fn(mockPrisma)); + mockPrisma.userCollection.findFirst.mockResolvedValueOnce(null); mockPrisma.userCollection.create.mockResolvedValueOnce( childRESTUserCollection, ); @@ -847,7 +836,7 @@ describe('createUserCollection', () => { user, childRESTUserCollection.title, JSON.stringify(childRESTUserCollection.data), - childRESTUserCollection.id, + childRESTUserCollection.parentID, ReqType.REST, ); expect(mockPubSub.publish).toHaveBeenCalledWith( @@ -857,13 +846,11 @@ describe('createUserCollection', () => { }); test('should send pubsub message to "user_coll//created" if child GQL user-collection is created successfully', async () => { - // isOwnerCheck - mockPrisma.userCollection.findUniqueOrThrow.mockResolvedValueOnce( - childGQLUserCollection, - ); - - //getChildCollectionsCount - mockPrisma.userCollection.findMany.mockResolvedValueOnce([]); + jest + .spyOn(userCollectionService, 'getUserCollection') + .mockResolvedValueOnce(E.right(rootGQLUserCollection)); + mockPrisma.$transaction.mockImplementation(async (fn) => fn(mockPrisma)); + mockPrisma.userCollection.findFirst.mockResolvedValueOnce(null); mockPrisma.userCollection.create.mockResolvedValueOnce( childGQLUserCollection, ); @@ -872,9 +859,10 @@ describe('createUserCollection', () => { user, childGQLUserCollection.title, JSON.stringify(childGQLUserCollection.data), - childGQLUserCollection.id, + childGQLUserCollection.parentID, ReqType.GQL, ); + expect(mockPubSub.publish).toHaveBeenCalledWith( `user_coll/${user.uid}/created`, childGQLUserCollectionCasted, @@ -882,13 +870,8 @@ describe('createUserCollection', () => { }); test('should send pubsub message to "user_coll//created" if REST root user-collection is created successfully', async () => { - // isOwnerCheck - mockPrisma.userCollection.findUniqueOrThrow.mockResolvedValueOnce( - rootRESTUserCollection, - ); - - //getRootCollectionsCount - mockPrisma.userCollection.findMany.mockResolvedValueOnce([]); + mockPrisma.$transaction.mockImplementation(async (fn) => fn(mockPrisma)); + mockPrisma.userCollection.findFirst.mockResolvedValueOnce(null); mockPrisma.userCollection.create.mockResolvedValueOnce( rootRESTUserCollection, ); @@ -897,7 +880,7 @@ describe('createUserCollection', () => { user, rootRESTUserCollection.title, JSON.stringify(rootRESTUserCollection.data), - rootRESTUserCollection.id, + null, ReqType.REST, ); expect(mockPubSub.publish).toHaveBeenCalledWith( @@ -907,13 +890,8 @@ describe('createUserCollection', () => { }); test('should send pubsub message to "user_coll//created" if GQL root user-collection is created successfully', async () => { - // isOwnerCheck - mockPrisma.userCollection.findUniqueOrThrow.mockResolvedValueOnce( - rootGQLUserCollection, - ); - - //getRootCollectionsCount - mockPrisma.userCollection.findMany.mockResolvedValueOnce([]); + mockPrisma.$transaction.mockImplementation(async (fn) => fn(mockPrisma)); + mockPrisma.userCollection.findFirst.mockResolvedValueOnce(null); mockPrisma.userCollection.create.mockResolvedValueOnce( rootGQLUserCollection, ); @@ -922,7 +900,7 @@ describe('createUserCollection', () => { user, rootGQLUserCollection.title, JSON.stringify(rootGQLUserCollection.data), - rootGQLUserCollection.id, + null, ReqType.GQL, ); expect(mockPubSub.publish).toHaveBeenCalledWith( @@ -1065,27 +1043,18 @@ describe('deleteUserCollection', () => { expect(result).toEqualLeft(USER_NOT_OWNER); }); test('should throw USER_COLL_NOT_FOUND when collectionID is invalid when deleting user-collection from UserCollectionTable ', async () => { - // getUserCollection - mockPrisma.userCollection.findUniqueOrThrow.mockResolvedValueOnce( - rootRESTUserCollection, - ); - // deleteCollectionData - // deleteCollectionData --> FindMany query 1st time - mockPrisma.userCollection.findMany.mockResolvedValueOnce([]); - // deleteCollectionData --> FindMany query 2nd time - mockPrisma.userCollection.findMany.mockResolvedValueOnce([]); - // deleteCollectionData --> DeleteMany query - mockPrisma.userRequest.deleteMany.mockResolvedValueOnce({ count: 0 }); - // deleteCollectionData --> updateOrderIndex - mockPrisma.userCollection.updateMany.mockResolvedValueOnce({ count: 0 }); - // deleteCollectionData --> removeUserCollection - mockPrisma.userCollection.delete.mockRejectedValueOnce('RecordNotFound'); + jest + .spyOn(userCollectionService, 'getUserCollection') + .mockResolvedValueOnce(E.right(rootRESTUserCollection)); + jest + .spyOn(userCollectionService as any, 'deleteCollectionData') + .mockResolvedValueOnce(E.left(USER_COLL_REORDERING_FAILED)); const result = await userCollectionService.deleteUserCollection( rootRESTUserCollection.id, user.uid, ); - expect(result).toEqualLeft(USER_COLL_NOT_FOUND); + expect(result).toEqualLeft(USER_COLL_REORDERING_FAILED); }); test('should send pubsub message to "user_coll//deleted" if user-collection is deleted successfully', async () => { // getUserCollection @@ -1251,25 +1220,14 @@ describe('moveUserCollection', () => { }); test('should successfully move a child user-collection into root', async () => { - // getUserCollection - mockPrisma.userCollection.findUniqueOrThrow.mockResolvedValueOnce( - childRESTUserCollection, - ); - // updateOrderIndex - mockPrisma.userCollection.updateMany.mockResolvedValueOnce({ count: 0 }); - // changeParent - // changeParent --> getRootCollectionsCount - mockPrisma.userCollection.findMany.mockResolvedValueOnce([ - rootRESTUserCollection, - ]); - mockPrisma.userCollection.findMany.mockResolvedValueOnce([ - rootRESTUserCollection, - ]); - mockPrisma.userCollection.update.mockResolvedValue({ - ...childRESTUserCollection, - parentID: null, - orderIndex: 2, - }); + jest + .spyOn(userCollectionService, 'getUserCollection') + .mockResolvedValueOnce(E.right(childRESTUserCollection)); + jest + .spyOn(userCollectionService as any, 'changeParentAndUpdateOrderIndex') + .mockResolvedValueOnce( + E.right({ ...childRESTUserCollection, parentID: null }), + ); const result = await userCollectionService.moveUserCollection( childRESTUserCollection.id, @@ -1283,21 +1241,12 @@ describe('moveUserCollection', () => { }); test('should throw USER_COLL_NOT_FOUND when trying to change parent of collection with invalid collectionID', async () => { - // getUserCollection - mockPrisma.userCollection.findUniqueOrThrow.mockResolvedValueOnce( - childRESTUserCollection, - ); - // updateOrderIndex - mockPrisma.userCollection.updateMany.mockResolvedValueOnce({ count: 0 }); - // changeParent - // changeParent --> getRootCollectionsCount - mockPrisma.userCollection.findMany.mockResolvedValueOnce([ - rootRESTUserCollection, - ]); - mockPrisma.userCollection.findMany.mockResolvedValueOnce([ - rootRESTUserCollection, - ]); - mockPrisma.userCollection.update.mockRejectedValueOnce('RecordNotFound'); + jest + .spyOn(userCollectionService, 'getUserCollection') + .mockResolvedValueOnce(E.right(childRESTUserCollection)); + jest + .spyOn(userCollectionService as any, 'changeParentAndUpdateOrderIndex') + .mockResolvedValueOnce(E.left(USER_COLL_NOT_FOUND)); const result = await userCollectionService.moveUserCollection( childRESTUserCollection.id, @@ -1308,25 +1257,14 @@ describe('moveUserCollection', () => { }); test('should send pubsub message to "user_coll//moved" when user-collection is moved to root successfully', async () => { - // getUserCollection - mockPrisma.userCollection.findUniqueOrThrow.mockResolvedValueOnce( - childRESTUserCollection, - ); - // updateOrderIndex - mockPrisma.userCollection.updateMany.mockResolvedValueOnce({ count: 0 }); - // changeParent - // changeParent --> getRootCollectionsCount - mockPrisma.userCollection.findMany.mockResolvedValueOnce([ - rootRESTUserCollection, - ]); - mockPrisma.userCollection.findMany.mockResolvedValueOnce([ - rootRESTUserCollection, - ]); - mockPrisma.userCollection.update.mockResolvedValue({ - ...childRESTUserCollection, - parentID: null, - orderIndex: 2, - }); + jest + .spyOn(userCollectionService, 'getUserCollection') + .mockResolvedValueOnce(E.right(childRESTUserCollection)); + jest + .spyOn(userCollectionService as any, 'changeParentAndUpdateOrderIndex') + .mockResolvedValueOnce( + E.right({ ...childRESTUserCollection, parentID: null }), + ); await userCollectionService.moveUserCollection( childRESTUserCollection.id, @@ -1343,33 +1281,23 @@ describe('moveUserCollection', () => { }); test('should successfully move a root user-collection into a child user-collection', async () => { - // getUserCollection - mockPrisma.userCollection.findUniqueOrThrow.mockResolvedValueOnce( - rootRESTUserCollection, - ); - // getUserCollection for destCollection - mockPrisma.userCollection.findUniqueOrThrow - .mockResolvedValueOnce(rootRESTUserCollection_2) - .mockResolvedValueOnce(null); - // isParent --> getUserCollection - mockPrisma.userCollection.findUnique.mockResolvedValueOnce( - childRESTUserCollection_2, - ); - // updateOrderIndex - mockPrisma.userCollection.updateMany.mockResolvedValueOnce({ count: 0 }); - // changeParent - // changeParent --> getRootCollectionsCount - mockPrisma.userCollection.findMany.mockResolvedValueOnce([ - rootRESTUserCollection, - ]); - mockPrisma.userCollection.findMany.mockResolvedValueOnce([ - rootRESTUserCollection, - ]); - mockPrisma.userCollection.update.mockResolvedValue({ - ...rootRESTUserCollection, - parentID: childRESTUserCollection_2.id, - orderIndex: 1, - }); + jest + .spyOn(userCollectionService, 'getUserCollection') + .mockResolvedValueOnce(E.right(rootRESTUserCollection)); + jest + .spyOn(userCollectionService, 'getUserCollection') + .mockResolvedValueOnce(E.right(childRESTUserCollection_2)); + jest + .spyOn(userCollectionService as any, 'isParent') + .mockResolvedValueOnce(O.some(true)); + jest + .spyOn(userCollectionService as any, 'changeParentAndUpdateOrderIndex') + .mockResolvedValueOnce( + E.right({ + ...rootRESTUserCollection, + parentID: childRESTUserCollection_2.id, + }), + ); const result = await userCollectionService.moveUserCollection( rootRESTUserCollection.id, @@ -1383,33 +1311,23 @@ describe('moveUserCollection', () => { }); test('should successfully move a child user-collection into another child user-collection', async () => { - // getUserCollection - mockPrisma.userCollection.findUniqueOrThrow.mockResolvedValueOnce( - rootRESTUserCollection, - ); - // getUserCollection for destCollection - mockPrisma.userCollection.findUniqueOrThrow - .mockResolvedValueOnce(rootRESTUserCollection_2) - .mockResolvedValueOnce(null); - // isParent --> getUserCollection - mockPrisma.userCollection.findUnique.mockResolvedValueOnce( - childRESTUserCollection, - ); - // updateOrderIndex - mockPrisma.userCollection.updateMany.mockResolvedValueOnce({ count: 0 }); - // changeParent - // changeParent --> getRootCollectionsCount - mockPrisma.userCollection.findMany.mockResolvedValueOnce([ - rootRESTUserCollection, - ]); - mockPrisma.userCollection.findMany.mockResolvedValueOnce([ - rootRESTUserCollection, - ]); - mockPrisma.userCollection.update.mockResolvedValue({ - ...rootRESTUserCollection, - parentID: childRESTUserCollection.id, - orderIndex: 1, - }); + jest + .spyOn(userCollectionService, 'getUserCollection') + .mockResolvedValueOnce(E.right(rootRESTUserCollection)); + jest + .spyOn(userCollectionService, 'getUserCollection') + .mockResolvedValueOnce(E.right(childRESTUserCollection_2)); + jest + .spyOn(userCollectionService as any, 'isParent') + .mockResolvedValueOnce(O.some(true)); + jest + .spyOn(userCollectionService as any, 'changeParentAndUpdateOrderIndex') + .mockResolvedValueOnce( + E.right({ + ...rootRESTUserCollection, + parentID: childRESTUserCollection.id, + }), + ); const result = await userCollectionService.moveUserCollection( rootRESTUserCollection.id, @@ -1423,33 +1341,23 @@ describe('moveUserCollection', () => { }); test('should send pubsub message to "user_coll//moved" when user-collection is moved into another child user-collection successfully', async () => { - // getUserCollection - mockPrisma.userCollection.findUniqueOrThrow.mockResolvedValueOnce( - rootRESTUserCollection, - ); - // getUserCollection for destCollection - mockPrisma.userCollection.findUniqueOrThrow - .mockResolvedValueOnce(rootRESTUserCollection_2) - .mockResolvedValueOnce(null); - // isParent --> getUserCollection - mockPrisma.userCollection.findUnique.mockResolvedValueOnce( - childRESTUserCollection, - ); - // updateOrderIndex - mockPrisma.userCollection.updateMany.mockResolvedValueOnce({ count: 0 }); - // changeParent - // changeParent --> getRootCollectionsCount - mockPrisma.userCollection.findMany.mockResolvedValueOnce([ - rootRESTUserCollection, - ]); - mockPrisma.userCollection.findMany.mockResolvedValueOnce([ - rootRESTUserCollection, - ]); - mockPrisma.userCollection.update.mockResolvedValue({ - ...rootRESTUserCollection, - parentID: childRESTUserCollection.id, - orderIndex: 1, - }); + jest + .spyOn(userCollectionService, 'getUserCollection') + .mockResolvedValueOnce(E.right(rootRESTUserCollection)); + jest + .spyOn(userCollectionService, 'getUserCollection') + .mockResolvedValueOnce(E.right(childRESTUserCollection_2)); + jest + .spyOn(userCollectionService as any, 'isParent') + .mockResolvedValueOnce(O.some(true)); + jest + .spyOn(userCollectionService as any, 'changeParentAndUpdateOrderIndex') + .mockResolvedValueOnce( + E.right({ + ...rootRESTUserCollection, + parentID: childRESTUserCollection.id, + }), + ); await userCollectionService.moveUserCollection( rootRESTUserCollection.id, diff --git a/packages/hoppscotch-backend/src/user-collection/user-collection.service.ts b/packages/hoppscotch-backend/src/user-collection/user-collection.service.ts index d1c24986..d8948400 100644 --- a/packages/hoppscotch-backend/src/user-collection/user-collection.service.ts +++ b/packages/hoppscotch-backend/src/user-collection/user-collection.service.ts @@ -1,4 +1,4 @@ -import { Injectable } from '@nestjs/common'; +import { ConflictException, Injectable } from '@nestjs/common'; import { USER_COLL_DEST_SAME, USER_COLL_IS_PARENT_COLL, @@ -13,6 +13,7 @@ import { USER_NOT_OWNER, USER_COLL_INVALID_JSON, USER_COLL_DATA_INVALID, + USER_COLLECTION_CREATION_FAILED, } from 'src/errors'; import { PrismaService } from 'src/prisma/prisma.service'; import { AuthUser } from 'src/types/AuthUser'; @@ -27,11 +28,13 @@ import { } from './user-collections.model'; import { ReqType } from 'src/types/RequestTypes'; import { + delay, isValidLength, stringToJson, transformCollectionData, } from 'src/utils'; import { CollectionFolder } from 'src/types/CollectionFolder'; +import { PrismaError } from 'src/prisma/prisma-error-codes'; @Injectable() export class UserCollectionService { @@ -41,6 +44,7 @@ export class UserCollectionService { ) {} TITLE_LENGTH = 1; + MAX_RETRIES = 5; // Maximum number of retries for database transactions /** * Typecast a database UserCollection to a UserCollection model @@ -60,42 +64,6 @@ export class UserCollectionService { }; } - /** - * Returns the count of child collections present for a given collectionID - * * The count returned is highest OrderIndex + 1 - * - * @param collectionID The Collection ID - * @returns Number of Child Collections - */ - private async getChildCollectionsCount(collectionID: string) { - const childCollectionCount = await this.prisma.userCollection.findMany({ - where: { parentID: collectionID }, - orderBy: { - orderIndex: 'desc', - }, - }); - if (!childCollectionCount.length) return 0; - return childCollectionCount[0].orderIndex; - } - - /** - * Returns the count of root collections present for a given userUID - * * The count returned is highest OrderIndex + 1 - * - * @param userID The User UID - * @returns Number of Root Collections - */ - private async getRootCollectionsCount(userID: string) { - const rootCollectionCount = await this.prisma.userCollection.findMany({ - where: { userUid: userID, parentID: null }, - orderBy: { - orderIndex: 'desc', - }, - }); - if (!rootCollectionCount.length) return 0; - return rootCollectionCount[0].orderIndex; - } - /** * Check to see if Collection belongs to User * @@ -205,11 +173,7 @@ export class UserCollectionService { async getUserCollection(collectionID: string) { try { const userCollection = await this.prisma.userCollection.findUniqueOrThrow( - { - where: { - id: collectionID, - }, - }, + { where: { id: collectionID } }, ); return E.right(userCollection); } catch (error) { @@ -222,7 +186,7 @@ export class UserCollectionService { * * @param user The User object * @param title The title of new UserCollection - * @param parentUserCollectionID The parent collectionID (null if root collection) + * @param parentID The parent collectionID (null if root collection) * @param type Type of Collection we want to create (REST/GQL) * @returns */ @@ -230,7 +194,7 @@ export class UserCollectionService { user: AuthUser, title: string, data: string | null = null, - parentUserCollectionID: string | null, + parentID: string | null, type: ReqType, ) { const isTitleValid = isValidLength(title, this.TITLE_LENGTH); @@ -244,10 +208,8 @@ export class UserCollectionService { } // If creating a child collection - if (parentUserCollectionID !== null) { - const parentCollection = await this.getUserCollection( - parentUserCollectionID, - ); + if (parentID !== null) { + const parentCollection = await this.getUserCollection(parentID); if (E.isLeft(parentCollection)) return E.left(parentCollection.left); // Check to see if parentUserCollectionID belongs to this User @@ -259,30 +221,44 @@ export class UserCollectionService { return E.left(USER_COLL_NOT_SAME_TYPE); } - const isParent = parentUserCollectionID - ? { - connect: { - id: parentUserCollectionID, - }, - } - : undefined; + let userCollection: UserCollection = null; + try { + userCollection = await this.prisma.$transaction(async (tx) => { + try { + // lock the rows + await this.prisma.lockTableExclusive(tx, 'UserCollection'); - const userCollection = await this.prisma.userCollection.create({ - data: { - title: title, - type: type, - user: { - connect: { - uid: user.uid, - }, - }, - parent: isParent, - data: data ?? undefined, - orderIndex: !parentUserCollectionID - ? (await this.getRootCollectionsCount(user.uid)) + 1 - : (await this.getChildCollectionsCount(parentUserCollectionID)) + 1, - }, - }); + // fetch last user collection + const lastUserCollection = await tx.userCollection.findFirst({ + where: { userUid: user.uid, parentID }, + orderBy: { orderIndex: 'desc' }, + select: { orderIndex: true }, + }); + + // create new user collection + return tx.userCollection.create({ + data: { + title: title, + type: type, + user: { connect: { uid: user.uid } }, + parent: parentID ? { connect: { id: parentID } } : undefined, + data: data ?? undefined, + orderIndex: lastUserCollection + ? lastUserCollection.orderIndex + 1 + : 1, + }, + }); + } catch (error) { + throw new ConflictException(error); + } + }); + } catch (error) { + console.error( + 'Error from UserCollectionService.createUserCollection', + error, + ); + return E.left(USER_COLLECTION_CREATION_FAILED); + } await this.pubsub.publish( `user_coll/${user.uid}/created`, @@ -384,12 +360,8 @@ export class UserCollectionService { try { const updatedUserCollection = await this.prisma.userCollection.update({ - where: { - id: userCollectionID, - }, - data: { - title: newTitle, - }, + where: { id: userCollectionID }, + data: { title: newTitle }, }); this.pubsub.publish( @@ -403,26 +375,6 @@ export class UserCollectionService { } } - /** - * Delete a UserCollection from the DB - * - * @param collectionID The Collection Id - * @returns The deleted UserCollection - */ - private async removeUserCollection(collectionID: string) { - try { - const deletedUserCollection = await this.prisma.userCollection.delete({ - where: { - id: collectionID, - }, - }); - - return E.right(deletedUserCollection); - } catch (error) { - return E.left(USER_COLL_NOT_FOUND); - } - } - /** * Delete child collection and requests of a UserCollection * @@ -452,26 +404,17 @@ export class UserCollectionService { }); // Update orderIndexes in userCollection table for user - await this.updateOrderIndex( - collection.parentID, + const isDeleted = await this.removeCollectionAndUpdateSiblingsOrderIndex( + collection, { gt: collection.orderIndex }, { decrement: 1 }, ); + if (E.isLeft(isDeleted)) return E.left(isDeleted.left); - // Delete collection from UserCollection table - const deletedUserCollection = await this.removeUserCollection( - collection.id, - ); - if (E.isLeft(deletedUserCollection)) - return E.left(deletedUserCollection.left); - - this.pubsub.publish( - `user_coll/${deletedUserCollection.right.userUid}/deleted`, - { - id: deletedUserCollection.right.id, - type: ReqType[deletedUserCollection.right.type], - }, - ); + this.pubsub.publish(`user_coll/${collection.userUid}/deleted`, { + id: collection.id, + type: ReqType[collection.type], + }); return E.right(true); } @@ -500,39 +443,60 @@ export class UserCollectionService { /** * Change parentID of UserCollection's - * - * @param collectionID The collection ID - * @param parentCollectionID The new parent's collection ID or change to root collection - * @returns If successful return an Either of true + + * @param collection The collection that is being moved + * @param newParentID The new parent's collection ID or change to root collection + * @returns If successful return an Either of collection or error message */ - private async changeParent( + private async changeParentAndUpdateOrderIndex( collection: UserCollection, - parentCollectionID: string | null, + newParentID: string | null, ) { + let updatedCollection: UserCollection = null; + try { - let collectionCount: number; + await this.prisma.$transaction(async (tx) => { + try { + // fetch last collection + const lastCollectionUnderNewParent = + await tx.userCollection.findFirst({ + where: { userUid: collection.userUid, parentID: newParentID }, + orderBy: { orderIndex: 'desc' }, + }); - if (!parentCollectionID) - collectionCount = await this.getRootCollectionsCount( - collection.userUid, - ); - collectionCount = await this.getChildCollectionsCount(parentCollectionID); + // update collection's parentID and orderIndex + updatedCollection = await tx.userCollection.update({ + where: { id: collection.id }, + data: { + // if parentCollectionID == null, collection becomes root collection + // if parentCollectionID != null, collection becomes child collection + parentID: newParentID, + orderIndex: lastCollectionUnderNewParent + ? lastCollectionUnderNewParent.orderIndex + 1 + : 1, + }, + }); - const updatedCollection = await this.prisma.userCollection.update({ - where: { - id: collection.id, - }, - data: { - // if parentCollectionID == null, collection becomes root collection - // if parentCollectionID != null, collection becomes child collection - parentID: parentCollectionID, - orderIndex: collectionCount + 1, - }, + // decrement orderIndex of all next sibling collections from original collection + await tx.userCollection.updateMany({ + where: { + parentID: collection.parentID, + orderIndex: { gt: collection.orderIndex }, + }, + data: { orderIndex: { decrement: 1 } }, + }); + } catch (error) { + throw new ConflictException(error); + } }); return E.right(updatedCollection); } catch (error) { - return E.left(USER_COLL_NOT_FOUND); + console.error( + 'Error from UserCollectionService.changeParentAndUpdateOrderIndex:', + error, + ); + return E.left(USER_COLL_REORDERING_FAILED); } } @@ -571,27 +535,63 @@ export class UserCollectionService { } /** - * Update the OrderIndex of all collections in given parentID - * - * @param parentID The Parent collectionID + * Delete collection and Update the OrderIndex of all collections in given parentID + * @param collection The collection to delete * @param orderIndexCondition Condition to decide what collections will be updated * @param dataCondition Increment/Decrement OrderIndex condition * @returns A Collection with updated OrderIndexes */ - private async updateOrderIndex( - parentID: string, + private async removeCollectionAndUpdateSiblingsOrderIndex( + collection: UserCollection, orderIndexCondition: Prisma.IntFilter, dataCondition: Prisma.IntFieldUpdateOperationsInput, ) { - const updatedUserCollection = await this.prisma.userCollection.updateMany({ - where: { - parentID: parentID, - orderIndex: orderIndexCondition, - }, - data: { orderIndex: dataCondition }, - }); + let retryCount = 0; + while (retryCount < this.MAX_RETRIES) { + try { + await this.prisma.$transaction(async (tx) => { + try { + // lock the rows + await this.prisma.lockTableExclusive(tx, 'UserCollection'); - return updatedUserCollection; + await tx.userCollection.delete({ + where: { id: collection.id }, + }); + + // update orderIndexes + await tx.userCollection.updateMany({ + where: { + parentID: collection.parentID, + orderIndex: orderIndexCondition, + }, + data: { orderIndex: dataCondition }, + }); + } catch (error) { + throw new ConflictException(error); + } + }); + + break; + } catch (error) { + console.error( + 'Error from UserCollectionService.updateOrderIndex:', + error, + ); + retryCount++; + if ( + retryCount >= this.MAX_RETRIES || + (error.code !== PrismaError.UNIQUE_CONSTRAINT_VIOLATION && + error.code !== PrismaError.TRANSACTION_DEADLOCK && + error.code !== PrismaError.TRANSACTION_TIMEOUT) // return for all DB error except deadlocks, unique constraint violations, transaction timeouts + ) + return E.left(USER_COLL_REORDERING_FAILED); + + await delay(retryCount * 100); + console.debug(`Retrying... (${retryCount})`); + } + } + + return E.right(true); } /** @@ -621,15 +621,13 @@ export class UserCollectionService { // Throw error if collection is already a root collection return E.left(USER_COLL_ALREADY_ROOT); } - // Move child collection into root and update orderIndexes for root userCollections - await this.updateOrderIndex( - collection.right.parentID, - { gt: collection.right.orderIndex }, - { decrement: 1 }, - ); // Change parent from child to root i.e child collection becomes a root collection - const updatedCollection = await this.changeParent(collection.right, null); + // Move child collection into root and update orderIndexes for child userCollections + const updatedCollection = await this.changeParentAndUpdateOrderIndex( + collection.right, + null, + ); if (E.isLeft(updatedCollection)) return E.left(updatedCollection.left); this.pubsub.publish( @@ -669,15 +667,9 @@ export class UserCollectionService { return E.left(USER_COLL_IS_PARENT_COLL); } - // Move root/child collection into another child collection and update orderIndexes of the previous parent - await this.updateOrderIndex( - collection.right.parentID, - { gt: collection.right.orderIndex }, - { decrement: 1 }, - ); - // Change parent from null to teamCollection i.e collection becomes a child collection - const updatedCollection = await this.changeParent( + // Move root/child collection into another child collection and update orderIndexes of the previous parent + const updatedCollection = await this.changeParentAndUpdateOrderIndex( collection.right, destCollection.right.id, ); @@ -731,27 +723,40 @@ export class UserCollectionService { // nextCollectionID == null i.e move collection to the end of the list try { await this.prisma.$transaction(async (tx) => { - // Step 1: Decrement orderIndex of all items that come after collection.orderIndex till end of list of items - await tx.userCollection.updateMany({ - where: { - parentID: collection.right.parentID, - orderIndex: { - gte: collection.right.orderIndex + 1, + try { + // Step 0: lock the rows + await this.prisma.acquireLocks( + tx, + 'UserCollection', + userID, + collection.right.parentID, + ); + + // Step 1: Decrement orderIndex of all items that come after collection.orderIndex till end of list of items + const collectionInTx = await tx.userCollection.findFirst({ + where: { id: collectionID }, + select: { orderIndex: true }, + }); + await tx.userCollection.updateMany({ + where: { + parentID: collection.right.parentID, + orderIndex: { gte: collectionInTx.orderIndex + 1 }, }, - }, - data: { - orderIndex: { decrement: 1 }, - }, - }); - // Step 2: Update orderIndex of collection to length of list - await tx.userCollection.update({ - where: { id: collection.right.id }, - data: { - orderIndex: await this.getCollectionCount( - collection.right.parentID, - ), - }, - }); + data: { orderIndex: { decrement: 1 } }, + }); + + // Step 2: Update orderIndex of collection to length of list + await tx.userCollection.update({ + where: { id: collection.right.id }, + data: { + orderIndex: await this.getCollectionCount( + collection.right.parentID, + ), + }, + }); + } catch (error) { + throw new ConflictException(error); + } }); this.pubsub.publish( @@ -783,36 +788,60 @@ export class UserCollectionService { try { await this.prisma.$transaction(async (tx) => { - // Step 1: Determine if we are moving collection up or down the list - const isMovingUp = - subsequentCollection.right.orderIndex < collection.right.orderIndex; - // Step 2: Update OrderIndex of items in list depending on moving up or down - const updateFrom = isMovingUp - ? subsequentCollection.right.orderIndex - : collection.right.orderIndex + 1; + try { + // Step 0: lock the rows + await this.prisma.acquireLocks( + tx, + 'UserCollection', + userID, + subsequentCollection.right.parentID, + ); - const updateTo = isMovingUp - ? collection.right.orderIndex - 1 - : subsequentCollection.right.orderIndex - 1; + // subsequentCollectionInTx and subsequentCollection are same, just to make sure, orderIndex value is concrete + const collectionInTx = await tx.userCollection.findFirst({ + where: { id: collectionID }, + select: { orderIndex: true }, + }); + const subsequentCollectionInTx = await tx.userCollection.findFirst({ + where: { id: nextCollectionID }, + select: { orderIndex: true }, + }); - await tx.userCollection.updateMany({ - where: { - parentID: collection.right.parentID, - orderIndex: { gte: updateFrom, lte: updateTo }, - }, - data: { - orderIndex: isMovingUp ? { increment: 1 } : { decrement: 1 }, - }, - }); - // Step 3: Update OrderIndex of collection - await tx.userCollection.update({ - where: { id: collection.right.id }, - data: { - orderIndex: isMovingUp - ? subsequentCollection.right.orderIndex - : subsequentCollection.right.orderIndex - 1, - }, - }); + // Step 1: Determine if we are moving collection up or down the list + const isMovingUp = + subsequentCollectionInTx.orderIndex < collectionInTx.orderIndex; + + // Step 2: Update OrderIndex of items in list depending on moving up or down + const updateFrom = isMovingUp + ? subsequentCollectionInTx.orderIndex + : collectionInTx.orderIndex + 1; + + const updateTo = isMovingUp + ? collectionInTx.orderIndex - 1 + : subsequentCollectionInTx.orderIndex - 1; + + await tx.userCollection.updateMany({ + where: { + parentID: collection.right.parentID, + orderIndex: { gte: updateFrom, lte: updateTo }, + }, + data: { + orderIndex: isMovingUp ? { increment: 1 } : { decrement: 1 }, + }, + }); + + // Step 3: Update OrderIndex of collection + await tx.userCollection.update({ + where: { id: collection.right.id }, + data: { + orderIndex: isMovingUp + ? subsequentCollectionInTx.orderIndex + : subsequentCollectionInTx.orderIndex - 1, + }, + }); + } catch (error) { + throw new ConflictException(error); + } }); this.pubsub.publish( @@ -991,8 +1020,20 @@ export class UserCollectionService { orderIndex: number, reqType: DBReqType, ): Prisma.UserCollectionCreateInput { + // Parse collection data if it exists + let data = null; + if (folder.data) { + try { + data = JSON.parse(folder.data); + } catch (error) { + // If data parsing fails, log error and continue without data + console.error('Failed to parse collection data:', error); + } + } + return { title: folder.name, + data, user: { connect: { uid: userID, @@ -1018,7 +1059,6 @@ export class UserCollectionService { this.generatePrismaQueryObj(f, userID, index + 1, reqType), ), }, - data: folder.data ?? undefined, }; } @@ -1029,6 +1069,7 @@ export class UserCollectionService { * @param userID The User ID * @param destCollectionID The Collection ID * @param reqType The Type of Collection + * @param isCollectionDuplication Boolean to publish collection create event on designated channel * @returns An Either of a Boolean if the creation operation was successful */ async importCollectionsFromJSON( @@ -1060,34 +1101,44 @@ export class UserCollectionService { return E.left(USER_COLL_NOT_SAME_TYPE); } - // Get number of root or child collections for destCollectionID(if destcollectionID != null) or destTeamID(if destcollectionID == null) - const count = !destCollectionID - ? await this.getRootCollectionsCount(userID) - : await this.getChildCollectionsCount(destCollectionID); + let userCollections: UserCollection[] = []; - // Generate Prisma Query Object for all child collections in collectionsList - const queryList = collectionsList.right.map((x) => - this.generatePrismaQueryObj(x, userID, count + 1, reqType), - ); + try { + await this.prisma.$transaction(async (tx) => { + try { + // lock the rows + await this.prisma.lockTableExclusive(tx, 'UserCollection'); - const parent = destCollectionID - ? { - connect: { - id: destCollectionID, - }, + // Get the last order index + const lastCollection = await tx.userCollection.findFirst({ + where: { userUid: userID, parentID: destCollectionID }, + orderBy: { orderIndex: 'desc' }, + }); + let lastOrderIndex = lastCollection ? lastCollection.orderIndex : 0; + + // Generate Prisma Query Object for all child collections in collectionsList + const queryList = collectionsList.right.map((x) => + this.generatePrismaQueryObj(x, userID, ++lastOrderIndex, reqType), + ); + + const parent = destCollectionID + ? { connect: { id: destCollectionID } } + : undefined; + + const promises = queryList.map((query) => + tx.userCollection.create({ + data: { ...query, parent }, + }), + ); + + userCollections = await Promise.all(promises); + } catch (error) { + throw new ConflictException(error); } - : undefined; - - const userCollections = await this.prisma.$transaction( - queryList.map((x) => - this.prisma.userCollection.create({ - data: { - ...x, - parent, - }, - }), - ), - ); + }); + } catch (error) { + return E.left(USER_COLLECTION_CREATION_FAILED); + } if (isCollectionDuplication) { const collectionData = await this.fetchCollectionData( diff --git a/packages/hoppscotch-backend/src/user-request/user-request.service.spec.ts b/packages/hoppscotch-backend/src/user-request/user-request.service.spec.ts index 2eb26bf9..3aefec33 100644 --- a/packages/hoppscotch-backend/src/user-request/user-request.service.spec.ts +++ b/packages/hoppscotch-backend/src/user-request/user-request.service.spec.ts @@ -11,12 +11,9 @@ import { import { PrismaService } from 'src/prisma/prisma.service'; import { PubSubService } from 'src/pubsub/pubsub.service'; import * as E from 'fp-ts/Either'; -import { GetUserRequestArgs } from './input-type.args'; +import { CreateUserRequestArgs, GetUserRequestArgs } from './input-type.args'; import { MoveUserRequestArgs } from './input-type.args'; -import { - CreateUserRequestArgs, - UpdateUserRequestArgs, -} from './input-type.args'; +import { UpdateUserRequestArgs } from './input-type.args'; import { UserRequest } from './user-request.model'; import { UserRequestService } from './user-request.service'; import { AuthUser } from 'src/types/AuthUser'; @@ -276,12 +273,10 @@ describe('UserRequestService', () => { type: userRequests[0].type, }; - mockPrisma.userRequest.count.mockResolvedValue( - dbUserRequests[0].orderIndex - 1, - ); mockUserCollectionService.getUserCollection.mockResolvedValue( E.right({ type: userRequests[0].type, userUid: user.uid } as any), ); + mockPrisma.$transaction.mockImplementation(async (fn) => fn(mockPrisma)); mockPrisma.userRequest.create.mockResolvedValue(dbUserRequests[0]); const result = userRequestService.createRequest( @@ -302,9 +297,10 @@ describe('UserRequestService', () => { type: userRequests[0].type, }; - mockPrisma.userRequest.count.mockResolvedValue( - dbUserRequests[0].orderIndex - 1, + mockUserCollectionService.getUserCollection.mockResolvedValue( + E.right({ type: userRequests[0].type, userUid: user.uid } as any), ); + mockPrisma.$transaction.mockImplementation(async (fn) => fn(mockPrisma)); mockPrisma.userRequest.create.mockResolvedValue(dbUserRequests[0]); await userRequestService.createRequest( @@ -332,10 +328,7 @@ describe('UserRequestService', () => { request: userRequests[0].request, type: userRequests[0].type, }; - - mockPrisma.userRequest.count.mockResolvedValue( - dbUserRequests[0].orderIndex - 1, - ); + mockPrisma.$transaction.mockImplementation(async (fn) => fn(mockPrisma)); mockPrisma.userRequest.create.mockResolvedValue(dbUserRequests[0]); await userRequestService.createRequest( @@ -358,10 +351,7 @@ describe('UserRequestService', () => { request: 'invalid json', type: userRequests[0].type, }; - - mockPrisma.userRequest.count.mockResolvedValue( - dbUserRequests[0].orderIndex - 1, - ); + mockPrisma.$transaction.mockImplementation(async (fn) => fn(mockPrisma)); mockPrisma.userRequest.create.mockResolvedValue(dbUserRequests[0]); const result = userRequestService.createRequest( @@ -499,7 +489,9 @@ describe('UserRequestService', () => { test('Should resolve right and delete user request', () => { const id = userRequests[0].id; + mockPrisma.$transaction.mockImplementation(async (fn) => fn(mockPrisma)); mockPrisma.userRequest.findFirst.mockResolvedValue(dbUserRequests[0]); + mockPrisma.userRequest.updateMany.mockResolvedValue(null); mockPrisma.userRequest.delete.mockResolvedValue(dbUserRequests[0]); const result = userRequestService.deleteRequest(id, user); @@ -509,8 +501,10 @@ describe('UserRequestService', () => { test('Should resolve right and perform prisma.delete with correct param', async () => { const id = userRequests[0].id; + mockPrisma.$transaction.mockImplementation(async (fn) => fn(mockPrisma)); mockPrisma.userRequest.findFirst.mockResolvedValue(dbUserRequests[0]); - mockPrisma.userRequest.delete.mockResolvedValue(null); + mockPrisma.userRequest.updateMany.mockResolvedValue(null); + mockPrisma.userRequest.delete.mockResolvedValue(dbUserRequests[0]); await userRequestService.deleteRequest(id, user); @@ -521,8 +515,10 @@ describe('UserRequestService', () => { test('Should resolve right and perform prisma.updateMany with correct param', async () => { const id = userRequests[0].id; + mockPrisma.$transaction.mockImplementation(async (fn) => fn(mockPrisma)); mockPrisma.userRequest.findFirst.mockResolvedValue(dbUserRequests[0]); - mockPrisma.userRequest.delete.mockResolvedValue(null); + mockPrisma.userRequest.updateMany.mockResolvedValue(null); + mockPrisma.userRequest.delete.mockResolvedValue(dbUserRequests[0]); await userRequestService.deleteRequest(id, user); @@ -537,8 +533,10 @@ describe('UserRequestService', () => { test('Should resolve and publish message to pubnub', async () => { const id = userRequests[0].id; + mockPrisma.$transaction.mockImplementation(async (fn) => fn(mockPrisma)); mockPrisma.userRequest.findFirst.mockResolvedValue(dbUserRequests[0]); - mockPrisma.userRequest.delete.mockResolvedValue(null); + mockPrisma.userRequest.updateMany.mockResolvedValue(null); + mockPrisma.userRequest.delete.mockResolvedValue(dbUserRequests[0]); await userRequestService.deleteRequest(id, user); @@ -571,7 +569,9 @@ describe('UserRequestService', () => { const nextRequest = dbUserRequests[4]; mockPrisma.$transaction.mockRejectedValueOnce(new Error()); - const result = await userRequestService.reorderRequests( + jest.spyOn(console, 'error').mockImplementation(() => undefined); + + const result = await (userRequestService as any).reorderRequests( srcCollID, request, destCollID, @@ -592,7 +592,7 @@ describe('UserRequestService', () => { }; mockPrisma.$transaction.mockResolvedValueOnce(E.right(updatedReq)); - const result = await userRequestService.reorderRequests( + const result = await (userRequestService as any).reorderRequests( srcCollID, request, destCollID, @@ -715,7 +715,7 @@ describe('UserRequestService', () => { E.right({ request: dbUserRequests[0], nextRequest: null }), ); jest - .spyOn(userRequestService, 'reorderRequests') + .spyOn(userRequestService as any, 'reorderRequests') .mockResolvedValue(E.right(dbUserRequests[0])); jest .spyOn(userRequestService, 'validateTypeEqualityForMoveRequest') @@ -745,7 +745,7 @@ describe('UserRequestService', () => { E.right({ request: dbUserRequests[0], nextRequest: null }), ); jest - .spyOn(userRequestService, 'reorderRequests') + .spyOn(userRequestService as any, 'reorderRequests') .mockResolvedValue(E.right(dbUserRequests[0])); jest .spyOn(userRequestService, 'validateTypeEqualityForMoveRequest') @@ -806,7 +806,7 @@ describe('UserRequestService', () => { }), ); jest - .spyOn(userRequestService, 'reorderRequests') + .spyOn(userRequestService as any, 'reorderRequests') .mockResolvedValue(E.left(USER_REQUEST_REORDERING_FAILED)); jest .spyOn(userRequestService, 'validateTypeEqualityForMoveRequest') diff --git a/packages/hoppscotch-backend/src/user-request/user-request.service.ts b/packages/hoppscotch-backend/src/user-request/user-request.service.ts index e0e952b4..30a9e17a 100644 --- a/packages/hoppscotch-backend/src/user-request/user-request.service.ts +++ b/packages/hoppscotch-backend/src/user-request/user-request.service.ts @@ -1,4 +1,4 @@ -import { Injectable } from '@nestjs/common'; +import { ConflictException, Injectable } from '@nestjs/common'; import { PrismaService } from '../prisma/prisma.service'; import { PubSubService } from '../pubsub/pubsub.service'; import * as E from 'fp-ts/Either'; @@ -130,32 +130,41 @@ export class UserRequestService { if (collection.right.type !== ReqType[type]) return E.left(USER_REQUEST_INVALID_TYPE); + let newRequest: DbUserRequest = null; try { - const requestCount = - await this.getRequestsCountInCollection(collectionID); + newRequest = await this.prisma.$transaction(async (tx) => { + try { + // lock the rows + await this.prisma.lockTableExclusive(tx, 'UserRequest'); - const request = await this.prisma.userRequest.create({ - data: { - collectionID, - title, - request: jsonRequest.right, - type: ReqType[type], - orderIndex: requestCount + 1, - userUid: user.uid, - }, + // fetch last user request + const lastUserRequest = await tx.userRequest.findFirst({ + where: { userUid: user.uid, collectionID }, + orderBy: { orderIndex: 'desc' }, + }); + + return tx.userRequest.create({ + data: { + collectionID, + title, + request: jsonRequest.right, + type: ReqType[type], + orderIndex: lastUserRequest ? lastUserRequest.orderIndex + 1 : 1, + userUid: user.uid, + }, + }); + } catch (error) { + throw new ConflictException(error); + } }); - - const userRequest = this.cast(request); - - await this.pubsub.publish( - `user_request/${user.uid}/created`, - userRequest, - ); - - return E.right(userRequest); - } catch (err) { + } catch (error) { + console.error('Error from UserRequestService.createRequest', error); return E.left(USER_REQUEST_CREATION_FAILED); } + + const userRequest = this.cast(newRequest); + await this.pubsub.publish(`user_request/${user.uid}/created`, userRequest); + return E.right(userRequest); } /** @@ -218,14 +227,28 @@ export class UserRequestService { }); if (!request) return E.left(USER_REQUEST_NOT_FOUND); - await this.prisma.userRequest.updateMany({ - where: { - collectionID: request.collectionID, - orderIndex: { gt: request.orderIndex }, - }, - data: { orderIndex: { decrement: 1 } }, - }); - await this.prisma.userRequest.delete({ where: { id } }); + try { + await this.prisma.$transaction(async (tx) => { + try { + // lock the rows + await this.prisma.lockTableExclusive(tx, 'UserRequest'); + + await tx.userRequest.updateMany({ + where: { + collectionID: request.collectionID, + orderIndex: { gt: request.orderIndex }, + }, + data: { orderIndex: { decrement: 1 } }, + }); + + await tx.userRequest.delete({ where: { id } }); + } catch (error) { + throw new ConflictException(error); + } + }); + } catch (error) { + return E.left(USER_REQUEST_NOT_FOUND); + } await this.pubsub.publish( `user_request/${user.uid}/deleted`, @@ -337,13 +360,13 @@ export class UserRequestService { srcCollID: string, destCollID: string, requestID: string, - nextRequestID: string, + nextRequestID: string | null, user: AuthUser, ): Promise< | E.Left | E.Right<{ request: DbUserRequest; - nextRequest: DbUserRequest; + nextRequest: DbUserRequest | null; }> > { const request = await this.prisma.userRequest.findFirst({ @@ -374,7 +397,7 @@ export class UserRequestService { * @param nextRequest - request that comes after the updated request in its new position * @returns Promise of an Either of `DbUserRequest` object or error message */ - async reorderRequests( + private async reorderRequests( srcCollID: string, request: DbUserRequest, destCollID: string, @@ -384,6 +407,21 @@ export class UserRequestService { return await this.prisma.$transaction< E.Left | E.Right >(async (tx) => { + // lock the rows + await this.prisma.acquireLocks(tx, 'UserRequest', null, null, [ + request.id, + nextRequest?.id, + ]); + + request = await tx.userRequest.findUnique({ + where: { id: request.id }, + }); + nextRequest = nextRequest + ? await tx.userRequest.findUnique({ + where: { id: nextRequest.id }, + }) + : null; + const isSameCollection = srcCollID === destCollID; const isMovingUp = nextRequest?.orderIndex < request.orderIndex; // false, if nextRequest is null @@ -443,7 +481,8 @@ export class UserRequestService { return E.right(updatedRequest); }); - } catch (err) { + } catch (error) { + console.error('Error from UserRequestService.reorderRequests', error); return E.left(USER_REQUEST_REORDERING_FAILED); } } diff --git a/packages/hoppscotch-backend/src/utils.ts b/packages/hoppscotch-backend/src/utils.ts index 9ccf1f88..a8e618f2 100644 --- a/packages/hoppscotch-backend/src/utils.ts +++ b/packages/hoppscotch-backend/src/utils.ts @@ -13,6 +13,15 @@ import { TeamAccessRole } from './team/team.model'; import { RESTError } from './types/RESTError'; import * as crypto from 'crypto'; +/** + * Delays the execution for a given number of milliseconds. + * @param ms The number of milliseconds to delay + * @returns A promise that resolves after the delay + */ +export function delay(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + /** * A workaround to throw an exception in an expression. * JS throw keyword creates a statement not an expression. diff --git a/packages/hoppscotch-common/src/helpers/auth/types/__tests__/bearer.spec.ts b/packages/hoppscotch-common/src/helpers/auth/types/__tests__/bearer.spec.ts index 27ba0f71..0bb59756 100644 --- a/packages/hoppscotch-common/src/helpers/auth/types/__tests__/bearer.spec.ts +++ b/packages/hoppscotch-common/src/helpers/auth/types/__tests__/bearer.spec.ts @@ -1,7 +1,7 @@ -import { describe, test, expect } from "vitest" -import { generateBearerAuthHeaders } from "../bearer" -import { createBaseRequest, mockEnvVars } from "./test-utils" import { HoppRESTAuth } from "@hoppscotch/data" +import { describe, expect, test } from "vitest" +import { generateBearerAuthHeaders } from "../bearer" +import { mockEnvVars } from "./test-utils" describe("Bearer Auth", () => { describe("generateBearerAuthHeaders", () => { diff --git a/packages/hoppscotch-common/src/helpers/auth/types/__tests__/jwt.spec.ts b/packages/hoppscotch-common/src/helpers/auth/types/__tests__/jwt.spec.ts index 032709f6..6477ac84 100644 --- a/packages/hoppscotch-common/src/helpers/auth/types/__tests__/jwt.spec.ts +++ b/packages/hoppscotch-common/src/helpers/auth/types/__tests__/jwt.spec.ts @@ -1,7 +1,7 @@ -import { describe, test, expect, vi, beforeEach } from "vitest" -import { generateJwtAuthHeaders } from "../jwt" -import { createBaseRequest, mockEnvVars } from "./test-utils" import { HoppRESTAuth } from "@hoppscotch/data" +import { beforeEach, describe, expect, test, vi } from "vitest" +import { generateJwtAuthHeaders } from "../jwt" +import { mockEnvVars } from "./test-utils" // Mock the jwt helper vi.mock("@hoppscotch/data", async () => { diff --git a/packages/hoppscotch-common/src/helpers/auth/types/__tests__/oauth2.spec.ts b/packages/hoppscotch-common/src/helpers/auth/types/__tests__/oauth2.spec.ts index 22f27975..0b38ef5f 100644 --- a/packages/hoppscotch-common/src/helpers/auth/types/__tests__/oauth2.spec.ts +++ b/packages/hoppscotch-common/src/helpers/auth/types/__tests__/oauth2.spec.ts @@ -1,7 +1,7 @@ -import { describe, test, expect } from "vitest" -import { generateOAuth2AuthHeaders } from "../oauth2" -import { createBaseRequest, mockEnvVars } from "./test-utils" import { HoppRESTAuth } from "@hoppscotch/data" +import { describe, expect, test } from "vitest" +import { generateOAuth2AuthHeaders } from "../oauth2" +import { mockEnvVars } from "./test-utils" describe("OAuth2 Auth", () => { describe("generateOAuth2AuthHeaders", () => { diff --git a/packages/hoppscotch-selfhost-desktop/src/api/mutations/ImportUserCollectionsFromJSON.graphql b/packages/hoppscotch-selfhost-desktop/src/api/mutations/ImportUserCollectionsFromJSON.graphql new file mode 100644 index 00000000..0fa6a920 --- /dev/null +++ b/packages/hoppscotch-selfhost-desktop/src/api/mutations/ImportUserCollectionsFromJSON.graphql @@ -0,0 +1,11 @@ +mutation ImportUserCollectionsFromJSON( + $jsonString: String! + $reqType: ReqType! + $parentCollectionID: ID +) { + importUserCollectionsFromJSON( + jsonString: $jsonString + reqType: $reqType + parentCollectionID: $parentCollectionID + ) +} diff --git a/packages/hoppscotch-selfhost-desktop/src/platform/collections/collections.api.ts b/packages/hoppscotch-selfhost-desktop/src/platform/collections/collections.api.ts index 340d045f..d08af2b4 100644 --- a/packages/hoppscotch-selfhost-desktop/src/platform/collections/collections.api.ts +++ b/packages/hoppscotch-selfhost-desktop/src/platform/collections/collections.api.ts @@ -37,6 +37,9 @@ import { GetUserRootCollectionsDocument, GetUserRootCollectionsQuery, GetUserRootCollectionsQueryVariables, + ImportUserCollectionsFromJsonDocument, + ImportUserCollectionsFromJsonMutation, + ImportUserCollectionsFromJsonMutationVariables, MoveUserCollectionDocument, MoveUserCollectionMutation, MoveUserCollectionMutationVariables, @@ -295,6 +298,21 @@ export const exportUserCollectionsToJSON = ( variables: { collectionID, collectionType }, }) +export const importUserCollectionsFromJSON = ( + jsonString: string, + reqType: ReqType, + parentCollectionID?: string +) => + runMutation< + ImportUserCollectionsFromJsonMutation, + ImportUserCollectionsFromJsonMutationVariables, + "" + >(ImportUserCollectionsFromJsonDocument, { + jsonString, + reqType, + parentCollectionID, + })() + export const runUserCollectionCreatedSubscription = () => runGQLSubscription({ query: UserCollectionCreatedDocument, variables: {} }) diff --git a/packages/hoppscotch-selfhost-desktop/src/platform/collections/collections.sync.ts b/packages/hoppscotch-selfhost-desktop/src/platform/collections/collections.sync.ts index 3a368e41..38705f60 100644 --- a/packages/hoppscotch-selfhost-desktop/src/platform/collections/collections.sync.ts +++ b/packages/hoppscotch-selfhost-desktop/src/platform/collections/collections.sync.ts @@ -26,6 +26,7 @@ import { deleteUserCollection, deleteUserRequest, editUserRequest, + importUserCollectionsFromJSON, moveUserCollection, moveUserRequest, renameUserCollection, @@ -33,6 +34,8 @@ import { updateUserCollectionOrder, } from "./collections.api" +import { ReqType } from "../../api/generated/graphql" + import * as E from "fp-ts/Either" // restCollectionsMapper uses the collectionPath as the local identifier @@ -168,6 +171,26 @@ const recursivelySyncCollections = async ( }) } +// Helper function to transform HoppCollection to backend format +const transformCollectionForBackend = (collection: HoppCollection): any => { + const data = { + auth: collection.auth ?? { + authType: "inherit", + authActive: true, + }, + headers: collection.headers ?? [], + variables: collection.variables ?? [], + _ref_id: collection._ref_id, + } + + return { + name: collection.name, + data: JSON.stringify(data), + folders: collection.folders.map(transformCollectionForBackend), + requests: collection.requests, + } +} + // TODO: generalize this // TODO: ask backend to send enough info on the subscription to not need this export const collectionReorderOrMovingOperations: { @@ -192,13 +215,25 @@ export const restCollectionsOperations: Array = [] export const storeSyncDefinition: StoreSyncDefinitionOf< typeof restCollectionStore > = { - appendCollections({ entries }) { - let indexStart = restCollectionStore.value.state.length - entries.length + async appendCollections({ entries }) { + // Transform collections to backend format + const transformedCollections = entries.map(transformCollectionForBackend) - entries.forEach((collection) => { - recursivelySyncCollections(collection, `${indexStart}`) - indexStart++ - }) + // Use bulk import API for better performance + const result = await importUserCollectionsFromJSON( + JSON.stringify(transformedCollections), + ReqType.Rest, + undefined // parentCollectionID is undefined for root collections + ) + + if (E.isLeft(result)) { + // Fallback to individual creation if bulk import fails + let indexStart = restCollectionStore.value.state.length - entries.length + entries.forEach((collection) => { + recursivelySyncCollections(collection, `${indexStart}`) + indexStart++ + }) + } }, async addCollection({ collection }) { const lastCreatedCollectionIndex = @@ -237,8 +272,6 @@ export const storeSyncDefinition: StoreSyncDefinitionOf< const parentCollectionBackendID = parentCollection?.id if (parentCollectionBackendID) { - const foldersLength = parentCollection.folders.length - const res = await createRESTChildUserCollection( name, parentCollectionBackendID @@ -247,12 +280,20 @@ export const storeSyncDefinition: StoreSyncDefinitionOf< if (E.isRight(res)) { const { id } = res.right.createRESTChildUserCollection - if (foldersLength) { - parentCollection.folders[foldersLength - 1].id = id - removeDuplicateRESTCollectionOrFolder( - id, - `${path}/${foldersLength - 1}` - ) + // Always try to assign the ID to the last created folder + const foldersLength = parentCollection.folders.length + if (foldersLength > 0) { + const lastFolderIndex = foldersLength - 1 + const lastFolder = parentCollection.folders[lastFolderIndex] + + // Only assign ID if the folder doesn't already have one (avoid overwriting) + if (!lastFolder.id) { + lastFolder.id = id + removeDuplicateRESTCollectionOrFolder( + id, + `${path}/${lastFolderIndex}` + ) + } } } } @@ -330,8 +371,6 @@ export const storeSyncDefinition: StoreSyncDefinitionOf< const parentCollectionBackendID = folder?.id if (parentCollectionBackendID) { - const newRequest = folder.requests[folder.requests.length - 1] - const res = await createRESTUserRequest( (request as HoppRESTRequest).name, JSON.stringify(request), @@ -341,12 +380,22 @@ export const storeSyncDefinition: StoreSyncDefinitionOf< if (E.isRight(res)) { const { id } = res.right.createRESTUserRequest - newRequest.id = id - removeDuplicateRESTCollectionOrFolder( - id, - `${path}/${folder.requests.length - 1}`, - "request" - ) + // Find the last request that doesn't have an ID (the newly added one) + const requestsLength = folder.requests.length + if (requestsLength > 0) { + const lastRequestIndex = requestsLength - 1 + const lastRequest = folder.requests[lastRequestIndex] + + // Only assign ID if the request doesn't already have one + if (!lastRequest.id) { + lastRequest.id = id + removeDuplicateRESTCollectionOrFolder( + id, + `${path}/${lastRequestIndex}`, + "request" + ) + } + } } } }, diff --git a/packages/hoppscotch-selfhost-desktop/src/platform/collections/gqlCollections.sync.ts b/packages/hoppscotch-selfhost-desktop/src/platform/collections/gqlCollections.sync.ts index 923840cd..775eb01a 100644 --- a/packages/hoppscotch-selfhost-desktop/src/platform/collections/gqlCollections.sync.ts +++ b/packages/hoppscotch-selfhost-desktop/src/platform/collections/gqlCollections.sync.ts @@ -25,11 +25,34 @@ import { deleteUserCollection, deleteUserRequest, editGQLUserRequest, + importUserCollectionsFromJSON, renameUserCollection, updateUserCollection, } from "./collections.api" +import { ReqType } from "../../api/generated/graphql" import * as E from "fp-ts/Either" + +// Helper function to transform HoppCollection to backend format +const transformCollectionForBackend = (collection: HoppCollection): any => { + const data = { + auth: collection.auth ?? { + authType: "inherit", + authActive: true, + }, + headers: collection.headers ?? [], + variables: collection.variables ?? [], + _ref_id: collection._ref_id, + } + + return { + name: collection.name, + data: JSON.stringify(data), + folders: collection.folders.map(transformCollectionForBackend), + requests: collection.requests, + } +} + import { moveOrReorderRequests } from "./collections.sync" // gqlCollectionsMapper uses the collectionPath as the local identifier @@ -193,13 +216,27 @@ export const gqlCollectionsOperations: Array = [] export const storeSyncDefinition: StoreSyncDefinitionOf< typeof graphqlCollectionStore > = { - appendCollections({ entries }) { - let indexStart = graphqlCollectionStore.value.state.length - entries.length + async appendCollections({ entries }) { + // Transform collections to backend format + const transformedCollections = entries.map(transformCollectionForBackend) - entries.forEach((collection) => { - recursivelySyncCollections(collection, `${indexStart}`) - indexStart++ - }) + // Use bulk import API for better performance + const result = await importUserCollectionsFromJSON( + JSON.stringify(transformedCollections), + ReqType.Gql, + undefined // parentCollectionID is undefined for root collections + ) + + if (E.isLeft(result)) { + // Fallback to individual creation if bulk import fails + let indexStart = + graphqlCollectionStore.value.state.length - entries.length + + entries.forEach((collection) => { + recursivelySyncCollections(collection, `${indexStart}`) + indexStart++ + }) + } }, async addCollection({ collection }) { const lastCreatedCollectionIndex = diff --git a/packages/hoppscotch-selfhost-web/src/api/mutations/ImportUserCollectionsFromJSON.graphql b/packages/hoppscotch-selfhost-web/src/api/mutations/ImportUserCollectionsFromJSON.graphql new file mode 100644 index 00000000..0fa6a920 --- /dev/null +++ b/packages/hoppscotch-selfhost-web/src/api/mutations/ImportUserCollectionsFromJSON.graphql @@ -0,0 +1,11 @@ +mutation ImportUserCollectionsFromJSON( + $jsonString: String! + $reqType: ReqType! + $parentCollectionID: ID +) { + importUserCollectionsFromJSON( + jsonString: $jsonString + reqType: $reqType + parentCollectionID: $parentCollectionID + ) +} diff --git a/packages/hoppscotch-selfhost-web/src/platform/collections/desktop/api.ts b/packages/hoppscotch-selfhost-web/src/platform/collections/desktop/api.ts index a1d8c80c..66ac4ab9 100644 --- a/packages/hoppscotch-selfhost-web/src/platform/collections/desktop/api.ts +++ b/packages/hoppscotch-selfhost-web/src/platform/collections/desktop/api.ts @@ -40,6 +40,9 @@ import { GetUserRootCollectionsDocument, GetUserRootCollectionsQuery, GetUserRootCollectionsQueryVariables, + ImportUserCollectionsFromJsonDocument, + ImportUserCollectionsFromJsonMutation, + ImportUserCollectionsFromJsonMutationVariables, MoveUserCollectionDocument, MoveUserCollectionMutation, MoveUserCollectionMutationVariables, @@ -311,6 +314,21 @@ export const exportUserCollectionsToJSON = ( variables: { collectionID, collectionType }, }) +export const importUserCollectionsFromJSON = ( + jsonString: string, + reqType: ReqType, + parentCollectionID?: string +) => + runMutation< + ImportUserCollectionsFromJsonMutation, + ImportUserCollectionsFromJsonMutationVariables, + "" + >(ImportUserCollectionsFromJsonDocument, { + jsonString, + reqType, + parentCollectionID, + })() + export const runUserCollectionCreatedSubscription = () => runGQLSubscription({ query: UserCollectionCreatedDocument, variables: {} }) diff --git a/packages/hoppscotch-selfhost-web/src/platform/collections/desktop/gqlCollections.sync.ts b/packages/hoppscotch-selfhost-web/src/platform/collections/desktop/gqlCollections.sync.ts index 2a56d87f..dde51c3d 100644 --- a/packages/hoppscotch-selfhost-web/src/platform/collections/desktop/gqlCollections.sync.ts +++ b/packages/hoppscotch-selfhost-web/src/platform/collections/desktop/gqlCollections.sync.ts @@ -26,6 +26,7 @@ import { deleteUserRequest, duplicateUserCollection, editGQLUserRequest, + importUserCollectionsFromJSON, updateUserCollection, } from "./api" @@ -34,13 +35,33 @@ import { ReqType } from "@api/generated/graphql" import { moveOrReorderRequests } from "./sync" // gqlCollectionsMapper uses the collectionPath as the local identifier +// Helper function to transform HoppCollection to backend format +const transformCollectionForBackend = (collection: HoppCollection): any => { + const data = { + auth: collection.auth ?? { + authType: "inherit", + authActive: true, + }, + headers: collection.headers ?? [], + variables: collection.variables ?? [], + _ref_id: collection._ref_id, + } + + return { + name: collection.name, + data: JSON.stringify(data), + folders: collection.folders.map(transformCollectionForBackend), + requests: collection.requests, + } +} + export const gqlCollectionsMapper = createMapper() // gqlRequestsMapper uses the collectionPath/requestIndex as the local identifier export const gqlRequestsMapper = createMapper() -// temp implementation until the backend implements an endpoint that accepts an entire collection -// TODO: use importCollectionsJSON to do this +// Optimized implementation using importUserCollectionsFromJSON for bulk operations +// This replaces individual createGQLRootUserCollection/createGQLChildUserCollection/createGQLUserRequest calls const recursivelySyncCollections = async ( collection: HoppCollection, collectionPath: string, @@ -194,15 +215,37 @@ export const gqlCollectionsOperations: Array = [] export const storeSyncDefinition: StoreSyncDefinitionOf< typeof graphqlCollectionStore > = { - appendCollections({ entries }) { - let indexStart = graphqlCollectionStore.value.state.length - entries.length + async appendCollections({ entries }) { + if (entries.length === 0) return - entries.forEach((collection) => { - recursivelySyncCollections(collection, `${indexStart}`) - indexStart++ - }) + // Transform collections to backend format + const transformedCollections = entries.map(transformCollectionForBackend) + + // Use the bulk import API instead of individual calls + const jsonString = JSON.stringify(transformedCollections) + + const result = await importUserCollectionsFromJSON( + jsonString, + ReqType.Gql, + undefined // undefined for root collections + ) + + // The backend handles creating all collections and requests in a single transaction + // The frontend collections will be updated through subscriptions + + if (E.isLeft(result)) { + // Fallback to individual calls if bulk import fails + let indexStart = + graphqlCollectionStore.value.state.length - entries.length + + entries.forEach((collection) => { + recursivelySyncCollections(collection, `${indexStart}`) + indexStart++ + }) + } }, async addCollection({ collection }) { + // Use individual API for single collection creation (not import) const lastCreatedCollectionIndex = graphqlCollectionStore.value.state.length - 1 diff --git a/packages/hoppscotch-selfhost-web/src/platform/collections/desktop/sync.ts b/packages/hoppscotch-selfhost-web/src/platform/collections/desktop/sync.ts index a4efc42f..a749392d 100644 --- a/packages/hoppscotch-selfhost-web/src/platform/collections/desktop/sync.ts +++ b/packages/hoppscotch-selfhost-web/src/platform/collections/desktop/sync.ts @@ -25,6 +25,7 @@ import { deleteUserRequest, duplicateUserCollection, editUserRequest, + importUserCollectionsFromJSON, moveUserCollection, moveUserRequest, updateUserCollection, @@ -35,13 +36,33 @@ import * as E from "fp-ts/Either" import { ReqType } from "@api/generated/graphql" // restCollectionsMapper uses the collectionPath as the local identifier +// Helper function to transform HoppCollection to backend format +const transformCollectionForBackend = (collection: HoppCollection): any => { + const data = { + auth: collection.auth ?? { + authType: "inherit", + authActive: true, + }, + headers: collection.headers ?? [], + variables: collection.variables ?? [], + _ref_id: collection._ref_id, + } + + return { + name: collection.name, + data: JSON.stringify(data), + folders: collection.folders.map(transformCollectionForBackend), + requests: collection.requests, + } +} + export const restCollectionsMapper = createMapper() // restRequestsMapper uses the collectionPath/requestIndex as the local identifier export const restRequestsMapper = createMapper() -// temp implementation until the backend implements an endpoint that accepts an entire collection -// TODO: use importCollectionsJSON to do this +// Optimized implementation using importUserCollectionsFromJSON for bulk operations +// This replaces individual createRESTRootUserCollection/createRESTChildUserCollection/createRESTUserRequest calls const recursivelySyncCollections = async ( collection: HoppCollection, collectionPath: string, @@ -189,15 +210,36 @@ export const restCollectionsOperations: Array = [] export const storeSyncDefinition: StoreSyncDefinitionOf< typeof restCollectionStore > = { - appendCollections({ entries }) { - let indexStart = restCollectionStore.value.state.length - entries.length + async appendCollections({ entries }) { + if (entries.length === 0) return - entries.forEach((collection) => { - recursivelySyncCollections(collection, `${indexStart}`) - indexStart++ - }) + // Transform collections to backend format + const transformedCollections = entries.map(transformCollectionForBackend) + + // Use the bulk import API instead of individual calls + const jsonString = JSON.stringify(transformedCollections) + + const result = await importUserCollectionsFromJSON( + jsonString, + ReqType.Rest, + undefined // undefined for root collections + ) + + // The backend handles creating all collections and requests in a single transaction + // The frontend collections will be updated through subscriptions + + if (E.isLeft(result)) { + // Fallback to individual calls if bulk import fails + let indexStart = restCollectionStore.value.state.length - entries.length + + entries.forEach((collection) => { + recursivelySyncCollections(collection, `${indexStart}`) + indexStart++ + }) + } }, async addCollection({ collection }) { + // Use individual API for single collection creation (not import) const lastCreatedCollectionIndex = restCollectionStore.value.state.length - 1 @@ -234,8 +276,6 @@ export const storeSyncDefinition: StoreSyncDefinitionOf< const parentCollectionBackendID = parentCollection?.id if (parentCollectionBackendID) { - const foldersLength = parentCollection.folders.length - const res = await createRESTChildUserCollection( name, parentCollectionBackendID @@ -244,12 +284,20 @@ export const storeSyncDefinition: StoreSyncDefinitionOf< if (E.isRight(res)) { const { id } = res.right.createRESTChildUserCollection - if (foldersLength) { - parentCollection.folders[foldersLength - 1].id = id - removeDuplicateRESTCollectionOrFolder( - id, - `${path}/${foldersLength - 1}` - ) + // Always try to assign the ID to the last created folder + const foldersLength = parentCollection.folders.length + if (foldersLength > 0) { + const lastFolderIndex = foldersLength - 1 + const lastFolder = parentCollection.folders[lastFolderIndex] + + // Only assign ID if the folder doesn't already have one (avoid overwriting) + if (!lastFolder.id) { + lastFolder.id = id + removeDuplicateRESTCollectionOrFolder( + id, + `${path}/${lastFolderIndex}` + ) + } } } } @@ -331,8 +379,6 @@ export const storeSyncDefinition: StoreSyncDefinitionOf< const parentCollectionBackendID = folder?.id if (parentCollectionBackendID) { - const newRequest = folder.requests[folder.requests.length - 1] - const res = await createRESTUserRequest( (request as HoppRESTRequest).name, JSON.stringify(request), @@ -342,12 +388,22 @@ export const storeSyncDefinition: StoreSyncDefinitionOf< if (E.isRight(res)) { const { id } = res.right.createRESTUserRequest - newRequest.id = id - removeDuplicateRESTCollectionOrFolder( - id, - `${path}/${folder.requests.length - 1}`, - "request" - ) + // Find the last request that doesn't have an ID (the newly added one) + const requestsLength = folder.requests.length + if (requestsLength > 0) { + const lastRequestIndex = requestsLength - 1 + const lastRequest = folder.requests[lastRequestIndex] + + // Only assign ID if the request doesn't already have one + if (!lastRequest.id) { + lastRequest.id = id + removeDuplicateRESTCollectionOrFolder( + id, + `${path}/${lastRequestIndex}`, + "request" + ) + } + } } } }, diff --git a/packages/hoppscotch-selfhost-web/src/platform/collections/web/api.ts b/packages/hoppscotch-selfhost-web/src/platform/collections/web/api.ts index a1d8c80c..66ac4ab9 100644 --- a/packages/hoppscotch-selfhost-web/src/platform/collections/web/api.ts +++ b/packages/hoppscotch-selfhost-web/src/platform/collections/web/api.ts @@ -40,6 +40,9 @@ import { GetUserRootCollectionsDocument, GetUserRootCollectionsQuery, GetUserRootCollectionsQueryVariables, + ImportUserCollectionsFromJsonDocument, + ImportUserCollectionsFromJsonMutation, + ImportUserCollectionsFromJsonMutationVariables, MoveUserCollectionDocument, MoveUserCollectionMutation, MoveUserCollectionMutationVariables, @@ -311,6 +314,21 @@ export const exportUserCollectionsToJSON = ( variables: { collectionID, collectionType }, }) +export const importUserCollectionsFromJSON = ( + jsonString: string, + reqType: ReqType, + parentCollectionID?: string +) => + runMutation< + ImportUserCollectionsFromJsonMutation, + ImportUserCollectionsFromJsonMutationVariables, + "" + >(ImportUserCollectionsFromJsonDocument, { + jsonString, + reqType, + parentCollectionID, + })() + export const runUserCollectionCreatedSubscription = () => runGQLSubscription({ query: UserCollectionCreatedDocument, variables: {} }) diff --git a/packages/hoppscotch-selfhost-web/src/platform/collections/web/gqlCollections.sync.ts b/packages/hoppscotch-selfhost-web/src/platform/collections/web/gqlCollections.sync.ts index 002c63a8..94520b02 100644 --- a/packages/hoppscotch-selfhost-web/src/platform/collections/web/gqlCollections.sync.ts +++ b/packages/hoppscotch-selfhost-web/src/platform/collections/web/gqlCollections.sync.ts @@ -26,6 +26,7 @@ import { deleteUserRequest, duplicateUserCollection, editGQLUserRequest, + importUserCollectionsFromJSON, updateUserCollection, } from "./api" @@ -34,13 +35,33 @@ import { ReqType } from "@api/generated/graphql" import { moveOrReorderRequests } from "./sync" // gqlCollectionsMapper uses the collectionPath as the local identifier +// Helper function to transform HoppCollection to backend format +const transformCollectionForBackend = (collection: HoppCollection): any => { + const data = { + auth: collection.auth ?? { + authType: "inherit", + authActive: true, + }, + headers: collection.headers ?? [], + variables: collection.variables ?? [], + _ref_id: collection._ref_id, + } + + return { + name: collection.name, + data: JSON.stringify(data), + folders: collection.folders.map(transformCollectionForBackend), + requests: collection.requests, + } +} + export const gqlCollectionsMapper = createMapper() // gqlRequestsMapper uses the collectionPath/requestIndex as the local identifier export const gqlRequestsMapper = createMapper() -// temp implementation until the backend implements an endpoint that accepts an entire collection -// TODO: use importCollectionsJSON to do this +// Optimized implementation using importUserCollectionsFromJSON for bulk operations +// This replaces individual createGQLRootUserCollection/createGQLChildUserCollection/createGQLUserRequest calls const recursivelySyncCollections = async ( collection: HoppCollection, collectionPath: string, @@ -194,15 +215,37 @@ export const gqlCollectionsOperations: Array = [] export const storeSyncDefinition: StoreSyncDefinitionOf< typeof graphqlCollectionStore > = { - appendCollections({ entries }) { - let indexStart = graphqlCollectionStore.value.state.length - entries.length + async appendCollections({ entries }) { + if (entries.length === 0) return - entries.forEach((collection) => { - recursivelySyncCollections(collection, `${indexStart}`) - indexStart++ - }) + // Transform collections to backend format + const transformedCollections = entries.map(transformCollectionForBackend) + + // Use the bulk import API instead of individual calls + const jsonString = JSON.stringify(transformedCollections) + + const result = await importUserCollectionsFromJSON( + jsonString, + ReqType.Gql, + undefined // undefined for root collections + ) + + // The backend handles creating all collections and requests in a single transaction + // The frontend collections will be updated through subscriptions + + if (E.isLeft(result)) { + // Fallback to individual calls if bulk import fails + let indexStart = + graphqlCollectionStore.value.state.length - entries.length + + entries.forEach((collection) => { + recursivelySyncCollections(collection, `${indexStart}`) + indexStart++ + }) + } }, async addCollection({ collection }) { + // Use individual API for single collection creation (not import) const lastCreatedCollectionIndex = graphqlCollectionStore.value.state.length - 1 diff --git a/packages/hoppscotch-selfhost-web/src/platform/collections/web/sync.ts b/packages/hoppscotch-selfhost-web/src/platform/collections/web/sync.ts index 3cc07e40..afe55fff 100644 --- a/packages/hoppscotch-selfhost-web/src/platform/collections/web/sync.ts +++ b/packages/hoppscotch-selfhost-web/src/platform/collections/web/sync.ts @@ -25,6 +25,7 @@ import { deleteUserRequest, duplicateUserCollection, editUserRequest, + importUserCollectionsFromJSON, moveUserCollection, moveUserRequest, updateUserCollection, @@ -35,13 +36,33 @@ import * as E from "fp-ts/Either" import { ReqType } from "@api/generated/graphql" // restCollectionsMapper uses the collectionPath as the local identifier +// Helper function to transform HoppCollection to backend format +const transformCollectionForBackend = (collection: HoppCollection): any => { + const data = { + auth: collection.auth ?? { + authType: "inherit", + authActive: true, + }, + headers: collection.headers ?? [], + variables: collection.variables ?? [], + _ref_id: collection._ref_id, + } + + return { + name: collection.name, + data: JSON.stringify(data), + folders: collection.folders.map(transformCollectionForBackend), + requests: collection.requests, + } +} + export const restCollectionsMapper = createMapper() // restRequestsMapper uses the collectionPath/requestIndex as the local identifier export const restRequestsMapper = createMapper() -// temp implementation until the backend implements an endpoint that accepts an entire collection -// TODO: use importCollectionsJSON to do this +// Optimized implementation using importUserCollectionsFromJSON for bulk operations +// This replaces individual createRESTRootUserCollection/createRESTChildUserCollection/createRESTUserRequest calls const recursivelySyncCollections = async ( collection: HoppCollection, collectionPath: string, @@ -192,15 +213,36 @@ export const restCollectionsOperations: Array = [] export const storeSyncDefinition: StoreSyncDefinitionOf< typeof restCollectionStore > = { - appendCollections({ entries }) { - let indexStart = restCollectionStore.value.state.length - entries.length + async appendCollections({ entries }) { + if (entries.length === 0) return - entries.forEach((collection) => { - recursivelySyncCollections(collection, `${indexStart}`) - indexStart++ - }) + // Transform collections to backend format + const transformedCollections = entries.map(transformCollectionForBackend) + + // Use the bulk import API instead of individual calls + const jsonString = JSON.stringify(transformedCollections) + + const result = await importUserCollectionsFromJSON( + jsonString, + ReqType.Rest, + undefined // undefined for root collections + ) + + // The backend handles creating all collections and requests in a single transaction + // The frontend collections will be updated through subscriptions + + if (E.isLeft(result)) { + // Fallback to individual calls if bulk import fails + let indexStart = restCollectionStore.value.state.length - entries.length + + entries.forEach((collection) => { + recursivelySyncCollections(collection, `${indexStart}`) + indexStart++ + }) + } }, async addCollection({ collection }) { + // Use individual API for single collection creation (not import) const lastCreatedCollectionIndex = restCollectionStore.value.state.length - 1 @@ -237,8 +279,6 @@ export const storeSyncDefinition: StoreSyncDefinitionOf< const parentCollectionBackendID = parentCollection?.id if (parentCollectionBackendID) { - const foldersLength = parentCollection.folders.length - const res = await createRESTChildUserCollection( name, parentCollectionBackendID @@ -247,12 +287,20 @@ export const storeSyncDefinition: StoreSyncDefinitionOf< if (E.isRight(res)) { const { id } = res.right.createRESTChildUserCollection - if (foldersLength) { - parentCollection.folders[foldersLength - 1].id = id - removeDuplicateRESTCollectionOrFolder( - id, - `${path}/${foldersLength - 1}` - ) + // Always try to assign the ID to the last created folder + const foldersLength = parentCollection.folders.length + if (foldersLength > 0) { + const lastFolderIndex = foldersLength - 1 + const lastFolder = parentCollection.folders[lastFolderIndex] + + // Only assign ID if the folder doesn't already have one (avoid overwriting) + if (!lastFolder.id) { + lastFolder.id = id + removeDuplicateRESTCollectionOrFolder( + id, + `${path}/${lastFolderIndex}` + ) + } } } } @@ -334,8 +382,6 @@ export const storeSyncDefinition: StoreSyncDefinitionOf< const parentCollectionBackendID = folder?.id if (parentCollectionBackendID) { - const newRequest = folder.requests[folder.requests.length - 1] - const res = await createRESTUserRequest( (request as HoppRESTRequest).name, JSON.stringify(request), @@ -345,12 +391,22 @@ export const storeSyncDefinition: StoreSyncDefinitionOf< if (E.isRight(res)) { const { id } = res.right.createRESTUserRequest - newRequest.id = id - removeDuplicateRESTCollectionOrFolder( - id, - `${path}/${folder.requests.length - 1}`, - "request" - ) + // Find the last request that doesn't have an ID (the newly added one) + const requestsLength = folder.requests.length + if (requestsLength > 0) { + const lastRequestIndex = requestsLength - 1 + const lastRequest = folder.requests[lastRequestIndex] + + // Only assign ID if the request doesn't already have one + if (!lastRequest.id) { + lastRequest.id = id + removeDuplicateRESTCollectionOrFolder( + id, + `${path}/${lastRequestIndex}`, + "request" + ) + } + } } } },