From 0b92038f62b3645db44403e53245f84640e356f3 Mon Sep 17 00:00:00 2001 From: Alvaro Lopez Garcia Date: Wed, 8 Apr 2015 16:44:00 +0200 Subject: [PATCH] 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. --- ooi/occi/core/entity.py | 2 +- ooi/occi/core/link.py | 2 +- ooi/occi/core/resource.py | 2 +- ooi/occi/infrastructure/compute.py | 2 +- ooi/occi/rendering/headers.py | 21 ++++++----- ooi/tests/fakes.py | 2 ++ .../middleware/test_compute_controller.py | 35 +++++++++++++------ ooi/tests/middleware/test_middleware.py | 7 ++-- ooi/utils.py | 3 ++ ooi/wsgi/__init__.py | 6 ++-- ooi/wsgi/serializers.py | 13 ++++--- 11 files changed, 63 insertions(+), 32 deletions(-) diff --git a/ooi/occi/core/entity.py b/ooi/occi/core/entity.py index 636dc4d..35fb778 100644 --- 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 index 0ccaa2d..005b787 100644 --- 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 index cf54dae..7dfcc3c 100644 --- 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 index 5f078fd..6a93272 100644 --- 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 index fcded22..49945be 100644 --- 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 @@ class CollectionRenderer(HeaderRenderer): 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 @@ class AttributeRenderer(HeaderRenderer): 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 index 2184d5b..114f716 100644 --- a/ooi/tests/fakes.py +++ b/ooi/tests/fakes.py @@ -24,6 +24,8 @@ from ooi import utils 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 index 516f6a8..3915cb5 100644 --- a/ooi/tests/middleware/test_compute_controller.py +++ b/ooi/tests/middleware/test_compute_controller.py @@ -20,6 +20,7 @@ import mock 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 @@ class TestComputeController(test_middleware.TestMiddleware): 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 @@ class TestComputeController(test_middleware.TestMiddleware): 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 index 7763f8a..7292f34 100644 --- a/ooi/tests/middleware/test_middleware.py +++ b/ooi/tests/middleware/test_middleware.py @@ -35,6 +35,7 @@ class TestMiddleware(base.TestCase): 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 @@ class TestMiddleware(base.TestCase): 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 @@ class TestMiddleware(base.TestCase): 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 index 271a4b6..4ec94eb 100644 --- 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 index fba2231..b51d8e4 100644 --- a/ooi/wsgi/__init__.py +++ b/ooi/wsgi/__init__.py @@ -285,7 +285,8 @@ class ResponseObject(object): 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 @@ class Fault(webob.exc.HTTPException): 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 index f6750e2..c4459b1 100644 --- a/ooi/wsgi/serializers.py +++ b/ooi/wsgi/serializers.py @@ -27,7 +27,12 @@ _MEDIA_TYPE_MAP = collections.OrderedDict([ ]) -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 @@ class TextSerializer(object): 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 @@ class HeaderSerializer(object): # 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, ""