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

URLQuery with index slice keys #74

Merged
merged 7 commits into from
Jul 2, 2018
Merged

URLQuery with index slice keys #74

merged 7 commits into from
Jul 2, 2018

Conversation

geseq
Copy link
Collaborator

@geseq geseq commented May 28, 2018

Certain APIs (I believe those written in PHP) seem to be unable to parse correctly URLQuery() result containing slices generated by URLValuesSliceKeySuffix="" or URLValuesSliceKeySuffix="[]".

The solution is to pass the slices with an index so that they are read correctly.

For instance, in order to parse slices of the form correctly:
objx.Map{"mapSlice": []objx.Map{objx.Map{"age": 40, "sex": "male"}, objx.Map{"height": 152}}}
the query needs to be:
mapSlice[0][age]=40&mapSlice[0][sex]=male&mapSlice[1][height]=152

Currently these two forms are possible which don't work:
mapSlice[age]=40&mapSlice[sex]=male&mapSlice[height]=152
mapSlice[][age]=40&mapSlice[][sex]=male&mapSlice[][height]=152

I've modified the code to check for URLValuesSliceKeySuffix="[i]" and use slice indexes when present

@geseq
Copy link
Collaborator Author

geseq commented May 28, 2018

Added tests wrt #71 but I will write additional tests separately.

@hanzei hanzei self-requested a review May 28, 2018 12:28
@hanzei
Copy link
Collaborator

hanzei commented May 31, 2018

@earncef Could give this a rebase?

@geseq
Copy link
Collaborator Author

geseq commented May 31, 2018

Done

Copy link
Collaborator

@hanzei hanzei left a comment

Choose a reason for hiding this comment

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

Nice work! I really like the suffix [i] option. But I dont think exposing this option via a variable is correct. A function seems to be the a better way. I would go with func SetURLValuesSliceKeySuffix(s string) error

conversions.go Outdated
@@ -131,25 +148,42 @@ func (m Map) parseURLValues(queryMap Map, vals url.Values, key string) {
m.parseURLValues(New(v), vals, key+"["+k+"]")
}
case val.IsMSISlice():
Copy link
Collaborator

Choose a reason for hiding this comment

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

This case isn't covered by any testcases. Mind adding some? I think you can reuse the val.IsObjxMapSlice() ones

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Due to a previous pull request, this part is no longer necessary. Updated the code and added tests.

@hanzei hanzei added this to the v0.2 milestone Jun 6, 2018
@geseq
Copy link
Collaborator Author

geseq commented Jun 7, 2018

I agree. What would you consider the case for error though? Not being one of "" or "[]" or "[I]"?

@geseq
Copy link
Collaborator Author

geseq commented Jun 25, 2018

@hanzei any update on the last commit?

@hanzei
Copy link
Collaborator

hanzei commented Jun 25, 2018

I agree. What would you consider the case for error though? Not being one of "" or "[]" or "[I]"?

Correct. I would limit SetURLValuesSliceKeySuffix to these three values and throw an error for every other.

@geseq
Copy link
Collaborator Author

geseq commented Jul 2, 2018

Done

@hanzei hanzei merged commit b8b73a3 into stretchr:master Jul 2, 2018
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

2 participants