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

Unify query_string parameters parsing #11057

Merged

Conversation

javanna
Copy link
Member

@javanna javanna commented May 8, 2015

There currently are small differences between search api and count, exists, validate query, explain api when it comes to reading query_string parameters. analyze_wildcard, lowercase_expanded_terms and lenient are only read by the search api and ignored by all other mentioned apis. Unified code to fix this and make sure it doesn't happen again. Also shared some code when it comes to printing out the query as part of SearchSourceBuilder conversion to ToXContent.

@spinscale
Copy link
Contributor

LGTM in general, but we may need to adapt the rest spec and add tests for the new APIs supporting the parameters?

@javanna
Copy link
Member Author

javanna commented May 11, 2015

Thanks @spinscale I pushed a new commit. Updated REST spec (including parameters that were already support but not listed) and added REST tests. Tests were completely missing for count api and search_exists, so I added some basic generic tests too.

@spinscale
Copy link
Contributor

nice, ++ for more tests

LGTM

There currently are small differences between search api and count, exists, validate query, explain api when it comes to reading query_string parameters.  `analyze_wildcard`, `lowercase_expanded_terms` and `lenient` are only read by the search api and ignored by all other mentioned apis. Unified code to fix this and make sure it doesn't happen again. Also shared some code when it comes to printing out the query as part of SearchSourceBuilder conversion to ToXContent.

Extended REST spec to include all the supported params (some that were already supported weren't listed), and added REST tests (also some basic tests for count and search_exists which weren't tested at all).

Closes elastic#11057
@javanna javanna force-pushed the enhancement/unify_query_string_search branch from c74dc9a to d7e585c Compare May 11, 2015 09:36
javanna added a commit to javanna/elasticsearch that referenced this pull request May 11, 2015
There currently are small differences between search api and count, exists, validate query, explain api when it comes to reading query_string parameters.  `analyze_wildcard`, `lowercase_expanded_terms` and `lenient` are only read by the search api and ignored by all other mentioned apis. Unified code to fix this and make sure it doesn't happen again. Also shared some code when it comes to printing out the query as part of SearchSourceBuilder conversion to ToXContent.

Extended REST spec to include all the supported params (some that were already supported weren't listed), and added REST tests (also some basic tests for count and search_exists which weren't tested at all).

Closes elastic#11057
@javanna javanna merged commit d7e585c into elastic:master May 11, 2015
@kevinkluge kevinkluge removed the review label May 11, 2015
javanna added a commit that referenced this pull request May 11, 2015
javanna added a commit that referenced this pull request May 11, 2015
karmi added a commit to elastic/elasticsearch-ruby that referenced this pull request May 13, 2015
@clintongormley clintongormley changed the title REST: Unify query_string parameters parsing Unify query_string parameters parsing May 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants