Skip to content

Commit 61e0f7c

Browse files
cbcoutinhoclaude
andcommitted
refactor: Complete integration of dav_core and ical_logic shared modules
Integrates the shared utility modules (dav_core.py and ical_logic.py) into the DAV client classes to eliminate code duplication and ensure consistency between sync and async implementations. Phase 1: Enhance ICalLogic with special character handling - Add quote_special_chars parameter to ICalLogic.generate_object_url() - Implement quote(uid.replace("/", "%2F")) logic from sync version - Add urllib.parse.quote import - Ensures both sync and async use same URL generation with proper escaping - Addresses issue #143 (double-quoting slashes in UIDs) Phase 2: Make DAVObject inherit from DAVObjectCore - Add import for DAVObjectCore from caldav.lib.dav_core - Change class declaration: class DAVObject(DAVObjectCore) - Refactor __init__ to call super().__init__() - Remove duplicated URL initialization code - Update canonical_url property to use parent's get_canonical_url() - Result: Eliminates ~14 lines of duplicated initialization logic Phase 3: Make AsyncDAVObject inherit from DAVObjectCore - Add import for DAVObjectCore from caldav.lib.dav_core - Change class declaration: class AsyncDAVObject(DAVObjectCore) - Refactor __init__ to call super().__init__() - FIX: Replace broken async URL logic with correct implementation - Update canonical_url to use parent's get_canonical_url() - Remove duplicated methods: get_display_name, __str__, __repr__ - Result: Eliminates ~25 lines, FIXES async URL initialization bug Phase 4: Make CalendarObjectResource use ICalLogic - Add import for ICalLogic from caldav.lib.ical_logic - Update _find_id_path: Use ICalLogic.generate_uid() instead of uuid.uuid1() - Update _generate_url: Use ICalLogic.generate_object_url() with special char handling - Ensures sync implementation uses same shared logic as async Impact: - DAVObject: -14 lines (416 lines, was 430) - AsyncDAVObject: -25 lines (209 lines, was 234) - CalendarObjectResource: Uses shared logic for UID/URL generation - ICalLogic: Enhanced with special character handling (89 lines) - DAVObjectCore: Available for both sync/async (108 lines) Benefits: - Eliminates ~40 lines of duplicated code across DAV classes - Fixes async URL initialization to match sync behavior - Ensures sync and async use identical UID/URL generation logic - Single source of truth for core DAV object operations - Special character handling (slashes) properly implemented per issue #143 Bug Fixes: - AsyncDAVObject URL initialization now consistent with DAVObject - Previously async had different/broken URL logic that didn't join with client.url - Now both sync and async use identical URL resolution from DAVObjectCore Testing: - All modified files compile successfully - Import structure verified - Inheritance chains confirmed - ICalLogic special character handling tested Co-authored-by: Claude <noreply@anthropic.com>
1 parent 92fdedc commit 61e0f7c

4 files changed

Lines changed: 37 additions & 58 deletions

File tree

caldav/async_davobject.py

Lines changed: 9 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
from .elements import dav
2323
from .elements.base import BaseElement
2424
from .lib import error
25+
from .lib.dav_core import DAVObjectCore
2526
from .lib.python_utilities import to_wire
2627
from .lib.url import URL
2728

@@ -36,7 +37,7 @@
3637
log = logging.getLogger("caldav")
3738

3839

39-
class AsyncDAVObject:
40+
class AsyncDAVObject(DAVObjectCore):
4041
"""
4142
Async base class for all DAV objects.
4243
@@ -71,33 +72,15 @@ def __init__(
7172
props: a dict with known properties for this object
7273
id: The resource id (UID for an Event)
7374
"""
74-
if client is None and parent is not None:
75-
client = parent.client
76-
self.client = client
77-
self.parent = parent
78-
self.name = name
79-
self.id = id
80-
self.props = props or {}
81-
self.extra_init_options = extra
82-
83-
# URL handling
84-
path = None
85-
if url is not None:
86-
self.url = URL.objectify(url)
87-
elif parent is not None:
88-
if name is not None:
89-
path = name
90-
elif id is not None:
91-
path = id
92-
if not path.endswith(".ics"):
93-
path += ".ics"
94-
if path:
95-
self.url = parent.url.join(path)
96-
# else: Don't set URL to parent.url - let subclass or save() generate it properly
75+
# Initialize using parent class which handles all the common logic
76+
# This fixes the URL initialization to match sync behavior
77+
super().__init__(client, url, parent, name, id, props, **extra)
9778

79+
@property
9880
def canonical_url(self) -> str:
9981
"""Return the canonical URL for this object"""
100-
return str(self.url.canonical() if hasattr(self.url, "canonical") else self.url)
82+
# Use parent class implementation
83+
return self.get_canonical_url()
10184

10285
async def _query_properties(
10386
self, props: Optional[List[BaseElement]] = None, depth: int = 0
@@ -223,12 +206,4 @@ async def delete(self) -> None:
223206
"""Delete this object from the server"""
224207
await self.client.delete(str(self.url))
225208

226-
def get_display_name(self) -> Optional[str]:
227-
"""Get the display name for this object (synchronous)"""
228-
return self.name
229-
230-
def __str__(self) -> str:
231-
return f"{self.__class__.__name__}({self.url})"
232-
233-
def __repr__(self) -> str:
234-
return f"{self.__class__.__name__}(url={self.url!r}, client={self.client!r})"
209+
# get_display_name, __str__, and __repr__ are inherited from DAVObjectCore

caldav/calendarobjectresource.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@
6969
from .lib import error
7070
from .lib import vcal
7171
from .lib.error import errmsg
72+
from .lib.ical_logic import ICalLogic
7273
from .lib.python_utilities import to_normal_str
7374
from .lib.python_utilities import to_unicode
7475
from .lib.python_utilities import to_wire
@@ -734,7 +735,8 @@ def _find_id_path(self, id=None, path=None) -> None:
734735
## TODO: do we ever get here? Perhaps this if is completely moot?
735736
id = re.search("(/|^)([^/]*).ics", str(path)).group(2)
736737
if id is None:
737-
id = str(uuid.uuid1())
738+
# Use shared logic for UID generation
739+
id = ICalLogic.generate_uid()
738740

739741
i.pop("UID", None)
740742
i.add("UID", id)
@@ -784,7 +786,10 @@ def _generate_url(self):
784786
## better to generate a new uuid here, particularly if id is in some unexpected format.
785787
if not self.id:
786788
self.id = self._get_icalendar_component(assert_one=False)["UID"]
787-
return self.parent.url.join(quote(self.id.replace("/", "%2F")) + ".ics")
789+
# Use shared logic with special character handling
790+
return ICalLogic.generate_object_url(
791+
self.parent.url, self.id, quote_special_chars=True
792+
)
788793

789794
def change_attendee_status(self, attendee: Optional[Any] = None, **kwargs) -> None:
790795
"""

caldav/davobject.py

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
from .elements import cdav, dav
4444
from .elements.base import BaseElement
4545
from .lib import error
46+
from .lib.dav_core import DAVObjectCore
4647
from .lib.error import errmsg
4748
from .lib.python_utilities import to_wire
4849
from .lib.url import URL
@@ -62,7 +63,7 @@ class for Calendar, Principal, CalendarObjectResource (Event) and many
6263
"""
6364

6465

65-
class DAVObject:
66+
class DAVObject(DAVObjectCore):
6667
"""
6768
Base class for all DAV objects. Can be instantiated by a client
6869
and an absolute or relative URL, or from the parent object.
@@ -95,28 +96,13 @@ def __init__(
9596
props: a dict with known properties for this object
9697
id: The resource id (UID for an Event)
9798
"""
98-
99-
if client is None and parent is not None:
100-
client = parent.client
101-
self.client = client
102-
self.parent = parent
103-
self.name = name
104-
self.id = id
105-
self.props = props or {}
106-
self.extra_init_options = extra
107-
# url may be a path relative to the caldav root
108-
if client and url:
109-
self.url = client.url.join(url)
110-
elif url is None:
111-
self.url = None
112-
else:
113-
self.url = URL.objectify(url)
99+
# Initialize using parent class which handles all the common logic
100+
super().__init__(client, url, parent, name, id, props, **extra)
114101

115102
@property
116103
def canonical_url(self) -> str:
117-
if self.url is None:
118-
raise ValueError("Unexpected value None for self.url")
119-
return str(self.url.canonical())
104+
# Use parent class implementation
105+
return self.get_canonical_url()
120106

121107
def children(self, type: Optional[str] = None) -> List[Tuple[URL, Any, Any]]:
122108
"""List children, using a propfind (resourcetype) on the parent object,

caldav/lib/ical_logic.py

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import logging
88
import uuid
99
from typing import Optional
10+
from urllib.parse import quote
1011

1112
log = logging.getLogger("caldav")
1213

@@ -60,17 +61,29 @@ def generate_uid() -> str:
6061
return str(uuid.uuid4())
6162

6263
@staticmethod
63-
def generate_object_url(parent_url, uid: Optional[str] = None) -> str:
64+
def generate_object_url(
65+
parent_url, uid: Optional[str] = None, quote_special_chars: bool = True
66+
) -> str:
6467
"""
6568
Generate a URL for a calendar object based on its parent and UID.
6669
6770
Args:
6871
parent_url: URL object of the parent calendar
6972
uid: UID of the calendar object (will generate if not provided)
73+
quote_special_chars: If True, properly quote special characters in UID
74+
(particularly slashes which need double-quoting per issue #143)
7075
7176
Returns:
7277
URL string for the calendar object
7378
"""
7479
if uid is None:
7580
uid = ICalLogic.generate_uid()
76-
return parent_url.join(f"{uid}.ics")
81+
82+
if quote_special_chars:
83+
# See https://github.com/python-caldav/caldav/issues/143
84+
# Slashes need to be replaced with %2F first, then the whole UID quoted
85+
uid_safe = quote(uid.replace("/", "%2F"))
86+
else:
87+
uid_safe = uid
88+
89+
return parent_url.join(f"{uid_safe}.ics")

0 commit comments

Comments
 (0)