From 8e96741c503e9eef235d5a1ea0860f9640454ec8 Mon Sep 17 00:00:00 2001
From: Franko Morales <fmorales@microsoft.com>
Date: Sat, 24 Jul 2021 14:08:47 -0700
Subject: [PATCH] Editing ChangeLog and Readme.

Also, Applying new comments received for the RedirectPolicy
---
 .../CHANGELOG.md                              |  4 ++-
 .../README.md                                 |  2 +-
 .../common/implementation/RedirectPolicy.java | 36 ++++++++++++++-----
 3 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/sdk/communication/azure-communication-callingserver/CHANGELOG.md b/sdk/communication/azure-communication-callingserver/CHANGELOG.md
index ba6484de97208..31324e375416a 100644
--- a/sdk/communication/azure-communication-callingserver/CHANGELOG.md
+++ b/sdk/communication/azure-communication-callingserver/CHANGELOG.md
@@ -1,7 +1,9 @@
 # Release History
 
-## 1.0.0-beta.3 (Unreleased)
+## 1.0.0-beta.4 (unreleased)
 
+## 1.0.0-beta.3 (2021-07-26)
+- 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.
 <dependency>
     <groupId>com.azure</groupId>
     <artifactId>azure-communication-callingserver</artifactId>
-    <version>1.0.0-beta.2</version>
+    <version>1.0.0-beta.3</version>
 </dependency>
 ```
 [//]: # ({x-version-update-end})
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
index 897851e3f8a89..f9008074d900a 100644
--- 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
@@ -7,6 +7,7 @@
 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;
@@ -19,6 +20,10 @@
 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<HttpResponse> process(HttpPipelineCallContext context, HttpPipelineNextPolicy next) {
@@ -26,25 +31,40 @@ public Mono<HttpResponse> process(HttpPipelineCallContext context, HttpPipelineN
     }
 
     private Mono<HttpResponse> attemptRedirection(HttpPipelineCallContext context, HttpPipelineNextPolicy next,
-                                                  int redirectNumber, Set<String> locations) {
+                                                  int redirectNumber, Set<String> attemptedRedirectLocations) {
         return next.clone().process().flatMap(httpResponse -> {
-            if (shouldRedirect(httpResponse, redirectNumber, locations)) {
+            if (isRedirectResponse(httpResponse)
+                && shouldRedirect(httpResponse, context, redirectNumber, attemptedRedirectLocations)) {
                 String newLocation = httpResponse.getHeaderValue(LOCATION_HEADER_NAME);
-                locations.add(newLocation);
+                attemptedRedirectLocations.add(newLocation);
 
                 HttpRequest newRequest = context.getHttpRequest().copy();
                 newRequest.setUrl(newLocation);
                 context.setHttpRequest(newRequest);
 
-                return attemptRedirection(context, next, redirectNumber + 1, locations);
+                return attemptRedirection(context, next, redirectNumber + 1, attemptedRedirectLocations);
             }
             return Mono.just(httpResponse);
         });
     }
 
-    private boolean shouldRedirect(HttpResponse response, int redirectNumber, Set<String> locations) {
-        return response.getStatusCode() == 302
-            && !locations.contains(response.getHeaderValue(LOCATION_HEADER_NAME))
-            && redirectNumber < MAX_REDIRECTS;
+    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<String> 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;
     }
+
 }