Skip to content

Commit

Permalink
updateChannel fixes. (#5465)
Browse files Browse the repository at this point in the history 
Fixes:
- It doesn't check for a downgrade to non-insiders on startup, only when the setting changes.
- There was a bug with the timer code that checks for a new insiders every hour.
- It wasn't bypassing the autoUpdate check when the user manually switches the updateChannel.
  • Loading branch information
@sean-mcmanus
sean-mcmanus authored Jun 22, 2020
1 parent 929d548 commit d80e04e
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 24 deletions.
26 changes: 18 additions & 8 deletions Extension/src/LanguageServer/extension.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -474,10 +474,15 @@ function realActivation(): void {
if (vscodeVersion.isGreaterThan(minimumSupportedVersionForInsidersUpgrades, "insider")) {
insiderUpdateEnabled = true;
if (settings.updateChannel === 'Default') {
suggestInsidersChannel();
const userVersion: PackageVersion = new PackageVersion(util.packageJson.version);
if (userVersion.suffix === "insiders") {
checkAndApplyUpdate(settings.updateChannel, false);
} else {
suggestInsidersChannel();
}
} else if (settings.updateChannel === 'Insiders') {
insiderUpdateTimer = global.setInterval(checkAndApplyUpdate, insiderUpdateTimerInterval, settings.updateChannel);
checkAndApplyUpdate(settings.updateChannel);
insiderUpdateTimer = global.setInterval(checkAndApplyUpdateOnTimer, insiderUpdateTimerInterval);
checkAndApplyUpdate(settings.updateChannel, false);
}
}
}
Expand Down Expand Up @@ -514,10 +519,10 @@ function onDidChangeSettings(event: vscode.ConfigurationChangeEvent): void {
if (newUpdateChannel === 'Default') {
clearInterval(insiderUpdateTimer);
} else if (newUpdateChannel === 'Insiders') {
insiderUpdateTimer = global.setInterval(checkAndApplyUpdate, insiderUpdateTimerInterval);
insiderUpdateTimer = global.setInterval(checkAndApplyUpdateOnTimer, insiderUpdateTimerInterval);
}

checkAndApplyUpdate(newUpdateChannel);
checkAndApplyUpdate(newUpdateChannel, true);
}
}
}
Expand Down Expand Up @@ -764,7 +769,7 @@ async function suggestInsidersChannel(): Promise<void> {
}
let buildInfo: BuildInfo | undefined;
try {
buildInfo = await getTargetBuildInfo("Insiders");
buildInfo = await getTargetBuildInfo("Insiders", false);
} catch (error) {
console.log(`${cppInstallVsixStr}${error.message}`);
if (error.message.indexOf('/') !== -1 || error.message.indexOf('\\') !== -1) {
Expand Down Expand Up @@ -856,20 +861,25 @@ async function applyUpdate(buildInfo: BuildInfo): Promise<void> {
}
}

async function checkAndApplyUpdateOnTimer(): Promise<void> {
return checkAndApplyUpdate('Insiders', false);
}

/**
* Query package.json and the GitHub API to determine whether the user should update, if so then install the update.
* The update can be an upgrade or downgrade depending on the the updateChannel setting.
* @param updateChannel The user's updateChannel setting.
* @param isFromSettingsChange True if the invocation is the result of a settings change.
*/
async function checkAndApplyUpdate(updateChannel: string): Promise<void> {
async function checkAndApplyUpdate(updateChannel: string, isFromSettingsChange: boolean): Promise<void> {
// If we have buildInfo cache, we should use it.
let buildInfo: BuildInfo | undefined = buildInfoCache;
// clear buildInfo cache.
buildInfoCache = undefined;

if (!buildInfo) {
try {
buildInfo = await getTargetBuildInfo(updateChannel);
buildInfo = await getTargetBuildInfo(updateChannel, isFromSettingsChange);
} catch (error) {
telemetry.logLanguageServerEvent('installVsix', { 'error': error.message, 'success': 'false' });
}
Expand Down
10 changes: 6 additions & 4 deletions Extension/src/githubAPI.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,11 @@ export interface BuildInfo {
/**
* Use the GitHub API to retrieve the download URL of the extension version the user should update to, if any.
* @param updateChannel The user's updateChannel setting.
* @param isFromSettingsChange True if the invocation is the result of a settings change.
* @return Download URL for the extension VSIX package that the user should install. If the user
* does not need to update, resolves to undefined.
*/
export async function getTargetBuildInfo(updateChannel: string): Promise<BuildInfo | undefined> {
export async function getTargetBuildInfo(updateChannel: string, isFromSettingsChange: boolean): Promise<BuildInfo | undefined> {
return getReleaseJson()
.then(builds => {
if (!builds || builds.length === 0) {
Expand All @@ -144,7 +145,7 @@ export async function getTargetBuildInfo(updateChannel: string): Promise<BuildIn
return undefined;
}

const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel);
const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel, isFromSettingsChange);
if (targetBuild === undefined) {
// no action
telemetry.logLanguageServerEvent("UpgradeCheck", { "action": "none" });
Expand Down Expand Up @@ -180,10 +181,11 @@ export async function getTargetBuildInfo(updateChannel: string): Promise<BuildIn
* @param builds The GitHub release list parsed as an array of Builds.
* @param userVersion The verion of the extension that the user is running.
* @param updateChannel The user's updateChannel setting.
* @param isFromSettingsChange True if the invocation is the result of a settings change.
* @return The Build if the user should update to it, otherwise undefined.
*/
export function getTargetBuild(builds: Build[], userVersion: PackageVersion, updateChannel: string): Build | undefined {
if (!vscode.workspace.getConfiguration("extensions", null).get<boolean>("autoUpdate")) {
export function getTargetBuild(builds: Build[], userVersion: PackageVersion, updateChannel: string, isFromSettingsChange: boolean): Build | undefined {
if (!isFromSettingsChange && !vscode.workspace.getConfiguration("extensions", null).get<boolean>("autoUpdate")) {
return undefined;
}
const latestVersion: PackageVersion = new PackageVersion(builds[0].name);
Expand Down
24 changes: 12 additions & 12 deletions Extension/test/unitTests/updowngrade.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ suite("UpgradeDowngrade", () => {
name: release0, assets: three_assets}];

const userVersion: PackageVersion = new PackageVersion(insider2);
const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel);
const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel, false);
assert.equal(targetBuild.name, release0);
});
});
Expand All @@ -46,7 +46,7 @@ suite("UpgradeDowngrade", () => {
name: release0, assets: three_assets}];

const userVersion: PackageVersion = new PackageVersion(insider1);
const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel);
const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel, false);
assert.equal(targetBuild, undefined);
});
test("Insider to Insider", () => {
Expand All @@ -56,7 +56,7 @@ suite("UpgradeDowngrade", () => {
name: release0, assets: three_assets}];

const userVersion: PackageVersion = new PackageVersion(insider3);
const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel);
const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel, false);
assert.equal(targetBuild, undefined);
});
test("Release to Insider", () => {
Expand All @@ -67,7 +67,7 @@ suite("UpgradeDowngrade", () => {
name: release0, assets: three_assets}];

const userVersion: PackageVersion = new PackageVersion(release1);
const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel);
const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel, false);
assert.equal(targetBuild, undefined);
});
});
Expand All @@ -81,7 +81,7 @@ suite("UpgradeDowngrade", () => {
name: release0, assets: three_assets}];

const userVersion: PackageVersion = new PackageVersion(insider3);
const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel);
const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel, false);
assert.equal(targetBuild.name, release0);
});
test("Insider to Insider", () => {
Expand All @@ -92,7 +92,7 @@ suite("UpgradeDowngrade", () => {
name: release0, assets: three_assets}];

const userVersion: PackageVersion = new PackageVersion(insider3);
const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel);
const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel, false);
assert.equal(targetBuild.name, insider1);
});
});
Expand All @@ -107,7 +107,7 @@ suite("UpgradeDowngrade", () => {
name: insider2, assets: three_assets}];

const userVersion: PackageVersion = new PackageVersion(release0);
const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel);
const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel, false);
assert.equal(targetBuild.name, release1);
});
test("Insider to Release", () => {
Expand All @@ -117,7 +117,7 @@ suite("UpgradeDowngrade", () => {
name: insider2, assets: three_assets}];

const userVersion: PackageVersion = new PackageVersion(insider2);
const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel);
const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel, false);
assert.equal(targetBuild.name, release1);
});
});
Expand All @@ -130,7 +130,7 @@ suite("UpgradeDowngrade", () => {
name: release0, assets: three_assets}];

const userVersion: PackageVersion = new PackageVersion(release0);
const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel);
const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel, false);
assert.equal(targetBuild.name, insider1);
});
test("Release to Insider, no Upgrade", () => {
Expand All @@ -140,7 +140,7 @@ suite("UpgradeDowngrade", () => {
name: release0, assets: three_assets}];

const userVersion: PackageVersion = new PackageVersion(release0);
const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel);
const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel, false);
assert.equal(targetBuild, undefined);
});
test("Insider to Insider, Upgrade", () => {
Expand All @@ -151,7 +151,7 @@ suite("UpgradeDowngrade", () => {
name: release0, assets: three_assets}];

const userVersion: PackageVersion = new PackageVersion(insider1);
const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel);
const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel, false);
assert.equal(targetBuild.name, insider2);
});
test("Insider to Insider, no Upgrade", () => {
Expand All @@ -161,7 +161,7 @@ suite("UpgradeDowngrade", () => {
name: release0, assets: three_assets}];

const userVersion: PackageVersion = new PackageVersion(insider2);
const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel);
const targetBuild: Build | undefined = getTargetBuild(builds, userVersion, updateChannel, false);
assert.equal(targetBuild, undefined);
});
});
Expand Down

0 comments on commit d80e04e

Please sign in to comment.