experiments: add ability to hide versions with old schema

connor4312 · connor4312 · commit 69e0508b636c · 2020-02-18T11:38:42.000-08:00
diff --git a/src/vs/workbench/contrib/experiments/common/experimentService.ts b/src/vs/workbench/contrib/experiments/common/experimentService.ts
@@ -83,8 +83,16 @@ interface IExperimentStorageState {
 	lastEditedDate?: string;
 }
 
+/**
+ * Current version of the experiment schema in this VS Code build. This *must*
+ * be incremented when adding a condition, otherwise experiments might activate
+ * on older versions of VS Code where not intended.
+ */
+export const currentSchemaVersion = 1;
+
 interface IRawExperiment {
 	id: string;
+	schemaVersion: number;
 	enabled?: boolean;
 	condition?: {
 		insidersOnly?: boolean;
@@ -220,8 +228,8 @@ export class ExperimentService extends Disposable implements IExperimentService
 			if (context.res.statusCode !== 200) {
 				return null;
 			}
-			const result: any = await asJson(context);
-			return result && Array.isArray(result['experiments']) ? result['experiments'] : [];
+			const result = await asJson<{ experiments?: IRawExperiment }>(context);
+			return result && Array.isArray(result.experiments) ? result.experiments : [];
 		} catch (_e) {
 			// Bad request or invalid JSON
 			return null;
@@ -249,6 +257,10 @@ export class ExperimentService extends Disposable implements IExperimentService
 				return Promise.resolve(null);
 			}
 
+			// Don't look at experiments with newer schema versions. We can't
+			// understand them, trying to process them might even cause errors.
+			rawExperiments = rawExperiments.filter(e => (e.schemaVersion || 0) <= currentSchemaVersion);
+
 			// Clear disbaled/deleted experiments from storage
 			const allExperimentIdsFromStorage = safeParse(this.storageService.get('allExperiments', StorageScope.GLOBAL), []);
 			const enabledExperiments = rawExperiments.filter(experiment => !!experiment.enabled).map(experiment => experiment.id.toLowerCase());
diff --git a/src/vs/workbench/contrib/experiments/test/electron-browser/experimentService.test.ts b/src/vs/workbench/contrib/experiments/test/electron-browser/experimentService.test.ts
@@ -5,7 +5,7 @@
 
 import * as assert from 'assert';
 import * as sinon from 'sinon';
-import { ExperimentActionType, ExperimentState, IExperiment, ExperimentService, getCurrentActivationRecord } from 'vs/workbench/contrib/experiments/common/experimentService';
+import { ExperimentActionType, ExperimentState, IExperiment, ExperimentService, getCurrentActivationRecord, currentSchemaVersion } from 'vs/workbench/contrib/experiments/common/experimentService';
 import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock';
 import { TestLifecycleService, TestExtensionService } from 'vs/workbench/test/browser/workbenchTestServices';
 import {
@@ -167,6 +167,36 @@ suite('Experiment Service', () => {
 		});
 	});
 
+	test('filters out experiments with newer schema versions', async () => {
+		experimentData = {
+			experiments: [
+				{
+					id: 'experiment1',
+					// no version == 0
+				},
+				{
+					id: 'experiment2',
+					schemaVersion: currentSchemaVersion,
+				},
+				{
+					id: 'experiment3',
+					schemaVersion: currentSchemaVersion + 1,
+				},
+			]
+		};
+
+		testObject = instantiationService.createInstance(TestExperimentService);
+		const actual = await Promise.all([
+			testObject.getExperimentById('experiment1'),
+			testObject.getExperimentById('experiment2'),
+			testObject.getExperimentById('experiment3'),
+		]);
+
+		assert.equal(actual[0]?.id, 'experiment1');
+		assert.equal(actual[1]?.id, 'experiment2');
+		assert.equal(actual[2], undefined);
+	});
+
 	test('Insiders only experiment shouldnt be enabled in stable', () => {
 		experimentData = {
 			experiments: [
