Process data before and after the storage is locked

radicale/__init__.py

@@ -24,6 +24,7 @@ Can be used with an external WSGI server or the built-in server.
 """
 
 import base64
+import contextlib
 import datetime
 import io
 import itertools
@@ -34,6 +35,7 @@ import posixpath
 import pprint
 import random
 import socket
+import sys
 import threading
 import time
 import zlib
@@ -545,6 +547,16 @@ class Application:
         except socket.timeout as e:
             logger.debug("client timed out", exc_info=True)
             return REQUEST_TIMEOUT
+        # Prepare before locking
+        props = xmlutils.props_from_request(xml_content)
+        props["tag"] = "VCALENDAR"
+        # TODO: use this?
+        # timezone = props.get("C:calendar-timezone")
+        try:
+            storage.check_and_sanitize_props(props)
+        except ValueError as e:
+            logger.warning(
+                "Bad MKCALENDAR request on %r: %s", path, e, exc_info=True)
         with self.Collection.acquire_lock("w", user):
             item = next(self.Collection.discover(path), None)
             if item:
@@ -558,12 +570,7 @@ class Application:
             if (not isinstance(parent_item, storage.BaseCollection) or
                     parent_item.get_meta("tag")):
                 return FORBIDDEN
-            props = xmlutils.props_from_request(xml_content)
-            props["tag"] = "VCALENDAR"
-            # TODO: use this?
-            # timezone = props.get("C:calendar-timezone")
             try:
-                storage.check_and_sanitize_props(props)
                 self.Collection.create_collection(path, props=props)
             except ValueError as e:
                 logger.warning(
@@ -585,6 +592,17 @@ class Application:
         except socket.timeout as e:
             logger.debug("client timed out", exc_info=True)
             return REQUEST_TIMEOUT
+        # Prepare before locking
+        props = xmlutils.props_from_request(xml_content)
+        try:
+            storage.check_and_sanitize_props(props)
+        except ValueError as e:
+            logger.warning(
+                "Bad MKCOL request on %r: %s", path, e, exc_info=True)
+            return BAD_REQUEST
+        if (props.get("tag") and "w" not in permissions or
+                not props.get("tag") and "W" not in permissions):
+            return NOT_ALLOWED
         with self.Collection.acquire_lock("w", user):
             item = next(self.Collection.discover(path), None)
             if item:
@@ -597,12 +615,7 @@ class Application:
             if (not isinstance(parent_item, storage.BaseCollection) or
                     parent_item.get_meta("tag")):
                 return FORBIDDEN
-            props = xmlutils.props_from_request(xml_content)
-            if (props.get("tag") and "w" not in permissions or
-                    not props.get("tag") and "W" not in permissions):
-                return NOT_ALLOWED
             try:
-                storage.check_and_sanitize_props(props)
                 self.Collection.create_collection(path, props=props)
             except ValueError as e:
                 logger.warning(
@@ -755,9 +768,110 @@ class Application:
         except socket.timeout as e:
             logger.debug("client timed out", exc_info=True)
             return REQUEST_TIMEOUT
+        # Prepare before locking
+        parent_path = storage.sanitize_path(
+            "/%s/" % posixpath.dirname(path.strip("/")))
+        permissions = self.Rights.authorized(user, path, "Ww")
+        parent_permissions = self.Rights.authorized(user, parent_path, "w")
+
+        def prepare(vobject_items, tag=None, write_whole_collection=None):
+            if (write_whole_collection or
+                    permissions and not parent_permissions):
+                write_whole_collection = True
+                content_type = environ.get("CONTENT_TYPE",
+                                           "").split(";")[0]
+                tags = {value: key
+                        for key, value in xmlutils.MIMETYPES.items()}
+                tag = storage.predict_tag_of_whole_collection(
+                    vobject_items, tags.get(content_type))
+                if not tag:
+                    raise ValueError("Can't determine collection tag")
+                collection_path = storage.sanitize_path(path).strip("/")
+            elif (write_whole_collection is not None and
+                    not write_whole_collection or
+                    not permissions and parent_permissions):
+                write_whole_collection = False
+                if tag is None:
+                    tag = storage.predict_tag_of_parent_collection(
+                        vobject_items)
+                collection_path = posixpath.dirname(
+                    storage.sanitize_path(path).strip("/"))
+            props = None
+            stored_exc_info = None
+            items = []
+            try:
+                if tag:
+                    storage.check_and_sanitize_items(
+                        vobject_items, is_collection=write_whole_collection,
+                        tag=tag)
+                    if write_whole_collection and tag == "VCALENDAR":
+                        vobject_components = []
+                        vobject_item, = vobject_items
+                        for content in ("vevent", "vtodo", "vjournal"):
+                            vobject_components.extend(
+                                getattr(vobject_item, "%s_list" % content, []))
+                        vobject_components_by_uid = itertools.groupby(
+                            sorted(vobject_components, key=storage.get_uid),
+                            storage.get_uid)
+                        for uid, components in vobject_components_by_uid:
+                            vobject_collection = vobject.iCalendar()
+                            for component in components:
+                                vobject_collection.add(component)
+                            item = storage.Item(
+                                collection_path=collection_path,
+                                item=vobject_collection)
+                            item.prepare()
+                            items.append(item)
+                    elif write_whole_collection and tag == "VADDRESSBOOK":
+                        for vobject_item in vobject_items:
+                            item = storage.Item(
+                                collection_path=collection_path,
+                                item=vobject_item)
+                            item.prepare()
+                            items.append(item)
+                    elif not write_whole_collection:
+                        vobject_item, = vobject_items
+                        item = storage.Item(collection_path=collection_path,
+                                            item=vobject_item)
+                        item.prepare()
+                        items.append(item)
+
+                if write_whole_collection:
+                    props = {}
+                    if tag:
+                        props["tag"] = tag
+                    if tag == "VCALENDAR" and vobject_items:
+                        if hasattr(vobject_items[0], "x_wr_calname"):
+                            calname = vobject_items[0].x_wr_calname.value
+                            if calname:
+                                props["D:displayname"] = calname
+                        if hasattr(vobject_items[0], "x_wr_caldesc"):
+                            caldesc = vobject_items[0].x_wr_caldesc.value
+                            if caldesc:
+                                props["C:calendar-description"] = caldesc
+                    storage.check_and_sanitize_props(props)
+            except Exception:
+                stored_exc_info = sys.exc_info()
+
+            # Use generator for items and delete references to free memory
+            # early
+            def items_generator():
+                while items:
+                    yield items.pop(0)
+
+            return (items_generator(), tag, write_whole_collection, props,
+                    stored_exc_info)
+
+        try:
+            vobject_items = tuple(vobject.readComponents(content or ""))
+        except Exception as e:
+            logger.warning(
+                "Bad PUT request on %r: %s", path, e, exc_info=True)
+            return BAD_REQUEST
+        (prepared_items, prepared_tag, prepared_write_whole_collection,
+         prepared_props, prepared_exc_info) = prepare(vobject_items)
+
         with self.Collection.acquire_lock("w", user):
-            parent_path = storage.sanitize_path(
-                "/%s/" % posixpath.dirname(path.strip("/")))
             item = next(self.Collection.discover(path), None)
             parent_item = next(self.Collection.discover(parent_path), None)
             if not parent_item:
@@ -766,8 +880,14 @@ class Application:
             write_whole_collection = (
                 isinstance(item, storage.BaseCollection) or
                 not parent_item.get_meta("tag"))
+
             if write_whole_collection:
-                if not self.Rights.authorized(user, path, "w"):
+                tag = prepared_tag
+            else:
+                tag = parent_item.get_meta("tag")
+
+            if write_whole_collection:
+                if not self.Rights.authorized(user, path, "w" if tag else "W"):
                     return NOT_ALLOWED
             elif not self.Rights.authorized(user, parent_path, "w"):
                 return NOT_ALLOWED
@@ -785,69 +905,43 @@ class Application:
                 # Creation asked but item found: item can't be replaced
                 return PRECONDITION_FAILED
 
-            try:
+            if (tag != prepared_tag or
-                items = tuple(vobject.readComponents(content or ""))
+                    prepared_write_whole_collection != write_whole_collection):
-                if write_whole_collection:
+                (prepared_items, prepared_tag, prepared_write_whole_collection,
-                    content_type = environ.get("CONTENT_TYPE",
+                 prepared_props, prepared_exc_info) = prepare(
-                                               "").split(";")[0]
+                    vobject_items, tag, write_whole_collection)
-                    tags = {value: key
+            props = prepared_props
-                            for key, value in xmlutils.MIMETYPES.items()}
+            if prepared_exc_info:
-                    tag = tags.get(content_type)
-                    if items and items[0].name == "VCALENDAR":
-                        tag = "VCALENDAR"
-                    elif items and items[0].name in ("VCARD", "VLIST"):
-                        tag = "VADDRESSBOOK"
-                    elif not tag and not items:
-                        # Maybe an empty address book
-                        tag = "VADDRESSBOOK"
-                    elif not tag:
-                        raise ValueError("Can't determine collection tag")
-                else:
-                    tag = parent_item.get_meta("tag")
-                storage.check_and_sanitize_items(
-                    items, is_collection=write_whole_collection, tag=tag)
-            except Exception as e:
                 logger.warning(
-                    "Bad PUT request on %r: %s", path, e, exc_info=True)
+                    "Bad PUT request on %r: %s", path, prepared_exc_info[1],
+                    exc_info=prepared_exc_info)
                 return BAD_REQUEST
 
             if write_whole_collection:
-                props = {}
-                if tag:
-                    props["tag"] = tag
-                if tag == "VCALENDAR" and items:
-                    if hasattr(items[0], "x_wr_calname"):
-                        calname = items[0].x_wr_calname.value
-                        if calname:
-                            props["D:displayname"] = calname
-                    if hasattr(items[0], "x_wr_caldesc"):
-                        caldesc = items[0].x_wr_caldesc.value
-                        if caldesc:
-                            props["C:calendar-description"] = caldesc
                 try:
-                    storage.check_and_sanitize_props(props)
+                    etag = self.Collection.create_collection(
-                    new_item = self.Collection.create_collection(
+                        path, prepared_items, props).etag
-                        path, items, props)
                 except ValueError as e:
                     logger.warning(
                         "Bad PUT request on %r: %s", path, e, exc_info=True)
                     return BAD_REQUEST
             else:
-                uid = storage.get_uid_from_object(items[0])
+                prepared_item, = prepared_items
-                if (item and item.uid != uid or
+                if (item and item.uid != prepared_item.uid or
-                        not item and parent_item.has_uid(uid)):
+                        not item and parent_item.has_uid(prepared_item.uid)):
                     return self._webdav_error_response(
                         "C" if tag == "VCALENDAR" else "CR",
                         "no-uid-conflict")
 
                 href = posixpath.basename(path.strip("/"))
                 try:
-                    new_item = parent_item.upload(href, items[0])
+                    etag = parent_item.upload(href, prepared_item).etag
                 except ValueError as e:
                     logger.warning(
                         "Bad PUT request on %r: %s", path, e, exc_info=True)
                     return BAD_REQUEST
-            headers = {"ETag": new_item.etag}
+
+            headers = {"ETag": etag}
             return client.CREATED, headers, None
 
     def do_REPORT(self, environ, base_prefix, path, user):
@@ -863,7 +957,8 @@ class Application:
         except socket.timeout as e:
             logger.debug("client timed out", exc_info=True)
             return REQUEST_TIMEOUT
-        with self.Collection.acquire_lock("r", user):
+        with contextlib.ExitStack() as lock_stack:
+            lock_stack.enter_context(self.Collection.acquire_lock("r", user))
             item = next(self.Collection.discover(path), None)
             if not item:
                 return NOT_FOUND
@@ -876,7 +971,8 @@ class Application:
             headers = {"Content-Type": "text/xml; charset=%s" % self.encoding}
             try:
                 status, xml_answer = xmlutils.report(
-                    base_prefix, path, xml_content, collection)
+                    base_prefix, path, xml_content, collection,
+                    lock_stack.close)
             except ValueError as e:
                 logger.warning(
                     "Bad REPORT request on %r: %s", path, e, exc_info=True)

radicale/storage.py

@@ -39,7 +39,7 @@ import time
 from contextlib import contextmanager
 from hashlib import md5
 from importlib import import_module
-from itertools import chain, groupby
+from itertools import chain
 from random import getrandbits
 from tempfile import NamedTemporaryFile, TemporaryDirectory
 
@@ -115,6 +115,27 @@ def load(configuration):
     return CollectionCopy
 
 
+def predict_tag_of_parent_collection(vobject_items):
+    if len(vobject_items) != 1:
+        return ""
+    if vobject_items[0].name == "VCALENDAR":
+        return "VCALENDAR"
+    if vobject_items[0].name in ("VCARD", "VLIST"):
+        return "VADDRESSBOOK"
+    return ""
+
+
+def predict_tag_of_whole_collection(vobject_items, fallback_tag=None):
+    if vobject_items and vobject_items[0].name == "VCALENDAR":
+        return "VCALENDAR"
+    if vobject_items and vobject_items[0].name in ("VCARD", "VLIST"):
+        return "VADDRESSBOOK"
+    if not fallback_tag and not vobject_items:
+        # Maybe an empty address book
+        return "VADDRESSBOOK"
+    return fallback_tag
+
+
 def check_and_sanitize_items(vobject_items, is_collection=False, tag=None):
     """Check vobject items for common errors and add missing UIDs.
 
@@ -360,12 +381,15 @@ class ComponentNotFoundError(ValueError):
 
 
 class Item:
-    def __init__(self, collection, item=None, href=None, last_modified=None,
+    def __init__(self, collection_path=None, collection=None, item=None,
-                 text=None, etag=None, uid=None, name=None,
+                 href=None, last_modified=None, text=None, etag=None, uid=None,
-                 component_name=None):
+                 name=None, component_name=None, time_range=None):
         """Initialize an item.
 
-        ``collection`` the parent collection.
+        ``collection_path`` the path of the parent collection (optional if
+        ``collection`` is set).
+
+        ``collection`` the parent collection (optional).
 
         ``href`` the href of the item.
 
@@ -380,9 +404,23 @@ class Item:
 
         ``uid`` the UID of the object (optional). See ``get_uid_from_object``.
 
+        ``name`` the name of the item (optional). See ``vobject_item.name``.
+
+        ``component_name`` the name of the primary component (optional).
+        See ``find_tag``.
+
+        ``time_range`` the enclosing time range.
+        See ``find_tag_and_time_range``.
+
         """
         if text is None and item is None:
             raise ValueError("at least one of 'text' or 'item' must be set")
+        if collection_path is None:
+            if collection is None:
+                raise ValueError("at least one of 'collection_path' or "
+                                 "'collection' must be set")
+            collection_path = collection.path
+        self._collection_path = collection_path
         self.collection = collection
         self.href = href
         self.last_modified = last_modified
@@ -392,6 +430,7 @@ class Item:
         self._uid = uid
         self._name = name
         self._component_name = component_name
+        self._time_range = time_range
 
     def __getattr__(self, attr):
         return getattr(self.item, attr)
@@ -402,7 +441,8 @@ class Item:
                 self._text = self.item.serialize()
             except Exception as e:
                 raise RuntimeError("Failed to serialize item %r from %r: %s" %
-                                   (self.href, self.collection.path, e)) from e
+                                   (self.href, self._collection_path,
+                                    e)) from e
         return self._text
 
     @property
@@ -412,7 +452,8 @@ class Item:
                 self._item = vobject.readOne(self._text)
             except Exception as e:
                 raise RuntimeError("Failed to parse item %r from %r: %s" %
-                                   (self.href, self.collection.path, e)) from e
+                                   (self.href, self._collection_path,
+                                    e)) from e
         return self._item
 
     @property
@@ -430,9 +471,9 @@ class Item:
 
     @property
     def name(self):
-        if self._name is not None:
+        if self._name is None:
-            return self._name
+            self._name = self.item.name or ""
-        return self.item.name
+        return self._name
 
     @property
     def component_name(self):
@@ -440,6 +481,24 @@ class Item:
             return self._component_name
         return xmlutils.find_tag(self.item)
 
+    @property
+    def time_range(self):
+        if self._time_range is None:
+            self._component_name, *self._time_range = (
+                xmlutils.find_tag_and_time_range(self.item))
+        return self._time_range
+
+    def prepare(self):
+        """Fill cache with values."""
+        orig_item = self._item
+        self.serialize()
+        self.etag
+        self.uid
+        self.name
+        self.time_range
+        self.component_name
+        self._item = orig_item
+
 
 class BaseCollection:
 
@@ -497,7 +556,7 @@ class BaseCollection:
         """
         if item.collection.path == to_collection.path and item.href == to_href:
             return
-        to_collection.upload(to_href, item.item)
+        to_collection.upload(to_href, item)
         item.collection.delete(item.href)
 
     @property
@@ -510,7 +569,7 @@ class BaseCollection:
         return '"%s"' % etag.hexdigest()
 
     @classmethod
-    def create_collection(cls, href, collection=None, props=None):
+    def create_collection(cls, href, items=None, props=None):
         """Create a collection.
 
         ``href`` is the sanitized path.
@@ -608,7 +667,7 @@ class BaseCollection:
                 return True
         return False
 
-    def upload(self, href, vobject_item):
+    def upload(self, href, item):
         """Upload a new or replace an existing item."""
         raise NotImplementedError
 
@@ -931,7 +990,7 @@ class Collection(BaseCollection):
         return item_errors == 0 and collection_errors == 0
 
     @classmethod
-    def create_collection(cls, href, collection=None, props=None):
+    def create_collection(cls, href, items=None, props=None):
         folder = cls._get_collection_root_folder()
 
         # Path should already be sanitized
@@ -953,25 +1012,11 @@ class Collection(BaseCollection):
             os.makedirs(tmp_filesystem_path)
             self = cls(sane_path, filesystem_path=tmp_filesystem_path)
             self.set_meta(props)
-
+            if items is not None:
-            if collection:
                 if props.get("tag") == "VCALENDAR":
-                    collection, = collection
+                    self._upload_all_nonatomic(items, suffix=".ics")
-                    items = []
-                    for content in ("vevent", "vtodo", "vjournal"):
-                        items.extend(
-                            getattr(collection, "%s_list" % content, []))
-                    items_by_uid = groupby(sorted(items, key=get_uid), get_uid)
-
-                    def vobject_items():
-                        for uid, items in items_by_uid:
-                            collection = vobject.iCalendar()
-                            for item in items:
-                                collection.add(item)
-                            yield collection
-                    self._upload_all_nonatomic(vobject_items(), suffix=".ics")
                 elif props.get("tag") == "VADDRESSBOOK":
-                    self._upload_all_nonatomic(collection, suffix=".vcf")
+                    self._upload_all_nonatomic(items, suffix=".vcf")
 
             # This operation is not atomic on the filesystem level but it's
             # very unlikely that one rename operations succeeds while the
@@ -983,7 +1028,7 @@ class Collection(BaseCollection):
 
         return cls(sane_path)
 
-    def _upload_all_nonatomic(self, vobject_items, suffix=""):
+    def _upload_all_nonatomic(self, items, suffix=""):
         """Upload a new set of items.
 
         This takes a list of vobject items and
@@ -994,11 +1039,10 @@ class Collection(BaseCollection):
                                     ".Radicale.cache", "item")
         self._makedirs_synced(cache_folder)
         hrefs = set()
-        for vobject_item in vobject_items:
+        for item in items:
-            uid = get_uid_from_object(vobject_item)
+            uid = item.uid
             try:
-                cache_content = self._item_cache_content(vobject_item)
+                cache_content = self._item_cache_content(item)
-                _, _, _, text, _, _, _, _ = cache_content
             except Exception as e:
                 raise ValueError(
                     "Failed to store item %r in temporary collection %r: %s" %
@@ -1036,7 +1080,7 @@ class Collection(BaseCollection):
             with self._atomic_write(os.path.join(self._filesystem_path, "ign"),
                                     newline="", sync_directory=False,
                                     replace_fn=replace_fn) as f:
-                f.write(text)
+                f.write(item.serialize())
             hrefs.add(href)
             with self._atomic_write(os.path.join(cache_folder, href), "wb",
                                     sync_directory=False) as f:
@@ -1267,30 +1311,23 @@ class Collection(BaseCollection):
                 continue
             yield href
 
-    def get(self, href, verify_href=True):
-        item, metadata = self._get_with_metadata(href, verify_href=verify_href)
-        return item
-
     def _item_cache_hash(self, raw_text):
         _hash = md5()
         _hash.update(ITEM_CACHE_TAG)
         _hash.update(raw_text)
         return _hash.hexdigest()
 
-    def _item_cache_content(self, vobject_item, cache_hash=None):
+    def _item_cache_content(self, item, cache_hash=None):
-        text = vobject_item.serialize()
+        text = item.serialize()
         if cache_hash is None:
             cache_hash = self._item_cache_hash(text.encode(self._encoding))
-        etag = get_etag(text)
+        return (cache_hash, item.uid, item.etag, text, item.name,
-        uid = get_uid_from_object(vobject_item)
+                item.component_name, *item.time_range)
-        name = vobject_item.name
-        tag, start, end = xmlutils.find_tag_and_time_range(vobject_item)
-        return cache_hash, uid, etag, text, name, tag, start, end
 
-    def _store_item_cache(self, href, vobject_item, cache_hash=None):
+    def _store_item_cache(self, href, item, cache_hash=None):
         cache_folder = os.path.join(self._filesystem_path, ".Radicale.cache",
                                     "item")
-        content = self._item_cache_content(vobject_item, cache_hash)
+        content = self._item_cache_content(item, cache_hash)
         self._makedirs_synced(cache_folder)
         try:
             # Race: Other processes might have created and locked the
@@ -1335,10 +1372,7 @@ class Collection(BaseCollection):
             e.name for e in os.scandir(cache_folder) if not
             os.path.isfile(os.path.join(self._filesystem_path, e.name))))
 
-    def _get_with_metadata(self, href, verify_href=True):
+    def get(self, href, verify_href=True):
-        """Like ``get`` but additonally returns the following metadata:
-        tag, start, end: see ``xmlutils.find_tag_and_time_range``. If
-        extraction of the metadata failed, the values are all ``None``."""
         if verify_href:
             try:
                 if not is_safe_filesystem_path_component(href):
@@ -1348,26 +1382,25 @@ class Collection(BaseCollection):
                 logger.debug(
                     "Can't translate name %r safely to filesystem in %r: %s",
                     href, self.path, e, exc_info=True)
-                return None, None
+                return None
         else:
             path = os.path.join(self._filesystem_path, href)
         try:
             with open(path, "rb") as f:
                 raw_text = f.read()
         except (FileNotFoundError, IsADirectoryError):
-            return None, None
+            return None
         except PermissionError:
             # Windows raises ``PermissionError`` when ``path`` is a directory
             if (os.name == "nt" and
                     os.path.isdir(path) and os.access(path, os.R_OK)):
-                return None, None
+                return None
             raise
         # The hash of the component in the file system. This is used to check,
         # if the entry in the cache is still valid.
         input_hash = self._item_cache_hash(raw_text)
         cache_hash, uid, etag, text, name, tag, start, end = \
             self._load_item_cache(href, input_hash)
-        vobject_item = None
         if input_hash != cache_hash:
             with self._acquire_cache_lock("item"):
                 # Lock the item cache to prevent multpile processes from
@@ -1383,10 +1416,11 @@ class Collection(BaseCollection):
                             raw_text.decode(self._encoding)))
                         check_and_sanitize_items(vobject_items,
                                                  tag=self.get_meta("tag"))
-                        vobject_item = vobject_items[0]
+                        vobject_item, = vobject_items
+                        temp_item = Item(collection=self, item=vobject_item)
                         cache_hash, uid, etag, text, name, tag, start, end = \
                             self._store_item_cache(
-                                href, vobject_item, input_hash)
+                                href, temp_item, input_hash)
                     except Exception as e:
                         raise RuntimeError("Failed to load item %r in %r: %s" %
                                            (href, self.path, e)) from e
@@ -1398,10 +1432,12 @@ class Collection(BaseCollection):
         last_modified = time.strftime(
             "%a, %d %b %Y %H:%M:%S GMT",
             time.gmtime(os.path.getmtime(path)))
+        # Don't keep reference to ``vobject_item``, because it requires a lot
+        # of memory.
         return Item(
-            self, href=href, last_modified=last_modified, etag=etag,
+            collection=self, href=href, last_modified=last_modified, etag=etag,
-            text=text, item=vobject_item, uid=uid, name=name,
+            text=text, uid=uid, name=name, component_name=tag,
-            component_name=tag), (tag, start, end)
+            time_range=(start, end))
 
     def get_multi(self, hrefs):
         # It's faster to check for file name collissions here, because
@@ -1433,33 +1469,29 @@ class Collection(BaseCollection):
             # no filter
             yield from ((item, simple) for item in self.get_all())
             return
-        for item, (itag, istart, iend) in (
+        for item in (self.get(h, verify_href=False) for h in self.list()):
-                self._get_with_metadata(href, verify_href=False)
+            istart, iend = item.time_range
-                for href in self.list()):
+            if tag == item.component_name and istart < end and iend > start:
-            if tag == itag and istart < end and iend > start:
                 yield item, simple and (start <= istart or iend <= end)
 
-    def upload(self, href, vobject_item):
+    def upload(self, href, item):
         if not is_safe_filesystem_path_component(href):
             raise UnsafePathError(href)
         try:
-            cache_hash, uid, etag, text, name, tag, _, _ = \
+            self._store_item_cache(href, item)
-                self._store_item_cache(href, vobject_item)
         except Exception as e:
             raise ValueError("Failed to store item %r in collection %r: %s" %
                              (href, self.path, e)) from e
         path = path_to_filesystem(self._filesystem_path, href)
         with self._atomic_write(path, newline="") as fd:
-            fd.write(text)
+            fd.write(item.serialize())
         # Clean the cache after the actual item is stored, or the cache entry
         # will be removed again.
         self._clean_item_cache()
-        item = Item(self, href=href, etag=etag, text=text, item=vobject_item,
-                    uid=uid, name=name, component_name=tag)
         # Track the change
         self._update_history_etag(href, item)
         self._clean_history_cache()
-        return item
+        return self.get(href, verify_href=False)
 
     def delete(self, href=None):
         if href is None:

radicale/xmlutils.py

@@ -653,8 +653,8 @@ def find_tag(vobject_item):
     if vobject_item.name == "VCALENDAR":
         for component in vobject_item.components():
             if component.name != "VTIMEZONE":
-                return component.name
+                return component.name or ""
-    return None
+    return ""
 
 
 def find_tag_and_time_range(vobject_item):
@@ -668,7 +668,7 @@ def find_tag_and_time_range(vobject_item):
     """
     tag = find_tag(vobject_item)
     if not tag:
-        return (None, TIMESTAMP_MIN, TIMESTAMP_MAX)
+        return (tag, TIMESTAMP_MIN, TIMESTAMP_MAX)
     start = end = None
 
     def range_fn(range_start, range_end, is_recurrence):
@@ -1116,7 +1116,7 @@ def proppatch(base_prefix, path, xml_request, collection):
     return multistatus
 
 
-def report(base_prefix, path, xml_request, collection):
+def report(base_prefix, path, xml_request, collection, unlock_storage_fn):
     """Read and answer REPORT requests.
 
     Read rfc3253-3.6 for info.
@@ -1230,8 +1230,15 @@ def report(base_prefix, path, xml_request, collection):
         if collection_requested:
             yield from collection.get_all_filtered(filters)
 
+    # Retrieve everything required for finishing the request.
+    retrieved_items = list(retrieve_items(collection, hreferences,
+                                          multistatus))
+    collection_tag = collection.get_meta("tag")
+    # Don't access storage after this!
+    unlock_storage_fn()
+
     def match(item, filter_):
-        tag = collection.get_meta("tag")
+        tag = collection_tag
         if (tag == "VCALENDAR" and filter_.tag != _tag("C", filter_)):
             if len(filter_) == 0:
                 return True
@@ -1253,8 +1260,11 @@ def report(base_prefix, path, xml_request, collection):
             return all(_prop_match(item.item, f, "CR") for f in filter_)
         raise ValueError("unsupported filter %r for %r" % (filter_.tag, tag))
 
-    for item, filters_matched in retrieve_items(collection, hreferences,
+    while retrieved_items:
-                                                multistatus):
+        # ``item.item`` might be accessed during filtering.
+        # Don't keep reference to ``item``, because VObject requires a lot of
+        # memory.
+        item, filters_matched = retrieved_items.pop(0)
         if filters and not filters_matched:
             try:
                 if not all(match(item, filter_) for filter_ in filters):