[ML] Improve returned type from isPopulatedObject().

elastic · Mar 31, 2021 · 4bc7527 · 4bc7527
1 parent a68405f
commit 4bc7527
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 27 deletions.
diff --git a/x-pack/plugins/ml/common/util/object_utils.test.ts b/x-pack/plugins/ml/common/util/object_utils.test.ts
@@ -8,25 +8,43 @@
 import { isPopulatedObject } from './object_utils';
 
 describe('object_utils', () => {
-  test('isPopulatedObject()', () => {
-    expect(isPopulatedObject(0)).toBe(false);
-    expect(isPopulatedObject('')).toBe(false);
-    expect(isPopulatedObject(null)).toBe(false);
-    expect(isPopulatedObject({})).toBe(false);
-    expect(isPopulatedObject({ attribute: 'value' })).toBe(true);
-    expect(isPopulatedObject({ attribute: 'value' }, ['otherAttribute'])).toBe(false);
-    expect(isPopulatedObject({ attribute: 'value' }, ['attribute'])).toBe(true);
-    expect(
-      isPopulatedObject({ attribute1: 'value1', attribute2: 'value2' }, [
-        'attribute1',
-        'attribute2',
-      ])
-    ).toBe(true);
-    expect(
-      isPopulatedObject({ attribute1: 'value1', attribute2: 'value2' }, [
-        'attribute1',
-        'otherAttribute',
-      ])
-    ).toBe(false);
+  describe('isPopulatedObject()', () => {
+    it('does not allow numbers', () => {
+      expect(isPopulatedObject(0)).toBe(false);
+    });
+    it('does not allow strings', () => {
+      expect(isPopulatedObject('')).toBe(false);
+    });
+    it('does not allow null', () => {
+      expect(isPopulatedObject(null)).toBe(false);
+    });
+    it('does not allow an empty object', () => {
+      expect(isPopulatedObject({})).toBe(false);
+    });
+    it('allows an object with an attribute', () => {
+      expect(isPopulatedObject({ attribute: 'value' })).toBe(true);
+    });
+    it('does not allow an object with a non-existing required attribute', () => {
+      expect(isPopulatedObject({ attribute: 'value' }, ['otherAttribute'])).toBe(false);
+    });
+    it('allows an object with an existing required attribute', () => {
+      expect(isPopulatedObject({ attribute: 'value' }, ['attribute'])).toBe(true);
+    });
+    it('allows an object with two existing required attributes', () => {
+      expect(
+        isPopulatedObject({ attribute1: 'value1', attribute2: 'value2' }, [
+          'attribute1',
+          'attribute2',
+        ])
+      ).toBe(true);
+    });
+    it('does not allow an object with two required attributes where one does not exist', () => {
+      expect(
+        isPopulatedObject({ attribute1: 'value1', attribute2: 'value2' }, [
+          'attribute1',
+          'otherAttribute',
+        ])
+      ).toBe(false);
+    });
   });
 });
diff --git a/x-pack/plugins/ml/common/util/object_utils.ts b/x-pack/plugins/ml/common/util/object_utils.ts
@@ -5,10 +5,23 @@
  * 2.0.
  */
 
-export const isPopulatedObject = <T = Record<string, unknown>>(
+/*
+ * A type guard to check record like object structures.
+ *
+ * Examples:
+ * - `isPopulatedObject({...})`
+ *      limits type to Record<string, unknown>
+ *
+ * - `isPopulatedObject({...}, ['attribute'])`
+ *      limits type to Record<'attribute', unknown>
+ *
+ * - `isPopulatedObject<keyof MyInterface>({...})`
+ *      limits type to a record with keys of the given interface
+ */
+export const isPopulatedObject = <U extends string = string>(
   arg: unknown,
-  requiredAttributes: string[] = []
-): arg is T => {
+  requiredAttributes: U[] = []
+): arg is Record<U, unknown> => {
   return (
     typeof arg === 'object' &&
     arg !== null &&

diff --git a/x-pack/plugins/transform/common/types/index_pattern.ts b/x-pack/plugins/transform/common/types/index_pattern.ts
@@ -13,9 +13,10 @@ import { isPopulatedObject } from '../shared_imports';
 export function isIndexPattern(arg: any): arg is IndexPattern {
   return (
     isPopulatedObject(arg, ['title', 'fields']) &&
-    // `getComputedFields` is inherited, so not possible to check with `hasOwnProperty`
-    'getComputedFields' in arg &&
-    typeof arg.getComputedFields === 'function' &&
+    // `getComputedFields` is inherited, so it's not possible to
+    // check with `hasOwnProperty` which is used by isPopulatedObject()
+    'getComputedFields' in (arg as IndexPattern) &&
+    typeof (arg as IndexPattern).getComputedFields === 'function' &&
     typeof arg.title === 'string' &&
     Array.isArray(arg.fields)
   );

diff --git a/x-pack/plugins/transform/public/app/common/request.ts b/x-pack/plugins/transform/public/app/common/request.ts
@@ -100,7 +100,7 @@ export function getCombinedRuntimeMappings(
     combinedRuntimeMappings = { ...combinedRuntimeMappings, ...runtimeMappings };
   }
 
-  if (isPopulatedObject<StepDefineExposedState['runtimeMappings']>(combinedRuntimeMappings)) {
+  if (isPopulatedObject<keyof StepDefineExposedState['runtimeMappings']>(combinedRuntimeMappings)) {
     return combinedRuntimeMappings;
   }
   return undefined;