Skip to content

Commit f72841c

Browse files
authored
Merge pull request #173 from topcoder-platform/submissions-performance
Submissions performance
2 parents eb903fd + b6fd7f5 commit f72841c

File tree

6 files changed

+569
-42
lines changed

6 files changed

+569
-42
lines changed

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

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,14 +179,15 @@ export class ReviewSummationController {
179179
`Getting review summations with filters - ${JSON.stringify(queryDto)}`,
180180
);
181181
const authUser: JwtUser = req['user'] as JwtUser;
182+
const wantsTabSeparated = this.requestWantsTabSeparated(req);
182183
const results = await this.service.searchSummation(
183184
authUser,
184185
queryDto,
185186
paginationDto,
186187
sortDto,
187188
);
188189

189-
if (!this.requestWantsTabSeparated(req)) {
190+
if (!wantsTabSeparated) {
190191
return results;
191192
}
192193

@@ -199,7 +200,13 @@ export class ReviewSummationController {
199200
});
200201
}
201202

202-
const payload = this.buildReviewSummationTsv(results);
203+
const completeResults = await this.loadAllReviewSummationsForExport(
204+
authUser,
205+
queryDto,
206+
sortDto,
207+
results,
208+
);
209+
const payload = this.buildReviewSummationTsv(completeResults);
203210
const safeChallengeSlug = this.buildFilenameSlug(challengeId);
204211
const filename = `${ReviewSummationController.TSV_FILENAME_PREFIX}-${safeChallengeSlug}.tsv`;
205212
res.setHeader(
@@ -321,6 +328,28 @@ export class ReviewSummationController {
321328
};
322329
}
323330

331+
private async loadAllReviewSummationsForExport(
332+
authUser: JwtUser,
333+
queryDto: ReviewSummationQueryDto,
334+
sortDto: SortDto | undefined,
335+
initialResults: PaginatedResponse<ReviewSummationResponseDto>,
336+
): Promise<PaginatedResponse<ReviewSummationResponseDto>> {
337+
const totalCount =
338+
initialResults.meta?.totalCount ?? initialResults.data.length;
339+
const currentCount = initialResults.data.length;
340+
341+
if (!Number.isFinite(totalCount) || totalCount <= currentCount) {
342+
return initialResults;
343+
}
344+
345+
return this.service.searchSummation(
346+
authUser,
347+
queryDto,
348+
{ page: 1, perPage: totalCount },
349+
sortDto,
350+
);
351+
}
352+
324353
private requestWantsTabSeparated(req: Request): boolean {
325354
const acceptHeader = Array.isArray(req.headers.accept)
326355
? req.headers.accept.join(',')

src/api/submission/submission.service.ts

Lines changed: 48 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3085,34 +3085,58 @@ export class SubmissionService {
30853085
return;
30863086
}
30873087

3088-
const latestIds = new Set<string>();
3089-
await Promise.all(
3090-
Array.from(uniquePairs.values()).map(
3091-
async ({ challengeId, memberId }) => {
3092-
const latest = await this.prisma.submission.findFirst({
3093-
where: {
3094-
challengeId,
3095-
memberId,
3096-
},
3097-
orderBy: [
3098-
{ submittedDate: 'desc' },
3099-
{ createdAt: 'desc' },
3100-
{ updatedAt: 'desc' },
3101-
],
3102-
select: { id: true },
3103-
});
3104-
3105-
if (latest?.id) {
3106-
latestIds.add(latest.id);
3107-
}
3108-
},
3088+
const challengeIds = Array.from(
3089+
new Set(
3090+
Array.from(uniquePairs.values()).map((entry) => entry.challengeId),
31093091
),
31103092
);
3093+
const memberIds = Array.from(
3094+
new Set(Array.from(uniquePairs.values()).map((entry) => entry.memberId)),
3095+
);
31113096

3112-
for (const submission of submissions) {
3113-
if (latestIds.has(submission.id)) {
3114-
(submission as any).isLatest = true;
3097+
if (!challengeIds.length || !memberIds.length) {
3098+
return;
3099+
}
3100+
3101+
try {
3102+
// Use a single windowed query to locate the latest submission per (challengeId, memberId)
3103+
const latestEntries = await this.prisma.$queryRaw<
3104+
Array<{ id: string }>
3105+
>(Prisma.sql`
3106+
SELECT "id"
3107+
FROM (
3108+
SELECT
3109+
"id",
3110+
ROW_NUMBER() OVER (
3111+
PARTITION BY "challengeId", "memberId"
3112+
ORDER BY "submittedDate" DESC NULLS LAST,
3113+
"createdAt" DESC,
3114+
"updatedAt" DESC NULLS LAST
3115+
) AS row_num
3116+
FROM "submission"
3117+
WHERE "challengeId" IN (${Prisma.join(challengeIds)})
3118+
AND "memberId" IN (${Prisma.join(memberIds)})
3119+
) ranked
3120+
WHERE row_num = 1
3121+
`);
3122+
3123+
const latestIds = new Set(
3124+
latestEntries
3125+
.map((entry) => String(entry.id ?? '').trim())
3126+
.filter((id) => id.length > 0),
3127+
);
3128+
3129+
for (const submission of submissions) {
3130+
if (latestIds.has(submission.id)) {
3131+
(submission as any).isLatest = true;
3132+
}
31153133
}
3134+
} catch (error) {
3135+
const message = error instanceof Error ? error.message : String(error);
3136+
this.logger.warn(
3137+
`[populateLatestSubmissionFlags] Failed to resolve latest submissions via bulk query: ${message}`,
3138+
);
3139+
throw error;
31163140
}
31173141
}
31183142

src/shared/config/auth.config.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ export const AuthConfig = {
88
// JSON array string of allowed issuers
99
validIssuers:
1010
process.env.VALID_ISSUERS ||
11-
'["https://testsachin.topcoder-dev.com/","https://test-sachin-rs256.auth0.com/","https://api.topcoder.com","https://api.topcoder-dev.com","https://topcoder-dev.auth0.com/", "https://auth.topcoder-dev.com/"]',
11+
'["https://testsachin.topcoder-dev.com/","https://test-sachin-rs256.auth0.com/","https://api.topcoder.com","https://api.topcoder-dev.com","https://topcoder-dev.auth0.com/","https://auth.topcoder-dev.com/","https://topcoder.auth0.com/","https://auth.topcoder.com/"]',
1212

1313
// Legacy JWT configuration (kept for backward compatibility)
1414
jwt: {

src/shared/guards/tokenRoles.guard.ts

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,30 @@ import {
77
SetMetadata,
88
} from '@nestjs/common';
99
import { Reflector } from '@nestjs/core';
10+
import { Request } from 'express';
1011
import { JwtService } from '../modules/global/jwt.service';
1112
import { SCOPES_KEY } from '../decorators/scopes.decorator';
13+
import { LoggerService } from '../modules/global/logger.service';
1214
import { UserRole } from '../enums/userRole.enum';
1315

1416
export const ROLES_KEY = 'roles';
1517
export const Roles = (...roles: UserRole[]) => SetMetadata(ROLES_KEY, roles);
1618

1719
@Injectable()
1820
export class TokenRolesGuard implements CanActivate {
21+
private readonly logger = LoggerService.forRoot(TokenRolesGuard.name);
22+
1923
constructor(
2024
private reflector: Reflector,
2125
private jwtService: JwtService,
2226
) {}
2327

2428
canActivate(context: ExecutionContext): boolean {
29+
const handler = context.getHandler?.();
30+
const controllerClass = context.getClass?.();
31+
const controllerName = controllerClass?.name || 'UnknownController';
32+
const handlerName = handler?.name || 'UnknownHandler';
33+
2534
// Get required roles and scopes from decorators
2635
const requiredRoles =
2736
this.reflector.get<UserRole[]>(ROLES_KEY, context.getHandler()) || [];
@@ -31,18 +40,49 @@ export class TokenRolesGuard implements CanActivate {
3140

3241
// If no roles or scopes are required, allow access
3342
if (requiredRoles.length === 0 && requiredScopes.length === 0) {
43+
this.logger.log({
44+
message: 'No roles or scopes required, allowing request',
45+
controllerName,
46+
handlerName,
47+
});
3448
return true;
3549
}
3650

3751
const request = context.switchToHttp().getRequest<Request>();
52+
const requestMeta = this.buildRequestLogMeta(
53+
request,
54+
controllerName,
55+
handlerName,
56+
);
57+
58+
this.logger.log({
59+
message: 'Evaluating token roles guard',
60+
...requestMeta,
61+
requiredRoles,
62+
requiredScopes,
63+
});
3864

3965
try {
4066
const user = request['user'];
4167

4268
if (!user && (requiredRoles.length || requiredScopes.length)) {
69+
this.logger.warn({
70+
message: 'Rejecting request due to missing user payload',
71+
...requestMeta,
72+
hasUser: false,
73+
});
4374
throw new UnauthorizedException('Missing or invalid token!');
4475
}
4576

77+
this.logger.log({
78+
message: 'User payload extracted from request',
79+
...requestMeta,
80+
userId: user?.userId,
81+
isMachine: Boolean(user?.isMachine),
82+
roles: user?.roles,
83+
scopes: user?.scopes,
84+
});
85+
4686
const normalizedRequiredRoles = requiredRoles.map((role) =>
4787
String(role).trim().toLowerCase(),
4888
);
@@ -51,10 +91,22 @@ export class TokenRolesGuard implements CanActivate {
5191
if (normalizedRequiredRoles.length > 0) {
5292
const normalizedUserRoles = this.normalizeUserRoles(user.roles);
5393

94+
this.logger.log({
95+
message: 'Checking role-based permissions',
96+
...requestMeta,
97+
normalizedRequiredRoles,
98+
normalizedUserRoles,
99+
});
100+
54101
const hasRole = normalizedRequiredRoles.some((role) =>
55102
normalizedUserRoles.includes(role),
56103
);
57104
if (hasRole) {
105+
this.logger.log({
106+
message: 'Access granted via role-based authorization',
107+
...requestMeta,
108+
normalizedRequiredRoles,
109+
});
58110
return true;
59111
}
60112

@@ -66,6 +118,13 @@ export class TokenRolesGuard implements CanActivate {
66118
user,
67119
)
68120
) {
121+
this.logger.log({
122+
message:
123+
'Access granted via submission list challenge fallback rule',
124+
...requestMeta,
125+
challengeId: request?.query?.challengeId,
126+
userId: user?.userId,
127+
});
69128
return true;
70129
}
71130
}
@@ -75,7 +134,20 @@ export class TokenRolesGuard implements CanActivate {
75134
const hasScope = requiredScopes.some((scope) =>
76135
user.scopes ? user.scopes.includes(scope) : false,
77136
);
137+
138+
this.logger.log({
139+
message: 'Checking scope-based permissions',
140+
...requestMeta,
141+
requiredScopes,
142+
tokenScopes: user.scopes,
143+
hasScope,
144+
});
145+
78146
if (hasScope) {
147+
this.logger.log({
148+
message: 'Access granted via scope-based authorization',
149+
...requestMeta,
150+
});
79151
return true;
80152
}
81153
}
@@ -89,10 +161,22 @@ export class TokenRolesGuard implements CanActivate {
89161
requiredRoles.length > 0 &&
90162
requiredScopes.length === 0
91163
) {
164+
this.logger.warn({
165+
message:
166+
'M2M token detected without role permissions for role-only endpoint',
167+
...requestMeta,
168+
tokenScopes: user.scopes,
169+
});
92170
throw new ForbiddenException('M2M token not allowed for this endpoint');
93171
}
94172

95173
// Access denied - neither roles nor scopes match
174+
this.logger.warn({
175+
message: 'Access denied due to insufficient permissions',
176+
...requestMeta,
177+
requiredRoles,
178+
requiredScopes,
179+
});
96180
throw new ForbiddenException('Insufficient permissions');
97181
} catch (error) {
98182
if (
@@ -101,6 +185,15 @@ export class TokenRolesGuard implements CanActivate {
101185
) {
102186
throw error;
103187
}
188+
this.logger.error(
189+
{
190+
message: 'Unexpected error validating token roles',
191+
...requestMeta,
192+
error:
193+
error instanceof Error ? error.message : String(error ?? 'error'),
194+
},
195+
error instanceof Error ? error.stack : undefined,
196+
);
104197
throw new UnauthorizedException('Invalid token');
105198
}
106199
}
@@ -192,4 +285,33 @@ export class TokenRolesGuard implements CanActivate {
192285

193286
return Array.from(normalizedRoles);
194287
}
288+
289+
private buildRequestLogMeta(
290+
request: any,
291+
controllerName: string,
292+
handlerName: string,
293+
) {
294+
const headers = (request?.headers || {}) as Record<
295+
string,
296+
string | string[] | undefined
297+
>;
298+
const correlationIdCandidate =
299+
headers['x-request-id'] ||
300+
headers['x-correlation-id'] ||
301+
headers['x-trace-id'];
302+
const method =
303+
typeof request?.method === 'string'
304+
? request.method.toUpperCase()
305+
: undefined;
306+
307+
return {
308+
controllerName,
309+
handlerName,
310+
method,
311+
path: request?.originalUrl || request?.url || request?.path,
312+
correlationId: Array.isArray(correlationIdCandidate)
313+
? correlationIdCandidate[0]
314+
: correlationIdCandidate,
315+
};
316+
}
195317
}

0 commit comments

Comments
 (0)