From ee5c175f98f5250eda2d05f841106662d691d5f3 Mon Sep 17 00:00:00 2001 From: Ramin Tadayon <67011668+axosoft-ramint@users.noreply.github.com> Date: Fri, 10 Mar 2023 09:37:48 -0700 Subject: [PATCH] 2495 Cannot use gitlens+ feature on public repo in some folders (#2510) * Use stored cache of repo visibility * Add commands to clear cache * Fix cache name * Extracts cache key into function for consistency * Update command wording --------- Co-authored-by: Eric Amodio --- package.json | 18 ++++ src/env/node/git/localGitProvider.ts | 28 +++--- src/git/gitProvider.ts | 8 +- src/git/gitProviderService.ts | 182 ++++++++++++++++++++++++++++------- src/git/models/remote.ts | 10 ++ src/plus/github/githubGitProvider.ts | 18 ++-- src/storage.ts | 9 ++ 7 files changed, 216 insertions(+), 57 deletions(-) diff --git a/package.json b/package.json index b3c24dd..99195be 100644 --- a/package.json +++ b/package.json @@ -4487,6 +4487,16 @@ "category": "GitLens+" }, { + "command": "gitlens.plus.resetRepositoryAccess", + "title": "Reset Repository Access Cache", + "category": "GitLens+" + }, + { + "command": "gitlens.plus.refreshRepositoryAccess", + "title": "Refresh Repository Access", + "category": "GitLens+" + }, + { "command": "gitlens.getStarted", "title": "Get Started", "category": "GitLens" @@ -7438,6 +7448,14 @@ "when": "gitlens:debugging" }, { + "command": "gitlens.plus.resetRepositoryAccess", + "when": "gitlens:plus" + }, + { + "command": "gitlens.plus.refreshRepositoryAccess", + "when": "gitlens:plus" + }, + { "command": "gitlens.showGraphPage", "when": "gitlens:enabled" }, diff --git a/src/env/node/git/localGitProvider.ts b/src/env/node/git/localGitProvider.ts index 45ac8c4..345d032 100644 --- a/src/env/node/git/localGitProvider.ts +++ b/src/env/node/git/localGitProvider.ts @@ -87,7 +87,7 @@ import { shortenRevision, } from '../../../git/models/reference'; import type { GitReflog } from '../../../git/models/reflog'; -import { getRemoteIconUri, GitRemote } from '../../../git/models/remote'; +import { getRemoteIconUri, getVisibilityCacheKey, GitRemote } from '../../../git/models/remote'; import { RemoteResourceType } from '../../../git/models/remoteResource'; import type { RepositoryChangeEvent } from '../../../git/models/repository'; import { Repository, RepositoryChange, RepositoryChangeComparisonMode } from '../../../git/models/repository'; @@ -507,27 +507,31 @@ export class LocalGitProvider implements GitProvider, Disposable { } } - async visibility(repoPath: string): Promise { + async visibility(repoPath: string): Promise<[visibility: RepositoryVisibility, cacheKey: string | undefined]> { const remotes = await this.getRemotes(repoPath, { sort: true }); - if (remotes.length === 0) return RepositoryVisibility.Local; + if (remotes.length === 0) return [RepositoryVisibility.Local, undefined]; let local = true; for await (const result of fastestSettled(remotes.map(r => this.getRemoteVisibility(r)))) { if (result.status !== 'fulfilled') continue; - if (result.value === RepositoryVisibility.Public) return RepositoryVisibility.Public; - if (result.value !== RepositoryVisibility.Local) { + if (result.value[0] === RepositoryVisibility.Public) { + return [RepositoryVisibility.Public, getVisibilityCacheKey(result.value[1])]; + } + if (result.value[0] !== RepositoryVisibility.Local) { local = false; } } - return local ? RepositoryVisibility.Local : RepositoryVisibility.Private; + return local + ? [RepositoryVisibility.Local, undefined] + : [RepositoryVisibility.Private, getVisibilityCacheKey(remotes)]; } @debug({ args: { 0: r => r.url } }) private async getRemoteVisibility( remote: GitRemote, - ): Promise { + ): Promise<[visibility: RepositoryVisibility, remote: GitRemote]> { const scope = getLogScope(); switch (remote.provider?.id) { @@ -539,22 +543,24 @@ export class LocalGitProvider implements GitProvider, Disposable { case 'gerrit': case 'google-source': { const url = remote.provider.url({ type: RemoteResourceType.Repo }); - if (url == null) return RepositoryVisibility.Private; + if (url == null) return [RepositoryVisibility.Private, remote]; // Check if the url returns a 200 status code try { const rsp = await fetch(url, { method: 'HEAD', agent: getProxyAgent() }); - if (rsp.ok) return RepositoryVisibility.Public; + if (rsp.ok) return [RepositoryVisibility.Public, remote]; Logger.debug(scope, `Response=${rsp.status}`); } catch (ex) { debugger; Logger.error(ex, scope); } - return RepositoryVisibility.Private; + return [RepositoryVisibility.Private, remote]; } default: - return maybeUri(remote.url) ? RepositoryVisibility.Private : RepositoryVisibility.Local; + return maybeUri(remote.url) + ? [RepositoryVisibility.Private, remote] + : [RepositoryVisibility.Local, remote]; } } diff --git a/src/git/gitProvider.ts b/src/git/gitProvider.ts index 666c959..de9919a 100644 --- a/src/git/gitProvider.ts +++ b/src/git/gitProvider.ts @@ -98,6 +98,12 @@ export const enum RepositoryVisibility { Local = 'local', } +export interface RepositoryVisibilityInfo { + visibility: RepositoryVisibility; + timestamp: number; + remotesHash?: string; +} + export interface GitProvider extends Disposable { get onDidChangeRepository(): Event; get onDidCloseRepository(): Event; @@ -118,7 +124,7 @@ export interface GitProvider extends Disposable { openRepositoryInitWatcher?(): RepositoryInitWatcher; supports(feature: Features): Promise; - visibility(repoPath: string): Promise; + visibility(repoPath: string, remotes?: GitRemote[]): Promise<[visibility: RepositoryVisibility, cacheKey: string | undefined]>; getOpenScmRepositories(): Promise; getScmRepository(repoPath: string): Promise; diff --git a/src/git/gitProviderService.ts b/src/git/gitProviderService.ts index 74c7e6e..f44fe23 100644 --- a/src/git/gitProviderService.ts +++ b/src/git/gitProviderService.ts @@ -22,6 +22,7 @@ import { asRepoComparisonKey, Repositories } from '../repositories'; import type { Subscription } from '../subscription'; import { isSubscriptionPaidPlan, SubscriptionPlanId } from '../subscription'; import { groupByFilterMap, groupByMap, joinUnique } from '../system/array'; +import { registerCommand } from '../system/command'; import { configuration } from '../system/configuration'; import { gate } from '../system/decorators/gate'; import { debug, log } from '../system/decorators/log'; @@ -41,6 +42,7 @@ import type { PagedResult, PreviousComparisonUrisResult, PreviousLineComparisonUrisResult, + RepositoryVisibilityInfo, ScmRepository, } from './gitProvider'; import { RepositoryVisibility } from './gitProvider'; @@ -149,10 +151,8 @@ export class GitProviderService implements Disposable { this._etag = Date.now(); this._accessCache.clear(); - this._visibilityCache.delete(undefined); - if (removed?.length) { - this._visibilityCache.clear(); - } + this._reposVisibilityCache = undefined; + this._onDidChangeRepositories.fire({ added: added ?? [], removed: removed ?? [], etag: this._etag }); if (added?.length) { @@ -200,6 +200,7 @@ export class GitProviderService implements Disposable { this.resetCaches('providers'); this.updateContext(); }), + ...this.registerCommands(), ); this.container.BranchDateFormatting.reset(); @@ -247,6 +248,13 @@ export class GitProviderService implements Disposable { } } + private registerCommands(): Disposable[] { + return [ + registerCommand('gitlens.plus.resetRepositoryAccess', () => this.clearAllRepoVisibilityCaches()), + registerCommand('gitlens.plus.refreshRepositoryAccess', () => this.clearAllOpenRepoVisibilityCaches()), + ]; + } + @debug() onSubscriptionChanged(e: SubscriptionChangeEvent) { this._accessCache.clear(); @@ -379,7 +387,7 @@ export class GitProviderService implements Disposable { const disposable = Disposable.from( provider, ...disposables, - provider.onDidChangeRepository(e => { + provider.onDidChangeRepository(async e => { if ( e.changed( RepositoryChange.Remotes, @@ -397,7 +405,14 @@ export class GitProviderService implements Disposable { queueMicrotask(() => this.fireRepositoriesChanged([], [e.repository])); } - this._visibilityCache.delete(e.repository.path); + if (e.changed(RepositoryChange.Remotes, RepositoryChangeComparisonMode.Any)) { + const remotes = await provider.getRemotes(e.repository.path); + const visibilityInfo = this.getVisibilityInfoFromCache(e.repository.path); + if (visibilityInfo != null) { + this.checkVisibilityCachedRemotes(e.repository.path, visibilityInfo, remotes); + } + } + this._onDidChangeRepository.fire(e); }), provider.onDidCloseRepository(e => { @@ -703,43 +718,125 @@ export class GitProviderService implements Disposable { return provider.supports(feature); } - private _visibilityCache: Map> & - Map> = new Map(); + private _reposVisibilityCache: RepositoriesVisibility | undefined; + private _repoVisibilityCache: Map | undefined; + + private ensureRepoVisibilityCache(): void { + if (this._repoVisibilityCache == null) { + const repoVisibility: [string, RepositoryVisibilityInfo][] | undefined = this.container.storage + .get('repoVisibility') + ?.map<[string, RepositoryVisibilityInfo]>(([key, visibilityInfo]) => [ + key, + { + visibility: visibilityInfo.visibility as RepositoryVisibility, + timestamp: visibilityInfo.timestamp, + remotesHash: visibilityInfo.remotesHash, + }, + ]); + this._repoVisibilityCache = new Map(repoVisibility); + } + } + + private clearRepoVisibilityCache(keys?: string[]): void { + if (keys == null) { + this._repoVisibilityCache = undefined; + void this.container.storage.delete('repoVisibility'); + } else { + keys?.forEach(key => this._repoVisibilityCache?.delete(key)); + const repoVisibility = Array.from(this._repoVisibilityCache?.entries() ?? []); + if (repoVisibility.length === 0) { + void this.container.storage.delete('repoVisibility'); + } else { + void this.container.storage.store('repoVisibility', repoVisibility); + } + } + } + + private getVisibilityInfoFromCache(key: string): RepositoryVisibilityInfo | undefined { + this.ensureRepoVisibilityCache(); + const visibilityInfo = this._repoVisibilityCache?.get(key); + if (visibilityInfo == null) return undefined; + + const now = Date.now(); + if (now - visibilityInfo.timestamp > 1000 * 60 * 60 * 24 * 30 /* TTL is 30 days */) { + this.clearRepoVisibilityCache([key]); + return undefined; + } + + return visibilityInfo; + } + + private checkVisibilityCachedRemotes( + key: string, + visibilityInfo: RepositoryVisibilityInfo | undefined, + remotes: GitRemote[], + ): boolean { + if (visibilityInfo == null) return true; + + if (visibilityInfo.visibility === RepositoryVisibility.Public) { + if (remotes.length == 0 || !remotes.some(r => r.id === visibilityInfo.remotesHash)) { + this.clearRepoVisibilityCache([key]); + return false; + } + } else if (visibilityInfo.visibility === RepositoryVisibility.Private) { + const remotesHash = remotes + .map(r => r.id) + .sort() + .join(','); + if (remotesHash !== visibilityInfo.remotesHash) { + this.clearRepoVisibilityCache([key]); + return false; + } + } + + return true; + } + + private updateVisibilityCache(key: string, visibilityInfo: RepositoryVisibilityInfo): void { + this.ensureRepoVisibilityCache(); + this._repoVisibilityCache?.set(key, visibilityInfo); + void this.container.storage.store('repoVisibility', Array.from(this._repoVisibilityCache!.entries())); + } + + clearAllRepoVisibilityCaches(): void { + this.clearRepoVisibilityCache(); + } + + clearAllOpenRepoVisibilityCaches(): void { + const openRepoProviderPaths = this.openRepositories.map(r => this.getProvider(r.path).path); + this.clearRepoVisibilityCache(openRepoProviderPaths); + } + visibility(): Promise; visibility(repoPath: string | Uri): Promise; async visibility(repoPath?: string | Uri): Promise { if (repoPath == null) { - let visibility = this._visibilityCache.get(undefined); + let visibility = this._reposVisibilityCache; if (visibility == null) { - visibility = this.visibilityCore(); - void visibility.then(v => { - this.container.telemetry.setGlobalAttribute('repositories.visibility', v); - this.container.telemetry.sendEvent('repositories/visibility'); - }); - this._visibilityCache.set(undefined, visibility); + visibility = await this.visibilityCore(); + this.container.telemetry.setGlobalAttribute('repositories.visibility', visibility); + this.container.telemetry.sendEvent('repositories/visibility'); + this._reposVisibilityCache = visibility; } return visibility; } const { path: cacheKey } = this.getProvider(repoPath); - let visibility = this._visibilityCache.get(cacheKey); + let visibility = this.getVisibilityInfoFromCache(cacheKey)?.visibility; if (visibility == null) { - visibility = this.visibilityCore(repoPath); - void visibility.then(v => - queueMicrotask(() => { - const repo = this.getRepository(repoPath); - this.container.telemetry.sendEvent('repository/visibility', { - 'repository.visibility': v, - 'repository.id': repo?.idHash, - 'repository.scheme': repo?.uri.scheme, - 'repository.closed': repo?.closed, - 'repository.folder.scheme': repo?.folder?.uri.scheme, - 'repository.provider.id': repo?.provider.id, - }); - }), - ); - this._visibilityCache.set(cacheKey, visibility); + visibility = await this.visibilityCore(repoPath); + queueMicrotask(() => { + const repo = this.getRepository(repoPath); + this.container.telemetry.sendEvent('repository/visibility', { + 'repository.visibility': visibility, + 'repository.id': repo?.idHash, + 'repository.scheme': repo?.uri.scheme, + 'repository.closed': repo?.closed, + 'repository.folder.scheme': repo?.folder?.uri.scheme, + 'repository.provider.id': repo?.provider.id, + }); + }); } return visibility; } @@ -748,16 +845,27 @@ export class GitProviderService implements Disposable { private visibilityCore(repoPath: string | Uri): Promise; @debug() private async visibilityCore(repoPath?: string | Uri): Promise { - function getRepoVisibility(this: GitProviderService, repoPath: string | Uri): Promise { + async function getRepoVisibility( + this: GitProviderService, + repoPath: string | Uri, + ): Promise { const { provider, path } = this.getProvider(repoPath); + const remotes = await provider.getRemotes(path, { sort: true }); + const visibilityInfo = this.getVisibilityInfoFromCache(path); + if (visibilityInfo == null || !this.checkVisibilityCachedRemotes(path, visibilityInfo, remotes)) { + const [visibility, remotesHash] = await provider.visibility(path); + if (visibility !== RepositoryVisibility.Local) { + this.updateVisibilityCache(path, { + visibility: visibility, + timestamp: Date.now(), + remotesHash: remotesHash, + }); + } - let visibility = this._visibilityCache.get(path); - if (visibility == null) { - visibility = provider.visibility(path); - this._visibilityCache.set(path, visibility); + return visibility; } - return visibility; + return visibilityInfo.visibility; } if (repoPath == null) { diff --git a/src/git/models/remote.ts b/src/git/models/remote.ts index de5b86d..86fd81d 100644 --- a/src/git/models/remote.ts +++ b/src/git/models/remote.ts @@ -163,3 +163,13 @@ export function getRemoteIconUri( ); return asWebviewUri != null ? asWebviewUri(uri) : uri; } + +export function getVisibilityCacheKey(remote: GitRemote): string; +export function getVisibilityCacheKey(remotes: GitRemote[]): string; +export function getVisibilityCacheKey(remotes: GitRemote | GitRemote[]): string { + if (!Array.isArray(remotes)) return remotes.id; + return remotes + .map(r => r.id) + .sort() + .join(','); +} diff --git a/src/plus/github/githubGitProvider.ts b/src/plus/github/githubGitProvider.ts index f03e406..505d460 100644 --- a/src/plus/github/githubGitProvider.ts +++ b/src/plus/github/githubGitProvider.ts @@ -62,7 +62,7 @@ import type { GitRebaseStatus } from '../../git/models/rebase'; import type { GitBranchReference, GitReference } from '../../git/models/reference'; import { createReference, isRevisionRange, isSha, isShaLike, isUncommitted } from '../../git/models/reference'; import type { GitReflog } from '../../git/models/reflog'; -import { getRemoteIconUri, GitRemote, GitRemoteType } from '../../git/models/remote'; +import { getRemoteIconUri, getVisibilityCacheKey, GitRemote, GitRemoteType } from '../../git/models/remote'; import type { RepositoryChangeEvent } from '../../git/models/repository'; import { Repository } from '../../git/models/repository'; import type { GitStash } from '../../git/models/stash'; @@ -229,22 +229,24 @@ export class GitHubGitProvider implements GitProvider, Disposable { } } - async visibility(repoPath: string): Promise { + async visibility(repoPath: string): Promise<[visibility: RepositoryVisibility, cacheKey: string | undefined]> { const remotes = await this.getRemotes(repoPath, { sort: true }); - if (remotes.length === 0) return RepositoryVisibility.Local; + if (remotes.length === 0) return [RepositoryVisibility.Local, undefined]; for await (const result of fastestSettled(remotes.map(r => this.getRemoteVisibility(r)))) { if (result.status !== 'fulfilled') continue; - if (result.value === RepositoryVisibility.Public) return RepositoryVisibility.Public; + if (result.value[0] === RepositoryVisibility.Public) { + return [RepositoryVisibility.Public, getVisibilityCacheKey(result.value[1])]; + } } - return RepositoryVisibility.Private; + return [RepositoryVisibility.Private, getVisibilityCacheKey(remotes)]; } private async getRemoteVisibility( remote: GitRemote, - ): Promise { + ): Promise<[visibility: RepositoryVisibility, remote: GitRemote]> { switch (remote.provider?.id) { case 'github': { const { github, metadata, session } = await this.ensureRepositoryContext(remote.repoPath); @@ -254,10 +256,10 @@ export class GitHubGitProvider implements GitProvider, Disposable { metadata.repo.name, ); - return visibility ?? RepositoryVisibility.Private; + return [visibility ?? RepositoryVisibility.Private, remote]; } default: - return RepositoryVisibility.Private; + return [RepositoryVisibility.Private, remote]; } } diff --git a/src/storage.ts b/src/storage.ts index faaac63..cf557b6 100644 --- a/src/storage.ts +++ b/src/storage.ts @@ -123,6 +123,7 @@ export type DeprecatedGlobalStorage = { export type GlobalStorage = { avatars: [string, StoredAvatar][]; + repoVisibility: [string, StoredRepoVisibilityInfo][]; 'confirm:sendToOpenAI': boolean; 'deepLinks:pending': StoredDeepLinkContext; 'home:actions:completed': CompletedActions[]; @@ -185,6 +186,14 @@ export interface StoredAvatar { timestamp: number; } +export type StoredRepositoryVisibility = 'private' | 'public' | 'local'; + +export interface StoredRepoVisibilityInfo { + visibility: StoredRepositoryVisibility; + timestamp: number; + remotesHash?: string; +} + export interface StoredBranchComparison { ref: string; notation: '..' | '...' | undefined;