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

Fix parsing of array query properties in JSON payload #8697

Merged
merged 3 commits into from Mar 7, 2019

Conversation

bratseth
Copy link
Member

@bratseth bratseth commented Mar 7, 2019

These were translated to request properties by calling
asString on the payload, which returns an empty value for arrays.
toString returns the correct value.

This also improves error messages for Slime JSON parsing
of queries, and in general.

@arnej27959 og @henrhoi please review

These were translated to request properties by calling
asString on the payload, which returns an empty value for arrays.
toString returns the correct value.

This also improves error messages for Slime JSON parsing
of queries, and in general.
Copy link
Contributor

@henrhoi henrhoi left a comment

Choose a reason for hiding this comment

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

Looks good to me! 😄

@@ -577,14 +566,14 @@ public void createRequestMapping(Inspector inspector, Map<String, String> map, S
map.put(qualifiedKey , value.asString());
break;
case ARRAY:
map.put(qualifiedKey, value.asString());
map.put(qualifiedKey, value.toString()); // XXX: Causes parsing the JSON twice (Query.setPropertiesFromRequestMap)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 .
Weird that I did not notice this bug while testing... 😅
In any case, it is good that it gets fixed now.

Co-Authored-By: bratseth <bratseth@oath.com>
@bratseth bratseth requested a review from henrhoi March 7, 2019 14:13
@bratseth bratseth merged commit a62d29d into master Mar 7, 2019
@bratseth bratseth deleted the bratseth/fix-parsing-array-in-grouping-select-json branch March 7, 2019 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants