From 1501798d9c9b90afb7df6a020a376f14fcb45088 Mon Sep 17 00:00:00 2001 From: Justin Gasper Date: Fri, 31 Oct 2025 15:13:49 +1100 Subject: [PATCH 1/2] Performance tweaks for large account in prod --- .../20251104090500_add_review_ordering_indexes/migration.sql | 3 +++ .../migration.sql | 3 +++ prisma/schema.prisma | 3 +++ 3 files changed, 9 insertions(+) create mode 100644 prisma/migrations/20251104090500_add_review_ordering_indexes/migration.sql create mode 100644 prisma/migrations/20251105000000_add_review_item_comment_indexes/migration.sql diff --git a/prisma/migrations/20251104090500_add_review_ordering_indexes/migration.sql b/prisma/migrations/20251104090500_add_review_ordering_indexes/migration.sql new file mode 100644 index 0000000..e067935 --- /dev/null +++ b/prisma/migrations/20251104090500_add_review_ordering_indexes/migration.sql @@ -0,0 +1,3 @@ +-- Improve review lookups that combine submission/phase filters with ordering +CREATE INDEX "review_submissionId_id_idx" ON "review"("submissionId", "id"); +CREATE INDEX "review_phaseId_id_idx" ON "review"("phaseId", "id"); diff --git a/prisma/migrations/20251105000000_add_review_item_comment_indexes/migration.sql b/prisma/migrations/20251105000000_add_review_item_comment_indexes/migration.sql new file mode 100644 index 0000000..f59d310 --- /dev/null +++ b/prisma/migrations/20251105000000_add_review_item_comment_indexes/migration.sql @@ -0,0 +1,3 @@ +-- Add a composite index to speed up paginated lookups by review item +CREATE INDEX IF NOT EXISTS "reviewItemComment_reviewItemId_sortOrder_id_idx" + ON "reviews"."reviewItemComment"("reviewItemId", "sortOrder", "id"); diff --git a/prisma/schema.prisma b/prisma/schema.prisma index eb41225..ee53157 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -180,8 +180,10 @@ model review { @@index([committed]) // Index for filtering by committed status @@index([submissionId]) // Index for filtering by submission + @@index([submissionId, id]) // Helps ORDER BY id after filtering by submission @@index([resourceId]) // Index for filtering by resource (reviewer) @@index([phaseId]) // Index for filtering by phase + @@index([phaseId, id]) // Helps ORDER BY id after filtering by phase @@index([scorecardId]) // Index for joining with scorecard table @@index([status]) // Index for filtering by review status @@index([status, phaseId]) @@ -228,6 +230,7 @@ model reviewItemComment { appeal appeal? @@index([reviewItemId]) // Index for joining with reviewItem table + @@index([reviewItemId, sortOrder, id]) // Helps ordered pagination on review item comments @@index([id]) // Index for direct ID lookups @@index([resourceId]) // Index for filtering by resource (commenter) @@index([type]) // Index for filtering by comment type From 477d1a838aeff27fa3517e8377013085f4250593 Mon Sep 17 00:00:00 2001 From: Justin Gasper Date: Fri, 31 Oct 2025 21:30:52 +1100 Subject: [PATCH 2/2] Handling of isFileSubmission flag, as well as the ability to import it from legacy data --- prisma/migrate.ts | 294 +++++++++++++++++- .../migration.sql | 3 + prisma/schema.prisma | 1 + src/api/submission/submission.controller.ts | 7 +- src/api/submission/submission.service.ts | 104 ++++++- src/dto/submission.dto.ts | 7 + 6 files changed, 412 insertions(+), 4 deletions(-) create mode 100644 prisma/migrations/20250218000100_add_submission_is_file_submission/migration.sql diff --git a/prisma/migrate.ts b/prisma/migrate.ts index c678cad..ce29aa7 100644 --- a/prisma/migrate.ts +++ b/prisma/migrate.ts @@ -1,4 +1,4 @@ -import { PrismaClient } from '@prisma/client'; +import { PrismaClient, Prisma } from '@prisma/client'; import * as path from 'path'; import * as fs from 'fs'; import * as readline from 'readline'; @@ -33,6 +33,7 @@ const DEFAULT_DATA_DIR = '/mnt/export/review_tables'; const DATA_DIR = process.env.DATA_DIR || DEFAULT_DATA_DIR; const batchSize = 1000; const logSize = 20000; +const submissionIsFileBatchSize = 500; const DEFAULT_ES_DATA_FILE = path.join( '/home/ubuntu', 'submissions-api.data.json', @@ -64,6 +65,55 @@ if (shouldRepairMaps) { ); } +const MIGRATION_TARGET_SUBMISSION_IS_FILE = 'submission-is-file'; +const SUPPORTED_MIGRATION_TARGETS = new Set([ + MIGRATION_TARGET_SUBMISSION_IS_FILE, +]); +const MIGRATION_TARGET_ALIASES: Record = { + [MIGRATION_TARGET_SUBMISSION_IS_FILE]: MIGRATION_TARGET_SUBMISSION_IS_FILE, + 'submission-is-file-flag': MIGRATION_TARGET_SUBMISSION_IS_FILE, + 'submission-isfilesubmission': MIGRATION_TARGET_SUBMISSION_IS_FILE, + 'submission:isfile': MIGRATION_TARGET_SUBMISSION_IS_FILE, + 'submission:isfilesubmission': MIGRATION_TARGET_SUBMISSION_IS_FILE, + 'is-file-submission': MIGRATION_TARGET_SUBMISSION_IS_FILE, + submission_is_file: MIGRATION_TARGET_SUBMISSION_IS_FILE, +}; + +const rawTargets = extractTargets(process.argv.slice(2)); +const normalizedTargets: string[] = []; +const unknownTargets: string[] = []; + +for (const target of rawTargets) { + const normalized = normalizeTarget(target); + if (normalized) { + normalizedTargets.push(normalized); + } else { + unknownTargets.push(target); + } +} + +if (unknownTargets.length > 0) { + throw new Error( + `Unknown migration target(s): ${unknownTargets.join(', ')}. Supported targets: ${Array.from(SUPPORTED_MIGRATION_TARGETS).join(', ')}`, + ); +} + +const requestedTargets = new Set(normalizedTargets); +const runFullMigration = requestedTargets.size === 0; +if (!runFullMigration && requestedTargets.size > 1) { + throw new Error( + `Multiple migration targets are not currently supported. Requested targets: ${Array.from(requestedTargets).join(', ')}`, + ); +} +const runSubmissionIsFileTarget = requestedTargets.has( + MIGRATION_TARGET_SUBMISSION_IS_FILE, +); +if (!runFullMigration) { + console.log( + `Executing targeted migration: ${Array.from(requestedTargets).join(', ')}`, + ); +} + const errorSummary = new Map< string, { count: number; files: Set; examples: string[] } @@ -108,6 +158,73 @@ const shouldProcessRecord = ( ); }; +const toBoolean = (value: unknown): boolean => { + if (typeof value === 'boolean') { + return value; + } + if (typeof value === 'number') { + if (value === 1) { + return true; + } + if (value === 0) { + return false; + } + } + if (typeof value === 'string') { + const normalized = value.trim().toLowerCase(); + if (['true', '1', 'yes', 'y'].includes(normalized)) { + return true; + } + if ( + ['false', '0', 'no', 'n', 'null', 'undefined', ''].includes(normalized) + ) { + return false; + } + } + return Boolean(value); +}; + +function extractTargets(args: string[]): string[] { + const results: string[] = []; + for (let i = 0; i < args.length; i += 1) { + const arg = args[i]; + if (arg === '--target' || arg === '--targets') { + const value = args[i + 1]; + if (!value) { + throw new Error( + `${arg} requires a value with comma-separated migration target names.`, + ); + } + results.push( + ...value + .split(',') + .map((item) => item.trim()) + .filter((item) => item.length > 0), + ); + i += 1; + continue; + } + const match = arg.match(/^--targets?=(.*)$/); + if (match) { + results.push( + ...match[1] + .split(',') + .map((item) => item.trim()) + .filter((item) => item.length > 0), + ); + } + } + return results; +} + +function normalizeTarget(target: string): string | undefined { + if (!target) { + return undefined; + } + const normalized = target.trim().toLowerCase(); + return MIGRATION_TARGET_ALIASES[normalized]; +} + const buildLogPrefix = (type: string, file: string, subtype?: string) => subtype ? `[${type}][${subtype}][${file}]` : `[${type}][${file}]`; @@ -689,6 +806,7 @@ function convertSubmissionES(esData): any { legacyChallengeId, submissionPhaseId: String(esData.submissionPhaseId), fileType: esData.fileType, + isFileSubmission: toBoolean(esData.isFileSubmission), esId: esData.id, submittedDate: esData.submittedDate ? new Date(esData.submittedDate) : null, updatedBy: esData.updatedBy ?? null, @@ -717,6 +835,171 @@ function convertSubmissionES(esData): any { return submission; } +async function migrateSubmissionIsFileFlagOnly() { + console.log( + `[${MIGRATION_TARGET_SUBMISSION_IS_FILE}] Using ElasticSearch export file: ${ES_DATA_FILE}`, + ); + if (!fs.existsSync(ES_DATA_FILE)) { + throw new Error( + `ElasticSearch export file not found at ${ES_DATA_FILE}. Set ES_DATA_FILE to override the default.`, + ); + } + console.log( + `[${MIGRATION_TARGET_SUBMISSION_IS_FILE}] Starting isFileSubmission synchronization...`, + ); + + const pendingUpdates = new Map(); + const updatedLegacyIds = new Set(); + const missingExamples: string[] = []; + + let totalUpdatedRows = 0; + let totalMissing = 0; + let processedRecords = 0; + let skippedMissingLegacyId = 0; + let skippedOutsideWindow = 0; + let parseErrors = 0; + let lineNumber = 0; + + const flushPending = async () => { + if (pendingUpdates.size === 0) { + return; + } + const entries = Array.from(pendingUpdates.entries()); + pendingUpdates.clear(); + const valueTuples = entries.map( + ([legacySubmissionId, isFileSubmission]) => + Prisma.sql`(${legacySubmissionId}, ${isFileSubmission})`, + ); + let updatedRows: Array<{ legacySubmissionId: string }> = []; + try { + updatedRows = await prisma.$queryRaw< + Array<{ legacySubmissionId: string }> + >(Prisma.sql` + WITH input("legacySubmissionId", "isFileSubmission") AS ( + VALUES ${Prisma.join(valueTuples)} + ) + UPDATE "submission" AS s + SET "isFileSubmission" = input."isFileSubmission" + FROM input + WHERE s."legacySubmissionId" = input."legacySubmissionId" + RETURNING s."legacySubmissionId" AS "legacySubmissionId" + `); + } catch (err) { + console.error( + `[${MIGRATION_TARGET_SUBMISSION_IS_FILE}] Failed to update a batch of ${entries.length} submissions.`, + ); + throw err; + } + totalUpdatedRows += updatedRows.length; + const batchUpdatedSet = new Set( + updatedRows.map((row) => String(row.legacySubmissionId)), + ); + batchUpdatedSet.forEach((id) => updatedLegacyIds.add(id)); + const batchMissing = entries.length - batchUpdatedSet.size; + totalMissing += batchMissing; + if (batchMissing > 0 && missingExamples.length < 5) { + for (const [legacyId] of entries) { + if (!batchUpdatedSet.has(legacyId)) { + missingExamples.push(legacyId); + if (missingExamples.length >= 5) { + break; + } + } + } + } + }; + + const fileStream = fs.createReadStream(ES_DATA_FILE); + const rl = readline.createInterface({ + input: fileStream, + crlfDelay: Infinity, + }); + + for await (const line of rl) { + lineNumber += 1; + if (!line) { + continue; + } + let parsed: any; + try { + parsed = JSON.parse(line); + } catch (err: any) { + parseErrors += 1; + if (parseErrors <= 5) { + console.warn( + `[${MIGRATION_TARGET_SUBMISSION_IS_FILE}] Failed to parse line ${lineNumber}: ${err?.message ?? err}`, + ); + } + continue; + } + const source = parsed?._source; + if (!source || source.resource !== 'submission') { + continue; + } + if (source.legacySubmissionId == null) { + skippedMissingLegacyId += 1; + continue; + } + const createdAudit = source.created ?? source.submittedDate ?? null; + const updatedAudit = source.updated ?? null; + if (!shouldProcessRecord(createdAudit, updatedAudit)) { + skippedOutsideWindow += 1; + continue; + } + const legacySubmissionId = String(source.legacySubmissionId); + const isFileSubmission = toBoolean(source.isFileSubmission); + pendingUpdates.set(legacySubmissionId, isFileSubmission); + processedRecords += 1; + if (pendingUpdates.size >= submissionIsFileBatchSize) { + await flushPending(); + } + if (processedRecords > 0 && processedRecords % logSize === 0) { + console.log( + `[${MIGRATION_TARGET_SUBMISSION_IS_FILE}] Queued ${processedRecords} submissions so far...`, + ); + } + } + + await flushPending(); + + const distinctUpdated = updatedLegacyIds.size; + + console.log( + `[${MIGRATION_TARGET_SUBMISSION_IS_FILE}] Processed ${processedRecords} submission records.`, + ); + console.log( + `[${MIGRATION_TARGET_SUBMISSION_IS_FILE}] Updated ${totalUpdatedRows} row(s) affecting ${distinctUpdated} submission(s).`, + ); + if (totalMissing > 0) { + console.warn( + `[${MIGRATION_TARGET_SUBMISSION_IS_FILE}] Unable to locate ${totalMissing} submission(s) in the database.`, + ); + if (missingExamples.length > 0) { + console.warn( + `[${MIGRATION_TARGET_SUBMISSION_IS_FILE}] Missing legacySubmissionId examples: ${missingExamples.join(', ')}`, + ); + } + } + if (skippedMissingLegacyId > 0) { + console.warn( + `[${MIGRATION_TARGET_SUBMISSION_IS_FILE}] Skipped ${skippedMissingLegacyId} record(s) without a legacySubmissionId.`, + ); + } + if (skippedOutsideWindow > 0) { + console.log( + `[${MIGRATION_TARGET_SUBMISSION_IS_FILE}] Skipped ${skippedOutsideWindow} record(s) outside the incremental window.`, + ); + } + if (parseErrors > 5) { + console.warn( + `[${MIGRATION_TARGET_SUBMISSION_IS_FILE}] Encountered ${parseErrors} parse error(s) while reading the export.`, + ); + } + console.log( + `[${MIGRATION_TARGET_SUBMISSION_IS_FILE}] isFileSubmission synchronization completed.`, + ); +} + async function migrateElasticSearch() { // migrate elastic search data const filepath = ES_DATA_FILE; @@ -3269,6 +3552,15 @@ async function migrateResourceSubmissions() { } async function migrate() { + if (!runFullMigration && runSubmissionIsFileTarget) { + await migrateSubmissionIsFileFlagOnly(); + return; + } + + if (!runFullMigration) { + throw new Error('No recognized migration targets were provided.'); + } + if (!fs.existsSync(DATA_DIR)) { throw new Error( `DATA_DIR "${DATA_DIR}" does not exist. Set DATA_DIR to a valid export path.`, diff --git a/prisma/migrations/20250218000100_add_submission_is_file_submission/migration.sql b/prisma/migrations/20250218000100_add_submission_is_file_submission/migration.sql new file mode 100644 index 0000000..bb4656f --- /dev/null +++ b/prisma/migrations/20250218000100_add_submission_is_file_submission/migration.sql @@ -0,0 +1,3 @@ +-- Add isFileSubmission flag to submissions to differentiate file vs URL entries +ALTER TABLE "submission" +ADD COLUMN IF NOT EXISTS "isFileSubmission" BOOLEAN NOT NULL DEFAULT false; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index ee53157..b3e2858 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -400,6 +400,7 @@ model submission { fileSize Int? viewCount Int? systemFileName String? + isFileSubmission Boolean @default(false) thurgoodJobId String? virusScan Boolean @default(false) diff --git a/src/api/submission/submission.controller.ts b/src/api/submission/submission.controller.ts index 1df5d04..7a4726f 100644 --- a/src/api/submission/submission.controller.ts +++ b/src/api/submission/submission.controller.ts @@ -89,6 +89,10 @@ export class SubmissionController { schema: { type: 'object', properties: { + file: { + type: 'string', + format: 'binary', + }, url: { type: 'string', format: 'url', @@ -107,13 +111,14 @@ export class SubmissionController { }) async createSubmission( @Req() req: Request, + @UploadedFile() file: Express.Multer.File, @Body() body: SubmissionRequestDto, ): Promise { console.log( `Creating submission with request body: ${JSON.stringify(body)}`, ); const authUser: JwtUser = req['user'] as JwtUser; - return this.service.createSubmission(authUser, body); + return this.service.createSubmission(authUser, body, file); } @Patch('/:submissionId') diff --git a/src/api/submission/submission.service.ts b/src/api/submission/submission.service.ts index 1fb4db3..19ea7d7 100644 --- a/src/api/submission/submission.service.ts +++ b/src/api/submission/submission.service.ts @@ -52,6 +52,7 @@ import { Readable, PassThrough } from 'stream'; import { EventBusService } from 'src/shared/modules/global/eventBus.service'; import { SubmissionAccessAuditResponseDto } from 'src/dto/submission-access-audit.dto'; import { Prisma } from '@prisma/client'; +import type { Express } from 'express'; type SubmissionMinimal = { id: string; @@ -59,6 +60,32 @@ type SubmissionMinimal = { url: string | null; }; +type SubmissionBusPayloadSource = Prisma.submissionGetPayload<{ + select: { + id: true; + type: true; + status: true; + memberId: true; + challengeId: true; + legacyChallengeId: true; + legacySubmissionId: true; + legacyUploadId: true; + submissionPhaseId: true; + fileType: true; + systemFileName: true; + submittedDate: true; + url: true; + isFileSubmission: true; + createdAt: true; + updatedAt: true; + createdBy: true; + updatedBy: true; + prizeId: true; + fileSize: true; + viewCount: true; + }; +}>; + type ChallengeRoleSummary = { hasCopilot: boolean; hasReviewer: boolean; @@ -1261,6 +1288,56 @@ export class SubmissionService { } } + private async publishSubmissionCreateEvent( + submission: SubmissionBusPayloadSource, + ): Promise { + const submittedDateValue = + submission.submittedDate instanceof Date + ? submission.submittedDate.toISOString() + : submission.submittedDate + ? new Date(submission.submittedDate).toISOString() + : null; + const updatedAtDate = + submission.updatedAt instanceof Date + ? submission.updatedAt + : submission.updatedAt + ? new Date(submission.updatedAt) + : submission.createdAt; + + const payload = { + resource: 'submission', + id: submission.id, + type: submission.type, + status: submission.status, + memberId: submission.memberId ?? null, + challengeId: submission.challengeId ?? null, + legacyChallengeId: Utils.bigIntToNumber(submission.legacyChallengeId), + legacySubmissionId: submission.legacySubmissionId ?? null, + legacyUploadId: submission.legacyUploadId ?? null, + submissionPhaseId: submission.submissionPhaseId ?? null, + systemFileName: submission.systemFileName ?? null, + fileType: submission.fileType ?? null, + fileSize: submission.fileSize ?? null, + viewCount: submission.viewCount ?? null, + url: submission.url ?? null, + isFileSubmission: Boolean(submission.isFileSubmission), + submittedDate: submittedDateValue, + created: submission.createdAt.toISOString(), + updated: updatedAtDate.toISOString(), + createdBy: submission.createdBy ?? null, + updatedBy: submission.updatedBy ?? null, + prizeId: Utils.bigIntToNumber(submission.prizeId), + }; + + await this.eventBusService.publish( + 'submission.notification.create', + payload, + ); + this.logger.log( + `Published submission.notification.create event for submission ${submission.id}`, + ); + } + private async publishSubmissionScanEvent( submission: SubmissionMinimal, ): Promise { @@ -1322,7 +1399,11 @@ export class SubmissionService { ); } - async createSubmission(authUser: JwtUser, body: SubmissionRequestDto) { + async createSubmission( + authUser: JwtUser, + body: SubmissionRequestDto, + file?: Express.Multer.File, + ) { console.log(`BODY: ${JSON.stringify(body)}`); // Enforce: non-admin, non-M2M users can only submit for themselves @@ -1458,6 +1539,12 @@ export class SubmissionService { } try { + const hasUploadedFile = + !!file && + ((typeof file.size === 'number' && file.size > 0) || + (file.buffer && file.buffer.length > 0)); + const isFileSubmission = hasUploadedFile; + // Derive common metadata if available let systemFileName: string | undefined; let fileType: string | undefined; @@ -1481,6 +1568,7 @@ export class SubmissionService { const data = await this.prisma.submission.create({ data: { ...body, + isFileSubmission, // populate commonly expected fields on create submittedDate: body.submittedDate ? new Date(body.submittedDate) @@ -1494,7 +1582,13 @@ export class SubmissionService { }, }); this.logger.log(`Submission created with ID: ${data.id}`); - await this.publishSubmissionScanEvent(data); + if (isFileSubmission) { + await this.publishSubmissionScanEvent(data); + } else { + this.logger.log( + `Skipping AV scan event for submission ${data.id} because it is not a file-based submission.`, + ); + } // Increment challenge submission counters if challengeId present if (body.challengeId) { try { @@ -1521,6 +1615,9 @@ export class SubmissionService { ); } } + await this.publishSubmissionCreateEvent( + data as SubmissionBusPayloadSource, + ); await this.populateLatestSubmissionFlags([data]); await this.stripIsLatestForUnlimitedChallenges([data]); return this.buildResponse(data); @@ -3643,6 +3740,9 @@ export class SubmissionService { if (Object.prototype.hasOwnProperty.call(data, 'isLatest')) { dto.isLatest = Boolean(data.isLatest); } + if (Object.prototype.hasOwnProperty.call(data, 'isFileSubmission')) { + dto.isFileSubmission = Boolean(data.isFileSubmission); + } return dto; } } diff --git a/src/dto/submission.dto.ts b/src/dto/submission.dto.ts index 3d19097..d0ba09d 100644 --- a/src/dto/submission.dto.ts +++ b/src/dto/submission.dto.ts @@ -311,6 +311,13 @@ export class SubmissionResponseDto { }) virusScan?: boolean; + @ApiProperty({ + description: + 'Indicates whether this submission was created from a file upload, or is a URL (for Wipro)', + required: false, + }) + isFileSubmission?: boolean; + @ApiProperty({ description: 'The creation timestamp', example: '2023-10-01T00:00:00Z',