Merge pull request #920 from pimutils/meta_delete

metasync: use None as no-value and delete missing values on syncing

tests/storage/__init__.py

@@ -312,20 +312,29 @@ class StorageTests:
         if self.storage_class.storage_name.endswith("dav"):
             assert urlquote(uid, "/@:") in href
 
+    @pytest.mark.asyncio
+    async def test_empty_metadata(self, requires_metadata, s):
+        if getattr(self, "dav_server", ""):
+            pytest.skip()
+
+        assert await s.get_meta("color") is None
+        assert await s.get_meta("displayname") is None
+
     @pytest.mark.asyncio
     async def test_metadata(self, requires_metadata, s):
-        if not getattr(self, "dav_server", ""):
+        if getattr(self, "dav_server", "") == "xandikos":
-            assert not await s.get_meta("color")
+            pytest.skip("xandikos does not support removing metadata.")
-            assert not await s.get_meta("displayname")
 
         try:
             await s.set_meta("color", None)
-            assert not await s.get_meta("color")
+            assert await s.get_meta("color") is None
             await s.set_meta("color", "#ff0000")
             assert await s.get_meta("color") == "#ff0000"
         except exceptions.UnsupportedMetadataError:
             pass
 
+    @pytest.mark.asyncio
+    async def test_encoding_metadata(self, requires_metadata, s):
         for x in ("hello world", "hello wörld"):
             await s.set_meta("displayname", x)
             rv = await s.get_meta("displayname")

tests/unit/test_metasync.py

@@ -28,6 +28,10 @@ async def test_basic(monkeypatch):
     b = MemoryStorage()
     status = {}
 
+    await a.set_meta("foo", None)
+    await metasync(a, b, status, keys=["foo"])
+    assert await a.get_meta("foo") is None and await b.get_meta("foo") is None
+
     await a.set_meta("foo", "bar")
     await metasync(a, b, status, keys=["foo"])
     assert await a.get_meta("foo") == await b.get_meta("foo") == "bar"
@@ -183,7 +187,7 @@ async def test_fuzzing(a, b, status, keys, conflict_resolution):
     await metasync(a, b, status, keys=keys, conflict_resolution=conflict_resolution)
 
     for key in keys:
-        s = status.get(key, "")
+        s = status.get(key)
         assert await a.get_meta(key) == await b.get_meta(key) == s
-        if expected_values.get(key, "") and s:
+        if expected_values.get(key) and s:
             assert s == expected_values[key]

vdirsyncer/metasync.py

@@ -14,20 +14,27 @@ class MetaSyncConflict(MetaSyncError):
     key = None
 
 
+def status_set_key(status, key, value):
+    if value is None:
+        status.pop(key, None)
+    else:
+        status[key] = value
+
+
 async def metasync(storage_a, storage_b, status, keys, conflict_resolution=None):
     async def _a_to_b():
         logger.info(f"Copying {key} to {storage_b}")
         await storage_b.set_meta(key, a)
-        status[key] = a
+        status_set_key(status, key, a)
 
     async def _b_to_a():
         logger.info(f"Copying {key} to {storage_a}")
         await storage_a.set_meta(key, b)
-        status[key] = b
+        status_set_key(status, key, b)
 
     async def _resolve_conflict():
         if a == b:
-            status[key] = a
+            status_set_key(status, key, a)
         elif conflict_resolution == "a wins":
             await _a_to_b()
         elif conflict_resolution == "b wins":

vdirsyncer/storage/base.py

@@ -1,5 +1,6 @@
 import contextlib
 import functools
+from typing import Optional
 
 from .. import exceptions
 from ..utils import uniq
@@ -219,31 +220,29 @@ class Storage(metaclass=StorageMeta):
         """
         yield
 
-    async def get_meta(self, key):
+    async def get_meta(self, key: str) -> Optional[str]:
         """Get metadata value for collection/storage.
 
         See the vdir specification for the keys that *have* to be accepted.
 
         :param key: The metadata key.
-        :type key: unicode
+        :return: The metadata or None, if metadata is missing.
         """
 
         raise NotImplementedError("This storage does not support metadata.")
 
-    async def set_meta(self, key, value):
+    async def set_meta(self, key: str, value: Optional[str]):
         """Get metadata value for collection/storage.
 
         :param key: The metadata key.
-        :type key: unicode
+        :param value: The value. Use None to delete the data.
-        :param value: The value.
-        :type value: unicode
         """
 
         raise NotImplementedError("This storage does not support metadata.")
 
 
-def normalize_meta_value(value):
+def normalize_meta_value(value) -> Optional[str]:
     # `None` is returned by iCloud for empty properties.
-    if not value or value == "None":
+    if value is None or value == "None":
-        value = ""
+        return
-    return value.strip()
+    return value.strip() if value else ""

vdirsyncer/storage/dav.py

@@ -4,6 +4,7 @@ import urllib.parse as urlparse
 import xml.etree.ElementTree as etree
 from inspect import getfullargspec
 from inspect import signature
+from typing import Optional
 
 import aiohttp
 import aiostream
@@ -679,7 +680,7 @@ class DAVStorage(Storage):
         for href, etag, _prop in rv:
             yield href, etag
 
-    async def get_meta(self, key):
+    async def get_meta(self, key) -> Optional[str]:
         try:
             tagname, namespace = self._property_table[key]
         except KeyError:
@@ -714,7 +715,7 @@ class DAVStorage(Storage):
             text = normalize_meta_value(getattr(prop, "text", None))
             if text:
                 return text
-        return ""
+        return None
 
     async def set_meta(self, key, value):
         try:
@@ -724,18 +725,23 @@ class DAVStorage(Storage):
 
         lxml_selector = f"{{{namespace}}}{tagname}"
         element = etree.Element(lxml_selector)
-        element.text = normalize_meta_value(value)
+        if value is None:
+            action = "remove"
+        else:
+            element.text = normalize_meta_value(value)
+            action = "set"
 
         data = """<?xml version="1.0" encoding="utf-8" ?>
             <propertyupdate xmlns="DAV:">
-                <set>
+                <{action}>
                     <prop>
                         {}
                     </prop>
-                </set>
+                </{action}>
             </propertyupdate>
         """.format(
-            etree.tostring(element, encoding="unicode")
+            etree.tostring(element, encoding="unicode"),
+            action=action,
         ).encode(
             "utf-8"
         )

vdirsyncer/storage/filesystem.py

@@ -183,7 +183,7 @@ class FilesystemStorage(Storage):
                 return normalize_meta_value(f.read().decode(self.encoding))
         except OSError as e:
             if e.errno == errno.ENOENT:
-                return ""
+                return None
             else:
                 raise
 
@@ -191,5 +191,11 @@ class FilesystemStorage(Storage):
         value = normalize_meta_value(value)
 
         fpath = os.path.join(self.path, key)
-        with atomic_write(fpath, mode="wb", overwrite=True) as f:
+        if value is None:
-            f.write(value.encode(self.encoding))
+            try:
+                os.remove(fpath)
+            except OSError:
+                pass
+        else:
+            with atomic_write(fpath, mode="wb", overwrite=True) as f:
+                f.write(value.encode(self.encoding))

vdirsyncer/storage/memory.py

@@ -69,4 +69,7 @@ class MemoryStorage(Storage):
         return normalize_meta_value(self.metadata.get(key))
 
     async def set_meta(self, key, value):
-        self.metadata[key] = normalize_meta_value(value)
+        if value is None:
+            self.metadata.pop(key, None)
+        else:
+            self.metadata[key] = normalize_meta_value(value)