From b9f8ef307924b50c1f397fbfc976faccc1fe250a Mon Sep 17 00:00:00 2001 From: Chris Dent Date: Tue, 29 Jul 2014 15:24:19 +0100 Subject: [PATCH] Use a FakeRequest object to test middleware swift was being imported to provide a blank Request object but very little of the functionality in that object was being used. FakeRequest provides the bare minimum funcionality required: * REQUEST_METHOD * PATH_INFO * wsgi.input based on cStringIO * custom header manipulation Other functionality from swift has either been duplicated or replaced: * InputProxy (which counts bytes of request bodies) has been duplicated. * logging uses common log functionality * path_split does straight string splits for the desired results Note that these changes do nothing to change functionality nor anything to address the performance concerns being evaluated and discussed elsewhere. Closes bug: 1285388 Change-Id: If0fbb8f00765ac915e5b426a3661492f4b4df9f4 --- ceilometer/objectstore/swift_middleware.py | 82 +++++++++++------ .../objectstore/test_swift_middleware.py | 87 +++++++++++-------- test-requirements-py3.txt | 1 - test-requirements.txt | 1 - 4 files changed, 105 insertions(+), 66 deletions(-) diff --git a/ceilometer/objectstore/swift_middleware.py b/ceilometer/objectstore/swift_middleware.py index 751b06873..f7b8a016d 100644 --- a/ceilometer/objectstore/swift_middleware.py +++ b/ceilometer/objectstore/swift_middleware.py @@ -41,37 +41,52 @@ before "proxy-server" and add the following filter in the file: from __future__ import absolute_import -from swift.common import utils - -try: - # Swift >= 1.7.5 - import swift.common.swob - REQUEST = swift.common.swob -except ImportError: - import webob - REQUEST = webob - -try: - # Swift > 1.7.5 ... module exists but doesn't contain class. - InputProxy = utils.InputProxy -except AttributeError: - # Swift <= 1.7.5 ... module exists and has class. - from swift.common.middleware import proxy_logging - InputProxy = proxy_logging.InputProxy - from ceilometer.openstack.common import context +from ceilometer.openstack.common import log from ceilometer.openstack.common import timeutils from ceilometer import pipeline from ceilometer import sample from ceilometer import service +LOG = log.getLogger(__name__) + + +class InputProxy(object): + """File-like object that counts bytes read. + + To be swapped in for wsgi.input for accounting purposes. + Borrowed from swift.common.utils. Duplidated here to avoid + dependency on swift package. + """ + def __init__(self, wsgi_input): + self.wsgi_input = wsgi_input + self.bytes_received = 0 + + def read(self, *args, **kwargs): + """Pass read request to the underlying file-like object + + Add bytes read to total. + """ + chunk = self.wsgi_input.read(*args, **kwargs) + self.bytes_received += len(chunk) + return chunk + + def readline(self, *args, **kwargs): + """Pass readline request to the underlying file-like object + + Add bytes read to total. + """ + line = self.wsgi_input.readline(*args, **kwargs) + self.bytes_received += len(line) + return line + + class CeilometerMiddleware(object): """Ceilometer middleware used for counting requests.""" def __init__(self, app, conf): self.app = app - self.logger = utils.get_logger(conf, log_route='ceilometer') self.metadata_headers = [h.strip().replace('-', '_').lower() for h in conf.get( @@ -116,7 +131,7 @@ class CeilometerMiddleware(object): input_proxy.bytes_received, bytes_sent) except Exception: - self.logger.exception('Failed to publish samples') + LOG.exception('Failed to publish samples') try: iterable = self.app(env, my_start_response) @@ -127,24 +142,37 @@ class CeilometerMiddleware(object): return iter_response(iterable) def publish_sample(self, env, bytes_received, bytes_sent): - req = REQUEST.Request(env) + path = env['PATH_INFO'] + method = env['REQUEST_METHOD'] + headers = dict((header.strip('HTTP_'), env[header]) for header + in env if header.startswith('HTTP_')) + try: - version, account, container, obj = utils.split_path(req.path, 2, - 4, True) + container = obj = None + version, account, remainder = path.replace( + '/', '', 1).split('/', 2) + if not version or not account: + raise ValueError('Invalid path: %s' % path) + if remainder: + if '/' in remainder: + container, obj = remainder.split('/', 1) + else: + container = remainder except ValueError: return + now = timeutils.utcnow().isoformat() resource_metadata = { - "path": req.path, + "path": path, "version": version, "container": container, "object": obj, } for header in self.metadata_headers: - if header.upper() in req.headers: - resource_metadata['http_header_%s' % header] = req.headers.get( + if header.upper() in headers: + resource_metadata['http_header_%s' % header] = headers.get( header.upper()) with self.pipeline_manager.publisher( @@ -175,7 +203,7 @@ class CeilometerMiddleware(object): # publish the event for each request # request method will be recorded in the metadata - resource_metadata['method'] = req.method.lower() + resource_metadata['method'] = method.lower() publisher([sample.Sample( name='storage.api.request', type=sample.TYPE_DELTA, diff --git a/ceilometer/tests/objectstore/test_swift_middleware.py b/ceilometer/tests/objectstore/test_swift_middleware.py index db770ae6a..c31cee685 100644 --- a/ceilometer/tests/objectstore/test_swift_middleware.py +++ b/ceilometer/tests/objectstore/test_swift_middleware.py @@ -19,14 +19,6 @@ import mock import six -try: - # Swift >= 1.7.5 - import swift.common.swob - REQUEST = swift.common.swob -except ImportError: - import webob - REQUEST = webob - from ceilometer.objectstore import swift_middleware from ceilometer.openstack.common.fixture import config from ceilometer.openstack.common.fixture import mockpatch @@ -51,6 +43,27 @@ class FakeApp(object): yield line +class FakeRequest(object): + """A bare bones request object + + The middleware will inspect this for request method, + wsgi.input and headers. + """ + + def __init__(self, path, environ=None, headers=None): + environ = environ or {} + headers = headers or {} + + environ['PATH_INFO'] = path + + if 'wsgi.input' not in environ: + environ['wsgi.input'] = six.moves.cStringIO('') + + for header, value in headers.iteritems(): + environ['HTTP_%s' % header.upper()] = value + self.environ = environ + + class TestSwiftMiddleware(tests_base.BaseTestCase): class _faux_pipeline_manager(pipeline.PipelineManager): @@ -86,8 +99,8 @@ class TestSwiftMiddleware(tests_base.BaseTestCase): def test_get(self): app = swift_middleware.CeilometerMiddleware(FakeApp(), {}) - req = REQUEST.Request.blank('/1.0/account/container/obj', - environ={'REQUEST_METHOD': 'GET'}) + req = FakeRequest('/1.0/account/container/obj', + environ={'REQUEST_METHOD': 'GET'}) resp = app(req.environ, self.start_response) self.assertEqual(["This string is 28 bytes long"], list(resp)) samples = self.pipeline_manager.pipelines[0].samples @@ -106,7 +119,7 @@ class TestSwiftMiddleware(tests_base.BaseTestCase): def test_put(self): app = swift_middleware.CeilometerMiddleware(FakeApp(body=['']), {}) - req = REQUEST.Request.blank( + req = FakeRequest( '/1.0/account/container/obj', environ={'REQUEST_METHOD': 'PUT', 'wsgi.input': @@ -128,7 +141,7 @@ class TestSwiftMiddleware(tests_base.BaseTestCase): def test_post(self): app = swift_middleware.CeilometerMiddleware(FakeApp(body=['']), {}) - req = REQUEST.Request.blank( + req = FakeRequest( '/1.0/account/container/obj', environ={'REQUEST_METHOD': 'POST', 'wsgi.input': six.moves.cStringIO('some other stuff')}) @@ -149,8 +162,8 @@ class TestSwiftMiddleware(tests_base.BaseTestCase): def test_head(self): app = swift_middleware.CeilometerMiddleware(FakeApp(body=['']), {}) - req = REQUEST.Request.blank('/1.0/account/container/obj', - environ={'REQUEST_METHOD': 'HEAD'}) + req = FakeRequest('/1.0/account/container/obj', + environ={'REQUEST_METHOD': 'HEAD'}) list(app(req.environ, self.start_response)) samples = self.pipeline_manager.pipelines[0].samples self.assertEqual(1, len(samples)) @@ -166,8 +179,8 @@ class TestSwiftMiddleware(tests_base.BaseTestCase): def test_bogus_request(self): """Test even for arbitrary request method, this will still work.""" app = swift_middleware.CeilometerMiddleware(FakeApp(body=['']), {}) - req = REQUEST.Request.blank('/1.0/account/container/obj', - environ={'REQUEST_METHOD': 'BOGUS'}) + req = FakeRequest('/1.0/account/container/obj', + environ={'REQUEST_METHOD': 'BOGUS'}) list(app(req.environ, self.start_response)) samples = self.pipeline_manager.pipelines[0].samples @@ -183,8 +196,8 @@ class TestSwiftMiddleware(tests_base.BaseTestCase): def test_get_container(self): app = swift_middleware.CeilometerMiddleware(FakeApp(), {}) - req = REQUEST.Request.blank('/1.0/account/container', - environ={'REQUEST_METHOD': 'GET'}) + req = FakeRequest('/1.0/account/container', + environ={'REQUEST_METHOD': 'GET'}) list(app(req.environ, self.start_response)) samples = self.pipeline_manager.pipelines[0].samples self.assertEqual(2, len(samples)) @@ -196,8 +209,8 @@ class TestSwiftMiddleware(tests_base.BaseTestCase): def test_no_metadata_headers(self): app = swift_middleware.CeilometerMiddleware(FakeApp(), {}) - req = REQUEST.Request.blank('/1.0/account/container', - environ={'REQUEST_METHOD': 'GET'}) + req = FakeRequest('/1.0/account/container', + environ={'REQUEST_METHOD': 'GET'}) list(app(req.environ, self.start_response)) samples = self.pipeline_manager.pipelines[0].samples self.assertEqual(2, len(samples)) @@ -213,10 +226,10 @@ class TestSwiftMiddleware(tests_base.BaseTestCase): app = swift_middleware.CeilometerMiddleware(FakeApp(), { 'metadata_headers': 'X_VAR1, x-var2, x-var3' }) - req = REQUEST.Request.blank('/1.0/account/container', - environ={'REQUEST_METHOD': 'GET'}, - headers={'X_VAR1': 'value1', - 'X_VAR2': 'value2'}) + req = FakeRequest('/1.0/account/container', + environ={'REQUEST_METHOD': 'GET'}, + headers={'X_VAR1': 'value1', + 'X_VAR2': 'value2'}) list(app(req.environ, self.start_response)) samples = self.pipeline_manager.pipelines[0].samples self.assertEqual(2, len(samples)) @@ -237,8 +250,8 @@ class TestSwiftMiddleware(tests_base.BaseTestCase): app = swift_middleware.CeilometerMiddleware(FakeApp(), { 'metadata_headers': 'x-var3' }) - req = REQUEST.Request.blank('/1.0/account/container', - environ={'REQUEST_METHOD': 'GET'}) + req = FakeRequest('/1.0/account/container', + environ={'REQUEST_METHOD': 'GET'}) list(app(req.environ, self.start_response)) samples = self.pipeline_manager.pipelines[0].samples self.assertEqual(2, len(samples)) @@ -252,15 +265,15 @@ class TestSwiftMiddleware(tests_base.BaseTestCase): def test_bogus_path(self): app = swift_middleware.CeilometerMiddleware(FakeApp(), {}) - req = REQUEST.Request.blank('/5.0//', - environ={'REQUEST_METHOD': 'GET'}) + req = FakeRequest('/5.0//', + environ={'REQUEST_METHOD': 'GET'}) list(app(req.environ, self.start_response)) samples = self.pipeline_manager.pipelines[0].samples self.assertEqual(0, len(samples)) def test_missing_resource_id(self): app = swift_middleware.CeilometerMiddleware(FakeApp(), {}) - req = REQUEST.Request.blank('/v1/', environ={'REQUEST_METHOD': 'GET'}) + req = FakeRequest('/v1/', environ={'REQUEST_METHOD': 'GET'}) list(app(req.environ, self.start_response)) samples = self.pipeline_manager.pipelines[0].samples self.assertEqual(0, len(samples)) @@ -270,8 +283,8 @@ class TestSwiftMiddleware(tests_base.BaseTestCase): def test_publish_sample_fail(self, mocked_publish_sample): mocked_publish_sample.side_effect = Exception("a exception") app = swift_middleware.CeilometerMiddleware(FakeApp(body=["test"]), {}) - req = REQUEST.Request.blank('/1.0/account/container', - environ={'REQUEST_METHOD': 'GET'}) + req = FakeRequest('/1.0/account/container', + environ={'REQUEST_METHOD': 'GET'}) resp = list(app(req.environ, self.start_response)) samples = self.pipeline_manager.pipelines[0].samples self.assertEqual(0, len(samples)) @@ -281,8 +294,8 @@ class TestSwiftMiddleware(tests_base.BaseTestCase): def test_reseller_prefix(self): # No reseller prefix set: ensure middleware uses AUTH_ app = swift_middleware.CeilometerMiddleware(FakeApp(), {}) - req = REQUEST.Request.blank('/1.0/AUTH_account/container/obj', - environ={'REQUEST_METHOD': 'GET'}) + req = FakeRequest('/1.0/AUTH_account/container/obj', + environ={'REQUEST_METHOD': 'GET'}) list(app(req.environ, self.start_response)) samples = self.pipeline_manager.pipelines[0].samples[0] self.assertEqual("account", samples.resource_id) @@ -290,8 +303,8 @@ class TestSwiftMiddleware(tests_base.BaseTestCase): # Custom reseller prefix set app = swift_middleware.CeilometerMiddleware( FakeApp(), {'reseller_prefix': 'CUSTOM_'}) - req = REQUEST.Request.blank('/1.0/CUSTOM_account/container/obj', - environ={'REQUEST_METHOD': 'GET'}) + req = FakeRequest('/1.0/CUSTOM_account/container/obj', + environ={'REQUEST_METHOD': 'GET'}) list(app(req.environ, self.start_response)) samples = self.pipeline_manager.pipelines[0].samples[0] self.assertEqual("account", samples.resource_id) @@ -300,8 +313,8 @@ class TestSwiftMiddleware(tests_base.BaseTestCase): # Custom reseller prefix set, but without trailing underscore app = swift_middleware.CeilometerMiddleware( FakeApp(), {'reseller_prefix': 'CUSTOM'}) - req = REQUEST.Request.blank('/1.0/CUSTOM_account/container/obj', - environ={'REQUEST_METHOD': 'GET'}) + req = FakeRequest('/1.0/CUSTOM_account/container/obj', + environ={'REQUEST_METHOD': 'GET'}) list(app(req.environ, self.start_response)) samples = self.pipeline_manager.pipelines[0].samples[0] self.assertEqual("account", samples.resource_id) diff --git a/test-requirements-py3.txt b/test-requirements-py3.txt index 68b576036..b60082f15 100644 --- a/test-requirements-py3.txt +++ b/test-requirements-py3.txt @@ -5,7 +5,6 @@ coverage>=3.6 discover fixtures>=0.3.14 httplib2>=0.7.5 -http://tarballs.openstack.org/swift/swift-master.tar.gz#egg=swift mock>=1.0 mox>=0.5.3 # Docs Requirements diff --git a/test-requirements.txt b/test-requirements.txt index 0da0d11b2..45a717ce3 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -5,7 +5,6 @@ coverage>=3.6 discover fixtures>=0.3.14 httplib2>=0.7.5 -http://tarballs.openstack.org/swift/swift-master.tar.gz#egg=swift mock>=1.0 mox>=0.5.3 MySQL-python