From 36c8237a1c868f2a702a7c52cfd34f4e0150c197 Mon Sep 17 00:00:00 2001
From: Dimitri Stiliadis <dstiliadis@users.noreply.github.com>
Date: Sun, 4 Aug 2019 18:16:55 -0700
Subject: [PATCH] Fix retires (#96)

---
 maniphttp/manipulator.go      |  8 ++++---
 maniphttp/manipulator_test.go | 42 ++++++++++++++++++++++++++++++++++-
 2 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/maniphttp/manipulator.go b/maniphttp/manipulator.go
index 49d5914b..bd9e8cdf 100644
--- a/maniphttp/manipulator.go
+++ b/maniphttp/manipulator.go
@@ -552,8 +552,9 @@ func (s *httpManipulator) send(
 	sp opentracing.Span,
 ) (*http.Response, error) {
 
-	var try int         // try number. Starts at 0
-	var lastError error // last error before retry.
+	var try int               // try number. Starts at 0
+	var lastError error       // last error before retry.
+	var tokenRenewedOnce bool // after an authorization failures token is renewed at most once.
 
 	// We get the context deadline.
 	deadline, ok := mctx.Context().Deadline()
@@ -701,7 +702,7 @@ func (s *httpManipulator) send(
 				return nil, err
 			}
 
-			if s.tokenManager != nil && (response.StatusCode == http.StatusForbidden || response.StatusCode == http.StatusUnauthorized) {
+			if !tokenRenewedOnce && s.tokenManager != nil && (response.StatusCode == http.StatusForbidden || response.StatusCode == http.StatusUnauthorized) {
 
 				// This is a special case where we try to renew our token
 				// in case of 401 or 403 error.
@@ -713,6 +714,7 @@ func (s *httpManipulator) send(
 					err := s.atomicRenewTokenFunc(mctx.Context())
 					if err == nil {
 						lastError = errs
+						tokenRenewedOnce = true
 						goto RETRY
 					}
 				}
diff --git a/maniphttp/manipulator_test.go b/maniphttp/manipulator_test.go
index 034875d8..a4db6f4c 100644
--- a/maniphttp/manipulator_test.go
+++ b/maniphttp/manipulator_test.go
@@ -1409,7 +1409,7 @@ func TestHTTP_send(t *testing.T) {
 		})
 	})
 
-	Convey("Given I have a server returning 403 and with a token manager", t, func() {
+	Convey("Given I have a server returning 403 and with a token manager that renews the token", t, func() {
 
 		ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			if r.Header.Get("Authorization") != "Bearer ok-token" {
@@ -1509,6 +1509,46 @@ func TestHTTP_send(t *testing.T) {
 		})
 	})
 
+	Convey("Given I have a server returning 403 for all my requests, I should return an error and tokenmanager should only be invoked once", t, func() {
+
+		ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.Header().Set("Content-Type", "application/json")
+			w.WriteHeader(http.StatusForbidden)
+			fmt.Fprint(w, `[{"code": 403, "title": "nope", "description": "boom"}]`)
+		}))
+		defer ts.Close()
+
+		tm := maniptest.NewTestTokenManager()
+		var tmCalled int64
+		tm.MockIssue(t, func(context.Context) (string, error) {
+			atomic.AddInt64(&tmCalled, 1)
+			time.Sleep(300 * time.Millisecond)
+			return "ok-token", nil
+		})
+
+		m, _ := New(context.Background(), "toto.com")
+		m.(*httpManipulator).tokenManager = tm
+		m.(*httpManipulator).username = "Bearer"
+		m.(*httpManipulator).password = "ok-token"
+		m.(*httpManipulator).atomicRenewTokenFunc = elemental.AtomicJob(m.(*httpManipulator).renewToken)
+
+		Convey("When I call send", func() {
+
+			ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
+			defer cancel()
+
+			_, err := m.(*httpManipulator).send(manipulate.NewContext(ctx), http.MethodPost, ts.URL, nil, nil, sp)
+
+			Convey("Then err should be not nil", func() {
+				So(err, ShouldNotBeNil)
+			})
+
+			Convey("Then the token manager should have been called only once", func() {
+				So(tmCalled, ShouldEqual, 1)
+			})
+		})
+	})
+
 	Convey("Given I have a server returning 423", t, func() {
 
 		m, _ := New(context.Background(), "toto.com")