From abfe901b5eaa995dc92d5e7e065e708c8172e422 Mon Sep 17 00:00:00 2001
From: Michael Krotscheck <krotscheck@gmail.com>
Date: Tue, 13 Sep 2016 21:55:26 -0700
Subject: [PATCH] AbstractService will try to detect the versions resource in
 more places.

We have no guarantees that the keystone service catalog will have the
root resource of any given service registered. As most versioned API
endpoints require tokens, we can reasonably assume that a 401 will
be encountered. This patch adds an extra check against the response
from the provided URL, and should a 401 be encountered, attempts
to resolve the versions from the root resource of the provided URL.

Change-Id: I655409f0eb9bfbd3489827db46faef026ede82f9
---
 package.json                           |  3 ++-
 src/util/abstract_service.js           | 25 +++++++++++++++++----
 test/unit/helpers/data/versions.js     | 23 ++++++++++++++++++--
 test/unit/util/abstract_serviceTest.js | 30 ++++++++++++++++++++++++++
 4 files changed, 74 insertions(+), 7 deletions(-)

diff --git a/package.json b/package.json
index dac6096..dbfe35b 100644
--- a/package.json
+++ b/package.json
@@ -31,7 +31,8 @@
   "dependencies": {
     "babel-runtime": "^6.11.6",
     "isomorphic-fetch": "^2.2.1",
-    "loglevel": "^1.4.1"
+    "loglevel": "^1.4.1",
+    "url-parse": "^1.1.3"
   },
   "devDependencies": {
     "babel-cli": "^6.10.1",
diff --git a/src/util/abstract_service.js b/src/util/abstract_service.js
index 8a8e425..aba087f 100644
--- a/src/util/abstract_service.js
+++ b/src/util/abstract_service.js
@@ -15,6 +15,7 @@
  */
 
 import Http from './http';
+import URL from 'url-parse';
 
 export default class AbstractService {
 
@@ -57,10 +58,26 @@ export default class AbstractService {
    * @returns {Promise.<T>} A promise that will resolve with the list of API versions.
    */
   versions () {
-    return this.http
-      .httpGet(this._endpointUrl)
-      .then((response) => response.json())
-      .then((body) => body.versions);
+    return new Promise((resolve, reject) => {
+      let promise = this.http
+        .httpGet(this._endpointUrl)
+        .catch((response) => {
+          if (response.status === 401) {
+            let rootUrl = new URL(this._endpointUrl);
+            rootUrl.set('pathname', '/');
+            rootUrl.set('query', '');
+            rootUrl.set('hash', '');
+
+            return this.http.httpGet(rootUrl.href);
+          } else {
+            return reject(response);
+          }
+        });
+
+      promise
+        .then((response) => response.json())
+        .then((body) => resolve(body.versions));
+    });
   }
 
   /**
diff --git a/test/unit/helpers/data/versions.js b/test/unit/helpers/data/versions.js
index 47e8d97..4e160cf 100644
--- a/test/unit/helpers/data/versions.js
+++ b/test/unit/helpers/data/versions.js
@@ -23,6 +23,7 @@
  * URLs to match the test data below.
  */
 const rootUrl = "http://example.com/";
+const subUrl = `${rootUrl}/v1`;
 
 /**
  * A mock list of supported versions for the below requests.
@@ -36,7 +37,7 @@ const versions = [
 /**
  * Build a new FetchMock configuration for the versions (root) endpoint.
  *
- * @returns {{}} A full FetchMock configuration for Glance's Root Resource.
+ * @returns {{}} A full FetchMock configuration for a versions resource.
  */
 function rootResponse() {
   return {
@@ -109,8 +110,26 @@ function rootResponse() {
   };
 }
 
+/**
+ * FetchMock configuration for a 401 response against the versioned API url.
+ *
+ * @param {int} httpStatus The HTTP status for the response.
+ * @returns {{}} A full FetchMock configuration a failing request..
+ */
+function subResponse(httpStatus = 401) {
+  return {
+    method: 'GET',
+    matcher: subUrl,
+    response: {
+      status: httpStatus
+    }
+  };
+}
+
 export {
   rootUrl,
+  subUrl,
   versions,
-  rootResponse
+  rootResponse,
+  subResponse
 };
diff --git a/test/unit/util/abstract_serviceTest.js b/test/unit/util/abstract_serviceTest.js
index 7c274a8..8dd88b1 100644
--- a/test/unit/util/abstract_serviceTest.js
+++ b/test/unit/util/abstract_serviceTest.js
@@ -73,6 +73,36 @@ describe('AbstractService', () => {
         .catch((error) => done.fail(error));
     });
 
+    // This test catches the case when the service catalog already points
+    // at an API version.
+    it("Should retry at the root URL if a 401 is encountered", (done) => {
+      const service = new AbstractService(mockData.subUrl, mockData.versions);
+
+      fetchMock.mock(mockData.subResponse());
+      fetchMock.mock(mockData.rootResponse());
+
+      service.versions()
+        .then((versions) => {
+          // Quick sanity check.
+          expect(versions.length).toBe(6);
+          done();
+        })
+        .catch((error) => done.fail(error));
+    });
+
+    it("Should not retry at the root URL if a different status is encountered", (done) => {
+      const service = new AbstractService(mockData.subUrl, mockData.versions);
+
+      fetchMock.mock(mockData.subResponse(500));
+
+      service.versions()
+        .then((response) => done.fail(response))
+        .catch((error) => {
+          expect(error).not.toBeNull();
+          done();
+        });
+    });
+
     it("Should NOT cache its results", (done) => {
       const service = new AbstractService(mockData.rootUrl, mockData.versions);
       const mockOptions = mockData.rootResponse();