Add get cron-trigger by id support

Currently we can only do a CURD action on a cron-trigger by name.
This patch refer to workflow implementation and re-encapsulate
DB API so that users can manage a cron-trigger by id or name.

Closes-Bug: 1684469

Change-Id: I9ff657b2604647e734b5539e9bd6a524a3a20efb

mistral/api/controllers/v2/cron_trigger.py

@@ -32,17 +32,16 @@ LOG = logging.getLogger(__name__)
 class CronTriggersController(rest.RestController):
     @rest_utils.wrap_wsme_controller_exception
     @wsme_pecan.wsexpose(resources.CronTrigger, wtypes.text)
-    def get(self, name):
+    def get(self, identifier):
         """Returns the named cron_trigger.
 
-        :param name: Name of cron trigger to retrieve
+        :param identifier: Id or name of cron trigger to retrieve
         """
         acl.enforce('cron_triggers:get', context.ctx())
 
-        LOG.info('Fetch cron trigger [name=%s]', name)
+        LOG.info('Fetch cron trigger [identifier=%s]', identifier)
-
-        db_model = db_api.get_cron_trigger(name)
 
+        db_model = db_api.get_cron_trigger(identifier)
         return resources.CronTrigger.from_db_model(db_model)
 
     @rest_utils.wrap_wsme_controller_exception
@@ -77,16 +76,16 @@ class CronTriggersController(rest.RestController):
 
     @rest_utils.wrap_wsme_controller_exception
     @wsme_pecan.wsexpose(None, wtypes.text, status_code=204)
-    def delete(self, name):
+    def delete(self, identifier):
         """Delete cron trigger.
 
-        :param name: Name of cron trigger to delete
+        :param identifier: Id or name of cron trigger to delete
         """
         acl.enforce('cron_triggers:delete', context.ctx())
 
-        LOG.info("Delete cron trigger [name=%s]", name)
+        LOG.info("Delete cron trigger [identifier=%s]", identifier)
 
-        triggers.delete_cron_trigger(name)
+        triggers.delete_cron_trigger(identifier)
 
     @rest_utils.wrap_wsme_controller_exception
     @wsme_pecan.wsexpose(resources.CronTriggers, types.uuid, int,

mistral/db/v2/api.py

@@ -376,13 +376,17 @@ def delete_delayed_calls(**kwargs):
 
 # Cron triggers.
 
-def get_cron_trigger(name):
+def get_cron_trigger(identifier):
-    return IMPL.get_cron_trigger(name)
+    return IMPL.get_cron_trigger(identifier)
 
 
-def load_cron_trigger(name):
+def get_cron_trigger_by_id(id):
+    return IMPL.get_cron_trigger_by_id(id)
+
+
+def load_cron_trigger(identifier):
     """Unlike get_cron_trigger this method is allowed to return None."""
-    return IMPL.load_cron_trigger(name)
+    return IMPL.load_cron_trigger(identifier)
 
 
 def get_cron_triggers(**kwargs):
@@ -415,16 +419,17 @@ def create_cron_trigger(values):
     return IMPL.create_cron_trigger(values)
 
 
-def update_cron_trigger(name, values, query_filter=None):
+def update_cron_trigger(identifier, values, query_filter=None):
-    return IMPL.update_cron_trigger(name, values, query_filter=query_filter)
+    return IMPL.update_cron_trigger(identifier, values,
+                                    query_filter=query_filter)
 
 
-def create_or_update_cron_trigger(name, values):
+def create_or_update_cron_trigger(identifier, values):
-    return IMPL.create_or_update_cron_trigger(name, values)
+    return IMPL.create_or_update_cron_trigger(identifier, values)
 
 
-def delete_cron_trigger(name):
+def delete_cron_trigger(identifier):
-    return IMPL.delete_cron_trigger(name)
+    return IMPL.delete_cron_trigger(identifier)
 
 
 def delete_cron_triggers(**kwargs):

mistral/db/v2/sqlalchemy/api.py

@@ -1141,20 +1141,35 @@ def _get_completed_root_executions_query(columns):
 
 
 @b.session_aware()
-def get_cron_trigger(name, session=None):
+def get_cron_trigger(identifier, session=None):
-    cron_trigger = _get_db_object_by_name(models.CronTrigger, name)
+    cron_trigger = _get_db_object_by_name_or_id(
+        models.CronTrigger,
+        identifier)
 
     if not cron_trigger:
         raise exc.DBEntityNotFoundError(
-            "Cron trigger not found [name=%s]" % name
+            "Cron trigger not found [identifier=%s]" % identifier
         )
 
     return cron_trigger
 
 
 @b.session_aware()
-def load_cron_trigger(name, session=None):
+def get_cron_trigger_by_id(id, session=None):
-    return _get_db_object_by_name(models.CronTrigger, name)
+    ctx = context.ctx()
+    cron_trigger = _get_db_object_by_id(models.CronTrigger, id,
+                                        insecure=ctx.is_admin)
+    if not cron_trigger:
+        raise exc.DBEntityNotFoundError(
+            "Cron trigger not found [id=%s]" % id
+        )
+
+    return cron_trigger
+
+
+@b.session_aware()
+def load_cron_trigger(identifier, session=None):
+    return _get_db_object_by_name_or_id(models.CronTrigger, identifier)
 
 
 @b.session_aware()
@@ -1200,8 +1215,8 @@ def create_cron_trigger(values, session=None):
 
 
 @b.session_aware()
-def update_cron_trigger(name, values, session=None, query_filter=None):
+def update_cron_trigger(identifier, values, session=None, query_filter=None):
-    cron_trigger = get_cron_trigger(name)
+    cron_trigger = get_cron_trigger(identifier)
 
     if query_filter:
         try:
@@ -1233,19 +1248,19 @@ def update_cron_trigger(name, values, session=None, query_filter=None):
 
 
 @b.session_aware()
-def create_or_update_cron_trigger(name, values, session=None):
+def create_or_update_cron_trigger(identifier, values, session=None):
-    cron_trigger = _get_db_object_by_name(models.CronTrigger, name)
+    cron_trigger = _get_db_object_by_name_or_id(models.CronTrigger, identifier)
 
     if not cron_trigger:
         return create_cron_trigger(values)
     else:
-        updated, _ = update_cron_trigger(name, values)
+        updated, _ = update_cron_trigger(identifier, values)
         return updated
 
 
 @b.session_aware()
-def delete_cron_trigger(name, session=None):
+def delete_cron_trigger(identifier, session=None):
-    cron_trigger = get_cron_trigger(name)
+    cron_trigger = get_cron_trigger(identifier)
 
     # Delete the cron trigger by ID and get the affected row count.
     table = models.CronTrigger.__table__

mistral/services/triggers.py

@@ -136,12 +136,12 @@ def create_cron_trigger(name, workflow_name, workflow_input,
     return trig
 
 
-def delete_cron_trigger(name, trust_id=None, delete_trust=True):
+def delete_cron_trigger(identifier, trust_id=None, delete_trust=True):
     if not trust_id:
-        trigger = db_api.get_cron_trigger(name)
+        trigger = db_api.get_cron_trigger(identifier)
         trust_id = trigger.trust_id
 
-    modified_count = db_api.delete_cron_trigger(name)
+    modified_count = db_api.delete_cron_trigger(identifier)
 
     if modified_count and delete_trust:
         # Delete trust only together with deleting trigger.

mistral/tests/unit/api/v2/test_cron_triggers.py

@@ -37,7 +37,7 @@ WF = models.WorkflowDefinition(
 WF.update({'id': '123e4567-e89b-12d3-a456-426655440000', 'name': 'my_wf'})
 
 TRIGGER = {
-    'id': '123',
+    'id': '02abb422-55ef-4bb2-8cb9-217a583a6a3f',
     'name': 'my_cron_trigger',
     'pattern': '* * * * *',
     'workflow_name': WF.name,
@@ -94,6 +94,13 @@ class TestCronTriggerController(base.APITest):
 
         self.assertEqual(404, resp.status_int)
 
+    @mock.patch.object(db_api, "get_cron_trigger", MOCK_TRIGGER)
+    def test_get_by_id(self):
+        resp = self.app.get(
+            "/v2/cron_triggers/02abb422-55ef-4bb2-8cb9-217a583a6a3f")
+        self.assertEqual(200, resp.status_int)
+        self.assertDictEqual(TRIGGER, resp.json)
+
     @mock.patch.object(db_api, "get_workflow_definition", MOCK_WF)
     @mock.patch.object(db_api, "create_cron_trigger")
     def test_post(self, mock_mtd):
@@ -143,6 +150,16 @@ class TestCronTriggerController(base.APITest):
         self.assertEqual(1, delete_trust.call_count)
         self.assertEqual(204, resp.status_int)
 
+    @mock.patch.object(db_api, "get_cron_trigger", MOCK_TRIGGER)
+    @mock.patch.object(db_api, "delete_cron_trigger", MOCK_DELETE)
+    @mock.patch.object(security, "delete_trust")
+    def test_delete_by_id(self, delete_trust):
+        resp = self.app.delete(
+            '/v2/cron_triggers/02abb422-55ef-4bb2-8cb9-217a583a6a3f')
+
+        self.assertEqual(1, delete_trust.call_count)
+        self.assertEqual(204, resp.status_int)
+
     @mock.patch.object(db_api, "delete_cron_trigger", MOCK_NOT_FOUND)
     def test_delete_not_found(self):
         resp = self.app.delete(

mistral/tests/unit/db/v2/test_sqlalchemy_db_api.py

@@ -2190,6 +2190,7 @@ class TaskExecutionTest(SQLAlchemyTest):
 
 CRON_TRIGGERS = [
     {
+        'id': '11111111-1111-1111-1111-111111111111',
         'name': 'trigger1',
         'pattern': '* * * * *',
         'workflow_name': 'my_wf',
@@ -2202,6 +2203,7 @@ CRON_TRIGGERS = [
         'project_id': '<default-project>'
     },
     {
+        'id': '22222222-2222-2222-2222-2222222c2222',
         'name': 'trigger2',
         'pattern': '* * * * *',
         'workflow_name': 'my_wf',
@@ -2286,6 +2288,19 @@ class CronTriggerTest(SQLAlchemyTest):
         self.assertEqual(updated, updated)
         self.assertEqual(0, updated_count)
 
+    def test_update_cron_trigger_by_id(self):
+        created = db_api.create_cron_trigger(CRON_TRIGGERS[0])
+
+        self.assertIsNone(created.updated_at)
+
+        updated, updated_count = db_api.update_cron_trigger(
+            created.id,
+            {'pattern': '*/1 * * * *'}
+        )
+
+        self.assertEqual('*/1 * * * *', updated.pattern)
+        self.assertEqual(1, updated_count)
+
     def test_create_or_update_cron_trigger(self):
         name = 'not-existing-id'
 
@@ -2317,6 +2332,46 @@ class CronTriggerTest(SQLAlchemyTest):
         self.assertEqual(created0, fetched[0])
         self.assertEqual(created1, fetched[1])
 
+    def test_get_cron_trigger(self):
+        cron_trigger = db_api.create_cron_trigger(CRON_TRIGGERS[0])
+
+        # Get by id is ok
+        fetched = db_api.get_cron_trigger(cron_trigger.id)
+        self.assertEqual(cron_trigger, fetched)
+
+        # Get by name is ok
+        fetched = db_api.get_cron_trigger(cron_trigger.name)
+        self.assertEqual(cron_trigger, fetched)
+
+    def test_get_cron_trigger_not_found(self):
+        self.assertRaises(
+            exc.DBEntityNotFoundError,
+            db_api.get_cron_trigger,
+            'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa',
+        )
+
+        self.assertRaises(
+            exc.DBEntityNotFoundError,
+            db_api.get_cron_trigger,
+            'not-exists-cron-trigger',
+        )
+
+    def test_get_cron_trigger_by_id(self):
+        cron_trigger_1 = db_api.create_cron_trigger(CRON_TRIGGERS[0])
+        cron_trigger_2 = db_api.create_cron_trigger(CRON_TRIGGERS[1])
+
+        fetched = db_api.get_cron_trigger_by_id(cron_trigger_1.id)
+        self.assertEqual(cron_trigger_1, fetched)
+
+        fetched = db_api.get_cron_trigger_by_id(cron_trigger_2.id)
+        self.assertEqual(cron_trigger_2, fetched)
+
+        self.assertRaises(
+            exc.DBEntityNotFoundError,
+            db_api.get_cron_trigger_by_id,
+            'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa',
+        )
+
     def test_get_cron_triggers_other_tenant(self):
         created0 = db_api.create_cron_trigger(CRON_TRIGGERS[0])
 
@@ -2348,6 +2403,22 @@ class CronTriggerTest(SQLAlchemyTest):
             created.name
         )
 
+    def test_delete_cron_trigger_by_id(self):
+        created = db_api.create_cron_trigger(CRON_TRIGGERS[0])
+
+        fetched = db_api.get_cron_trigger(created.name)
+
+        self.assertEqual(created, fetched)
+
+        rowcount = db_api.delete_cron_trigger(created.id)
+
+        self.assertEqual(1, rowcount)
+        self.assertRaises(
+            exc.DBEntityNotFoundError,
+            db_api.get_cron_trigger,
+            created.id
+        )
+
     def test_cron_trigger_repr(self):
         s = db_api.create_cron_trigger(CRON_TRIGGERS[0]).__repr__()
 

releasenotes/notes/support-manage-cron-trigger-by-id-ab544e8068b84967.yaml

@@ -0,0 +1,3 @@
+---
+features:
+  - Support to manage a cron-trigger instance by id.