[Communication] - Identity and Common - Addressed final identity and common apiview comments (Azure#19530)

jbeauregardb · web-flow · commit a317d242b826 · 2021-03-03T10:51:19.000-07:00
* Addressed final identity and common apiview comments

* Address final changes

* Currently fixing unnamed module issue
diff --git a/sdk/communication/azure-communication-chat/pom.xml b/sdk/communication/azure-communication-chat/pom.xml
@@ -123,4 +123,26 @@
       </plugin>
     </plugins>
   </build>
+  <profiles>
+    <profile>
+      <id>java-lts</id>
+      <activation>
+        <jdk>[11,)</jdk>
+      </activation>
+      <build>
+        <plugins>
+          <plugin>
+            <groupId>org.apache.maven.plugins</groupId>
+            <artifactId>maven-surefire-plugin</artifactId>
+            <version>3.0.0-M3</version> <!-- {x-version-update;org.apache.maven.plugins:maven-surefire-plugin;external_dependency} -->
+            <configuration>
+              <argLine>
+                --add-opens com.azure.communication.common/com.azure.communication.common.implementation=ALL-UNNAMED
+              </argLine>
+            </configuration>
+          </plugin>
+        </plugins>
+      </build>
+    </profile>
+  </profiles>
 </project>
diff --git a/sdk/communication/azure-communication-chat/src/main/java/com/azure/communication/chat/implementation/CommunicationBearerTokenCredential.java b/sdk/communication/azure-communication-chat/src/main/java/com/azure/communication/chat/implementation/CommunicationBearerTokenCredential.java
@@ -3,8 +3,6 @@
 
 package com.azure.communication.chat.implementation;
 
-import java.util.concurrent.ExecutionException;
-
 import com.azure.communication.common.CommunicationTokenCredential;
 import com.azure.core.credential.AccessToken;
 import com.azure.core.credential.TokenCredential;
@@ -31,12 +29,6 @@ public CommunicationBearerTokenCredential(CommunicationTokenCredential communica
 
     @Override
     public Mono<AccessToken> getToken(TokenRequestContext request) {
-        try {
-            return credential.getToken();
-        } catch (InterruptedException ex) {
-            return Mono.error(ex);
-        } catch (ExecutionException ex) {
-            return Mono.error(ex);
-        }
+        return credential.getToken();
     }
 }
diff --git a/sdk/communication/azure-communication-chat/src/main/java/com/azure/communication/chat/implementation/converters/CommunicationIdentifierConverter.java b/sdk/communication/azure-communication-chat/src/main/java/com/azure/communication/chat/implementation/converters/CommunicationIdentifierConverter.java
@@ -49,7 +49,7 @@ public static CommunicationIdentifier convert(CommunicationIdentifierModel ident
             return new MicrosoftTeamsUserIdentifier(teamsUserIdentifierModel.getUserId(),
                 teamsUserIdentifierModel.isAnonymous())
                 .setRawId(rawId)
-                .setCloudEnvironment(new CommunicationCloudEnvironment(teamsUserIdentifierModel.getCloud().toString()));
+                .setCloudEnvironment(new CommunicationCloudEnvironment().fromString(teamsUserIdentifierModel.getCloud().toString()));
         }
 
         Objects.requireNonNull(rawId);
diff --git a/sdk/communication/azure-communication-common/CHANGELOG.md b/sdk/communication/azure-communication-common/CHANGELOG.md
@@ -1,7 +1,14 @@
 # Release History
 
+## 1.0.0-beta.6 (2021-03-09)
+### Breaking Changes
+- Renamed `CommunicationTokenRefreshOptions.getRefreshProactively()` to `CommunicationTokenRefreshOptions.isRefreshProactively()`
+- Constructor for `CommunicationCloudEnvironment` has been removed and now to set an environment value, the `fromString()` method must be called
+- `CommunicationCloudEnvironment`, `CommunicationTokenRefreshOptions `, `CommunicationUserIdentifier`, `MicrosoftTeamsUserIdentifier`,
+`PhoneNumberIdentifier`, `UnknownIdentifier`, are all final classes now.
 ## 1.0.0-beta.5 (2021-03-02)
-Updated `azure-communication-common` version
+- Updated `azure-communication-common` version
+
 
 ## 1.0.0-beta.4 (2021-02-09)
 ### Breaking Changes
diff --git a/sdk/communication/azure-communication-common/pom.xml b/sdk/communication/azure-communication-common/pom.xml
@@ -38,8 +38,8 @@
   <properties>
     <src.dir>src/main</src.dir>
     <test.dir>src/test</test.dir>
-    <jacoco.min.linecoverage>0.65</jacoco.min.linecoverage>
-    <jacoco.min.branchcoverage>0.50</jacoco.min.branchcoverage>
+    <jacoco.min.linecoverage>0.69</jacoco.min.linecoverage>
+    <jacoco.min.branchcoverage>0.52</jacoco.min.branchcoverage>
   </properties>
 
   <dependencies>
diff --git a/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationCloudEnvironment.java b/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationCloudEnvironment.java
@@ -3,40 +3,50 @@
 package com.azure.communication.common;
 
 import java.util.Objects;
+import com.azure.core.util.ExpandableStringEnum;
 
 /**
  * The cloud that the identifier belongs to.
  */
-public class CommunicationCloudEnvironment {
+public final class CommunicationCloudEnvironment extends ExpandableStringEnum<CommunicationCloudEnvironment> {
     private static final String PUBLIC_VALUE = "public";
     private static final String DOD_VALUE = "dod";
     private static final String GCCH_VALUE = "gcch";
 
-    private final String environmentValue;
+    private String environmentValue;
 
     /**
-     * Create CommunicationCloudEnvironment with name string
-     * @param environmentValue name of hte cloud environment
+     * Creates a default CommunicationCloudEnvironment
      */
-    public CommunicationCloudEnvironment(String environmentValue) {
+    public CommunicationCloudEnvironment() {
+        this.environmentValue = PUBLIC_VALUE;
+    }
+
+    /**
+     * Sets an environment value from a String
+     * @param environmentValue string representation of the environment value
+     * @return CommunicationCloudEnvironment object itself
+     */
+    public CommunicationCloudEnvironment fromString(String environmentValue) {
         Objects.requireNonNull(environmentValue);
         this.environmentValue = environmentValue;
+        return this;
     }
 
     /**
      * Represent Azure public cloud
      */
-    public static final CommunicationCloudEnvironment PUBLIC = new CommunicationCloudEnvironment(PUBLIC_VALUE);
+    public static final CommunicationCloudEnvironment PUBLIC = new CommunicationCloudEnvironment().fromString(PUBLIC_VALUE);
 
     /**
      * Represent Azure Dod cloud
      */
-    public static final CommunicationCloudEnvironment DOD = new CommunicationCloudEnvironment(DOD_VALUE);
+    public static final CommunicationCloudEnvironment DOD = new CommunicationCloudEnvironment().fromString(DOD_VALUE);
 
     /**
      * Represent Azure Gcch cloud
      */
-    public static final CommunicationCloudEnvironment GCCH = new CommunicationCloudEnvironment(GCCH_VALUE);
+    public static final CommunicationCloudEnvironment GCCH = new CommunicationCloudEnvironment().fromString(GCCH_VALUE);
 
     @Override
     public boolean equals(Object that) {
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
@@ -2,8 +2,6 @@
 // Licensed under the MIT License.
 package com.azure.communication.common;
 
-import java.util.concurrent.ExecutionException;
-
 import com.azure.core.util.FluxUtil;
 import com.azure.core.util.logging.ClientLogger;
 
@@ -59,7 +57,7 @@ public CommunicationTokenCredential(CommunicationTokenRefreshOptions tokenRefres
         refresher = tokenRefresher;
         if (tokenRefreshOptions.getToken() != null) {
             setToken(tokenRefreshOptions.getToken());
-            if (tokenRefreshOptions.getRefreshProactively()) {
+            if (tokenRefreshOptions.isRefreshProactively()) {
                 OffsetDateTime nextFetchTime = accessToken.getExpiresAt().minusMinutes(DEFAULT_EXPIRING_OFFSET_MINUTES);
                 fetchingTask = new FetchingTask(this, nextFetchTime);
             }
@@ -70,10 +68,8 @@ public CommunicationTokenCredential(CommunicationTokenRefreshOptions tokenRefres
      * Get Azure core access token from credential
      *
      * @return Asynchronous call to fetch actual token
-     * @throws ExecutionException when supplier throws this exception
-     * @throws InterruptedException when supplier throws this exception
      */
-    public Mono<AccessToken> getToken() throws InterruptedException, ExecutionException {
+    public Mono<AccessToken> getToken() {
         if (isClosed) {
             return FluxUtil.monoError(logger,
                 new RuntimeException("getToken called on closed CommunicationTokenCredential object"));
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
@@ -5,7 +5,7 @@
 /**
  * Options for refreshing CommunicationTokenCredential
  */
-public class CommunicationTokenRefreshOptions {
+public final class CommunicationTokenRefreshOptions {
     private final TokenRefresher tokenRefresher;
     private final boolean refreshProactively;
     private final String initialToken;
@@ -51,7 +51,7 @@ public TokenRefresher getTokenRefresher() {
     /**
      * @return whether or not to refresh token proactively
      */
-    public boolean getRefreshProactively() {
+    public boolean isRefreshProactively() {
         return refreshProactively;
     }
 
diff --git a/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationUserIdentifier.java b/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/CommunicationUserIdentifier.java
@@ -7,7 +7,7 @@
 /**
  * Communication identifier for Communication Services Users
  */
-public class CommunicationUserIdentifier extends CommunicationIdentifier {
+public final class CommunicationUserIdentifier extends CommunicationIdentifier {
 
     private final String id;
 
diff --git a/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/MicrosoftTeamsUserIdentifier.java b/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/MicrosoftTeamsUserIdentifier.java
@@ -8,28 +8,14 @@
 /**
  * Communication identifier for Microsoft Teams User
  */
-public class MicrosoftTeamsUserIdentifier extends CommunicationIdentifier {
+public final class MicrosoftTeamsUserIdentifier extends CommunicationIdentifier {
 
     private final String userId;
     private final boolean isAnonymous;
     private CommunicationCloudEnvironment cloudEnvironment = CommunicationCloudEnvironment.PUBLIC;
 
     private String rawId;
 
-    /**
-     * Creates a MicrosoftTeamsUserIdentifier object
-     *
-     * @param userId Id of the Microsoft Teams user. If the user isn't anonymous, the id is the AAD object id of the user.
-     * @param isAnonymous set this to true if the user is anonymous,
-     *                    for example when joining a meeting with a share link
-     * @param cloudEnvironment the cloud environment in which this identifier is created
-     * @throws IllegalArgumentException thrown if userId parameter fail the validation.
-     */
-    public MicrosoftTeamsUserIdentifier(String userId, boolean isAnonymous, CommunicationCloudEnvironment  cloudEnvironment) {
-        this(userId, isAnonymous);
-        this.cloudEnvironment = cloudEnvironment;
-    }
-
     /**
      * Creates a MicrosoftTeamsUserIdentifier object
      *
@@ -76,7 +62,7 @@ public boolean isAnonymous() {
      * @param cloudEnvironment the cloud environment in which this identifier is created
      * @return this object
      */
-    public MicrosoftTeamsUserIdentifier setCloudEnvironment(CommunicationCloudEnvironment  cloudEnvironment) {
+    public MicrosoftTeamsUserIdentifier setCloudEnvironment(CommunicationCloudEnvironment cloudEnvironment) {
         this.cloudEnvironment = cloudEnvironment;
         return this;
     }
diff --git a/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/PhoneNumberIdentifier.java b/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/PhoneNumberIdentifier.java
@@ -7,7 +7,7 @@
 /**
  * Communication identifier for Communication Services Phone Numbers
  */
-public class PhoneNumberIdentifier extends CommunicationIdentifier {
+public final class PhoneNumberIdentifier extends CommunicationIdentifier {
 
     private final String phoneNumber;
     private String rawId;
diff --git a/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/UnknownIdentifier.java b/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/UnknownIdentifier.java
@@ -7,7 +7,7 @@
 /**
  * Catch-all for all other Communication identifiers for Communication Services
  */
-public class UnknownIdentifier extends CommunicationIdentifier {
+public final class UnknownIdentifier extends CommunicationIdentifier {
 
     private final String id;
 
diff --git a/sdk/communication/azure-communication-common/src/main/java/module-info.java b/sdk/communication/azure-communication-common/src/main/java/module-info.java
@@ -16,6 +16,4 @@
         com.azure.communication.identity,
         com.azure.communication.phonenumbers,
         com.azure.communication.chat;
-
-    opens com.azure.communication.common.implementation;
 }
diff --git a/sdk/communication/azure-communication-identity/CHANGELOG.md b/sdk/communication/azure-communication-identity/CHANGELOG.md
@@ -1,5 +1,9 @@
 # Release History
 
+## 1.0.0-beta.6 (2021-03-09)
+### Added
+- Added a retryPolicy() chain method to the `CommunicationIdentityClientBuilder`
+
 ## 1.0.0-beta.5 (2021-03-02)
 ### Breaking
 - `CommunicationIdentityAsyncClient.issueToken` and `CommunicationIdentityClient.issueToken` is renamed to `CommunicationIdentityAsyncClient.getToken` and `CommunicationIdentityClient.getToken`.
diff --git a/sdk/communication/azure-communication-identity/src/main/java/com/azure/communication/identity/CommunicationIdentityClient.java b/sdk/communication/azure-communication-identity/src/main/java/com/azure/communication/identity/CommunicationIdentityClient.java
@@ -117,12 +117,11 @@ public Response<CommunicationUserIdentifierWithTokenResult> createUserWithTokenW
      * data.
      *
      * @param communicationUser The user to be deleted.
-     * @return the response
      */
     @ServiceMethod(returns = ReturnType.SINGLE)
-    public Void deleteUser(CommunicationUserIdentifier communicationUser) {
+    public void deleteUser(CommunicationUserIdentifier communicationUser) {
         Objects.requireNonNull(communicationUser);
-        return client.deleteAsync(communicationUser.getId()).block();
+        client.deleteAsync(communicationUser.getId()).block();
     }
 
     /**
@@ -144,12 +143,11 @@ public Response<Void> deleteUserWithResponse(CommunicationUserIdentifier communi
      * Revokes all the tokens created for an identifier.
      * 
      * @param communicationUser The user to be revoked token.
-     * @return the response
      */
     @ServiceMethod(returns = ReturnType.SINGLE)
-    public Void revokeTokens(CommunicationUserIdentifier communicationUser) {
+    public void revokeTokens(CommunicationUserIdentifier communicationUser) {
         Objects.requireNonNull(communicationUser);
-        return client.revokeAccessTokensAsync(communicationUser.getId()).block();
+        client.revokeAccessTokensAsync(communicationUser.getId()).block();
     }
 
     /**
diff --git a/sdk/communication/azure-communication-identity/src/main/java/com/azure/communication/identity/CommunicationIdentityClientBuilder.java b/sdk/communication/azure-communication-identity/src/main/java/com/azure/communication/identity/CommunicationIdentityClientBuilder.java
@@ -48,6 +48,7 @@ public final class CommunicationIdentityClientBuilder {
     private HttpClient httpClient;
     private HttpLogOptions httpLogOptions = new HttpLogOptions();
     private HttpPipeline pipeline;
+    private RetryPolicy retryPolicy;
     private Configuration configuration;
     private ClientOptions clientOptions;
     private final Map<String, String> properties = CoreUtils.getProperties(COMMUNICATION_IDENTITY_PROPERTIES);
@@ -175,6 +176,20 @@ public CommunicationIdentityClientBuilder httpLogOptions(HttpLogOptions logOptio
         return this;
     }
 
+    /**
+     * Sets the {@link RetryPolicy} that is used when each request is sent.
+     * <p>
+     * The default retry policy will be used in the pipeline, if not provided.
+     *
+     * @param retryPolicy User's retry policy applied to each request.
+     * @return The updated {@link CommunicationIdentityClientBuilder} object.
+     * @throws NullPointerException If the specified {@code retryPolicy} is null.
+     */
+    public CommunicationIdentityClientBuilder retryPolicy(RetryPolicy retryPolicy) {
+        this.retryPolicy = Objects.requireNonNull(retryPolicy, "The retry policy cannot be bull");
+        return this;
+    }
+
     /**
      * Sets the {@link CommunicationIdentityServiceVersion} that is used when making API requests.
      * <p>

Original file line number	Diff line number	Diff line change
`@@ -49,7 +49,7 @@ public static CommunicationIdentifier convert(CommunicationIdentifierModel ident`
`49`	`49`	`return new MicrosoftTeamsUserIdentifier(teamsUserIdentifierModel.getUserId(),`
`50`	`50`	`teamsUserIdentifierModel.isAnonymous())`
`51`	`51`	`.setRawId(rawId)`
`52`		`- .setCloudEnvironment(new CommunicationCloudEnvironment(teamsUserIdentifierModel.getCloud().toString()));`
	`52`	`+ .setCloudEnvironment(new CommunicationCloudEnvironment().fromString(teamsUserIdentifierModel.getCloud().toString()));`
`53`	`53`	`}`
`54`	`54`
`55`	`55`	`Objects.requireNonNull(rawId);`