Skip to content

Commit a2cffea

Browse files
alexetalexet
authored
Merge pull request #202 from henrymercer/reduce-update-check-frequency
Reduce update check frequency
2 parents e966c33 + 93ed820 commit a2cffea

File tree

4 files changed

+273
-62
lines changed

4 files changed

+273
-62
lines changed

‎extensions/ql-vscode/src/distribution.ts

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import * as unzipper from "unzipper";
66
import * as url from "url";
77
import { ExtensionContext, Event } from "vscode";
88
import { DistributionConfig } from "./config";
9-
import { ProgressUpdate, showAndLogErrorMessage } from "./helpers";
9+
import { InvocationRateLimiter, InvocationRateLimiterResultKind, ProgressUpdate, showAndLogErrorMessage } from "./helpers";
1010
import { logger } from "./logging";
1111
import { getCodeQlCliVersion, tryParseVersionString, Version } from "./cli-version";
1212

@@ -55,6 +55,11 @@ export class DistributionManager implements DistributionProvider {
5555
this._config = config;
5656
this._extensionSpecificDistributionManager = new ExtensionSpecificDistributionManager(extensionContext, config, versionConstraint);
5757
this._onDidChangeDistribution = config.onDidChangeDistributionConfiguration;
58+
this._updateCheckRateLimiter = new InvocationRateLimiter(
59+
extensionContext,
60+
"extensionSpecificDistributionUpdateCheck",
61+
() => this._extensionSpecificDistributionManager.checkForUpdatesToDistribution()
62+
);
5863
this._versionConstraint = versionConstraint;
5964
}
6065

@@ -128,14 +133,21 @@ export class DistributionManager implements DistributionProvider {
128133
*
129134
* Returns a failed promise if an unexpected error occurs during installation.
130135
*/
131-
public async checkForUpdatesToExtensionManagedDistribution(): Promise<DistributionUpdateCheckResult> {
136+
public async checkForUpdatesToExtensionManagedDistribution(
137+
minSecondsSinceLastUpdateCheck: number): Promise<DistributionUpdateCheckResult> {
132138
const codeQlPath = await this.getCodeQlPathWithoutVersionCheck();
133139
const extensionManagedCodeQlPath = await this._extensionSpecificDistributionManager.getCodeQlPathWithoutVersionCheck();
134140
if (codeQlPath !== undefined && codeQlPath !== extensionManagedCodeQlPath) {
135141
// A distribution is present but it isn't managed by the extension.
136-
return createInvalidDistributionLocationResult();
142+
return createInvalidLocationResult();
143+
}
144+
const updateCheckResult = await this._updateCheckRateLimiter.invokeFunctionIfIntervalElapsed(minSecondsSinceLastUpdateCheck);
145+
switch (updateCheckResult.kind) {
146+
case InvocationRateLimiterResultKind.Invoked:
147+
return updateCheckResult.result;
148+
case InvocationRateLimiterResultKind.RateLimited:
149+
return createAlreadyCheckedRecentlyResult();
137150
}
138-
return this._extensionSpecificDistributionManager.checkForUpdatesToDistribution();
139151
}
140152

141153
/**
@@ -154,6 +166,7 @@ export class DistributionManager implements DistributionProvider {
154166

155167
private readonly _config: DistributionConfig;
156168
private readonly _extensionSpecificDistributionManager: ExtensionSpecificDistributionManager;
169+
private readonly _updateCheckRateLimiter: InvocationRateLimiter<DistributionUpdateCheckResult>;
157170
private readonly _onDidChangeDistribution: Event<void> | undefined;
158171
private readonly _versionConstraint: VersionConstraint;
159172
}
@@ -196,7 +209,7 @@ class ExtensionSpecificDistributionManager {
196209
const latestRelease = await this.getLatestRelease();
197210

198211
if (extensionSpecificRelease !== undefined && codeQlPath !== undefined && latestRelease.id === extensionSpecificRelease.id) {
199-
return createDistributionAlreadyUpToDateResult();
212+
return createAlreadyUpToDateResult();
200213
}
201214
return createUpdateAvailableResult(latestRelease);
202215
}
@@ -234,7 +247,7 @@ class ExtensionSpecificDistributionManager {
234247

235248
if (progressCallback && contentLength !== null) {
236249
const totalNumBytes = parseInt(contentLength, 10);
237-
const bytesToDisplayMB = (numBytes: number) => `${(numBytes/(1024*1024)).toFixed(1)} MB`;
250+
const bytesToDisplayMB = (numBytes: number) => `${(numBytes / (1024 * 1024)).toFixed(1)} MB`;
238251
const updateProgress = () => {
239252
progressCallback({
240253
step: numBytesDownloaded,
@@ -258,7 +271,7 @@ class ExtensionSpecificDistributionManager {
258271
.on("error", reject)
259272
);
260273

261-
this.bumpDistributionFolderIndex();
274+
await this.bumpDistributionFolderIndex();
262275

263276
logger.log(`Extracting CodeQL CLI to ${this.getDistributionStoragePath()}`);
264277
await extractZipArchive(archivePath, this.getDistributionStoragePath());
@@ -293,10 +306,10 @@ class ExtensionSpecificDistributionManager {
293306
return new ReleasesApiConsumer(ownerName, repositoryName, this._config.personalAccessToken);
294307
}
295308

296-
private bumpDistributionFolderIndex(): void {
309+
private async bumpDistributionFolderIndex(): Promise<void> {
297310
const index = this._extensionContext.globalState.get(
298311
ExtensionSpecificDistributionManager._currentDistributionFolderIndexStateKey, 0);
299-
this._extensionContext.globalState.update(
312+
await this._extensionContext.globalState.update(
300313
ExtensionSpecificDistributionManager._currentDistributionFolderIndexStateKey, index + 1);
301314
}
302315

@@ -317,8 +330,8 @@ class ExtensionSpecificDistributionManager {
317330
return this._extensionContext.globalState.get(ExtensionSpecificDistributionManager._installedReleaseStateKey);
318331
}
319332

320-
private storeInstalledRelease(release: Release | undefined): void {
321-
this._extensionContext.globalState.update(ExtensionSpecificDistributionManager._installedReleaseStateKey, release);
333+
private async storeInstalledRelease(release: Release | undefined): Promise<void> {
334+
await this._extensionContext.globalState.update(ExtensionSpecificDistributionManager._installedReleaseStateKey, release);
322335
}
323336

324337
private readonly _config: DistributionConfig;
@@ -532,39 +545,50 @@ interface NoDistributionResult {
532545
}
533546

534547
export enum DistributionUpdateCheckResultKind {
548+
AlreadyCheckedRecentlyResult,
535549
AlreadyUpToDate,
536-
InvalidDistributionLocation,
550+
InvalidLocation,
537551
UpdateAvailable
538552
}
539553

540-
type DistributionUpdateCheckResult = DistributionAlreadyUpToDateResult | InvalidDistributionLocationResult |
554+
type DistributionUpdateCheckResult = AlreadyCheckedRecentlyResult | AlreadyUpToDateResult | InvalidLocationResult |
541555
UpdateAvailableResult;
542556

543-
export interface DistributionAlreadyUpToDateResult {
557+
export interface AlreadyCheckedRecentlyResult {
558+
kind: DistributionUpdateCheckResultKind.AlreadyCheckedRecentlyResult
559+
}
560+
561+
export interface AlreadyUpToDateResult {
544562
kind: DistributionUpdateCheckResultKind.AlreadyUpToDate;
545563
}
546564

547565
/**
548566
* The distribution could not be installed or updated because it is not managed by the extension.
549567
*/
550-
export interface InvalidDistributionLocationResult {
551-
kind: DistributionUpdateCheckResultKind.InvalidDistributionLocation;
568+
export interface InvalidLocationResult {
569+
kind: DistributionUpdateCheckResultKind.InvalidLocation;
552570
}
553571

554572
export interface UpdateAvailableResult {
555573
kind: DistributionUpdateCheckResultKind.UpdateAvailable;
556574
updatedRelease: Release;
557575
}
558576

559-
function createDistributionAlreadyUpToDateResult(): DistributionAlreadyUpToDateResult {
577+
function createAlreadyCheckedRecentlyResult(): AlreadyCheckedRecentlyResult {
578+
return {
579+
kind: DistributionUpdateCheckResultKind.AlreadyCheckedRecentlyResult
580+
};
581+
}
582+
583+
function createAlreadyUpToDateResult(): AlreadyUpToDateResult {
560584
return {
561585
kind: DistributionUpdateCheckResultKind.AlreadyUpToDate
562586
};
563587
}
564588

565-
function createInvalidDistributionLocationResult(): InvalidDistributionLocationResult {
589+
function createInvalidLocationResult(): InvalidLocationResult {
566590
return {
567-
kind: DistributionUpdateCheckResultKind.InvalidDistributionLocation
591+
kind: DistributionUpdateCheckResultKind.InvalidLocation
568592
};
569593
}
570594

‎extensions/ql-vscode/src/extension.ts

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@ import * as archiveFilesystemProvider from './archive-filesystem-provider';
44
import { DistributionConfigListener, QueryServerConfigListener, QueryHistoryConfigListener } from './config';
55
import { DatabaseManager } from './databases';
66
import { DatabaseUI } from './databases-ui';
7-
import { DistributionUpdateCheckResultKind, DistributionManager, FindDistributionResult, FindDistributionResultKind, GithubApiError,
8-
DEFAULT_DISTRIBUTION_VERSION_CONSTRAINT, GithubRateLimitedError } from './distribution';
7+
import {
8+
DistributionUpdateCheckResultKind, DistributionManager, FindDistributionResult, FindDistributionResultKind, GithubApiError,
9+
DEFAULT_DISTRIBUTION_VERSION_CONSTRAINT, GithubRateLimitedError
10+
} from './distribution';
911
import * as helpers from './helpers';
1012
import { spawnIdeServer } from './ide-server';
1113
import { InterfaceManager, WebviewReveal } from './interface';
@@ -83,29 +85,32 @@ export async function activate(ctx: ExtensionContext): Promise<void> {
8385
helpers.showAndLogErrorMessage(`Can't execute ${command}: waiting to finish loading CodeQL CLI.`);
8486
});
8587

86-
interface ReportingConfig {
88+
interface DistributionUpdateConfig {
89+
isUserInitiated: boolean;
8790
shouldDisplayMessageWhenNoUpdates: boolean;
88-
shouldErrorIfUpdateFails: boolean;
8991
}
9092

91-
async function installOrUpdateDistributionWithProgressTitle(progressTitle: string, reportingConfig: ReportingConfig): Promise<void> {
92-
const result = await distributionManager.checkForUpdatesToExtensionManagedDistribution();
93+
async function installOrUpdateDistributionWithProgressTitle(progressTitle: string, config: DistributionUpdateConfig): Promise<void> {
94+
const minSecondsSinceLastUpdateCheck = config.isUserInitiated ? 0 : 86400;
95+
const noUpdatesLoggingFunc = config.shouldDisplayMessageWhenNoUpdates ?
96+
helpers.showAndLogInformationMessage : async (message: string) => logger.log(message);
97+
const result = await distributionManager.checkForUpdatesToExtensionManagedDistribution(minSecondsSinceLastUpdateCheck);
9398
switch (result.kind) {
99+
case DistributionUpdateCheckResultKind.AlreadyCheckedRecentlyResult:
100+
logger.log("Didn't perform CodeQL CLI update check since a check was already performed within the previous " +
101+
`${minSecondsSinceLastUpdateCheck} seconds.`);
102+
break;
94103
case DistributionUpdateCheckResultKind.AlreadyUpToDate:
95-
if (reportingConfig.shouldDisplayMessageWhenNoUpdates) {
96-
helpers.showAndLogInformationMessage("CodeQL CLI already up to date.");
97-
}
104+
await noUpdatesLoggingFunc("CodeQL CLI already up to date.");
98105
break;
99-
case DistributionUpdateCheckResultKind.InvalidDistributionLocation:
100-
if (reportingConfig.shouldDisplayMessageWhenNoUpdates) {
101-
helpers.showAndLogErrorMessage("CodeQL CLI is installed externally so could not be updated.");
102-
}
106+
case DistributionUpdateCheckResultKind.InvalidLocation:
107+
await noUpdatesLoggingFunc("CodeQL CLI is installed externally so could not be updated.");
103108
break;
104109
case DistributionUpdateCheckResultKind.UpdateAvailable:
105110
if (beganMainExtensionActivation) {
106111
const updateAvailableMessage = `Version "${result.updatedRelease.name}" of the CodeQL CLI is now available. ` +
107112
"The update will be installed after Visual Studio Code restarts. Restart now to upgrade?";
108-
ctx.globalState.update(shouldUpdateOnNextActivationKey, true);
113+
await ctx.globalState.update(shouldUpdateOnNextActivationKey, true);
109114
if (await helpers.showInformationMessageWithAction(updateAvailableMessage, "Restart and Upgrade")) {
110115
await commands.executeCommand("workbench.action.reloadWindow");
111116
}
@@ -118,7 +123,7 @@ export async function activate(ctx: ExtensionContext): Promise<void> {
118123
await helpers.withProgress(progressOptions, progress =>
119124
distributionManager.installExtensionManagedDistributionRelease(result.updatedRelease, progress));
120125

121-
ctx.globalState.update(shouldUpdateOnNextActivationKey, false);
126+
await ctx.globalState.update(shouldUpdateOnNextActivationKey, false);
122127
helpers.showAndLogInformationMessage(`CodeQL CLI updated to version "${result.updatedRelease.name}".`);
123128
}
124129
break;
@@ -127,7 +132,7 @@ export async function activate(ctx: ExtensionContext): Promise<void> {
127132
}
128133
}
129134

130-
async function installOrUpdateDistribution(reportingConfig: ReportingConfig): Promise<void> {
135+
async function installOrUpdateDistribution(config: DistributionUpdateConfig): Promise<void> {
131136
if (isInstallingOrUpdatingDistribution) {
132137
throw new Error("Already installing or updating CodeQL CLI");
133138
}
@@ -137,11 +142,11 @@ export async function activate(ctx: ExtensionContext): Promise<void> {
137142
const messageText = willUpdateCodeQl ? "Updating CodeQL CLI" :
138143
codeQlInstalled ? "Checking for updates to CodeQL CLI" : "Installing CodeQL CLI";
139144
try {
140-
await installOrUpdateDistributionWithProgressTitle(messageText, reportingConfig);
145+
await installOrUpdateDistributionWithProgressTitle(messageText, config);
141146
} catch (e) {
142147
// Don't rethrow the exception, because if the config is changed, we want to be able to retry installing
143148
// or updating the distribution.
144-
const alertFunction = (codeQlInstalled && !reportingConfig.shouldErrorIfUpdateFails) ?
149+
const alertFunction = (codeQlInstalled && !config.isUserInitiated) ?
145150
helpers.showAndLogWarningMessage : helpers.showAndLogErrorMessage;
146151
const taskDescription = (willUpdateCodeQl ? "update" :
147152
codeQlInstalled ? "check for updates to" : "install") + " CodeQL CLI";
@@ -180,8 +185,8 @@ export async function activate(ctx: ExtensionContext): Promise<void> {
180185
return result;
181186
}
182187

183-
async function installOrUpdateThenTryActivate(reportingConfig: ReportingConfig): Promise<void> {
184-
await installOrUpdateDistribution(reportingConfig);
188+
async function installOrUpdateThenTryActivate(config: DistributionUpdateConfig): Promise<void> {
189+
await installOrUpdateDistribution(config);
185190

186191
// Display the warnings even if the extension has already activated.
187192
const distributionResult = await getDistributionDisplayingDistributionWarnings();
@@ -194,26 +199,26 @@ export async function activate(ctx: ExtensionContext): Promise<void> {
194199
const chosenAction = await helpers.showAndLogErrorMessage(`Can't execute ${command}: missing CodeQL CLI.`, installActionName);
195200
if (chosenAction === installActionName) {
196201
installOrUpdateThenTryActivate({
197-
shouldDisplayMessageWhenNoUpdates: false,
198-
shouldErrorIfUpdateFails: true
202+
isUserInitiated: true,
203+
shouldDisplayMessageWhenNoUpdates: false
199204
});
200205
}
201206
});
202207
}
203208
}
204209

205210
ctx.subscriptions.push(distributionConfigListener.onDidChangeDistributionConfiguration(() => installOrUpdateThenTryActivate({
206-
shouldDisplayMessageWhenNoUpdates: false,
207-
shouldErrorIfUpdateFails: true
211+
isUserInitiated: true,
212+
shouldDisplayMessageWhenNoUpdates: false
208213
})));
209214
ctx.subscriptions.push(commands.registerCommand(checkForUpdatesCommand, () => installOrUpdateThenTryActivate({
210-
shouldDisplayMessageWhenNoUpdates: true,
211-
shouldErrorIfUpdateFails: true
215+
isUserInitiated: true,
216+
shouldDisplayMessageWhenNoUpdates: true
212217
})));
213218

214219
await installOrUpdateThenTryActivate({
215-
shouldDisplayMessageWhenNoUpdates: false,
216-
shouldErrorIfUpdateFails: !!ctx.globalState.get(shouldUpdateOnNextActivationKey)
220+
isUserInitiated: !!ctx.globalState.get(shouldUpdateOnNextActivationKey),
221+
shouldDisplayMessageWhenNoUpdates: false
217222
});
218223
}
219224

0 commit comments

Comments
 (0)