From d3ded36f7fcc11f69831975549a949669d5a7802 Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Thu, 22 Sep 2016 21:37:20 +0200 Subject: [PATCH] Fix project assignment in VolumeType OVO We have a problem with the assignment of the "project" field on the VolumeType OVO class, because the DB queries return in some cases a list of strings -for example in "volume_type_get" method- while on others they return a VolumeTypeProjects SQLAlchemy model class. The source of all this confussion is that some DB methods do not return ORM instances but dictionaries with transformed data, which is something that should never have been done in the first place. This patch does not try to fix the underlying issue, just the incorrect attribute assignment in the least intrussive way possible. Change-Id: I0dd6b8ab2e726f69ceaaae886058e7a6181dded1 Closes-Bug: #1626704 --- cinder/objects/volume_type.py | 9 +++++++- cinder/tests/unit/objects/test_volume_type.py | 21 +++++++++++++++++++ cinder/tests/unit/test_qos_specs.py | 2 +- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/cinder/objects/volume_type.py b/cinder/objects/volume_type.py index 10215b102b4..e39b5d26276 100644 --- a/cinder/objects/volume_type.py +++ b/cinder/objects/volume_type.py @@ -14,6 +14,7 @@ from oslo_utils import versionutils from oslo_versionedobjects import fields +import six from cinder import db from cinder import exception @@ -84,7 +85,13 @@ class VolumeType(base.CinderPersistentObject, base.CinderObject, elif specs and isinstance(specs, dict): type.extra_specs = specs if 'projects' in expected_attrs: - type.projects = db_type.get('projects', []) + # NOTE(geguileo): Until projects stops being a polymorphic value we + # have to do a conversion here for VolumeTypeProjects ORM instance + # lists. + projects = db_type.get('projects', []) + if projects and not isinstance(projects[0], six.string_types): + projects = [p.project_id for p in projects] + type.projects = projects if 'qos_specs' in expected_attrs: qos_specs = objects.QualityOfServiceSpecs(context) qos_specs._from_db_object(context, qos_specs, db_type['qos_specs']) diff --git a/cinder/tests/unit/objects/test_volume_type.py b/cinder/tests/unit/objects/test_volume_type.py index 7f837acfc0d..0b44d89e3d6 100644 --- a/cinder/tests/unit/objects/test_volume_type.py +++ b/cinder/tests/unit/objects/test_volume_type.py @@ -17,6 +17,7 @@ from oslo_utils import timeutils import pytz import six +from cinder.db.sqlalchemy import models from cinder import objects from cinder.tests.unit import fake_constants as fake from cinder.tests.unit import fake_volume @@ -33,6 +34,26 @@ class TestVolumeType(test_objects.BaseObjectsTestCase): fake.VOLUME_TYPE_ID) self._compare(self, db_volume_type, volume_type) + @mock.patch('cinder.db.sqlalchemy.api._volume_type_get_full') + def test_get_by_id_with_projects(self, volume_type_get): + projects = [models.VolumeTypeProjects(project_id=fake.PROJECT_ID), + models.VolumeTypeProjects(project_id=fake.PROJECT2_ID)] + db_volume_type = fake_volume.fake_db_volume_type(projects=projects) + volume_type_get.return_value = db_volume_type + volume_type = objects.VolumeType.get_by_id(self.context, + fake.VOLUME_TYPE_ID) + db_volume_type['projects'] = [p.project_id for p in projects] + self._compare(self, db_volume_type, volume_type) + + @mock.patch('cinder.db.sqlalchemy.api._volume_type_get_full') + def test_get_by_id_with_string_projects(self, volume_type_get): + projects = [fake.PROJECT_ID, fake.PROJECT2_ID] + db_volume_type = fake_volume.fake_db_volume_type(projects=projects) + volume_type_get.return_value = db_volume_type + volume_type = objects.VolumeType.get_by_id(self.context, + fake.VOLUME_TYPE_ID) + self._compare(self, db_volume_type, volume_type) + @mock.patch('cinder.db.sqlalchemy.api._volume_type_get_full') def test_get_by_id_null_spec(self, volume_type_get): db_volume_type = fake_volume.fake_db_volume_type( diff --git a/cinder/tests/unit/test_qos_specs.py b/cinder/tests/unit/test_qos_specs.py index cb89497d50d..69380c5e96e 100644 --- a/cinder/tests/unit/test_qos_specs.py +++ b/cinder/tests/unit/test_qos_specs.py @@ -51,7 +51,7 @@ def fake_db_get_vol_type(vol_type_number=1): 'description': 'desc', 'deleted': False, 'is_public': True, - 'projects': None, + 'projects': [], 'extra_specs': None}