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

Add support for multimaps in query strings #2139

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

betabandido
Copy link

@betabandido betabandido commented Nov 19, 2019

Currently, Gin supports parsing maps in query strings using GetQueryMap. The current implementation, however, is limited to simple maps where values are strings. A simple change to the code enables parsing more complex maps where values are string arrays. For instance, the following URL contains such a map:

/test?map[key1]=value11&map[key1]=value12&map[key2]=value2

We have use-cases where we need to parse such URLs. Given the change is relatively minor, and it is backwards compatible, it would be nice to have this feature added.

I chose multimap to refer to a map containing string arrays (see https://en.wikipedia.org/wiki/Multimap). But, there might be a better name.

I will document the new methods in the README if you think it is a feature you want to add to Gin.

I decided not to duplicate the get method to reduce code duplication. The tradeoff is a slight performance reduction. I did some performance evaluation and the results were:

Original GetQueryMap: 216 ns / call
Modified GetQueryMap: 419 ns / call

I believe this is a negligible increase in performance, but you should have a better idea on the latency breakdown across the whole lifespan of a process request. So, let me know if you think it is better not to pay the extra latency.

@codecov
Copy link

codecov bot commented Nov 19, 2019

Codecov Report

Merging #2139 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2139      +/-   ##
==========================================
+ Coverage   98.74%   98.75%   +<.01%     
==========================================
  Files          40       40              
  Lines        2236     2248      +12     
==========================================
+ Hits         2208     2220      +12     
  Misses         14       14              
  Partials       14       14
Impacted Files Coverage Δ
context.go 98.69% <100%> (+0.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db9174a...8ed04d7. Read the comment docs.

@appleboy appleboy added this to the 1.6 milestone Nov 24, 2019
@thinkerou thinkerou modified the milestones: 1.6, 1.7 Feb 28, 2020
@thinkerou thinkerou modified the milestones: 1.7, v1.8 Oct 27, 2020
@thinkerou thinkerou modified the milestones: v1.8, v1.9 May 28, 2022
@thinkerou thinkerou modified the milestones: v1.9, v1.10 Jan 17, 2023
@appleboy
Copy link
Member

Please fix the conflicts and move to 1.11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants