From 6a58c144f540508e3a75c977585577e0b757d550 Mon Sep 17 00:00:00 2001
From: Karthikeyan Chinnakonda <karthikeyan@hasura.io>
Date: Tue, 23 Jun 2020 20:51:34 +0530
Subject: [PATCH] server: fix updating of headers behaviour in the update cron
 trigger API and create future events immediately (#5151)

* server: fix bug to update headers in an existing cron trigger and create future events

Co-authored-by: Tirumarai Selvan <tiru@hasura.io>
---
 .../Hasura/RQL/DDL/ScheduledTrigger.hs        | 11 ++-
 server/tests-py/test_scheduled_triggers.py    | 70 ++++++++++++++++++-
 2 files changed, 77 insertions(+), 4 deletions(-)

diff --git a/server/src-lib/Hasura/RQL/DDL/ScheduledTrigger.hs b/server/src-lib/Hasura/RQL/DDL/ScheduledTrigger.hs
index cc2322d99c0..8baed4acbea 100644
--- a/server/src-lib/Hasura/RQL/DDL/ScheduledTrigger.hs
+++ b/server/src-lib/Hasura/RQL/DDL/ScheduledTrigger.hs
@@ -95,10 +95,11 @@ updateCronTriggerInCatalog CronTriggerMetadata {..} = liftTx $ do
         cron_schedule = $3,
         payload = $4,
         retry_conf = $5,
-        include_in_metadata = $6,
-        comment = $7
+        header_conf = $6,
+        include_in_metadata = $7,
+        comment = $8
     WHERE name = $1
-   |] (ctName, Q.AltJ ctWebhook, ctSchedule, Q.AltJ <$> ctPayload, Q.AltJ ctRetryConf
+   |] (ctName, Q.AltJ ctWebhook, ctSchedule, Q.AltJ <$> ctPayload, Q.AltJ ctRetryConf,Q.AltJ ctHeaders
       , ctIncludeInMetadata, ctComment) False
   -- since the cron trigger is updated, clear all its future events which are not retries
   Q.unitQE defaultTxErrorHandler
@@ -106,6 +107,10 @@ updateCronTriggerInCatalog CronTriggerMetadata {..} = liftTx $ do
     DELETE FROM hdb_catalog.hdb_cron_events
     WHERE trigger_name = $1 AND scheduled_time > now() AND tries = 0
    |] (Identity ctName) False
+  -- create the next 100 cron events, as the future events were deleted
+  currentTime <- liftIO C.getCurrentTime
+  let scheduleTimes = generateScheduleTimes currentTime 100 ctSchedule
+  insertCronEvents $ map (CronEventSeed ctName) scheduleTimes
 
 runDeleteCronTrigger :: (CacheRWM m, MonadTx m) => ScheduledTriggerName -> m EncJSON
 runDeleteCronTrigger (ScheduledTriggerName stName) = do
diff --git a/server/tests-py/test_scheduled_triggers.py b/server/tests-py/test_scheduled_triggers.py
index 5db3851f048..c2587eca1e8 100644
--- a/server/tests-py/test_scheduled_triggers.py
+++ b/server/tests-py/test_scheduled_triggers.py
@@ -5,6 +5,7 @@ from datetime import datetime,timedelta
 from croniter import croniter
 from validate import validate_event_webhook,validate_event_headers
 from queue import Empty
+import json
 import time
 
 # The create and delete tests should ideally go in setup and teardown YAML files,
@@ -142,7 +143,7 @@ class TestCronTrigger(object):
         TestCronTrigger.init_time = datetime.utcnow()
         # the cron events will be generated based on the current time, they
         # will not be exactly the same though(the server now and now here)
-        assert cron_st_code == 200,resp
+        assert cron_st_code == 200,cron_st_resp
         assert cron_st_resp['message'] == 'success'
 
     def test_check_generated_cron_scheduled_events(self,hge_ctx):
@@ -174,6 +175,73 @@ class TestCronTrigger(object):
             actual_schedule_timestamps.append(datetime_ts)
         assert actual_schedule_timestamps == expected_schedule_timestamps
 
+    def test_update_existing_cron_trigger(self,hge_ctx):
+        expected_schedule_timestamps = []
+        iter = croniter(self.cron_schedule,datetime.utcnow())
+        for i in range(100):
+            expected_schedule_timestamps.append(iter.next(datetime))
+        q = {
+            "type":"create_cron_trigger",
+            "args":{
+                "name":self.cron_trigger_name,
+                "webhook":"{{SCHEDULED_TRIGGERS_WEBHOOK_DOMAIN}}" + "/foo",
+                "schedule":self.cron_schedule,
+                "headers":[
+                    {
+                        "name":"header-name",
+                        "value":"header-value"
+                    }
+                ],
+                "payload":{"foo":"baz"},
+                "include_in_metadata":False,
+                "replace":True
+            }
+        }
+        st,resp = hge_ctx.v1q(q)
+        assert st == 200, resp
+
+        sql = '''
+        select header_conf::json
+        from hdb_catalog.hdb_cron_triggers where
+        name = '{}' '''
+        q = {
+            "type":"run_sql",
+            "args":{
+                "sql":sql.format(self.cron_trigger_name)
+            }
+        }
+        st,resp = hge_ctx.v1q(q)
+        assert st == 200,resp
+        print ("resp",resp['result'][1][0])
+        assert json.loads(resp['result'][1][0]) == [{
+            "name":"header-name",
+            "value":"header-value"
+        }]
+
+        # Get timestamps in UTC from the db to compare it with
+        # the croniter generated timestamps
+        # After updating the cron trigger, the future events should
+        # have been created
+        sql = '''
+        select timezone('utc',scheduled_time) as scheduled_time
+        from hdb_catalog.hdb_cron_events where
+        trigger_name = '{}' order by scheduled_time asc;'''
+        q = {
+            "type":"run_sql",
+            "args":{
+                "sql":sql.format(self.cron_trigger_name)
+            }
+        }
+        st,resp = hge_ctx.v1q(q)
+        assert st == 200,resp
+        ts_resp = resp['result'][1:]
+        assert len(ts_resp) == 100
+        actual_schedule_timestamps = []
+        for ts in ts_resp:
+            datetime_ts = datetime.strptime(ts[0],"%Y-%m-%d %H:%M:%S")
+            actual_schedule_timestamps.append(datetime_ts)
+        assert actual_schedule_timestamps == expected_schedule_timestamps
+
     def test_delete_cron_scheduled_trigger(self,hge_ctx):
         q = {
             "type":"delete_cron_trigger",