From 32ecd3b50aed66b952cb73ef7cab7b999381ef18 Mon Sep 17 00:00:00 2001 From: Dan Smith Date: Wed, 8 Mar 2017 07:10:35 -0800 Subject: [PATCH] Sort CellMappingList.get_all() for safety The result of this method should be stable in database order, but since we depend on it for some pagination, this patch sorts the results by the most efficient thing we have to be sure. Related to blueprint cells-aware-api Change-Id: I81fb8a2a342ad54adc386c02a5fe4a90800fcbb0 --- nova/objects/cell_mapping.py | 5 ++++- nova/tests/unit/objects/test_cell_mapping.py | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/nova/objects/cell_mapping.py b/nova/objects/cell_mapping.py index 11d22ef57799..74bdccd01882 100644 --- a/nova/objects/cell_mapping.py +++ b/nova/objects/cell_mapping.py @@ -10,6 +10,8 @@ # License for the specific language governing permissions and limitations # under the License. +from sqlalchemy.sql.expression import asc + from nova.db.sqlalchemy import api as db_api from nova.db.sqlalchemy import api_models from nova import exception @@ -127,7 +129,8 @@ class CellMappingList(base.ObjectListBase, base.NovaObject): @staticmethod @db_api.api_context_manager.reader def _get_all_from_db(context): - return context.session.query(api_models.CellMapping).all() + return context.session.query(api_models.CellMapping).order_by( + asc(api_models.CellMapping.id)).all() @base.remotable_classmethod def get_all(cls, context): diff --git a/nova/tests/unit/objects/test_cell_mapping.py b/nova/tests/unit/objects/test_cell_mapping.py index 07c94b8a1cd2..9b8dc31f6bd2 100644 --- a/nova/tests/unit/objects/test_cell_mapping.py +++ b/nova/tests/unit/objects/test_cell_mapping.py @@ -138,6 +138,20 @@ class _TestCellMappingListObject(object): get_all_from_db.assert_called_once_with(self.context) self.compare_obj(mapping_obj.objects[0], db_mapping) + def test_get_all_sorted(self): + for ident in (10, 3): + cm = objects.CellMapping(context=self.context, + id=ident, + uuid=getattr(uuids, 'cell%i' % ident), + transport_url='fake://%i' % ident, + database_connection='fake://%i' % ident) + cm.create() + obj = objects.CellMappingList.get_all(self.context) + ids = [c.id for c in obj] + # Find the two normal cells, plus the two we created, but in the right + # order + self.assertEqual([1, 2, 3, 10], ids) + class TestCellMappingListObject(test_objects._LocalTest, _TestCellMappingListObject):