Versioned common Space interface (#160237)

Closes #159708

## Summary

This PR replaces the common Space interface with a versioned interface
per [The road to versioned HTTP APIs
doc](https://docs.google.com/document/d/1wSj6S5mvbiZ-YeGnrH3McXl0EgLHIXj5T1kkVrbkov4/edit?pli=1#heading=h.ldcj84g80m8x),
and guidance of [Versioning
Interfaces](https://docs.elastic.dev/kibana-dev-docs/versioning-interfaces).
Additionally, this PR replaces the implicit use of saved object
attributes with an explicit conversion from the SO attributes to
versioned interface properties.

### Tests
-
x-pack/plugins/spaces/server/spaces_client/spaces_client_service.test.ts
- x-pack/test/functional/apps/spaces
- x-pack/test/api_integration/apis/spaces

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

x-pack/plugins/spaces/common/index.ts

@@ -13,4 +13,10 @@ export {
   DEFAULT_SPACE_ID,
 } from './constants';
 export { addSpaceIdToPath, getSpaceIdFromPath } from './lib/spaces_url_parser';
-export type { Space, GetAllSpacesOptions, GetAllSpacesPurpose, GetSpaceResult } from './types';
+export type {
+  Space,
+  GetAllSpacesOptions,
+  GetAllSpacesPurpose,
+  GetSpaceResult,
+} from './types/latest';
+export { spaceV1 } from './types';

x-pack/plugins/spaces/common/is_reserved_space.test.ts

@@ -5,8 +5,8 @@
  * 2.0.
  */
 
+import type { Space } from '.';
 import { isReservedSpace } from './is_reserved_space';
-import type { Space } from './types';
 
 test('it returns true for reserved spaces', () => {
   const space: Space = {

x-pack/plugins/spaces/common/is_reserved_space.ts

@@ -7,7 +7,7 @@
 
 import { get } from 'lodash';
 
-import type { Space } from './types';
+import type { Space } from '.';
 
 /**
  * Returns whether the given Space is reserved or not.

x-pack/plugins/spaces/common/types/index.ts

@@ -0,0 +1,8 @@
+/*
+ * 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; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+export * as spaceV1 from './space/v1';

x-pack/plugins/spaces/common/types/latest.ts

@@ -0,0 +1,8 @@
+/*
+ * 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; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+export * from './space/v1';

x-pack/plugins/spaces/server/routes/api/__fixtures__/create_mock_so_repository.ts

@@ -27,13 +27,14 @@ export const createMockSavedObjectsRepository = (spaces: any[] = []) => {
       if (spaces.find((s) => s.id === id)) {
         throw SavedObjectsErrorHelpers.decorateConflictError(new Error(), 'space conflict');
       }
-      return {};
+      return { id, attributes };
     }),
     update: jest.fn((type, id) => {
-      if (!spaces.find((s) => s.id === id)) {
+      const result = spaces.find((s) => s.id === id);
+      if (!result) {
         throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id);
       }
-      return {};
+      return result[0];
     }),
     bulkUpdate: jest.fn(),
     delete: jest.fn((type: string, id: string) => {

x-pack/plugins/spaces/server/spaces_client/spaces_client.test.ts

@@ -5,9 +5,10 @@
  * 2.0.
  */
 
+import type { SavedObject } from '@kbn/core-saved-objects-server';
 import { savedObjectsRepositoryMock } from '@kbn/core/server/mocks';
 
-import type { GetAllSpacesPurpose } from '../../common';
+import type { GetAllSpacesPurpose, Space } from '../../common';
 import type { ConfigType } from '../config';
 import { ConfigSchema } from '../config';
 import { SpacesClient } from './spaces_client';
@@ -21,51 +22,72 @@ const createMockConfig = (mockConfig: ConfigType = { enabled: true, maxSpaces: 1
 };
 
 describe('#getAll', () => {
-  const savedObjects = [
+  const savedObjects: Array<SavedObject<unknown>> = [
     {
+      // foo has all of the attributes expected by the space interface
       id: 'foo',
+      type: 'space',
+      references: [],
       attributes: {
         name: 'foo-name',
         description: 'foo-description',
-        bar: 'foo-bar',
+        color: '#FFFFFF',
+        initials: 'FB',
+        imageUrl: 'go-bots/predates/transformers',
+        disabledFeatures: [],
+        _reserved: true,
+        bar: 'foo-bar', // an extra attribute that will be ignored during conversion
       },
     },
     {
+      // bar his missing attributes of color and image url
       id: 'bar',
+      type: 'space',
+      references: [],
       attributes: {
         name: 'bar-name',
         description: 'bar-description',
-        bar: 'bar-bar',
+        initials: 'BA',
+        disabledFeatures: [],
+        bar: 'bar-bar', // an extra attribute that will be ignored during conversion
       },
     },
     {
+      // baz only has the bare minumum atributes
       id: 'baz',
+      type: 'space',
+      references: [],
       attributes: {
         name: 'baz-name',
         description: 'baz-description',
-        bar: 'baz-bar',
+        bar: 'baz-bar', // an extra attribute that will be ignored during conversion
       },
     },
   ];
 
-  const expectedSpaces = [
+  const expectedSpaces: Space[] = [
     {
       id: 'foo',
       name: 'foo-name',
       description: 'foo-description',
-      bar: 'foo-bar',
+      color: '#FFFFFF',
+      initials: 'FB',
+      imageUrl: 'go-bots/predates/transformers',
+      disabledFeatures: [],
+      _reserved: true,
     },
     {
       id: 'bar',
       name: 'bar-name',
       description: 'bar-description',
-      bar: 'bar-bar',
+      initials: 'BA',
+      disabledFeatures: [],
     },
     {
       id: 'baz',
       name: 'baz-name',
       description: 'baz-description',
-      bar: 'baz-bar',
+      disabledFeatures: [],
     },
   ];
 
@@ -101,22 +123,31 @@ describe('#getAll', () => {
 });
 
 describe('#get', () => {
-  const savedObject = {
+  const savedObject: SavedObject = {
     id: 'foo',
-    type: 'foo',
+    type: 'space',
     references: [],
     attributes: {
       name: 'foo-name',
       description: 'foo-description',
-      bar: 'foo-bar',
+      color: '#FFFFFF',
+      initials: 'FB',
+      imageUrl: 'go-bots/predates/transformers',
+      disabledFeatures: [],
+      _reserved: true,
+      bar: 'foo-bar', // an extra attribute that will be ignored during conversion
     },
   };
 
-  const expectedSpace = {
+  const expectedSpace: Space = {
     id: 'foo',
     name: 'foo-name',
     description: 'foo-description',
-    bar: 'foo-bar',
+    color: '#FFFFFF',
+    initials: 'FB',
+    imageUrl: 'go-bots/predates/transformers',
+    disabledFeatures: [],
+    _reserved: true,
   };
 
   test(`gets space using callWithRequestRepository`, async () => {
@@ -136,41 +167,35 @@ describe('#get', () => {
 
 describe('#create', () => {
   const id = 'foo';
-
-  const spaceToCreate = {
-    id,
-    name: 'foo-name',
-    description: 'foo-description',
-    bar: 'foo-bar',
-    _reserved: true,
-    disabledFeatures: [],
-  };
-
   const attributes = {
     name: 'foo-name',
     description: 'foo-description',
-    bar: 'foo-bar',
+    color: '#FFFFFF',
+    initials: 'FB',
+    imageUrl: 'go-bots/predates/transformers',
     disabledFeatures: [],
   };
 
-  const savedObject = {
+  const spaceToCreate = {
     id,
-    type: 'foo',
+    ...attributes,
+    _reserved: true,
+    bar: 'foo-bar', // will not make it to the saved object attributes
+  };
+
+  const savedObject: SavedObject = {
+    id,
+    type: 'space',
     references: [],
     attributes: {
-      name: 'foo-name',
-      description: 'foo-description',
-      bar: 'foo-bar',
-      disabledFeatures: [],
+      ...attributes,
+      foo: 'bar', // will get stripped in conversion
     },
   };
 
-  const expectedReturnedSpace = {
+  const expectedReturnedSpace: Space = {
     id,
-    name: 'foo-name',
-    description: 'foo-description',
-    bar: 'foo-bar',
-    disabledFeatures: [],
+    ...attributes,
   };
 
   test(`creates space using callWithRequestRepository when we're under the max`, async () => {
@@ -226,42 +251,37 @@ describe('#create', () => {
 });
 
 describe('#update', () => {
-  const spaceToUpdate = {
-    id: 'foo',
-    name: 'foo-name',
-    description: 'foo-description',
-    bar: 'foo-bar',
-    _reserved: false,
-    disabledFeatures: [],
-  };
-
   const attributes = {
     name: 'foo-name',
     description: 'foo-description',
-    bar: 'foo-bar',
+    color: '#FFFFFF',
+    initials: 'FB',
+    imageUrl: 'go-bots/predates/transformers',
     disabledFeatures: [],
   };
 
-  const savedObject = {
+  const spaceToUpdate = {
     id: 'foo',
-    type: 'foo',
+    ...attributes,
+    _reserved: false, // will have no affect
+    bar: 'foo-bar', // will not make it to the saved object attributes
+  };
+
+  const savedObject: SavedObject = {
+    id: 'foo',
+    type: 'space',
     references: [],
     attributes: {
-      name: 'foo-name',
-      description: 'foo-description',
-      bar: 'foo-bar',
+      ...attributes,
       _reserved: true,
-      disabledFeatures: [],
+      foo: 'bar', // will get stripped in conversion
     },
   };
 
-  const expectedReturnedSpace = {
+  const expectedReturnedSpace: Space = {
     id: 'foo',
-    name: 'foo-name',
-    description: 'foo-description',
-    bar: 'foo-bar',
+    ...attributes,
     _reserved: true,
-    disabledFeatures: [],
   };
 
   test(`updates space using callWithRequestRepository`, async () => {
@@ -283,9 +303,9 @@ describe('#update', () => {
 describe('#delete', () => {
   const id = 'foo';
 
-  const reservedSavedObject = {
+  const reservedSavedObject: SavedObject = {
     id,
-    type: 'foo',
+    type: 'space',
     references: [],
     attributes: {
       name: 'foo-name',
@@ -295,9 +315,9 @@ describe('#delete', () => {
     },
   };
 
-  const notReservedSavedObject = {
+  const notReservedSavedObject: SavedObject = {
     id,
-    type: 'foo',
+    type: 'space',
     references: [],
     attributes: {
       name: 'foo-name',
@@ -335,30 +355,25 @@ describe('#delete', () => {
     expect(mockCallWithRequestRepository.delete).toHaveBeenCalledWith('space', id);
     expect(mockCallWithRequestRepository.deleteByNamespace).toHaveBeenCalledWith(id);
   });
+});
 
-  describe('#disableLegacyUrlAliases', () => {
-    test(`updates legacy URL aliases using callWithRequestRepository`, async () => {
-      const mockDebugLogger = createMockDebugLogger();
-      const mockConfig = createMockConfig();
-      const mockCallWithRequestRepository = savedObjectsRepositoryMock.create();
+describe('#disableLegacyUrlAliases', () => {
+  test(`updates legacy URL aliases using callWithRequestRepository`, async () => {
+    const mockDebugLogger = createMockDebugLogger();
+    const mockConfig = createMockConfig();
+    const mockCallWithRequestRepository = savedObjectsRepositoryMock.create();
 
-      const client = new SpacesClient(
-        mockDebugLogger,
-        mockConfig,
-        mockCallWithRequestRepository,
-        []
-      );
-      const aliases = [
-        { targetSpace: 'space1', targetType: 'foo', sourceId: '123' },
-        { targetSpace: 'space2', targetType: 'bar', sourceId: '456' },
-      ];
-      await client.disableLegacyUrlAliases(aliases);
+    const client = new SpacesClient(mockDebugLogger, mockConfig, mockCallWithRequestRepository, []);
+    const aliases = [
+      { targetSpace: 'space1', targetType: 'foo', sourceId: '123' },
+      { targetSpace: 'space2', targetType: 'bar', sourceId: '456' },
+    ];
+    await client.disableLegacyUrlAliases(aliases);
 
-      expect(mockCallWithRequestRepository.bulkUpdate).toHaveBeenCalledTimes(1);
-      expect(mockCallWithRequestRepository.bulkUpdate).toHaveBeenCalledWith([
-        { type: 'legacy-url-alias', id: 'space1:foo:123', attributes: { disabled: true } },
-        { type: 'legacy-url-alias', id: 'space2:bar:456', attributes: { disabled: true } },
-      ]);
-    });
+    expect(mockCallWithRequestRepository.bulkUpdate).toHaveBeenCalledTimes(1);
+    expect(mockCallWithRequestRepository.bulkUpdate).toHaveBeenCalledWith([
+      { type: 'legacy-url-alias', id: 'space1:foo:123', attributes: { disabled: true } },
+      { type: 'legacy-url-alias', id: 'space2:bar:456', attributes: { disabled: true } },
+    ]);
   });
 });

x-pack/plugins/spaces/server/spaces_client/spaces_client.ts

@@ -6,7 +6,6 @@
  */
 
 import Boom from '@hapi/boom';
-import { omit } from 'lodash';
 
 import type { LegacyUrlAliasTarget } from '@kbn/core-saved-objects-common';
 import type {
@@ -15,11 +14,11 @@ import type {
   SavedObject,
 } from '@kbn/core/server';
 
-import type { GetAllSpacesOptions, GetAllSpacesPurpose, GetSpaceResult, Space } from '../../common';
 import { isReservedSpace } from '../../common';
+import type { spaceV1 as v1 } from '../../common';
 import type { ConfigType } from '../config';
 
-const SUPPORTED_GET_SPACE_PURPOSES: GetAllSpacesPurpose[] = [
+const SUPPORTED_GET_SPACE_PURPOSES: v1.GetAllSpacesPurpose[] = [
   'any',
   'copySavedObjectsIntoSpace',
   'findSavedObjects',
@@ -36,26 +35,26 @@ export interface ISpacesClient {
    * Retrieve all available spaces.
    * @param options controls which spaces are retrieved.
    */
-  getAll(options?: GetAllSpacesOptions): Promise<GetSpaceResult[]>;
+  getAll(options?: v1.GetAllSpacesOptions): Promise<v1.GetSpaceResult[]>;
 
   /**
    * Retrieve a space by its id.
    * @param id the space id.
    */
-  get(id: string): Promise<Space>;
+  get(id: string): Promise<v1.Space>;
 
   /**
    * Creates a space.
    * @param space the space to create.
    */
-  create(space: Space): Promise<Space>;
+  create(space: v1.Space): Promise<v1.Space>;
 
   /**
    * Updates a space.
    * @param id  the id of the space to update.
    * @param space the updated space.
    */
-  update(id: string, space: Space): Promise<Space>;
+  update(id: string, space: v1.Space): Promise<v1.Space>;
 
   /**
    * Returns a {@link ISavedObjectsPointInTimeFinder} to help page through
@@ -88,7 +87,7 @@ export class SpacesClient implements ISpacesClient {
     private readonly nonGlobalTypeNames: string[]
   ) {}
 
-  public async getAll(options: GetAllSpacesOptions = {}): Promise<GetSpaceResult[]> {
+  public async getAll(options: v1.GetAllSpacesOptions = {}): Promise<v1.GetSpaceResult[]> {
     const { purpose = DEFAULT_PURPOSE } = options;
     if (!SUPPORTED_GET_SPACE_PURPOSES.includes(purpose)) {
       throw Boom.badRequest(`unsupported space purpose: ${purpose}`);
@@ -113,7 +112,7 @@ export class SpacesClient implements ISpacesClient {
     return this.transformSavedObjectToSpace(savedObject);
   }
 
-  public async create(space: Space) {
+  public async create(space: v1.Space) {
     const { total } = await this.repository.find({
       type: 'space',
       page: 1,
@@ -127,8 +126,8 @@ export class SpacesClient implements ISpacesClient {
 
     this.debugLogger(`SpacesClient.create(), using RBAC. Attempting to create space`);
 
-    const attributes = omit(space, ['id', '_reserved']);
     const id = space.id;
+    const attributes = this.generateSpaceAttributes(space);
     const createdSavedObject = await this.repository.create('space', attributes, { id });
 
     this.debugLogger(`SpacesClient.create(), created space object`);
@@ -136,8 +135,8 @@ export class SpacesClient implements ISpacesClient {
     return this.transformSavedObjectToSpace(createdSavedObject);
   }
 
-  public async update(id: string, space: Space) {
-    const attributes = omit(space, 'id', '_reserved');
+  public async update(id: string, space: v1.Space) {
+    const attributes = this.generateSpaceAttributes(space);
     await this.repository.update('space', id, attributes);
     const updatedSavedObject = await this.repository.get('space', id);
     return this.transformSavedObjectToSpace(updatedSavedObject);
@@ -170,10 +169,27 @@ export class SpacesClient implements ISpacesClient {
     await this.repository.bulkUpdate(objectsToUpdate);
   }
 
-  private transformSavedObjectToSpace(savedObject: SavedObject<any>) {
+  private transformSavedObjectToSpace(savedObject: SavedObject<any>): v1.Space {
     return {
       id: savedObject.id,
-      ...savedObject.attributes,
-    } as Space;
+      name: savedObject.attributes.name ?? '',
+      description: savedObject.attributes.description,
+      color: savedObject.attributes.color,
+      initials: savedObject.attributes.initials,
+      imageUrl: savedObject.attributes.imageUrl,
+      disabledFeatures: savedObject.attributes.disabledFeatures ?? [],
+      _reserved: savedObject.attributes._reserved,
+    } as v1.Space;
+  }
+
+  private generateSpaceAttributes(space: v1.Space) {
+    return {
+      name: space.name,
+      description: space.description,
+      color: space.color,
+      initials: space.initials,
+      imageUrl: space.imageUrl,
+      disabledFeatures: space.disabledFeatures,
+    };
   }
 }