From 100d598587092402f4e19f46ec1b36e8f5d8c9c1 Mon Sep 17 00:00:00 2001 From: Minnie Liu Date: Thu, 18 Mar 2021 13:56:30 -0700 Subject: [PATCH 1/5] Addressing Common ApiView Comments --- .../common/CommunicationTokenCredential.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationTokenCredential.java b/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationTokenCredential.java index 27ee5aaaaa7b..5f05ed89d2ad 100644 --- a/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationTokenCredential.java +++ b/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationTokenCredential.java @@ -15,6 +15,7 @@ import java.util.Objects; import java.util.Timer; import java.util.TimerTask; +import java.util.function.Supplier; import com.azure.communication.common.implementation.TokenParser; @@ -28,7 +29,7 @@ public final class CommunicationTokenCredential implements AutoCloseable { private AccessToken accessToken; private final TokenParser tokenParser = new TokenParser(); - private TokenRefresher refresher; + private Supplier> refresher; private FetchingTask fetchingTask; private boolean isClosed = false; @@ -52,7 +53,7 @@ public CommunicationTokenCredential(String token) { * @param tokenRefreshOptions implementation to supply fresh token when reqested */ public CommunicationTokenCredential(CommunicationTokenRefreshOptions tokenRefreshOptions) { - TokenRefresher tokenRefresher = tokenRefreshOptions.getTokenRefresher(); + Supplier> tokenRefresher = tokenRefreshOptions.getTokenRefresher(); Objects.requireNonNull(tokenRefresher, "'tokenRefresher' cannot be null."); refresher = tokenRefresher; if (tokenRefreshOptions.getToken() != null) { @@ -114,10 +115,10 @@ private void setToken(String freshToken) { } private Mono fetchFreshToken() { - Mono tokenAsync = refresher.getToken(); + Mono tokenAsync = refresher.get(); if (tokenAsync == null) { return FluxUtil.monoError(logger, - new RuntimeException("TokenRefresher returned null when getTokenAsync is called")); + new RuntimeException("get() function for token refresher is null")); } return tokenAsync; } From 00a614692870ddb7c67d63244f1bd540f75e7813 Mon Sep 17 00:00:00 2001 From: Minnie Liu Date: Thu, 18 Mar 2021 13:58:13 -0700 Subject: [PATCH 2/5] Addressing Common ApiView Comments --- .../CommunicationTokenRefreshOptions.java | 11 +++++++---- .../communication/common/TokenRefresher.java | 17 ----------------- .../CommunicationTokenCredentialTests.java | 9 +++++---- 3 files changed, 12 insertions(+), 25 deletions(-) delete mode 100644 sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/TokenRefresher.java diff --git a/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationTokenRefreshOptions.java b/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationTokenRefreshOptions.java index 8f4f1095b074..8d9ff064d4f3 100644 --- a/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationTokenRefreshOptions.java +++ b/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationTokenRefreshOptions.java @@ -2,11 +2,14 @@ // Licensed under the MIT License. package com.azure.communication.common; +import java.util.function.Supplier; +import reactor.core.publisher.Mono; + /** * Options for refreshing CommunicationTokenCredential */ public final class CommunicationTokenRefreshOptions { - private final TokenRefresher tokenRefresher; + private final Supplier> tokenRefresher; private final boolean refreshProactively; private final String initialToken; @@ -19,7 +22,7 @@ public final class CommunicationTokenRefreshOptions { * with setCallbackOffsetMinutes or default value of * two minutes */ - public CommunicationTokenRefreshOptions(TokenRefresher tokenRefresher, boolean refreshProactively) { + public CommunicationTokenRefreshOptions(Supplier> tokenRefresher, boolean refreshProactively) { this.tokenRefresher = tokenRefresher; this.refreshProactively = refreshProactively; this.initialToken = null; @@ -35,7 +38,7 @@ public CommunicationTokenRefreshOptions(TokenRefresher tokenRefresher, boolean r * two minutes * @param initialToken the optional serialized JWT token */ - public CommunicationTokenRefreshOptions(TokenRefresher tokenRefresher, boolean refreshProactively, String initialToken) { + public CommunicationTokenRefreshOptions(Supplier> tokenRefresher, boolean refreshProactively, String initialToken) { this.tokenRefresher = tokenRefresher; this.refreshProactively = refreshProactively; this.initialToken = initialToken; @@ -44,7 +47,7 @@ public CommunicationTokenRefreshOptions(TokenRefresher tokenRefresher, boolean r /** * @return the token refresher to provide capacity to fetch fresh token */ - public TokenRefresher getTokenRefresher() { + public Supplier> getTokenRefresher() { return tokenRefresher; } diff --git a/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/TokenRefresher.java b/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/TokenRefresher.java deleted file mode 100644 index 4b55d527de5a..000000000000 --- a/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/TokenRefresher.java +++ /dev/null @@ -1,17 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. -package com.azure.communication.common; - -import reactor.core.publisher.Mono; - -/** - * Interface to provide capacity to fetch fresh token - */ -@FunctionalInterface -public interface TokenRefresher { - /** - * Asynchronous call to fetch a fresh token - * @return Wrapper for asynchronous call - */ - Mono getToken(); -} diff --git a/sdk/communication/azure-communication-common/src/test/java/com/azure/communication/common/CommunicationTokenCredentialTests.java b/sdk/communication/azure-communication-common/src/test/java/com/azure/communication/common/CommunicationTokenCredentialTests.java index d0fc9aa27de8..244a14da6b4e 100644 --- a/sdk/communication/azure-communication-common/src/test/java/com/azure/communication/common/CommunicationTokenCredentialTests.java +++ b/sdk/communication/azure-communication-common/src/test/java/com/azure/communication/common/CommunicationTokenCredentialTests.java @@ -19,6 +19,7 @@ import java.io.IOException; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; +import java.util.function.Supplier; public class CommunicationTokenCredentialTests { private final JwtTokenMocker tokenMocker = new JwtTokenMocker(); @@ -55,7 +56,7 @@ public void constructWithExpiredTokenWithoutRefresher() tokenCredential.close(); } - class MockImmediateRefresher implements TokenRefresher { + class MockImmediateRefresher implements Supplier> { private int numCalls = 0; private Runnable onCallReturn; @@ -72,7 +73,7 @@ public void resetCallCount() { } @Override - public Mono getToken() { + public Mono get() { numCalls++; if (this.onCallReturn != null) { this.onCallReturn.run(); @@ -237,7 +238,7 @@ public void shouldStopRefreshTimerWhenClosed() throws InterruptedException, Exec assertFalse(tokenCredential.hasProactiveFetcher()); } - class ExceptionRefresher implements TokenRefresher { + class ExceptionRefresher implements Supplier> { private int numCalls; private Runnable onCallReturn; @@ -254,7 +255,7 @@ public void resetCallCount() { } @Override - public Mono getToken() { + public Mono get() { numCalls++; if (this.onCallReturn != null) { this.onCallReturn.run(); From 14569e2003d27e3491cc31cf4e046700bff24267 Mon Sep 17 00:00:00 2001 From: Minnie Liu Date: Thu, 18 Mar 2021 15:10:56 -0700 Subject: [PATCH 3/5] Address comments --- .../communication/common/CommunicationTokenCredential.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationTokenCredential.java b/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationTokenCredential.java index 5f05ed89d2ad..bee6d6880797 100644 --- a/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationTokenCredential.java +++ b/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationTokenCredential.java @@ -118,7 +118,7 @@ private Mono fetchFreshToken() { Mono tokenAsync = refresher.get(); if (tokenAsync == null) { return FluxUtil.monoError(logger, - new RuntimeException("get() function for token refresher is null")); + new RuntimeException("get() function of the token refresher should not return null.")); } return tokenAsync; } From ace4dc4266076d8dc0e809a5b9c10ed494c6cf70 Mon Sep 17 00:00:00 2001 From: Minnie Liu Date: Thu, 18 Mar 2021 17:03:14 -0700 Subject: [PATCH 4/5] Address more comments --- .../communication/common/CommunicationTokenCredential.java | 4 ++-- .../common/CommunicationTokenRefreshOptions.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationTokenCredential.java b/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationTokenCredential.java index bee6d6880797..2c140c1a7b94 100644 --- a/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationTokenCredential.java +++ b/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationTokenCredential.java @@ -56,8 +56,8 @@ public CommunicationTokenCredential(CommunicationTokenRefreshOptions tokenRefres Supplier> tokenRefresher = tokenRefreshOptions.getTokenRefresher(); Objects.requireNonNull(tokenRefresher, "'tokenRefresher' cannot be null."); refresher = tokenRefresher; - if (tokenRefreshOptions.getToken() != null) { - setToken(tokenRefreshOptions.getToken()); + if (tokenRefreshOptions.getInitialToken() != null) { + setToken(tokenRefreshOptions.getInitialToken()); if (tokenRefreshOptions.isRefreshProactively()) { OffsetDateTime nextFetchTime = accessToken.getExpiresAt().minusMinutes(DEFAULT_EXPIRING_OFFSET_MINUTES); fetchingTask = new FetchingTask(this, nextFetchTime); diff --git a/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationTokenRefreshOptions.java b/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationTokenRefreshOptions.java index 8d9ff064d4f3..bdc0e7acda69 100644 --- a/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationTokenRefreshOptions.java +++ b/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationTokenRefreshOptions.java @@ -59,9 +59,9 @@ public boolean isRefreshProactively() { } /** - * @return the serialized JWT token + * @return the initial serialized JWT token */ - public String getToken() { + public String getInitialToken() { return initialToken; } } From 4fa692783a3af6a51d2701eb2c9983ee56569876 Mon Sep 17 00:00:00 2001 From: Minnie Liu Date: Fri, 19 Mar 2021 09:35:57 -0700 Subject: [PATCH 5/5] Updating javadoc as per comment --- .../communication/common/CommunicationTokenRefreshOptions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationTokenRefreshOptions.java b/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationTokenRefreshOptions.java index bdc0e7acda69..82d2020c2511 100644 --- a/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationTokenRefreshOptions.java +++ b/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationTokenRefreshOptions.java @@ -59,7 +59,7 @@ public boolean isRefreshProactively() { } /** - * @return the initial serialized JWT token + * @return the initial token */ public String getInitialToken() { return initialToken;