From 4ddcb0fef434eb5cc942f656616fb89275166eea Mon Sep 17 00:00:00 2001
From: Markus Unterwaditzer <markus@unterwaditzer.net>
Date: Fri, 30 Jan 2015 15:11:37 +0100
Subject: [PATCH] Retry with random filename if UID is rejected.

see #173
---
 docs/vdir.rst                    | 16 ++++------------
 tests/storage/test_filesystem.py |  6 ++++++
 vdirsyncer/storage/filesystem.py | 28 +++++++++++++++++++---------
 3 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/docs/vdir.rst b/docs/vdir.rst
index 0855e87..30441e9 100644
--- a/docs/vdir.rst
+++ b/docs/vdir.rst
@@ -28,18 +28,10 @@ event.
 
 The filename *should* consist of the ``ident``, followed by the file extension.
 The ``ident`` is either the ``UID``, if the item has one, else a string with
-similar properties as the ``UID``:
-
-   Type name: UID
-
-   Type purpose: To specify a value that represents a globally unique
-   identifier corresponding to the individual or resource associated
-   with the vCard.
-
-   -- The vCard_ RFC
-
-Character limitations imposed by the filesystem *should* be circumvented by
-replacing the offending characters with underscores ``_``.
+similar properties as the ``UID``. However, several restrictions of the
+underlying filesystem might make an implementation of this naming scheme for
+items' filenames impossible. The approach to deal with such cases is left to
+the client, which are free to choose a different scheme for filenames instead.
 
 .. _vCard: https://tools.ietf.org/html/rfc6350
 .. _iCalendar: https://tools.ietf.org/html/rfc5545
diff --git a/tests/storage/test_filesystem.py b/tests/storage/test_filesystem.py
index 5328121..ab90819 100644
--- a/tests/storage/test_filesystem.py
+++ b/tests/storage/test_filesystem.py
@@ -49,3 +49,9 @@ class TestFilesystemStorage(StorageTests):
         s.upload(Item(u'UID:a/b/c'))
         item_file, = tmpdir.listdir()
         assert str(item_file).endswith('a_b_c.txt')
+
+    def test_too_long_uid(self, tmpdir):
+        s = self.storage_class(str(tmpdir), '.txt')
+        item = Item(u'UID:' + u'hue' * 600)
+        href, etag = s.upload(item)
+        assert item.uid not in href
diff --git a/vdirsyncer/storage/filesystem.py b/vdirsyncer/storage/filesystem.py
index 5a20953..8518e77 100644
--- a/vdirsyncer/storage/filesystem.py
+++ b/vdirsyncer/storage/filesystem.py
@@ -2,6 +2,7 @@
 
 import errno
 import os
+import uuid
 
 from atomicwrites import atomic_write
 
@@ -70,11 +71,14 @@ class FilesystemStorage(Storage):
     def _get_filepath(self, href):
         return os.path.join(self.path, href)
 
-    def _get_href(self, item):
+    def _deterministic_href(self, item):
         # XXX: POSIX only defines / and \0 as invalid chars, but we should make
         # this work crossplatform.
         return item.ident.replace('/', '_') + self.fileext
 
+    def _random_href(self):
+        return str(uuid.uuid4()) + self.fileext
+
     def list(self):
         for fname in os.listdir(self.path):
             fpath = os.path.join(self.path, fname)
@@ -88,25 +92,34 @@ class FilesystemStorage(Storage):
                 return (Item(f.read().decode(self.encoding)),
                         get_etag_from_file(fpath))
         except IOError as e:
-            import errno
             if e.errno == errno.ENOENT:
                 raise exceptions.NotFoundError(href)
             else:
                 raise
 
     def upload(self, item):
-        href = self._get_href(item)
-        fpath = self._get_filepath(href)
-
         if not isinstance(item.raw, text_type):
             raise TypeError('item.raw must be a unicode string.')
 
+        try:
+            href = self._deterministic_href(item)
+            return self._upload_impl(item, href)
+        except OSError as e:
+            if e.errno == errno.ENAMETOOLONG:
+                logger.debug('UID as filename rejected, trying with random '
+                             'one.')
+                href = self._random_href()
+                return self._upload_impl(item, href)
+            else:
+                raise
+
+    def _upload_impl(self, item, href):
+        fpath = self._get_filepath(href)
         try:
             with atomic_write(fpath, mode='wb', overwrite=False) as f:
                 f.write(item.raw.encode(self.encoding))
                 return href, get_etag_from_fileobject(f)
         except OSError as e:
-            import errno
             if e.errno == errno.EEXIST:
                 raise exceptions.AlreadyExistingError(item)
             else:
@@ -114,9 +127,6 @@ class FilesystemStorage(Storage):
 
     def update(self, href, item, etag):
         fpath = self._get_filepath(href)
-        if href != self._get_href(item) and item.uid:
-            logger.warning('href != uid + fileext: href={}; uid={}'
-                           .format(href, item.uid))
         if not os.path.exists(fpath):
             raise exceptions.NotFoundError(item.uid)
         actual_etag = get_etag_from_file(fpath)