Additional testing and refactoring

Author: legrego

x-pack/plugins/security/server/plugin.ts

@@ -37,7 +37,7 @@ import { securityFeatures } from './features';
 import { ElasticsearchService } from './elasticsearch';
 import { SessionManagementService } from './session_management';
 import { registerSecurityUsageCollector } from './usage_collector';
-import { SecureSpacesClientWrapper, LegacySpacesAuditLogger } from './spaces';
+import { setupSpacesClient } from './spaces';
 
 export type SpacesService = Pick<
   SpacesPluginSetup['spacesService'],
@@ -211,23 +211,11 @@ export class Plugin {
       getCurrentUser: this.authc.getCurrentUser,
     });
 
-    if (spaces) {
-      spaces.spacesService.clientService.setClientRepositoryFactory(
-        (request, savedObjectsStart) => {
-          if (authz.mode.useRbacForRequest(request)) {
-            return savedObjectsStart.createInternalRepository(['space']);
-          }
-          return savedObjectsStart.createScopedRepository(request, ['space']);
-        }
-      );
-
-      const spacesAuditLogger = new LegacySpacesAuditLogger(audit.getLogger());
-
-      spaces.spacesService.clientService.registerClientWrapper(
-        (request, baseClient) =>
-          new SecureSpacesClientWrapper(baseClient, request, authz, spacesAuditLogger)
-      );
-    }
+    setupSpacesClient({
+      spaces,
+      audit,
+      authz,
+    });
 
     setupSavedObjects({
       legacyAuditLogger,

x-pack/plugins/security/server/spaces/index.ts

@@ -4,5 +4,4 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-export { LegacySpacesAuditLogger } from './legacy_audit_logger';
-export { SecureSpacesClientWrapper } from './secure_spaces_client_wrapper';
+export { setupSpacesClient } from './setup_spaces_client';

x-pack/plugins/security/server/spaces/secure_spaces_client_wrapper.test.ts

@@ -6,7 +6,7 @@
 
 import { httpServerMock } from '../../../../../src/core/server/mocks';
 
-import { SecureSpacesClientWrapper } from '.';
+import { SecureSpacesClientWrapper } from './secure_spaces_client_wrapper';
 
 import { spacesClientMock } from '../../../spaces/server/mocks';
 import { deepFreeze } from '@kbn/std';

x-pack/plugins/security/server/spaces/setup_spaces_client.test.ts

@@ -0,0 +1,34 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { spacesMock } from '../../../spaces/server/mocks';
+
+import { auditServiceMock } from '../audit/index.mock';
+import { authorizationMock } from '../authorization/index.mock';
+import { setupSpacesClient } from './setup_spaces_client';
+
+describe('setupSpacesClient', () => {
+  it('does not setup the spaces client when spaces is disabled', () => {
+    const authz = authorizationMock.create();
+    const audit = auditServiceMock.create();
+
+    setupSpacesClient({ authz, audit });
+
+    expect(audit.getLogger).not.toHaveBeenCalled();
+  });
+
+  it('configures the repository factory, wrapper, and audit logger', () => {
+    const authz = authorizationMock.create();
+    const audit = auditServiceMock.create();
+    const spaces = spacesMock.createSetup();
+
+    setupSpacesClient({ authz, audit, spaces });
+
+    expect(spaces.spacesClient.registerClientWrapper).toHaveBeenCalledTimes(1);
+    expect(spaces.spacesClient.setClientRepositoryFactory).toHaveBeenCalledTimes(1);
+    expect(audit.getLogger).toHaveBeenCalledTimes(1);
+  });
+});

x-pack/plugins/security/server/spaces/setup_spaces_client.ts

@@ -0,0 +1,38 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { SpacesPluginSetup } from '../../../spaces/server';
+import { AuditServiceSetup } from '../audit';
+import { AuthorizationServiceSetup } from '../authorization';
+import { LegacySpacesAuditLogger } from './legacy_audit_logger';
+import { SecureSpacesClientWrapper } from './secure_spaces_client_wrapper';
+
+interface Deps {
+  audit: AuditServiceSetup;
+  authz: AuthorizationServiceSetup;
+  spaces?: SpacesPluginSetup;
+}
+
+export const setupSpacesClient = ({ audit, authz, spaces }: Deps) => {
+  if (!spaces) {
+    return;
+  }
+  const { spacesClient } = spaces;
+
+  spacesClient.setClientRepositoryFactory((request, savedObjectsStart) => {
+    if (authz.mode.useRbacForRequest(request)) {
+      return savedObjectsStart.createInternalRepository(['space']);
+    }
+    return savedObjectsStart.createScopedRepository(request, ['space']);
+  });
+
+  const spacesAuditLogger = new LegacySpacesAuditLogger(audit.getLogger());
+
+  spacesClient.registerClientWrapper(
+    (request, baseClient) =>
+      new SecureSpacesClientWrapper(baseClient, request, authz, spacesAuditLogger)
+  );
+};

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

@@ -17,7 +17,7 @@ import { Plugin } from './plugin';
 
 export { SpacesPluginSetup, SpacesPluginStart } from './plugin';
 export { SpacesServiceSetup, SpacesServiceStart } from './spaces_service';
-export { ISpacesClient } from './lib/spaces_client';
+export { ISpacesClient } from './spaces_client';
 export { Space } from '../common/model/space';
 
 export const config = { schema: ConfigSchema };

x-pack/plugins/spaces/server/lib/spaces_tutorial_context_factory.test.ts

@@ -4,17 +4,14 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import * as Rx from 'rxjs';
 import { DEFAULT_SPACE_ID } from '../../common/constants';
 import { createSpacesTutorialContextFactory } from './spaces_tutorial_context_factory';
 import { SpacesService } from '../spaces_service';
-import { coreMock, loggingSystemMock } from '../../../../../src/core/server/mocks';
+import { coreMock } from '../../../../../src/core/server/mocks';
 import { spacesServiceMock } from '../spaces_service/spaces_service.mock';
-import { spacesConfig } from './__fixtures__';
+import { spacesClientServiceMock } from '../spaces_client/spaces_client_service.mock';
 
-const log = loggingSystemMock.createLogger();
-
-const service = new SpacesService(log);
+const service = new SpacesService();
 
 describe('createSpacesTutorialContextFactory', () => {
   it('should create a valid context factory', async () => {
@@ -36,11 +33,13 @@ describe('createSpacesTutorialContextFactory', () => {
 
   it('should create context with the current space id for the default space', async () => {
     service.setup({
-      http: coreMock.createSetup().http,
-      config$: Rx.of(spacesConfig),
+      basePath: coreMock.createSetup().http.basePath,
     });
     const contextFactory = createSpacesTutorialContextFactory(() =>
-      service.start(coreMock.createStart())
+      service.start({
+        basePath: coreMock.createStart().http.basePath,
+        spacesClientService: spacesClientServiceMock.createStart(),
+      })
     );
 
     const request = {};

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

@@ -3,10 +3,14 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
+import { spacesClientServiceMock } from './spaces_client/spaces_client_service.mock';
 import { spacesServiceMock } from './spaces_service/spaces_service.mock';
 
 function createSetupMock() {
-  return { spacesService: spacesServiceMock.createSetupContract() };
+  return {
+    spacesService: spacesServiceMock.createSetupContract(),
+    spacesClient: spacesClientServiceMock.createSetup(),
+  };
 }
 
 function createStartMock() {
@@ -20,4 +24,4 @@ export const spacesMock = {
   createStart: createStartMock,
 };
 
-export { spacesClientMock } from './lib/spaces_client/spaces_client.mock';
+export { spacesClientMock } from './spaces_client/spaces_client.mock';

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

@@ -28,10 +28,6 @@ describe('Spaces Plugin', () => {
             "setClientRepositoryFactory": [Function],
           },
           "spacesService": Object {
-            "clientService": Object {
-              "registerClientWrapper": [Function],
-              "setClientRepositoryFactory": [Function],
-            },
             "getSpaceId": [Function],
             "namespaceToSpaceId": [Function],
             "spaceIdToNamespace": [Function],

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

@@ -31,7 +31,11 @@ import { setupCapabilities } from './capabilities';
 import { SpacesSavedObjectsService } from './saved_objects';
 import { DefaultSpaceService } from './default_space';
 import { SpacesLicenseService } from '../common/licensing';
-import { SpacesClientRepositoryFactory, SpacesClientWrapper } from './lib/spaces_client';
+import {
+  SpacesClientRepositoryFactory,
+  SpacesClientService,
+  SpacesClientWrapper,
+} from './spaces_client';
 
 export interface PluginsSetup {
   features: FeaturesPluginSetup;
@@ -65,6 +69,8 @@ export class Plugin {
 
   private readonly spacesLicenseService = new SpacesLicenseService();
 
+  private readonly spacesClientService: SpacesClientService;
+
   private readonly spacesService: SpacesService;
 
   private spacesServiceStart?: SpacesServiceStart;
@@ -75,23 +81,18 @@ export class Plugin {
     this.config$ = initializerContext.config.create<ConfigType>();
     this.kibanaIndexConfig$ = initializerContext.config.legacy.globalConfig$;
     this.log = initializerContext.logger.get();
-    this.spacesService = new SpacesService(this.log);
-  }
-
-  public start(core: CoreStart) {
-    this.spacesServiceStart = this.spacesService.start(core);
-    return {
-      spacesService: this.spacesServiceStart,
-    };
+    this.spacesService = new SpacesService();
+    this.spacesClientService = new SpacesClientService((message) => this.log.debug(message));
   }
 
   public async setup(
     core: CoreSetup<PluginsStart>,
     plugins: PluginsSetup
   ): Promise<SpacesPluginSetup> {
+    const spacesClientSetup = this.spacesClientService.setup({ config$: this.config$ });
+
     const spacesServiceSetup = this.spacesService.setup({
-      http: core.http,
-      config$: this.config$,
+      basePath: core.http.basePath,
     });
 
     const getSpacesService = () => {
@@ -139,12 +140,7 @@ export class Plugin {
     initSpacesRequestInterceptors({
       http: core.http,
       log: this.log,
-      getSpacesService: () => {
-        if (!this.spacesServiceStart) {
-          throw new Error('Spaces Service is not yet initialized');
-        }
-        return this.spacesServiceStart;
-      },
+      getSpacesService,
       features: plugins.features,
     });
 
@@ -165,10 +161,21 @@ export class Plugin {
     }
 
     return {
+      spacesClient: spacesClientSetup,
       spacesService: spacesServiceSetup,
-      spacesClient: {
-        ...spacesServiceSetup.clientService,
-      },
+    };
+  }
+
+  public start(core: CoreStart) {
+    const spacesClientStart = this.spacesClientService.start(core);
+
+    this.spacesServiceStart = this.spacesService.start({
+      basePath: core.http.basePath,
+      spacesClientService: spacesClientStart,
+    });
+
+    return {
+      spacesService: this.spacesServiceStart,
     };
   }