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 request to patch user metadata #429

Merged
merged 1 commit into from
Jan 13, 2021
Merged

Fix request to patch user metadata #429

merged 1 commit into from
Jan 13, 2021

Conversation

lbalmaceda
Copy link
Contributor

Changes

The serialization of the user_metadata parameter was broken, as the internal classes required a String value to be passed, and the server expected an object / dictionary.

The solution proposed here involves an internal method (not exposed) and a cast to a type we know it's the right expected one, as the RequestFactory implementation that provides the request instance is ours as well.

The RequestOptions had to be changed to support being passed parameters of the Any (Object) type. This is this way since if we provide a hidden internal implementation that handles this one scenario and let the interface as it was, we could be leaving out those developers that decide to extend the NetworkingClient and implement their own PATCH requests; as they would still be receiving a String value.

@lbalmaceda lbalmaceda added CH: Fixed medium Medium review labels Jan 12, 2021
@lbalmaceda lbalmaceda added this to the v2-Next milestone Jan 12, 2021
@lbalmaceda lbalmaceda requested a review from a team as a code owner January 12, 2021 17:57
val patch = factory.patch(
url.toString(),
userProfileAdapter
) as BaseRequest<UserProfile, ManagementException>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

"unsafe cast": would throw if the cast cannot be made. But we know the type our RequestFactor uses internally.

@@ -40,7 +40,8 @@ public class DefaultClient() : NetworkingClient {
when (options.method) {
is HttpMethod.GET -> {
// add parameters as query
options.parameters.map { urlBuilder.addQueryParameter(it.key, it.value) }
options.parameters.filterValues { it is String }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

when building the query, OKHttp requires us to pass String? type for the values. I'm filtering those that are not a String. Since the Request interface requires String, String this should not be an issue for Requests built by our API clients.

@lbalmaceda lbalmaceda merged commit 6080c95 into v2-dev Jan 13, 2021
@lbalmaceda lbalmaceda deleted the fix-patch-metadata branch January 13, 2021 08:56
@lbalmaceda lbalmaceda modified the milestones: v2-Next, 2.0.0-beta.0 Jan 19, 2021
@gier3k gier3k mentioned this pull request Oct 26, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CH: Fixed medium Medium review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants