From 4ca0e6b67c934f96a8cd10c7cd52516835dc54c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=94=D0=B8=D0=BB=D1=8F=D0=BD=20=D0=9F=D0=B0=D0=BB=D0=B0?= =?UTF-8?q?=D1=83=D0=B7=D0=BE=D0=B2?= Date: Sat, 2 May 2020 12:54:52 +0000 Subject: [PATCH] cli/discover: remove/add local collections if the remote collection is deleted/created This works when the destination backend is 'filesystem'. -- add a new parameter to storage section: implicit = ["create", "delete"] Changes cli/utils.py:save_status(): when data is None, remove the underlaying file. --- CHANGELOG.rst | 4 ++++ docs/config.rst | 8 ++++++++ tests/system/cli/test_config.py | 7 ++++++- tests/system/utils/test_main.py | 2 +- vdirsyncer/cli/config.py | 8 ++++++++ vdirsyncer/cli/discover.py | 29 +++++++++++++++++++++++++++++ vdirsyncer/cli/utils.py | 24 ++++++++++++++++++++++-- vdirsyncer/storage/base.py | 24 +++++++++++++++++++++++- vdirsyncer/storage/filesystem.py | 18 +++++++++++++++--- 9 files changed, 116 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index ddb1018d..0f66f160 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -29,6 +29,10 @@ Version 0.19.0 you're validating certificate fingerprints, use `sha256` instead. - The ``google`` storage types no longer require ``requests-oauthlib``, but require ``python-aiohttp-oauthlib`` instead. +- Add ``implicit`` option to storage section. It creates/deletes implicitly + collections in the destinations, when new collections are created/deleted + in the source. The deletion is implemented only for the "filesystem" storage. + See :ref:`storage_config`. Version 0.18.0 ============== diff --git a/docs/config.rst b/docs/config.rst index f6a65563..98fd9e63 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -415,6 +415,8 @@ Local #encoding = "utf-8" #post_hook = null #fileignoreext = ".tmp" + #implicit = "create" + #implicit = ["create", "delete"] Can be used with `khal `_. See :doc:`vdir` for a more formal description of the format. @@ -437,6 +439,12 @@ Local new/updated file. :param fileeignoreext: The file extention to ignore. It is only useful if fileext is set to the empty string. The default is ``.tmp``. + :param implicit: When a new collection is created on the source, and the + value is "create", create the collection in the destination without + asking questions. When the value is "delete" and a collection + is removed on the source, remove it in the destination. The value + can be a string or an array of strings. The deletion is implemented + only for the "filesystem" storage. .. storage:: singlefile diff --git a/tests/system/cli/test_config.py b/tests/system/cli/test_config.py index 3e037c99..64c16c2e 100644 --- a/tests/system/cli/test_config.py +++ b/tests/system/cli/test_config.py @@ -60,8 +60,13 @@ def test_read_config(read_config): "yesno": False, "number": 42, "instance_name": "bob_a", + "implicit": [], + }, + "bob_b": { + "type": "carddav", + "instance_name": "bob_b", + "implicit": [], }, - "bob_b": {"type": "carddav", "instance_name": "bob_b"}, } diff --git a/tests/system/utils/test_main.py b/tests/system/utils/test_main.py index 44035827..2409932d 100644 --- a/tests/system/utils/test_main.py +++ b/tests/system/utils/test_main.py @@ -21,7 +21,7 @@ def test_get_storage_init_args(): from vdirsyncer.storage.memory import MemoryStorage all, required = utils.get_storage_init_args(MemoryStorage) - assert all == {"fileext", "collection", "read_only", "instance_name"} + assert all == {"fileext", "collection", "read_only", "instance_name", "implicit"} assert not required diff --git a/vdirsyncer/cli/config.py b/vdirsyncer/cli/config.py index 516d2b52..2bf238f7 100644 --- a/vdirsyncer/cli/config.py +++ b/vdirsyncer/cli/config.py @@ -115,6 +115,14 @@ def _parse_section(self, section_type, name, options): raise ValueError("More than one general section.") self._general = options elif section_type == "storage": + if "implicit" not in options: + options["implicit"] = [] + elif isinstance(options["implicit"], str): + options["implicit"] = [options["implicit"]] + elif not isinstance(options["implicit"], list): + raise ValueError( + "`implicit` parameter must be a list, string or absent." + ) self._storages[name] = options elif section_type == "pair": self._pairs[name] = options diff --git a/vdirsyncer/cli/discover.py b/vdirsyncer/cli/discover.py index 21c91aeb..7f7e1318 100644 --- a/vdirsyncer/cli/discover.py +++ b/vdirsyncer/cli/discover.py @@ -8,7 +8,9 @@ import aiostream from .. import exceptions +from . import cli_logger from .utils import handle_collection_not_found +from .utils import handle_collection_was_removed from .utils import handle_storage_init_error from .utils import load_status from .utils import save_status @@ -118,6 +120,33 @@ async def collections_for_pair( "cache_key": cache_key, }, ) + + if "from b" in (pair.collections or []): + only_in_a = set(a_discovered.get_self().keys()) - set( + b_discovered.get_self().keys() + ) + if only_in_a and "delete" in pair.config_a["implicit"]: + for a in only_in_a: + try: + handle_collection_was_removed(pair.config_a, a) + save_status(status_path, pair.name, a, data_type="metadata") + save_status(status_path, pair.name, a, data_type="items") + except NotImplementedError as e: + cli_logger.error(e) + + if "from a" in (pair.collections or []): + only_in_b = set(b_discovered.get_self().keys()) - set( + a_discovered.get_self().keys() + ) + if only_in_b and "delete" in pair.config_b["implicit"]: + for b in only_in_b: + try: + handle_collection_was_removed(pair.config_b, b) + save_status(status_path, pair.name, b, data_type="metadata") + save_status(status_path, pair.name, b, data_type="items") + except NotImplementedError as e: + cli_logger.error(e) + return rv diff --git a/vdirsyncer/cli/utils.py b/vdirsyncer/cli/utils.py index 15d37633..5314c9d9 100644 --- a/vdirsyncer/cli/utils.py +++ b/vdirsyncer/cli/utils.py @@ -231,10 +231,15 @@ def manage_sync_status(base_path, pair_name, collection_name): def save_status(base_path, pair, collection=None, data_type=None, data=None): assert data_type is not None - assert data is not None status_name = get_status_name(pair, collection) path = expand_path(os.path.join(base_path, status_name)) + "." + data_type prepare_status_path(path) + if data is None: + try: + os.remove(path) + except OSError: # the file has not existed + pass + return with atomic_write(path, mode="w", overwrite=True) as f: json.dump(data, f) @@ -334,6 +339,19 @@ def assert_permissions(path, wanted): os.chmod(path, wanted) +def handle_collection_was_removed(config, collection): + if "delete" in config["implicit"]: + storage_type = config["type"] + cls, config = storage_class_from_config(config) + config["collection"] = collection + try: + args = cls.delete_collection(**config) + args["type"] = storage_type + return args + except NotImplementedError as e: + cli_logger.error(e) + + async def handle_collection_not_found(config, collection, e=None): storage_name = config.get("instance_name", None) @@ -343,7 +361,9 @@ async def handle_collection_not_found(config, collection, e=None): ) ) - if click.confirm("Should vdirsyncer attempt to create it?"): + if "create" in config["implicit"] or click.confirm( + "Should vdirsyncer attempt to create it?" + ): storage_type = config["type"] cls, config = storage_class_from_config(config) config["collection"] = collection diff --git a/vdirsyncer/storage/base.py b/vdirsyncer/storage/base.py index aff3cdf2..8b96b573 100644 --- a/vdirsyncer/storage/base.py +++ b/vdirsyncer/storage/base.py @@ -42,6 +42,9 @@ class Storage(metaclass=StorageMeta): :param read_only: Whether the synchronization algorithm should avoid writes to this storage. Some storages accept no value other than ``True``. + :param implicit: Whether the synchronization shall create/delete collections + in the destination, when these were created/removed from the source. Must + be a possibly empty list of strings. """ fileext = ".txt" @@ -65,9 +68,16 @@ class Storage(metaclass=StorageMeta): # The attribute values to show in the representation of the storage. _repr_attributes = () - def __init__(self, instance_name=None, read_only=None, collection=None): + def __init__( + self, + instance_name=None, + read_only=None, + collection=None, + implicit=None, + ): if read_only is None: read_only = self.read_only + self.implicit = implicit # unused from within the Storage classes if self.read_only and not read_only: raise exceptions.UserError("This storage can only be read-only.") self.read_only = bool(read_only) @@ -109,6 +119,18 @@ async def create_collection(cls, collection, **kwargs): """ raise NotImplementedError() + @classmethod + def delete_collection(cls, collection, **kwargs): + """ + Delete the specified collection and return the new arguments. + + ``collection=None`` means the arguments are already pointing to a + possible collection location. + + The returned args should contain the collection name, for UI purposes. + """ + raise NotImplementedError() + def __repr__(self): try: if self.instance_name: diff --git a/vdirsyncer/storage/filesystem.py b/vdirsyncer/storage/filesystem.py index 6656705e..746d2461 100644 --- a/vdirsyncer/storage/filesystem.py +++ b/vdirsyncer/storage/filesystem.py @@ -1,6 +1,7 @@ import errno import logging import os +import shutil import subprocess from atomicwrites import atomic_write @@ -62,9 +63,7 @@ async def discover(cls, path, **kwargs): def _validate_collection(cls, path): if not os.path.isdir(path): return False - if os.path.basename(path).startswith("."): - return False - return True + return not os.path.basename(path).startswith(".") @classmethod async def create_collection(cls, collection, **kwargs): @@ -80,6 +79,19 @@ async def create_collection(cls, collection, **kwargs): kwargs["collection"] = collection return kwargs + @classmethod + def delete_collection(cls, collection, **kwargs): + kwargs = dict(kwargs) + path = kwargs["path"] + + if collection is not None: + path = os.path.join(path, collection) + shutil.rmtree(path, ignore_errors=True) + + kwargs["path"] = path + kwargs["collection"] = collection + return kwargs + def _get_filepath(self, href): return os.path.join(self.path, href)