[SOR] validation schema: use previous version

packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/repository.ts

@@ -208,6 +208,7 @@ export class SavedObjectsRepository implements ISavedObjectsRepository {
   private _mappings: IndexMapping;
   private _registry: ISavedObjectTypeRegistry;
   private _allowedTypes: string[];
+  private typeValidatorMap: Record<string, SavedObjectsTypeValidator> = {};
   private readonly client: RepositoryEsClient;
   private readonly _encryptionExtension?: ISavedObjectsEncryptionExtension;
   private readonly _securityExtension?: ISavedObjectsSecurityExtension;
@@ -403,7 +404,7 @@ export class SavedObjectsRepository implements ISavedObjectsRepository {
      * migration to fail, but it's the best we can do without devising a way to run validations
      * inside the migration algorithm itself.
      */
-    this.validateObjectAttributes(type, migrated as SavedObjectSanitizedDoc<T>);
+    this.validateObjectForCreate(type, migrated as SavedObjectSanitizedDoc<T>);
 
     const raw = this._serializer.savedObjectToRaw(migrated as SavedObjectSanitizedDoc<T>);
 
@@ -629,7 +630,7 @@ export class SavedObjectsRepository implements ISavedObjectsRepository {
          * inside the migration algorithm itself.
          */
         try {
-          this.validateObjectAttributes(object.type, migrated);
+          this.validateObjectForCreate(object.type, migrated);
         } catch (error) {
           return {
             tag: 'Left',
@@ -2757,25 +2758,31 @@ export class SavedObjectsRepository implements ISavedObjectsRepository {
   }
 
   /** Validate a migrated doc against the registered saved object type's schema. */
-  private validateObjectAttributes(type: string, doc: SavedObjectSanitizedDoc) {
-    const savedObjectType = this._registry.getType(type);
-    if (!savedObjectType?.schemas) {
+  private validateObjectForCreate(type: string, doc: SavedObjectSanitizedDoc) {
+    if (!this._registry.getType(type)) {
       return;
     }
-
-    const validator = new SavedObjectsTypeValidator({
-      logger: this._logger.get('type-validator'),
-      type,
-      validationMap: savedObjectType.schemas,
-    });
-
+    const validator = this.getTypeValidator(type);
     try {
-      validator.validate(this._migrator.kibanaVersion, doc);
+      validator.validate(doc, this._migrator.kibanaVersion);
     } catch (error) {
       throw SavedObjectsErrorHelpers.createBadRequestError(error.message);
     }
   }
 
+  private getTypeValidator(type: string): SavedObjectsTypeValidator {
+    if (!this.typeValidatorMap[type]) {
+      const savedObjectType = this._registry.getType(type);
+      this.typeValidatorMap[type] = new SavedObjectsTypeValidator({
+        logger: this._logger.get('type-validator'),
+        type,
+        validationMap: savedObjectType!.schemas ?? {},
+        defaultVersion: this._migrator.kibanaVersion,
+      });
+    }
+    return this.typeValidatorMap[type]!;
+  }
+
   /** This is used when objects are created. */
   private validateOriginId(type: string, objectOrOptions: { originId?: string }) {
     if (

packages/core/saved-objects/core-saved-objects-base-server-internal/src/validation/schema.ts

@@ -19,33 +19,40 @@ type SavedObjectSanitizedDocSchema = {
   [K in keyof Required<SavedObjectSanitizedDoc>]: Type<SavedObjectSanitizedDoc[K]>;
 };
 
+const baseSchema = schema.object<SavedObjectSanitizedDocSchema>({
+  id: schema.string(),
+  type: schema.string(),
+  references: schema.arrayOf(
+    schema.object({
+      name: schema.string(),
+      type: schema.string(),
+      id: schema.string(),
+    }),
+    { defaultValue: [] }
+  ),
+  namespace: schema.maybe(schema.string()),
+  namespaces: schema.maybe(schema.arrayOf(schema.string())),
+  migrationVersion: schema.maybe(schema.recordOf(schema.string(), schema.string())),
+  coreMigrationVersion: schema.maybe(schema.string()),
+  typeMigrationVersion: schema.maybe(schema.string()),
+  updated_at: schema.maybe(schema.string()),
+  created_at: schema.maybe(schema.string()),
+  version: schema.maybe(schema.string()),
+  originId: schema.maybe(schema.string()),
+  managed: schema.maybe(schema.boolean()),
+  attributes: schema.maybe(schema.any()),
+});
+
 /**
  * Takes a {@link SavedObjectsValidationSpec} and returns a full schema representing
  * a {@link SavedObjectSanitizedDoc}, with the spec applied to the object's `attributes`.
  *
  * @internal
  */
-export const createSavedObjectSanitizedDocSchema = (attributesSchema: SavedObjectsValidationSpec) =>
-  schema.object<SavedObjectSanitizedDocSchema>({
+export const createSavedObjectSanitizedDocSchema = (
+  attributesSchema: SavedObjectsValidationSpec
+) => {
+  return baseSchema.extends({
     attributes: attributesSchema,
-    id: schema.string(),
-    type: schema.string(),
-    references: schema.arrayOf(
-      schema.object({
-        name: schema.string(),
-        type: schema.string(),
-        id: schema.string(),
-      }),
-      { defaultValue: [] }
-    ),
-    namespace: schema.maybe(schema.string()),
-    namespaces: schema.maybe(schema.arrayOf(schema.string())),
-    migrationVersion: schema.maybe(schema.recordOf(schema.string(), schema.string())),
-    coreMigrationVersion: schema.maybe(schema.string()),
-    typeMigrationVersion: schema.maybe(schema.string()),
-    updated_at: schema.maybe(schema.string()),
-    created_at: schema.maybe(schema.string()),
-    version: schema.maybe(schema.string()),
-    originId: schema.maybe(schema.string()),
-    managed: schema.maybe(schema.boolean()),
   });
+};

packages/core/saved-objects/core-saved-objects-base-server-internal/src/validation/validator.test.ts

@@ -10,79 +10,162 @@ import { schema } from '@kbn/config-schema';
 import { loggerMock, type MockedLogger } from '@kbn/logging-mocks';
 import type {
   SavedObjectSanitizedDoc,
+  SavedObjectsValidationSpec,
   SavedObjectsValidationMap,
 } from '@kbn/core-saved-objects-server';
 import { SavedObjectsTypeValidator } from './validator';
 
+const defaultVersion = '3.3.0';
+const type = 'my-type';
+
 describe('Saved Objects type validator', () => {
   let validator: SavedObjectsTypeValidator;
   let logger: MockedLogger;
+  let validationMap: SavedObjectsValidationMap;
 
-  const type = 'my-type';
-  const validationMap: SavedObjectsValidationMap = {
-    '1.0.0': schema.object({
-      foo: schema.string(),
-    }),
-  };
-
-  const createMockObject = (attributes: Record<string, unknown>): SavedObjectSanitizedDoc => ({
-    attributes,
+  const createMockObject = (parts: Partial<SavedObjectSanitizedDoc>): SavedObjectSanitizedDoc => ({
+    type,
     id: 'test-id',
     references: [],
-    type,
+    attributes: {},
+    ...parts,
   });
 
   beforeEach(() => {
     logger = loggerMock.create();
-    validator = new SavedObjectsTypeValidator({ logger, type, validationMap });
   });
 
   afterEach(() => {
-    jest.clearAllMocks();
+    jest.resetAllMocks();
   });
 
-  it('should do nothing if no matching validation could be found', () => {
-    const data = createMockObject({ foo: false });
-    expect(validator.validate('3.0.0', data)).toBeUndefined();
-    expect(logger.debug).not.toHaveBeenCalled();
-  });
+  describe('validation behavior', () => {
+    beforeEach(() => {
+      validationMap = {
+        '1.0.0': schema.object({
+          foo: schema.string(),
+        }),
+      };
+      validator = new SavedObjectsTypeValidator({ logger, type, validationMap, defaultVersion });
+    });
 
-  it('should log when a validation fails', () => {
-    const data = createMockObject({ foo: false });
-    expect(() => validator.validate('1.0.0', data)).toThrowError();
-    expect(logger.warn).toHaveBeenCalledTimes(1);
-  });
+    it('should log when a validation fails', () => {
+      const data = createMockObject({ attributes: { foo: false } });
+      expect(() => validator.validate(data, '1.0.0')).toThrowError();
+      expect(logger.warn).toHaveBeenCalledTimes(1);
+    });
 
-  it('should work when given valid values', () => {
-    const data = createMockObject({ foo: 'hi' });
-    expect(() => validator.validate('1.0.0', data)).not.toThrowError();
-  });
+    it('should work when given valid values', () => {
+      const data = createMockObject({ attributes: { foo: 'hi' } });
+      expect(() => validator.validate(data, '1.0.0')).not.toThrowError();
+    });
 
-  it('should throw an error when given invalid values', () => {
-    const data = createMockObject({ foo: false });
-    expect(() => validator.validate('1.0.0', data)).toThrowErrorMatchingInlineSnapshot(
-      `"[attributes.foo]: expected value of type [string] but got [boolean]"`
-    );
-  });
+    it('should throw an error when given invalid values', () => {
+      const data = createMockObject({ attributes: { foo: false } });
+      expect(() => validator.validate(data, '1.0.0')).toThrowErrorMatchingInlineSnapshot(
+        `"[attributes.foo]: expected value of type [string] but got [boolean]"`
+      );
+    });
 
-  it('should throw an error if fields other than attributes are malformed', () => {
-    const data = createMockObject({ foo: 'hi' });
-    // @ts-expect-error Intentionally malformed object
-    data.updated_at = false;
-    expect(() => validator.validate('1.0.0', data)).toThrowErrorMatchingInlineSnapshot(
-      `"[updated_at]: expected value of type [string] but got [boolean]"`
-    );
+    it('should throw an error if fields other than attributes are malformed', () => {
+      const data = createMockObject({ attributes: { foo: 'hi' } });
+      // @ts-expect-error Intentionally malformed object
+      data.updated_at = false;
+      expect(() => validator.validate(data, '1.0.0')).toThrowErrorMatchingInlineSnapshot(
+        `"[updated_at]: expected value of type [string] but got [boolean]"`
+      );
+    });
+
+    it('works when the validation map is a function', () => {
+      const fnValidationMap: () => SavedObjectsValidationMap = () => validationMap;
+      validator = new SavedObjectsTypeValidator({
+        logger,
+        type,
+        validationMap: fnValidationMap,
+        defaultVersion,
+      });
+
+      const data = createMockObject({ attributes: { foo: 'hi' } });
+      expect(() => validator.validate(data, '1.0.0')).not.toThrowError();
+    });
   });
 
-  it('works when the validation map is a function', () => {
-    const fnValidationMap: () => SavedObjectsValidationMap = () => validationMap;
-    validator = new SavedObjectsTypeValidator({
-      logger,
-      type,
-      validationMap: fnValidationMap,
+  describe('schema selection', () => {
+    beforeEach(() => {
+      validationMap = {
+        '2.0.0': createStubSpec(),
+        '2.7.0': createStubSpec(),
+        '3.0.0': createStubSpec(),
+        '3.5.0': createStubSpec(),
+        '4.0.0': createStubSpec(),
+        '4.3.0': createStubSpec(),
+      };
+      validator = new SavedObjectsTypeValidator({ logger, type, validationMap, defaultVersion });
     });
 
-    const data = createMockObject({ foo: 'hi' });
-    expect(() => validator.validate('1.0.0', data)).not.toThrowError();
+    const createStubSpec = (): jest.Mocked<SavedObjectsValidationSpec> => {
+      const stub = schema.object({}, { unknowns: 'allow', defaultValue: {} });
+      jest.spyOn(stub as any, 'getSchema');
+      return stub as jest.Mocked<SavedObjectsValidationSpec>;
+    };
+
+    const getCalledVersion = () => {
+      for (const [version, validation] of Object.entries(validationMap)) {
+        if (((validation as any).getSchema as jest.MockedFn<any>).mock.calls.length > 0) {
+          return version;
+        }
+      }
+      return undefined;
+    };
+
+    it('should use the correct schema when specifying the version', () => {
+      let data = createMockObject({ typeMigrationVersion: '2.2.0' });
+      validator.validate(data, '3.2.0');
+      expect(getCalledVersion()).toEqual('3.0.0');
+
+      jest.clearAllMocks();
+
+      data = createMockObject({ typeMigrationVersion: '3.5.0' });
+      validator.validate(data, '4.5.0');
+      expect(getCalledVersion()).toEqual('4.3.0');
+    });
+
+    it('should use the correct schema for documents with typeMigrationVersion', () => {
+      let data = createMockObject({ typeMigrationVersion: '3.2.0' });
+      validator.validate(data);
+      expect(getCalledVersion()).toEqual('3.0.0');
+
+      jest.clearAllMocks();
+
+      data = createMockObject({ typeMigrationVersion: '3.5.0' });
+      validator.validate(data);
+      expect(getCalledVersion()).toEqual('3.5.0');
+    });
+
+    it('should use the correct schema for documents with migrationVersion', () => {
+      let data = createMockObject({
+        migrationVersion: {
+          [type]: '4.6.0',
+        },
+      });
+      validator.validate(data);
+      expect(getCalledVersion()).toEqual('4.3.0');
+
+      jest.clearAllMocks();
+
+      data = createMockObject({
+        migrationVersion: {
+          [type]: '4.0.0',
+        },
+      });
+      validator.validate(data);
+      expect(getCalledVersion()).toEqual('4.0.0');
+    });
+
+    it('should use the correct schema for documents without a version specified', () => {
+      const data = createMockObject({});
+      validator.validate(data);
+      expect(getCalledVersion()).toEqual('3.0.0');
+    });
   });
 });