Added 'goal' ObjectField for Strategy object

In this changeset, I added the "goal" ObjectField which can
either be loaded by setting the new "eager" parameter as True or
not loaded (as before) by setting it to False.
The advantage of introducing this eager parameter is that this way,
we can reduce to a minimum the overhead of DB queries whenever the
related goal is not actually needed.

Partially-Implements: blueprint watcher-versioned-objects
Change-Id: I103c9ed161d2cedf7b43c55f9e095ef66bf44dea

watcher/tests/api/v1/test_strategies.py

@@ -21,6 +21,11 @@ from watcher.tests.objects import utils as obj_utils
 
 class TestListStrategy(api_base.FunctionalTest):
 
+    def setUp(self):
+        super(TestListStrategy, self).setUp()
+        self.fake_goal = obj_utils.create_test_goal(
+            self.context, uuid=utils.generate_uuid())
+
     def _assert_strategy_fields(self, strategy):
         strategy_fields = ['uuid', 'name', 'display_name', 'goal_uuid']
         for field in strategy_fields:
@@ -61,7 +66,6 @@ class TestListStrategy(api_base.FunctionalTest):
         self.assertEqual(404, response.status_int)
 
     def test_detail(self):
-        obj_utils.create_test_goal(self.context)
         strategy = obj_utils.create_test_strategy(self.context)
         response = self.get_json('/strategies/detail')
         self.assertEqual(strategy.uuid, response['strategies'][0]["uuid"])
@@ -78,7 +82,6 @@ class TestListStrategy(api_base.FunctionalTest):
         self.assertEqual(404, response.status_int)
 
     def test_many(self):
-        obj_utils.create_test_goal(self.context)
         strategy_list = []
         for idx in range(1, 6):
             strategy = obj_utils.create_test_strategy(
@@ -132,12 +135,12 @@ class TestListStrategy(api_base.FunctionalTest):
     def test_filter_by_goal_uuid(self):
         goal1 = obj_utils.create_test_goal(
             self.context,
-            id=1,
+            id=2,
             uuid=utils.generate_uuid(),
             name='My_Goal 1')
         goal2 = obj_utils.create_test_goal(
             self.context,
-            id=2,
+            id=3,
             uuid=utils.generate_uuid(),
             name='My Goal 2')
 
@@ -164,12 +167,12 @@ class TestListStrategy(api_base.FunctionalTest):
     def test_filter_by_goal_name(self):
         goal1 = obj_utils.create_test_goal(
             self.context,
-            id=1,
+            id=2,
             uuid=utils.generate_uuid(),
             name='My_Goal 1')
         goal2 = obj_utils.create_test_goal(
             self.context,
-            id=2,
+            id=3,
             uuid=utils.generate_uuid(),
             name='My Goal 2')
 
@@ -196,6 +199,11 @@ class TestListStrategy(api_base.FunctionalTest):
 
 class TestStrategyPolicyEnforcement(api_base.FunctionalTest):
 
+    def setUp(self):
+        super(TestStrategyPolicyEnforcement, self).setUp()
+        self.fake_goal = obj_utils.create_test_goal(
+            self.context, uuid=utils.generate_uuid())
+
     def _common_policy_check(self, rule, func, *arg, **kwarg):
         self.policy.set_rules({
             "admin_api": "(role:admin or role:administrator)",
@@ -227,8 +235,8 @@ class TestStrategyPolicyEnforcement(api_base.FunctionalTest):
             expect_errors=True)
 
 
-class TestStrategyEnforcementWithAdminContext(TestListStrategy,
-                                              api_base.AdminRoleTest):
+class TestStrategyEnforcementWithAdminContext(
+        TestListStrategy, api_base.AdminRoleTest):
 
     def setUp(self):
         super(TestStrategyEnforcementWithAdminContext, self).setUp()

watcher/tests/db/utils.py

@@ -199,7 +199,7 @@ def create_test_scoring_engine(**kwargs):
 
 
 def get_test_strategy(**kwargs):
-    return {
+    strategy_data = {
         'id': kwargs.get('id', 1),
         'uuid': kwargs.get('uuid', 'cb3d0b58-4415-4d90-b75b-1e96878730e3'),
         'name': kwargs.get('name', 'TEST'),
@@ -208,9 +208,16 @@ def get_test_strategy(**kwargs):
         'created_at': kwargs.get('created_at'),
         'updated_at': kwargs.get('updated_at'),
         'deleted_at': kwargs.get('deleted_at'),
-        'parameters_spec': kwargs.get('parameters_spec', {})
+        'parameters_spec': kwargs.get('parameters_spec', {}),
     }
 
+    # goal ObjectField doesn't allow None nor dict, so if we want to simulate a
+    # non-eager object loading, the field should not be referenced at all.
+    if kwargs.get('goal'):
+        strategy_data['goal'] = kwargs.get('goal')
+
+    return strategy_data
+
 
 def get_test_service(**kwargs):
     return {

watcher/tests/decision_engine/planner/test_default_planner.py

@@ -159,8 +159,11 @@ class TestDefaultPlanner(base.DbTestCase):
             'migrate': 3
         }
 
-        obj_utils.create_test_audit_template(self.context)
-        self.strategy = obj_utils.create_test_strategy(self.context)
+        self.goal = obj_utils.create_test_goal(self.context)
+        self.strategy = obj_utils.create_test_strategy(
+            self.context, goal_id=self.goal.id)
+        obj_utils.create_test_audit_template(
+            self.context, goal_id=self.goal.id, strategy_id=self.strategy.id)
 
         p = mock.patch.object(db_api.BaseConnection, 'create_action_plan')
         self.mock_create_action_plan = p.start()
@@ -185,7 +188,8 @@ class TestDefaultPlanner(base.DbTestCase):
     @mock.patch.object(objects.Strategy, 'get_by_name')
     def test_schedule_scheduled_empty(self, m_get_by_name):
         m_get_by_name.return_value = self.strategy
-        audit = db_utils.create_test_audit(uuid=utils.generate_uuid())
+        audit = db_utils.create_test_audit(
+            goal_id=self.goal.id, strategy_id=self.strategy.id)
         fake_solution = SolutionFakerSingleHyp.build()
         action_plan = self.default_planner.schedule(self.context,
                                                     audit.id, fake_solution)
@@ -194,8 +198,9 @@ class TestDefaultPlanner(base.DbTestCase):
     @mock.patch.object(objects.Strategy, 'get_by_name')
     def test_scheduler_warning_empty_action_plan(self, m_get_by_name):
         m_get_by_name.return_value = self.strategy
-        audit = db_utils.create_test_audit(uuid=utils.generate_uuid())
+        audit = db_utils.create_test_audit(
+            goal_id=self.goal.id, strategy_id=self.strategy.id)
         fake_solution = SolutionFaker.build()
-        action_plan = self.default_planner.schedule(self.context,
-                                                    audit.id, fake_solution)
+        action_plan = self.default_planner.schedule(
+            self.context, audit.id, fake_solution)
         self.assertIsNotNone(action_plan.uuid)

watcher/tests/objects/test_objects.py

@@ -410,7 +410,7 @@ class TestObject(_LocalTest, _TestObject):
 # The fingerprint values should only be changed if there is a version bump.
 expected_object_fingerprints = {
     'Goal': '1.0-93881622db05e7b67a65ca885b4a022e',
-    'Strategy': '1.0-e60f62cc854c6e63fb1c3befbfc8629e',
+    'Strategy': '1.1-73f164491bdd4c034f48083a51bdeb7b',
     'AuditTemplate': '1.0-7432ee4d3ce0c7cbb9d11a4565ee8eb6',
     'Audit': '1.0-ebfc5360d019baf583a10a8a27071c97',
     'ActionPlan': '1.0-cc76fd7f0e8479aeff817dd266341de4',

watcher/tests/objects/test_strategy.py

@@ -15,7 +15,9 @@
 
 
 import mock
+
 from watcher.common import exception
+from watcher.db.sqlalchemy import api as db_api
 from watcher import objects
 from watcher.tests.db import base
 from watcher.tests.db import utils
@@ -23,107 +25,143 @@ from watcher.tests.db import utils
 
 class TestStrategyObject(base.DbTestCase):
 
+    goal_id = 2
+
+    scenarios = [
+        ('non_eager', dict(
+            eager=False, fake_strategy=utils.get_test_strategy(
+                goal_id=goal_id))),
+        ('eager_with_non_eager_load', dict(
+            eager=True, fake_strategy=utils.get_test_strategy(
+                goal_id=goal_id))),
+        ('eager_with_eager_load', dict(
+            eager=True, fake_strategy=utils.get_test_strategy(
+                goal_id=goal_id, goal=utils.get_test_goal(id=goal_id)))),
+    ]
+
     def setUp(self):
         super(TestStrategyObject, self).setUp()
-        self.fake_strategy = utils.get_test_strategy()
+        self.fake_goal = utils.create_test_goal(id=self.goal_id)
 
-    def test_get_by_id(self):
+    def eager_load_strategy_assert(self, strategy):
+        if self.eager:
+            self.assertIsNotNone(strategy.goal)
+            fields_to_check = set(
+                super(objects.Goal, objects.Goal).fields
+            ).symmetric_difference(objects.Goal.fields)
+            db_data = {
+                k: v for k, v in self.fake_goal.as_dict().items()
+                if k in fields_to_check}
+            object_data = {
+                k: v for k, v in strategy.goal.as_dict().items()
+                if k in fields_to_check}
+            self.assertEqual(db_data, object_data)
+
+    @mock.patch.object(db_api.Connection, 'get_strategy_by_id')
+    def test_get_by_id(self, mock_get_strategy):
         strategy_id = self.fake_strategy['id']
-        with mock.patch.object(self.dbapi, 'get_strategy_by_id',
-                               autospec=True) as mock_get_strategy:
-            mock_get_strategy.return_value = self.fake_strategy
-            strategy = objects.Strategy.get(self.context, strategy_id)
-            mock_get_strategy.assert_called_once_with(self.context,
-                                                      strategy_id)
-            self.assertEqual(self.context, strategy._context)
+        mock_get_strategy.return_value = self.fake_strategy
+        strategy = objects.Strategy.get(
+            self.context, strategy_id, eager=self.eager)
+        mock_get_strategy.assert_called_once_with(
+            self.context, strategy_id, eager=self.eager)
+        self.assertEqual(self.context, strategy._context)
+        self.eager_load_strategy_assert(strategy)
 
-    def test_get_by_uuid(self):
+    @mock.patch.object(db_api.Connection, 'get_strategy_by_uuid')
+    def test_get_by_uuid(self, mock_get_strategy):
         uuid = self.fake_strategy['uuid']
-        with mock.patch.object(self.dbapi, 'get_strategy_by_uuid',
-                               autospec=True) as mock_get_strategy:
-            mock_get_strategy.return_value = self.fake_strategy
-            strategy = objects.Strategy.get(self.context, uuid)
-            mock_get_strategy.assert_called_once_with(self.context, uuid)
-            self.assertEqual(self.context, strategy._context)
+        mock_get_strategy.return_value = self.fake_strategy
+        strategy = objects.Strategy.get(self.context, uuid, eager=self.eager)
+        mock_get_strategy.assert_called_once_with(
+            self.context, uuid, eager=self.eager)
+        self.assertEqual(self.context, strategy._context)
+        self.eager_load_strategy_assert(strategy)
 
     def test_get_bad_uuid(self):
         self.assertRaises(exception.InvalidIdentity,
                           objects.Strategy.get, self.context, 'not-a-uuid')
 
-    def test_list(self):
-        with mock.patch.object(self.dbapi, 'get_strategy_list',
-                               autospec=True) as mock_get_list:
-            mock_get_list.return_value = [self.fake_strategy]
-            strategies = objects.Strategy.list(self.context)
-            self.assertEqual(1, mock_get_list.call_count, 1)
-            self.assertEqual(1, len(strategies))
-            self.assertIsInstance(strategies[0], objects.Strategy)
-            self.assertEqual(self.context, strategies[0]._context)
+    @mock.patch.object(db_api.Connection, 'get_strategy_list')
+    def test_list(self, mock_get_list):
+        mock_get_list.return_value = [self.fake_strategy]
+        strategies = objects.Strategy.list(self.context, eager=self.eager)
+        self.assertEqual(1, mock_get_list.call_count, 1)
+        self.assertEqual(1, len(strategies))
+        self.assertIsInstance(strategies[0], objects.Strategy)
+        self.assertEqual(self.context, strategies[0]._context)
+        for strategy in strategies:
+            self.eager_load_strategy_assert(strategy)
 
-    def test_create(self):
-        with mock.patch.object(self.dbapi, 'create_strategy',
-                               autospec=True) as mock_create_strategy:
-            mock_create_strategy.return_value = self.fake_strategy
-            strategy = objects.Strategy(self.context, **self.fake_strategy)
-
-            strategy.create()
-            mock_create_strategy.assert_called_once_with(self.fake_strategy)
-            self.assertEqual(self.context, strategy._context)
-
-    def test_destroy(self):
+    @mock.patch.object(db_api.Connection, 'update_strategy')
+    @mock.patch.object(db_api.Connection, 'get_strategy_by_id')
+    def test_save(self, mock_get_strategy, mock_update_strategy):
         _id = self.fake_strategy['id']
-        with mock.patch.object(self.dbapi, 'get_strategy_by_id',
-                               autospec=True) as mock_get_strategy:
-            mock_get_strategy.return_value = self.fake_strategy
-            with mock.patch.object(self.dbapi, 'destroy_strategy',
-                                   autospec=True) as mock_destroy_strategy:
-                strategy = objects.Strategy.get_by_id(self.context, _id)
-                strategy.destroy()
-                mock_get_strategy.assert_called_once_with(self.context, _id)
-                mock_destroy_strategy.assert_called_once_with(_id)
-                self.assertEqual(self.context, strategy._context)
+        mock_get_strategy.return_value = self.fake_strategy
+        strategy = objects.Strategy.get_by_id(
+            self.context, _id, eager=self.eager)
+        strategy.name = 'UPDATED NAME'
+        strategy.save()
 
-    def test_save(self):
-        _id = self.fake_strategy['id']
-        with mock.patch.object(self.dbapi, 'get_strategy_by_id',
-                               autospec=True) as mock_get_strategy:
-            mock_get_strategy.return_value = self.fake_strategy
-            with mock.patch.object(self.dbapi, 'update_strategy',
-                                   autospec=True) as mock_update_strategy:
-                strategy = objects.Strategy.get_by_id(self.context, _id)
-                strategy.name = 'UPDATED NAME'
-                strategy.save()
+        mock_get_strategy.assert_called_once_with(
+            self.context, _id, eager=self.eager)
+        mock_update_strategy.assert_called_once_with(
+            _id, {'name': 'UPDATED NAME'})
+        self.assertEqual(self.context, strategy._context)
+        self.eager_load_strategy_assert(strategy)
 
-                mock_get_strategy.assert_called_once_with(self.context, _id)
-                mock_update_strategy.assert_called_once_with(
-                    _id, {'name': 'UPDATED NAME'})
-                self.assertEqual(self.context, strategy._context)
-
-    def test_refresh(self):
+    @mock.patch.object(db_api.Connection, 'get_strategy_by_id')
+    def test_refresh(self, mock_get_strategy):
         _id = self.fake_strategy['id']
         returns = [dict(self.fake_strategy, name="first name"),
                    dict(self.fake_strategy, name="second name")]
-        expected = [mock.call(self.context, _id),
-                    mock.call(self.context, _id)]
-        with mock.patch.object(self.dbapi, 'get_strategy_by_id',
-                               side_effect=returns,
-                               autospec=True) as mock_get_strategy:
-            strategy = objects.Strategy.get(self.context, _id)
-            self.assertEqual("first name", strategy.name)
-            strategy.refresh()
-            self.assertEqual("second name", strategy.name)
-            self.assertEqual(expected, mock_get_strategy.call_args_list)
-            self.assertEqual(self.context, strategy._context)
+        mock_get_strategy.side_effect = returns
+        expected = [mock.call(self.context, _id, eager=self.eager),
+                    mock.call(self.context, _id, eager=self.eager)]
+        strategy = objects.Strategy.get(self.context, _id, eager=self.eager)
+        self.assertEqual("first name", strategy.name)
+        strategy.refresh(eager=self.eager)
+        self.assertEqual("second name", strategy.name)
+        self.assertEqual(expected, mock_get_strategy.call_args_list)
+        self.assertEqual(self.context, strategy._context)
+        self.eager_load_strategy_assert(strategy)
 
-    def test_soft_delete(self):
+
+class TestCreateDeleteStrategyObject(base.DbTestCase):
+
+    def setUp(self):
+        super(TestCreateDeleteStrategyObject, self).setUp()
+        self.fake_goal = utils.create_test_goal()
+        self.fake_strategy = utils.get_test_strategy(goal_id=self.fake_goal.id)
+
+    @mock.patch.object(db_api.Connection, 'create_strategy')
+    def test_create(self, mock_create_strategy):
+        mock_create_strategy.return_value = self.fake_strategy
+        strategy = objects.Strategy(self.context, **self.fake_strategy)
+        strategy.create()
+        mock_create_strategy.assert_called_once_with(self.fake_strategy)
+        self.assertEqual(self.context, strategy._context)
+
+    @mock.patch.object(db_api.Connection, 'soft_delete_strategy')
+    @mock.patch.object(db_api.Connection, 'get_strategy_by_id')
+    def test_soft_delete(self, mock_get_strategy, mock_soft_delete):
         _id = self.fake_strategy['id']
-        with mock.patch.object(self.dbapi, 'get_strategy_by_id',
-                               autospec=True) as mock_get_strategy:
-            mock_get_strategy.return_value = self.fake_strategy
-            with mock.patch.object(self.dbapi, 'soft_delete_strategy',
-                                   autospec=True) as mock_soft_delete:
-                strategy = objects.Strategy.get_by_id(self.context, _id)
-                strategy.soft_delete()
-                mock_get_strategy.assert_called_once_with(self.context, _id)
-                mock_soft_delete.assert_called_once_with(_id)
-                self.assertEqual(self.context, strategy._context)
+        mock_get_strategy.return_value = self.fake_strategy
+        strategy = objects.Strategy.get_by_id(self.context, _id)
+        strategy.soft_delete()
+        mock_get_strategy.assert_called_once_with(
+            self.context, _id, eager=False)
+        mock_soft_delete.assert_called_once_with(_id)
+        self.assertEqual(self.context, strategy._context)
+
+    @mock.patch.object(db_api.Connection, 'destroy_strategy')
+    @mock.patch.object(db_api.Connection, 'get_strategy_by_id')
+    def test_destroy(self, mock_get_strategy, mock_destroy_strategy):
+        _id = self.fake_strategy['id']
+        mock_get_strategy.return_value = self.fake_strategy
+        strategy = objects.Strategy.get_by_id(self.context, _id)
+        strategy.destroy()
+        mock_get_strategy.assert_called_once_with(
+            self.context, _id, eager=False)
+        mock_destroy_strategy.assert_called_once_with(_id)
+        self.assertEqual(self.context, strategy._context)

watcher/tests/objects/utils.py

@@ -217,6 +217,14 @@ def get_test_strategy(context, **kw):
     strategy = objects.Strategy(context)
     for key in db_strategy:
         setattr(strategy, key, db_strategy[key])
+
+    # ObjectField checks for the object type, so if we want to simulate a
+    # non-eager object loading, the field should not be referenced at all.
+    # Contrarily, eager loading need the data to be casted to the object type
+    # that was specified by the ObjectField.
+    if kw.get('goal'):
+        strategy.goal = objects.Goal(context, **kw.get('goal'))
+
     return strategy