diff --git a/.env.template b/.env.template index 2423abd..0d34aae 100644 --- a/.env.template +++ b/.env.template @@ -36,6 +36,9 @@ BIRTHDAY_EVENT_CATEGORY=Birthday # Whether to update existing events when templates change BIRTHDAY_UPDATE_EXISTING=true +# Delete birthday events whose contact or birthday is gone (only after a complete fetch) +BIRTHDAY_DELETE_ORPHANS=false + # ============================================================================ # OPTIONAL: Scheduling Configuration # ============================================================================ diff --git a/README.md b/README.md index 1dc5cf6..0d11e15 100644 --- a/README.md +++ b/README.md @@ -73,6 +73,7 @@ docker-compose up -d | `BIRTHDAY_REMINDER_MESSAGE` | `Reminder: {name}'s birthday is in {days} days!` | Reminder message template | | `BIRTHDAY_EVENT_CATEGORY` | `Birthday` | Event category | | `BIRTHDAY_UPDATE_EXISTING` | `true` | Update existing events | +| `BIRTHDAY_DELETE_ORPHANS` | `false` | Delete orphaned events | ### Logging & Debug diff --git a/bdaysync/caldav_client.py b/bdaysync/caldav_client.py index c7cde89..3d60e26 100644 --- a/bdaysync/caldav_client.py +++ b/bdaysync/caldav_client.py @@ -3,14 +3,22 @@ """ import logging +import re from datetime import datetime, timedelta -from typing import Dict, Optional +from typing import Dict, List, Optional import vobject import caldav from config import get_birthday_config logger = logging.getLogger(__name__) +BIRTHDAY_UID_RE = re.compile(r'^birthday-(.+)-\d{4}(\d{4})$') + + +def birthday_slug(name: str) -> str: + """Name part of a birthday event UID: birthday-{slug}-{YYYYMMDD}""" + return name.replace(' ', '-').lower() + class CalDAVClient: """Client for creating events in CalDAV server""" @@ -55,6 +63,7 @@ def _load_config(self): self.reminder_template = config['reminder_template'] self.event_category = config['event_category'] self.update_existing = config['update_existing'] + self.delete_orphans_enabled = config['delete_orphans'] logger.info("Birthday event configuration:") logger.info(f" Title template: {self.event_title_template}") @@ -63,6 +72,7 @@ def _load_config(self): logger.info(f" Reminder message: {self.reminder_template}") logger.info(f" Category: {self.event_category}") logger.info(f" Update existing: {self.update_existing}") + logger.info(f" Delete orphans: {self.delete_orphans_enabled}") def create_birthday_event(self, contact: Dict, year: int = None) -> bool: """Create a birthday event for a contact""" @@ -90,7 +100,7 @@ def create_birthday_event(self, contact: Dict, year: int = None) -> bool: return False # Create unique UID - event_uid = f"birthday-{name.replace(' ', '-').lower()}-{event_date.strftime('%Y%m%d')}" + event_uid = f"birthday-{birthday_slug(name)}-{event_date.strftime('%Y%m%d')}" # Create iCalendar event cal = vobject.iCalendar() @@ -164,6 +174,30 @@ def _format_reminder_message(self, name: str, days_before: int) -> str: else: return f"{name}'s birthday is in {days_before} days!" + def delete_orphans(self, contacts: List[Dict]) -> int: + """Delete birthday-* events whose name and month/day match no current contact.""" + wanted = {(birthday_slug(c['name']), c['birthday'].strftime('%m%d')) for c in contacts} + + deleted = 0 + for ev in self.calendar.events(): + try: + parsed = vobject.readOne(ev.data) + if not hasattr(parsed, 'vevent') or not hasattr(parsed.vevent, 'uid'): + continue + uid = parsed.vevent.uid.value + match = BIRTHDAY_UID_RE.match(uid) + if not match: + continue + if match.groups() in wanted: + continue + logger.info(f"Deleting orphan birthday event: {uid}") + ev.delete() + deleted += 1 + except Exception as e: + logger.warning(f"Error while considering event for orphan delete: {e}") + continue + return deleted + def _find_existing_event(self, name: str, date) -> Optional: """Find existing birthday event for a contact""" try: @@ -197,7 +231,7 @@ def _find_existing_event(self, name: str, date) -> Optional: # Also check by UID pattern if hasattr(cal.vevent, 'uid'): uid = cal.vevent.uid.value - expected_uid = f"birthday-{name.replace(' ', '-').lower()}" + expected_uid = f"birthday-{birthday_slug(name)}" if uid.startswith(expected_uid): return event except Exception as e: @@ -222,7 +256,7 @@ def _find_existing_event(self, name: str, date) -> Optional: return event if hasattr(cal.vevent, 'uid'): uid = cal.vevent.uid.value - expected_uid = f"birthday-{name.replace(' ', '-').lower()}" + expected_uid = f"birthday-{birthday_slug(name)}" if uid.startswith(expected_uid): return event except Exception as e: diff --git a/bdaysync/cardav_client.py b/bdaysync/cardav_client.py index 0903233..ff64f1c 100644 --- a/bdaysync/cardav_client.py +++ b/bdaysync/cardav_client.py @@ -2,10 +2,11 @@ CardDAV client for fetching contacts with birthdays """ -import re import logging +import time from datetime import datetime from typing import List, Dict, Optional +from xml.etree import ElementTree import vobject import requests from requests.auth import HTTPBasicAuth, HTTPDigestAuth @@ -15,7 +16,7 @@ class CardDAVClient: """Client for reading contacts from CardDAV server""" - + def __init__(self, server_url: str, username: str, password: str): self.server_url = server_url.rstrip('/') self.username = username @@ -28,6 +29,7 @@ def __init__(self, server_url: str, username: str, password: str): # Discover addressbooks self.addressbook_urls = [] + self.fetch_complete = False self._test_auth_and_discover() def _test_auth_and_discover(self): @@ -37,9 +39,19 @@ def _test_auth_and_discover(self): try: # Test Basic auth first - headers = {'Depth': '1'} + headers = { + 'Content-Type': 'application/xml; charset=utf-8', + 'Depth': '1', + } + propfind_body = ''' + + + + + ''' response = requests.request('PROPFIND', self.server_url, - auth=self.basic_auth, headers=headers, timeout=10) + auth=self.basic_auth, headers=headers, + data=propfind_body, timeout=10) logger.info(f"Basic auth response: {response.status_code}") if response.status_code in [200, 207]: @@ -49,7 +61,8 @@ def _test_auth_and_discover(self): # Try Digest auth logger.info("Basic auth failed, trying Digest authentication...") response = requests.request('PROPFIND', self.server_url, - auth=self.digest_auth, headers=headers, timeout=10) + auth=self.digest_auth, headers=headers, + data=propfind_body, timeout=10) logger.info(f"Digest auth response: {response.status_code}") if response.status_code in [200, 207]: @@ -65,12 +78,7 @@ def _test_auth_and_discover(self): self.addressbook_urls = self._extract_addressbooks(response.text) if not self.addressbook_urls: - # If no addressbooks found, maybe this URL IS an addressbook - if self._is_addressbook(response.text): - logger.info("Provided URL appears to be a single addressbook") - self.addressbook_urls = [self.server_url] - else: - raise Exception("No addressbooks found at the provided URL") + raise Exception("No addressbooks found at the provided URL") logger.info(f"Discovered {len(self.addressbook_urls)} addressbooks:") for ab_url in self.addressbook_urls: @@ -85,43 +93,51 @@ def _test_auth_and_discover(self): def _extract_addressbooks(self, xml_response: str) -> List[str]: """Extract addressbook collection URLs from PROPFIND response""" + return self._find_addressbooks(xml_response) + + def _find_addressbooks(self, xml_response: str) -> List[str]: + """Find CardDAV addressbook collections in a DAV multistatus response.""" + dav_namespace = 'DAV:' + carddav_namespace = 'urn:ietf:params:xml:ns:carddav' + + try: + root = ElementTree.fromstring(xml_response) + except ElementTree.ParseError as error: + logger.warning(f"Could not parse CardDAV discovery XML: {error}") + return [] + addressbooks = [] - - # Find all response blocks - response_pattern = r']*>(.*?)' - responses = re.findall(response_pattern, xml_response, re.DOTALL | re.IGNORECASE) - - for response_block in responses: - # Extract href from this response block - href_match = re.search(r']*>([^<]+)', response_block, re.IGNORECASE) - if not href_match: + for response in root.findall(f'{{{dav_namespace}}}response'): + href = response.findtext(f'{{{dav_namespace}}}href') + if not href: continue - - href = href_match.group(1).strip() + + has_addressbook_type = False + for propstat in response.findall(f'{{{dav_namespace}}}propstat'): + status = propstat.findtext(f'{{{dav_namespace}}}status', '') + if not status.startswith('HTTP/') or ' 2' not in status: + continue + + resource_type = propstat.find(f'{{{dav_namespace}}}prop/{{{dav_namespace}}}resourcetype') + if resource_type is not None and resource_type.find(f'{{{carddav_namespace}}}addressbook') is not None: + has_addressbook_type = True + break + + href = href.strip() logger.debug(f"Found href: {href}") - - # Check if this response contains addressbook resourcetype - if ('card:addressbook' in response_block or - 'addressbook' in response_block.lower() and - ' bool: - """Check if the response indicates this URL is an addressbook collection""" - return ('card:addressbook' in xml_response or - ('addressbook' in xml_response.lower() and - ' List[Dict]: """Fetch all contacts from all discovered addressbooks""" all_contacts = [] + # Cleared by any listing, download or parse failure. Orphan delete relies on it. + self.fetch_complete = True for addressbook_url in self.addressbook_urls: logger.info(f"Processing addressbook: {addressbook_url}") @@ -129,9 +145,23 @@ def get_contacts(self) -> List[Dict]: all_contacts.extend(contacts) logger.info(f"Found {len(contacts)} contacts with birthdays in this addressbook") + logger.info(f"CardDAV fetch complete: {self.fetch_complete}") logger.info(f"Total contacts with birthdays across all addressbooks: {len(all_contacts)}") return all_contacts + def _http_get_retry(self, url: str, attempts: int = 3): + """GET with retries for transient connection errors.""" + last_error = None + for i in range(1, attempts + 1): + try: + return requests.get(url, auth=self.auth, timeout=10) + except requests.exceptions.RequestException as e: + last_error = e + logger.warning(f"GET failed ({i}/{attempts}) for {url}: {e}") + if i < attempts: + time.sleep(i) + raise last_error + def _get_contacts_from_addressbook(self, addressbook_url: str) -> List[Dict]: """Fetch contacts from a specific addressbook""" contacts = [] @@ -146,15 +176,14 @@ def _get_contacts_from_addressbook(self, addressbook_url: str) -> List[Dict]: propfind_body = ''' - - ''' logger.debug(f"Discovering resources in addressbook: {addressbook_url}") response = requests.request('PROPFIND', addressbook_url, - auth=self.auth, headers=headers, data=propfind_body) + auth=self.auth, headers=headers, data=propfind_body, + timeout=30) logger.debug(f"PROPFIND response status: {response.status_code}") @@ -175,7 +204,7 @@ def _get_contacts_from_addressbook(self, addressbook_url: str) -> List[Dict]: full_url = self._resolve_url(vcard_url) logger.debug(f"Fetching vCard {i+1}/{len(vcard_urls)} from: {full_url}") - vcard_response = requests.get(full_url, auth=self.auth, timeout=10) + vcard_response = self._http_get_retry(full_url) logger.debug(f"vCard response status: {vcard_response.status_code}") if vcard_response.status_code == 200: @@ -189,15 +218,19 @@ def _get_contacts_from_addressbook(self, addressbook_url: str) -> List[Dict]: logger.debug(f"No birthday found in vCard: {vcard_url}") else: logger.warning(f"Failed to fetch vCard {vcard_url}: {vcard_response.status_code}") + self.fetch_complete = False except Exception as e: logger.warning(f"Error processing vCard {vcard_url}: {e}") + self.fetch_complete = False continue else: logger.error(f"Failed to discover resources in {addressbook_url}: {response.status_code}") logger.error(f"Response: {response.text[:500]}") + self.fetch_complete = False except Exception as e: logger.error(f"Error fetching contacts from {addressbook_url}: {e}") + self.fetch_complete = False if logger.getEffectiveLevel() <= logging.DEBUG: import traceback logger.debug(traceback.format_exc()) @@ -206,38 +239,23 @@ def _get_contacts_from_addressbook(self, addressbook_url: str) -> List[Dict]: def _extract_vcard_urls(self, xml_response: str) -> List[str]: """Extract vCard URLs from PROPFIND response""" + dav_namespace = 'DAV:' + root = ElementTree.fromstring(xml_response) + urls = [] - - # Find all href elements containing .vcf files - vcf_pattern = r']*>([^<]*\.vcf)' - vcf_matches = re.findall(vcf_pattern, xml_response, re.IGNORECASE) - - for url in vcf_matches: - url = url.strip() - if url: - urls.append(url) - logger.debug(f"Found vCard URL: {url}") - - # Also try a more general pattern for any vcard content type - href_pattern = r']*>([^<]+)' - content_type_pattern = r']*>([^<]*vcard[^<]*)' - - href_matches = re.findall(href_pattern, xml_response, re.IGNORECASE) - content_matches = re.findall(content_type_pattern, xml_response, re.IGNORECASE) - - # If we found content type matches, try to match them with hrefs - if content_matches and not urls: - for href in href_matches: - href = href.strip() - if not href.endswith('/') and not href.endswith('.vcf'): - # Check if this href appears near a vcard content type - href_index = xml_response.find(f'{href}') - if href_index > 0: - # Look for vcard content type within 500 chars after href - nearby_text = xml_response[href_index:href_index + 500] - if 'vcard' in nearby_text.lower(): - urls.append(href) - logger.debug(f"Found vCard URL by content type: {href}") + for response in root.findall(f'{{{dav_namespace}}}response'): + href = (response.findtext(f'{{{dav_namespace}}}href') or '').strip() + if not href or href.endswith('/'): + continue + content_type = (response.findtext( + f'{{{dav_namespace}}}propstat/{{{dav_namespace}}}prop/' + f'{{{dav_namespace}}}getcontenttype' + ) or '') + # SOGo/sabre set getcontenttype to a vcard MIME type. iCloud omits + # that property and uses *.vcf hrefs instead. + if 'vcard' in content_type.lower() or href.lower().endswith('.vcf'): + urls.append(href) + logger.debug(f"Found vCard URL: {href}") logger.info(f"Extracted {len(urls)} vCard URLs") return urls @@ -256,13 +274,12 @@ def _resolve_url(self, url: str) -> str: return f"{self.server_url.rstrip('/')}/{url.lstrip('/')}" def _parse_vcard(self, vcard_text: str) -> Optional[Dict]: - """Parse individual vCard""" + """Parse individual vCard. Returns None without BDAY, raises on unreadable data.""" try: # Clean up the vCard text vcard_text = vcard_text.strip() if not vcard_text.startswith('BEGIN:VCARD'): - logger.debug("Invalid vCard: doesn't start with BEGIN:VCARD") - return None + raise ValueError("Invalid vCard: doesn't start with BEGIN:VCARD") vcard = vobject.readOne(vcard_text) contact = {} @@ -307,12 +324,10 @@ def _parse_vcard(self, vcard_text: str) -> Optional[Dict]: month_day = bday_clean[2:] # Remove -- contact['birthday'] = datetime.strptime(f"2000-{month_day}", '%Y-%m-%d').date() else: - logger.warning(f"Unknown birthday format for {contact['name']}: {bday}") - return None + raise ValueError("unknown format") except ValueError as e: - logger.warning(f"Could not parse birthday for {contact['name']}: {bday} - {e}") - return None + raise ValueError(f"Could not parse birthday for {contact['name']}: {bday} - {e}") from e elif hasattr(bday, 'date'): contact['birthday'] = bday.date() @@ -334,7 +349,6 @@ def _parse_vcard(self, vcard_text: str) -> Optional[Dict]: logger.debug(f"No birthday found for contact: {contact['name']}") return None - except Exception as e: - logger.warning(f"Error parsing vCard: {e}") + except Exception: logger.debug(f"vCard content: {vcard_text[:500]}...") - return None + raise diff --git a/bdaysync/config.py b/bdaysync/config.py index 6a2385f..fc344ce 100644 --- a/bdaysync/config.py +++ b/bdaysync/config.py @@ -81,7 +81,8 @@ def get_birthday_config(): 'reminder_days_str': os.getenv('BIRTHDAY_REMINDER_DAYS', '1'), 'reminder_template': os.getenv('BIRTHDAY_REMINDER_MESSAGE', 'Reminder: {name}\'s birthday is in {days} days!'), 'event_category': os.getenv('BIRTHDAY_EVENT_CATEGORY', 'Birthday'), - 'update_existing': os.getenv('BIRTHDAY_UPDATE_EXISTING', 'true').lower() == 'true' + 'update_existing': os.getenv('BIRTHDAY_UPDATE_EXISTING', 'true').lower() == 'true', + 'delete_orphans': os.getenv('BIRTHDAY_DELETE_ORPHANS', 'false').lower() == 'true' } def get_scheduler_config(): diff --git a/bdaysync/main.py b/bdaysync/main.py index af476c3..e7c4239 100644 --- a/bdaysync/main.py +++ b/bdaysync/main.py @@ -116,6 +116,14 @@ def main_sync(): created_count += 1 logger.info(f"Successfully created {created_count} birthday events") + + if caldav_client.delete_orphans_enabled: + if cardav_client.fetch_complete: + deleted = caldav_client.delete_orphans(contacts) + logger.info(f"Deleted {deleted} orphan birthday events") + else: + logger.warning("Incomplete CardDAV fetch; skipping orphan delete") + return True except Exception as e: diff --git a/bdaysync/scheduler.py b/bdaysync/scheduler.py index 856a437..9050ea1 100644 --- a/bdaysync/scheduler.py +++ b/bdaysync/scheduler.py @@ -27,6 +27,7 @@ def __init__(self, sync_func, diagnostic_func): self.startup_delay = config['startup_delay'] self.last_sync = None + self.last_schedule_check = datetime.now() # Setup signal handlers for graceful shutdown signal.signal(signal.SIGTERM, self._signal_handler) @@ -54,13 +55,12 @@ def _should_sync_interval(self): return True return datetime.now() - self.last_sync >= timedelta(hours=self.sync_interval_hours) - def _should_sync_cron(self, schedule): + def _should_sync_cron(self, schedule, last_check, now): """Check if we should sync based on cron schedule""" try: - cron = croniter(schedule, datetime.now() - timedelta(minutes=1)) - next_time = cron.get_next(datetime) - return next_time <= datetime.now() - except: + return croniter(schedule, last_check).get_next(datetime) <= now + except Exception as e: + logger.error(f"Invalid cron schedule '{schedule}': {e}") return False def _perform_sync(self, diagnostic=False): @@ -105,7 +105,8 @@ def _get_next_schedule_info(self): 'sync_schedule': self.sync_schedule, 'diagnostic_schedule': self.diagnostic_schedule } - except: + except Exception as e: + logger.error(f"Could not calculate the next scheduled run: {e}") return None def run_daemon(self): @@ -141,6 +142,7 @@ def run_daemon(self): while self.running: try: loop_count += 1 + now = datetime.now() # Check if it's time for a sync sync_needed = False @@ -149,9 +151,14 @@ def run_daemon(self): if self.sync_interval_hours > 0: sync_needed = self._should_sync_interval() else: - sync_needed = self._should_sync_cron(self.sync_schedule) + sync_needed = self._should_sync_cron( + self.sync_schedule, self.last_schedule_check, now + ) - diagnostic_needed = self._should_sync_cron(self.diagnostic_schedule) + diagnostic_needed = self._should_sync_cron( + self.diagnostic_schedule, self.last_schedule_check, now + ) + self.last_schedule_check = now if diagnostic_needed: self._perform_sync(diagnostic=True) @@ -173,9 +180,3 @@ def run_daemon(self): self._wait_with_interrupt_check(60) logger.info("Scheduler daemon stopped") - - def run_once(self): - """Run sync once and exit""" - logger.info("Running single sync operation...") - success = self._perform_sync() - return 0 if success else 1 diff --git a/bdaysync/test_sync.py b/bdaysync/test_sync.py new file mode 100644 index 0000000..09d42ae --- /dev/null +++ b/bdaysync/test_sync.py @@ -0,0 +1,157 @@ +""" +Safety tests for orphan deletion. Run from bdaysync/: python -m unittest test_sync +""" + +import logging +import unittest +from datetime import date +from unittest import mock + +import requests + +import caldav_client +import cardav_client +import config +import main + +logging.disable(logging.CRITICAL) + +LISTING = ''' + + {ab} + {ab}a.vcf +''' + +VCARD = "BEGIN:VCARD\r\nVERSION:3.0\r\nFN:Anna\r\n{bday}END:VCARD\r\n" + + +def response(status, text): + return mock.Mock(status_code=status, text=text) + + +def fetch(listings, vcard=VCARD.format(bday="BDAY:1990-01-02\r\n"), vcard_status=200): + """Run get_contacts against fake addressbooks; a listing is a response or an exception.""" + client = cardav_client.CardDAVClient.__new__(cardav_client.CardDAVClient) + client.server_url = 'https://dav.example' + client.auth = None + client.addressbook_urls = list(listings) + + def propfind(method, url, **kwargs): + listing = listings[url] + if isinstance(listing, Exception): + raise listing + return listing + + with mock.patch.object(cardav_client.requests, 'request', side_effect=propfind), \ + mock.patch.object(cardav_client.requests, 'get', return_value=response(vcard_status, vcard)): + contacts = client.get_contacts() + return contacts, client.fetch_complete + + +class CardDAVFetchComplete(unittest.TestCase): + OK = {'https://dav.example/ab1/': response(207, LISTING.format(ab='/ab1/'))} + + def test_all_vcards_fetched_is_complete(self): + contacts, complete = fetch(self.OK) + self.assertEqual(len(contacts), 1) + self.assertTrue(complete) + + def test_contact_without_birthday_keeps_fetch_complete(self): + _, complete = fetch(self.OK, vcard=VCARD.format(bday="")) + self.assertTrue(complete) + + def test_failed_addressbook_listing_is_incomplete(self): + failures = { + 'exception': requests.exceptions.ConnectionError('down'), + 'http error': response(503, 'unavailable'), + 'broken xml': response(207, '