From ae606a54900ea21b1b1c778bfc890ad10e72d579 Mon Sep 17 00:00:00 2001 From: Franko Morales <67804607+cochi2@users.noreply.github.com> Date: Tue, 27 Jul 2021 13:37:52 -0700 Subject: [PATCH] FIX: Editing ChangeLog and Readme for CallingServer. (#23167) Also, Applying new comments received for the RedirectPolicy --- .../checkstyle/checkstyle-suppressions.xml | 2 +- .../CHANGELOG.md | 5 +- .../README.md | 2 +- .../azure-communication-callingserver/pom.xml | 23 ++++++ .../CallingServerClientBuilder.java | 2 +- .../implementation/RedirectPolicy.java | 70 +++++++++++++++++++ .../implementation/RedirectPolicyTests.java | 2 +- .../azure-communication-common/README.md | 2 +- .../azure-communication-common/pom.xml | 35 ---------- .../common/implementation/RedirectPolicy.java | 50 ------------- 10 files changed, 101 insertions(+), 92 deletions(-) create mode 100644 sdk/communication/azure-communication-callingserver/src/main/java/com/azure/communication/callingserver/implementation/RedirectPolicy.java rename sdk/communication/{azure-communication-common/src/test/java/com/azure/communication/common => azure-communication-callingserver/src/test/java/com/azure/communication/callingserver}/implementation/RedirectPolicyTests.java (98%) delete mode 100644 sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/implementation/RedirectPolicy.java diff --git a/eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml b/eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml index e9199779c6333..5e14428e154d9 100755 --- a/eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml +++ b/eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml @@ -608,5 +608,5 @@ the main ServiceBusClientBuilder. --> + files="com.azure.communication.callingserver.implementation.RedirectPolicy.java"/> diff --git a/sdk/communication/azure-communication-callingserver/CHANGELOG.md b/sdk/communication/azure-communication-callingserver/CHANGELOG.md index ba6484de97208..9f033f7a91547 100644 --- a/sdk/communication/azure-communication-callingserver/CHANGELOG.md +++ b/sdk/communication/azure-communication-callingserver/CHANGELOG.md @@ -1,7 +1,8 @@ # Release History -## 1.0.0-beta.3 (Unreleased) - +## 1.0.0-beta.3 (2021-07-26) +### Features Added +- Added RedirectPolicy as a new HttpPolicy to redirect requests based on the HttpResponse. ## 1.0.0-beta.2 (2021-06-25) - Updated sdk and apis documentation. diff --git a/sdk/communication/azure-communication-callingserver/README.md b/sdk/communication/azure-communication-callingserver/README.md index 78bf2a12adef7..1cd8c87059566 100644 --- a/sdk/communication/azure-communication-callingserver/README.md +++ b/sdk/communication/azure-communication-callingserver/README.md @@ -21,7 +21,7 @@ This package contains a Java SDK for Azure Communication CallingServer Service. com.azure azure-communication-callingserver - 1.0.0-beta.2 + 1.0.0-beta.3 ``` [//]: # ({x-version-update-end}) diff --git a/sdk/communication/azure-communication-callingserver/pom.xml b/sdk/communication/azure-communication-callingserver/pom.xml index 88a123dbecc65..330174043d067 100644 --- a/sdk/communication/azure-communication-callingserver/pom.xml +++ b/sdk/communication/azure-communication-callingserver/pom.xml @@ -176,4 +176,27 @@ + + + + is.jdk.11 + + [9,) + + + + + org.apache.maven.plugins + maven-surefire-plugin + 3.0.0-M3 + + + --add-opens com.azure.communication.callingserver/com.azure.communication.callingserver.implementation=ALL-UNNAMED + + + + + + + diff --git a/sdk/communication/azure-communication-callingserver/src/main/java/com/azure/communication/callingserver/CallingServerClientBuilder.java b/sdk/communication/azure-communication-callingserver/src/main/java/com/azure/communication/callingserver/CallingServerClientBuilder.java index 35fc1fe8ba8da..f8cea4a37d14a 100644 --- a/sdk/communication/azure-communication-callingserver/src/main/java/com/azure/communication/callingserver/CallingServerClientBuilder.java +++ b/sdk/communication/azure-communication-callingserver/src/main/java/com/azure/communication/callingserver/CallingServerClientBuilder.java @@ -5,7 +5,7 @@ import com.azure.communication.callingserver.implementation.AzureCommunicationCallingServerServiceImpl; import com.azure.communication.callingserver.implementation.AzureCommunicationCallingServerServiceImplBuilder; -import com.azure.communication.common.implementation.RedirectPolicy; +import com.azure.communication.callingserver.implementation.RedirectPolicy; import com.azure.communication.common.implementation.CommunicationConnectionString; import com.azure.communication.common.implementation.HmacAuthenticationPolicy; import com.azure.core.annotation.ServiceClientBuilder; diff --git a/sdk/communication/azure-communication-callingserver/src/main/java/com/azure/communication/callingserver/implementation/RedirectPolicy.java b/sdk/communication/azure-communication-callingserver/src/main/java/com/azure/communication/callingserver/implementation/RedirectPolicy.java new file mode 100644 index 0000000000000..53a763b5e069c --- /dev/null +++ b/sdk/communication/azure-communication-callingserver/src/main/java/com/azure/communication/callingserver/implementation/RedirectPolicy.java @@ -0,0 +1,70 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +package com.azure.communication.callingserver.implementation; + +import com.azure.core.http.HttpPipelineCallContext; +import com.azure.core.http.HttpPipelineNextPolicy; +import com.azure.core.http.HttpRequest; +import com.azure.core.http.HttpResponse; +import com.azure.core.http.policy.HttpPipelinePolicy; +import com.azure.core.util.logging.ClientLogger; +import reactor.core.publisher.Mono; + +import java.util.HashSet; +import java.util.Set; + +/** + * HttpPipelinePolicy to redirect requests when a redirect response (Http codes 301 or 302) is received to the + * new location marked by the Location header. + */ +public final class RedirectPolicy implements HttpPipelinePolicy { + private static final int MAX_REDIRECTS = 10; + private static final String LOCATION_HEADER_NAME = "Location"; + private static final int SC_MOVED_PERMANENTLY = 301; + private static final int SC_MOVED_TEMPORARILY = 302; + + private final ClientLogger logger = new ClientLogger(RedirectPolicy.class); + + @Override + public Mono process(HttpPipelineCallContext context, HttpPipelineNextPolicy next) { + return attemptRedirection(context, next, 0, new HashSet<>()); + } + + private Mono attemptRedirection(HttpPipelineCallContext context, HttpPipelineNextPolicy next, + int redirectNumber, Set attemptedRedirectLocations) { + return next.clone().process().flatMap(httpResponse -> { + if (isRedirectResponse(httpResponse) + && shouldRedirect(httpResponse, context, redirectNumber, attemptedRedirectLocations)) { + String newLocation = httpResponse.getHeaderValue(LOCATION_HEADER_NAME); + attemptedRedirectLocations.add(newLocation); + + HttpRequest newRequest = context.getHttpRequest().copy(); + newRequest.setUrl(newLocation); + context.setHttpRequest(newRequest); + + return attemptRedirection(context, next, redirectNumber + 1, attemptedRedirectLocations); + } + return Mono.just(httpResponse); + }); + } + + private boolean isRedirectResponse(HttpResponse response) { + return response.getStatusCode() == SC_MOVED_TEMPORARILY || response.getStatusCode() == SC_MOVED_PERMANENTLY; + } + + private boolean shouldRedirect(HttpResponse response, HttpPipelineCallContext context, int retryCount, + Set attemptedRedirectLocations) { + if (retryCount > MAX_REDIRECTS) { + logger.error(String.format("Request to %s has been redirected more than %s times.", + context.getHttpRequest().getUrl(), MAX_REDIRECTS)); + return false; + } + if (attemptedRedirectLocations.contains(response.getHeaderValue(LOCATION_HEADER_NAME))) { + logger.error(String.format("Request to %s was redirected more than once to: %s", + context.getHttpRequest().getUrl(), response.getHeaderValue(LOCATION_HEADER_NAME))); + return false; + } + return true; + } + +} diff --git a/sdk/communication/azure-communication-common/src/test/java/com/azure/communication/common/implementation/RedirectPolicyTests.java b/sdk/communication/azure-communication-callingserver/src/test/java/com/azure/communication/callingserver/implementation/RedirectPolicyTests.java similarity index 98% rename from sdk/communication/azure-communication-common/src/test/java/com/azure/communication/common/implementation/RedirectPolicyTests.java rename to sdk/communication/azure-communication-callingserver/src/test/java/com/azure/communication/callingserver/implementation/RedirectPolicyTests.java index 8e0e57247d089..0322ae31546ef 100644 --- a/sdk/communication/azure-communication-common/src/test/java/com/azure/communication/common/implementation/RedirectPolicyTests.java +++ b/sdk/communication/azure-communication-callingserver/src/test/java/com/azure/communication/callingserver/implementation/RedirectPolicyTests.java @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -package com.azure.communication.common.implementation; +package com.azure.communication.callingserver.implementation; import com.azure.core.http.HttpClient; import com.azure.core.http.HttpMethod; diff --git a/sdk/communication/azure-communication-common/README.md b/sdk/communication/azure-communication-common/README.md index eb6553634f226..df2d5c6fd41ff 100644 --- a/sdk/communication/azure-communication-common/README.md +++ b/sdk/communication/azure-communication-common/README.md @@ -62,4 +62,4 @@ Check out other client libraries for Azure communication service [cla]: https://cla.microsoft.com [coc]: https://opensource.microsoft.com/codeofconduct/ [coc_faq]: https://opensource.microsoft.com/codeofconduct/faq/ -[coc_contact]: mailto:opencode@microsoft.com \ No newline at end of file +[coc_contact]: mailto:opencode@microsoft.com diff --git a/sdk/communication/azure-communication-common/pom.xml b/sdk/communication/azure-communication-common/pom.xml index ebce18bf9706d..d76ce1c5074ae 100644 --- a/sdk/communication/azure-communication-common/pom.xml +++ b/sdk/communication/azure-communication-common/pom.xml @@ -84,40 +84,5 @@ 3.4.8 test - - org.mockito - mockito-core - 3.9.0 - test - - - org.hamcrest - hamcrest-all - 1.3 - test - - - - - java-lts - - [11,) - - - - - org.apache.maven.plugins - maven-surefire-plugin - 3.0.0-M3 - - - --add-opens com.azure.communication.common/com.azure.communication.common.implementation=ALL-UNNAMED - - - - - - - diff --git a/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/implementation/RedirectPolicy.java b/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/implementation/RedirectPolicy.java deleted file mode 100644 index 897851e3f8a89..0000000000000 --- a/sdk/communication/azure-communication-common/src/main/java/com/azure/communication/common/implementation/RedirectPolicy.java +++ /dev/null @@ -1,50 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. -package com.azure.communication.common.implementation; - -import com.azure.core.http.HttpPipelineCallContext; -import com.azure.core.http.HttpPipelineNextPolicy; -import com.azure.core.http.HttpRequest; -import com.azure.core.http.HttpResponse; -import com.azure.core.http.policy.HttpPipelinePolicy; -import reactor.core.publisher.Mono; - -import java.util.HashSet; -import java.util.Set; - -/** - * HttpPipelinePolicy to redirect requests when 302 message is received to the new location marked by the - * Location header. - */ -public final class RedirectPolicy implements HttpPipelinePolicy { - private static final int MAX_REDIRECTS = 10; - private static final String LOCATION_HEADER_NAME = "Location"; - - @Override - public Mono process(HttpPipelineCallContext context, HttpPipelineNextPolicy next) { - return attemptRedirection(context, next, 0, new HashSet<>()); - } - - private Mono attemptRedirection(HttpPipelineCallContext context, HttpPipelineNextPolicy next, - int redirectNumber, Set locations) { - return next.clone().process().flatMap(httpResponse -> { - if (shouldRedirect(httpResponse, redirectNumber, locations)) { - String newLocation = httpResponse.getHeaderValue(LOCATION_HEADER_NAME); - locations.add(newLocation); - - HttpRequest newRequest = context.getHttpRequest().copy(); - newRequest.setUrl(newLocation); - context.setHttpRequest(newRequest); - - return attemptRedirection(context, next, redirectNumber + 1, locations); - } - return Mono.just(httpResponse); - }); - } - - private boolean shouldRedirect(HttpResponse response, int redirectNumber, Set locations) { - return response.getStatusCode() == 302 - && !locations.contains(response.getHeaderValue(LOCATION_HEADER_NAME)) - && redirectNumber < MAX_REDIRECTS; - } -}