Merge "Improve url generation func to cover more cases"

valence/redfish/redfish.py

@@ -38,10 +38,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)
 
 

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")