Skip to content

Commit

Permalink
Fix off-by-one random trigger caused by watch going into queue AND be…
Browse files Browse the repository at this point in the history
…ing hit with [edit]
  • Loading branch information
dgtlmoon committed Sep 17, 2024
1 parent d250ff9 commit e1211db
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 67 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@

import apprise
# include the decorator
from apprise.decorators import notify

Expand Down
2 changes: 1 addition & 1 deletion changedetectionio/flask_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ def ajax_callback_send_notification_test(watch_uuid=None):
from .apprise_asset import asset
apobj = apprise.Apprise(asset=asset)
# so that the custom endpoints are registered
from changedetectionio.apprise import apprise_custom_api_call_wrapper
from changedetectionio.apprise_plugin import apprise_custom_api_call_wrapper
is_global_settings_form = request.args.get('mode', '') == 'global-settings'
is_group_settings_form = request.args.get('mode', '') == 'group-settings'

Expand Down
2 changes: 1 addition & 1 deletion changedetectionio/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ def __call__(self, form, field):
import apprise
apobj = apprise.Apprise()
# so that the custom endpoints are registered
from changedetectionio.apprise import apprise_custom_api_call_wrapper
from changedetectionio.apprise_plugin import apprise_custom_api_call_wrapper
for server_url in field.data:
if not apobj.add(server_url):
message = field.gettext('\'%s\' is not a valid AppRise URL.' % (server_url))
Expand Down
3 changes: 2 additions & 1 deletion changedetectionio/notification.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@

def process_notification(n_object, datastore):
# so that the custom endpoints are registered
from changedetectionio.apprise import apprise_custom_api_call_wrapper
from changedetectionio.apprise_plugin import apprise_custom_api_call_wrapper

from .safe_jinja import render as jinja_render
now = time.time()
if n_object.get('notification_timestamp'):
Expand Down
123 changes: 66 additions & 57 deletions changedetectionio/tests/test_filter_failure_notification.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ def run_filter_test(client, live_server, content_filter):
# Response WITHOUT the filter ID element
set_original_response()

# Goto the edit page, add our ignore text
notification_url = url_for('test_notification_endpoint', _external=True).replace('http', 'json')

# Add our URL to the import page
test_url = url_for('test_endpoint', _external=True)

# cleanup for the next
client.get(
url_for("form_delete", uuid="all"),
Expand All @@ -36,87 +42,93 @@ def run_filter_test(client, live_server, content_filter):
if os.path.isfile("test-datastore/notification.txt"):
os.unlink("test-datastore/notification.txt")

# Add our URL to the import page
test_url = url_for('test_endpoint', _external=True)
res = client.post(
url_for("form_quick_watch_add"),
data={"url": test_url, "tags": ''},
url_for("import_page"),
data={"urls": test_url},
follow_redirects=True
)

assert b"Watch added" in res.data

# Give the thread time to pick up the first version
assert b"1 Imported" in res.data
wait_for_all_checks(client)

# Goto the edit page, add our ignore text
# Add our URL to the import page
url = url_for('test_notification_endpoint', _external=True)
notification_url = url.replace('http', 'json')

print(">>>> Notification URL: " + notification_url)

# Just a regular notification setting, this will be used by the special 'filter not found' notification
notification_form_data = {"notification_urls": notification_url,
"notification_title": "New ChangeDetection.io Notification - {{watch_url}}",
"notification_body": "BASE URL: {{base_url}}\n"
"Watch URL: {{watch_url}}\n"
"Watch UUID: {{watch_uuid}}\n"
"Watch title: {{watch_title}}\n"
"Watch tag: {{watch_tag}}\n"
"Preview: {{preview_url}}\n"
"Diff URL: {{diff_url}}\n"
"Snapshot: {{current_snapshot}}\n"
"Diff: {{diff}}\n"
"Diff Full: {{diff_full}}\n"
"Diff as Patch: {{diff_patch}}\n"
":-)",
"notification_format": "Text"}

notification_form_data.update({
"url": test_url,
"tags": "my tag",
"title": "my title 123",
"headers": "",
"filter_failure_notification_send": 'y',
"include_filters": content_filter,
"fetch_backend": "html_requests"})

# A POST here will also reset the filter failure counter (filter_failure_notification_threshold_attempts)
uuid = extract_UUID_from_client(client)

assert live_server.app.config['DATASTORE'].data['watching'][uuid]['consecutive_filter_failures'] == 0, "No filter = No filter failure"

watch_data = {"notification_urls": notification_url,
"notification_title": "New ChangeDetection.io Notification - {{watch_url}}",
"notification_body": "BASE URL: {{base_url}}\n"
"Watch URL: {{watch_url}}\n"
"Watch UUID: {{watch_uuid}}\n"
"Watch title: {{watch_title}}\n"
"Watch tag: {{watch_tag}}\n"
"Preview: {{preview_url}}\n"
"Diff URL: {{diff_url}}\n"
"Snapshot: {{current_snapshot}}\n"
"Diff: {{diff}}\n"
"Diff Full: {{diff_full}}\n"
"Diff as Patch: {{diff_patch}}\n"
":-)",
"notification_format": "Text",
"fetch_backend": "html_requests",
"filter_failure_notification_send": 'y',
"headers": "",
"tags": "my tag",
"title": "my title 123",
"time_between_check-hours": 5, # So that the queue runner doesnt also put it in
"url": test_url,
}

res = client.post(
url_for("edit_page", uuid="first"),
data=notification_form_data,
url_for("edit_page", uuid=uuid),
data=watch_data,
follow_redirects=True
)

assert b"Updated watch." in res.data
wait_for_all_checks(client)
assert live_server.app.config['DATASTORE'].data['watching'][uuid]['consecutive_filter_failures'] == 0, "No filter = No filter failure"

# Now add a filter, because recheck hours == 5, ONLY pressing of the [edit] or [recheck all] should trigger
watch_data['include_filters'] = content_filter
res = client.post(
url_for("edit_page", uuid=uuid),
data=watch_data,
follow_redirects=True
)
assert b"Updated watch." in res.data

# It should have checked once so far and given this error (because we hit SAVE)

# Now the notification should not exist, because we didnt reach the threshold
wait_for_all_checks(client)
assert not os.path.isfile("test-datastore/notification.txt")
# for uuid, watch in client.application.config.get('DATASTORE').data.get('watching').items():
# logger.debug(f"{uuid} has consecutive_filter_failures: {watch.get('consecutive_filter_failures', 5)}")
time.sleep(2)

# Hitting [save] would have triggered a recheck, and we have a filter, so this would be ONE failure
assert live_server.app.config['DATASTORE'].data['watching'][uuid]['consecutive_filter_failures'] == 1, "Should have been checked once"


# recheck it up to just before the threshold, including the fact that in the previous POST it would have rechecked (and incremented)
for i in range(0, App._FILTER_FAILURE_THRESHOLD_ATTEMPTS_DEFAULT-1):
# Add 4 more checks
checked=0
attempt_threshold_setting = live_server.app.config['DATASTORE'].data['settings']['application'].get('filter_failure_notification_threshold_attempts', 0)
for i in range(0, attempt_threshold_setting-2):
checked+=1
client.get(url_for("form_watch_checknow"), follow_redirects=True)
wait_for_all_checks(client)
time.sleep(2) # delay for apprise to fire
res = client.get(url_for("index"))
assert b'Warning, no filters were found' in res.data
assert not os.path.isfile("test-datastore/notification.txt"), f"test-datastore/notification.txt should not exist - Attempt {i} when threshold is {App._FILTER_FAILURE_THRESHOLD_ATTEMPTS_DEFAULT}"

# We should see something in the frontend
res = client.get(url_for("index"))
assert b'Warning, no filters were found' in res.data
assert live_server.app.config['DATASTORE'].data['watching'][uuid]['consecutive_filter_failures'] == 5

# One more check should trigger the _FILTER_FAILURE_THRESHOLD_ATTEMPTS_DEFAULT threshold
client.get(url_for("form_watch_checknow"), follow_redirects=True)
wait_for_all_checks(client)

wait_for_notification_endpoint_output()

# Now it should exist and contain our "filter not found" alert
assert os.path.isfile("test-datastore/notification.txt")

with open("test-datastore/notification.txt", 'r') as f:
notification = f.read()

Expand All @@ -129,7 +141,7 @@ def run_filter_test(client, live_server, content_filter):
set_response_with_filter()

# Try several times, it should NOT have 'filter not found'
for i in range(0, App._FILTER_FAILURE_THRESHOLD_ATTEMPTS_DEFAULT):
for i in range(0, attempt_threshold_setting+2):
client.get(url_for("form_watch_checknow"), follow_redirects=True)
wait_for_all_checks(client)

Expand All @@ -142,9 +154,6 @@ def run_filter_test(client, live_server, content_filter):
assert not 'CSS/xPath filter was not present in the page' in notification

# Re #1247 - All tokens got replaced correctly in the notification
res = client.get(url_for("index"))
uuid = extract_UUID_from_client(client)
# UUID is correct, but notification contains tag uuid as UUIID wtf
assert uuid in notification

# cleanup for the next
Expand Down
10 changes: 4 additions & 6 deletions changedetectionio/update_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -365,15 +365,13 @@ def run(self):
watch.save_xpath_data(data=e.xpath_data)

# Only when enabled, send the notification

if watch.get('filter_failure_notification_send', False):
c = watch.get('consecutive_filter_failures', 5)
c = watch.get('consecutive_filter_failures', 0)
c += 1
# Send notification if we reached the threshold?
threshold = self.datastore.data['settings']['application'].get('filter_failure_notification_threshold_attempts', 0)
# zero/first should also be 1
c += 1
logger.debug(f"Filter for {uuid} not found, consecutive_filter_failures: {c} of threshold {threshold}")
if threshold and c >= threshold:
if c >= threshold:
if not watch.get('notification_muted'):
logger.debug(f"Sending filter failed notification for {uuid}")
self.send_filter_failure_notification(uuid)
Expand Down Expand Up @@ -430,7 +428,7 @@ def run(self):
)

if watch.get('filter_failure_notification_send', False):
c = watch.get('consecutive_filter_failures', 5)
c = watch.get('consecutive_filter_failures', 0)
c += 1
# Send notification if we reached the threshold?
threshold = self.datastore.data['settings']['application'].get('filter_failure_notification_threshold_attempts',
Expand Down

0 comments on commit e1211db

Please sign in to comment.