From 69ba66b8303031d83bb23a9bb31b450412a2050b Mon Sep 17 00:00:00 2001 From: Alvaro Lopez Garcia Date: Wed, 8 Apr 2015 11:12:46 +0200 Subject: [PATCH] Preserve original request when calling OpenStack This change creates a new request instead of modifying the original one when it needs to be modified so as to access OpenStack. Before we were modifying the original request (i.e. the path, body and content_type) and this is wrong, since we cannot access those attributes after it has been modified. --- ooi/api/base.py | 27 ++++-- .../middleware/test_compute_controller.py | 4 - ooi/tests/test_base_controller.py | 87 +++++++++++++++++++ 3 files changed, 109 insertions(+), 9 deletions(-) create mode 100644 ooi/tests/test_base_controller.py diff --git a/ooi/api/base.py b/ooi/api/base.py index 25ef4a6..503d30e 100644 --- a/ooi/api/base.py +++ b/ooi/api/base.py @@ -14,6 +14,8 @@ # License for the specific language governing permissions and limitations # under the License. +import copy + from ooi import utils import webob.exc @@ -25,14 +27,29 @@ class Controller(object): self.openstack_version = openstack_version def _get_req(self, req, path=None, content_type=None, body=None): - req.script_name = self.openstack_version + """Return a new Request object to interact with OpenStack. + + This method will create a new request starting with the same WSGI + environment as the original request, prepared to interact with + OpenStack. Namely, it will override the script name to match the + OpenStack version. It will also override the path, content_type and + body of the request, if any of those keyword arguments are passed. + + :param req: the original request + :param path: new path for the request + :param content_type: new content type for the request + :param body: new body for the request + :returns: a Request object + """ + new_req = webob.Request(copy.copy(req.environ)) + new_req.script_name = self.openstack_version if path is not None: - req.path_info = path + new_req.path_info = path if content_type is not None: - req.content_type = content_type + new_req.content_type = content_type if body is not None: - req.body = utils.utf8(body) - return req + new_req.body = utils.utf8(body) + return new_req @staticmethod def get_from_response(response, element, default): diff --git a/ooi/tests/middleware/test_compute_controller.py b/ooi/tests/middleware/test_compute_controller.py index 95abf44..516f6a8 100644 --- a/ooi/tests/middleware/test_compute_controller.py +++ b/ooi/tests/middleware/test_compute_controller.py @@ -92,8 +92,6 @@ class TestComputeController(test_middleware.TestMiddleware): resp = req.get_response(app) - self.assertEqual("/%s/servers" % tenant["id"], req.path_info) - expected_result = "" self.assertContentType(resp) self.assertExpectedResult(expected_result, resp) @@ -107,8 +105,6 @@ class TestComputeController(test_middleware.TestMiddleware): resp = req.get_response(app) - self.assertEqual("/%s/servers" % tenant["id"], req.path_info) - self.assertEqual(200, resp.status_code) expected = [] for s in fakes.servers[tenant["id"]]: diff --git a/ooi/tests/test_base_controller.py b/ooi/tests/test_base_controller.py new file mode 100644 index 0000000..9409e00 --- /dev/null +++ b/ooi/tests/test_base_controller.py @@ -0,0 +1,87 @@ +# -*- coding: utf-8 -*- + +# Copyright 2015 Spanish National Research Council +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import json + +import webob +import webob.exc + +import ooi.api.base +from ooi.tests import base +from ooi import utils + + +class TestController(base.TestCase): + def test_controller(self): + class Foo(object): + pass + controller = ooi.api.base.Controller(Foo(), "version") + self.assertIsInstance(controller.app, Foo) + self.assertEqual("version", controller.openstack_version) + + def test_new_request(self): + controller = ooi.api.base.Controller(None, "version") + req = webob.Request.blank("foo") + new_req = controller._get_req(req) + self.assertEqual("version", new_req.script_name) + self.assertEqual("foo", new_req.path_info) + self.assertIsNot(req, new_req) + + def test_new_request_with_path(self): + controller = ooi.api.base.Controller(None, "version") + req = webob.Request.blank("foo") + new_req = controller._get_req(req, path="bar") + self.assertEqual("bar", new_req.path_info) + + def test_new_request_with_body(self): + controller = ooi.api.base.Controller(None, "version") + req = webob.Request.blank("foo") + new_req = controller._get_req(req, body="bar") + self.assertEqual(utils.utf8("bar"), new_req.body) + + def test_new_request_with_content_type(self): + controller = ooi.api.base.Controller(None, "version") + req = webob.Request.blank("foo") + new_req = controller._get_req(req, content_type="foo/bar") + self.assertEqual("foo/bar", new_req.content_type) + + def test_get_from_response(self): + d = {"element": {"foo": "bar"}} + body = json.dumps(d) + response = webob.Response(status=200, body=body) + result = ooi.api.base.Controller.get_from_response(response, + "element", + {}) + self.assertEqual(d["element"], result) + + def test_get_from_response_with_default(self): + d = {"element": {"foo": "bar"}} + body = json.dumps({}) + response = webob.Response(status=200, body=body) + result = ooi.api.base.Controller.get_from_response(response, + "element", + d["element"]) + self.assertEqual(d["element"], result) + + def test_get_from_response_with_exception(self): + d = {"unauthorized": {"message": "unauthorized"}} + body = json.dumps(d) + response = webob.Response(status=403, body=body) + self.assertRaises(webob.exc.HTTPForbidden, + ooi.api.base.Controller.get_from_response, + response, + "foo", + {})