Update mappings if/when new SO types are introduced (#197061)

## Summary

Addresses elastic/elastic-entity-model#70
Fixes regression introduced in
#176803

packages/core/saved-objects/core-saved-objects-migration-server-internal/src/actions/check_target_mappings.test.ts

@@ -176,8 +176,9 @@ describe('checkTargetTypesMappings', () => {
 
             const result = await task();
             expect(result).toEqual(
-              Either.right({
-                type: 'types_match' as const,
+              Either.left({
+                type: 'types_added' as const,
+                newTypes: ['type3'],
               })
             );
           });

packages/core/saved-objects/core-saved-objects-migration-server-internal/src/actions/check_target_mappings.ts

@@ -11,7 +11,7 @@ import * as Either from 'fp-ts/lib/Either';
 import * as TaskEither from 'fp-ts/lib/TaskEither';
 
 import type { IndexMapping, VirtualVersionMap } from '@kbn/core-saved-objects-base-server-internal';
-import { getUpdatedTypes } from '../core/compare_mappings';
+import { getNewAndUpdatedTypes } from '../core/compare_mappings';
 
 /** @internal */
 export interface CheckTargetTypesMappingsParams {
@@ -38,6 +38,12 @@ export interface TypesChanged {
   updatedTypes: string[];
 }
 
+/** @internal */
+export interface TypesAdded {
+  type: 'types_added';
+  newTypes: string[];
+}
+
 export const checkTargetTypesMappings =
   ({
     indexTypes,
@@ -46,7 +52,7 @@ export const checkTargetTypesMappings =
     latestMappingsVersions,
     hashToVersionMap = {},
   }: CheckTargetTypesMappingsParams): TaskEither.TaskEither<
-    IndexMappingsIncomplete | TypesChanged,
+    IndexMappingsIncomplete | TypesChanged | TypesAdded,
     TypesMatch
   > =>
   async () => {
@@ -58,7 +64,7 @@ export const checkTargetTypesMappings =
       return Either.left({ type: 'index_mappings_incomplete' as const });
     }
 
-    const updatedTypes = getUpdatedTypes({
+    const { newTypes, updatedTypes } = getNewAndUpdatedTypes({
       indexTypes,
       indexMeta: indexMappings?._meta,
       latestMappingsVersions,
@@ -70,6 +76,11 @@ export const checkTargetTypesMappings =
         type: 'types_changed' as const,
         updatedTypes,
       });
+    } else if (newTypes.length) {
+      return Either.left({
+        type: 'types_added' as const,
+        newTypes,
+      });
     } else {
       return Either.right({ type: 'types_match' as const });
     }

packages/core/saved-objects/core-saved-objects-migration-server-internal/src/actions/index.ts

@@ -112,7 +112,7 @@ import type { UnknownDocsFound } from './check_for_unknown_docs';
 import type { IncompatibleClusterRoutingAllocation } from './check_cluster_routing_allocation';
 import type { ClusterShardLimitExceeded } from './create_index';
 import type { SynchronizationFailed } from './synchronize_migrators';
-import type { IndexMappingsIncomplete, TypesChanged } from './check_target_mappings';
+import type { IndexMappingsIncomplete, TypesAdded, TypesChanged } from './check_target_mappings';
 
 export type {
   CheckForUnknownDocsParams,
@@ -193,6 +193,7 @@ export interface ActionErrorTypeMap {
   synchronization_failed: SynchronizationFailed;
   index_mappings_incomplete: IndexMappingsIncomplete;
   types_changed: TypesChanged;
+  types_added: TypesAdded;
   operation_not_supported: OperationNotSupported;
   source_equals_target: SourceEqualsTarget;
 }

packages/core/saved-objects/core-saved-objects-migration-server-internal/src/actions/update_source_mappings_properties.test.ts

@@ -47,6 +47,7 @@ describe('updateSourceMappingsProperties', () => {
       appMappings: {
         properties: {
           a: { type: 'keyword' },
+          b: { type: 'long' },
           c: { type: 'long' },
           ...getBaseMappings().properties,
         },
@@ -68,8 +69,10 @@ describe('updateSourceMappingsProperties', () => {
   it('should not update mappings when there are no changes', async () => {
     // we overwrite the app mappings to have the "unchanged" values with respect to the index mappings
     const sameMappingsParams = chain(params)
+      // let's not introduce 'c' for now
+      .set('indexTypes', ['a', 'b'])
       // even if the app versions are more recent, we emulate a scenario where mappings haven NOT changed
-      .set('latestMappingsVersions', { a: '10.1.0', b: '10.1.0', c: '10.1.0' })
+      .set('latestMappingsVersions', { a: '10.1.0', b: '10.1.0' })
       .value();
     const result = await updateSourceMappingsProperties(sameMappingsParams)();
 
@@ -78,6 +81,28 @@ describe('updateSourceMappingsProperties', () => {
     expect(result).toHaveProperty('right', 'update_mappings_succeeded');
   });
 
+  it('should update mappings if there are new types', async () => {
+    // we overwrite the app mappings to have the "unchanged" values with respect to the index mappings
+    const sameMappingsParams = chain(params)
+      // even if the app versions are more recent, we emulate a scenario where mappings haven NOT changed
+      .set('latestMappingsVersions', { a: '10.1.0', b: '10.1.0', c: '10.1.0' })
+      .value();
+    const result = await updateSourceMappingsProperties(sameMappingsParams)();
+
+    expect(client.indices.putMapping).toHaveBeenCalledTimes(1);
+    expect(client.indices.putMapping).toHaveBeenCalledWith(
+      expect.objectContaining({
+        properties: expect.objectContaining({
+          a: { type: 'keyword' },
+          b: { type: 'long' },
+          c: { type: 'long' },
+        }),
+      })
+    );
+    expect(Either.isRight(result)).toEqual(true);
+    expect(result).toHaveProperty('right', 'update_mappings_succeeded');
+  });
+
   it('should return that mappings are updated when changes are compatible', async () => {
     const result = await updateSourceMappingsProperties(params)();
 

packages/core/saved-objects/core-saved-objects-migration-server-internal/src/core/compare_mappings.test.ts

@@ -9,32 +9,38 @@
 
 import type { IndexMappingMeta } from '@kbn/core-saved-objects-base-server-internal';
 import { getBaseMappings } from './build_active_mappings';
-import { getUpdatedTypes, getUpdatedRootFields } from './compare_mappings';
+import { getUpdatedRootFields, getNewAndUpdatedTypes } from './compare_mappings';
 
-describe('getUpdatedTypes', () => {
+describe('getNewAndUpdatedTypes', () => {
   test('returns all types if _meta is missing in indexMappings', () => {
     const indexTypes = ['foo', 'bar'];
     const latestMappingsVersions = {};
 
-    expect(getUpdatedTypes({ indexTypes, indexMeta: undefined, latestMappingsVersions })).toEqual([
-      'foo',
-      'bar',
-    ]);
+    const { newTypes, updatedTypes } = getNewAndUpdatedTypes({
+      indexTypes,
+      indexMeta: undefined,
+      latestMappingsVersions,
+    });
+    expect(newTypes).toEqual([]);
+    expect(updatedTypes).toEqual(['foo', 'bar']);
   });
 
   test('returns all types if migrationMappingPropertyHashes and mappingVersions are missing in indexMappings', () => {
     const indexTypes = ['foo', 'bar'];
     const indexMeta: IndexMappingMeta = {};
     const latestMappingsVersions = {};
 
-    expect(getUpdatedTypes({ indexTypes, indexMeta, latestMappingsVersions })).toEqual([
-      'foo',
-      'bar',
-    ]);
+    const { newTypes, updatedTypes } = getNewAndUpdatedTypes({
+      indexTypes,
+      indexMeta,
+      latestMappingsVersions,
+    });
+    expect(newTypes).toEqual([]);
+    expect(updatedTypes).toEqual(['foo', 'bar']);
   });
 
   describe('when ONLY migrationMappingPropertyHashes exists in indexMappings', () => {
-    test('uses the provided hashToVersionMap to compare changes and return only the types that have changed', async () => {
+    test('uses the provided hashToVersionMap to compare changes and return new types and types that have changed', async () => {
       const indexTypes = ['type1', 'type2', 'type4'];
       const indexMeta: IndexMappingMeta = {
         migrationMappingPropertyHashes: {
@@ -56,14 +62,19 @@ describe('getUpdatedTypes', () => {
         type4: '10.5.0', // new type, no need to pick it up
       };
 
-      expect(
-        getUpdatedTypes({ indexTypes, indexMeta, latestMappingsVersions, hashToVersionMap })
-      ).toEqual(['type2']);
+      const { newTypes, updatedTypes } = getNewAndUpdatedTypes({
+        indexTypes,
+        indexMeta,
+        latestMappingsVersions,
+        hashToVersionMap,
+      });
+      expect(newTypes).toEqual(['type4']);
+      expect(updatedTypes).toEqual(['type2']);
     });
   });
 
   describe('when mappingVersions exist in indexMappings', () => {
-    test('compares the modelVersions and returns only the types that have changed', async () => {
+    test('compares the modelVersions and returns new types and types that have changed', async () => {
       const indexTypes = ['type1', 'type2', 'type4'];
 
       const indexMeta: IndexMappingMeta = {
@@ -90,9 +101,14 @@ describe('getUpdatedTypes', () => {
         // empty on purpose, not used as mappingVersions is present in indexMappings
       };
 
-      expect(
-        getUpdatedTypes({ indexTypes, indexMeta, latestMappingsVersions, hashToVersionMap })
-      ).toEqual(['type2']);
+      const { newTypes, updatedTypes } = getNewAndUpdatedTypes({
+        indexTypes,
+        indexMeta,
+        latestMappingsVersions,
+        hashToVersionMap,
+      });
+      expect(newTypes).toEqual(['type4']);
+      expect(updatedTypes).toEqual(['type2']);
     });
   });
 });

packages/core/saved-objects/core-saved-objects-migration-server-internal/src/core/compare_mappings.ts

@@ -33,43 +33,56 @@ export const getUpdatedRootFields = (indexMappings: IndexMapping): string[] => {
     .map(([propertyName]) => propertyName);
 };
 
+interface GetUpdatedTypesParams {
+  indexMeta?: IndexMappingMeta;
+  indexTypes: string[];
+  latestMappingsVersions: VirtualVersionMap;
+  hashToVersionMap?: Record<string, string>;
+}
+
 /**
  * Compares the current vs stored mappings' hashes or modelVersions.
- * Returns a list with all the types that have been updated.
+ * Returns 2 lists: one with all the new types and one with the types that have been updated.
  * @param indexMeta The meta information stored in the SO index
  * @param knownTypes The list of SO types that belong to the index and are enabled
  * @param latestMappingsVersions A map holding [type => version] with the latest versions where mappings have changed for each type
  * @param hashToVersionMap A map holding information about [md5 => modelVersion] equivalence
- * @returns the list of types that have been updated (in terms of their mappings)
+ * @returns the lists of new types and updated types
  */
-export const getUpdatedTypes = ({
+export const getNewAndUpdatedTypes = ({
   indexMeta,
   indexTypes,
   latestMappingsVersions,
   hashToVersionMap = {},
-}: {
-  indexMeta?: IndexMappingMeta;
-  indexTypes: string[];
-  latestMappingsVersions: VirtualVersionMap;
-  hashToVersionMap?: Record<string, string>;
-}): string[] => {
+}: GetUpdatedTypesParams) => {
   if (!indexMeta || (!indexMeta.mappingVersions && !indexMeta.migrationMappingPropertyHashes)) {
     // if we currently do NOT have meta information stored in the index
     // we consider that all types have been updated
-    return indexTypes;
+    return { newTypes: [], updatedTypes: indexTypes };
   }
 
   // If something exists in stored, but is missing in current
   // we don't care, as it could be a disabled plugin, etc
   // and keeping stale stuff around is better than migrating unecessesarily.
-  return indexTypes.filter((type) =>
-    isTypeUpdated({
+  const newTypes: string[] = [];
+  const updatedTypes: string[] = [];
+
+  indexTypes.forEach((type) => {
+    const status = checkTypeStatus({
       type,
       mappingVersion: latestMappingsVersions[type],
       indexMeta,
       hashToVersionMap,
-    })
-  );
+    });
+
+    if (status === 'new') {
+      newTypes.push(type);
+    } else if (status === 'updated') {
+      updatedTypes.push(type);
+    }
+  });
+
+  return { newTypes, updatedTypes };
 };
 
 /**
@@ -78,9 +91,9 @@ export const getUpdatedTypes = ({
  * @param mappingVersion The most recent model version that includes mappings changes
  * @param indexMeta The meta information stored in the SO index
  * @param hashToVersionMap A map holding information about [md5 => modelVersion] equivalence
- * @returns true if the mappings for the given type have changed since Kibana was last started
+ * @returns 'new' | 'updated' | 'unchanged' depending on whether the type has changed
  */
-function isTypeUpdated({
+function checkTypeStatus({
   type,
   mappingVersion,
   indexMeta,
@@ -90,7 +103,7 @@ function isTypeUpdated({
   mappingVersion: string;
   indexMeta: IndexMappingMeta;
   hashToVersionMap: Record<string, string>;
-}): boolean {
+}): 'new' | 'updated' | 'unchanged' {
   const latestMappingsVersion = Semver.parse(mappingVersion);
   if (!latestMappingsVersion) {
     throw new Error(
@@ -104,26 +117,28 @@ function isTypeUpdated({
     if (!indexVersion) {
       // either a new type, and thus there's not need to update + pickup any docs
       // or an old re-enabled type, which will be updated on OUTDATED_DOCUMENTS_TRANSFORM
-      return false;
+      return 'new';
     }
 
     // if the last version where mappings have changed is more recent than the one stored in the index
     // it means that the type has been updated
-    return latestMappingsVersion.compare(indexVersion) === 1;
+    return latestMappingsVersion.compare(indexVersion) === 1 ? 'updated' : 'unchanged';
   } else if (indexMeta.migrationMappingPropertyHashes) {
     const latestHash = indexMeta.migrationMappingPropertyHashes?.[type];
 
     if (!latestHash) {
       // either a new type, and thus there's not need to update + pickup any docs
       // or an old re-enabled type, which will be updated on OUTDATED_DOCUMENTS_TRANSFORM
-      return false;
+      return 'new';
     }
 
     const indexEquivalentVersion = hashToVersionMap[`${type}|${latestHash}`];
-    return !indexEquivalentVersion || latestMappingsVersion.compare(indexEquivalentVersion) === 1;
+    return !indexEquivalentVersion || latestMappingsVersion.compare(indexEquivalentVersion) === 1
+      ? 'updated'
+      : 'unchanged';
   }
 
   // at this point, the mappings do not contain any meta informataion
   // we consider the type has been updated, out of caution
-  return true;
+  return 'updated';
 }