Merge "Added action_plan ObjectField for Action"

watcher/objects/action.py

@@ -16,7 +16,8 @@
 
 from watcher.common import exception
 from watcher.common import utils
-from watcher.db import api as dbapi
+from watcher.db import api as db_api
+from watcher import objects
 from watcher.objects import base
 from watcher.objects import fields as wfields
 
@@ -35,60 +36,69 @@ class Action(base.WatcherPersistentObject, base.WatcherObject,
              base.WatcherObjectDictCompat):
 
     # Version 1.0: Initial version
-    VERSION = '1.0'
+    # Version 1.1: Added 'action_plan' object field
+    VERSION = '1.1'
 
-    dbapi = dbapi.get_instance()
+    dbapi = db_api.get_instance()
 
     fields = {
         'id': wfields.IntegerField(),
         'uuid': wfields.UUIDField(),
-        'action_plan_id': wfields.IntegerField(nullable=True),
+        'action_plan_id': wfields.IntegerField(),
         'action_type': wfields.StringField(nullable=True),
         'input_parameters': wfields.DictField(nullable=True),
         'state': wfields.StringField(nullable=True),
         'next': wfields.IntegerField(nullable=True),
+
+        'action_plan': wfields.ObjectField('ActionPlan', nullable=True),
+    }
+    object_fields = {
+        'action_plan': (objects.ActionPlan, 'action_plan_id'),
     }
 
     @base.remotable_classmethod
-    def get(cls, context, action_id):
+    def get(cls, context, action_id, eager=False):
         """Find a action based on its id or uuid and return a Action object.
 
         :param action_id: the id *or* uuid of a action.
+        :param eager: Load object fields if True (Default: False)
         :returns: a :class:`Action` object.
         """
         if utils.is_int_like(action_id):
-            return cls.get_by_id(context, action_id)
+            return cls.get_by_id(context, action_id, eager=eager)
         elif utils.is_uuid_like(action_id):
-            return cls.get_by_uuid(context, action_id)
+            return cls.get_by_uuid(context, action_id, eager=eager)
         else:
             raise exception.InvalidIdentity(identity=action_id)
 
     @base.remotable_classmethod
-    def get_by_id(cls, context, action_id):
+    def get_by_id(cls, context, action_id, eager=False):
         """Find a action based on its integer id and return a Action object.
 
         :param action_id: the id of a action.
+        :param eager: Load object fields if True (Default: False)
         :returns: a :class:`Action` object.
         """
-        db_action = cls.dbapi.get_action_by_id(context, action_id)
+        db_action = cls.dbapi.get_action_by_id(context, action_id, eager=eager)
-        action = Action._from_db_object(cls(context), db_action)
+        action = cls._from_db_object(cls(context), db_action, eager=eager)
         return action
 
     @base.remotable_classmethod
-    def get_by_uuid(cls, context, uuid):
+    def get_by_uuid(cls, context, uuid, eager=False):
         """Find a action based on uuid and return a :class:`Action` object.
 
         :param uuid: the uuid of a action.
         :param context: Security context
+        :param eager: Load object fields if True (Default: False)
         :returns: a :class:`Action` object.
         """
-        db_action = cls.dbapi.get_action_by_uuid(context, uuid)
+        db_action = cls.dbapi.get_action_by_uuid(context, uuid, eager=eager)
-        action = Action._from_db_object(cls(context), db_action)
+        action = cls._from_db_object(cls(context), db_action, eager=eager)
         return action
 
     @base.remotable_classmethod
     def list(cls, context, limit=None, marker=None, filters=None,
-             sort_key=None, sort_dir=None):
+             sort_key=None, sort_dir=None, eager=False):
         """Return a list of Action objects.
 
         :param context: Security context.
@@ -97,6 +107,7 @@ class Action(base.WatcherPersistentObject, base.WatcherObject,
         :param filters: Filters to apply. Defaults to None.
         :param sort_key: column to sort results by.
         :param sort_dir: direction to sort. "asc" or "desc".
+        :param eager: Load object fields if True (Default: False)
         :returns: a list of :class:`Action` object.
         """
         db_actions = cls.dbapi.get_action_list(context,
@@ -104,17 +115,23 @@ class Action(base.WatcherPersistentObject, base.WatcherObject,
                                                marker=marker,
                                                filters=filters,
                                                sort_key=sort_key,
-                                               sort_dir=sort_dir)
+                                               sort_dir=sort_dir,
+                                               eager=eager)
 
-        return [cls._from_db_object(cls(context), obj)
+        return [cls._from_db_object(cls(context), obj, eager=eager)
                 for obj in db_actions]
 
     @base.remotable
     def create(self):
-        """Create a Action record in the DB"""
+        """Create an :class:`Action` record in the DB.
+
+        :returns: An :class:`Action` object.
+        """
         values = self.obj_get_changes()
         db_action = self.dbapi.create_action(values)
-        self._from_db_object(self, db_action)
+        # Note(v-francoise): Always load eagerly upon creation so we can send
+        # notifications containing information about the related relationships
+        self._from_db_object(self, db_action, eager=True)
 
     def destroy(self):
         """Delete the Action from the DB"""
@@ -134,14 +151,15 @@ class Action(base.WatcherPersistentObject, base.WatcherObject,
         self.obj_reset_changes()
 
     @base.remotable
-    def refresh(self):
+    def refresh(self, eager=False):
         """Loads updates for this Action.
 
         Loads a action with the same uuid from the database and
         checks for updated attributes. Updates are applied from
         the loaded action column by column, if there are any updates.
+        :param eager: Load object fields if True (Default: False)
         """
-        current = self.__class__.get_by_uuid(self._context, uuid=self.uuid)
+        current = self.get_by_uuid(self._context, uuid=self.uuid, eager=eager)
         self.obj_refresh(current)
 
     @base.remotable

watcher/tests/api/v1/test_actions.py

@@ -325,9 +325,9 @@ class TestListAction(api_base.FunctionalTest):
             action_list.append(action.uuid)
         response = self.get_json('/actions')
         response_actions = response['actions']
-        for id in [0, 1, 2, 3]:
+        for id_ in range(4):
-            self.assertEqual(response_actions[id]['next_uuid'],
+            self.assertEqual(response_actions[id_]['next_uuid'],
-                             response_actions[id + 1]['uuid'])
+                             response_actions[id_ + 1]['uuid'])
 
     def test_many_without_soft_deleted(self):
         action_list = []
@@ -450,8 +450,7 @@ class TestPatch(api_base.FunctionalTest):
 
         response = self.patch_json(
             '/actions/%s' % self.action.uuid,
-            [{'path': '/state', 'value': new_state,
+            [{'path': '/state', 'value': new_state, 'op': 'replace'}],
-             'op': 'replace'}],
             expect_errors=True)
         self.assertEqual('application/json', response.content_type)
         self.assertEqual(403, response.status_int)
@@ -465,6 +464,7 @@ class TestDelete(api_base.FunctionalTest):
         self.goal = obj_utils.create_test_goal(self.context)
         self.strategy = obj_utils.create_test_strategy(self.context)
         self.audit = obj_utils.create_test_audit(self.context)
+        self.action_plan = obj_utils.create_test_action_plan(self.context)
         self.action = obj_utils.create_test_action(self.context, next=None)
         p = mock.patch.object(db_api.BaseConnection, 'update_action')
         self.mock_action_update = p.start()
@@ -488,6 +488,13 @@ class TestDelete(api_base.FunctionalTest):
 
 class TestActionPolicyEnforcement(api_base.FunctionalTest):
 
+    def setUp(self):
+        super(TestActionPolicyEnforcement, self).setUp()
+        obj_utils.create_test_goal(self.context)
+        obj_utils.create_test_strategy(self.context)
+        obj_utils.create_test_audit(self.context)
+        obj_utils.create_test_action_plan(self.context)
+
     def _common_policy_check(self, rule, func, *arg, **kwarg):
         self.policy.set_rules({
             "admin_api": "(role:admin or role:administrator)",

watcher/tests/applier/action_plan/test_default_action_handler.py

@@ -26,6 +26,7 @@ from watcher.tests.objects import utils as obj_utils
 
 
 class TestDefaultActionPlanHandler(base.DbTestCase):
+
     def setUp(self):
         super(TestDefaultActionPlanHandler, self).setUp()
         obj_utils.create_test_goal(self.context)

watcher/tests/db/test_purge.py

@@ -265,13 +265,13 @@ class TestPurgeCommand(base.DbTestCase):
         with freezegun.freeze_time(self.fake_today):
             # orphan audit template
             audit_template4 = obj_utils.create_test_audit_template(
-                self.context, goal_id=self.goal2.id,  # Does not exist
+                self.context, goal_id=self.goal2.id,
                 name=self.generate_unique_name(prefix="Audit Template 4 "),
-                strategy_id=None, id=self._generate_id(),
+                strategy_id=self.strategy1.id, id=self._generate_id(),
                 uuid=utils.generate_uuid())
             audit4 = obj_utils.create_test_audit(
                 self.context, audit_template_id=audit_template4.id,
-                id=self._generate_id(),
+                strategy_id=self.strategy1.id, id=self._generate_id(),
                 uuid=utils.generate_uuid())
             action_plan4 = obj_utils.create_test_action_plan(
                 self.context,
@@ -289,7 +289,7 @@ class TestPurgeCommand(base.DbTestCase):
                 uuid=utils.generate_uuid())
             audit5 = obj_utils.create_test_audit(
                 self.context, audit_template_id=audit_template5.id,
-                id=self._generate_id(),
+                strategy_id=self.strategy1.id, id=self._generate_id(),
                 uuid=utils.generate_uuid())
             action_plan5 = obj_utils.create_test_action_plan(
                 self.context,
@@ -362,7 +362,7 @@ class TestPurgeCommand(base.DbTestCase):
         with freezegun.freeze_time(self.fake_today):
             # orphan audit template
             audit_template4 = obj_utils.create_test_audit_template(
-                self.context, goal_id=self.goal2.id,  # Does not exist
+                self.context, goal_id=self.goal2.id,
                 name=self.generate_unique_name(prefix="Audit Template 4 "),
                 strategy_id=None, id=self._generate_id(),
                 uuid=utils.generate_uuid())
@@ -386,7 +386,7 @@ class TestPurgeCommand(base.DbTestCase):
                 uuid=utils.generate_uuid())
             audit5 = obj_utils.create_test_audit(
                 self.context, audit_template_id=audit_template5.id,
-                id=self._generate_id(),
+                strategy_id=self.strategy1.id, id=self._generate_id(),
                 uuid=utils.generate_uuid())
             action_plan5 = obj_utils.create_test_action_plan(
                 self.context,

watcher/tests/db/utils.py

@@ -102,7 +102,7 @@ def create_test_audit(**kwargs):
 
 
 def get_test_action(**kwargs):
-    return {
+    action_data = {
         'id': kwargs.get('id', 1),
         'uuid': kwargs.get('uuid', '10a47dd1-4874-4298-91cf-eff046dbdb8d'),
         'action_plan_id': kwargs.get('action_plan_id', 1),
@@ -120,6 +120,13 @@ def get_test_action(**kwargs):
         'deleted_at': kwargs.get('deleted_at'),
     }
 
+    # 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('action_plan'):
+        action_data['action_plan'] = kwargs.get('action_plan')
+
+    return action_data
+
 
 def create_test_action(**kwargs):
     """Create test action entry in DB and return Action DB object.

watcher/tests/objects/test_action.py

@@ -14,103 +14,164 @@
 #    under the License.
 
 import mock
+
 from watcher.common import exception
+from watcher.db.sqlalchemy import api as db_api
 from watcher import objects
-from watcher.objects import action as actionobject
 from watcher.tests.db import base
 from watcher.tests.db import utils
 
 
 class TestActionObject(base.DbTestCase):
 
+    action_plan_id = 2
+
+    scenarios = [
+        ('non_eager', dict(
+            eager=False,
+            fake_action=utils.get_test_action(
+                action_plan_id=action_plan_id))),
+        ('eager_with_non_eager_load', dict(
+            eager=True,
+            fake_action=utils.get_test_action(
+                action_plan_id=action_plan_id))),
+        ('eager_with_eager_load', dict(
+            eager=True,
+            fake_action=utils.get_test_action(
+                action_plan_id=action_plan_id,
+                action_plan=utils.get_test_action_plan(id=action_plan_id)))),
+    ]
+
     def setUp(self):
         super(TestActionObject, self).setUp()
-        self.fake_action = utils.get_test_action()
+        self.fake_action_plan = utils.create_test_action_plan(
+            id=self.action_plan_id)
 
-    def test_get_by_id(self):
+    def eager_action_assert(self, action):
+        if self.eager:
+            self.assertIsNotNone(action.action_plan)
+            fields_to_check = set(
+                super(objects.ActionPlan, objects.ActionPlan).fields
+            ).symmetric_difference(objects.ActionPlan.fields)
+            db_data = {
+                k: v for k, v in self.fake_action_plan.as_dict().items()
+                if k in fields_to_check}
+            object_data = {
+                k: v for k, v in action.action_plan.as_dict().items()
+                if k in fields_to_check}
+            self.assertEqual(db_data, object_data)
+
+    @mock.patch.object(db_api.Connection, 'get_action_by_id')
+    def test_get_by_id(self, mock_get_action):
+        mock_get_action.return_value = self.fake_action
         action_id = self.fake_action['id']
-        with mock.patch.object(self.dbapi, 'get_action_by_id',
+        action = objects.Action.get(self.context, action_id, eager=self.eager)
-                               autospec=True) as mock_get_action:
+        mock_get_action.assert_called_once_with(
-            mock_get_action.return_value = self.fake_action
+            self.context, action_id, eager=self.eager)
-            action = objects.Action.get(self.context, action_id)
+        self.assertEqual(self.context, action._context)
-            mock_get_action.assert_called_once_with(self.context,
+        self.eager_action_assert(action)
-                                                    action_id)
-            self.assertEqual(self.context, action._context)
 
-    def test_get_by_uuid(self):
+    @mock.patch.object(db_api.Connection, 'get_action_by_uuid')
+    def test_get_by_uuid(self, mock_get_action):
+        mock_get_action.return_value = self.fake_action
         uuid = self.fake_action['uuid']
-        with mock.patch.object(self.dbapi, 'get_action_by_uuid',
+        action = objects.Action.get(self.context, uuid, eager=self.eager)
-                               autospec=True) as mock_get_action:
+        mock_get_action.assert_called_once_with(
-            mock_get_action.return_value = self.fake_action
+            self.context, uuid, eager=self.eager)
-            action = objects.Action.get(self.context, uuid)
+        self.assertEqual(self.context, action._context)
-            mock_get_action.assert_called_once_with(self.context, uuid)
-            self.assertEqual(self.context, action._context)
 
     def test_get_bad_id_and_uuid(self):
         self.assertRaises(exception.InvalidIdentity,
-                          objects.Action.get, self.context, 'not-a-uuid')
+                          objects.Action.get, self.context, 'not-a-uuid',
+                          eager=self.eager)
 
-    def test_list(self):
+    @mock.patch.object(db_api.Connection, 'get_action_list')
-        with mock.patch.object(self.dbapi, 'get_action_list',
+    def test_list(self, mock_get_list):
-                               autospec=True) as mock_get_list:
+        mock_get_list.return_value = [self.fake_action]
-            mock_get_list.return_value = [self.fake_action]
+        actions = objects.Action.list(self.context, eager=self.eager)
-            actions = objects.Action.list(self.context)
+        self.assertEqual(1, mock_get_list.call_count)
-            self.assertEqual(1, mock_get_list.call_count)
+        self.assertEqual(1, len(actions))
-            self.assertEqual(1, len(actions))
+        self.assertIsInstance(actions[0], objects.Action)
-            self.assertIsInstance(actions[0], objects.Action)
+        self.assertEqual(self.context, actions[0]._context)
-            self.assertEqual(self.context, actions[0]._context)
+        for action in actions:
+            self.eager_action_assert(action)
 
-    def test_create(self):
+    @mock.patch.object(db_api.Connection, 'update_action')
-        with mock.patch.object(self.dbapi, 'create_action',
+    @mock.patch.object(db_api.Connection, 'get_action_by_uuid')
-                               autospec=True) as mock_create_action:
+    def test_save(self, mock_get_action, mock_update_action):
-            mock_create_action.return_value = self.fake_action
+        mock_get_action.return_value = self.fake_action
-            action = objects.Action(self.context, **self.fake_action)
-
-            action.create()
-            mock_create_action.assert_called_once_with(self.fake_action)
-            self.assertEqual(self.context, action._context)
-
-    def test_destroy(self):
         uuid = self.fake_action['uuid']
-        with mock.patch.object(self.dbapi, 'get_action_by_uuid',
+        action = objects.Action.get_by_uuid(
-                               autospec=True) as mock_get_action:
+            self.context, uuid, eager=self.eager)
-            mock_get_action.return_value = self.fake_action
+        action.state = objects.action.State.SUCCEEDED
-            with mock.patch.object(self.dbapi, 'destroy_action',
+        action.save()
-                                   autospec=True) as mock_destroy_action:
-                action = objects.Action.get_by_uuid(self.context, uuid)
-                action.destroy()
-                mock_get_action.assert_called_once_with(self.context, uuid)
-                mock_destroy_action.assert_called_once_with(uuid)
-                self.assertEqual(self.context, action._context)
 
-    def test_save(self):
+        mock_get_action.assert_called_once_with(
-        uuid = self.fake_action['uuid']
+            self.context, uuid, eager=self.eager)
-        with mock.patch.object(self.dbapi, 'get_action_by_uuid',
+        mock_update_action.assert_called_once_with(
-                               autospec=True) as mock_get_action:
+            uuid, {'state': objects.action.State.SUCCEEDED})
-            mock_get_action.return_value = self.fake_action
+        self.assertEqual(self.context, action._context)
-            with mock.patch.object(self.dbapi, 'update_action',
-                                   autospec=True) as mock_update_action:
-                action = objects.Action.get_by_uuid(self.context, uuid)
-                action.state = actionobject.State.SUCCEEDED
-                action.save()
 
-                mock_get_action.assert_called_once_with(self.context, uuid)
+    @mock.patch.object(db_api.Connection, 'get_action_by_uuid')
-                mock_update_action.assert_called_once_with(
+    def test_refresh(self, mock_get_action):
-                    uuid, {'state': actionobject.State.SUCCEEDED})
-                self.assertEqual(self.context, action._context)
-
-    def test_refresh(self):
-        uuid = self.fake_action['uuid']
         returns = [dict(self.fake_action, state="first state"),
                    dict(self.fake_action, state="second state")]
-        expected = [mock.call(self.context, uuid),
+        mock_get_action.side_effect = returns
-                    mock.call(self.context, uuid)]
+        uuid = self.fake_action['uuid']
-        with mock.patch.object(self.dbapi, 'get_action_by_uuid',
+        expected = [mock.call(self.context, uuid, eager=self.eager),
-                               side_effect=returns,
+                    mock.call(self.context, uuid, eager=self.eager)]
-                               autospec=True) as mock_get_action:
+        action = objects.Action.get(self.context, uuid, eager=self.eager)
-            action = objects.Action.get(self.context, uuid)
+        self.assertEqual("first state", action.state)
-            self.assertEqual("first state", action.state)
+        action.refresh(eager=self.eager)
-            action.refresh()
+        self.assertEqual("second state", action.state)
-            self.assertEqual("second state", action.state)
+        self.assertEqual(expected, mock_get_action.call_args_list)
-            self.assertEqual(expected, mock_get_action.call_args_list)
+        self.assertEqual(self.context, action._context)
-            self.assertEqual(self.context, action._context)
+        self.eager_action_assert(action)
+
+
+class TestCreateDeleteActionObject(base.DbTestCase):
+
+    def setUp(self):
+        super(TestCreateDeleteActionObject, self).setUp()
+        self.fake_strategy = utils.create_test_strategy(name="DUMMY")
+        self.fake_audit = utils.create_test_audit()
+        self.fake_action_plan = utils.create_test_action_plan()
+        self.fake_action = utils.get_test_action()
+
+    @mock.patch.object(db_api.Connection, 'create_action')
+    def test_create(self, mock_create_action):
+        mock_create_action.return_value = self.fake_action
+        action = objects.Action(self.context, **self.fake_action)
+        action.create()
+
+        mock_create_action.assert_called_once_with(self.fake_action)
+        self.assertEqual(self.context, action._context)
+
+    @mock.patch.object(db_api.Connection, 'update_action')
+    @mock.patch.object(db_api.Connection, 'soft_delete_action')
+    @mock.patch.object(db_api.Connection, 'get_action_by_uuid')
+    def test_soft_delete(self, mock_get_action,
+                         mock_soft_delete_action, mock_update_action):
+        mock_get_action.return_value = self.fake_action
+        uuid = self.fake_action['uuid']
+        action = objects.Action.get_by_uuid(self.context, uuid)
+        action.soft_delete()
+        mock_get_action.assert_called_once_with(
+            self.context, uuid, eager=False)
+        mock_soft_delete_action.assert_called_once_with(uuid)
+        mock_update_action.assert_called_once_with(
+            uuid, {'state': objects.action.State.DELETED})
+        self.assertEqual(self.context, action._context)
+
+    @mock.patch.object(db_api.Connection, 'destroy_action')
+    @mock.patch.object(db_api.Connection, 'get_action_by_uuid')
+    def test_destroy(self, mock_get_action, mock_destroy_action):
+        mock_get_action.return_value = self.fake_action
+        uuid = self.fake_action['uuid']
+        action = objects.Action.get_by_uuid(self.context, uuid)
+        action.destroy()
+
+        mock_get_action.assert_called_once_with(
+            self.context, uuid, eager=False)
+        mock_destroy_action.assert_called_once_with(uuid)
+        self.assertEqual(self.context, action._context)

watcher/tests/objects/test_objects.py

@@ -414,7 +414,7 @@ expected_object_fingerprints = {
     'AuditTemplate': '1.1-b291973ffc5efa2c61b24fe34fdccc0b',
     'Audit': '1.1-dc246337c8d511646cb537144fcb0f3a',
     'ActionPlan': '1.1-299bd9c76f2402a0b2167f8e4d744a05',
-    'Action': '1.0-a78f69c0da98e13e601f9646f6b2f883',
+    'Action': '1.1-52c77e4db4ce0aa9480c9760faec61a1',
     'EfficacyIndicator': '1.0-655b71234a82bc7478aff964639c4bb0',
     'ScoringEngine': '1.0-4abbe833544000728e17bd9e83f97576',
     'Service': '1.0-4b35b99ada9677a882c9de2b30212f35',