-
Notifications
You must be signed in to change notification settings - Fork 7
Ref API changes #1070
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Ref API changes #1070
Changes from all commits
b90019e
b55537e
5789e62
061c516
7df6edc
ff39b63
2cd5e34
481efe3
74c1505
687ef41
e959a36
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| -- AlterTable | ||
| ALTER TABLE "ExternalApiUsage" ADD COLUMN "queryingData" JSONB; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| -- CreateIndex: Add GIN index for ExternalApiUsage.queryingData field | ||
| -- This allows efficient JSON containment queries on the queryingData field | ||
|
|
||
| CREATE INDEX CONCURRENTLY IF NOT EXISTS "idx_external_api_usage_querying_data_gin" | ||
| ON "ExternalApiUsage" USING GIN ("queryingData"); | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| -- DropIndex | ||
| DROP INDEX "idx_external_api_usage_querying_data_gin"; | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,8 @@ | ||||||||||||||||||||||||||||||
| import { ExternalApi } from '@prisma/client'; | ||||||||||||||||||||||||||||||
| import { Response } from 'express'; | ||||||||||||||||||||||||||||||
| import { z } from 'zod'; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| import { prisma } from '../../../client.js'; | ||||||||||||||||||||||||||||||
| import { sendSuccess, sendError } from '../../../core/api.js'; | ||||||||||||||||||||||||||||||
| import { AuthenticatedRequest } from '../../../core/types.js'; | ||||||||||||||||||||||||||||||
| import { logger as parentLogger } from '../../../logger.js'; | ||||||||||||||||||||||||||||||
|
|
@@ -10,6 +12,7 @@ const logger = parentLogger.child({ module: 'RefereeRecommender::PresignedUrlCon | |||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const generatePresignedUrlSchema = z.object({ | ||||||||||||||||||||||||||||||
| body: z.object({ | ||||||||||||||||||||||||||||||
| fileHash: z.string().min(1, 'fileHash is required'), | ||||||||||||||||||||||||||||||
| fileName: z.string().min(1, 'fileName is required'), | ||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
|
Comment on lines
13
to
18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Avoid duplicating request schemas; centralize and add strict hash validation. This file introduces its own
-import { z } from 'zod';
+import { z } from 'zod';
+import { generatePresignedUrlSchema } from '../../../schemas/externalApi.schema.js';
-const generatePresignedUrlSchema = z.object({
- body: z.object({
- fileHash: z.string().min(1, 'fileHash is required'),
- fileName: z.string().min(1, 'fileName is required'),
- }),
-});Companion change in 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||
|
|
@@ -24,17 +27,51 @@ export const generatePresignedUrl = async (req: AuthenticatedRequest, res: Respo | |||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const { body } = generatePresignedUrlSchema.parse(req); | ||||||||||||||||||||||||||||||
| const { fileName } = body; | ||||||||||||||||||||||||||||||
| const { fileHash, fileName } = body; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| logger.info( | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| userId: user.id, | ||||||||||||||||||||||||||||||
| fileHash, | ||||||||||||||||||||||||||||||
| fileName, | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| 'Generating presigned URL for referee recommender', | ||||||||||||||||||||||||||||||
| 'Checking cache and potentially generating presigned URL for referee recommender', | ||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Validate file extension (assuming PDFs for now) | ||||||||||||||||||||||||||||||
| // First, check if we have cached results for this fileHash | ||||||||||||||||||||||||||||||
| const cachedResult = await prisma.externalApiUsage.findFirst({ | ||||||||||||||||||||||||||||||
| where: { | ||||||||||||||||||||||||||||||
| userId: user.id, | ||||||||||||||||||||||||||||||
| apiType: ExternalApi.REFEREE_FINDER, | ||||||||||||||||||||||||||||||
| queryingData: { | ||||||||||||||||||||||||||||||
| path: ['fileHash'], | ||||||||||||||||||||||||||||||
| equals: fileHash, | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| orderBy: { | ||||||||||||||||||||||||||||||
| createdAt: 'desc', // Get the most recent result | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if (cachedResult) { | ||||||||||||||||||||||||||||||
| logger.info( | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| userId: user.id, | ||||||||||||||||||||||||||||||
| fileHash, | ||||||||||||||||||||||||||||||
| cachedResultId: cachedResult.id, | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| 'Found cached results for fileHash', | ||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| return sendSuccess(res, { | ||||||||||||||||||||||||||||||
| cached: true, | ||||||||||||||||||||||||||||||
| message: 'Results for this file are already available', | ||||||||||||||||||||||||||||||
| resultKey: RefereeRecommenderService.prepareFormattedFileName(fileHash), | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
Comment on lines
+66
to
+71
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Return an
- return sendSuccess(res, {
+ const resultKey = RefereeRecommenderService.prepareFormattedFileName(fileHash);
+ return sendSuccess(res, {
cached: true,
message: 'Results for this file are already available',
- resultKey: RefereeRecommenderService.prepareFormattedFileName(fileHash),
+ resultKey,
+ // For compatibility with GET /results
+ UploadedFileName: resultKey,
});📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // No cached results found, proceed with generating presigned URL | ||||||||||||||||||||||||||||||
| // Validate file extension | ||||||||||||||||||||||||||||||
| if (!fileName.toLowerCase().endsWith('.pdf')) { | ||||||||||||||||||||||||||||||
| return sendError(res, 'Only PDF files are supported', 400); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
@@ -49,7 +86,7 @@ export const generatePresignedUrl = async (req: AuthenticatedRequest, res: Respo | |||||||||||||||||||||||||||||
| return sendError(res, 'Failed to generate presigned URL', 500); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const { presignedUrl, fileName: generatedFileName } = result.value; | ||||||||||||||||||||||||||||||
| const { presignedUrl, downloadUrl, fileName: generatedFileName } = result.value; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| logger.info( | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
|
|
@@ -60,9 +97,11 @@ export const generatePresignedUrl = async (req: AuthenticatedRequest, res: Respo | |||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| return sendSuccess(res, { | ||||||||||||||||||||||||||||||
| cached: false, | ||||||||||||||||||||||||||||||
| presignedUrl, | ||||||||||||||||||||||||||||||
| downloadUrl, | ||||||||||||||||||||||||||||||
| fileName: generatedFileName, | ||||||||||||||||||||||||||||||
| expiresIn: 3600, // 1 hour | ||||||||||||||||||||||||||||||
| fileHash, | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||
| if (error instanceof z.ZodError) { | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Drop the index CONCURRENTLY (and defensively).
Dropping a large GIN index with
DROP INDEXcan take an ACCESS EXCLUSIVE lock and block writes. PreferDROP INDEX CONCURRENTLY IF EXISTSto avoid long blocking in production.Apply this diff:
Note:
CONCURRENTLYcannot run inside a transaction. Ensure this migration is executed without a surrounding transaction.Verify other migrations with CONCURRENTLY (create/drop) and ensure none are wrapped in transactions:
🏁 Script executed:
Length of output: 830
Drop the GIN index without blocking writes
To avoid an ACCESS EXCLUSIVE lock when removing a large GIN index, switch to
DROP INDEX CONCURRENTLY IF EXISTS.• File (desci-server/prisma/migrations/20250818194118_gin_index_ext_api_querying_data/migration.sql), line 2
• Note:
CONCURRENTLYcannot run inside a transaction—ensure this migration is executed inQUERY_RUN_IN_TRANSACTION = falsemode or otherwise unwrapped.I’ve verified that the only other GIN-index migration uses
CREATE INDEX CONCURRENTLY IF NOT EXISTS(20250818192759_add_gin_index_external_api_querying_data), and no other drops are currently concurrent.📝 Committable suggestion
🤖 Prompt for AI Agents