Skip to content

Commit 1898362

Browse files
Wellington01Wellington01
authored
Merge pull request #1259 from Fanduzi/fix/1110-pr-close-perf
perf(kodyRules): eliminate duplicate API calls in syncFromChangedFiles
2 parents e4dc60f + fc7dc93 commit 1898362

1 file changed

Lines changed: 113 additions & 116 deletions

File tree

‎libs/kodyRules/infrastructure/adapters/services/kodyRulesSync.service.ts‎

Lines changed: 113 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -351,8 +351,26 @@ export class KodyRulesSyncService {
351351
repository.id,
352352
);
353353

354+
// Fetch PR details once — shared across all code paths below.
355+
const prDetails =
356+
await this.codeManagementService.getPullRequestByNumber({
357+
organizationAndTeamData,
358+
repository: { id: repository.id, name: repository.name },
359+
prNumber: pullRequestNumber,
360+
});
361+
362+
const { head, base } = this.extractRefsFromPullRequest(prDetails);
363+
const pullRequestParam: any = {
364+
number: pullRequestNumber,
365+
head: head ? { ref: head } : undefined,
366+
base: base ? { ref: base } : undefined,
367+
};
368+
354369
// If the sync is disabled, we need to force sync the files that have @kody-sync
355370
const forceSyncFiles: string[] = [];
371+
// Cache decoded file content from the @kody-sync scan so the
372+
// main loop below doesn't re-fetch the same files.
373+
const contentCache = new Map<string, string>();
356374
if (!syncEnabled) {
357375
// First, we need to check which files can be rule files
358376
const directoryPatterns = await this.getDirectoryPatterns(
@@ -369,25 +387,6 @@ export class KodyRulesSyncService {
369387
isRuleFile(f.previous_filename),
370388
);
371389

372-
// Get the PR details once
373-
const prDetails =
374-
await this.codeManagementService.getPullRequestByNumber({
375-
organizationAndTeamData,
376-
repository: {
377-
id: repository.id,
378-
name: repository.name,
379-
},
380-
prNumber: pullRequestNumber,
381-
});
382-
383-
const { head, base } =
384-
this.extractRefsFromPullRequest(prDetails);
385-
const pullRequestParam: any = {
386-
number: pullRequestNumber,
387-
head: head ? { ref: head } : undefined,
388-
base: base ? { ref: base } : undefined,
389-
};
390-
391390
// Now we need to check which files have @kody-sync in the content
392391
for (const f of ruleChanges) {
393392
if (f.status === 'removed') continue;
@@ -402,18 +401,23 @@ export class KodyRulesSyncService {
402401
pullRequest: pullRequestParam,
403402
});
404403

405-
if (content && this.shouldForceSync(content)) {
406-
forceSyncFiles.push(f.filename);
407-
this.logger.log({
408-
message:
409-
'File marked for force sync with @kody-sync',
410-
context: KodyRulesSyncService.name,
411-
metadata: {
412-
filename: f.filename,
413-
repositoryId: repository.id,
414-
organizationAndTeamData,
415-
},
416-
});
404+
if (content) {
405+
// Cache for reuse in the main loop below.
406+
contentCache.set(f.filename, content);
407+
408+
if (this.shouldForceSync(content)) {
409+
forceSyncFiles.push(f.filename);
410+
this.logger.log({
411+
message:
412+
'File marked for force sync with @kody-sync',
413+
context: KodyRulesSyncService.name,
414+
metadata: {
415+
filename: f.filename,
416+
repositoryId: repository.id,
417+
organizationAndTeamData,
418+
},
419+
});
420+
}
417421
}
418422
}
419423

@@ -476,20 +480,6 @@ export class KodyRulesSyncService {
476480
});
477481
}
478482

479-
const prDetails =
480-
await this.codeManagementService.getPullRequestByNumber({
481-
organizationAndTeamData,
482-
repository: { id: repository.id, name: repository.name },
483-
prNumber: pullRequestNumber,
484-
});
485-
486-
const { head, base } = this.extractRefsFromPullRequest(prDetails);
487-
const pullRequestParam: any = {
488-
number: pullRequestNumber,
489-
head: head ? { ref: head } : undefined,
490-
base: base ? { ref: base } : undefined,
491-
};
492-
493483
const directoryPatterns = await this.getDirectoryPatterns(
494484
organizationAndTeamData,
495485
repository.id,
@@ -531,87 +521,94 @@ export class KodyRulesSyncService {
531521
? f.previous_filename
532522
: f.filename;
533523

534-
const contentResp =
535-
await this.codeManagementService.getRepositoryContentFile({
536-
organizationAndTeamData,
537-
repository: {
538-
id: repository.id,
539-
name: repository.name,
540-
},
541-
file: { filename: f.filename },
542-
pullRequest: pullRequestParam,
543-
});
544-
// Fallbacks if the source branch was deleted on merge (e.g., GitLab):
545-
// 1) Try with base as head
546-
// 2) Try with default branch as head
547-
let effectiveContent = contentResp;
548-
if (!effectiveContent?.data?.content) {
549-
const baseRef = pullRequestParam.base?.ref;
550-
if (baseRef) {
551-
try {
552-
const baseAsHead =
553-
await this.codeManagementService.getRepositoryContentFile(
554-
{
555-
organizationAndTeamData,
556-
repository: {
557-
id: repository.id,
558-
name: repository.name,
524+
// Reuse cached content from the @kody-sync scan when
525+
// available to avoid a duplicate API call for the same
526+
// file (the scan already fetched it moments ago).
527+
let decoded: string | null = contentCache.get(f.filename) ?? null;
528+
529+
if (!decoded) {
530+
const contentResp =
531+
await this.codeManagementService.getRepositoryContentFile({
532+
organizationAndTeamData,
533+
repository: {
534+
id: repository.id,
535+
name: repository.name,
536+
},
537+
file: { filename: f.filename },
538+
pullRequest: pullRequestParam,
539+
});
540+
// Fallbacks if the source branch was deleted on merge (e.g., GitLab):
541+
// 1) Try with base as head
542+
// 2) Try with default branch as head
543+
let effectiveContent = contentResp;
544+
if (!effectiveContent?.data?.content) {
545+
const baseRef = pullRequestParam.base?.ref;
546+
if (baseRef) {
547+
try {
548+
const baseAsHead =
549+
await this.codeManagementService.getRepositoryContentFile(
550+
{
551+
organizationAndTeamData,
552+
repository: {
553+
id: repository.id,
554+
name: repository.name,
555+
},
556+
file: { filename: f.filename },
557+
pullRequest: { head: { ref: baseRef } },
559558
},
560-
file: { filename: f.filename },
561-
pullRequest: { head: { ref: baseRef } },
562-
},
563-
);
564-
if (baseAsHead?.data?.content) {
565-
effectiveContent = baseAsHead;
559+
);
560+
if (baseAsHead?.data?.content) {
561+
effectiveContent = baseAsHead;
562+
}
563+
} catch {
564+
// Ignore error
566565
}
567-
} catch {
568-
// Ignore error
569566
}
570567
}
571-
}
572-
if (!effectiveContent?.data?.content) {
573-
try {
574-
const defaultBranch =
575-
await this.codeManagementService.getDefaultBranch({
576-
organizationAndTeamData,
577-
repository: {
578-
id: repository.id,
579-
name: repository.name,
580-
},
581-
});
582-
if (defaultBranch) {
583-
const defAsHead =
584-
await this.codeManagementService.getRepositoryContentFile(
585-
{
586-
organizationAndTeamData,
587-
repository: {
588-
id: repository.id,
589-
name: repository.name,
590-
},
591-
file: { filename: f.filename },
592-
pullRequest: {
593-
head: { ref: defaultBranch },
594-
},
568+
if (!effectiveContent?.data?.content) {
569+
try {
570+
const defaultBranch =
571+
await this.codeManagementService.getDefaultBranch({
572+
organizationAndTeamData,
573+
repository: {
574+
id: repository.id,
575+
name: repository.name,
595576
},
596-
);
597-
if (defAsHead?.data?.content) {
598-
effectiveContent = defAsHead;
577+
});
578+
if (defaultBranch) {
579+
const defAsHead =
580+
await this.codeManagementService.getRepositoryContentFile(
581+
{
582+
organizationAndTeamData,
583+
repository: {
584+
id: repository.id,
585+
name: repository.name,
586+
},
587+
file: { filename: f.filename },
588+
pullRequest: {
589+
head: { ref: defaultBranch },
590+
},
591+
},
592+
);
593+
if (defAsHead?.data?.content) {
594+
effectiveContent = defAsHead;
595+
}
599596
}
597+
} catch {
598+
// Ignore error
600599
}
601-
} catch {
602-
// Ignore error
603600
}
604-
}
605601

606-
const rawContent = effectiveContent?.data?.content;
607-
if (!rawContent) {
608-
continue;
609-
}
602+
const rawContent = effectiveContent?.data?.content;
603+
if (!rawContent) {
604+
continue;
605+
}
610606

611-
const decoded =
612-
contentResp?.data?.encoding === 'base64'
613-
? Buffer.from(rawContent, 'base64').toString('utf-8')
614-
: rawContent;
607+
decoded =
608+
effectiveContent?.data?.encoding === 'base64'
609+
? Buffer.from(rawContent, 'base64').toString('utf-8')
610+
: rawContent;
611+
}
615612

616613
//Verify if the file should be ignored due to the @kody-ignore marker
617614
if (this.shouldIgnoreFile(decoded)) {

0 commit comments

Comments
 (0)