GitHub - add more logs to avatar resolution (#238266)
parent
3d0aeb47a2
commit
4b4cd6b702
|
@ -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<Uri>();
|
||||
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
|
||||
|
|
|
@ -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)));
|
||||
|
|
|
@ -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<AvatarQueryCommit>(query.commits, compareAvatarQuery);
|
||||
|
||||
const results = new Map<string, string | undefined>();
|
||||
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
|
||||
|
|
Loading…
Reference in New Issue