NITs and review comments

docs/development/core/server/kibana-plugin-server.savedobjectsservicestart.createinternalrepository.md

@@ -11,8 +11,3 @@ Creates a [Saved Objects repository](./kibana-plugin-server.isavedobjectsreposit
 ```typescript
 createInternalRepository: (extraTypes?: string[]) => ISavedObjectsRepository;
 ```
-
-## Remarks
-
-The repository should only be used for creating and registering a client factory or client wrapper. Using the repository directly for interacting with Saved Objects is an anti-pattern. Use the Saved Objects client from the [SavedObjectsServiceStart\#getScopedClient](./kibana-plugin-server.savedobjectsservicestart.md) method or the [route handler context](./kibana-plugin-server.requesthandlercontext.md) instead.
-

docs/development/core/server/kibana-plugin-server.savedobjectsservicestart.createscopedrepository.md

@@ -14,5 +14,5 @@ createScopedRepository: (req: KibanaRequest, extraTypes?: string[]) => ISavedObj
 
 ## Remarks
 
-The repository should only be used for creating and registering a client factory or client wrapper. Using the repository directly for interacting with Saved Objects is an anti-pattern. Use the Saved Objects client from the [SavedObjectsServiceStart\#getScopedClient](./kibana-plugin-server.savedobjectsservicestart.md) method or the [route handler context](./kibana-plugin-server.requesthandlercontext.md) instead.
+Prefer using `getScopedClient`<!-- -->. This should only be used when using methods not exposed on [SavedObjectsClientContract](./kibana-plugin-server.savedobjectsclientcontract.md)
 

src/core/server/saved_objects/saved_objects_service.test.ts

@@ -23,31 +23,33 @@ import {
   clientProviderInstanceMock,
 } from './saved_objects_service.test.mocks';
 
-import { SavedObjectsService, SavedObjectsSetupDeps } from './saved_objects_service';
+import { SavedObjectsService } from './saved_objects_service';
 import { mockCoreContext } from '../core_context.mock';
 import * as legacyElasticsearch from 'elasticsearch';
 import { Env } from '../config';
 import { configServiceMock } from '../mocks';
+import { elasticsearchServiceMock } from '../elasticsearch/elasticsearch_service.mock';
+import { legacyServiceMock } from '../legacy/legacy_service.mock';
 import { SavedObjectsClientFactoryProvider } from './service/lib';
 
-afterEach(() => {
-  jest.clearAllMocks();
-});
-
 describe('SavedObjectsService', () => {
-  const createSetupDeps = ({ clusterClient = { callAsInternalUser: jest.fn() } }) => {
-    return ({
-      elasticsearch: { adminClient: clusterClient },
-      legacyPlugins: { uiExports: { savedObjectMappings: [] }, pluginExtendedConfig: {} },
-    } as unknown) as SavedObjectsSetupDeps;
+  const createSetupDeps = () => {
+    return {
+      elasticsearch: elasticsearchServiceMock.createInternalSetup(),
+      legacyPlugins: legacyServiceMock.createDiscoverPlugins(),
+    };
   };
 
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
   describe('#setup()', () => {
     describe('#setClientFactoryProvider', () => {
       it('registers the factory to the clientProvider', async () => {
         const coreContext = mockCoreContext.create();
         const soService = new SavedObjectsService(coreContext);
-        const setup = await soService.setup(createSetupDeps({}));
+        const setup = await soService.setup(createSetupDeps());
 
         const factory = jest.fn();
         const factoryProvider: SavedObjectsClientFactoryProvider = () => factory;
@@ -61,7 +63,7 @@ describe('SavedObjectsService', () => {
       it('throws if a factory is already registered', async () => {
         const coreContext = mockCoreContext.create();
         const soService = new SavedObjectsService(coreContext);
-        const setup = await soService.setup(createSetupDeps({}));
+        const setup = await soService.setup(createSetupDeps());
 
         const firstFactory = () => jest.fn();
         const secondFactory = () => jest.fn();
@@ -80,7 +82,7 @@ describe('SavedObjectsService', () => {
       it('registers the wrapper to the clientProvider', async () => {
         const coreContext = mockCoreContext.create();
         const soService = new SavedObjectsService(coreContext);
-        const setup = await soService.setup(createSetupDeps({}));
+        const setup = await soService.setup(createSetupDeps());
 
         const wrapperA = jest.fn();
         const wrapperB = jest.fn();
@@ -108,19 +110,18 @@ describe('SavedObjectsService', () => {
   describe('#start()', () => {
     it('creates a KibanaMigrator which retries NoConnections errors from callAsInternalUser', async () => {
       const coreContext = mockCoreContext.create();
-      let i = 0;
-      const clusterClient = {
-        callAsInternalUser: jest
-          .fn()
-          .mockImplementation(() =>
-            i++ <= 2
-              ? Promise.reject(new legacyElasticsearch.errors.NoConnections())
-              : Promise.resolve('success')
-          ),
-      };
 
       const soService = new SavedObjectsService(coreContext);
-      const coreSetup = createSetupDeps({ clusterClient });
+      const coreSetup = createSetupDeps();
+
+      let i = 0;
+      coreSetup.elasticsearch.adminClient.callAsInternalUser = jest
+        .fn()
+        .mockImplementation(() =>
+          i++ <= 2
+            ? Promise.reject(new legacyElasticsearch.errors.NoConnections())
+            : Promise.resolve('success')
+        );
 
       await soService.setup(coreSetup);
       await soService.start({}, 1);
@@ -134,7 +135,7 @@ describe('SavedObjectsService', () => {
       });
       const soService = new SavedObjectsService(coreContext);
 
-      await soService.setup(createSetupDeps({}));
+      await soService.setup(createSetupDeps());
       await soService.start({});
       expect(migratorInstanceMock.runMigrations).toHaveBeenCalledWith(true);
     });
@@ -143,7 +144,7 @@ describe('SavedObjectsService', () => {
       const configService = configServiceMock.create({ atPath: { skip: true } });
       const coreContext = mockCoreContext.create({ configService });
       const soService = new SavedObjectsService(coreContext);
-      await soService.setup(createSetupDeps({}));
+      await soService.setup(createSetupDeps());
       await soService.start({});
       expect(migratorInstanceMock.runMigrations).toHaveBeenCalledWith(true);
     });
@@ -152,7 +153,7 @@ describe('SavedObjectsService', () => {
       const configService = configServiceMock.create({ atPath: { skip: false } });
       const coreContext = mockCoreContext.create({ configService });
       const soService = new SavedObjectsService(coreContext);
-      await soService.setup(createSetupDeps({}));
+      await soService.setup(createSetupDeps());
       expect(migratorInstanceMock.runMigrations).toHaveBeenCalledTimes(0);
 
       const startContract = await soService.start({});

src/core/server/saved_objects/saved_objects_service.ts

@@ -131,28 +131,19 @@ export interface SavedObjectsServiceStart {
    * uses the credentials from the passed in request to authenticate with
    * Elasticsearch.
    *
+   * @param req - The request to create the scoped repository from.
+   * @param extraTypes - A list of additional hidden types the repository should have access to.
+   *
    * @remarks
-   * The repository should only be used for creating and registering a client
-   * factory or client wrapper. Using the repository directly for interacting
-   * with Saved Objects is an anti-pattern. Use the Saved Objects client from
-   * the
-   * {@link SavedObjectsServiceStart | SavedObjectsServiceStart#getScopedClient }
-   * method or the {@link RequestHandlerContext | route handler context}
-   * instead.
+   * Prefer using `getScopedClient`. This should only be used when using methods
+   * not exposed on {@link SavedObjectsClientContract}
    */
   createScopedRepository: (req: KibanaRequest, extraTypes?: string[]) => ISavedObjectsRepository;
   /**
    * Creates a {@link ISavedObjectsRepository | Saved Objects repository} that
    * uses the internal Kibana user for authenticating with Elasticsearch.
    *
-   * @remarks
-   * The repository should only be used for creating and registering a client
-   * factory or client wrapper. Using the repository directly for interacting
-   * with Saved Objects is an anti-pattern. Use the Saved Objects client from
-   * the
-   * {@link SavedObjectsServiceStart | SavedObjectsServiceStart#getScopedClient }
-   * method or the {@link RequestHandlerContext | route handler context}
-   * instead.
+   * @param extraTypes - A list of additional hidden types the repository should have access to.
    */
   createInternalRepository: (extraTypes?: string[]) => ISavedObjectsRepository;
 }