Improve and fix locations and URLs

We are not taking into account the application url (i.e. the scheme://server:port/application) when we are cheking the responses. This change sets an application URL that we should check for each of the expected locations. Moreover, when we were creating the OCCI kinds for each of the objects we were using absolute locations, so change to relative ones.
alvarolopez · Apr 8, 2015 · 0b92038 · 0b92038
1 parent 69ba66b
commit 0b92038
Show file tree

Hide file tree

Showing 11 changed files with 63 additions and 32 deletions.
diff --git a/ooi/occi/core/entity.py b/ooi/occi/core/entity.py
@@ -62,7 +62,7 @@ class Entity(object):
                                                 "occi.core.title"])
 
     kind = kind.Kind(helpers.build_scheme('core'), 'entity',
-                     'entity', attributes, '/entity/')
+                     'entity', attributes, 'entity/')
 
     def __init__(self, title, mixins, id=None):
         helpers.check_type(mixins, mixin.Mixin)

diff --git a/ooi/occi/core/link.py b/ooi/occi/core/link.py
@@ -31,7 +31,7 @@ class Link(entity.Entity):
                                                 "occi.core.target"])
 
     kind = kind.Kind(helpers.build_scheme("core"), 'link', 'link',
-                     attributes, '/link/')
+                     attributes, 'link/')
 
     def __init__(self, title, mixins, source, target):
         super(Link, self).__init__(title, mixins)

diff --git a/ooi/occi/core/resource.py b/ooi/occi/core/resource.py
@@ -36,7 +36,7 @@ class Resource(entity.Entity):
     attributes = attribute.AttributeCollection(["occi.core.summary"])
 
     kind = kind.Kind(helpers.build_scheme('core'), 'resource',
-                     'resource', attributes, '/resource/',
+                     'resource', attributes, 'resource/',
                      related=[entity.Entity.kind])
 
     def __init__(self, title, mixins, id=None, summary=None):

diff --git a/ooi/occi/infrastructure/compute.py b/ooi/occi/infrastructure/compute.py
@@ -42,7 +42,7 @@ class ComputeResource(resource.Resource):
                                            "occi.compute.state"])
     actions = (start, stop, restart, suspend)
     kind = kind.Kind(helpers.build_scheme('infrastructure'), 'compute',
-                     'compute resource', attributes, '/compute/',
+                     'compute resource', attributes, 'compute/',
                      actions=actions,
                      related=[resource.Resource.kind])
 

diff --git a/ooi/occi/rendering/headers.py b/ooi/occi/rendering/headers.py
@@ -55,11 +55,12 @@ class KindRenderer(CategoryRenderer):
 
 
 class ActionRenderer(CategoryRenderer):
-    def render(self, instance=None, env={}):
-        # We have an instance id, render it as a link
-        if instance is not None:
+    def render(self, ass_obj=None, env={}):
+        # We have an associated object, render it as a link to that object
+        if ass_obj is not None:
             url = env.get("application_url", "")
-            url = utils.join_url(url, [instance, self.obj.location])
+            term = ass_obj.kind.term + "/"
+            url = utils.join_url(url, [term, ass_obj.id, self.obj.location])
             d = {"location": url,
                  "rel": self.obj.type_id}
             link = "<%(location)s>; rel=%(rel)s" % d
@@ -80,7 +81,7 @@ def render(self, env={}):
         for what in [self.obj.kinds, self.obj.mixins, self.obj.actions,
                      self.obj.resources, self.obj.links]:
             for el in what:
-                url = app_url + el.location
+                url = utils.join_url(app_url, el.location)
                 ret.append(('X-OCCI-Location', '%s' % url))
         return ret
 
@@ -100,16 +101,18 @@ def render(self, env={}):
 class ResourceRenderer(HeaderRenderer):
     def render(self, env={}):
         ret = []
-        ret.extend(KindRenderer(self.obj.kind).render())
+        ret.extend(KindRenderer(self.obj.kind).render(env=env))
         for m in self.obj.mixins:
-            ret.extend(MixinRenderer(m).render())
+            ret.extend(MixinRenderer(m).render(env=env))
         for a in self.obj.attributes:
             # FIXME(aloga): I dont like this test here
             if self.obj.attributes[a].value is None:
                 continue
-            ret.extend(AttributeRenderer(self.obj.attributes[a]).render())
+            r = AttributeRenderer(self.obj.attributes[a])
+            ret.extend(r.render(env=env))
         for a in self.obj.actions:
-            ret.extend(ActionRenderer(a).render(instance=self.obj.id))
+            r = ActionRenderer(a)
+            ret.extend(r.render(ass_obj=self.obj, env=env))
         for l in self.obj.links:
             pass
             # FIXME(aloga): we need to fix this

diff --git a/ooi/tests/fakes.py b/ooi/tests/fakes.py
@@ -24,6 +24,8 @@
 import ooi.wsgi
 
 
+application_url = "https://foo.example.org:8774/ooiv1"
+
 tenants = {
     "foo": {"id": uuid.uuid4().hex,
             "name": "foo"},

diff --git a/ooi/tests/middleware/test_compute_controller.py b/ooi/tests/middleware/test_compute_controller.py
@@ -20,6 +20,7 @@
 
 from ooi.tests import fakes
 from ooi.tests.middleware import test_middleware
+from ooi import utils
 
 
 def build_occi_server(server):
@@ -58,14 +59,22 @@ def build_occi_server(server):
         'occi.core.id="%s"' % server_id,
     ]
     links = []
-    links.append('<%s?action=restart>; rel=http://schemas.ogf.org/occi/'
-                 'infrastructure/compute/action#restart' % server_id)
-    links.append('<%s?action=start>; rel=http://schemas.ogf.org/occi/'
-                 'infrastructure/compute/action#start' % server_id)
-    links.append('<%s?action=stop>; rel=http://schemas.ogf.org/occi/'
-                 'infrastructure/compute/action#stop' % server_id)
-    links.append('<%s?action=suspend>; rel=http://schemas.ogf.org/occi/'
-                 'infrastructure/compute/action#suspend' % server_id)
+    links.append('<%s/compute/%s?action=restart>; '
+                 'rel=http://schemas.ogf.org/occi/'
+                 'infrastructure/compute/action#restart' %
+                 (fakes.application_url, server_id))
+    links.append('<%s/compute/%s?action=start>; '
+                 'rel=http://schemas.ogf.org/occi/'
+                 'infrastructure/compute/action#start' %
+                 (fakes.application_url, server_id))
+    links.append('<%s/compute/%s?action=stop>; '
+                 'rel=http://schemas.ogf.org/occi/'
+                 'infrastructure/compute/action#stop' %
+                 (fakes.application_url, server_id))
+    links.append('<%s/compute/%s?action=suspend>; '
+                 'rel=http://schemas.ogf.org/occi/'
+                 'infrastructure/compute/action#suspend' %
+                 (fakes.application_url, server_id))
 
     result = []
     for c in cats:
@@ -108,7 +117,11 @@ def test_list_vms_one_vm(self):
         self.assertEqual(200, resp.status_code)
         expected = []
         for s in fakes.servers[tenant["id"]]:
-            expected.append(("X-OCCI-Location", "/compute/%s" % s["id"]))
+            expected.append(
+                ("X-OCCI-Location", utils.join_url(self.application_url + "/",
+                                                   "compute/%s" % s["id"]))
+            )
+        self.assertContentType(resp)
         self.assertExpectedResult(expected, resp)
 
     def test_show_vm(self):
@@ -154,7 +167,9 @@ def test_create_vm(self):
                               headers=headers)
         resp = req.get_response(app)
 
-        expected = [("X-OCCI-Location", "/compute/%s" % "foo")]
+        expected = [("X-OCCI-Location",
+                     utils.join_url(self.application_url + "/",
+                                    "compute/%s" % "foo"))]
         self.assertEqual(200, resp.status_code)
         self.assertExpectedResult(expected, resp)
         self.assertContentType(resp)

diff --git a/ooi/tests/middleware/test_middleware.py b/ooi/tests/middleware/test_middleware.py
@@ -35,6 +35,7 @@ def setUp(self):
         super(TestMiddleware, self).setUp()
 
         self.accept = None
+        self.application_url = fakes.application_url
 
     def get_app(self, resp=None):
         return wsgi.OCCIMiddleware(fakes.FakeApp())
@@ -47,9 +48,7 @@ def assertExpectedResult(self, expected, result):
         expected = ["%s: %s" % e for e in expected]
         # NOTE(aloga): the order of the result does not matter
         results = result.text.splitlines()
-        results.sort()
-        expected.sort()
-        self.assertEqual(expected, results)
+        self.assertItemsEqual(expected, results)
 
     def _build_req(self, path, tenant_id, **kwargs):
         if self.accept is not None:
@@ -59,6 +58,8 @@ def _build_req(self, path, tenant_id, **kwargs):
         m.user.project_id = tenant_id
         environ = {"keystone.token_auth": m}
 
+        kwargs["base_url"] = self.application_url
+
         return webob.Request.blank(path, environ=environ, **kwargs)
 
     def test_404(self):

diff --git a/ooi/utils.py b/ooi/utils.py
@@ -42,5 +42,8 @@ def join_url(base, parts):
         parts = [parts]
 
     for p in parts:
+        if p.startswith("/"):
+            # We won't get an absolute url
+            p = p[1:]
         url = urlparse.urljoin(url, p)
     return url
diff --git a/ooi/wsgi/__init__.py b/ooi/wsgi/__init__.py
@@ -285,7 +285,8 @@ def serialize(self, request, content_type, default_serializers=None):
         else:
             _mtype, _serializer = self.get_serializer(content_type,
                                                       default_serializers)
-            serializer = _serializer()
+            env = {"application_url": request.application_url + "/"}
+            serializer = _serializer(env)
 
         response = webob.Response()
         response.status_int = self.code
@@ -379,7 +380,8 @@ def __call__(self, req):
         mtype = serializers.get_media_map().get(content_type,
                                                 "text")
         serializer = serializers.get_default_serializers()[mtype]
-        serialized_exc = serializer().serialize(self.wrapped_exc)
+        env = {}
+        serialized_exc = serializer(env).serialize(self.wrapped_exc)
         self.wrapped_exc.body = serialized_exc[1]
         self.wrapped_exc.content_type = content_type
 

diff --git a/ooi/wsgi/serializers.py b/ooi/wsgi/serializers.py
@@ -27,7 +27,12 @@
 ])
 
 
-class TextSerializer(object):
+class BaseSerializer(object):
+    def __init__(self, env):
+        self.env = env
+
+
+class TextSerializer(BaseSerializer):
     def serialize(self, data):
         if not isinstance(data, list):
             data = [data]
@@ -36,11 +41,11 @@ def serialize(self, data):
         for d in data:
             renderers.append(text_rendering.get_renderer(d))
 
-        ret = "\n".join([r.render() for r in renderers])
+        ret = "\n".join([r.render(env=self.env) for r in renderers])
         return None, utils.utf8(ret)
 
 
-class HeaderSerializer(object):
+class HeaderSerializer(BaseSerializer):
     def serialize(self, data):
         if not isinstance(data, list):
             data = [data]
@@ -51,7 +56,7 @@ def serialize(self, data):
 
         # Header renderers will return a list, so we must flatten the results
         # before returning them
-        headers = [i for r in renderers for i in r.render()]
+        headers = [i for r in renderers for i in r.render(env=self.env)]
         return headers, ""