Initial Search Application Search API with templates #94869

carlosdelest · 2023-03-29T13:03:53Z

Adds a search endpoint to Search Applications:

GET /_application/search_application/<name>/_search

{ "params": { "param1": "value1", "param2": "value2" } }

where params contains overrides for the Search Application defined template, for example:

PUT /_application/search_application/<name>

{ "indices": ["test1"], "template": { "script": { "source": { "query": { "term": { "{{param1}}": "{{param2}}" } } }, "params": { "param1": "hello", "param2": "world" } } } }

For the above Search Application and search request, the template would render like:

 "query": { "term": { "value1": "value2" } }

... as the search specified parameters would override the template ones.

Based on this work by Jim and Ioana 💯

…ate as a separate field

…emplate parameters

… not specified

carlosdelest · 2023-03-29T14:02:46Z

x-pack/plugin/ent-search/qa/rest/roles.yml

+ ]
 privileges: [ "manage" ]
+ - names: [
+ # indices used for indexing and searching


Some indices for the search use case, with read and write permissions

carlosdelest · 2023-03-29T14:21:30Z

...n/ent-search/src/main/java/org/elasticsearch/xpack/application/search/SearchApplication.java

+ * @param searchApplicationTemplate The search application template to be used on search
 */
- public SearchApplication(String name, String[] indices, @Nullable String analyticsCollectionName, long updatedAtMillis) {
+ public SearchApplication(


A little messy rebase on the ent-search-module branch 😱 . Added the Search Template to the application.

carlosdelest · 2023-03-29T14:26:00Z

...h/src/main/java/org/elasticsearch/xpack/application/search/SearchApplicationQueryParams.java

+
+import static org.elasticsearch.xcontent.ConstructingObjectParser.optionalConstructorArg;
+
+public class SearchApplicationQueryParams implements ToXContentObject, Writeable {


Created a new class for the query parameters. Just now it's simple enough, but we could potentially extend it with other options / params in the future, so I decided to create a separate class for handling this

Can it be implemented in the Request directly? The other params you're talking about are would be for the request so I guess we can add everything there?

I think you're right. Changed it to be included in the Request.

carlosdelest · 2023-03-29T14:27:26Z

...java/org/elasticsearch/xpack/application/search/action/RestQuerySearchApplicationAction.java

+ @Override
+ protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException {
+ SearchApplicationQueryParams queryParams = new SearchApplicationQueryParams();
+ if (restRequest.hasContent()) {


This leaves open the possibility of not including a body at all, and just issue the request with no body.

sethmlarson · 2023-03-30T12:31:45Z

rest-api-spec/src/main/resources/rest-api-spec/api/search_application.query.json

@@ -0,0 +1,40 @@
+{
+ "search_application.query": {


Should the name of this API be search_application.search to match the path?

Good catch!

Right now, it matches the action names (RestQuerySearchApplicationAction, QuerySearchApplicationAction, TransportQuerySearchApplicationAction). I guess Query is a better name as SearchSearchApplicationAction would be very confusing.

I'm in favour of refactoring these names and drop the SearchApplication from them, but that's something that can wait for later.

Changed to search_application.search, thanks for the discussion @sethmlarson !

jimczi

Looks great @carlosdelest! I left some minor comments.

jimczi · 2023-03-31T07:26:02Z

...h/src/main/java/org/elasticsearch/xpack/application/search/SearchApplicationQueryParams.java

+
+import static org.elasticsearch.xcontent.ConstructingObjectParser.optionalConstructorArg;
+
+public class SearchApplicationQueryParams implements ToXContentObject, Writeable {


Can it be implemented in the Request directly? The other params you're talking about are would be for the request so I guess we can add everything there?

...h/src/main/java/org/elasticsearch/xpack/application/search/SearchApplicationQueryParams.java

...arch/src/main/java/org/elasticsearch/xpack/application/search/SearchApplicationTemplate.java

…uest

…purposes

kderusso

Looks great! One change that needs to happen in QuerySearchApplicationAction, everything else is non-blocking nitpicky comments. Thank you!

rest-api-spec/src/main/resources/rest-api-spec/api/search_application.search.json

kderusso · 2023-04-03T15:53:38Z

...a/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/20_search_application_put.yml

Nice additions to this yaml test. Do we want a specific test for success when no template is specified? (Maybe it's not strictly necessary since it's implicitly tested in the GET yaml tests?)

kderusso · 2023-04-03T15:56:48Z

...est/src/yamlRestTest/resources/rest-api-spec/test/entsearch/55_search_application_search.yml

+ name: test-search-application
+
+ - match: { hits.total.value: 2 }
+ - match: { hits.hits.0._id: "doc1" }


Is there any risk of test stability here, if the two correct results are returned in a different order than we expect? Do we want to consider maybe verifying that the two docIds were returned, not the order?

kderusso · 2023-04-03T15:58:16Z

...est/src/yamlRestTest/resources/rest-api-spec/test/entsearch/55_search_application_search.yml

Nice tests! Do we want to consider adding a test case for inputting an invalid template value (such as in your demo)? Or is that taken care of sufficiently elsewhere?

.../main/java/org/elasticsearch/xpack/application/search/action/GetSearchApplicationAction.java

...ain/java/org/elasticsearch/xpack/application/search/action/QuerySearchApplicationAction.java

kderusso · 2023-04-03T17:13:10Z

...org/elasticsearch/xpack/application/search/action/TransportQuerySearchApplicationAction.java

+
+public class TransportQuerySearchApplicationAction extends SearchApplicationTransportAction<
+ QuerySearchApplicationAction.Request,
+ SearchResponse> {


I think it's OK to return a SearchResponse in this PR, but some day in the future we may want to tack on additional data to a custom response that includes a search response.

...org/elasticsearch/xpack/application/search/action/TransportQuerySearchApplicationAction.java

kderusso · 2023-04-03T17:15:40Z

...rch/src/test/java/org/elasticsearch/xpack/application/search/SearchApplicationTestUtils.java

 randomFrom(new String[] { null, randomAlphaOfLengthBetween(1, 10) }),
- randomLongBetween(0, Long.MAX_VALUE)
+ randomLongBetween(0, Long.MAX_VALUE),
+ randomBoolean() ? getRandomSearchApplicationTemplate() : null


ioanatia and others added 17 commits March 29, 2023 13:39

Add new template field to SearchApplication

1b6dbab

Script should not be nullable

a619ec5

Add tests

472f690

Rename to SearchApplicationTemplate

21b47c1

Use mustache templates

b92c41d

No need to modify the system index mapping and store the search templ…

487338c

…ate as a separate field

Add rest api tests

ae40a27

More renames from SearchTemplate to SearchApplicationTemplates

7ee5797

Initial API

35b04cd

simple template execution

a57c08e

Use SearchApplicationQueryParams to include query parameters, merge t…

8743711

…emplate parameters

Make the query work with GET and POST, and assume default params when…

cd334f4

… not specified

Add serialization tests

7da0724

Add YAML tests

14989fa

Refactor TransportAction for legibility

21c67d4

Additional tests for Search Application Query

20f274f

Fix rebasing

ebb2a31

elasticsearchmachine added the external-contributor Pull request authored by a developer outside the Elasticsearch team label Mar 29, 2023

carlosdelest changed the title ~~Templates api search application~~ Initial Search Application Search API with templates Mar 29, 2023

carlosdelest commented Mar 29, 2023

View reviewed changes

carlosdelest marked this pull request as ready for review March 29, 2023 15:06

carlosdelest requested review from afoucret, ioanatia, jimczi and kderusso March 29, 2023 15:06

elasticsearchmachine added the needs:triage Requires assignment of a team area label label Mar 29, 2023

carlosdelest added 2 commits March 29, 2023 17:19

Added javadoc, minor formatting changes

901dd89

Serves me right for documenting

9e0c865

stu-elastic removed the needs:triage Requires assignment of a team area label label Mar 29, 2023

elasticsearchmachine added the needs:triage Requires assignment of a team area label label Mar 29, 2023

carlosdelest added Team:Enterprise Search Meta label for Enterprise Search team >feature v8.8.0 and removed needs:triage Requires assignment of a team area label labels Mar 30, 2023

sethmlarson reviewed Mar 30, 2023

View reviewed changes

carlosdelest mentioned this pull request Mar 30, 2023

Add template parameters schema to Search Applications carlosdelest/elasticsearch#2

Closed

jimczi approved these changes Mar 31, 2023

View reviewed changes

carlosdelest added 5 commits March 31, 2023 11:10

Remove SearchApplicationQueryParams and included them directly in Req…

d5b6e07

…uest

Fix imports

c0ff95c

Change search_application.query to search_application.search for API …

a52909e

…purposes

Add mustache template validation

38b93d2

Add mustache template validation

2f051cf

kderusso approved these changes Apr 3, 2023

View reviewed changes

jimczi deleted the branch elastic:ent-search-module April 4, 2023 09:36

jimczi closed this Apr 4, 2023

kderusso mentioned this pull request Apr 4, 2023

Initial Search Application Search API with templates #95026

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Initial Search Application Search API with templates #94869

Initial Search Application Search API with templates #94869

Uh oh!

carlosdelest commented Mar 29, 2023 •

edited

Loading

carlosdelest Mar 29, 2023

carlosdelest Mar 29, 2023

carlosdelest Mar 29, 2023

jimczi Mar 31, 2023

carlosdelest Mar 31, 2023

carlosdelest Mar 29, 2023

sethmlarson Mar 30, 2023

carlosdelest Mar 30, 2023

carlosdelest Mar 31, 2023

jimczi left a comment

jimczi Mar 31, 2023

Uh oh!

Uh oh!

Uh oh!

kderusso left a comment

Uh oh!

kderusso Apr 3, 2023

kderusso Apr 3, 2023

kderusso Apr 3, 2023

Uh oh!

Uh oh!

Uh oh!

kderusso Apr 3, 2023

Uh oh!

kderusso Apr 3, 2023

Labels

7 participants


		import static org.elasticsearch.xcontent.ConstructingObjectParser.optionalConstructorArg;

		public class SearchApplicationQueryParams implements ToXContentObject, Writeable {

Initial Search Application Search API with templates #94869

Initial Search Application Search API with templates #94869

Uh oh!

Conversation

carlosdelest commented Mar 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jimczi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kderusso left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Labels

7 participants

carlosdelest commented Mar 29, 2023 •

edited

Loading