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

Server saved objects client through request context #44143

Merged
merged 12 commits into from
Oct 16, 2019

Conversation

rudolf
Copy link
Contributor

@rudolf rudolf commented Aug 27, 2019

Summary

Expose the Saved Objects client to server-side plugins through the request handler context.

Since this moves the Saved Objects Client Provider into Core I also had to fix #47012 as part of this work. SavedObjects#find with KQL filtering no longer depends on indexPatterns.

KQL uses indexPatterns only for creating queries against scripted fields or optimizing queries where all fields in the index should be returned. Neither of these are relevant to SavedObjects#find. In the future KQL is likely to become more dependant on indexPatterns but I'd rather wait till we have a valuable use case before introducing this dependency.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@rudolf rudolf added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Aug 27, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@epixa epixa mentioned this pull request Sep 30, 2019
20 tasks
@rudolf rudolf force-pushed the server-saved-objects-client branch from 5747758 to 25e8fb3 Compare October 1, 2019 13:04
@elasticmachine
Copy link
Contributor

💔 Build Failed

@rudolf rudolf force-pushed the server-saved-objects-client branch from 25e8fb3 to 08e8a40 Compare October 2, 2019 19:41
@rudolf rudolf marked this pull request as ready for review October 2, 2019 19:45
@rudolf rudolf requested a review from a team as a code owner October 2, 2019 19:45
@elasticmachine
Copy link
Contributor

💔 Build Failed

@rudolf rudolf added the release_note:skip Skip the PR/issue when compiling release notes label Oct 3, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@rudolf rudolf requested a review from a team as a code owner October 11, 2019 20:15
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Contributor

@mshustov mshustov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some nits

src/core/server/legacy/legacy_service.ts Outdated Show resolved Hide resolved
} as unknown) as SavedObjectsSetupDeps;

const savedObjectsSetup = await soService.setup(coreSetup);
expect(savedObjectsSetup.clientProvider).toBeInstanceOf(SavedObjectsClientProvider);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TypeScript guarantees this. Doesn't it?

@@ -56,7 +56,7 @@ describe('Search service', () => {

registerSearchRoute(routerMock);
const handler = routerMock.post.mock.calls[0][1];
await handler(mockContext, mockRequest, mockResponse);
await handler((mockContext as unknown) as RequestHandlerContext, mockRequest, mockResponse);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we provide RequestHandlerContextMock? can be done as a follow up

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@streamich streamich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AppArch changes LGTM

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@mshustov mshustov requested a review from XavierM October 16, 2019 08:31
@rudolf rudolf merged commit 3d28467 into elastic:master Oct 16, 2019
@rudolf rudolf deleted the server-saved-objects-client branch October 16, 2019 08:36
rudolf added a commit to rudolf/kibana that referenced this pull request Oct 16, 2019
* Expose Saved Objects client in request context

* API Integration test for savedobjects in req context

* SavedObjectsClient docs

* SavedObjectsClient#find remove dependency on indexPatterns

And use the saved objects mappings instead

* Review comments

* Review comments, fixes and tests

* Use correct type for KQL syntax check
rudolf added a commit that referenced this pull request Oct 16, 2019
* Expose Saved Objects client in request context

* API Integration test for savedobjects in req context

* SavedObjectsClient docs

* SavedObjectsClient#find remove dependency on indexPatterns

And use the saved objects mappings instead

* Review comments

* Review comments, fixes and tests

* Use correct type for KQL syntax check
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:New Platform Feature:Saved Objects release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.5.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SavedObjects KQL support: remove dependency on IndexPatterns
6 participants