diff --git a/extensions/github/src/branchProtection.ts b/extensions/github/src/branchProtection.ts index a7d18c41b9f..52b4fbfae22 100644 --- a/extensions/github/src/branchProtection.ts +++ b/extensions/github/src/branchProtection.ts @@ -48,7 +48,7 @@ const REPOSITORY_RULESETS_QUERY = ` } `; -export class GithubBranchProtectionProviderManager { +export class GitHubBranchProtectionProviderManager { private readonly disposables = new DisposableStore(); private readonly providerDisposables = new DisposableStore(); @@ -61,7 +61,7 @@ export class GithubBranchProtectionProviderManager { if (enabled) { for (const repository of this.gitAPI.repositories) { - this.providerDisposables.add(this.gitAPI.registerBranchProtectionProvider(repository.rootUri, new GithubBranchProtectionProvider(repository, this.globalState, this.logger, this.telemetryReporter))); + this.providerDisposables.add(this.gitAPI.registerBranchProtectionProvider(repository.rootUri, new GitHubBranchProtectionProvider(repository, this.globalState, this.logger, this.telemetryReporter))); } } else { this.providerDisposables.dispose(); @@ -77,7 +77,7 @@ export class GithubBranchProtectionProviderManager { private readonly telemetryReporter: TelemetryReporter) { this.disposables.add(this.gitAPI.onDidOpenRepository(repository => { if (this._enabled) { - this.providerDisposables.add(gitAPI.registerBranchProtectionProvider(repository.rootUri, new GithubBranchProtectionProvider(repository, this.globalState, this.logger, this.telemetryReporter))); + this.providerDisposables.add(gitAPI.registerBranchProtectionProvider(repository.rootUri, new GitHubBranchProtectionProvider(repository, this.globalState, this.logger, this.telemetryReporter))); } })); @@ -102,7 +102,7 @@ export class GithubBranchProtectionProviderManager { } -export class GithubBranchProtectionProvider implements BranchProtectionProvider { +export class GitHubBranchProtectionProvider implements BranchProtectionProvider { private readonly _onDidChangeBranchProtection = new EventEmitter(); onDidChangeBranchProtection = this._onDidChangeBranchProtection.event; @@ -173,12 +173,12 @@ export class GithubBranchProtectionProvider implements BranchProtectionProvider } // Repository details - this.logger.trace(`Fetching repository details for "${repository.owner}/${repository.repo}".`); + this.logger.trace(`[GitHubBranchProtectionProvider][updateRepositoryBranchProtection] Fetching repository details for "${repository.owner}/${repository.repo}".`); const repositoryDetails = await this.getRepositoryDetails(repository.owner, repository.repo); // Check repository write permission if (repositoryDetails.viewerPermission !== 'ADMIN' && repositoryDetails.viewerPermission !== 'MAINTAIN' && repositoryDetails.viewerPermission !== 'WRITE') { - this.logger.trace(`Skipping branch protection for "${repository.owner}/${repository.repo}" due to missing repository write permission.`); + this.logger.trace(`[GitHubBranchProtectionProvider][updateRepositoryBranchProtection] Skipping branch protection for "${repository.owner}/${repository.repo}" due to missing repository write permission.`); continue; } @@ -201,7 +201,7 @@ export class GithubBranchProtectionProvider implements BranchProtectionProvider // Save branch protection to global state await this.globalState.update(this.globalStateKey, branchProtection); - this.logger.trace(`Branch protection for "${this.repository.rootUri.toString()}": ${JSON.stringify(branchProtection)}.`); + this.logger.trace(`[GitHubBranchProtectionProvider][updateRepositoryBranchProtection] Branch protection for "${this.repository.rootUri.toString()}": ${JSON.stringify(branchProtection)}.`); /* __GDPR__ "branchProtection" : { @@ -211,7 +211,7 @@ export class GithubBranchProtectionProvider implements BranchProtectionProvider */ this.telemetryReporter.sendTelemetryEvent('branchProtection', undefined, { rulesetCount: this.branchProtection.length }); } catch (err) { - this.logger.warn(`Failed to update repository branch protection: ${err.message}`); + this.logger.warn(`[GitHubBranchProtectionProvider][updateRepositoryBranchProtection] Failed to update repository branch protection: ${err.message}`); if (err instanceof AuthenticationError) { // A GitHub authentication session could be missing if the user has not yet diff --git a/extensions/github/src/extension.ts b/extensions/github/src/extension.ts index 72a9111326e..de0349ba9d3 100644 --- a/extensions/github/src/extension.ts +++ b/extensions/github/src/extension.ts @@ -13,7 +13,7 @@ import { DisposableStore, repositoryHasGitHubRemote } from './util'; import { GithubPushErrorHandler } from './pushErrorHandler'; import { GitBaseExtension } from './typings/git-base'; import { GithubRemoteSourcePublisher } from './remoteSourcePublisher'; -import { GithubBranchProtectionProviderManager } from './branchProtection'; +import { GitHubBranchProtectionProviderManager } from './branchProtection'; import { GitHubCanonicalUriProvider } from './canonicalUriProvider'; import { VscodeDevShareProvider } from './shareProviders'; import { GitHubSourceControlHistoryItemDetailsProvider } from './historyItemDetailsProvider'; @@ -98,7 +98,7 @@ function initializeGitExtension(context: ExtensionContext, telemetryReporter: Te disposables.add(registerCommands(gitAPI)); disposables.add(new GithubCredentialProviderManager(gitAPI)); - disposables.add(new GithubBranchProtectionProviderManager(gitAPI, context.globalState, logger, telemetryReporter)); + disposables.add(new GitHubBranchProtectionProviderManager(gitAPI, context.globalState, logger, telemetryReporter)); disposables.add(gitAPI.registerPushErrorHandler(new GithubPushErrorHandler(telemetryReporter))); disposables.add(gitAPI.registerRemoteSourcePublisher(new GithubRemoteSourcePublisher(gitAPI))); disposables.add(gitAPI.registerSourceControlHistoryItemDetailsProvider(new GitHubSourceControlHistoryItemDetailsProvider(gitAPI, logger))); diff --git a/extensions/github/src/historyItemDetailsProvider.ts b/extensions/github/src/historyItemDetailsProvider.ts index 7c09ba0c618..33383141f48 100644 --- a/extensions/github/src/historyItemDetailsProvider.ts +++ b/extensions/github/src/historyItemDetailsProvider.ts @@ -106,7 +106,10 @@ export class GitHubSourceControlHistoryItemDetailsProvider implements SourceCont return undefined; } + try { + const logs = { cached: 0, email: 0, github: 0, incomplete: 0 }; + // Warm up the in-memory cache with the first page // (100 users) from this list of assignable users await this._loadAssignableUsers(descriptor); @@ -120,43 +123,46 @@ export class GitHubSourceControlHistoryItemDetailsProvider implements SourceCont const authorQuery = groupBy(query.commits, compareAvatarQuery); const results = new Map(); - await Promise.all(authorQuery.map(async q => { - if (q.length === 0) { + await Promise.all(authorQuery.map(async commits => { + if (commits.length === 0) { return; } // Query the in-memory cache for the user const avatarUrl = repositoryStore.users.find( - user => user.email === q[0].authorEmail || user.name === q[0].authorName)?.avatarUrl; + user => user.email === commits[0].authorEmail || user.name === commits[0].authorName)?.avatarUrl; // Cache hit if (avatarUrl) { // Add avatar for each commit - q.forEach(({ hash }) => results.set(hash, `${avatarUrl}&s=${query.size}`)); + logs.cached += commits.length; + commits.forEach(({ hash }) => results.set(hash, `${avatarUrl}&s=${query.size}`)); return; } // Check if any of the commit are being tracked in the list // of known commits that have incomplte author information - if (q.some(({ hash }) => repositoryStore.commits.has(hash))) { - q.forEach(({ hash }) => results.set(hash, undefined)); + if (commits.some(({ hash }) => repositoryStore.commits.has(hash))) { + commits.forEach(({ hash }) => results.set(hash, undefined)); return; } // Try to extract the user identifier from GitHub no-reply emails - const userIdFromEmail = getUserIdFromNoReplyEmail(q[0].authorEmail); + const userIdFromEmail = getUserIdFromNoReplyEmail(commits[0].authorEmail); if (userIdFromEmail) { + logs.email += commits.length; const avatarUrl = getAvatarLink(userIdFromEmail, query.size); - q.forEach(({ hash }) => results.set(hash, avatarUrl)); + commits.forEach(({ hash }) => results.set(hash, avatarUrl)); return; } // Get the commit details - const commitAuthor = await this._getCommitAuthor(descriptor, q[0].hash); + const commitAuthor = await this._getCommitAuthor(descriptor, commits[0].hash); if (!commitAuthor) { // The commit has incomplete author information, so // we should not try to query the authors details again - for (const { hash } of q) { + logs.incomplete += commits.length; + for (const { hash } of commits) { repositoryStore.commits.add(hash); results.set(hash, undefined); } @@ -167,9 +173,12 @@ export class GitHubSourceControlHistoryItemDetailsProvider implements SourceCont repositoryStore.users.push(commitAuthor); // Add avatar for each commit - q.forEach(({ hash }) => results.set(hash, `${commitAuthor.avatarUrl}&s=${query.size}`)); + logs.github += commits.length; + commits.forEach(({ hash }) => results.set(hash, `${commitAuthor.avatarUrl}&s=${query.size}`)); })); + this._logger.trace(`[GitHubSourceControlHistoryItemDetailsProvider][provideAvatar] Avatar resolution for ${query.commits.length} commit(s) in ${repository.rootUri.fsPath} complete: ${JSON.stringify(logs)}.`); + return results; } catch (err) { // A GitHub authentication session could be missing if the user has not yet