openstack
/
cinder
Code Issues Proposed changes

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
This commit is contained in:
Gorka Eguileor 2015-07-31 20:01:20 +02:00
parent 4c1acb6b82
commit 433cf95e06
3 changed files with 107 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

64
cinder/api/v2/snapshots.py
View File

@ -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):

75
cinder/api/views/snapshots.py Normal file
View File

@ -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

29
cinder/tests/unit/api/v2/test_snapshots.py
View File

@ -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'