Skip to content
This repository was archived by the owner on Sep 15, 2025. It is now read-only.

Conversation

@crazytonyli
Copy link
Contributor

Description

There are two changes in this PR.

URL encoding in query and form

We used to have different implementation around encoding name-value pair into URL query and web form in POST request body: URLComponent's query encoding implementation is used in building URL query, and our own customized implementation (which aligns with Alamofire's implementation for backwards compatibility) is used to build web form.

Now, the same implementation, which is our customized one, is used to build both query and web form. Backwards compatibility is the only reason for this change.

HTTPRequestBuilder now does not modify the original URL

Say we have HTTPRequestBuilder instance created using an URL "https://wp.org/api?locale=en". When adding a query .query(name: "locale", value: "zh"), the new query replaces the one in the original url. This behaviour is changed in this PR. The added query will be appended onto the original URL, which means the original url content will always be in the final result. To be honest, I don't like this new behavior, but that's what Alamofire does and we'll need to do it for backwards compatibility.

Testing Details

See added unit tests.


  • Please check here if your pull request includes additional test coverage.
  • I have considered if this change warrants release notes and have added them to the appropriate section in the CHANGELOG.md if necessary.

@crazytonyli crazytonyli requested a review from mokagio January 17, 2024 20:57
@crazytonyli crazytonyli requested a review from jkmassel January 22, 2024 22:59
Copy link
Contributor

@jkmassel jkmassel left a comment

Choose a reason for hiding this comment

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

Little things, the rest we discussed on Zoom

static let nestedParameters: [String: Any] =
[
"number": 1,
"nsnumbe-true": NSNumber(value: true),
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: typo?

@@ -1,5 +1,9 @@
import Foundation

/// A builder type that adding HTTP request data onto an URL.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: typo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🙈 Fixed in 59bd717.

@crazytonyli crazytonyli enabled auto-merge January 23, 2024 22:53
@crazytonyli crazytonyli merged commit 5fba5e8 into trunk Jan 23, 2024
@crazytonyli crazytonyli deleted the http-request-builder-unify-query-and-form branch January 23, 2024 23:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants