Merge pull request #2061 from nwithan8/bugfix/vcalendar_hook

Improve/reduce unneeded notification hook preparation
This commit is contained in:
Peter Bieringer
2026-03-30 18:44:17 +02:00
committed by GitHub
7 changed files with 123 additions and 61 deletions

View File

@@ -89,34 +89,38 @@ class ApplicationPartDelete(ApplicationBase):
if not access.check("D", item):
logger.info("delete of collection is prevented by config/option [rights] permit_delete_collection and not explicit allowed by permission 'D': %s", path)
return httputils.NOT_ALLOWED
for i in item.get_all():
hook_notification_item_list.append(
HookNotificationItem(
notification_item_type=HookNotificationItemTypes.DELETE,
path=access.path,
content=i.uid,
uid=i.uid,
old_content=i.serialize(), # type: ignore
new_content=None
if self._hook.enabled:
for i in item.get_all():
hook_notification_item_list.append(
HookNotificationItem(
notification_item_type=HookNotificationItemTypes.DELETE,
path=access.path,
content=i.uid,
content_type=i.name,
uid=i.uid,
old_content=i.serialize(), # type: ignore
new_content=None
)
)
)
xml_answer = xml_delete(base_prefix, path, item)
else:
assert item.collection is not None
assert item.href is not None
hook_notification_item_list.append(
HookNotificationItem(
notification_item_type=HookNotificationItemTypes.DELETE,
path=access.path,
content=item.uid,
uid=item.uid,
old_content=item.serialize(), # type: ignore
new_content=None,
if self._hook.enabled:
hook_notification_item_list.append(
HookNotificationItem(
notification_item_type=HookNotificationItemTypes.DELETE,
path=access.path,
content=item.uid,
content_type=item.name,
uid=item.uid,
old_content=item.serialize(), # type: ignore
new_content=None,
)
)
)
xml_answer = xml_delete(
base_prefix, path, item.collection, item.href)
for notification_item in hook_notification_item_list:
for notification_item in hook_notification_item_list: # Will be empty if hook not enabled
self._hook.notify(notification_item)
headers = {"Content-Type": "text/xml; charset=%s" % self._encoding}
return client.OK, headers, self._xml_response(xml_answer), None

View File

@@ -205,15 +205,17 @@ class ApplicationPartProppatch(ApplicationBase):
xml_content,
encoding=self._encoding
).decode(encoding=self._encoding)
hook_notification_item = HookNotificationItem(
notification_item_type=HookNotificationItemTypes.CPATCH,
path=access.path,
content=content,
uid=None,
old_content=None,
new_content=content
)
self._hook.notify(hook_notification_item)
if self._hook.enabled:
hook_notification_item = HookNotificationItem(
notification_item_type=HookNotificationItemTypes.CPATCH,
path=access.path,
content=content,
content_type=None, # Can't easily determine content type, won't trigger email hook
uid=None,
old_content=None,
new_content=content
)
self._hook.notify(hook_notification_item)
except ValueError as e:
# return better matching HTTP result in case errno is provided and catched
errno_match = re.search("\\[Errno ([0-9]+)\\]", str(e))

View File

@@ -306,28 +306,32 @@ class ApplicationPartPut(ApplicationBase):
href=path,
items=prepared_items,
props=props)
for item in prepared_items:
# Try to grab the previously-existing item by href
existing_item = replaced_items.get(item.href, None) # type: ignore
if existing_item:
hook_notification_item = HookNotificationItem(
notification_item_type=HookNotificationItemTypes.UPSERT,
path=access.path,
content=existing_item.serialize(),
uid=None,
old_content=existing_item.serialize(),
new_content=item.serialize()
)
else: # We assume the item is new because it was not in the replaced_items
hook_notification_item = HookNotificationItem(
notification_item_type=HookNotificationItemTypes.UPSERT,
path=access.path,
content=item.serialize(),
uid=None,
old_content=None,
new_content=item.serialize()
)
self._hook.notify(hook_notification_item)
# Only run the hook if the hook is enabled
if self._hook.enabled:
for item in prepared_items:
# Try to grab the previously-existing item by href
existing_item = replaced_items.get(item.href, None) # type: ignore
if existing_item:
hook_notification_item = HookNotificationItem(
notification_item_type=HookNotificationItemTypes.UPSERT,
path=access.path,
content=existing_item.serialize(),
content_type=item.name,
uid=None,
old_content=existing_item.serialize(),
new_content=item.serialize(),
)
else: # We assume the item is new because it was not in the replaced_items
hook_notification_item = HookNotificationItem(
notification_item_type=HookNotificationItemTypes.UPSERT,
path=access.path,
content=item.serialize(),
content_type=item.name,
uid=None,
old_content=None,
new_content=item.serialize()
)
self._hook.notify(hook_notification_item)
except ValueError as e:
logger.warning(
"Bad PUT request on %r (create_collection): %s", path, e, exc_info=True)
@@ -345,15 +349,17 @@ class ApplicationPartPut(ApplicationBase):
try:
uploaded_item, replaced_item = parent_item.upload(href, prepared_item)
etag = uploaded_item.etag
hook_notification_item = HookNotificationItem(
notification_item_type=HookNotificationItemTypes.UPSERT,
path=access.path,
content=prepared_item.serialize(),
uid=None,
old_content=replaced_item.serialize() if replaced_item else None,
new_content=prepared_item.serialize()
)
self._hook.notify(hook_notification_item)
if self._hook.enabled:
hook_notification_item = HookNotificationItem(
notification_item_type=HookNotificationItemTypes.UPSERT,
path=access.path,
content=prepared_item.serialize(),
content_type=prepared_item.name,
uid=None,
old_content=replaced_item.serialize() if replaced_item else None,
new_content=prepared_item.serialize(),
)
self._hook.notify(hook_notification_item)
except ValueError as e:
# return better matching HTTP result in case errno is provided and catched
errno_match = re.search("\\[Errno ([0-9]+)\\]", str(e))

View File

@@ -33,6 +33,11 @@ class BaseHook:
"""
self.configuration = configuration
@property
def enabled(self) -> bool:
"""Check if this hook is enabled."""
raise NotImplementedError
def notify(self, notification_item):
"""Upload a new or replace an existing item."""
raise NotImplementedError
@@ -55,10 +60,12 @@ def _cleanup(path):
class HookNotificationItem:
def __init__(self, notification_item_type, path, content=None, uid=None, new_content=None, old_content=None):
def __init__(self, notification_item_type, path, content=None, content_type=None, uid=None, new_content=None,
old_content=None):
self.type = notification_item_type.value
self.point = _cleanup(path)
self._content_legacy = content
self.content_type = content_type
self.uid = uid
self.new_content = new_content
self.old_content = old_content
@@ -67,6 +74,30 @@ class HookNotificationItem:
def content(self): # For backward compatibility
return self._content_legacy or self.uid or self.new_content or self.old_content
@property
def is_calendar_item(self) -> bool:
"""Check if this notification item is related to a VCALENDAR item."""
if not self.content_type:
return False
return self.content_type.upper().strip() == "VCALENDAR"
@property
def is_card_item(self) -> bool:
"""Check if this notification item is related to a VCARD item."""
if not self.content_type:
return False
return self.content_type.upper().strip() == "VCARD"
@property
def is_addressbook_item(self) -> bool:
"""Check if this notification item is related to a VADDRESSBOOK item."""
if not self.content_type:
return False
return self.content_type.upper().strip() == "VADDRESSBOOK"
@property
def replaces_existing_item(self) -> bool:
"""Check if this notification item replaces/deletes an existing item."""

View File

@@ -930,12 +930,21 @@ class Hook(BaseHook):
self.email_config
)
@property
def enabled(self) -> bool:
"""Check if this hook is enabled (has a notify method)."""
return self.email_config.host is not None
def notify(self, notification_item) -> None:
"""
Entrypoint for processing a single notification item.
Overrides default notify method from BaseHook.
Triggered by Radicale when a notifiable event occurs (e.g. item added, updated or deleted)
"""
# Skip any non-VCALENDAR items
if not notification_item.is_calendar_item:
return
if isinstance(notification_item, HookNotificationItem):
self._process_event_and_notify(notification_item)

View File

@@ -2,5 +2,10 @@ from radicale import hook
class Hook(hook.BaseHook):
@property
def enabled(self) -> bool:
"""Check if this hook is enabled."""
return False
def notify(self, notification_item):
"""Notify nothing. Empty hook."""

View File

@@ -53,6 +53,11 @@ class Hook(hook.BaseHook):
logger.debug("Hook 'rabbitmq': _make_declare_queue_synced")
self._channel.queue_declare(queue=self._topic, durable=True, arguments={"x-queue-type": self._queue_type})
@property
def enabled(self) -> bool:
"""Check if this hook is enabled."""
return self._endpoint is not None
def notify(self, notification_item):
if isinstance(notification_item, HookNotificationItem):
self._notify(notification_item, True)