Skip to content

Commit

Permalink
Reworks plus feature enablement for less gates
Browse files Browse the repository at this point in the history
Optimizes startup flow and churn during discovery
  • Loading branch information
eamodio committed Sep 29, 2022
1 parent a9c83b9 commit 77aea41
Show file tree
Hide file tree
Showing 12 changed files with 198 additions and 132 deletions.
22 changes: 11 additions & 11 deletions package.json
Expand Up @@ -8725,14 +8725,14 @@
"scm/sourceControl": [
{
"command": "gitlens.showGraphPage",
"when": "gitlens:enabled && config.gitlens.menus.scm.graph && config.gitlens.plusFeatures.enabled && scmProvider == git",
"when": "gitlens:enabled && config.gitlens.menus.scm.graph && gitlens:plus:enabled && scmProvider == git",
"group": "6_gitlens@1"
}
],
"scm/title": [
{
"command": "gitlens.showGraphPage",
"when": "gitlens:enabled && config.gitlens.menus.scmRepositoryInline.graph && config.gitlens.plusFeatures.enabled && scmProvider == git",
"when": "gitlens:enabled && config.gitlens.menus.scmRepositoryInline.graph && gitlens:plus:enabled && scmProvider == git",
"group": "navigation@-1000"
},
{
Expand All @@ -8742,7 +8742,7 @@
},
{
"command": "gitlens.showGraphPage",
"when": "gitlens:enabled && config.gitlens.menus.scmRepository.graph && config.gitlens.plusFeatures.enabled && scmProvider == git",
"when": "gitlens:enabled && config.gitlens.menus.scmRepository.graph && gitlens:plus:enabled && scmProvider == git",
"group": "2_z_gitlens@2"
}
],
Expand Down Expand Up @@ -8918,7 +8918,7 @@
},
{
"command": "gitlens.showGraphPage",
"when": "view =~ /^gitlens\\.views\\.commits/ && config.gitlens.plusFeatures.enabled",
"when": "view =~ /^gitlens\\.views\\.commits/ && gitlens:plus:enabled",
"group": "navigation@11"
},
{
Expand Down Expand Up @@ -8988,7 +8988,7 @@
},
{
"command": "gitlens.showGraphPage",
"when": "view =~ /^gitlens\\.views\\.commits/ && config.gitlens.plusFeatures.enabled",
"when": "view =~ /^gitlens\\.views\\.commits/ && gitlens:plus:enabled",
"group": "8_gitlens_toggles@0"
},
{
Expand Down Expand Up @@ -10353,7 +10353,7 @@
},
{
"command": "gitlens.showGraphPage",
"when": "viewItem =~ /gitlens:repo-folder\\b/ && config.gitlens.plusFeatures.enabled",
"when": "viewItem =~ /gitlens:repo-folder\\b/ && gitlens:plus:enabled",
"group": "inline@100"
},
{
Expand Down Expand Up @@ -10389,7 +10389,7 @@
},
{
"command": "gitlens.showGraphPage",
"when": "viewItem =~ /gitlens:repo-folder\\b/ && config.gitlens.plusFeatures.enabled",
"when": "viewItem =~ /gitlens:repo-folder\\b/ && gitlens:plus:enabled",
"group": "3_gitlens_explore@1"
},
{
Expand Down Expand Up @@ -11900,7 +11900,7 @@
{
"view": "gitlens.views.worktrees",
"contents": "[Create Worktree...](command:gitlens.views.createWorktree)",
"when": "gitlens:plus:allowed"
"when": "!gitlens:plus:required"
},
{
"view": "gitlens.views.worktrees",
Expand Down Expand Up @@ -11934,7 +11934,7 @@
"type": "webview",
"id": "gitlens.views.home",
"name": "Home",
"when": "!gitlens:disabled && config.gitlens.plusFeatures.enabled",
"when": "!gitlens:disabled && gitlens:plus:enabled",
"contextualTitle": "GitLens",
"icon": "$(gitlens-gitlens)",
"visibility": "visible"
Expand All @@ -11945,7 +11945,7 @@
"type": "webview",
"id": "gitlens.views.timeline",
"name": "Visual File History",
"when": "!gitlens:disabled && config.gitlens.plusFeatures.enabled",
"when": "!gitlens:disabled && gitlens:plus:enabled",
"contextualTitle": "GitLens",
"icon": "$(gitlens-history-view)",
"visibility": "visible"
Expand Down Expand Up @@ -12028,7 +12028,7 @@
{
"id": "gitlens.views.worktrees",
"name": "Worktrees",
"when": "!gitlens:disabled && !gitlens:hasVirtualFolders && config.gitlens.plusFeatures.enabled",
"when": "!gitlens:disabled && !gitlens:hasVirtualFolders && gitlens:plus:enabled",
"contextualTitle": "GitLens",
"icon": "$(gitlens-worktrees-view)",
"visibility": "collapsed"
Expand Down
2 changes: 1 addition & 1 deletion src/constants.ts
Expand Up @@ -269,7 +269,7 @@ export const enum ContextKeys {
Vsls = 'gitlens:vsls',

Plus = 'gitlens:plus',
PlusAllowed = 'gitlens:plus:allowed',
PlusEnabled = 'gitlens:plus:enabled',
PlusRequired = 'gitlens:plus:required',
PlusState = 'gitlens:plus:state',
}
Expand Down
8 changes: 7 additions & 1 deletion src/context.ts
@@ -1,9 +1,14 @@
import { commands } from 'vscode';
import { commands, EventEmitter } from 'vscode';
import type { ContextKeys } from './constants';
import { CoreCommands } from './constants';

const contextStorage = new Map<string, unknown>();

const _onDidChangeContext = new EventEmitter<
ContextKeys | `${ContextKeys.ActionPrefix}${string}` | `${ContextKeys.KeyPrefix}${string}`
>();
export const onDidChangeContext = _onDidChangeContext.event;

export function getContext<T>(key: ContextKeys): T | undefined;
export function getContext<T>(key: ContextKeys, defaultValue: T): T;
export function getContext<T>(key: ContextKeys, defaultValue?: T): T | undefined {
Expand All @@ -16,4 +21,5 @@ export async function setContext(
): Promise<void> {
contextStorage.set(key, value);
void (await commands.executeCommand(CoreCommands.SetContext, key, value));
_onDidChangeContext.fire(key);
}
12 changes: 12 additions & 0 deletions src/features.ts
Expand Up @@ -8,6 +8,18 @@ export const enum Features {
}

export type FeatureAccess =
| {
allowed: true;
subscription: { current: Subscription; required?: undefined };
visibility?: RepositoryVisibility;
}
| {
allowed: false | 'mixed';
subscription: { current: Subscription; required?: RequiredSubscriptionPlans };
visibility?: RepositoryVisibility;
};

export type RepoFeatureAccess =
| {
allowed: true;
subscription: { current: Subscription; required?: undefined };
Expand Down
126 changes: 75 additions & 51 deletions src/git/gitProviderService.ts
Expand Up @@ -17,14 +17,14 @@ import { ContextKeys, CoreGitConfiguration, GlyphChars, Schemes } from '../const
import type { Container } from '../container';
import { setContext } from '../context';
import { AccessDeniedError, ProviderNotFoundError } from '../errors';
import type { FeatureAccess, Features, PlusFeatures } from '../features';
import type { FeatureAccess, Features, PlusFeatures, RepoFeatureAccess } from '../features';
import type { RemoteProvider } from '../git/remotes/remoteProvider';
import { Logger } from '../logger';
import type { SubscriptionChangeEvent } from '../plus/subscription/subscriptionService';
import type { RepoComparisonKey } from '../repositories';
import { asRepoComparisonKey, Repositories } from '../repositories';
import type { RequiredSubscriptionPlans, Subscription } from '../subscription';
import { getSubscriptionPlanPriority, isSubscriptionPaidPlan, SubscriptionPlanId } from '../subscription';
import type { Subscription } from '../subscription';
import { isSubscriptionPaidPlan, SubscriptionPlanId } from '../subscription';
import { groupByFilterMap, groupByMap } from '../system/array';
import { gate } from '../system/decorators/gate';
import { debug, getLogScope, log } from '../system/decorators/log';
Expand Down Expand Up @@ -72,6 +72,7 @@ import type { RemoteProviders } from './remotes/remoteProviders';
import type { RichRemoteProvider } from './remotes/richRemoteProvider';
import type { GitSearch, SearchQuery } from './search';

const emptyArray = Object.freeze([]) as unknown as any[];
const maxDefaultBranchWeight = 100;
const weightedDefaultBranches = new Map<string, number>([
['master', maxDefaultBranchWeight],
Expand Down Expand Up @@ -264,14 +265,16 @@ export class GitProviderService implements Disposable {
}

get openRepositories(): Repository[] {
if (this.repositoryCount === 0) return emptyArray as Repository[];

const repositories = [...filter(this.repositories, r => !r.closed)];
if (repositories.length === 0) return repositories;

return Repository.sort(repositories);
}

get openRepositoryCount(): number {
return count(this.repositories, r => !r.closed);
return this.repositoryCount === 0 ? 0 : count(this.repositories, r => !r.closed);
}

get repositories(): IterableIterator<Repository> {
Expand Down Expand Up @@ -515,14 +518,23 @@ export class GitProviderService implements Disposable {
return this._subscription ?? (this._subscription = await this.container.subscription.getSubscription());
}

private _accessCache = new Map<string | undefined, Promise<FeatureAccess>>();
async access(feature?: PlusFeatures, repoPath?: string | Uri): Promise<FeatureAccess> {
let cacheKey;
if (repoPath != null) {
const { path } = this.getProvider(repoPath);
cacheKey = path;
private _accessCache: Map<string, Promise<RepoFeatureAccess>> &
Map<undefined, Promise<FeatureAccess | RepoFeatureAccess>> = new Map();
async access(feature: PlusFeatures | undefined, repoPath: string | Uri): Promise<RepoFeatureAccess>;
async access(feature?: PlusFeatures, repoPath?: string | Uri): Promise<FeatureAccess | RepoFeatureAccess>;
async access(feature?: PlusFeatures, repoPath?: string | Uri): Promise<FeatureAccess | RepoFeatureAccess> {
if (repoPath == null) {
let access = this._accessCache.get(undefined);
if (access == null) {
access = this.accessCore(feature, repoPath);
this._accessCache.set(undefined, access);
}
return access;
}

const { path } = this.getProvider(repoPath);
const cacheKey = path;

let access = this._accessCache.get(cacheKey);
if (access == null) {
access = this.accessCore(feature, repoPath);
Expand All @@ -532,42 +544,51 @@ export class GitProviderService implements Disposable {
return access;
}

private async accessCore(feature: PlusFeatures | undefined, repoPath: string | Uri): Promise<RepoFeatureAccess>;
private async accessCore(
feature?: PlusFeatures,
repoPath?: string | Uri,
): Promise<FeatureAccess | RepoFeatureAccess>;
@debug()
private async accessCore(feature?: PlusFeatures, repoPath?: string | Uri): Promise<FeatureAccess> {
private async accessCore(
_feature?: PlusFeatures,
repoPath?: string | Uri,
): Promise<FeatureAccess | RepoFeatureAccess> {
const subscription = await this.getSubscription();
if (subscription.account?.verified === false) {
return { allowed: false, subscription: { current: subscription } };
}

const plan = subscription.plan.effective.id;
if (isSubscriptionPaidPlan(plan)) {
return { allowed: true, subscription: { current: subscription } };
return { allowed: subscription.account?.verified !== false, subscription: { current: subscription } };
}

function getRepoAccess(
this: GitProviderService,
repoPath: string | Uri,
force: boolean = false,
): Promise<FeatureAccess> {
): Promise<RepoFeatureAccess> {
const { path: cacheKey } = this.getProvider(repoPath);

let access = force ? undefined : this._accessCache.get(cacheKey);
if (access == null) {
access = this.visibility(repoPath).then(visibility => {
if (visibility === RepositoryVisibility.Private) {
access = this.visibility(repoPath).then(
visibility => {
if (visibility === RepositoryVisibility.Private) {
return {
allowed: false,
subscription: { current: subscription, required: SubscriptionPlanId.Pro },
visibility: visibility,
};
}

return {
allowed: false,
subscription: { current: subscription, required: SubscriptionPlanId.Pro },
allowed: true,
subscription: { current: subscription },
visibility: visibility,
};
}

return {
allowed: true,
subscription: { current: subscription },
visibility: visibility,
};
});
},
// If there is a failure assume access is allowed
() => ({ allowed: true, subscription: { current: subscription } }),
);

this._accessCache.set(cacheKey, access);
}
Expand All @@ -585,28 +606,26 @@ export class GitProviderService implements Disposable {
return getRepoAccess.call(this, repositories[0].path);
}

let allowed = true;
let requiredPlan: RequiredSubscriptionPlans | undefined;
let requiredPriority = -1;

const maxPriority = getSubscriptionPlanPriority(SubscriptionPlanId.Pro);

for await (const result of fastestSettled(repositories.map(r => getRepoAccess.call(this, r.path)))) {
if (result.status !== 'fulfilled' || result.value.allowed) continue;

allowed = false;
const priority = getSubscriptionPlanPriority(result.value.subscription.required);
if (requiredPriority < priority) {
requiredPriority = priority;
requiredPlan = result.value.subscription.required;
}

if (requiredPriority >= maxPriority) break;
const visibility = await this.visibility();
switch (visibility) {
case RepositoriesVisibility.Private:
return {
allowed: false,
subscription: { current: subscription, required: SubscriptionPlanId.Pro },
visibility: RepositoryVisibility.Private,
};
case RepositoriesVisibility.Mixed:
return {
allowed: 'mixed',
subscription: { current: subscription, required: SubscriptionPlanId.Pro },
};
default:
return {
allowed: true,
subscription: { current: subscription },
visibility: RepositoryVisibility.Public,
};
}

return allowed
? { allowed: true, subscription: { current: subscription } }
: { allowed: false, subscription: { current: subscription, required: requiredPlan } };
}

// Pass force = true to bypass the cache and avoid a promise loop (where we used the cached promise we just created to try to resolve itself 🤦)
Expand All @@ -615,7 +634,7 @@ export class GitProviderService implements Disposable {

async ensureAccess(feature: PlusFeatures, repoPath?: string): Promise<void> {
const { allowed, subscription } = await this.access(feature, repoPath);
if (!allowed) throw new AccessDeniedError(subscription.current, subscription.required);
if (allowed === false) throw new AccessDeniedError(subscription.current, subscription.required);
}

supports(repoPath: string | Uri, feature: Features): Promise<boolean> {
Expand Down Expand Up @@ -2203,7 +2222,12 @@ export class GitProviderService implements Disposable {
path?: string,
options?: { force?: boolean; timeout?: number },
): Promise<string>;
async resolveReference(repoPath: string, ref: string, uri?: Uri, options?: { force?: boolean; timeout?: number }): Promise<string>;
async resolveReference(
repoPath: string,
ref: string,
uri?: Uri,
options?: { force?: boolean; timeout?: number },
): Promise<string>;
@gate()
@log()
async resolveReference(
Expand Down

0 comments on commit 77aea41

Please sign in to comment.