From 6045ad97feafc633122b170aed33211b8db26083 Mon Sep 17 00:00:00 2001 From: Unrud Date: Thu, 25 Aug 2016 05:47:31 +0200 Subject: [PATCH 1/6] Move upload_all from BaseCollection to Collection This is not used anywhere outside of Collection and probably never will be as WebDAV doesn't support bulk uploads. --- radicale/storage.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/radicale/storage.py b/radicale/storage.py index 102bfc1..8b444f3 100644 --- a/radicale/storage.py +++ b/radicale/storage.py @@ -321,19 +321,6 @@ class BaseCollection: """Upload a new item.""" raise NotImplementedError - def upload_all(self, vobject_items): - """Upload a new set of items. - - This takes a mapping of href and vobject items and - returns a list of uploaded items. - Might bring optimizations on some storages. - - """ - return [ - self.upload(href, vobject_item) - for href, vobject_item in vobject_items.items() - ] - def update(self, href, vobject_item): """Update an item. @@ -596,6 +583,19 @@ class Collection(BaseCollection): return cls(sane_path, principal=principal) + def upload_all(self, vobject_items): + """Upload a new set of items. + + This takes a mapping of href and vobject items and + returns a list of uploaded items. + Might bring optimizations on some storages. + + """ + return [ + self.upload(href, vobject_item) + for href, vobject_item in vobject_items.items() + ] + @classmethod def move(cls, item, to_collection, to_href): os.replace( From c57307c58599cac0679383dad0a56078f8272be8 Mon Sep 17 00:00:00 2001 From: Unrud Date: Thu, 25 Aug 2016 05:52:26 +0200 Subject: [PATCH 2/6] Rename collections to vobject_items Like the parameter name of upload_all --- radicale/storage.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/radicale/storage.py b/radicale/storage.py index 8b444f3..b1ad1a7 100644 --- a/radicale/storage.py +++ b/radicale/storage.py @@ -552,7 +552,7 @@ class Collection(BaseCollection): items.extend( getattr(collection, "%s_list" % content, [])) items_by_uid = groupby(sorted(items, key=get_uid), get_uid) - collections = {} + vobject_items = {} for uid, items in items_by_uid: new_collection = vobject.iCalendar() for item in items: @@ -561,14 +561,14 @@ class Collection(BaseCollection): # Prevent infinite loop for _ in range(10000): href = self._find_available_file_name() - if href not in collections: + if href not in vobject_items: break else: raise FileExistsError( errno.EEXIST, "No usable file name found") - collections[href] = new_collection + vobject_items[href] = new_collection - self.upload_all(collections) + self.upload_all(vobject_items) elif props.get("tag") == "VCARD": for card in collection: self.upload(self._find_available_file_name(), card) From bc0f8b0a4728fef24d1f92bc0e9d1df8010affab Mon Sep 17 00:00:00 2001 From: Unrud Date: Thu, 25 Aug 2016 05:55:21 +0200 Subject: [PATCH 3/6] Remove duplicate code --- radicale/storage.py | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/radicale/storage.py b/radicale/storage.py index b1ad1a7..6a0523c 100644 --- a/radicale/storage.py +++ b/radicale/storage.py @@ -411,11 +411,12 @@ class Collection(BaseCollection): raise self._sync_directory(directory) - def _find_available_file_name(self): + @staticmethod + def _find_available_file_name(exists_fn): # Prevent infinite loop for _ in range(10000): file_name = hex(getrandbits(32))[2:] - if not self.has(file_name): + if not exists_fn(file_name): return file_name raise FileExistsError(errno.EEXIST, "No usable file name found") @@ -557,21 +558,12 @@ class Collection(BaseCollection): new_collection = vobject.iCalendar() for item in items: new_collection.add(item) - - # Prevent infinite loop - for _ in range(10000): - href = self._find_available_file_name() - if href not in vobject_items: - break - else: - raise FileExistsError( - errno.EEXIST, "No usable file name found") + href = self._find_available_file_name(vobject_items.get) vobject_items[href] = new_collection - self.upload_all(vobject_items) elif props.get("tag") == "VCARD": for card in collection: - self.upload(self._find_available_file_name(), card) + self.upload(self._find_available_file_name(self.has), card) # This operation is not atomic on the filesystem level but it's # very unlikely that one rename operations succeeds while the From e31ea57883b51d0d3433ef364e6e16dc6528f994 Mon Sep 17 00:00:00 2001 From: Unrud Date: Thu, 25 Aug 2016 05:58:09 +0200 Subject: [PATCH 4/6] Use upload_all for addressbook --- radicale/storage.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/radicale/storage.py b/radicale/storage.py index 6a0523c..ef4fcfc 100644 --- a/radicale/storage.py +++ b/radicale/storage.py @@ -562,8 +562,11 @@ class Collection(BaseCollection): vobject_items[href] = new_collection self.upload_all(vobject_items) elif props.get("tag") == "VCARD": + vobject_items = {} for card in collection: - self.upload(self._find_available_file_name(self.has), card) + href = self._find_available_file_name(vobject_items.get) + vobject_items[href] = card + self.upload_all(vobject_items) # This operation is not atomic on the filesystem level but it's # very unlikely that one rename operations succeeds while the From 30d287ce00a4312731084183cbf10319ba903c3a Mon Sep 17 00:00:00 2001 From: Unrud Date: Thu, 25 Aug 2016 06:00:15 +0200 Subject: [PATCH 5/6] Write files nonatomic in upload_all It's only used in temporary collections. --- radicale/storage.py | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/radicale/storage.py b/radicale/storage.py index ef4fcfc..fc81507 100644 --- a/radicale/storage.py +++ b/radicale/storage.py @@ -582,14 +582,25 @@ class Collection(BaseCollection): """Upload a new set of items. This takes a mapping of href and vobject items and - returns a list of uploaded items. - Might bring optimizations on some storages. + uploads them nonatomic and without existence checks. """ - return [ - self.upload(href, vobject_item) - for href, vobject_item in vobject_items.items() - ] + fs = [] + for href, item in vobject_items.items(): + path = path_to_filesystem(self._filesystem_path, href) + fs.append(open(path, "w", encoding=self.encoding, newline="")) + fs[-1].write(item.serialize()) + fsync_fn = lambda fd: None + if self.configuration.getboolean("storage", "fsync"): + if os.name == "posix" and hasattr(fcntl, "F_FULLFSYNC"): + fsync_fn = lambda fd: fcntl.fcntl(fd, fcntl.F_FULLFSYNC) + else: + fsync_fn = os.fsync + # sync everything at once because it's slightly faster. + for f in fs: + fsync_fn(f.fileno()) + f.close() + self._sync_directory(self._filesystem_path) @classmethod def move(cls, item, to_collection, to_href): From ea63f461a8833a4a213de5993ca40af060e62872 Mon Sep 17 00:00:00 2001 From: Unrud Date: Thu, 25 Aug 2016 06:02:16 +0200 Subject: [PATCH 6/6] Rename upload_all to upload_all_nonatomic --- radicale/storage.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/radicale/storage.py b/radicale/storage.py index fc81507..1c76e28 100644 --- a/radicale/storage.py +++ b/radicale/storage.py @@ -560,13 +560,13 @@ class Collection(BaseCollection): new_collection.add(item) href = self._find_available_file_name(vobject_items.get) vobject_items[href] = new_collection - self.upload_all(vobject_items) + self.upload_all_nonatomic(vobject_items) elif props.get("tag") == "VCARD": vobject_items = {} for card in collection: href = self._find_available_file_name(vobject_items.get) vobject_items[href] = card - self.upload_all(vobject_items) + self.upload_all_nonatomic(vobject_items) # This operation is not atomic on the filesystem level but it's # very unlikely that one rename operations succeeds while the @@ -578,7 +578,7 @@ class Collection(BaseCollection): return cls(sane_path, principal=principal) - def upload_all(self, vobject_items): + def upload_all_nonatomic(self, vobject_items): """Upload a new set of items. This takes a mapping of href and vobject items and