From cb2026ee64ccd374d0311b87ffae493382125c45 Mon Sep 17 00:00:00 2001 From: Sergei Shmakov Date: Mon, 14 Apr 2025 15:08:59 +0200 Subject: [PATCH] Adds `IssueOrPullRequestType` to cache keys for issue/PR retrieval preventing potential cache collisions. (#4193, #4233) --- CHANGELOG.md | 1 + src/cache.ts | 40 ++++++++++++++++++++++------ src/plus/integrations/integration.ts | 1 + 3 files changed, 34 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 78ecc96659f2d..49f6a7d72062d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p ### Fixed +- Fixes cache collision between issues and PRs in autolinks ([#4193](https://github.com/gitkraken/vscode-gitlens/issues/4193)) - Fixes incorrect PR Link Across Azure DevOps Projects ([#4207](https://github.com/gitkraken/vscode-gitlens/issues/4207)) - Fixes detail view incorrectly parses GitHub account in commit message ([#3246](https://github.com/gitkraken/vscode-gitlens/issues/3246)) - Fixes `undefined` sometimes showing for search results in the _Search Commits_ command and _Search & Compare_ view diff --git a/src/cache.ts b/src/cache.ts index a24f6125ed8ea..ba88be8beddf7 100644 --- a/src/cache.ts +++ b/src/cache.ts @@ -4,7 +4,7 @@ import type { Container } from './container'; import type { Account } from './git/models/author'; import type { DefaultBranch } from './git/models/defaultBranch'; import type { Issue } from './git/models/issue'; -import type { IssueOrPullRequest } from './git/models/issueOrPullRequest'; +import type { IssueOrPullRequest, IssueOrPullRequestType } from './git/models/issueOrPullRequest'; import type { PullRequest } from './git/models/pullRequest'; import type { RepositoryMetadata } from './git/models/repositoryMetadata'; import type { HostingIntegration, IntegrationBase, ResourceDescriptor } from './plus/integrations/integration'; @@ -111,6 +111,7 @@ export class CacheProvider implements Disposable { getIssueOrPullRequest( id: string, + type: IssueOrPullRequestType | undefined, resource: ResourceDescriptor, integration: IntegrationBase | undefined, cacheable: Cacheable, @@ -119,11 +120,11 @@ export class CacheProvider implements Disposable { const { key, etag } = getResourceKeyAndEtag(resource, integration); if (resource == null) { - return this.get('issuesOrPrsById', `id:${id}:${key}`, etag, cacheable, options); + return this.get('issuesOrPrsById', `id:${id}:${key}:${type ?? 'unknown'}`, etag, cacheable, options); } return this.get( 'issuesOrPrsByIdAndRepo', - `id:${id}:${key}:${JSON.stringify(resource)}}`, + `id:${id}:${key}:${type ?? 'unknown'}:${JSON.stringify(resource)}}`, etag, cacheable, options, @@ -140,11 +141,17 @@ export class CacheProvider implements Disposable { const { key, etag } = getResourceKeyAndEtag(resource, integration); if (resource == null) { - return this.get('issuesById', `id:${id}:${key}`, etag, cacheable, options); + return this.get( + 'issuesById', + `id:${id}:${key}:${'issue' satisfies IssueOrPullRequestType}`, + etag, + cacheable, + options, + ); } return this.get( 'issuesByIdAndResource', - `id:${id}:${key}:${JSON.stringify(resource)}}`, + `id:${id}:${key}:${'issue' satisfies IssueOrPullRequestType}:${JSON.stringify(resource)}}`, etag, cacheable, options, @@ -161,9 +168,21 @@ export class CacheProvider implements Disposable { const { key, etag } = getResourceKeyAndEtag(resource, integration); if (resource == null) { - return this.get('prsById', `id:${id}:${key}`, etag, cacheable, options); + return this.get( + 'prsById', + `id:${id}:${key}:${'pullrequest' satisfies IssueOrPullRequestType}`, + etag, + cacheable, + options, + ); } - return this.get('prsById', `id:${id}:${key}:${JSON.stringify(resource)}}`, etag, cacheable, options); + return this.get( + 'prsById', + `id:${id}:${key}:${'pullrequest' satisfies IssueOrPullRequestType}:${JSON.stringify(resource)}}`, + etag, + cacheable, + options, + ); } getPullRequestForBranch( @@ -264,7 +283,12 @@ export class CacheProvider implements Disposable { if (isPromise(item.value)) { void item.value.then(v => { if (v != null) { - this.set('issuesOrPrsById', `id:${v.id}:${key}`, v, etag); + this.set( + 'issuesOrPrsById', + `id:${v.id}:${key}:${'pullrequest' satisfies IssueOrPullRequestType}`, + v, + etag, + ); } }); } diff --git a/src/plus/integrations/integration.ts b/src/plus/integrations/integration.ts index c354076eede94..291c487a0a22b 100644 --- a/src/plus/integrations/integration.ts +++ b/src/plus/integrations/integration.ts @@ -465,6 +465,7 @@ export abstract class IntegrationBase< const issueOrPR = this.container.cache.getIssueOrPullRequest( id, + options?.type, resource, this, () => ({