Address code review comments

eng/versioning/version_client.txt

@@ -50,7 +50,7 @@ com.azure:azure-communication-common;1.0.0-beta.4;1.0.0-beta.5
 com.azure:azure-communication-administration;1.0.0-beta.4;1.0.0-beta.5
 com.azure:azure-communication-sms;1.0.0-beta.4;1.0.0-beta.5
 com.azure:azure-communication-identity;1.0.0-beta.4;1.0.0-beta.5
-com.azure:azure-communication-phonenumbers;1.0.0-beta.4;1.0.0-beta.5
+com.azure:azure-communication-phonenumbers;1.0.0-beta.4;1.0.0-beta.4
 com.azure:azure-core;1.13.0;1.14.0-beta.1
 com.azure:azure-core-amqp;2.0.2;2.1.0-beta.1
 com.azure:azure-core-amqp-experimental;1.0.0-beta.1;1.0.0-beta.1

sdk/communication/azure-communication-administration/CHANGELOG.md

@@ -7,8 +7,8 @@
 - Added support for Azure Active Directory Authentication.
 
 ### Breaking Changes
-- CommunicationIdentityClient and CommunicationIdentityAsyncClient is moved to a new package, `azure-communication-identity`.
-- PhoneNumbersClient and PhoneNumbersAsyncClient is moved to a new package, `azure-communication-phonenumbers`.
+- CommunicationIdentityClient and CommunicationIdentityAsyncClient is moved to a new package, `azure.communication.identity`.
+- PhoneNumbersClient and PhoneNumbersAsyncClient is moved to a new package, `azure.communication.phonenumbers`.
 
 ## 1.0.0-beta.3 (2020-11-16)
 ### Added

sdk/communication/azure-communication-phonenumbers/CHANGELOG.md

@@ -2,11 +2,11 @@
 
 ## 1.0.0-beta.4 (Unreleased)
 ### Added
-- Added PhoneNumbersClient and PhoneNumbersAsyncClient (originally was part of the azure-communication-aministration package).
+- Added PhoneNumbersClient and PhoneNumbersAsyncClient (originally was part of the azure.communication.administration package).
 - Added support for Azure Active Directory Authentication.
 
 ### Breaking Changes
-- PhoneNumberAsyncClient has been replaced with PhoneNumbersAsyncClient, which has the same functionality but different APIs. To learn more about how PhoneNumbersAsyncClient works, refer to the README.md
-- PhoneNumberClient has been replaced with PhoneNumbersClient, which has the same functionality but different APIs. To learn more about how PhoneNumbersClient works, refer to the README.md
+- PhoneNumberAsyncClient has been replaced with PhoneNumbersAsyncClient, which has the same functionality but different APIs. To learn more about how PhoneNumbersAsyncClient works, refer to the [README.md][https://github.com/Azure/azure-sdk-for-java/blob/master/sdk/communication/azure-communication-phonenumbers/README.md].
+- PhoneNumberClient has been replaced with PhoneNumbersClient, which has the same functionality but different APIs. To learn more about how PhoneNumbersClient works, refer to the [README.md][https://github.com/Azure/azure-sdk-for-java/blob/master/sdk/communication/azure-communication-phonenumbers/README.md].
 
 

sdk/communication/azure-communication-phonenumbers/pom.xml

@@ -62,7 +62,7 @@
     <dependency>
       <groupId>com.azure</groupId>
       <artifactId>azure-communication-common</artifactId>
-      <version>1.0.0-beta.4</version> <!-- {x-version-update;com.azure:azure-communication-common;current} -->
+      <version>1.0.0-beta.5</version> <!-- {x-version-update;com.azure:azure-communication-common;current} -->
     </dependency>
     <dependency>
       <groupId>org.junit.jupiter</groupId>

sdk/communication/azure-communication-phonenumbers/src/main/java/com/azure/communication/phonenumbers/PhoneNumbersAsyncClient.java

@@ -56,6 +56,7 @@ public final class PhoneNumbersAsyncClient {
      * @param phoneNumber The phone number id in E.164 format. The leading plus can be either + or encoded
      *                    as %2B.
      * @return {@link AcquiredPhoneNumber} representing the acquired telephone number.
+     * @throws NullPointerException if {@code phoneNumber} is null.
      */
     @ServiceMethod(returns = ReturnType.SINGLE)
     public Mono<AcquiredPhoneNumber> getPhoneNumber(String phoneNumber) {
@@ -72,6 +73,7 @@ public Mono<AcquiredPhoneNumber> getPhoneNumber(String phoneNumber) {
      * @param phoneNumber The phone number id in E.164 format. The leading plus can be either + or encoded
      *                    as %2B.
      * @return {@link AcquiredPhoneNumber} representing the acquired telephone number.
+     * @throws NullPointerException if {@code phoneNumber} is null.
      */
     @ServiceMethod(returns = ReturnType.SINGLE)
     public Mono<Response<AcquiredPhoneNumber>> getPhoneNumberWithResponse(String phoneNumber) {
@@ -102,16 +104,16 @@ public PagedFlux<AcquiredPhoneNumber> listPhoneNumbers() {
      *
      * @param phoneNumber The phone number id in E.164 format. The leading plus can be either + or encoded
      *                    as %2B.
-     * @param update Update to an acquired phone number.
-     * @return A {@link Mono} containing
-     * a {@link AcquiredPhoneNumber} representing the acquired phone number
+     * @param updateRequest Update request to an acquired phone number.
+     * @return A {@link Mono} containing a {@link AcquiredPhoneNumber} representing the acquired phone number
+     * @throws NullPointerException if {@code phoneNumber} or {@code updateRequest} is null.
      */
     @ServiceMethod(returns = ReturnType.SINGLE)
-    public Mono<AcquiredPhoneNumber> updatePhoneNumber(String phoneNumber, PhoneNumberUpdateRequest update) {
+    public Mono<AcquiredPhoneNumber> updatePhoneNumber(String phoneNumber, PhoneNumberUpdateRequest updateRequest) {
         try {
             Objects.requireNonNull(phoneNumber, "'phoneNumber' cannot be null.");
-            Objects.requireNonNull(update, "'update' cannot be null.");
-            return client.updateAsync(phoneNumber, update);
+            Objects.requireNonNull(updateRequest, "'updateRequest' cannot be null.");
+            return client.updateAsync(phoneNumber, updateRequest);
         } catch (RuntimeException ex) {
             return monoError(logger, ex);
         }
@@ -123,17 +125,17 @@ public Mono<AcquiredPhoneNumber> updatePhoneNumber(String phoneNumber, PhoneNumb
      *
      * @param phoneNumber The phone number id in E.164 format. The leading plus can be either + or encoded
      *                    as %2B.
-     * @param update Update to an acquired phone number.
-     * @return A {@link Mono} containing
-     * a {@link AcquiredPhoneNumber} representing the acquired phone number
+     * @param updateRequest Update request to an acquired phone number.
+     * @return A {@link Mono} containing a {@link AcquiredPhoneNumber} representing the acquired phone number
+     * @throws NullPointerException if {@code phoneNumber} or {@code updateRequest} is null.
      */
     @ServiceMethod(returns = ReturnType.SINGLE)
     public Mono<Response<AcquiredPhoneNumber>> 
-        updatePhoneNumberWithResponse(String phoneNumber, PhoneNumberUpdateRequest update) {
+        updatePhoneNumberWithResponse(String phoneNumber, PhoneNumberUpdateRequest updateRequest) {
         try {
             Objects.requireNonNull(phoneNumber, "'phoneNumber' cannot be null.");
-            Objects.requireNonNull(update, "'update' cannot be null.");
-            return client.updateWithResponseAsync(phoneNumber, update);
+            Objects.requireNonNull(updateRequest, "'updateRequest' cannot be null.");
+            return client.updateWithResponseAsync(phoneNumber, updateRequest);
         } catch (RuntimeException ex) {
             return monoError(logger, ex);
         }
@@ -146,6 +148,7 @@ public Mono<AcquiredPhoneNumber> updatePhoneNumber(String phoneNumber, PhoneNumb
      * @param searchRequest {@link PhoneNumberSearchRequest} specifying the search request
      * until it gets a result from the server
      * @return A {@link PollerFlux} object with the reservation result
+     * @throws NullPointerException if {@code countryCode} or {@code searchRequest} is null.
      */
     @ServiceMethod(returns = ReturnType.LONG_RUNNING_OPERATION)
     public PollerFlux<PhoneNumberOperation, PhoneNumberSearchResult> beginSearchAvailablePhoneNumbers(
@@ -188,9 +191,10 @@ PollerFlux<PhoneNumberOperation, PhoneNumberSearchResult> beginSearchAvailablePh
     }
 
     private String parseIdFromUrl(String url) {
+        Objects.requireNonNull(url, "'url' cannot be null.");
         String[] items = url.split("/");
         String id = items[items.length - 1];
-        return id.substring(0, id.indexOf("?"));
+        return id.substring(0, Math.min(id.indexOf("?"), id.length()));
     }
 
     private Function<PollingContext<PhoneNumberOperation>, Mono<PollResponse<PhoneNumberOperation>>>
@@ -242,6 +246,7 @@ private String parseIdFromUrl(String url) {
      *
      * @param searchId ID of the search
      * @return A {@link PollerFlux} object.
+     * @throws NullPointerException if {@code searchId} is null.
      */
     @ServiceMethod(returns = ReturnType.LONG_RUNNING_OPERATION)
     public PollerFlux<PhoneNumberOperation, Void> beginPurchasePhoneNumbers(String searchId) {
@@ -282,11 +287,10 @@ PollerFlux<PhoneNumberOperation, Void> beginPurchasePhoneNumbers(String searchId
      *
      * This function returns a Long Running Operation poller that allows you to wait indefinitely until the 
      * operation is complete.
-     *
-     *
      * @param phoneNumber The phone number id in E.164 format. The leading plus can be either + or encoded
      *                    as %2B.
      * @return A {@link PollerFlux} object.
+     * @throws NullPointerException if {@code phoneNumber} is null.
      */
     @ServiceMethod(returns = ReturnType.LONG_RUNNING_OPERATION)
     public PollerFlux<PhoneNumberOperation, Void> beginReleasePhoneNumber(String phoneNumber) {
@@ -327,7 +331,8 @@ PollerFlux<PhoneNumberOperation, Void> beginReleasePhoneNumber(String phoneNumbe
      * @param phoneNumber The phone number id in E.164 format. The leading plus can be either + or encoded
      *                    as %2B.
      * @param capabilitiesUpdateRequest Update capabilities of an acquired phone number.
-     * @return A {@link PollerFlux} object
+     * @return A {@link PollerFlux} object.
+     * @throws NullPointerException if {@code phoneNumber} or {@code capabilitiesUpdateRequest} is null.
      */
     @ServiceMethod(returns = ReturnType.LONG_RUNNING_OPERATION)
     public PollerFlux<PhoneNumberOperation, AcquiredPhoneNumber> 

sdk/communication/azure-communication-phonenumbers/src/main/java/com/azure/communication/phonenumbers/PhoneNumbersClient.java

@@ -41,6 +41,7 @@ public final class PhoneNumbersClient {
      * @param phoneNumber The phone number id in E.164 format. The leading plus can be either + or encoded
      *                    as %2B.
      * @return {@link AcquiredPhoneNumber} representing the acquired telephone number.
+     * @throws NullPointerException if {@code phoneNumber} is null.
      */
     @ServiceMethod(returns = ReturnType.SINGLE)
     public AcquiredPhoneNumber getPhoneNumber(String phoneNumber) {
@@ -54,6 +55,7 @@ public AcquiredPhoneNumber getPhoneNumber(String phoneNumber) {
      *                    as %2B.
      * @param context A {@link Context} representing the request context.
      * @return {@link AcquiredPhoneNumber} representing the acquired telephone number.
+     * @throws NullPointerException if {@code phoneNumber} is null.
      */
     @ServiceMethod(returns = ReturnType.SINGLE)
     public Response<AcquiredPhoneNumber> getPhoneNumberWithResponse(String phoneNumber, Context context) {
@@ -80,31 +82,33 @@ public PagedIterable<AcquiredPhoneNumber> listPhoneNumbers(Context context) {
      *
      * @param phoneNumber The phone number id in E.164 format. The leading plus can be either + or encoded
      *                    as %2B.
-     * @param update {@link PhoneNumberUpdateRequest} specifying updates to an acquired phone number.
+     * @param updateRequest {@link PhoneNumberUpdateRequest} specifying updates to an acquired phone number.
      * @return {@link AcquiredPhoneNumber} representing the updated acquired phone number
+     * @throws NullPointerException if {@code phoneNumber} or {@code updateRequest} is null.
      */
     @ServiceMethod(returns = ReturnType.SINGLE)
-    public AcquiredPhoneNumber updatePhoneNumber(String phoneNumber, PhoneNumberUpdateRequest update) {
+    public AcquiredPhoneNumber updatePhoneNumber(String phoneNumber, PhoneNumberUpdateRequest updateRequest) {
         Objects.requireNonNull(phoneNumber, "'phoneNumber' cannot be null.");
-        Objects.requireNonNull(update, "'update' cannot be null.");
-        return client.updateAsync(phoneNumber, update).block();
+        Objects.requireNonNull(updateRequest, "'updateRequest' cannot be null.");
+        return client.updateAsync(phoneNumber, updateRequest).block();
     }
 
     /**
      * Update an acquired phone number with response.
      *
      * @param phoneNumber The phone number id in E.164 format. The leading plus can be either + or encoded
      *                    as %2B.
-     * @param update Update to an acquired phone number.
+     * @param updateRequest Update request to an acquired phone number.
      * @param context A {@link Context} representing the request context.
      * @return {@link AcquiredPhoneNumber} representing the updated acquired phone number
+     * @throws NullPointerException if {@code phoneNumber} or {@code updateRequest} is null.
      */
     @ServiceMethod(returns = ReturnType.SINGLE)
-    public Response<AcquiredPhoneNumber> updatePhoneNumberWithResponse(String phoneNumber, PhoneNumberUpdateRequest update, Context context) {
+    public Response<AcquiredPhoneNumber> updatePhoneNumberWithResponse(String phoneNumber, PhoneNumberUpdateRequest updateRequest, Context context) {
         Objects.requireNonNull(phoneNumber, "'phoneNumber' cannot be null.");
-        Objects.requireNonNull(update, "'update' cannot be null.");
+        Objects.requireNonNull(updateRequest, "'updateRequest' cannot be null.");
         context = context == null ? Context.NONE : context;
-        return client.updateWithResponseAsync(phoneNumber, update, context).block();
+        return client.updateWithResponseAsync(phoneNumber, updateRequest, context).block();
     }
 
     /**
@@ -117,6 +121,7 @@ public Response<AcquiredPhoneNumber> updatePhoneNumberWithResponse(String phoneN
      * @param searchRequest The search request
      * @param context A {@link Context} representing the request context.
      * @return A {@link SyncPoller} object with the reservation result
+     * @throws NullPointerException if {@code countryCode} or {@code searchRequest} is null.
      */
     @ServiceMethod(returns = ReturnType.SINGLE)
     public SyncPoller<PhoneNumberOperation, PhoneNumberSearchResult> beginSearchAvailablePhoneNumbers(
@@ -133,6 +138,7 @@ public SyncPoller<PhoneNumberOperation, PhoneNumberSearchResult> beginSearchAvai
      * @param searchId ID of the search
      * @param context A {@link Context} representing the request context.
      * @return A {@link SyncPoller} object.
+     * @throws NullPointerException if {@code searchId} is null.
      */
     @ServiceMethod(returns = ReturnType.SINGLE)
     public SyncPoller<PhoneNumberOperation, Void> beginPurchasePhoneNumbers(String searchId, Context context) {
@@ -150,6 +156,7 @@ public SyncPoller<PhoneNumberOperation, Void> beginPurchasePhoneNumbers(String s
      *                    as %2B.
      * @param context A {@link Context} representing the request context.
      * @return A {@link SyncPoller} object.
+     * @throws NullPointerException if {@code phoneNumber} is null.
      */
     @ServiceMethod(returns = ReturnType.SINGLE)
     public SyncPoller<PhoneNumberOperation, Void> beginReleasePhoneNumber(String phoneNumber, Context context) {
@@ -166,6 +173,7 @@ public SyncPoller<PhoneNumberOperation, Void> beginReleasePhoneNumber(String pho
      * @param capabilitiesUpdateRequest Update capabilities of an acquired phone number.
      * @param context A {@link Context} representing the request context.
      * @return A {@link SyncPoller} object
+     * @throws NullPointerException if {@code phoneNumber} or {@code capabilitiesUpdateRequest} is null.
      */
     @ServiceMethod(returns = ReturnType.SINGLE)
     public SyncPoller<PhoneNumberOperation, AcquiredPhoneNumber> beginUpdatePhoneNumberCapabilities(String phoneNumber, PhoneNumberCapabilitiesRequest capabilitiesUpdateRequest, Context context) {

sdk/communication/azure-communication-phonenumbers/src/main/java/com/azure/communication/phonenumbers/PhoneNumbersClientBuilder.java

@@ -20,6 +20,7 @@
 import com.azure.core.http.policy.HttpPipelinePolicy;
 import com.azure.core.http.policy.RetryPolicy;
 import com.azure.core.http.policy.UserAgentPolicy;
+import com.azure.core.util.ClientOptions;
 import com.azure.core.util.Configuration;
 import com.azure.core.util.CoreUtils;
 import com.azure.core.util.logging.ClientLogger;
@@ -49,6 +50,7 @@ public final class PhoneNumbersClientBuilder {
     private AzureKeyCredential azureKeyCredential;
     private TokenCredential tokenCredential;
     private Configuration configuration;
+    private ClientOptions clientOptions;
     private final List<HttpPipelinePolicy> additionalPolicies = new ArrayList<>();
 
     /**
@@ -169,6 +171,18 @@ public PhoneNumbersClientBuilder addPolicy(HttpPipelinePolicy policy) {
         return this;
     }
 
+    /**
+     * Sets the client options for all the requests made through the client.
+     *
+     * @param clientOptions {@link ClientOptions}.
+     * @return The updated {@link PhoneNumbersClientBuilder} object.
+     * @throws NullPointerException If {@code clientOptions} is {@code null}.
+     */
+    public PhoneNumbersClientBuilder clientOptions(ClientOptions clientOptions) {
+        this.clientOptions = Objects.requireNonNull(clientOptions, "'clientOptions' cannot be null.");
+        return this;
+    }
+
     /**
      * Sets the {@link PhoneNumbersServiceVersion} that is used when making API requests.
      * <p>
@@ -304,6 +318,7 @@ private HttpPipeline createHttpPipeline() {
         return new HttpPipelineBuilder()
             .policies(policyList.toArray(new HttpPipelinePolicy[0]))
             .httpClient(this.httpClient)
+            .clientOptions(clientOptions)
             .build();
     }
 

sdk/communication/azure-communication-phonenumbers/src/test/java/com/azure/communication/phonenumbers/PhoneNumbersAsyncClientIntegrationTest.java

@@ -46,6 +46,20 @@ public void getPhoneNumber(HttpClient httpClient) {
             .verifyComplete();
     }
 
+    @ParameterizedTest
+    @MethodSource("com.azure.core.test.TestBase#getHttpClients")
+    public void getPhoneNumberWithAAD(HttpClient httpClient) {
+        String phoneNumber = getTestPhoneNumber(PHONE_NUMBER);
+        StepVerifier.create(
+            this.getClientWithManagedIdentity(httpClient, "getPhoneNumber").getPhoneNumber(phoneNumber)
+            )
+            .assertNext((AcquiredPhoneNumber number) -> {
+                assertEquals(phoneNumber, number.getPhoneNumber());
+                assertEquals(COUNTRY_CODE, number.getCountryCode());
+            })
+            .verifyComplete();
+    }
+
     @ParameterizedTest
     @MethodSource("com.azure.core.test.TestBase#getHttpClients")
     public void getPhoneNumberWithResponse(HttpClient httpClient) {
@@ -214,6 +228,11 @@ private PhoneNumbersAsyncClient getClientWithConnectionString(HttpClient httpCli
         return addLoggingPolicy(builder, testName).buildAsyncClient();
     }
 
+    private PhoneNumbersAsyncClient getClientWithManagedIdentity(HttpClient httpClient, String testName) {
+        PhoneNumbersClientBuilder builder = super.getClientBuilderUsingManagedIdentity(httpClient);
+        return addLoggingPolicy(builder, testName).buildAsyncClient();
+    }
+
     private String getTestPhoneNumber(String phoneNumber) {
         if (getTestMode() == TestMode.PLAYBACK) {
             phoneNumber = "+REDACTED";