Atomic creation of collections

radicale/__init__.py

@@ -480,11 +480,11 @@ class Application:
         collection = write_collections[0]
 
         props = xmlutils.props_from_request(content)
+        props["tag"] = "VCALENDAR"
         # TODO: use this?
         # timezone = props.get("C:calendar-timezone")
         collection = self.Collection.create_collection(
-            environ["PATH_INFO"], tag="VCALENDAR")
-        collection.set_meta(props)
+            environ["PATH_INFO"], props=props)
         return client.CREATED, {}, None
 
     def do_MKCOL(self, environ, read_collections, write_collections, content,
@@ -496,8 +496,8 @@ class Application:
         collection = write_collections[0]
 
         props = xmlutils.props_from_request(content)
-        collection = self.Collection.create_collection(environ["PATH_INFO"])
-        collection.set_meta(props)
+        collection = self.Collection.create_collection(
+            environ["PATH_INFO"], props=props)
         return client.CREATED, {}, None
 
     def do_MOVE(self, environ, read_collections, write_collections, content,

radicale/storage.py

@@ -38,6 +38,7 @@ from hashlib import md5
 from importlib import import_module
 from itertools import groupby
 from random import getrandbits
+from tempfile import TemporaryDirectory
 
 from atomicwrites import AtomicWriter
 import vobject
@@ -163,6 +164,23 @@ def path_to_filesystem(root, *paths):
     return safe_path
 
 
+def sync_directory(path):
+    """Sync directory to disk
+
+    This only works on POSIX and does nothing on other systems.
+
+    """
+    if os.name == "posix":
+        fd = os.open(path, 0)
+        try:
+            if hasattr(fcntl, "F_FULLFSYNC"):
+                fcntl.fcntl(fd, fcntl.F_FULLFSYNC)
+            else:
+                os.fsync(fd)
+        finally:
+            os.close(fd)
+
+
 class _EncodedAtomicWriter(AtomicWriter):
     def __init__(self, path, encoding, mode="w", overwrite=True):
         self._encoding = encoding
@@ -225,13 +243,15 @@ class BaseCollection:
         return get_etag(self.serialize())
 
     @classmethod
-    def create_collection(cls, href, collection=None, tag=None):
+    def create_collection(cls, href, collection=None, props=None):
         """Create a collection.
 
         ``collection`` is a list of vobject components.
 
-        ``tag`` is the type of collection (VCALENDAR or VADDRESSBOOK). If
-        ``tag`` is not given, it is guessed from the collection.
+        ``props`` are metadata values for the collection.
+
+        ``props["tag"]`` is the type of collection (VCALENDAR or VADDRESSBOOK). If
+        the key ``tag`` is missing, it is guessed from the collection.
 
         """
         raise NotImplementedError
@@ -326,7 +346,8 @@ class BaseCollection:
 class Collection(BaseCollection):
     """Collection stored in several files per calendar."""
 
-    def __init__(self, path, principal=False):
+    def __init__(self, path, principal=False, folder=None):
+        if not folder:
             folder = self._get_collection_root_folder()
         # path should already be sanitized
         self.path = sanitize_path(path).strip("/")
@@ -414,25 +435,42 @@ class Collection(BaseCollection):
                     yield cls(posixpath.join(path, sub_path))
 
     @classmethod
-    def create_collection(cls, href, collection=None, tag=None):
+    def create_collection(cls, href, collection=None, props=None):
         folder = cls._get_collection_root_folder()
-        path = path_to_filesystem(folder, href)
 
-        self = cls(href)
-        if os.path.exists(path):
-            return self
-        else:
-            os.makedirs(path)
-        if not tag and collection:
-            tag = collection[0].name
+        # path should already be sanitized
+        sane_path = sanitize_path(href).strip("/")
+        attributes = sane_path.split("/")
+        if not attributes[0]:
+            attributes.pop()
+        principal = len(attributes) == 1
+        filesystem_path = path_to_filesystem(folder, sane_path)
 
-        if tag == "VCALENDAR":
-            self.set_meta({"tag": "VCALENDAR"})
+        if not props:
+            props = {}
+        if not props.get("tag") and collection:
+            props["tag"] = collection[0].name
+        if not props:
+            os.makedirs(filesystem_path, exist_ok=True)
+            return cls(sane_path, principal=principal)
+
+        parent_dir = os.path.dirname(filesystem_path)
+        os.makedirs(parent_dir, exist_ok=True)
+        with TemporaryDirectory(prefix=".Radicale.tmp-",
+                                dir=parent_dir) as tmp_dir:
+            # The temporary directory itself can't be renamed
+            tmp_filesystem_path = os.path.join(tmp_dir, "collection")
+            os.makedirs(tmp_filesystem_path)
+            # path is unsafe
+            self = cls("/", principal=principal, folder=tmp_filesystem_path)
+            self.set_meta(props)
+            if props.get("tag") == "VCALENDAR":
                 if collection:
                     collection, = collection
                     items = []
                     for content in ("vevent", "vtodo", "vjournal"):
-                    items.extend(getattr(collection, "%s_list" % content, []))
+                        items.extend(getattr(collection, "%s_list" % content,
+                                             []))
 
                     def get_uid(item):
                         return hasattr(item, "uid") and item.uid.value
@@ -446,14 +484,13 @@ class Collection(BaseCollection):
                             new_collection.add(item)
                         self.upload(
                             self._find_available_file_name(), new_collection)
-
-        elif tag == "VCARD":
-            self.set_meta({"tag": "VADDRESSBOOK"})
+            elif props.get("tag") == "VCARD":
                 if collection:
                     for card in collection:
                         self.upload(self._find_available_file_name(), card)
-
-        return self
+            os.rename(tmp_filesystem_path, filesystem_path)
+            sync_directory(parent_dir)
+        return cls(sane_path, principal=principal)
 
     def list(self):
         try: