From 433cf95e060fedfbde470a492d8924e22673939f Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Fri, 31 Jul 2015 20:01:20 +0200 Subject: [PATCH] Adapt SnapshotController to view builder Snapshots were not using the view builder to translate responses, unlike other resources like volumes and backups. This patch moves snapshots to use the view builder like the rest. Change-Id: Iec6873aedcf126f00c81fd016acfc8db9a5e512c --- cinder/api/v2/snapshots.py | 64 +++++++----------- cinder/api/views/snapshots.py | 75 ++++++++++++++++++++++ cinder/tests/unit/api/v2/test_snapshots.py | 29 +++------ 3 files changed, 107 insertions(+), 61 deletions(-) create mode 100644 cinder/api/views/snapshots.py diff --git a/cinder/api/v2/snapshots.py b/cinder/api/v2/snapshots.py index 010549df0..8bb815210 100644 --- a/cinder/api/v2/snapshots.py +++ b/cinder/api/v2/snapshots.py @@ -22,6 +22,7 @@ from webob import exc from cinder.api import common from cinder.api.openstack import wsgi +from cinder.api.views import snapshots as snapshot_views from cinder.api import xmlutil from cinder import exception from cinder.i18n import _, _LI @@ -33,35 +34,6 @@ from cinder.volume import utils as volume_utils LOG = logging.getLogger(__name__) -def _translate_snapshot_detail_view(snapshot): - """Maps keys for snapshots details view.""" - - d = _translate_snapshot_summary_view(snapshot) - - # NOTE(gagupta): No additional data / lookups at the moment - return d - - -def _translate_snapshot_summary_view(snapshot): - """Maps keys for snapshots summary view.""" - d = {} - - d['id'] = snapshot['id'] - d['created_at'] = snapshot['created_at'] - d['name'] = snapshot['display_name'] - d['description'] = snapshot['display_description'] - d['volume_id'] = snapshot['volume_id'] - d['status'] = snapshot['status'] - d['size'] = snapshot['volume_size'] - - if snapshot.get('metadata') and isinstance(snapshot.get('metadata'), - dict): - d['metadata'] = snapshot['metadata'] - else: - d['metadata'] = {} - return d - - def make_snapshot(elem): elem.set('id') elem.set('status') @@ -92,6 +64,8 @@ class SnapshotsTemplate(xmlutil.TemplateBuilder): class SnapshotsController(wsgi.Controller): """The Snapshots API controller for the OpenStack API.""" + _view_builder_class = snapshot_views.ViewBuilder + def __init__(self, ext_mgr=None): self.volume_api = volume.API() self.ext_mgr = ext_mgr @@ -108,7 +82,7 @@ class SnapshotsController(wsgi.Controller): except exception.SnapshotNotFound as error: raise exc.HTTPNotFound(explanation=error.msg) - return {'snapshot': _translate_snapshot_detail_view(snapshot)} + return self._view_builder.detail(req, snapshot) def delete(self, req, id): """Delete a snapshot.""" @@ -127,23 +101,23 @@ class SnapshotsController(wsgi.Controller): @wsgi.serializers(xml=SnapshotsTemplate) def index(self, req): """Returns a summary list of snapshots.""" - return self._items(req, entity_maker=_translate_snapshot_summary_view) + return self._items(req, is_detail=False) @wsgi.serializers(xml=SnapshotsTemplate) def detail(self, req): """Returns a detailed list of snapshots.""" - return self._items(req, entity_maker=_translate_snapshot_detail_view) + return self._items(req, is_detail=True) - def _items(self, req, entity_maker): - """Returns a list of snapshots, transformed through entity_maker.""" + def _items(self, req, is_detail=True): + """Returns a list of snapshots, transformed through view builder.""" context = req.environ['cinder.context'] - # pop out limit and offset , they are not search_opts + # Pop out non search_opts and create local variables search_opts = req.GET.copy() search_opts.pop('limit', None) search_opts.pop('offset', None) - # filter out invalid option + # Filter out invalid options allowed_search_options = ('status', 'volume_id', 'name') utils.remove_invalid_filter_options(context, search_opts, allowed_search_options) @@ -155,10 +129,18 @@ class SnapshotsController(wsgi.Controller): snapshots = self.volume_api.get_all_snapshots(context, search_opts=search_opts) + limited_list = common.limited(snapshots.objects, req) req.cache_db_snapshots(limited_list) - res = [entity_maker(snapshot) for snapshot in limited_list] - return {'snapshots': res} + snapshot_count = len(snapshots) + + if is_detail: + snapshots = self._view_builder.detail_list(req, limited_list, + snapshot_count) + else: + snapshots = self._view_builder.summary_list(req, limited_list, + snapshot_count) + return snapshots @wsgi.response(202) @wsgi.serializers(xml=SnapshotTemplate) @@ -213,9 +195,7 @@ class SnapshotsController(wsgi.Controller): **kwargs) req.cache_db_snapshot(new_snapshot) - retval = _translate_snapshot_detail_view(new_snapshot) - - return {'snapshot': retval} + return self._view_builder.detail(req, new_snapshot) @wsgi.serializers(xml=SnapshotTemplate) def update(self, req, id, body): @@ -268,7 +248,7 @@ class SnapshotsController(wsgi.Controller): volume_utils.notify_about_snapshot_usage(context, snapshot, 'update.end') - return {'snapshot': _translate_snapshot_detail_view(snapshot)} + return self._view_builder.detail(req, snapshot) def create_resource(ext_mgr): diff --git a/cinder/api/views/snapshots.py b/cinder/api/views/snapshots.py new file mode 100644 index 000000000..56e281a4f --- /dev/null +++ b/cinder/api/views/snapshots.py @@ -0,0 +1,75 @@ +# Copyright (c) 2015 Red Hat, Inc. +# All Rights Reserved. +# +# 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. + +from oslo_log import log as logging + +from cinder.api import common + + +LOG = logging.getLogger(__name__) + + +class ViewBuilder(common.ViewBuilder): + """Model snapshot API responses as a python dictionary.""" + + _collection_name = "snapshots" + + def __init__(self): + """Initialize view builder.""" + super(ViewBuilder, self).__init__() + + def summary_list(self, request, snapshots, snapshot_count): + """Show a list of snapshots without many details.""" + return self._list_view(self.summary, request, snapshots, + snapshot_count) + + def detail_list(self, request, snapshots, snapshot_count): + """Detailed view of a list of snapshots.""" + return self._list_view(self.detail, request, snapshots, snapshot_count, + coll_name=self._collection_name + '/detail') + + def summary(self, request, snapshot): + """Generic, non-detailed view of a snapshot.""" + if isinstance(snapshot.metadata, dict): + metadata = snapshot.metadata + else: + metadata = {} + + return { + 'snapshot': { + 'id': snapshot.id, + 'created_at': snapshot.created_at, + 'name': snapshot.display_name, + 'description': snapshot.display_description, + 'volume_id': snapshot.volume_id, + 'status': snapshot.status, + 'size': snapshot.volume_size, + 'metadata': metadata, + } + } + + def detail(self, request, snapshot): + """Detailed view of a single snapshot.""" + # NOTE(geguileo): No additional data at the moment + return self.summary(request, snapshot) + + def _list_view(self, func, request, snapshots, snapshot_count, + coll_name=_collection_name): + """Provide a view for a list of snapshots.""" + snapshots_list = [func(request, snapshot)['snapshot'] + for snapshot in snapshots] + snapshots_dict = {self._collection_name: snapshots_list} + + return snapshots_dict diff --git a/cinder/tests/unit/api/v2/test_snapshots.py b/cinder/tests/unit/api/v2/test_snapshots.py index fb0e63094..92c123c31 100644 --- a/cinder/tests/unit/api/v2/test_snapshots.py +++ b/cinder/tests/unit/api/v2/test_snapshots.py @@ -28,6 +28,7 @@ from cinder.tests.unit.api import fakes from cinder.tests.unit.api.v2 import stubs from cinder.tests.unit import fake_snapshot from cinder.tests.unit import fake_volume +from cinder.tests.unit import utils from cinder import volume @@ -51,17 +52,6 @@ def _get_default_snapshot_param(): } -def stub_snapshot_create(self, context, - volume_id, name, - description, metadata): - snapshot = _get_default_snapshot_param() - snapshot['volume_id'] = volume_id - snapshot['display_name'] = name - snapshot['display_description'] = description - snapshot['metadata'] = metadata - return snapshot - - def stub_snapshot_delete(self, context, snapshot): if snapshot['id'] != UUID: raise exception.SnapshotNotFound(snapshot['id']) @@ -89,16 +79,16 @@ class SnapshotApiTest(test.TestCase): stubs.stub_snapshot_get_all_by_project) self.stubs.Set(db, 'snapshot_get_all', stubs.stub_snapshot_get_all) + self.ctx = context.RequestContext('admin', 'fakeproject', True) @mock.patch( 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') def test_snapshot_create(self, mock_validate): - self.stubs.Set(volume.api.API, "create_snapshot", stub_snapshot_create) - self.stubs.Set(volume.api.API, 'get', stubs.stub_volume_get) + volume = utils.create_volume(self.ctx) snapshot_name = 'Snapshot Test Name' snapshot_description = 'Snapshot Test Desc' snapshot = { - "volume_id": '12', + "volume_id": volume.id, "force": False, "name": snapshot_name, "description": snapshot_description @@ -113,15 +103,14 @@ class SnapshotApiTest(test.TestCase): self.assertEqual(snapshot_description, resp_dict['snapshot']['description']) self.assertTrue(mock_validate.called) + db.volume_destroy(self.ctx, volume.id) def test_snapshot_create_force(self): - self.stubs.Set(volume.api.API, "create_snapshot_force", - stub_snapshot_create) - self.stubs.Set(volume.api.API, 'get', stubs.stub_volume_get) + volume = utils.create_volume(self.ctx, status='in-use') snapshot_name = 'Snapshot Test Name' snapshot_description = 'Snapshot Test Desc' snapshot = { - "volume_id": '12', + "volume_id": volume.id, "force": True, "name": snapshot_name, "description": snapshot_description @@ -137,7 +126,7 @@ class SnapshotApiTest(test.TestCase): resp_dict['snapshot']['description']) snapshot = { - "volume_id": "12", + "volume_id": volume.id, "force": "**&&^^%%$$##@@", "name": "Snapshot Test Name", "description": "Snapshot Test Desc" @@ -149,6 +138,8 @@ class SnapshotApiTest(test.TestCase): req, body) + db.volume_destroy(self.ctx, volume.id) + def test_snapshot_create_without_volume_id(self): snapshot_name = 'Snapshot Test Name' snapshot_description = 'Snapshot Test Desc' -- 2.45.2