Merge pull request #295 from topcoder-platform/develop

Performance updates for API responses

Author: jmgasper

src/api/review-summation/review-summation.service.spec.ts

@@ -8,7 +8,7 @@ import { UserRole } from 'src/shared/enums/userRole.enum';
 
 describe('ReviewSummationService', () => {
   describe('searchSummation', () => {
-    it('allows registered marathon submitters to view challenge summations with only safe progress metadata', async () => {
+    it('allows registered marathon submitters to view challenge summations without metadata', async () => {
       const prismaMock = {
         reviewSummation: {
           findMany: jest.fn().mockResolvedValue([
@@ -127,27 +127,81 @@ describe('ReviewSummationService', () => {
           }),
         }),
       );
+      const findManyArg = prismaMock.reviewSummation.findMany.mock.calls[0][0];
+      expect(findManyArg.select).not.toHaveProperty('metadata');
       expect(result.data).toHaveLength(2);
       expect(result.data.map((summation) => summation.submitterId)).toEqual([
         111, 222,
       ]);
-      expect(result.data[0].metadata).toEqual({
+      expect(result.data[0]).not.toHaveProperty('metadata');
+      expect(result.data[1]).not.toHaveProperty('metadata');
+      expect(JSON.stringify(result.data)).not.toContain('123456789');
+      expect(JSON.stringify(result.data)).not.toContain('987654321');
+    });
+
+    it('returns metadata to machine callers that explicitly request it', async () => {
+      const metadata = {
         testProcess: 'provisional',
-        testProgress: 0.5,
-        testStatus: 'IN PROGRESS',
-        testProgressDetails: {
-          completedTests: 5,
-          progress: 0.5,
-          status: 'IN PROGRESS',
-          totalTests: 10,
+        testScores: [
+          {
+            score: 11,
+            seed: 123456789,
+          },
+        ],
+      };
+      const prismaMock = {
+        reviewSummation: {
+          findMany: jest.fn().mockResolvedValue([
+            {
+              id: 'summation-1',
+              submissionId: 'submission-1',
+              aggregateScore: -1,
+              scorecardId: null,
+              isPassing: false,
+              isFinal: false,
+              isProvisional: true,
+              isExample: false,
+              reviewedDate: null,
+              createdAt: new Date('2026-05-01T00:00:00.000Z'),
+              createdBy: null,
+              updatedAt: null,
+              updatedBy: null,
+              metadata,
+            },
+          ]),
+          count: jest.fn().mockResolvedValue(1),
         },
-      });
-      expect(JSON.stringify(result.data[0].metadata)).not.toContain(
-        '123456789',
+      };
+      const service = new ReviewSummationService(
+        prismaMock as any,
+        {} as any,
+        {} as any,
+        { member: { findMany: jest.fn().mockResolvedValue([]) } } as any,
+        {} as any,
       );
-      expect(JSON.stringify(result.data[1].metadata)).not.toContain(
-        '987654321',
+
+      const result = await service.searchSummation(
+        {
+          isMachine: true,
+          roles: [],
+        },
+        {
+          submissionId: 'submission-1',
+          metadata: 'true',
+          provisional: 'true',
+        },
+        {
+          page: 1,
+          perPage: 10,
+        },
       );
+
+      const findManyArg = prismaMock.reviewSummation.findMany.mock.calls[0][0];
+      expect(findManyArg.select).toHaveProperty('metadata', true);
+      expect(findManyArg.select).not.toHaveProperty('submission');
+      expect(result.data).toHaveLength(1);
+      expect(result.data[0].metadata).toEqual(metadata);
+      expect(result.data[0]).not.toHaveProperty('submission');
     });
   });
 });

src/api/review-summation/review-summation.service.ts

@@ -26,7 +26,47 @@ import { MemberPrismaService } from 'src/shared/modules/global/member-prisma.ser
 import { ResourceApiService } from 'src/shared/modules/global/resource.service';
 import { UserRole } from 'src/shared/enums/userRole.enum';
 import { Prisma } from '@prisma/client';
-import { buildSafeReviewSummationMetadata } from 'src/shared/utils/review-summation-metadata.util';
+
+const REVIEW_SUMMATION_RESPONSE_SELECT = {
+  id: true,
+  submissionId: true,
+  legacySubmissionId: true,
+  aggregateScore: true,
+  scorecardId: true,
+  scorecardLegacyId: true,
+  isPassing: true,
+  isFinal: true,
+  isProvisional: true,
+  isExample: true,
+  reviewedDate: true,
+  createdAt: true,
+  createdBy: true,
+  updatedAt: true,
+  updatedBy: true,
+} satisfies Prisma.reviewSummationSelect;
+
+const REVIEW_SUMMATION_RESPONSE_WITH_METADATA_SELECT = {
+  ...REVIEW_SUMMATION_RESPONSE_SELECT,
+  metadata: true,
+} satisfies Prisma.reviewSummationSelect;
+
+const REVIEW_SUMMATION_WITH_SUBMITTER_SELECT = {
+  ...REVIEW_SUMMATION_RESPONSE_SELECT,
+  submission: {
+    select: {
+      memberId: true,
+    },
+  },
+} satisfies Prisma.reviewSummationSelect;
+
+const REVIEW_SUMMATION_WITH_SUBMITTER_AND_METADATA_SELECT = {
+  ...REVIEW_SUMMATION_RESPONSE_WITH_METADATA_SELECT,
+  submission: {
+    select: {
+      memberId: true,
+    },
+  },
+} satisfies Prisma.reviewSummationSelect;
 
 @Injectable()
 export class ReviewSummationService {
@@ -562,9 +602,10 @@ export class ReviewSummationService {
 
       const data = await this.prisma.reviewSummation.create({
         data: createData,
+        select: REVIEW_SUMMATION_RESPONSE_SELECT,
       });
       this.logger.log(`Review summation created with ID: ${data.id}`);
-      return data as ReviewSummationResponseDto;
+      return this.buildResponse(data);
     } catch (error) {
       // Re-throw NotFoundException and BadRequestException as-is
       if (
@@ -671,8 +712,6 @@ export class ReviewSummationService {
         : undefined;
       const challengeIdFilter =
         rawChallengeId && rawChallengeId.length ? rawChallengeId : undefined;
-      const includeMetadata =
-        (queryDto.metadata ?? '').toLowerCase() === 'true';
 
       if (isSubmitterOnly) {
         const userId =
@@ -839,23 +878,23 @@ export class ReviewSummationService {
       };
 
       const shouldEnrichSubmitterMetadata = Boolean(challengeIdFilter);
+      const includeMetadata =
+        (authUser?.isMachine ?? false) &&
+        parseBooleanString(queryDto.metadata) === true;
+      const summationSelect = shouldEnrichSubmitterMetadata
+        ? includeMetadata
+          ? REVIEW_SUMMATION_WITH_SUBMITTER_AND_METADATA_SELECT
+          : REVIEW_SUMMATION_WITH_SUBMITTER_SELECT
+        : includeMetadata
+          ? REVIEW_SUMMATION_RESPONSE_WITH_METADATA_SELECT
+          : REVIEW_SUMMATION_RESPONSE_SELECT;
 
       const summations = await this.prisma.reviewSummation.findMany({
         where: whereClause,
         skip,
         take: perPage,
         orderBy,
-        ...(shouldEnrichSubmitterMetadata
-          ? {
-              include: {
-                submission: {
-                  select: {
-                    memberId: true,
-                  },
-                },
-              },
-            }
-          : {}),
+        select: summationSelect,
       });
 
       const submitterInfoByMemberId = new Map<
@@ -929,11 +968,10 @@ export class ReviewSummationService {
       });
 
       const data: ReviewSummationResponseDto[] = summations.map((summation) => {
-        const { submission, metadata, ...rest } =
-          summation as typeof summation & {
-            submission?: { memberId: string | null };
-            metadata?: Prisma.JsonValue | null;
-          };
+        const summationRecord = summation as typeof summation & {
+          submission?: { memberId: string | null };
+        };
+        const { submission } = summationRecord;
 
         let submitterId: number | null = null;
         let submitterHandle: string | null = null;
@@ -958,20 +996,17 @@ export class ReviewSummationService {
           }
         }
 
-        const base: ReviewSummationResponseDto = {
-          ...rest,
-          submitterId,
-          submitterHandle,
-          submitterMaxRating,
-        } as ReviewSummationResponseDto;
-
-        if (includeMetadata) {
-          base.metadata = isSubmitterOnly
-            ? buildSafeReviewSummationMetadata(metadata)
-            : (metadata ?? null);
-        }
-
-        return base;
+        return this.buildResponse(
+          summationRecord,
+          {
+            submitterId,
+            submitterHandle,
+            submitterMaxRating,
+          },
+          {
+            includeMetadata,
+          },
+        );
       });
 
       this.logger.log(
@@ -1081,9 +1116,10 @@ export class ReviewSummationService {
       const data = await this.prisma.reviewSummation.update({
         where: { id },
         data: updateData,
+        select: REVIEW_SUMMATION_RESPONSE_SELECT,
       });
       this.logger.log(`Review summation updated successfully: ${id}`);
-      return data as ReviewSummationResponseDto;
+      return this.buildResponse(data);
     } catch (error) {
       // Re-throw NotFoundException and BadRequestException from checkSummation and validation as-is
       if (
@@ -1165,13 +1201,14 @@ export class ReviewSummationService {
     try {
       const data = await this.prisma.reviewSummation.findUnique({
         where: { id },
+        select: REVIEW_SUMMATION_RESPONSE_SELECT,
       });
       if (!data || !data.id) {
         throw new NotFoundException(
           `Review summation with ID ${id} not found. Please verify the summation ID is correct.`,
         );
       }
-      return data;
+      return this.buildResponse(data);
     } catch (error) {
       // Re-throw NotFoundException as-is
       if (error instanceof NotFoundException) {
@@ -1189,4 +1226,29 @@ export class ReviewSummationService {
       });
     }
   }
+
+  /**
+   * Builds a review summation response object.
+   * @param data Review summation row or row-like object to serialize.
+   * @param extras Optional computed submitter fields to append.
+   * @param options Response serialization options, including internal metadata access.
+   * @returns Review summation response DTO with internal relations removed.
+   * @throws This method does not throw.
+   * Used by all review summation response paths as a final guard against exposing per-seed metadata by default.
+   */
+  private buildResponse(
+    data: Record<string, unknown>,
+    extras: Partial<ReviewSummationResponseDto> = {},
+    options: { includeMetadata?: boolean } = {},
+  ): ReviewSummationResponseDto {
+    const response: Record<string, unknown> = {
+      ...data,
+      ...extras,
+    };
+    if (!options.includeMetadata) {
+      delete response.metadata;
+    }
+    delete response.submission;
+    return response as unknown as ReviewSummationResponseDto;
+  }
 }

src/api/submission/submission.service.spec.ts

@@ -2246,7 +2246,7 @@ describe('SubmissionService', () => {
       expect(submissionResult.url).toBeNull();
     });
 
-    it('sanitizes review summation metadata for submitter-owned submissions', async () => {
+    it('omits review summation metadata for submitter-owned submissions', async () => {
       const now = new Date('2026-05-01T12:00:00Z');
       const submissions = [
         {
@@ -2307,19 +2307,16 @@ describe('SubmissionService', () => {
         { page: 1, perPage: 10 } as any,
       );
 
-      const metadata = result.data[0].reviewSummation?.[0].metadata;
-      expect(metadata).toEqual({
-        testProcess: 'system',
-        testProgress: 0.75,
-        testStatus: 'IN PROGRESS',
-        testProgressDetails: {
-          completedTests: 15,
-          progress: 0.75,
-          status: 'IN PROGRESS',
-          totalTests: 20,
-        },
-      });
-      expect(JSON.stringify(metadata)).not.toContain('987654321');
+      const findManyArg = prismaMock.submission.findMany.mock.calls[0][0];
+      expect(findManyArg.include.reviewSummation.select).not.toHaveProperty(
+        'metadata',
+      );
+      expect(result.data[0].reviewSummation?.[0]).not.toHaveProperty(
+        'metadata',
+      );
+      expect(JSON.stringify(result.data[0].reviewSummation)).not.toContain(
+        '987654321',
+      );
     });
   });