Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.x] block SO setup API calls after startup (#58718) #58844

Merged
merged 1 commit into from
Feb 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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