Refactors autolinks files into models/utils layout

Adds caching for refset resolution to improve performance

Author: eamodio

src/autolinks/__tests__/autolinks.test.ts

@@ -1,8 +1,8 @@
 import * as assert from 'assert';
 import { suite, test } from 'mocha';
 import { map } from '../../system/iterable';
-import type { Autolink, RefSet } from '../autolinks.utils';
-import { getAutolinks, getBranchAutolinks } from '../autolinks.utils';
+import type { Autolink, RefSet } from '../models/autolinks';
+import { getAutolinks, getBranchAutolinks } from '../utils/-webview/autolinks.utils';
 
 const mockRefSets = (prefixes: string[] = ['']): RefSet[] =>
 	prefixes.map(prefix => [

src/autolinks/autolinks.ts renamed to src/autolinks/autolinksProvider.ts

@@ -7,6 +7,7 @@ import type { Container } from '../container';
 import type { GitRemote } from '../git/models/remote';
 import type { RemoteProviderId } from '../git/remotes/remoteProvider';
 import { getIssueOrPullRequestHtmlIcon, getIssueOrPullRequestMarkdownIcon } from '../git/utils/-webview/icons';
+import type { ConfiguredIntegrationsChangeEvent } from '../plus/integrations/authentication/configuredIntegrationService';
 import type { HostingIntegration, Integration, IssueIntegration } from '../plus/integrations/integration';
 import { IntegrationBase } from '../plus/integrations/integration';
 import { remoteProviderIdToIntegrationId } from '../plus/integrations/integrationService';
@@ -18,6 +19,7 @@ import { join, map } from '../system/iterable';
 import { Logger } from '../system/logger';
 import { escapeMarkdown } from '../system/markdown';
 import { getSettledValue, isPromise } from '../system/promise';
+import { PromiseCache } from '../system/promiseCache';
 import { capitalize, encodeHtmlWeak, getSuperscript } from '../system/string';
 import type {
 	Autolink,
@@ -26,24 +28,28 @@ import type {
 	EnrichedAutolink,
 	MaybeEnrichedAutolink,
 	RefSet,
-} from './autolinks.utils';
+} from './models/autolinks';
 import {
 	ensureCachedRegex,
 	getAutolinks,
 	getBranchAutolinks,
 	isDynamic,
 	numRegex,
 	supportedAutolinkIntegrations,
-} from './autolinks.utils';
+} from './utils/-webview/autolinks.utils';
 
 const emptyAutolinkMap = Object.freeze(new Map<string, Autolink>());
 
-export class Autolinks implements Disposable {
-	protected _disposable: Disposable | undefined;
+export class AutolinksProvider implements Disposable {
+	private _disposable: Disposable | undefined;
 	private _references: CacheableAutolinkReference[] = [];
+	private _refsetCache = new PromiseCache<string | undefined, RefSet[]>({ accessTTL: 1000 * 60 * 60 });
 
 	constructor(private readonly container: Container) {
-		this._disposable = Disposable.from(configuration.onDidChange(this.onConfigurationChanged, this));
+		this._disposable = Disposable.from(
+			configuration.onDidChange(this.onConfigurationChanged, this),
+			container.integrations.onDidChangeConfiguredIntegrations(this.onConfiguredIntegrationsChanged, this),
+		);
 
 		this.setAutolinksFromConfig();
 	}
@@ -55,9 +61,14 @@ export class Autolinks implements Disposable {
 	private onConfigurationChanged(e?: ConfigurationChangeEvent) {
 		if (configuration.changed(e, 'autolinks')) {
 			this.setAutolinksFromConfig();
+			this._refsetCache.clear();
 		}
 	}
 
+	private onConfiguredIntegrationsChanged(_e: ConfiguredIntegrationsChangeEvent) {
+		this._refsetCache.clear();
+	}
+
 	private setAutolinksFromConfig() {
 		const autolinks = configuration.get('autolinks');
 		// Since VS Code's configuration objects are live we need to copy them to avoid writing back to the configuration
@@ -118,57 +129,40 @@ export class Autolinks implements Disposable {
 	}
 
 	/** Collects custom-configured autolink references into @param refsets */
-	private collectCustomAutolinks(
-		remote: GitRemote | undefined,
-		refsets: RefSet[],
-		options?: { excludeCustom?: boolean },
-	): void {
-		if (this._references.length && (remote?.provider == null || !options?.excludeCustom)) {
+	private collectCustomAutolinks(refsets: RefSet[]): void {
+		if (this._references.length) {
 			refsets.push([undefined, this._references]);
 		}
 	}
 
-	private async getRefSets(remote?: GitRemote, options?: { excludeCustom?: boolean }) {
-		const refsets: RefSet[] = [];
+	private async getRefSets(remote?: GitRemote) {
+		return this._refsetCache.get(remote?.remoteKey, async () => {
+			const refsets: RefSet[] = [];
 
-		await this.collectIntegrationAutolinks(remote, refsets);
-		this.collectRemoteAutolinks(remote, refsets);
-		this.collectCustomAutolinks(remote, refsets, options);
+			await this.collectIntegrationAutolinks(remote, refsets);
+			this.collectRemoteAutolinks(remote, refsets);
+			this.collectCustomAutolinks(refsets);
 
-		return refsets;
+			return refsets;
+		});
 	}
 
 	/** @returns A sorted list of autolinks. the first match is the most relevant */
-	async getBranchAutolinks(
-		branchName: string,
-		remote?: GitRemote,
-		options?: { excludeCustom?: boolean },
-	): Promise<Map<string, Autolink>> {
-		const refsets = await this.getRefSets(remote, options);
+	async getBranchAutolinks(branchName: string, remote?: GitRemote): Promise<Map<string, Autolink>> {
+		const refsets = await this.getRefSets(remote);
 		if (refsets.length === 0) return emptyAutolinkMap;
 
 		return getBranchAutolinks(branchName, refsets);
 	}
 
-	async getAutolinks(message: string, remote?: GitRemote): Promise<Map<string, Autolink>>;
-	async getAutolinks(
-		message: string,
-		remote: GitRemote,
-		// eslint-disable-next-line @typescript-eslint/unified-signatures
-		options?: { excludeCustom?: boolean },
-	): Promise<Map<string, Autolink>>;
-	@debug<Autolinks['getAutolinks']>({
+	@debug<AutolinksProvider['getAutolinks']>({
 		args: {
 			0: '<message>',
 			1: false,
 		},
 	})
-	async getAutolinks(
-		message: string,
-		remote?: GitRemote,
-		options?: { excludeCustom?: boolean },
-	): Promise<Map<string, Autolink>> {
-		const refsets = await this.getRefSets(remote, options);
+	async getAutolinks(message: string, remote?: GitRemote): Promise<Map<string, Autolink>> {
+		const refsets = await this.getRefSets(remote);
 		if (refsets.length === 0) return emptyAutolinkMap;
 
 		return getAutolinks(message, refsets);
@@ -191,7 +185,7 @@ export class Autolinks implements Disposable {
 		autolinks: Map<string, Autolink>,
 		remote: GitRemote | undefined,
 	): Promise<Map<string, EnrichedAutolink> | undefined>;
-	@debug<Autolinks['getEnrichedAutolinks']>({
+	@debug<AutolinksProvider['getEnrichedAutolinks']>({
 		args: {
 			0: messageOrAutolinks =>
 				typeof messageOrAutolinks === 'string' ? '<message>' : `autolinks=${messageOrAutolinks.size}`,
@@ -264,7 +258,7 @@ export class Autolinks implements Disposable {
 		return enrichedAutolinks;
 	}
 
-	@debug<Autolinks['linkify']>({
+	@debug<AutolinksProvider['linkify']>({
 		args: {
 			0: '<text>',
 			2: remotes => remotes?.length,

src/autolinks/index.ts

@@ -0,0 +1,88 @@
+import type { IssueOrPullRequest } from '../../git/models/issueOrPullRequest';
+import type { ProviderReference } from '../../git/models/remoteProvider';
+import type { ResourceDescriptor } from '../../plus/integrations/integration';
+import type { MaybePausedResult } from '../../system/promise';
+
+export type AutolinkType = 'issue' | 'pullrequest';
+export type AutolinkReferenceType = 'commit' | 'branch';
+
+export interface AutolinkReference {
+	/** Short prefix to match to generate autolinks for the external resource */
+	readonly prefix: string;
+	/** URL of the external resource to link to */
+	readonly url: string;
+	/** Whether alphanumeric characters should be allowed in `<num>` */
+	readonly alphanumeric: boolean;
+	/** Whether case should be ignored when matching the prefix */
+	readonly ignoreCase: boolean;
+	readonly title: string | undefined;
+
+	readonly type?: AutolinkType;
+	readonly referenceType?: AutolinkReferenceType;
+	readonly description?: string;
+	readonly descriptor?: ResourceDescriptor;
+}
+
+export interface Autolink extends AutolinkReference {
+	provider?: ProviderReference;
+	id: string;
+	index?: number;
+
+	tokenize?:
+		| ((
+				text: string,
+				outputFormat: 'html' | 'markdown' | 'plaintext',
+				tokenMapping: Map<string, string>,
+				enrichedAutolinks?: Map<string, MaybeEnrichedAutolink>,
+				prs?: Set<string>,
+				footnotes?: Map<number, string>,
+		  ) => string)
+		| null;
+}
+
+export type EnrichedAutolink = [
+	issueOrPullRequest: Promise<IssueOrPullRequest | undefined> | undefined,
+	autolink: Autolink,
+];
+
+export type MaybeEnrichedAutolink = readonly [
+	issueOrPullRequest: MaybePausedResult<IssueOrPullRequest | undefined> | undefined,
+	autolink: Autolink,
+];
+
+export interface CacheableAutolinkReference extends AutolinkReference {
+	tokenize?:
+		| ((
+				text: string,
+				outputFormat: 'html' | 'markdown' | 'plaintext',
+				tokenMapping: Map<string, string>,
+				enrichedAutolinks?: Map<string, MaybeEnrichedAutolink>,
+				prs?: Set<string>,
+				footnotes?: Map<number, string>,
+		  ) => string)
+		| null;
+
+	messageHtmlRegex?: RegExp;
+	messageMarkdownRegex?: RegExp;
+	messageRegex?: RegExp;
+	branchNameRegex?: RegExp;
+}
+
+export interface DynamicAutolinkReference {
+	tokenize?:
+		| ((
+				text: string,
+				outputFormat: 'html' | 'markdown' | 'plaintext',
+				tokenMapping: Map<string, string>,
+				enrichedAutolinks?: Map<string, MaybeEnrichedAutolink>,
+				prs?: Set<string>,
+				footnotes?: Map<number, string>,
+		  ) => string)
+		| null;
+	parse: (text: string, autolinks: Map<string, Autolink>) => void;
+}
+
+export type RefSet = [
+	ProviderReference | undefined,
+	(AutolinkReference | DynamicAutolinkReference)[] | CacheableAutolinkReference[],
+];

src/autolinks/models/autolinks.ts

@@ -1,75 +1,13 @@
-import { IssueIntegrationId } from '../constants.integrations';
-import type { IssueOrPullRequest } from '../git/models/issueOrPullRequest';
-import type { ProviderReference } from '../git/models/remoteProvider';
-import type { ResourceDescriptor } from '../plus/integrations/integration';
-import { escapeMarkdown } from '../system/markdown';
-import type { MaybePausedResult } from '../system/promise';
-import { encodeHtmlWeak, escapeRegex } from '../system/string';
-
-export type AutolinkType = 'issue' | 'pullrequest';
-export type AutolinkReferenceType = 'commit' | 'branch';
-
-export interface AutolinkReference {
-	/** Short prefix to match to generate autolinks for the external resource */
-	readonly prefix: string;
-	/** URL of the external resource to link to */
-	readonly url: string;
-	/** Whether alphanumeric characters should be allowed in `<num>` */
-	readonly alphanumeric: boolean;
-	/** Whether case should be ignored when matching the prefix */
-	readonly ignoreCase: boolean;
-	readonly title: string | undefined;
-
-	readonly type?: AutolinkType;
-	readonly referenceType?: AutolinkReferenceType;
-	readonly description?: string;
-	readonly descriptor?: ResourceDescriptor;
-}
-
-export interface Autolink extends AutolinkReference {
-	provider?: ProviderReference;
-	id: string;
-	index?: number;
-
-	tokenize?:
-		| ((
-				text: string,
-				outputFormat: 'html' | 'markdown' | 'plaintext',
-				tokenMapping: Map<string, string>,
-				enrichedAutolinks?: Map<string, MaybeEnrichedAutolink>,
-				prs?: Set<string>,
-				footnotes?: Map<number, string>,
-		  ) => string)
-		| null;
-}
-
-export type EnrichedAutolink = [
-	issueOrPullRequest: Promise<IssueOrPullRequest | undefined> | undefined,
-	autolink: Autolink,
-];
-
-export type MaybeEnrichedAutolink = readonly [
-	issueOrPullRequest: MaybePausedResult<IssueOrPullRequest | undefined> | undefined,
-	autolink: Autolink,
-];
-
-export interface CacheableAutolinkReference extends AutolinkReference {
-	tokenize?:
-		| ((
-				text: string,
-				outputFormat: 'html' | 'markdown' | 'plaintext',
-				tokenMapping: Map<string, string>,
-				enrichedAutolinks?: Map<string, MaybeEnrichedAutolink>,
-				prs?: Set<string>,
-				footnotes?: Map<number, string>,
-		  ) => string)
-		| null;
-
-	messageHtmlRegex?: RegExp;
-	messageMarkdownRegex?: RegExp;
-	messageRegex?: RegExp;
-	branchNameRegex?: RegExp;
-}
+import { IssueIntegrationId } from '../../../constants.integrations';
+import { escapeMarkdown } from '../../../system/markdown';
+import { encodeHtmlWeak, escapeRegex } from '../../../system/string';
+import type {
+	Autolink,
+	AutolinkReference,
+	CacheableAutolinkReference,
+	DynamicAutolinkReference,
+	RefSet,
+} from '../../models/autolinks';
 
 export function serializeAutolink(value: Autolink): Autolink {
 	const serialized: Autolink = {
@@ -95,20 +33,6 @@ export function serializeAutolink(value: Autolink): Autolink {
 	return serialized;
 }
 
-export interface DynamicAutolinkReference {
-	tokenize?:
-		| ((
-				text: string,
-				outputFormat: 'html' | 'markdown' | 'plaintext',
-				tokenMapping: Map<string, string>,
-				enrichedAutolinks?: Map<string, MaybeEnrichedAutolink>,
-				prs?: Set<string>,
-				footnotes?: Map<number, string>,
-		  ) => string)
-		| null;
-	parse: (text: string, autolinks: Map<string, Autolink>) => void;
-}
-
 export const supportedAutolinkIntegrations = [IssueIntegrationId.Jira];
 
 export function isDynamic(ref: AutolinkReference | DynamicAutolinkReference): ref is DynamicAutolinkReference {
@@ -119,11 +43,6 @@ function isCacheable(ref: AutolinkReference | DynamicAutolinkReference): ref is
 	return 'prefix' in ref && ref.prefix != null && 'url' in ref && ref.url != null;
 }
 
-export type RefSet = [
-	ProviderReference | undefined,
-	(AutolinkReference | DynamicAutolinkReference)[] | CacheableAutolinkReference[],
-];
-
 /**
  * Compares autolinks
  * @returns non-0 result that means a probability of the autolink `b` is more relevant of the autolink `a`