Some improvements to join_collection

Which induces a behavior change in singlefilestorage, as now
join_collection wouldn't write a wrapper if no items are given

tests/__init__.py

@@ -11,25 +11,14 @@
 
 import vdirsyncer.log
 from vdirsyncer.utils import text_type
+from vdirsyncer.utils.vobject import normalize_item as _normalize_item
 vdirsyncer.log.set_level(vdirsyncer.log.logging.DEBUG)
 
 
 def normalize_item(item):
-    # - X-RADICALE-NAME is used by radicale, because hrefs don't really exist
-    #   in their filesystem backend
-    # - PRODID is changed by radicale for some reason after upload, but nobody
-    #   cares about that anyway
-    rv = []
     if not isinstance(item, text_type):
         item = item.raw
-
-    for line in item.splitlines():
-        line = line.strip()
-        line = line.strip().split(u':', 1)
-        if line[0] in ('X-RADICALE-NAME', 'PRODID', 'REV'):
-            continue
-        rv.append(u':'.join(line))
-    return tuple(sorted(rv))
+    return tuple(sorted(_normalize_item(item).splitlines()))
 
 
 def assert_item_equals(a, b):

tests/storage/test_singlefile.py

@@ -9,7 +9,7 @@
 
 import pytest
 
-from .. import assert_item_equals
+from .. import assert_item_equals, EVENT_TEMPLATE
 from . import StorageTests
 from vdirsyncer.storage.singlefile import SingleFileStorage
 
@@ -17,13 +17,14 @@ from vdirsyncer.storage.singlefile import SingleFileStorage
 class TestSingleFileStorage(StorageTests):
 
     storage_class = SingleFileStorage
+    item_template = EVENT_TEMPLATE
 
     @pytest.fixture(autouse=True)
     def setup(self, tmpdir):
         self._path = str(tmpdir.join('test.txt'))
 
     def get_storage_args(self, **kwargs):
-        return dict(path=self._path, wrapper=u'MYWRAPPER')
+        return dict(path=self._path)
 
     def test_discover(self):
         '''This test doesn't make any sense here.'''

tests/utils/test_vobject.py

@@ -36,7 +36,10 @@ def test_split_collection_simple():
 
 
 def test_join_collection_simple():
-    given = join_collection(_simple_split, wrapper=u'VADDRESSBOOK')
+    item_type = _simple_split[0].splitlines()[0][len(u'BEGIN:'):]
+    given = join_collection(_simple_split, wrappers={
+        item_type: (u'VADDRESSBOOK', ())
+    })
     print(given)
     print(_simple_joined)
     assert normalize_item(given) == normalize_item(_simple_joined)

vdirsyncer/storage/singlefile.py

@@ -58,7 +58,7 @@ class SingleFileStorage(Storage):
 
     _items = None
 
-    def __init__(self, path, wrapper=None, encoding='utf-8', create=True,
+    def __init__(self, path, encoding='utf-8', create=True,
                  collection=None, **kwargs):
         super(SingleFileStorage, self).__init__(**kwargs)
         path = expand_path(path)
@@ -76,7 +76,6 @@ class SingleFileStorage(Storage):
         self.path = path
         self.encoding = encoding
         self.create = create
-        self.wrapper = wrapper
 
     def list(self):
         self._items = collections.OrderedDict()
@@ -88,6 +87,9 @@ class SingleFileStorage(Storage):
             import errno
             if e.errno != errno.ENOENT or not self.create:  # file not found
                 raise
+            text = None
+
+        if not text:
             return ()
 
         rv = []
@@ -149,7 +151,6 @@ class SingleFileStorage(Storage):
     def _write(self):
         text = join_collection(
             (item.raw for item, etag in itervalues(self._items)),
-            wrapper=self.wrapper
         )
         try:
             with safe_write(self.path, self._write_mode) as f:

vdirsyncer/utils/vobject.py

@@ -13,18 +13,36 @@ import icalendar.parser
 
 from . import text_type, itervalues
 
+IGNORE_PROPS = frozenset((
+    # PRODID is changed by radicale for some reason after upload
+    'PRODID',
+    # VERSION can get lost in singlefile storage
+    'VERSION',
+    # X-RADICALE-NAME is used by radicale, because hrefs don't really exist in
+    # their filesystem backend
+    'X-RADICALE-NAME',
+    # REV is from the VCARD specification and is supposed to change when the
+    # item does -- however, we can determine that ourselves
+    'REV'
+))
 
-def hash_item(text):
+
+def normalize_item(text, ignore_props=IGNORE_PROPS):
+    
     try:
         lines = to_unicode_lines(icalendar.cal.Component.from_ical(text))
     except Exception:
         lines = sorted(text.splitlines())
 
-    hashable = u'\r\n'.join(line.strip() for line in lines
-                            if line.strip() and
-                            not line.startswith(u'PRODID:') and
-                            not line.startswith(u'VERSION:'))
-    return hashlib.sha256(hashable.encode('utf-8')).hexdigest()
+    return u'\r\n'.join(line.strip()
+                        for line in lines
+                        if line.strip() and
+                        not any(line.startswith(p + ':')
+                                for p in IGNORE_PROPS))
+
+
+def hash_item(text):
+    return hashlib.sha256(normalize_item(text).encode('utf-8')).hexdigest()
 
 
 def split_collection(text, inline=(u'VTIMEZONE',),
@@ -67,34 +85,49 @@ def to_unicode_lines(item):
             yield icalendar.parser.foldline(content_line)
 
 
-def join_collection(items, wrapper=None):
-    timezones = {}
+def join_collection(items, wrappers={
+    u'VCALENDAR': (u'VCALENDAR', (u'VTIMEZONE',)),
+    u'VCARD': (u'VADDRESSBOOK', ())
+}):
+    '''
+    :param wrappers: {
+        item_type: wrapper_type, items_to_inline
+    }
+    '''
+    inline = {}
     components = []
+    wrapper_type = None
+    inline_types = None
+    item_type = None
+
+    def handle_item(item):
+        if item.name in inline_types:
+            inline[item.name] = item
+        else:
+            components.append(item)
 
     for item in items:
         component = icalendar.cal.Component.from_ical(item)
-        if component.name == u'VCALENDAR':
-            assert wrapper is None or wrapper == u'VCALENDAR'
-            wrapper = u'VCALENDAR'
-            for subcomponent in component.subcomponents:
-                if subcomponent.name == u'VTIMEZONE':
-                    timezones[subcomponent['TZID']] = subcomponent
-                else:
-                    components.append(subcomponent)
-        else:
-            if component.name == u'VCARD':
-                assert wrapper is None or wrapper == u'VADDRESSBOOK'
-                wrapper = u'VADDRESSBOOK'
-            components.append(component)
+
+        if item_type is None:
+            item_type = component.name
+            wrapper_type, inline_types = wrappers[item_type]
+
+        if component.name == item_type:
+            if item_type == wrapper_type:
+                for subcomponent in component.subcomponents:
+                    handle_item(subcomponent)
+            else:
+                handle_item(component)
 
     start = end = u''
-    if wrapper is not None:
-        start = u'BEGIN:{}'.format(wrapper)
-        end = u'END:{}'.format(wrapper)
+    if wrapper_type is not None:
+        start = u'BEGIN:{}'.format(wrapper_type)
+        end = u'END:{}'.format(wrapper_type)
 
     lines = [start]
-    for timezone in itervalues(timezones):
-        lines.extend(to_unicode_lines(timezone))
+    for inlined_item in itervalues(inline):
+        lines.extend(to_unicode_lines(inlined_item))
     for component in components:
         lines.extend(to_unicode_lines(component))
     lines.append(end)