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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import com.auth0.android.Auth0
import com.auth0.android.Auth0Exception
import com.auth0.android.authentication.ParameterBuilder
import com.auth0.android.request.*
import com.auth0.android.request.internal.BaseRequest
import com.auth0.android.request.internal.GsonAdapter
import com.auth0.android.request.internal.GsonAdapter.Companion.forListOf
import com.auth0.android.request.internal.GsonAdapter.Companion.forMap
Expand Down Expand Up @@ -186,8 +187,12 @@ public class UsersAPIClient @VisibleForTesting(otherwise = VisibleForTesting.PRI
val userProfileAdapter: JsonAdapter<UserProfile> = GsonAdapter(
UserProfile::class.java, gson
)
return factory.patch(url.toString(), userProfileAdapter)
.addParameter(USER_METADATA_KEY, gson.toJson(userMetadata))
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.

patch.addParameter(USER_METADATA_KEY, userMetadata)
return patch
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.

.map { urlBuilder.addQueryParameter(it.key, it.value as String) }
requestBuilder.method(options.method.toString(), null)
}
else -> {
Expand All @@ -66,6 +67,7 @@ public class DefaultClient() : NetworkingClient {
val builder = OkHttpClient.Builder()

// logging
//TODO: OFF by default!
if (enableLogging) {
val logger: Interceptor = HttpLoggingInterceptor()
.setLevel(HttpLoggingInterceptor.Level.BODY)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ package com.auth0.android.request
* Holder for the information required to configure a request
*/
public class RequestOptions(public val method: HttpMethod) {
public val parameters: MutableMap<String, String> = mutableMapOf()
public val parameters: MutableMap<String, Any> = mutableMapOf()
public val headers: MutableMap<String, String> = mutableMapOf()
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ public open class BaseRequest<T, U : Auth0Exception> internal constructor(
return this
}

internal fun addParameter(name: String, value: Any): Request<T, U> {
options.parameters[name] = value
return this
}

/**
* Runs asynchronously and executes the network request, without blocking the current thread.
* The result is parsed into a <T> value and posted in the callback's onSuccess method or a <U>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@
import org.hamcrest.collection.IsMapWithSize;
import org.junit.After;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
Expand Down Expand Up @@ -288,7 +287,6 @@ public void shouldUnlinkAccountSync() throws Exception {
}

@Test
@Ignore("PATCH method not supported by HttpUrlConnection")
public void shouldUpdateUserMetadata() throws Exception {
mockAPI.willReturnUserProfile();

Expand Down Expand Up @@ -316,7 +314,6 @@ public void shouldUpdateUserMetadata() throws Exception {
}

@Test
@Ignore("PATCH method not supported by HttpUrlConnection")
public void shouldUpdateUserMetadataSync() throws Exception {
mockAPI.willReturnUserProfile();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1078,15 +1078,15 @@ public class WebAuthProviderTest {
)
)
)
MatcherAssert.assertThat<Map<String, String>>(
MatcherAssert.assertThat<Map<String, Any>>(
codeOptionsCaptor.firstValue.parameters,
IsMapContaining.hasEntry("code", "1234")
)
MatcherAssert.assertThat<Map<String, String>>(
MatcherAssert.assertThat<Map<String, Any>>(
codeOptionsCaptor.firstValue.parameters,
IsMapContaining.hasEntry("grant_type", "authorization_code")
)
MatcherAssert.assertThat<Map<String, String>>(
MatcherAssert.assertThat<Map<String, Any>>(
codeOptionsCaptor.firstValue.parameters,
IsMapContaining.hasKey("code_verifier")
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public void shouldSetGrantType() throws Exception {
.execute();

verify(client).load(eq(BASE_URL), optionsCaptor.capture());
Map<String, String> values = optionsCaptor.getValue().getParameters();
Map<String, Object> values = optionsCaptor.getValue().getParameters();
assertThat(values, aMapWithSize(1));
assertThat(values, hasEntry("grant_type", "grantType"));
}
Expand All @@ -88,7 +88,7 @@ public void shouldSetConnection() throws Exception {
.execute();

verify(client).load(eq(BASE_URL), optionsCaptor.capture());
Map<String, String> values = optionsCaptor.getValue().getParameters();
Map<String, Object> values = optionsCaptor.getValue().getParameters();
assertThat(values, aMapWithSize(1));
assertThat(values, hasEntry("connection", "my-connection"));
}
Expand All @@ -102,7 +102,7 @@ public void shouldSetRealm() throws Exception {
.execute();

verify(client).load(eq(BASE_URL), optionsCaptor.capture());
Map<String, String> values = optionsCaptor.getValue().getParameters();
Map<String, Object> values = optionsCaptor.getValue().getParameters();
assertThat(values, aMapWithSize(1));
assertThat(values, hasEntry("realm", "my-realm"));
}
Expand All @@ -116,7 +116,7 @@ public void shouldSetScope() throws Exception {
.execute();

verify(client).load(eq(BASE_URL), optionsCaptor.capture());
Map<String, String> values = optionsCaptor.getValue().getParameters();
Map<String, Object> values = optionsCaptor.getValue().getParameters();
assertThat(values, aMapWithSize(1));
assertThat(values, hasEntry("scope", "email profile"));
}
Expand All @@ -130,7 +130,7 @@ public void shouldSetAudience() throws Exception {
.execute();

verify(client).load(eq(BASE_URL), optionsCaptor.capture());
Map<String, String> values = optionsCaptor.getValue().getParameters();
Map<String, Object> values = optionsCaptor.getValue().getParameters();
assertThat(values, aMapWithSize(1));
assertThat(values, hasEntry("audience", "my-api"));
}
Expand All @@ -148,7 +148,7 @@ public void shouldAddAuthenticationParameters() throws Exception {
.execute();

verify(client).load(eq(BASE_URL), optionsCaptor.capture());
Map<String, String> values = optionsCaptor.getValue().getParameters();
Map<String, Object> values = optionsCaptor.getValue().getParameters();
assertThat(values, aMapWithSize(2));
assertThat(values, hasEntry("extra", "value"));
assertThat(values, hasEntry("123", "890"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ public void shouldAddParameter() throws Exception {
baseRequest.execute();

verify(client).load(eq(BASE_URL), optionsCaptor.capture());
Map<String, String> values = optionsCaptor.getValue().getParameters();
Map<String, Object> values = optionsCaptor.getValue().getParameters();
assertThat(values, aMapWithSize(1));
assertThat(values, hasEntry("A", "1"));
}
Expand All @@ -148,7 +148,7 @@ public void shouldAddParameters() throws Exception {
baseRequest.execute();

verify(client).load(eq(BASE_URL), optionsCaptor.capture());
Map<String, String> values = optionsCaptor.getValue().getParameters();
Map<String, Object> values = optionsCaptor.getValue().getParameters();
assertThat(values, aMapWithSize(2));
assertThat(values, hasEntry("A", "1"));
assertThat(values, hasEntry("B", "2"));
Expand Down