Merge master into feature/debugconfig

Author: aws-toolkit-automation

package.nls.json

@@ -184,6 +184,7 @@
     "AWS.message.error.schemas.downloadCodeBindings.cancelled:": "Download code bindings cancelled",
     "AWS.message.error.schemas.downloadCodeBindings.invalid_code_generation_status": "Invalid Code generation status {0}",
     "AWS.message.error.schemas.downloadCodeBindings.timout": "Failed to download code for schema {0} before timeout. Please try again later",
+    "AWS.message.error.settings.telemetry.invalid_type": "The aws.telemetry value must be a boolean",
     "AWS.message.info.schemas.downloadCodeBindings.start": "Downloading code for schema {0}...",
     "AWS.message.info.schemas.downloadCodeBindings.generate": "{0}: Generating code (this may take a few seconds the first time)...",
     "AWS.message.info.schemas.downloadCodeBindings.extracting": "{0}: Extracting/copying code...",

src/shared/telemetry/activation.ts

@@ -15,6 +15,8 @@ import * as nls from 'vscode-nls'
 const localize = nls.loadMessageBundle()
 
 const LEGACY_SETTINGS_TELEMETRY_VALUE_DISABLE = 'Disable'
+const LEGACY_SETTINGS_TELEMETRY_VALUE_ENABLE = 'Enable'
+const TELEMETRY_SETTING_DEFAULT = true
 
 const telemetryNoticeText: string = localize(
     'AWS.telemetry.notificationMessage',
@@ -43,6 +45,9 @@ export async function activate(activateArguments: {
 }) {
     ext.telemetry = new DefaultTelemetryService(activateArguments.extensionContext, activateArguments.awsContext)
 
+    // Convert setting to boolean if it is not already
+    await sanitizeTelemetrySetting(activateArguments.toolkitSettings)
+
     // Configure telemetry based on settings, and default to enabled
     applyTelemetryEnabledState(ext.telemetry, activateArguments.toolkitSettings)
 
@@ -65,6 +70,7 @@ export async function activate(activateArguments: {
                     return
                 }
 
+                validateTelemetrySettingType(activateArguments.toolkitSettings)
                 applyTelemetryEnabledState(ext.telemetry, activateArguments.toolkitSettings)
             }
         },
@@ -73,6 +79,24 @@ export async function activate(activateArguments: {
     )
 }
 
+/*
+ * Formats the AWS telemetry setting to a boolean: false if setting value was 'Disable' or false, true for everything else
+ */
+export async function sanitizeTelemetrySetting(toolkitSettings: SettingsConfiguration): Promise<void> {
+    const value = toolkitSettings.readSetting<any>(AWS_TELEMETRY_KEY)
+
+    if (typeof value === 'boolean') {
+        return
+    }
+
+    // Set telemetry value to boolean if the current value matches the legacy value
+    if (value === LEGACY_SETTINGS_TELEMETRY_VALUE_DISABLE) {
+        await toolkitSettings.writeSetting<any>(AWS_TELEMETRY_KEY, false, vscode.ConfigurationTarget.Global)
+    } else if (value === LEGACY_SETTINGS_TELEMETRY_VALUE_ENABLE) {
+        await toolkitSettings.writeSetting<any>(AWS_TELEMETRY_KEY, true, vscode.ConfigurationTarget.Global)
+    }
+}
+
 export function isTelemetryEnabled(toolkitSettings: SettingsConfiguration): boolean {
     // Setting used to be an enum, but is now a boolean.
     // We don't have api-based strong type support, so we have to process this value manually.
@@ -83,13 +107,21 @@ export function isTelemetryEnabled(toolkitSettings: SettingsConfiguration): bool
         return false
     }
 
-    // Current value is expected to be a boolean
     if (typeof value === 'boolean') {
         return value
+    } else {
+        return TELEMETRY_SETTING_DEFAULT
     }
+}
 
-    // Treat anything else (unexpected values, datatypes, or undefined) as opt-in
-    return true
+function validateTelemetrySettingType(toolkitSettings: SettingsConfiguration): void {
+    const value = toolkitSettings.readSetting<any>(AWS_TELEMETRY_KEY)
+    if (typeof value !== 'boolean') {
+        getLogger().error('In settings.json, aws.telemetry value must be a boolean')
+        vscode.window.showErrorMessage(
+            localize('AWS.message.error.settings.telemetry.invalid_type', 'The aws.telemetry value must be a boolean')
+        )
+    }
 }
 
 function applyTelemetryEnabledState(telemetry: TelemetryService, toolkitSettings: SettingsConfiguration) {

src/test/shared/telemetry/activation.test.ts

@@ -16,6 +16,7 @@ import {
     TELEMETRY_NOTICE_VERSION_ACKNOWLEDGED,
     hasUserSeenTelemetryNotice,
     setHasUserSeenTelemetryNotice,
+    sanitizeTelemetrySetting,
 } from '../../../shared/telemetry/activation'
 import { DefaultSettingsConfiguration } from '../../../shared/settingsConfiguration'
 import { extensionSettingsPrefix } from '../../../shared/constants'
@@ -70,7 +71,7 @@ describe('handleTelemetryNoticeResponse', () => {
     })
 })
 
-describe('isTelemetryEnabled', () => {
+describe('Telemetry on activation', () => {
     let settings: vscode.WorkspaceConfiguration
     const toolkitSettings = new DefaultSettingsConfiguration(extensionSettingsPrefix)
 
@@ -88,23 +89,72 @@ describe('isTelemetryEnabled', () => {
     })
 
     const scenarios = [
-        { initialSettingValue: 'Enable', expectedIsEnabledValue: true, desc: 'Original opt-in value' },
-        { initialSettingValue: 'Disable', expectedIsEnabledValue: false, desc: 'Original opt-out value' },
-        { initialSettingValue: 'Use IDE settings', expectedIsEnabledValue: true, desc: 'Original deferral value' },
-        { initialSettingValue: true, expectedIsEnabledValue: true, desc: 'Opt in' },
-        { initialSettingValue: false, expectedIsEnabledValue: false, desc: 'Opt out' },
-        { initialSettingValue: 1234, expectedIsEnabledValue: true, desc: 'Unexpected numbers' },
-        { initialSettingValue: { label: 'garbageData' }, expectedIsEnabledValue: true, desc: 'Unexpected object' },
-        { initialSettingValue: [{ label: 'garbageDataList' }], expectedIsEnabledValue: true, desc: 'Unexpected array' },
-        { initialSettingValue: undefined, expectedIsEnabledValue: true, desc: 'Unset value' },
+        {
+            initialSettingValue: 'Enable',
+            expectedIsEnabledValue: true,
+            desc: 'Original opt-in value',
+            expectedSanitizedValue: true,
+        },
+        {
+            initialSettingValue: 'Disable',
+            expectedIsEnabledValue: false,
+            desc: 'Original opt-out value',
+            expectedSanitizedValue: false,
+        },
+        {
+            initialSettingValue: 'Use IDE settings',
+            expectedIsEnabledValue: true,
+            desc: 'Original deferral value',
+            expectedSanitizedValue: 'Use IDE settings',
+        },
+        { initialSettingValue: true, expectedIsEnabledValue: true, desc: 'Opt in', expectedSanitizedValue: true },
+        { initialSettingValue: false, expectedIsEnabledValue: false, desc: 'Opt out', expectedSanitizedValue: false },
+        {
+            initialSettingValue: 1234,
+            expectedIsEnabledValue: true,
+            desc: 'Unexpected numbers',
+            expectedSanitizedValue: 1234,
+        },
+        {
+            initialSettingValue: { label: 'garbageData' },
+            expectedIsEnabledValue: true,
+            desc: 'Unexpected object',
+            expectedSanitizedValue: { label: 'garbageData' },
+        },
+        {
+            initialSettingValue: [{ label: 'garbageDataList' }],
+            expectedIsEnabledValue: true,
+            desc: 'Unexpected array',
+            expectedSanitizedValue: [{ label: 'garbageDataList' }],
+        },
+        {
+            initialSettingValue: undefined,
+            expectedIsEnabledValue: true,
+            desc: 'Unset value',
+            expectedSanitizedValue: true,
+        }, // The 'expectedSanitizedValue' is true based on the package.json configuration declaration
     ]
 
-    scenarios.forEach(scenario => {
-        it(scenario.desc, async () => {
-            await settings.update('telemetry', scenario.initialSettingValue, vscode.ConfigurationTarget.Global)
+    describe('isTelemetryEnabled', () => {
+        scenarios.forEach(scenario => {
+            it(scenario.desc, async () => {
+                await settings.update('telemetry', scenario.initialSettingValue, vscode.ConfigurationTarget.Global)
+
+                const isEnabled = isTelemetryEnabled(toolkitSettings)
+                assert.strictEqual(isEnabled, scenario.expectedIsEnabledValue)
+            })
+        })
+    })
+
+    describe('sanitizeTelemetrySetting', () => {
+        scenarios.forEach(scenario => {
+            it(scenario.desc, async () => {
+                await settings.update('telemetry', scenario.initialSettingValue, vscode.ConfigurationTarget.Global)
 
-            const isEnabled = isTelemetryEnabled(toolkitSettings)
-            assert.strictEqual(isEnabled, scenario.expectedIsEnabledValue)
+                await sanitizeTelemetrySetting(toolkitSettings)
+                const sanitizedSetting = toolkitSettings.readSetting<any>('telemetry')
+                assert.deepStrictEqual(sanitizedSetting, scenario.expectedSanitizedValue)
+            })
         })
     })
 })