Skip to content

Commit

Permalink
block SO setup API calls after startup (#58718) (#58844)
Browse files Browse the repository at this point in the history
  • Loading branch information
pgayvallet authored Feb 28, 2020
1 parent 53d7f08 commit 85fd92a
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ export interface SavedObjectsServiceSetup

When plugins access the Saved Objects client, a new client is created using the factory provided to `setClientFactory` and wrapped by all wrappers registered through `addClientWrapper`<!-- -->.

All the setup APIs will throw if called after the service has started, and therefor cannot be used from legacy plugin code. Legacy plugins should use the legacy savedObject service until migrated.

## Example 1


Expand Down
8 changes: 7 additions & 1 deletion src/core/MIGRATION_EXAMPLES.md
Original file line number Diff line number Diff line change
Expand Up @@ -917,4 +917,10 @@ Would be converted to:
```typescript
const migration: SavedObjectMigrationFn = (doc, { log }) => {...}
```
```
### Remarks
The `registerType` API will throw if called after the service has started, and therefor cannot be used from
legacy plugin code. Legacy plugins should use the legacy savedObjects service and the legacy way to register
saved object types until migrated.
30 changes: 30 additions & 0 deletions src/core/server/saved_objects/saved_objects_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,36 @@ describe('SavedObjectsService', () => {
expect(migratorInstanceMock.runMigrations).toHaveBeenCalledTimes(1);
});

it('throws when calling setup APIs once started', async () => {
const coreContext = createCoreContext({ skipMigration: false });
const soService = new SavedObjectsService(coreContext);
const setup = await soService.setup(createSetupDeps());
await soService.start({});

expect(() => {
setup.setClientFactoryProvider(jest.fn());
}).toThrowErrorMatchingInlineSnapshot(
`"cannot call \`setClientFactoryProvider\` after service startup."`
);

expect(() => {
setup.addClientWrapper(0, 'dummy', jest.fn());
}).toThrowErrorMatchingInlineSnapshot(
`"cannot call \`addClientWrapper\` after service startup."`
);

expect(() => {
setup.registerType({
name: 'someType',
hidden: false,
namespaceAgnostic: false,
mappings: { properties: {} },
});
}).toThrowErrorMatchingInlineSnapshot(
`"cannot call \`registerType\` after service startup."`
);
});

describe('#getTypeRegistry', () => {
it('returns the internal type registry of the service', async () => {
const coreContext = createCoreContext({ skipMigration: false });
Expand Down
15 changes: 15 additions & 0 deletions src/core/server/saved_objects/saved_objects_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ import { registerRoutes } from './routes';
* the factory provided to `setClientFactory` and wrapped by all wrappers
* registered through `addClientWrapper`.
*
* All the setup APIs will throw if called after the service has started, and therefor cannot be used
* from legacy plugin code. Legacy plugins should use the legacy savedObject service until migrated.
*
* @example
* ```ts
* import { SavedObjectsClient, CoreSetup } from 'src/core/server';
Expand Down Expand Up @@ -275,6 +278,7 @@ export class SavedObjectsService
private migrator$ = new Subject<KibanaMigrator>();
private typeRegistry = new SavedObjectTypeRegistry();
private validations: PropertyValidators = {};
private started = false;

constructor(private readonly coreContext: CoreContext) {
this.logger = coreContext.logger.get('savedobjects-service');
Expand Down Expand Up @@ -316,19 +320,28 @@ export class SavedObjectsService

return {
setClientFactoryProvider: provider => {
if (this.started) {
throw new Error('cannot call `setClientFactoryProvider` after service startup.');
}
if (this.clientFactoryProvider) {
throw new Error('custom client factory is already set, and can only be set once');
}
this.clientFactoryProvider = provider;
},
addClientWrapper: (priority, id, factory) => {
if (this.started) {
throw new Error('cannot call `addClientWrapper` after service startup.');
}
this.clientFactoryWrappers.push({
priority,
id,
factory,
});
},
registerType: type => {
if (this.started) {
throw new Error('cannot call `registerType` after service startup.');
}
this.typeRegistry.registerType(type);
},
};
Expand Down Expand Up @@ -415,6 +428,8 @@ export class SavedObjectsService
clientProvider.addClientWrapperFactory(priority, id, factory);
});

this.started = true;

return {
migrator,
clientProvider,
Expand Down

0 comments on commit 85fd92a

Please sign in to comment.