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

Feature request: customise the bahaviour for parameter array #965

Closed
demetrio812 opened this issue Dec 17, 2015 · 5 comments
Closed

Feature request: customise the bahaviour for parameter array #965

demetrio812 opened this issue Dec 17, 2015 · 5 comments

Comments

@demetrio812
Copy link

@demetrio812 demetrio812 commented Dec 17, 2015

Hello,
currently if I use an array for a parameter Alamofire URL method encodes it with square bracket like:

&foo[]=bar&foo[]=bar2

I understand the meaning and it is also specified that:

Since there is no published specification for how to encode collection types, the convention of appending `[]` to the key for array values (`foo[]=1&foo[]=2`), and appending the key surrounded by square brackets for nested dictionary values (`foo[bar]=baz`)

However, even some modern servers that uses RestEasy with JAX-RS accept this format for arrays:

&foo=bar&foo=bar2

For this reason, I'd like to propose to have the ability to customise this behaviour with an option in order to support both cases.

It's a simple change that I can help doing and I think it will add flexibility to the library and will avoid the usage of the Custom method for this simple case.

What do you think?

Thanks,
Dem

@davdroman
Copy link

@davdroman davdroman commented Dec 17, 2015

Exactly the issue I'm dealing with currently. I'm pretty sure this would be pretty straightforward to implement.

Here's a custom bracket-less ParameterEncoding object for anyone interested in a workaround:

ParameterEncoding.Custom { requestConvertible, parameters in
    let (mutableRequest, error) = ParameterEncoding.URL.encode(requestConvertible, parameters: parameters)
    mutableRequest.URL = NSURL(string: mutableRequest.URLString.stringByReplacingOccurrencesOfString("%5B%5D=", withString: "="))
    return (mutableRequest, error)
}

Too much boilerplate code for a simple little option IMO.

An elegant solution I can think of is:

public enum ParameterEncoding {
    case URL(brackets: Bool)
    [...]
}

And then:

Alamofire.request(.GET, url, encoding: .URL(brackets: false))
[...]
@cnoon
Copy link
Member

@cnoon cnoon commented Dec 19, 2015

I'm going to go ahead and close this issue out. Please see my comments in #966. Cheers. 🍻

@demetrio812
Copy link
Author

@demetrio812 demetrio812 commented Dec 19, 2015

Needless to say I don't agree that it's not commonly used, maybe in your experience it's not but I'm using a commonly used modern java framework so it's not that unusual. Also, beside the pull proposal from DavidRoman, it can be implemented in a backward compatible way (default config is like it is now, and you can change if you want) and you will support both commonly used use cases.

Obviously you (the team) decide what you want to do, but saying "it's not commonly used" is not an acceptable answer in my opinion.

Same thing for multiple encoding methods in a single request (#374): you (the team) say it's not common but again it's not common for you, it's a very common pattern that happens regularly and it feels like Alamofire doesn't support basic patterns out of the box. You also felt that and fixed what originally was "uncommon" saying it was common for you: #742

So again, in my opinion the issue of this specific ticket is a common problem that should be addressed by Alamofire.

Cheers,
Dem

@cnoon
Copy link
Member

@cnoon cnoon commented Dec 20, 2015

Just to be clear, at no point did I claim that the PR was declined because "it's not commonly used". I declined it for other reasons which were very cleared documented. Additionally, I think you are assuming a bit too much here. At no point have we claimed we'll never support this feature. It's actually in our Trello backlog and has been for a while. I think I could have done a better job explaining that in my previous comment to avoid confusion...my bad there.

We completely understand there's a need for such a feature and want to support feature requests the best we possibly can. With that said, we have a very large community that we always need to consider when adding complexity into our APIs. This is definitely one of those features that we need to be careful how we introduce.

Please be patient because there are many features here we are actively supporting. If you feel that you would like to see this feature added sooner than we will be able to, then by all means submit a complete, well thought out pull request. We'll be more than happy to consider it. But please understand that it is certainly in the community's best interest for us to decline PRs once in a while that aren't up to our standards or designed in a way that we deem suitable.

@virusnik
Copy link

@virusnik virusnik commented Oct 13, 2016

Thank you for "New Parameter Encoding Protocol". It is the great benefit
Now how to use it? How to convert code from @davdroman answer?

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

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.