@kbn/config: Clear empty-object properties after their props are unset (#120889) (#121161)

# Conflicts:
#	docs/development/core/public/kibana-plugin-core-public.doclinksstart.md

docs/development/core/server/kibana-plugin-core-server.eventloopdelaysmonitor.collect.md

@@ -6,6 +6,8 @@
 
 Collect gathers event loop delays metrics from nodejs perf\_hooks.monitorEventLoopDelay the histogram calculations start from the last time `reset` was called or this EventLoopDelaysMonitor instance was created.
 
+Returns metrics in milliseconds.
+
 <b>Signature:</b>
 
 ```typescript

docs/development/core/server/kibana-plugin-core-server.eventloopdelaysmonitor.md

@@ -20,7 +20,7 @@ export declare class EventLoopDelaysMonitor
 
 |  Method | Modifiers | Description |
 |  --- | --- | --- |
-|  [collect()](./kibana-plugin-core-server.eventloopdelaysmonitor.collect.md) |  | Collect gathers event loop delays metrics from nodejs perf\_hooks.monitorEventLoopDelay the histogram calculations start from the last time <code>reset</code> was called or this EventLoopDelaysMonitor instance was created. |
+|  [collect()](./kibana-plugin-core-server.eventloopdelaysmonitor.collect.md) |  | Collect gathers event loop delays metrics from nodejs perf\_hooks.monitorEventLoopDelay the histogram calculations start from the last time <code>reset</code> was called or this EventLoopDelaysMonitor instance was created.<!-- -->Returns metrics in milliseconds. |
 |  [reset()](./kibana-plugin-core-server.eventloopdelaysmonitor.reset.md) |  | Resets the collected histogram data. |
 |  [stop()](./kibana-plugin-core-server.eventloopdelaysmonitor.stop.md) |  | Disables updating the interval timer for collecting new data points. |
 

docs/development/core/server/kibana-plugin-core-server.intervalhistogram.md

@@ -4,7 +4,7 @@
 
 ## IntervalHistogram interface
 
-an IntervalHistogram object that samples and reports the event loop delay over time. The delays will be reported in nanoseconds.
+an IntervalHistogram object that samples and reports the event loop delay over time. The delays will be reported in milliseconds.
 
 <b>Signature:</b>
 

docs/development/core/server/kibana-plugin-core-server.md

@@ -101,7 +101,7 @@ The plugin integrates with the core system via lifecycle events: `setup`<!-- -->
 |  [IExternalUrlPolicy](./kibana-plugin-core-server.iexternalurlpolicy.md) | A policy describing whether access to an external destination is allowed. |
 |  [IKibanaResponse](./kibana-plugin-core-server.ikibanaresponse.md) | A response data object, expected to returned as a result of [RequestHandler](./kibana-plugin-core-server.requesthandler.md) execution |
 |  [IKibanaSocket](./kibana-plugin-core-server.ikibanasocket.md) | A tiny abstraction for TCP socket. |
-|  [IntervalHistogram](./kibana-plugin-core-server.intervalhistogram.md) | an IntervalHistogram object that samples and reports the event loop delay over time. The delays will be reported in nanoseconds. |
+|  [IntervalHistogram](./kibana-plugin-core-server.intervalhistogram.md) | an IntervalHistogram object that samples and reports the event loop delay over time. The delays will be reported in milliseconds. |
 |  [IRenderOptions](./kibana-plugin-core-server.irenderoptions.md) |  |
 |  [IRouter](./kibana-plugin-core-server.irouter.md) | Registers route handlers for specified resource path and method. See [RouteConfig](./kibana-plugin-core-server.routeconfig.md) and [RequestHandler](./kibana-plugin-core-server.requesthandler.md) for more information about arguments to route registrations. |
 |  [ISavedObjectsPointInTimeFinder](./kibana-plugin-core-server.isavedobjectspointintimefinder.md) |  |

packages/kbn-config/src/deprecation/apply_deprecations.test.ts

@@ -116,6 +116,36 @@ describe('applyDeprecations', () => {
     expect(migrated).toEqual({ foo: 'bar', newname: 'renamed' });
   });
 
+  it('nested properties take into account if their parents are empty objects, and remove them if so', () => {
+    const initialConfig = {
+      foo: 'bar',
+      deprecated: { nested: 'deprecated' },
+      nested: {
+        from: {
+          rename: 'renamed',
+        },
+        to: {
+          keep: 'keep',
+        },
+      },
+    };
+
+    const { config: migrated } = applyDeprecations(initialConfig, [
+      wrapHandler(deprecations.unused('deprecated.nested')),
+      wrapHandler(deprecations.rename('nested.from.rename', 'nested.to.renamed')),
+    ]);
+
+    expect(migrated).toStrictEqual({
+      foo: 'bar',
+      nested: {
+        to: {
+          keep: 'keep',
+          renamed: 'renamed',
+        },
+      },
+    });
+  });
+
   it('does not alter the initial config', () => {
     const initialConfig = { foo: 'bar', deprecated: 'deprecated' };
 

packages/kbn-config/src/deprecation/apply_deprecations.ts

@@ -6,13 +6,14 @@
  * Side Public License, v 1.
  */
 
-import { cloneDeep, unset } from 'lodash';
+import { cloneDeep } from 'lodash';
 import { set } from '@elastic/safer-lodash-set';
 import type {
   AddConfigDeprecation,
   ChangedDeprecatedPaths,
   ConfigDeprecationWithContext,
 } from './types';
+import { unsetAndCleanEmptyParent } from './unset_and_clean_empty_parent';
 
 const noopAddDeprecationFactory: () => AddConfigDeprecation = () => () => undefined;
 
@@ -45,7 +46,7 @@ export const applyDeprecations = (
       if (commands.unset) {
         changedPaths.unset.push(...commands.unset.map((c) => c.path));
         commands.unset.forEach(function ({ path: commandPath }) {
-          unset(result, commandPath);
+          unsetAndCleanEmptyParent(result, commandPath);
         });
       }
     }

packages/kbn-config/src/deprecation/types.ts

@@ -186,6 +186,25 @@ export interface ConfigDeprecationFactory {
    *   rename('oldKey', 'newKey'),
    * ]
    * ```
+   *
+   * @remarks
+   * If the oldKey is a nested property and it's the last property in an object, it may remove any empty-object parent keys.
+   * ```
+   * // Original object
+   * {
+   * 	a: {
+   * 		b: { c: 1 },
+   * 		d: { e: 1 }
+   * 	}
+   * }
+   *
+   * // If rename('a.b.c', 'a.d.c'), the resulting object removes the entire "a.b" tree because "c" was the last property in that branch
+   * {
+   * 	a: {
+   * 		d: { c: 1, e: 1 }
+   * 	}
+   * }
+   * ```
    */
   rename(
     oldKey: string,
@@ -207,6 +226,25 @@ export interface ConfigDeprecationFactory {
    *   renameFromRoot('oldplugin.key', 'newplugin.key'),
    * ]
    * ```
+   *
+   * @remarks
+   * If the oldKey is a nested property and it's the last property in an object, it may remove any empty-object parent keys.
+   * ```
+   * // Original object
+   * {
+   * 	a: {
+   * 		b: { c: 1 },
+   * 		d: { e: 1 }
+   * 	}
+   * }
+   *
+   * // If renameFromRoot('a.b.c', 'a.d.c'), the resulting object removes the entire "a.b" tree because "c" was the last property in that branch
+   * {
+   * 	a: {
+   * 		d: { c: 1, e: 1 }
+   * 	}
+   * }
+   * ```
    */
   renameFromRoot(
     oldKey: string,
@@ -225,6 +263,25 @@ export interface ConfigDeprecationFactory {
    *   unused('deprecatedKey'),
    * ]
    * ```
+   *
+   * @remarks
+   * If the path is a nested property and it's the last property in an object, it may remove any empty-object parent keys.
+   * ```
+   * // Original object
+   * {
+   * 	a: {
+   * 		b: { c: 1 },
+   * 		d: { e: 1 }
+   * 	}
+   * }
+   *
+   * // If unused('a.b.c'), the resulting object removes the entire "a.b" tree because "c" was the last property in that branch
+   * {
+   * 	a: {
+   * 		d: { e: 1 }
+   * 	}
+   * }
+   * ```
    */
   unused(unusedKey: string, details?: Partial<DeprecatedConfigDetails>): ConfigDeprecation;
 
@@ -242,6 +299,25 @@ export interface ConfigDeprecationFactory {
    *   unusedFromRoot('somepath.deprecatedProperty'),
    * ]
    * ```
+   *
+   * @remarks
+   * If the path is a nested property and it's the last property in an object, it may remove any empty-object parent keys.
+   * ```
+   * // Original object
+   * {
+   * 	a: {
+   * 		b: { c: 1 },
+   * 		d: { e: 1 }
+   * 	}
+   * }
+   *
+   * // If unused('a.b.c'), the resulting object removes the entire "a.b" tree because "c" was the last property in that branch
+   * {
+   * 	a: {
+   * 		d: { e: 1 }
+   * 	}
+   * }
+   * ```
    */
   unusedFromRoot(unusedKey: string, details?: Partial<DeprecatedConfigDetails>): ConfigDeprecation;
 }

packages/kbn-config/src/deprecation/unset_and_clean_empty_parent.test.ts

@@ -0,0 +1,41 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import { unsetAndCleanEmptyParent } from './unset_and_clean_empty_parent';
+
+describe('unsetAndcleanEmptyParent', () => {
+  test('unsets the property of the root object, and returns an empty root object', () => {
+    const config = { toRemove: 'toRemove' };
+    unsetAndCleanEmptyParent(config, 'toRemove');
+    expect(config).toStrictEqual({});
+  });
+
+  test('unsets a nested property of the root object, and removes the empty parent property', () => {
+    const config = { nestedToRemove: { toRemove: 'toRemove' } };
+    unsetAndCleanEmptyParent(config, 'nestedToRemove.toRemove');
+    expect(config).toStrictEqual({});
+  });
+
+  describe('Navigating to parent known issue: Array paths', () => {
+    // We navigate to the parent property by splitting the "." and dropping the last item in the path.
+    // This means that paths that are declared as prop1[idx] cannot apply the parent's cleanup logic.
+    // The use cases for this are quite limited, so we'll accept it as a documented limitation.
+
+    test('does not remove a parent array when the index is specified with square brackets', () => {
+      const config = { nestedToRemove: [{ toRemove: 'toRemove' }] };
+      unsetAndCleanEmptyParent(config, 'nestedToRemove[0].toRemove');
+      expect(config).toStrictEqual({ nestedToRemove: [{}] });
+    });
+
+    test('removes a parent array when the index is specified with dots', () => {
+      const config = { nestedToRemove: [{ toRemove: 'toRemove' }] };
+      unsetAndCleanEmptyParent(config, 'nestedToRemove.0.toRemove');
+      expect(config).toStrictEqual({});
+    });
+  });
+});

packages/kbn-config/src/deprecation/unset_and_clean_empty_parent.ts

@@ -0,0 +1,42 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import { get, unset } from 'lodash';
+
+/**
+ * Unsets the path and checks if the parent property is an empty object.
+ * If so, it removes the property from the config object (mutation is applied).
+ *
+ * @internal
+ */
+export const unsetAndCleanEmptyParent = (
+  config: Record<string, unknown>,
+  path: string | string[]
+): void => {
+  // 1. Unset the provided path
+  const didUnset = unset(config, path);
+
+  // Check if the unset actually removed anything.
+  // This way we avoid some CPU cycles when the previous action didn't apply any changes.
+  if (didUnset) {
+    // 2. Check if the parent property in the resulting object is an empty object
+    const pathArray = Array.isArray(path) ? path : path.split('.');
+    const parentPath = pathArray.slice(0, -1);
+    if (parentPath.length === 0) {
+      return;
+    }
+    const parentObj = get(config, parentPath);
+    if (
+      typeof parentObj === 'object' &&
+      parentObj !== null &&
+      Object.keys(parentObj).length === 0
+    ) {
+      unsetAndCleanEmptyParent(config, parentPath);
+    }
+  }
+};

src/plugins/data/server/config_deprecations.test.ts

@@ -50,7 +50,7 @@ describe('Config Deprecations', () => {
       },
     };
     const { messages, migrated } = applyConfigDeprecations(cloneDeep(config));
-    expect(migrated.kibana.autocompleteTerminateAfter).not.toBeDefined();
+    expect(migrated.kibana?.autocompleteTerminateAfter).not.toBeDefined();
     expect(migrated.data.autocomplete.valueSuggestions.terminateAfter).toEqual(123);
     expect(messages).toMatchInlineSnapshot(`
       Array [
@@ -66,7 +66,7 @@ describe('Config Deprecations', () => {
       },
     };
     const { messages, migrated } = applyConfigDeprecations(cloneDeep(config));
-    expect(migrated.kibana.autocompleteTimeout).not.toBeDefined();
+    expect(migrated.kibana?.autocompleteTimeout).not.toBeDefined();
     expect(migrated.data.autocomplete.valueSuggestions.timeout).toEqual(123);
     expect(messages).toMatchInlineSnapshot(`
       Array [

x-pack/plugins/security/server/config_deprecations.test.ts

@@ -182,7 +182,7 @@ describe('Config Deprecations', () => {
       },
     };
     const { messages, migrated } = applyConfigDeprecations(cloneDeep(config));
-    expect(migrated.security.showInsecureClusterWarning).not.toBeDefined();
+    expect(migrated.security?.showInsecureClusterWarning).not.toBeDefined();
     expect(migrated.xpack.security.showInsecureClusterWarning).toEqual(false);
     expect(messages).toMatchInlineSnapshot(`
       Array [