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

Media-type parser #61987

Merged
merged 32 commits into from
Sep 17, 2020
Merged
Changes from 1 commit
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
5e42390
Split responsibility for format parsing
pgomulka Sep 2, 2020
01f001d
parse * and ndjson
pgomulka Sep 2, 2020
50a88a0
make format not accepting applicaiton/
pgomulka Sep 2, 2020
fb03ffd
post data request should parse applicaiton/json style
pgomulka Sep 2, 2020
09f281e
unused import
pgomulka Sep 2, 2020
8bc024f
fix sql parsing
pgomulka Sep 2, 2020
070508c
split format and accept header
pgomulka Sep 3, 2020
3c7ab16
fix and todos
pgomulka Sep 3, 2020
59a7f42
Merge branch 'master' into xcontent_format_parsing
pgomulka Sep 3, 2020
968b1c9
media type parser
pgomulka Sep 4, 2020
46f8f33
media type parser
pgomulka Sep 4, 2020
cbbe093
precommit
pgomulka Sep 4, 2020
222caee
rename and null check
pgomulka Sep 7, 2020
6bdec13
Merge branch 'master' into header_version_split
pgomulka Sep 7, 2020
ee97564
Merge branch 'master' into header_version_split
pgomulka Sep 7, 2020
7f52e11
fix text format parsing
pgomulka Sep 7, 2020
63db70c
Merge branch 'master' into header_version_split
pgomulka Sep 8, 2020
57ddb40
Apply suggestions from code review
pgomulka Sep 9, 2020
b5f1eff
code review follow up
pgomulka Sep 9, 2020
90e798d
Merge branch 'header_version_split' of github.com:pgomulka/elasticsea…
pgomulka Sep 9, 2020
fa49be4
javadoc and validation
pgomulka Sep 14, 2020
31d92ac
Update x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/pl…
pgomulka Sep 14, 2020
a925fbe
Merge branch 'header_version_split' of github.com:pgomulka/elasticsea…
pgomulka Sep 14, 2020
23c4e41
javadoc fix
pgomulka Sep 14, 2020
b1e3fb1
remove shortName
pgomulka Sep 14, 2020
c17a895
javadoc fix
pgomulka Sep 14, 2020
7d6bd08
fix compile error
pgomulka Sep 14, 2020
3c93954
fix test compile
pgomulka Sep 14, 2020
77068a8
Merge branch 'master' into header_version_split
pgomulka Sep 15, 2020
8fb0cd4
Merge branch 'master' into header_version_split
pgomulka Sep 15, 2020
63fb6c7
remove todo and a testcase
pgomulka Sep 16, 2020
4598a0a
Apply suggestions from code review
pgomulka Sep 17, 2020
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
Prev Previous commit
Next Next commit
fix and todos
  • Loading branch information
pgomulka committed Sep 3, 2020

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
commit 3c7ab16fc839d7ade510e8025ad39f106d12752f
Original file line number Diff line number Diff line change
@@ -98,6 +98,7 @@ public XContentBuilder newBuilder(@Nullable XContentType requestContentType, boo
public XContentBuilder newBuilder(@Nullable XContentType requestContentType, @Nullable XContentType responseContentType,
boolean useFiltering) throws IOException {
if (responseContentType == null) {
//TODO PG shoudld format vs acceptHeader be always the same, do we allow overriding?
Copy link
Member

Choose a reason for hiding this comment

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

Maybe open an issue for this. My guess is that there is some precedence already and it might be that format overrides the header

Copy link
Contributor Author

Choose a reason for hiding this comment

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

raised #62294 to discuss this

responseContentType = XContentType.fromFormat(format);
if(responseContentType == null) {
pgomulka marked this conversation as resolved.
Show resolved Hide resolved
responseContentType = XContentType.fromMediaType(acceptHeader);
Original file line number Diff line number Diff line change
@@ -299,7 +299,7 @@ public void testErrorToAndFromXContent() throws IOException {

final XContentType xContentType = randomFrom(XContentType.values());

Map<String, String> params = Collections.singletonMap("format", xContentType.mediaType());
Map<String, String> params = Collections.singletonMap("format", xContentType.shortName());
RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withParams(params).build();
RestChannel channel = detailed ? new DetailedExceptionRestChannel(request) : new SimpleExceptionRestChannel(request);

Original file line number Diff line number Diff line change
@@ -97,6 +97,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
* that doesn't parse it'll throw an {@link IllegalArgumentException}
* which we turn into a 400 error.
*/
//TODO PG this all logic needs a review from SQL team
Copy link
Contributor

Choose a reason for hiding this comment

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

remove reminder.

XContentType xContentType = getXContentType(accept, acceptHeader, contentTypeHeader);
textFormat = xContentType == null ? TextFormat.fromMediaTypeOrFormat(accept) : null;

Original file line number Diff line number Diff line change
@@ -82,6 +82,7 @@ private XContentType detectContentType(String content) {
//There must be a __<content_type__:: prefix so the minimum length before detecting '__::' is 3
int endOfContentName = content.indexOf("__::", 3);
if (endOfContentName != -1) {
//TODO PG what do we expect here?
Copy link
Contributor

Choose a reason for hiding this comment

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

You can remove this todo, answered in another comment.

return XContentType.fromFormat(content.substring(2, endOfContentName));
}
}
Original file line number Diff line number Diff line change
@@ -327,7 +327,7 @@ public void testSlmPolicyAndStats() throws IOException {

if (isRunningAgainstOldCluster() == false) {
Response response = client().performRequest(new Request("GET", "_slm/stats"));
XContentType xContentType = XContentType.fromFormat(response.getEntity().getContentType().getValue());
XContentType xContentType = XContentType.fromMediaType(response.getEntity().getContentType().getValue());
try (XContentParser parser = xContentType.xContent().createParser(NamedXContentRegistry.EMPTY,
DeprecationHandler.THROW_UNSUPPORTED_OPERATION, response.getEntity().getContent())) {
assertEquals(new SnapshotLifecycleStats(), SnapshotLifecycleStats.parse(parser));
Original file line number Diff line number Diff line change
@@ -328,7 +328,7 @@ private TransformStats getTransformStats(String id) throws IOException {
final Request getStats = new Request("GET", getTransformEndpoint() + id + "/_stats");
Response response = client().performRequest(getStats);
assertEquals(200, response.getStatusLine().getStatusCode());
XContentType xContentType = XContentType.fromFormat(response.getEntity().getContentType().getValue());
XContentType xContentType = XContentType.fromMediaType(response.getEntity().getContentType().getValue());
try (XContentParser parser = xContentType.xContent().createParser(
NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION,
response.getEntity().getContent())) {