From 4674935b994fda4ff85a54d44d219489a800db3a Mon Sep 17 00:00:00 2001
From: Lin Yang <lin.a.yang@intel.com>
Date: Wed, 23 Nov 2016 21:24:32 -0800
Subject: [PATCH] Improve url generation func to cover more cases

Support url generation for service entry like without slash, like
"redfish/v1/Systems" and "/Systems". Added more test cases to cover
user input base podm url with tailing slash.

Change-Id: If795ebed42d673c6671408d996a7add0b8178cf1
Closes-Bug: #1649386
---
 valence/redfish/redfish.py                 | 11 +++-
 valence/tests/unit/redfish/test_redfish.py | 59 ++++++++++++++++++++--
 2 files changed, 64 insertions(+), 6 deletions(-)

diff --git a/valence/redfish/redfish.py b/valence/redfish/redfish.py
index f95ffce..f559083 100644
--- a/valence/redfish/redfish.py
+++ b/valence/redfish/redfish.py
@@ -36,10 +36,17 @@ def update_service_root():
 
 
 def get_rfs_url(serviceext):
-    if cfg.redfish_base_ext in serviceext:
+    # Strip slash to make sure all input with/without slash
+    redfish_base_ext = cfg.redfish_base_ext.strip("/")
+    serviceext = serviceext.strip("/")
+
+    # Check whether serviceext statswith redfish_base_ext "redfish/v1", if yes,
+    # use it as relative_url, otherwise add "redfish/v1" before it.
+    if serviceext.startswith(redfish_base_ext):
         relative_url = serviceext
     else:
-        relative_url = os.path.join(cfg.redfish_base_ext, serviceext)
+        relative_url = os.path.normpath(
+            "/".join([redfish_base_ext, serviceext]))
     return requests.compat.urljoin(cfg.podm_url, relative_url)
 
 
diff --git a/valence/tests/unit/redfish/test_redfish.py b/valence/tests/unit/redfish/test_redfish.py
index 8b66b93..b02dfb2 100644
--- a/valence/tests/unit/redfish/test_redfish.py
+++ b/valence/tests/unit/redfish/test_redfish.py
@@ -11,6 +11,7 @@
 #    under the License.
 
 import mock
+from requests.compat import urljoin
 from unittest import TestCase
 
 from valence import config as cfg
@@ -20,16 +21,66 @@ from valence.tests.unit import fakes
 
 class TestRedfish(TestCase):
 
-    def test_get_rfs_url_no_service_ext(self):
-        expected = cfg.podm_url + "/redfish/v1/Systems/1"
+    def test_get_rfs_url(self):
+        cfg.podm_url = "https://127.0.0.1:8443"
+        expected = urljoin(cfg.podm_url, "redfish/v1/Systems/1")
+
+        # test without service_ext
+        result = redfish.get_rfs_url("/Systems/1/")
+        self.assertEqual(expected, result)
+
+        result = redfish.get_rfs_url("/Systems/1")
+        self.assertEqual(expected, result)
+
+        result = redfish.get_rfs_url("Systems/1/")
+        self.assertEqual(expected, result)
+
         result = redfish.get_rfs_url("Systems/1")
         self.assertEqual(expected, result)
 
-    def test_get_rfs_url_with_service_ext(self):
-        expected = cfg.podm_url + "/redfish/v1/Systems/1"
+        # test with service_ext
+        result = redfish.get_rfs_url("/redfish/v1/Systems/1/")
+        self.assertEqual(expected, result)
+
         result = redfish.get_rfs_url("/redfish/v1/Systems/1")
         self.assertEqual(expected, result)
 
+        result = redfish.get_rfs_url("redfish/v1/Systems/1/")
+        self.assertEqual(expected, result)
+
+        result = redfish.get_rfs_url("redfish/v1/Systems/1")
+        self.assertEqual(expected, result)
+
+    def test_get_rfs_url_with_tailing_slash(self):
+        cfg.podm_url = "https://127.0.0.1:8443/"
+        expected = urljoin(cfg.podm_url, "redfish/v1/Systems/1")
+
+        # test without service_ext
+        result = redfish.get_rfs_url("/Systems/1/")
+        self.assertEqual(expected, result)
+
+        result = redfish.get_rfs_url("/Systems/1")
+        self.assertEqual(expected, result)
+
+        result = redfish.get_rfs_url("Systems/1/")
+        self.assertEqual(expected, result)
+
+        result = redfish.get_rfs_url("Systems/1")
+        self.assertEqual(expected, result)
+
+        # test with service_ext
+        result = redfish.get_rfs_url("/redfish/v1/Systems/1/")
+        self.assertEqual(expected, result)
+
+        result = redfish.get_rfs_url("/redfish/v1/Systems/1")
+        self.assertEqual(expected, result)
+
+        result = redfish.get_rfs_url("redfish/v1/Systems/1/")
+        self.assertEqual(expected, result)
+
+        result = redfish.get_rfs_url("redfish/v1/Systems/1")
+        self.assertEqual(expected, result)
+
     @mock.patch('valence.redfish.redfish.send_request')
     def test_get_base_resource_url_chassis(self, mock_request):
         fake_resp = fakes.mock_request_get(fakes.fake_service_root(), "200")