From 3ba8e4843deaf0f584c0171f4875f8da5202779f Mon Sep 17 00:00:00 2001 From: Eric Promislow Date: Tue, 14 Nov 2023 14:51:23 -0800 Subject: [PATCH] Attempt #3 to explain how the settings migrator works. Signed-off-by: Eric Promislow --- pkg/rancher-desktop/config/settingsImpl.ts | 72 +++++++++++----------- 1 file changed, 35 insertions(+), 37 deletions(-) diff --git a/pkg/rancher-desktop/config/settingsImpl.ts b/pkg/rancher-desktop/config/settingsImpl.ts index 597f458a6fc..33321865c73 100644 --- a/pkg/rancher-desktop/config/settingsImpl.ts +++ b/pkg/rancher-desktop/config/settingsImpl.ts @@ -328,54 +328,48 @@ function parseSaveError(err: any) { return friendlierMsg; } -// @path {string} points to a possible field in the settings structure (I'm sure there's a typescript -// notation to describe it, but it's more readable in English than to try to come up with that incantation). -// `fn` {(string) => void} takes the old value, and knows what to do with it. If it isn't specified, the -// function that works with this data will carry out a useful default action. -// +/** + * ReplacementDirective describes how a setting can be migrated. + */ interface ReplacementDirective { + /** + * The path to a possible field in the settings structure + */ path: string; + /** + * Callback to process the value at the given path. If it isn't specified, + * see the doc comment for `processReplacements`, the only function that uses this type. + */ fn?: null|((oldValue: any) => void); } /** * This function looks for existing fields in `settings`, and either calls the supplied function `fn` with the * existing value, or if no `fn` is specified, assigns the value to `settings[replacement][last-part-of-path]`. - * See the arrays that are used to define the `replacements` arguments in the calls to this function as a reference. + * Rather than try to explain what's going on in general terms, see the comments marked with "EXPLANATION" + * on the inputs to this function. * @param settings - the settings object * @param replacements - a table used to update the settings object based on existing obsolete fields that need to be moved. - * - * There are three kinds of replacements (actually two, but one is a special case of the other). - * In one of the cases, we specify a path and a replacement function -- if the path exists in the - * current settings block, the callback is called with the paths' value, and the callback can do - * whatever it needs to in order to move the value to a new place in the settings block. - * - * But there are many cases that fit a pattern, and no specific callback is needed. For example, - * migration 4=>5 moves `settings.debug` and `settings.pathManagementStrategy` into `settings.application...`. - * So the input to this function just specifies a parent entry of `application` for these two paths. - * If `settings.debug` exists, it's moved into `settings.application.debug`. - * - * Some of the settings weren't at the top-level, such as `kubernetes.hostResalver`, which is moved into - * `settings.virtualMachine`, also in migration 4=>5. This replacement looks a lot like the one for `settings.debug`, - * except the new location is determined by the current parent in the migration table (`virtualMachine`), and - * we take the last part of the dotted path, namely `hostResolver`. So we map `settings.kubernetes.hostResolver` - * to `settings.virtualMachine.hostResolver` without needing a custom function. */ function processReplacements(settings: any, replacements: Record) { for (const replacement in replacements) { for (const { path, fn } of replacements[replacement]) { if (_.hasIn(settings, path)) { + // Get the current value for the old field + const currentValue = _.get(settings, path); + if (!_.hasIn(settings, replacement)) { _.set(settings, replacement, {}); } if (fn) { - fn(_.get(settings, path)); + fn(currentValue); } else { // `as string` maps `undefined|string` to `string` const lastPathPart: string = path.split('.').pop() as string; - _.set(settings[replacement], lastPathPart, _.get(settings, path)); + _.set(settings[replacement], lastPathPart, currentValue); } + // Delete the old field _.unset(settings, path); } } @@ -383,17 +377,18 @@ function processReplacements(settings: any, replacements: Record void> = { 1: (settings) => { - // Implement setting change from version 3 to 4 + // EXPLANATION: We don't call `processReplacements` in this step + // because this migration only deletes an existing field. if (_.hasIn(settings, 'kubernetes.rancherMode')) { delete settings.kubernetes.rancherMode; } @@ -410,14 +405,19 @@ export const updateTable: Record void> = { const replacements: Record = { application: [ { + // EXPLANATION: Use a callback to move the opposite of `.kubernetes.suppressSudo` to `.application.adminAccess` path: 'kubernetes.suppressSudo', fn: (oldValue: any) => { settings.application.adminAccess = !oldValue; }, }, + // EXPLANATION: Default action: move `.debug` to `.application.debug` (because `application` is the + // name of this node's parent. { path: 'debug' }, { path: 'pathManagementStrategy' }, { + // EXPLANATION: Use a callback to move `.telemetry` to `.application.telemetry.enabled` + // `processReplacements` will init `application.telemetry` to `{}` path: 'telemetry', fn: (oldValue: any) => { settings.application.telemetry = { enabled: oldValue }; @@ -447,6 +447,9 @@ export const updateTable: Record void> = { }, ], virtualMachine: [ + // EXPLANATION: Default action: move `.kubernetes.hostResolver` to `.virtualMachine.hostResolver` + // This is the general case of the default action, where the new path consists of + // (parent-name="virtualMachine") . (tail(path) = "hostResolver") { path: 'kubernetes.hostResolver' }, { path: 'kubernetes.memoryInGB' }, { path: 'kubernetes.numberCPUs' }, @@ -482,8 +485,7 @@ export const updateTable: Record void> = { { path: 'virtualMachine.experimental.socketVMNet', fn: (oldValue: any) => { - settings.experimental.virtualMachine ??= {}; - settings.experimental.virtualMachine.socketVMNet = oldValue; + _.set(settings.experimental, 'virtualMachine.socketVMNet', oldValue); }, }, ], @@ -548,10 +550,6 @@ function migrateSettingsToCurrentVersion(settings: Record): Setting return defaultSettings; } settings = migrateSpecifiedSettingsToCurrentVersion(settings) as Settings; - // if (!Object.values(ContainerEngine).map(String).includes(settings.containerEngine.name)) { - // console.warn(`Replacing unrecognized saved container engine pref of '${ settings.containerEngine?.name }' with ${ ContainerEngine.CONTAINERD }`); - // settings.containerEngine.name = ContainerEngine.CONTAINERD; - // } return _.defaultsDeep(settings, defaultSettings); }