From 8960f401b71d02519267819cfc617b8cd7305d4c Mon Sep 17 00:00:00 2001 From: dgtlmoon Date: Mon, 13 Jan 2025 13:13:18 +0100 Subject: [PATCH] Notifications - Custom POST:// GET:// etc endpoints - returning 204 and other 20x responses are OK (don't show an error was detected)(#2897) --- changedetectionio/apprise_plugin/__init__.py | 8 +++----- changedetectionio/tests/test_notification.py | 16 ++++++++++++++-- changedetectionio/tests/util.py | 5 ++++- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/changedetectionio/apprise_plugin/__init__.py b/changedetectionio/apprise_plugin/__init__.py index ad8392cab1e..dc434673333 100644 --- a/changedetectionio/apprise_plugin/__init__.py +++ b/changedetectionio/apprise_plugin/__init__.py @@ -27,19 +27,17 @@ def apprise_custom_api_call_wrapper(body, title, notify_type, *args, **kwargs): method = re.sub(rf's$', '', schema) requests_method = getattr(requests, method) - headers = CaseInsensitiveDict({}) params = CaseInsensitiveDict({}) # Added to requests auth = None has_error = False - # Convert /foobar?+some-header=hello to proper header dictionary results = apprise_parse_url(url) # Add our headers that the user can potentially over-ride if they wish # to to our returned result set and tidy entries by unquoting them - headers = {unquote_plus(x): unquote_plus(y) - for x, y in results['qsd+'].items()} + headers = CaseInsensitiveDict({unquote_plus(x): unquote_plus(y) + for x, y in results['qsd+'].items()}) # https://github.com/caronc/apprise/wiki/Notify_Custom_JSON#get-parameter-manipulation # In Apprise, it relies on prefixing each request arg with "-", because it uses say &method=update as a flag for apprise @@ -81,7 +79,7 @@ def apprise_custom_api_call_wrapper(body, title, notify_type, *args, **kwargs): params=params ) - if r.status_code not in (requests.codes.created, requests.codes.ok): + if not (200 <= r.status_code < 300): status_str = f"Error sending '{method.upper()}' request to {url} - Status: {r.status_code}: '{r.reason}'" logger.error(status_str) has_error = True diff --git a/changedetectionio/tests/test_notification.py b/changedetectionio/tests/test_notification.py index 0c20a9f10b0..ceae292ed7d 100644 --- a/changedetectionio/tests/test_notification.py +++ b/changedetectionio/tests/test_notification.py @@ -29,7 +29,7 @@ def test_check_notification(client, live_server, measure_memory_usage): # Re 360 - new install should have defaults set res = client.get(url_for("settings_page")) - notification_url = url_for('test_notification_endpoint', _external=True).replace('http', 'json') + notification_url = url_for('test_notification_endpoint', _external=True).replace('http', 'json')+"?status_code=204" assert default_notification_body.encode() in res.data assert default_notification_title.encode() in res.data @@ -135,7 +135,14 @@ def test_check_notification(client, live_server, measure_memory_usage): # Trigger a check client.get(url_for("form_watch_checknow"), follow_redirects=True) + wait_for_all_checks(client) time.sleep(3) + + # Check no errors were recorded + res = client.get(url_for("index")) + assert b'notification-error' not in res.data + + # Verify what was sent as a notification, this file should exist with open("test-datastore/notification.txt", "r") as f: notification_submission = f.read() @@ -284,7 +291,7 @@ def test_notification_custom_endpoint_and_jinja2(client, live_server, measure_me # CUSTOM JSON BODY CHECK for POST:// set_original_response() # https://github.com/caronc/apprise/wiki/Notify_Custom_JSON#header-manipulation - test_notification_url = url_for('test_notification_endpoint', _external=True).replace('http://', 'post://')+"?xxx={{ watch_url }}&+custom-header=123&+second=hello+world%20%22space%22" + test_notification_url = url_for('test_notification_endpoint', _external=True).replace('http://', 'post://')+"?status_code=204&xxx={{ watch_url }}&+custom-header=123&+second=hello+world%20%22space%22" res = client.post( url_for("settings_page"), @@ -319,6 +326,11 @@ def test_notification_custom_endpoint_and_jinja2(client, live_server, measure_me time.sleep(2) # plus extra delay for notifications to fire + + # Check no errors were recorded, because we asked for 204 which is slightly uncommon but is still OK + res = client.get(url_for("index")) + assert b'notification-error' not in res.data + with open("test-datastore/notification.txt", 'r') as f: x = f.read() j = json.loads(x) diff --git a/changedetectionio/tests/util.py b/changedetectionio/tests/util.py index 9353155e784..250f6742eb5 100644 --- a/changedetectionio/tests/util.py +++ b/changedetectionio/tests/util.py @@ -244,8 +244,11 @@ def test_notification_endpoint(): f.write(request.content_type) print("\n>> Test notification endpoint was hit.\n", data) - return "Text was set" + content = "Text was set" + status_code = request.args.get('status_code',200) + resp = make_response(content, status_code) + return resp # Just return the verb in the request @live_server.app.route('/test-basicauth', methods=['GET'])