From 4a362bb7e8ddb3ee458ddc7a48679c018ab6846e Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Thu, 15 Oct 2020 00:09:56 -0400 Subject: [PATCH] Adds avatar retries Moves avatar cache to global storage Improves avatar cache updates/resets Improves contributor avatar fetch update --- src/annotations/gutterBlameAnnotationProvider.ts | 2 +- src/avatars.ts | 247 ++++++++++++++--------- src/constants.ts | 4 +- src/container.ts | 4 +- src/extension.ts | 4 +- src/git/formatters/commitFormatter.ts | 2 +- src/git/gitService.ts | 8 +- src/git/models/commit.ts | 2 +- src/git/models/contributor.ts | 2 +- src/views/contributorsView.ts | 6 +- src/views/nodes/commitFileNode.ts | 2 +- src/views/nodes/commitNode.ts | 2 +- src/views/nodes/contributorNode.ts | 4 +- src/views/nodes/contributorsNode.ts | 12 +- src/webviews/rebaseEditor.ts | 4 +- 15 files changed, 194 insertions(+), 111 deletions(-) diff --git a/src/annotations/gutterBlameAnnotationProvider.ts b/src/annotations/gutterBlameAnnotationProvider.ts index a6e2856..1b6cc89 100644 --- a/src/annotations/gutterBlameAnnotationProvider.ts +++ b/src/annotations/gutterBlameAnnotationProvider.ts @@ -215,7 +215,7 @@ export class GutterBlameAnnotationProvider extends BlameAnnotationProviderBase { height: '16px', width: '16px', textDecoration: `none;position:absolute;top:1px;left:5px;background:url(${( - await commit.getAvatarUri({ fallback: gravatarDefault }) + await commit.getAvatarUri({ defaultStyle: gravatarDefault }) ).toString()});background-size:16px 16px`, }; map.set(commit.email!, avatarDecoration); diff --git a/src/avatars.ts b/src/avatars.ts index cfe1276..d34439d 100644 --- a/src/avatars.ts +++ b/src/avatars.ts @@ -2,23 +2,51 @@ import * as fs from 'fs'; import { EventEmitter, Uri } from 'vscode'; import { GravatarDefaultStyle } from './config'; -import { WorkspaceState } from './constants'; +import { GlobalState } from './constants'; import { Container } from './container'; import { GitRevisionReference } from './git/git'; -import { Functions, Strings } from './system'; +import { Functions, Iterables, Strings } from './system'; +import { MillisecondsPerDay, MillisecondsPerHour, MillisecondsPerMinute } from './system/date'; import { ContactPresenceStatus } from './vsls/vsls'; -// TODO@eamodio Use timestamp -// TODO@eamodio Clear avatar cache on remote / provider connection change +const _onDidFetchAvatar = new EventEmitter<{ email: string }>(); +_onDidFetchAvatar.event( + Functions.debounce(() => { + const avatars = + avatarCache != null + ? [ + ...Iterables.filterMap(avatarCache, ([key, avatar]) => + avatar.uri != null + ? [ + key, + { + uri: avatar.uri.toString(), + timestamp: avatar.timestamp, + }, + ] + : undefined, + ), + ] + : undefined; + void Container.context.globalState.update(GlobalState.Avatars, avatars); + }, 1000), +); -interface Avatar { - uri?: T | null; - fallback: T; +export namespace Avatars { + export const onDidFetch = _onDidFetchAvatar.event; +} + +interface Avatar { + uri?: Uri; + fallback?: Uri; timestamp: number; - // TODO@eamodio Add a fail count, to avoid failing on a single failure + retries: number; } -type SerializedAvatar = Avatar; +interface SerializedAvatar { + uri: string; + timestamp: number; +} let avatarCache: Map | undefined; const avatarQueue = new Map>(); @@ -29,99 +57,107 @@ const presenceCache = new Map(); const gitHubNoReplyAddressRegex = /^(?:(?\d+)\+)?(?[a-zA-Z\d-]{1,39})@users\.noreply\.github\.com$/; -const _onDidFetchAvatar = new EventEmitter<{ email: string }>(); -export const onDidFetchAvatar = _onDidFetchAvatar.event; - -onDidFetchAvatar( - Functions.debounce(() => { - void Container.context.workspaceState.update( - WorkspaceState.Avatars, - avatarCache == null - ? undefined - : [...avatarCache.entries()].map<[string, SerializedAvatar]>(([key, value]) => [ - key, - { - uri: value.uri != null ? value.uri.toString() : value.uri, - fallback: value.fallback.toString(), - timestamp: value.timestamp, - }, - ]), - ); - }, 1000), -); - -export function clearAvatarCache() { - avatarCache?.clear(); - avatarQueue.clear(); - void Container.context.workspaceState.update(WorkspaceState.Avatars, undefined); -} - -function ensureAvatarCache(cache: Map | undefined): asserts cache is Map { - if (cache == null) { - const avatars: [string, Avatar][] | undefined = Container.context.workspaceState - .get<[string, SerializedAvatar][]>(WorkspaceState.Avatars) - ?.map<[string, Avatar]>(([key, value]) => [ - key, - { - uri: value.uri != null ? Uri.parse(value.uri) : value.uri, - fallback: Uri.parse(value.fallback), - timestamp: value.timestamp, - }, - ]); - avatarCache = new Map(avatars); - } -} +const retryDecay = [ + MillisecondsPerDay * 7, // First item is cache expiration (since retries will be 0) + MillisecondsPerMinute, + MillisecondsPerMinute * 5, + MillisecondsPerMinute * 10, + MillisecondsPerHour, + MillisecondsPerDay, + MillisecondsPerDay * 7, +]; export function getAvatarUri( email: string | undefined, repoPathOrCommit: string | GitRevisionReference | undefined, - { fallback, size = 16 }: { fallback?: GravatarDefaultStyle; size?: number } = {}, + { defaultStyle, size = 16 }: { defaultStyle?: GravatarDefaultStyle; size?: number } = {}, ): Uri | Promise { ensureAvatarCache(avatarCache); if (email == null || email.length === 0) { - const key = `${missingGravatarHash}:${size}`; - - let avatar = avatarCache.get(key); - if (avatar == null) { - avatar = { - fallback: Uri.parse( - `https://www.gravatar.com/avatar/${missingGravatarHash}.jpg?s=${size}&d=${fallback}`, - ), - timestamp: Date.now(), - }; - avatarCache.set(key, avatar); - } - - return avatar.uri ?? avatar.fallback; + const avatar = createOrUpdateAvatar( + `${missingGravatarHash}:${size}`, + undefined, + missingGravatarHash, + size, + defaultStyle, + ); + return avatar.uri ?? avatar.fallback!; } const hash = Strings.md5(email.trim().toLowerCase(), 'hex'); const key = `${hash}:${size}`; - let avatar = avatarCache.get(key); - if (avatar == null) { - avatar = { - uri: getAvatarUriFromGitHubNoReplyAddress(email, size), - fallback: Uri.parse(`https://www.gravatar.com/avatar/${hash}.jpg?s=${size}&d=${fallback}`), - timestamp: Date.now(), - }; - avatarCache.set(key, avatar); - } - + const avatar = createOrUpdateAvatar( + key, + getAvatarUriFromGitHubNoReplyAddress(email, size), + hash, + size, + defaultStyle, + ); if (avatar.uri != null) return avatar.uri; let query = avatarQueue.get(key); - if (query == null && avatar.uri === undefined && repoPathOrCommit != null) { - query = getAvatarUriFromRemoteProvider(key, email, repoPathOrCommit, avatar.fallback, { size: size }).then( - uri => uri ?? avatar!.uri ?? avatar!.fallback, + if (query == null && repoPathOrCommit != null && hasAvatarExpired(avatar)) { + query = getAvatarUriFromRemoteProvider(avatar, key, email, repoPathOrCommit, { size: size }).then( + uri => uri ?? avatar.uri ?? avatar.fallback!, ); avatarQueue.set(key, query); } if (query != null) return query; - return avatar.uri ?? avatar.fallback; + return avatar.uri ?? avatar.fallback!; +} + +function createOrUpdateAvatar( + key: string, + uri: Uri | undefined, + hash: string, + size: number, + defaultStyle?: GravatarDefaultStyle, +): Avatar { + let avatar = avatarCache!.get(key); + if (avatar == null) { + avatar = { + uri: uri, + fallback: getAvatarUriFromGravatar(hash, size, defaultStyle), + timestamp: 0, + retries: 0, + }; + avatarCache!.set(key, avatar); + } else if (avatar.fallback == null) { + avatar.fallback = getAvatarUriFromGravatar(hash, size, defaultStyle); + } + return avatar; +} + +function ensureAvatarCache(cache: Map | undefined): asserts cache is Map { + if (cache == null) { + const avatars: [string, Avatar][] | undefined = Container.context.globalState + .get<[string, SerializedAvatar][]>(GlobalState.Avatars) + ?.map<[string, Avatar]>(([key, avatar]) => [ + key, + { + uri: Uri.parse(avatar.uri), + timestamp: avatar.timestamp, + retries: 0, + }, + ]); + avatarCache = new Map(avatars); + } +} + +function hasAvatarExpired(avatar: Avatar) { + return Date.now() >= avatar.timestamp + retryDecay[Math.min(avatar.retries, retryDecay.length - 1)]; +} + +function getAvatarUriFromGravatar( + hash: string, + size: number, + defaultStyle: GravatarDefaultStyle = GravatarDefaultStyle.Robot, +): Uri { + return Uri.parse(`https://www.gravatar.com/avatar/${hash}.jpg?s=${size}&d=${defaultStyle}`); } function getAvatarUriFromGitHubNoReplyAddress(email: string, size: number = 16): Uri | undefined { @@ -133,10 +169,10 @@ function getAvatarUriFromGitHubNoReplyAddress(email: string, size: number = 16): } async function getAvatarUriFromRemoteProvider( + avatar: Avatar, key: string, email: string, repoPathOrCommit: string | GitRevisionReference, - fallback: Uri, { size = 16 }: { size?: number } = {}, ) { ensureAvatarCache(avatarCache); @@ -152,26 +188,29 @@ async function getAvatarUriFromRemoteProvider( account = await remote?.provider.getAccountForCommit(repoPathOrCommit.ref, { avatarSize: size }); } if (account == null) { - avatarCache.set(key, { uri: null, fallback: fallback, timestamp: Date.now() }); + // If we have no account assume that won't change (without a reset), so set the timestamp to "never expire" + avatar.uri = undefined; + avatar.timestamp = Number.MAX_SAFE_INTEGER; + avatar.retries = 0; return undefined; } - const uri = Uri.parse(account.avatarUrl); - avatarCache.set(key, { uri: uri, fallback: fallback, timestamp: Date.now() }); + avatar.uri = Uri.parse(account.avatarUrl); + avatar.timestamp = Date.now(); + avatar.retries = 0; + if (account.email != null && Strings.equalsIgnoreCase(email, account.email)) { - avatarCache.set(`${Strings.md5(account.email.trim().toLowerCase(), 'hex')}:${size}`, { - uri: uri, - fallback: fallback, - timestamp: Date.now(), - }); + avatarCache.set(`${Strings.md5(account.email.trim().toLowerCase(), 'hex')}:${size}`, { ...avatar }); } _onDidFetchAvatar.fire({ email: email }); - return uri; + return avatar.uri; } catch { - avatarCache.set(key, { uri: null, fallback: fallback, timestamp: Date.now() }); + avatar.uri = undefined; + avatar.timestamp = Date.now(); + avatar.retries++; return undefined; } finally { @@ -192,3 +231,29 @@ export function getPresenceDataUri(status: ContactPresenceStatus) { return dataUri; } + +export function resetAvatarCache(reset: 'all' | 'failed' | 'fallback') { + switch (reset) { + case 'all': + void Container.context.globalState.update(GlobalState.Avatars, undefined); + avatarCache?.clear(); + avatarQueue.clear(); + break; + + case 'failed': + for (const avatar of avatarCache?.values() ?? []) { + // Reset failed requests + if (avatar.uri == null) { + avatar.timestamp = 0; + avatar.retries = 0; + } + } + break; + + case 'fallback': + for (const avatar of avatarCache?.values() ?? []) { + avatar.fallback = undefined; + } + break; + } +} diff --git a/src/constants.ts b/src/constants.ts index 187cbbd..fa37e53 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -132,7 +132,8 @@ export enum GlyphChars { } export enum GlobalState { - GitLensVersion = 'gitlensVersion', + Avatars = 'gitlens:avatars', + Version = 'gitlensVersion', } export const ImageMimetypes: Record = { @@ -182,7 +183,6 @@ export interface StarredRepositories { } export enum WorkspaceState { - Avatars = 'gitlens:avatars', BranchComparisons = 'gitlens:branch:comparisons', DefaultRemote = 'gitlens:remote:default', PinnedComparisons = 'gitlens:pinned:comparisons', diff --git a/src/container.ts b/src/container.ts index 891d607..54d9f5a 100644 --- a/src/container.ts +++ b/src/container.ts @@ -3,7 +3,7 @@ import { commands, ConfigurationChangeEvent, ConfigurationScope, ExtensionContex import { Autolinks } from './annotations/autolinks'; import { FileAnnotationController } from './annotations/fileAnnotationController'; import { LineAnnotationController } from './annotations/lineAnnotationController'; -import { clearAvatarCache } from './avatars'; +import { resetAvatarCache } from './avatars'; import { GitCodeLensController } from './codelens/codeLensController'; import { Commands, ToggleFileAnnotationCommandArgs } from './commands'; import { AnnotationsToggleMode, Config, configuration, ConfigurationWillChangeEvent } from './configuration'; @@ -108,7 +108,7 @@ export class Container { } if (configuration.changed(e.change, 'defaultGravatarsStyle')) { - clearAvatarCache(); + resetAvatarCache('fallback'); } if (configuration.changed(e.change, 'mode') || configuration.changed(e.change, 'modes')) { diff --git a/src/extension.ts b/src/extension.ts index f8fc054..64fb2e3 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -55,7 +55,7 @@ export async function activate(context: ExtensionContext) { const cfg = configuration.get(); - const previousVersion = context.globalState.get(GlobalState.GitLensVersion); + const previousVersion = context.globalState.get(GlobalState.Version); await migrateSettings(context, previousVersion); try { @@ -83,7 +83,7 @@ export async function activate(context: ExtensionContext) { notifyOnUnsupportedGitVersion(gitVersion); void showWelcomeOrWhatsNew(gitlensVersion, previousVersion); - void context.globalState.update(GlobalState.GitLensVersion, gitlensVersion); + void context.globalState.update(GlobalState.Version, gitlensVersion); Logger.log( `GitLens (v${gitlensVersion}${cfg.mode.active ? `, mode: ${cfg.mode.active}` : ''}) activated ${ diff --git a/src/git/formatters/commitFormatter.ts b/src/git/formatters/commitFormatter.ts index 4836d5e..29bea78 100644 --- a/src/git/formatters/commitFormatter.ts +++ b/src/git/formatters/commitFormatter.ts @@ -223,7 +223,7 @@ export class CommitFormatter extends Formatter { private async _getAvatarMarkdown(title: string) { const size = Container.config.hovers.avatarSize; const avatarPromise = this._item.getAvatarUri({ - fallback: Container.config.defaultGravatarsStyle, + defaultStyle: Container.config.defaultGravatarsStyle, size: size, }); return this._padOrTruncate( diff --git a/src/git/gitService.ts b/src/git/gitService.ts index 3cfa4a3..c30fad5 100644 --- a/src/git/gitService.ts +++ b/src/git/gitService.ts @@ -19,6 +19,7 @@ import { WorkspaceFoldersChangeEvent, } from 'vscode'; import { API as BuiltInGitApi, Repository as BuiltInGitRepository, GitExtension } from '../@types/git'; +import { resetAvatarCache } from '../avatars'; import { BranchSorting, configuration, TagSorting } from '../configuration'; import { CommandContext, DocumentSchemes, GlyphChars, setCommandContext } from '../constants'; import { Container } from '../container'; @@ -140,7 +141,12 @@ export class GitService implements Disposable { window.onDidChangeWindowState(this.onWindowStateChanged, this), workspace.onDidChangeWorkspaceFolders(this.onWorkspaceFoldersChanged, this), configuration.onDidChange(this.onConfigurationChanged, this), - Authentication.onDidChange(() => this._remotesWithApiProviderCache.clear()), + Authentication.onDidChange(e => { + if (e.reason === 'connected') { + resetAvatarCache('failed'); + } + this._remotesWithApiProviderCache.clear(); + }), ); this.onConfigurationChanged(configuration.initializingChangeEvent); diff --git a/src/git/models/commit.ts b/src/git/models/commit.ts index 6878f17..4fb053f 100644 --- a/src/git/models/commit.ts +++ b/src/git/models/commit.ts @@ -225,7 +225,7 @@ export abstract class GitCommit implements GitRevisionReference { return GitUri.getFormattedPath(this.fileName, options); } - getAvatarUri(options?: { fallback?: GravatarDefaultStyle; size?: number }): Uri | Promise { + getAvatarUri(options?: { defaultStyle?: GravatarDefaultStyle; size?: number }): Uri | Promise { return getAvatarUri(this.email, this, options); } diff --git a/src/git/models/contributor.ts b/src/git/models/contributor.ts index f908505..ad3b7ee 100644 --- a/src/git/models/contributor.ts +++ b/src/git/models/contributor.ts @@ -20,7 +20,7 @@ export class GitContributor { public readonly current: boolean = false, ) {} - getAvatarUri(options?: { fallback?: GravatarDefaultStyle; size?: number }): Uri | Promise { + getAvatarUri(options?: { defaultStyle?: GravatarDefaultStyle; size?: number }): Uri | Promise { return getAvatarUri(this.email, undefined /*this.repoPath*/, options); } diff --git a/src/views/contributorsView.ts b/src/views/contributorsView.ts index 176838b..d25c4c0 100644 --- a/src/views/contributorsView.ts +++ b/src/views/contributorsView.ts @@ -1,5 +1,6 @@ 'use strict'; import { commands, ConfigurationChangeEvent, Disposable, TreeItem, TreeItemCollapsibleState } from 'vscode'; +import { Avatars } from '../avatars'; import { configuration, ContributorsViewConfig, ViewFilesLayout } from '../configuration'; import { Container } from '../container'; import { Repository, RepositoryChange, RepositoryChangeEvent } from '../git/git'; @@ -13,9 +14,8 @@ import { unknownGitUri, ViewNode, } from './nodes'; -import { debug, Functions, gate } from '../system'; +import { debug, gate } from '../system'; import { ViewBase } from './viewBase'; -import { onDidFetchAvatar } from '../avatars'; export class ContributorsRepositoryNode extends SubscribeableViewNode { protected splatted = true; @@ -75,7 +75,7 @@ export class ContributorsRepositoryNode extends SubscribeableViewNode this.refresh(), 500)), + Avatars.onDidFetch(e => this.child?.updateAvatar(e.email)), ); } diff --git a/src/views/nodes/commitFileNode.ts b/src/views/nodes/commitFileNode.ts index f882b35..0811117 100644 --- a/src/views/nodes/commitFileNode.ts +++ b/src/views/nodes/commitFileNode.ts @@ -90,7 +90,7 @@ export class CommitFileNode extends ViewRefFileNode { if (!this.commit.isUncommitted && !(this.view instanceof StashesView) && this.view.config.avatars) { item.iconPath = this._options.unpublished ? new ThemeIcon('arrow-up') - : await this.commit.getAvatarUri({ fallback: Container.config.defaultGravatarsStyle }); + : await this.commit.getAvatarUri({ defaultStyle: Container.config.defaultGravatarsStyle }); } } diff --git a/src/views/nodes/commitNode.ts b/src/views/nodes/commitNode.ts index 5611753..d1f3ead 100644 --- a/src/views/nodes/commitNode.ts +++ b/src/views/nodes/commitNode.ts @@ -121,7 +121,7 @@ export class CommitNode extends ViewRefNode