[data.search] Expose SearchSource on the server. by lukeelmers · Pull Request #78383 · elastic/kibana

lukeelmers · 2020-09-24T04:32:26Z

Summary

This is the final PR which exposes SearchSource on the server:

Refactors msearch route that's used in the legacy "default" search strategy so that the business logic is extracted into a helper function which can be reused in SearchSource
Adds contract to search service on the server
- API is await data.search.searchSource.asScoped(kibanaRequest);, which returns the same API as is available on the client.
Adds a plugin functional test suite to verify the service on the server

Testing

This PR relies on the added functional tests to validate that everything works, as there is not currently anywhere in Kibana which uses this service on the server.

Dev Docs

The high-level search API SearchSource is now available on the server. Usage:

function async myRouteHandler(context, request, response) {
  const searchSource = await data.search.searchSource.asScoped(request);
  searchSource.createEmpty(); // API after calling `asScoped` matches the client-side service
}

lukeelmers · 2020-09-24T04:36:39Z

src/plugins/data/server/search/routes/call_msearch.ts

@lukasolson Let me know if I got this abort logic right. I tested and AFAICT it all works as expected.

lukeelmers · 2020-09-24T04:37:18Z

src/plugins/data/server/search/routes/msearch.ts

This got simplified to a helper which could also be called outside of a route handler

lukeelmers · 2020-09-24T04:38:03Z

src/plugins/data/server/search/search_service.ts

@lukasolson @lizozom @ppisljar We should discuss the fakeRequestHandlerContext here -- I added it to our list to cover offline.

i am fine moving forward with this but lets open an issue for refactoring we want to do

Takeaway from our offline discussion: we will keep as-is for now as @ppisljar suggests, and discuss the proposed low-level search service changes separately.

Here's the issue for discussing changes to the low-level search API: #78461

lukeelmers · 2020-09-24T04:38:42Z

src/plugins/data/server/search/search_service.ts

loadingCount$ isn't really necessary on the server, so we just initialize BehaviorSubject that never changes

lukeelmers · 2020-09-24T04:40:03Z

src/plugins/data/server/search/search_service.ts

Accepting a KibanaRequest was pretty much my only option for the API since it's what index patterns require on the server. The question (as outlined in the code comment below) is whether it's necessary to also require RequestHandlerContext for the low-level search, when all of those items can already be derived from a KibanaRequest

lukeelmers · 2020-09-24T04:41:19Z

test/plugin_functional/plugins/data_search/server/plugin.ts

These tests create a few dummy routes that can be called from the client to execute various pieces of the searchSource service on the server.

lukeelmers · 2020-09-24T04:41:53Z

test/plugin_functional/test_suites/data_plugin/index_patterns.ts

relocated to top-level before that loads before both index patterns and search

elasticmachine · 2020-09-24T04:43:12Z

Pinging @elastic/kibana-app-arch (Team:AppArch)

ppisljar

code LGTM

lukeelmers · 2020-09-26T01:05:51Z

@elasticmachine merge upstream

lukeelmers · 2020-09-26T01:06:11Z

@elasticmachine merge upstream

elasticmachine · 2020-09-26T01:06:12Z

merge conflict between base and head

kibanamachine · 2020-09-29T18:29:35Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 5aebda3

Metrics [docs]

@kbn/optimizer bundle module count

id	value	diff	baseline
`data`	569	+1	568

distributable file count

id	value	diff	baseline
`default`	45826	+2	45824
`oss`	26576	+2	26574

page load bundle size

id	value	diff	baseline
`data`	1.3MB	+949.0B	1.3MB

History

💚 Build #77955 succeeded b75a84b481d038dfaf47a68b0139d353841ed7c7
💔 Build #77477 failed 5fca7111f6a6f165dafcac5a629f3388f0fb326a
💔 Build #77236 failed 7a0b5c0076aef2901c04ba8c105e5e92fe44fc0a
💚 Build #77000 succeeded f48dda8446ee51869bbd4e602e3fc500241eb19f
💚 Build #76807 succeeded 52036713a98533f5448b1c458da72e41198650fa

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

# Conflicts: # src/plugins/data/server/server.api.md

lizozom · 2020-09-30T09:19:06Z

src/plugins/data/common/search/search_source/search_source.ts


 export interface SearchSourceDependencies extends FetchHandlers {
-  search: ISearchGeneric;
+  search: (request: IEsSearchRequest, options: ISearchOptions) => Promise<IEsSearchResponse>;


@lukeelmers why did you do this change here?
ISearchGeneric is the exact equivalent of this more explicit typescript.

Was the intention here to limit the usage of SearchSource to DSL only?

It's not quite the exact equivalent -- ISearchGeneric returns an observable, however on the server, the low-level search returns a promise.

In order to ensure the same behavior of SearchSource on both client & server, I had to wrap the dependency into something that always returns a promise.

That said, I did not include the ability to provide generic type params like ISearchGeneric does, and probably should have. I will open a PR to update this.

(Also per our offline discussion, if low-level search on the server eventually moves to returning an observable as it does on the client, we can come back and adjust this)

PR adding the generic type params: #79608

lukeelmers self-assigned this Sep 24, 2020

lukeelmers added Feature:Search Querying infrastructure in Kibana release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. release_note:enhancement v7.10.0 v8.0.0 review labels Sep 24, 2020

lukeelmers commented Sep 24, 2020

View reviewed changes

lukeelmers requested review from lizozom, lukasolson and ppisljar September 24, 2020 04:42

lukeelmers added the Team:AppArch label Sep 24, 2020

lukeelmers marked this pull request as ready for review September 24, 2020 04:43

lukeelmers requested a review from a team as a code owner September 24, 2020 04:43

lukeelmers mentioned this pull request Sep 24, 2020

[data.search.searchSource]: Expose SearchSource on server #65069

Closed

28 tasks

ppisljar approved these changes Sep 24, 2020

View reviewed changes

lukeelmers force-pushed the feat/ssource-server branch from 5203671 to f48dda8 Compare September 24, 2020 16:15

lukeelmers mentioned this pull request Sep 24, 2020

[Discuss] Removing RequestHandlerContext from the low-level data.search API. #78461

Closed

lukeelmers force-pushed the feat/ssource-server branch 2 times, most recently from 5fca711 to b75a84b Compare September 29, 2020 16:23

lukeelmers added 6 commits September 29, 2020 10:58

Add common SearchSourceService.

9f1f162

Move logic for msearch route into separate helper.

2f33854

Add unit tests for injectReferences.

a797855

Expose SearchSource from server.

461bc0b

Add plugin functional tests.

f252234

Update generated docs.

5aebda3

lukeelmers force-pushed the feat/ssource-server branch from b75a84b to 5aebda3 Compare September 29, 2020 17:00

lukeelmers merged commit ccb789d into elastic:master Sep 29, 2020

lukeelmers deleted the feat/ssource-server branch September 29, 2020 18:38

lukeelmers mentioned this pull request Sep 29, 2020

[7.x] [data.search] Expose SearchSource on the server. (#78383) #78891

Merged

lukeelmers added a commit to lukeelmers/kibana that referenced this pull request Sep 29, 2020

[data.search] Expose SearchSource on the server. (elastic#78383)

10a9b6b

# Conflicts: # src/plugins/data/server/server.api.md

lukeelmers added a commit that referenced this pull request Sep 30, 2020

[data.search] Expose SearchSource on the server. (#78383) (#78891)

a95cdeb

# Conflicts: # src/plugins/data/server/server.api.md

lizozom reviewed Sep 30, 2020

View reviewed changes

lukeelmers mentioned this pull request Oct 6, 2020

[data.search.searchSource] Add generic type params to search dependency. #79608

Merged

Conversation

lukeelmers commented Sep 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Dev Docs

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lukeelmers Sep 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Sep 24, 2020

Uh oh!

ppisljar left a comment

Choose a reason for hiding this comment

Uh oh!

lukeelmers commented Sep 26, 2020

Uh oh!

lukeelmers commented Sep 26, 2020

Uh oh!

elasticmachine commented Sep 26, 2020

Uh oh!

kibanamachine commented Sep 29, 2020

💚 Build Succeeded

Metrics [docs]

@kbn/optimizer bundle module count

distributable file count

page load bundle size

History

Uh oh!

lizozom Sep 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

lukeelmers commented Sep 24, 2020 •

edited

Loading

lukeelmers Sep 24, 2020 •

edited

Loading

lizozom Sep 30, 2020 •

edited

Loading