From 7ee2b4617177b83fba8dee0c196d40d2dae23a5d Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Sat, 27 May 2023 16:49:13 +0530 Subject: [PATCH 1/6] feat: Allow setting cron as Server Script frequency (cherry picked from commit b00aac92ba741bd3f25cb2302bd2018d1a49f61b) # Conflicts: # frappe/core/doctype/server_script/server_script.json --- .../doctype/server_script/server_script.json | 42 ++++++++++++++++++- .../doctype/server_script/server_script.py | 8 +++- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/frappe/core/doctype/server_script/server_script.json b/frappe/core/doctype/server_script/server_script.json index 5446cc1a3969..c0c793fd276f 100644 --- a/frappe/core/doctype/server_script/server_script.json +++ b/frappe/core/doctype/server_script/server_script.json @@ -9,6 +9,7 @@ "script_type", "reference_doctype", "event_frequency", + "cron_format", "doctype_event", "api_method", "allow_guest", @@ -95,13 +96,48 @@ "fieldtype": "Select", "label": "Event Frequency", "mandatory_depends_on": "eval:doc.script_type == \"Scheduler Event\"", - "options": "All\nHourly\nDaily\nWeekly\nMonthly\nYearly\nHourly Long\nDaily Long\nWeekly Long\nMonthly Long" + "options": "All\nHourly\nDaily\nWeekly\nMonthly\nYearly\nHourly Long\nDaily Long\nWeekly Long\nMonthly Long\nCron" }, { "fieldname": "module", "fieldtype": "Link", "label": "Module (for export)", "options": "Module Def" +<<<<<<< HEAD +======= + }, + { + "depends_on": "eval:doc.script_type==='API'", + "fieldname": "rate_limiting_section", + "fieldtype": "Section Break", + "label": "Rate Limiting" + }, + { + "default": "0", + "fieldname": "enable_rate_limit", + "fieldtype": "Check", + "label": "Enable Rate Limit" + }, + { + "default": "5", + "depends_on": "enable_rate_limit", + "fieldname": "rate_limit_count", + "fieldtype": "Int", + "label": "Request Limit" + }, + { + "default": "86400", + "depends_on": "enable_rate_limit", + "fieldname": "rate_limit_seconds", + "fieldtype": "Int", + "label": "Time Window (Seconds)" + }, + { + "depends_on": "eval:doc.event_frequency==='Cron'", + "fieldname": "cron_format", + "fieldtype": "Data", + "label": "Cron Format" +>>>>>>> b00aac92ba (feat: Allow setting cron as Server Script frequency) } ], "index_web_pages_for_search": 1, @@ -111,7 +147,11 @@ "link_fieldname": "server_script" } ], +<<<<<<< HEAD "modified": "2022-06-13 06:04:20.937969", +======= + "modified": "2023-05-27 16:33:16.595424", +>>>>>>> b00aac92ba (feat: Allow setting cron as Server Script frequency) "modified_by": "Administrator", "module": "Core", "name": "Server Script", diff --git a/frappe/core/doctype/server_script/server_script.py b/frappe/core/doctype/server_script/server_script.py index 17cddee1e88b..86a3414d00b7 100644 --- a/frappe/core/doctype/server_script/server_script.py +++ b/frappe/core/doctype/server_script/server_script.py @@ -50,7 +50,9 @@ def sync_scheduled_jobs(self): def sync_scheduler_events(self): """Create or update Scheduled Job Type documents for Scheduler Event Server Scripts""" if not self.disabled and self.event_frequency and self.script_type == "Scheduler Event": - setup_scheduler_events(script_name=self.name, frequency=self.event_frequency) + setup_scheduler_events( + script_name=self.name, frequency=self.event_frequency, cron_format=self.cron_format + ) def clear_scheduled_events(self): """Deletes existing scheduled jobs by Server Script if self.event_frequency has changed""" @@ -169,7 +171,7 @@ def get_keys(obj): return items -def setup_scheduler_events(script_name, frequency): +def setup_scheduler_events(script_name: str, frequency: str, cron_format: str | None = None): """Creates or Updates Scheduled Job Type documents based on the specified script name and frequency Args: @@ -186,6 +188,7 @@ def setup_scheduler_events(script_name, frequency): "method": method, "frequency": frequency, "server_script": script_name, + "cron_format": cron_format, } ).insert() @@ -198,6 +201,7 @@ def setup_scheduler_events(script_name, frequency): return doc.frequency = frequency + doc.cron_format = cron_format doc.save() frappe.msgprint(_("Scheduled execution for script {0} has updated").format(script_name)) From 908805873429aa06120d43a58423ffed425b292e Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 29 May 2023 17:42:04 +0530 Subject: [PATCH 2/6] test: cron/scheduled scripts (cherry picked from commit 888209f6d2d11967a7422445a96b737ae9405ece) # Conflicts: # frappe/core/doctype/server_script/test_server_script.py --- .../server_script/test_server_script.py | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/frappe/core/doctype/server_script/test_server_script.py b/frappe/core/doctype/server_script/test_server_script.py index 3abc53bd525f..bae8f8389ec5 100644 --- a/frappe/core/doctype/server_script/test_server_script.py +++ b/frappe/core/doctype/server_script/test_server_script.py @@ -3,6 +3,11 @@ import requests import frappe +<<<<<<< HEAD +======= +from frappe.core.doctype.scheduled_job_type.scheduled_job_type import sync_jobs +from frappe.frappeclient import FrappeClient, FrappeException +>>>>>>> 888209f6d2 (test: cron/scheduled scripts) from frappe.tests.utils import FrappeTestCase from frappe.utils import get_site_url @@ -233,3 +238,88 @@ def test_scripts_all_the_way_down(self): ) script.insert() script.execute_method() +<<<<<<< HEAD +======= + + def test_server_script_rate_limiting(self): + # why not + script1 = frappe.get_doc( + doctype="Server Script", + name="rate_limited_server_script", + script_type="API", + enable_rate_limit=1, + allow_guest=1, + rate_limit_count=5, + api_method="rate_limited_endpoint", + script="""frappe.flags = {"test": True}""", + ) + + script1.insert() + + script2 = frappe.get_doc( + doctype="Server Script", + name="rate_limited_server_script2", + script_type="API", + enable_rate_limit=1, + allow_guest=1, + rate_limit_count=5, + api_method="rate_limited_endpoint2", + script="""frappe.flags = {"test": False}""", + ) + + script2.insert() + + frappe.db.commit() + + site = frappe.utils.get_site_url(frappe.local.site) + client = FrappeClient(site) + + # Exhaust rate limti + for _ in range(5): + client.get_api(script1.api_method) + + self.assertRaises(FrappeException, client.get_api, script1.api_method) + + # Exhaust rate limti + for _ in range(5): + client.get_api(script2.api_method) + + self.assertRaises(FrappeException, client.get_api, script2.api_method) + + script1.delete() + script2.delete() + frappe.db.commit() + + def test_server_script_scheduled(self): + scheduled_script = frappe.get_doc( + doctype="Server Script", + name="scheduled_script_wo_cron", + script_type="Scheduler Event", + script="""frappe.flags = {"test": True}""", + event_frequency="Hourly", + ).insert() + + cron_script = frappe.get_doc( + doctype="Server Script", + name="scheduled_script_w_cron", + script_type="Scheduler Event", + script="""frappe.flags = {"test": True}""", + event_frequency="Cron", + cron_format="0 0 1 1 *", # 1st january + ).insert() + + # Ensure that jobs remain in DB after migrate + sync_jobs() + self.assertTrue(frappe.db.exists("Scheduled Job Type", {"server_script": scheduled_script.name})) + + cron_job_name = frappe.db.get_value("Scheduled Job Type", {"server_script": cron_script.name}) + self.assertTrue(cron_job_name) + + cron_job = frappe.get_doc("Scheduled Job Type", cron_job_name) + self.assertEqual(cron_job.next_execution.day, 1) + self.assertEqual(cron_job.next_execution.month, 1) + + cron_script.cron_format = "0 0 2 1 *" # 2nd january + cron_job.reload() + self.assertEqual(cron_job.next_execution.day, 2) +>>>>>>> 888209f6d2 (test: cron/scheduled scripts) From 9d413caf0ab2286b9d23437a566161830e76e5ec Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 29 May 2023 18:16:23 +0530 Subject: [PATCH 3/6] fix: Pass cron format only if frequency is Cron (cherry picked from commit a1b16792f96aa97d9525fd731445b378ce7390dd) --- frappe/core/doctype/server_script/server_script.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frappe/core/doctype/server_script/server_script.py b/frappe/core/doctype/server_script/server_script.py index 86a3414d00b7..0896eb663156 100644 --- a/frappe/core/doctype/server_script/server_script.py +++ b/frappe/core/doctype/server_script/server_script.py @@ -50,8 +50,9 @@ def sync_scheduled_jobs(self): def sync_scheduler_events(self): """Create or update Scheduled Job Type documents for Scheduler Event Server Scripts""" if not self.disabled and self.event_frequency and self.script_type == "Scheduler Event": + cron_format = self.cron_format if self.event_frequency == "Cron" else None setup_scheduler_events( - script_name=self.name, frequency=self.event_frequency, cron_format=self.cron_format + script_name=self.name, frequency=self.event_frequency, cron_format=cron_format ) def clear_scheduled_events(self): From 5ba85e7359a12060debf2dc730f053b23c01b3b4 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 29 May 2023 18:30:17 +0530 Subject: [PATCH 4/6] test: save server script (cherry picked from commit 9f53b56bdb29401a85a52f98c35d8684526cc2eb) --- frappe/core/doctype/server_script/test_server_script.py | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/core/doctype/server_script/test_server_script.py b/frappe/core/doctype/server_script/test_server_script.py index bae8f8389ec5..d7d02825243c 100644 --- a/frappe/core/doctype/server_script/test_server_script.py +++ b/frappe/core/doctype/server_script/test_server_script.py @@ -320,6 +320,7 @@ def test_server_script_scheduled(self): self.assertEqual(cron_job.next_execution.month, 1) cron_script.cron_format = "0 0 2 1 *" # 2nd january + cron_script.save() cron_job.reload() self.assertEqual(cron_job.next_execution.day, 2) >>>>>>> 888209f6d2 (test: cron/scheduled scripts) From 69aadb3546ed6346fd8bb58dd74cea481b2ae2fa Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 29 May 2023 18:34:39 +0530 Subject: [PATCH 5/6] fix: Clear scheduled events if cron_format is changed (cherry picked from commit cdef2ccdd6a48d333c00d0dd651510fd84b185fe) --- frappe/core/doctype/server_script/server_script.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/server_script/server_script.py b/frappe/core/doctype/server_script/server_script.py index 0896eb663156..da9c999ef6da 100644 --- a/frappe/core/doctype/server_script/server_script.py +++ b/frappe/core/doctype/server_script/server_script.py @@ -56,8 +56,10 @@ def sync_scheduler_events(self): ) def clear_scheduled_events(self): - """Deletes existing scheduled jobs by Server Script if self.event_frequency has changed""" - if self.script_type == "Scheduler Event" and self.has_value_changed("event_frequency"): + """Deletes existing scheduled jobs by Server Script if self.event_frequency or self.cron_format has changed""" + if self.script_type == "Scheduler Event" and ( + self.has_value_changed("event_frequency") or self.has_value_changed("cron_format") + ): for scheduled_job in self.scheduled_jobs: frappe.delete_doc("Scheduled Job Type", scheduled_job.name) From 5883227c5a54f5877e6a19962d34cf40714ace67 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 30 May 2023 11:14:08 +0530 Subject: [PATCH 6/6] chore: Resolve conflicts --- .../doctype/server_script/server_script.json | 33 ----------- .../server_script/test_server_script.py | 56 ------------------- 2 files changed, 89 deletions(-) diff --git a/frappe/core/doctype/server_script/server_script.json b/frappe/core/doctype/server_script/server_script.json index c0c793fd276f..597a3067ae02 100644 --- a/frappe/core/doctype/server_script/server_script.json +++ b/frappe/core/doctype/server_script/server_script.json @@ -103,41 +103,12 @@ "fieldtype": "Link", "label": "Module (for export)", "options": "Module Def" -<<<<<<< HEAD -======= - }, - { - "depends_on": "eval:doc.script_type==='API'", - "fieldname": "rate_limiting_section", - "fieldtype": "Section Break", - "label": "Rate Limiting" - }, - { - "default": "0", - "fieldname": "enable_rate_limit", - "fieldtype": "Check", - "label": "Enable Rate Limit" - }, - { - "default": "5", - "depends_on": "enable_rate_limit", - "fieldname": "rate_limit_count", - "fieldtype": "Int", - "label": "Request Limit" - }, - { - "default": "86400", - "depends_on": "enable_rate_limit", - "fieldname": "rate_limit_seconds", - "fieldtype": "Int", - "label": "Time Window (Seconds)" }, { "depends_on": "eval:doc.event_frequency==='Cron'", "fieldname": "cron_format", "fieldtype": "Data", "label": "Cron Format" ->>>>>>> b00aac92ba (feat: Allow setting cron as Server Script frequency) } ], "index_web_pages_for_search": 1, @@ -147,11 +118,7 @@ "link_fieldname": "server_script" } ], -<<<<<<< HEAD "modified": "2022-06-13 06:04:20.937969", -======= - "modified": "2023-05-27 16:33:16.595424", ->>>>>>> b00aac92ba (feat: Allow setting cron as Server Script frequency) "modified_by": "Administrator", "module": "Core", "name": "Server Script", diff --git a/frappe/core/doctype/server_script/test_server_script.py b/frappe/core/doctype/server_script/test_server_script.py index d7d02825243c..704e33a19525 100644 --- a/frappe/core/doctype/server_script/test_server_script.py +++ b/frappe/core/doctype/server_script/test_server_script.py @@ -3,11 +3,7 @@ import requests import frappe -<<<<<<< HEAD -======= from frappe.core.doctype.scheduled_job_type.scheduled_job_type import sync_jobs -from frappe.frappeclient import FrappeClient, FrappeException ->>>>>>> 888209f6d2 (test: cron/scheduled scripts) from frappe.tests.utils import FrappeTestCase from frappe.utils import get_site_url @@ -238,57 +234,6 @@ def test_scripts_all_the_way_down(self): ) script.insert() script.execute_method() -<<<<<<< HEAD -======= - - def test_server_script_rate_limiting(self): - # why not - script1 = frappe.get_doc( - doctype="Server Script", - name="rate_limited_server_script", - script_type="API", - enable_rate_limit=1, - allow_guest=1, - rate_limit_count=5, - api_method="rate_limited_endpoint", - script="""frappe.flags = {"test": True}""", - ) - - script1.insert() - - script2 = frappe.get_doc( - doctype="Server Script", - name="rate_limited_server_script2", - script_type="API", - enable_rate_limit=1, - allow_guest=1, - rate_limit_count=5, - api_method="rate_limited_endpoint2", - script="""frappe.flags = {"test": False}""", - ) - - script2.insert() - - frappe.db.commit() - - site = frappe.utils.get_site_url(frappe.local.site) - client = FrappeClient(site) - - # Exhaust rate limti - for _ in range(5): - client.get_api(script1.api_method) - - self.assertRaises(FrappeException, client.get_api, script1.api_method) - - # Exhaust rate limti - for _ in range(5): - client.get_api(script2.api_method) - - self.assertRaises(FrappeException, client.get_api, script2.api_method) - - script1.delete() - script2.delete() - frappe.db.commit() def test_server_script_scheduled(self): scheduled_script = frappe.get_doc( @@ -323,4 +268,3 @@ def test_server_script_scheduled(self): cron_script.save() cron_job.reload() self.assertEqual(cron_job.next_execution.day, 2) ->>>>>>> 888209f6d2 (test: cron/scheduled scripts)