diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 7be2ff9..11526ea 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,9 @@ Version 0.4.1 *yet to be released* +- All ``create`` arguments from all storages are gone. Vdirsyncer now asks if + it should try to create collections. + Version 0.4.0 ============= diff --git a/example.cfg b/example.cfg index b71278c..db1c292 100644 --- a/example.cfg +++ b/example.cfg @@ -39,8 +39,6 @@ collections = ["from b"] type = filesystem path = ~/.contacts/ fileext = .vcf -# Create the directory if it doesn't exist: `true` or `false` -#create = true [storage bob_contacts_remote] type = carddav diff --git a/tests/storage/test_filesystem.py b/tests/storage/test_filesystem.py index 1a4bb71..25339c6 100644 --- a/tests/storage/test_filesystem.py +++ b/tests/storage/test_filesystem.py @@ -35,20 +35,12 @@ class TestFilesystemStorage(StorageTests): return {'path': path, 'fileext': '.txt', 'collection': collection} return inner - def test_create_is_false(self, tmpdir): - with pytest.raises(IOError): - self.storage_class(str(tmpdir) + '/lol', '.txt', create=False) - def test_is_not_directory(self, tmpdir): with pytest.raises(IOError): f = tmpdir.join('hue') f.write('stub') self.storage_class(str(tmpdir) + '/hue', '.txt') - def test_create_is_true(self, tmpdir): - self.storage_class(str(tmpdir) + '/asd', '.txt') - assert tmpdir.listdir() == [tmpdir.join('asd')] - def test_broken_data(self, tmpdir): s = self.storage_class(str(tmpdir), '.txt') diff --git a/tests/storage/test_http_with_singlefile.py b/tests/storage/test_http_with_singlefile.py index 5c0cce7..c063a32 100644 --- a/tests/storage/test_http_with_singlefile.py +++ b/tests/storage/test_http_with_singlefile.py @@ -53,7 +53,7 @@ class TestHttpStorage(StorageTests): @pytest.fixture(autouse=True) def setup_tmpdir(self, tmpdir, monkeypatch): - self.tmpfile = str(tmpdir.join('collection.txt')) + self.tmpfile = str(tmpdir.ensure('collection.txt')) def _request(method, url, *args, **kwargs): assert method == 'GET' diff --git a/tests/storage/test_singlefile.py b/tests/storage/test_singlefile.py index e5163dd..a70c22d 100644 --- a/tests/storage/test_singlefile.py +++ b/tests/storage/test_singlefile.py @@ -21,7 +21,7 @@ class TestSingleFileStorage(StorageTests): @pytest.fixture(autouse=True) def setup(self, tmpdir): - self._path = str(tmpdir.join('test.txt')) + self._path = str(tmpdir.ensure('test.txt')) @pytest.fixture def get_storage_args(self): @@ -33,14 +33,3 @@ class TestSingleFileStorage(StorageTests): def test_collection_arg(self, tmpdir): with pytest.raises(ValueError): self.storage_class(str(tmpdir.join('foo.ics')), collection='ha') - - def test_create_arg(self, tmpdir): - s = self.storage_class(str(tmpdir) + '/foo.ics') - assert not s.list() - - s.create = False - with pytest.raises(IOError): - s.list() - - with pytest.raises(IOError): - s = self.storage_class(str(tmpdir) + '/foo.ics', create=False) diff --git a/tests/test_cli.py b/tests/test_cli.py index b41e269..b42c95f 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -126,6 +126,9 @@ def test_simple_run(tmpdir, runner): fileext = .txt ''').format(str(tmpdir))) + tmpdir.mkdir('path_a') + tmpdir.mkdir('path_b') + result = runner.invoke(['sync']) assert not result.exception @@ -152,6 +155,9 @@ def test_empty_storage(tmpdir, runner): fileext = .txt ''').format(str(tmpdir))) + tmpdir.mkdir('path_a') + tmpdir.mkdir('path_b') + result = runner.invoke(['sync']) assert not result.exception @@ -274,7 +280,10 @@ def test_collections_cache_invalidation(tmpdir, runner): foo = tmpdir.mkdir('foo') bar = tmpdir.mkdir('bar') - foo.mkdir('a').join('itemone.txt').write('UID:itemone') + for x in 'abc': + foo.mkdir(x) + bar.mkdir(x) + foo.join('a/itemone.txt').write('UID:itemone') result = runner.invoke(['sync']) assert not result.exception @@ -302,6 +311,8 @@ def test_collections_cache_invalidation(tmpdir, runner): tmpdir.join('status').remove() bar2 = tmpdir.mkdir('bar2') + for x in 'abc': + bar2.mkdir(x) result = runner.invoke(['sync']) assert not result.exception @@ -329,8 +340,10 @@ def test_invalid_pairs_as_cli_arg(tmpdir, runner): collections = ["a", "b", "c"] ''').format(str(tmpdir))) - tmpdir.mkdir('foo') - tmpdir.mkdir('bar') + for base in ('foo', 'bar'): + base = tmpdir.mkdir(base) + for c in 'abc': + base.mkdir(c) result = runner.invoke(['sync', 'foobar/d']) assert result.exception @@ -356,11 +369,12 @@ def test_discover_command(tmpdir, runner): ''').format(str(tmpdir))) foo = tmpdir.mkdir('foo') - tmpdir.mkdir('bar') + bar = tmpdir.mkdir('bar') - foo.mkdir('a') - foo.mkdir('b') - foo.mkdir('c') + for x in 'abc': + foo.mkdir(x) + bar.mkdir(x) + bar.mkdir('d') result = runner.invoke(['sync']) assert not result.exception @@ -369,10 +383,14 @@ def test_discover_command(tmpdir, runner): assert 'Syncing foobar/a' in lines assert 'Syncing foobar/b' in lines assert 'Syncing foobar/c' in lines + assert 'Syncing foobar/d' not in lines foo.mkdir('d') result = runner.invoke(['sync']) assert not result.exception + assert 'Syncing foobar/a' in lines + assert 'Syncing foobar/b' in lines + assert 'Syncing foobar/c' in lines assert 'Syncing foobar/d' not in result.output result = runner.invoke(['discover']) @@ -380,6 +398,9 @@ def test_discover_command(tmpdir, runner): result = runner.invoke(['sync']) assert not result.exception + assert 'Syncing foobar/a' in lines + assert 'Syncing foobar/b' in lines + assert 'Syncing foobar/c' in lines assert 'Syncing foobar/d' in result.output @@ -403,12 +424,12 @@ def test_multiple_pairs(tmpdir, runner): runner.write_with_general(''.join(get_cfg())) result = runner.invoke(['sync']) - assert sorted(result.output.splitlines()) == [ + assert set(result.output.splitlines()) > set([ 'Discovering collections for pair bambaz', 'Discovering collections for pair foobar', 'Syncing bambaz', 'Syncing foobar', - ] + ]) def test_invalid_collections_arg(tmpdir, runner): @@ -437,6 +458,37 @@ def test_invalid_collections_arg(tmpdir, runner): ) +def test_create_collections(tmpdir, runner): + runner.write_with_general(dedent(''' + [pair foobar] + a = foo + b = bar + collections = ["a", "b", "c"] + + [storage foo] + type = filesystem + path = {base}/foo/ + fileext = .txt + + [storage bar] + type = filesystem + path = {base}/bar/ + fileext = .txt + '''.format(base=str(tmpdir)))) + + result = runner.invoke(['sync']) + assert result.exception + entries = set(x.basename for x in tmpdir.listdir()) + assert 'foo' not in entries and 'bar' not in entries + + result = runner.invoke(['sync'], input='y\n' * 6) + assert not result.exception + assert \ + set(x.basename for x in tmpdir.join('foo').listdir()) == \ + set(x.basename for x in tmpdir.join('bar').listdir()) == \ + set('abc') + + def test_parse_config_value(): x = cli.utils.parse_config_value with pytest.raises(ValueError): diff --git a/vdirsyncer/cli/utils.py b/vdirsyncer/cli/utils.py index 368db19..211e0ba 100644 --- a/vdirsyncer/cli/utils.py +++ b/vdirsyncer/cli/utils.py @@ -16,7 +16,7 @@ import sys import threading from itertools import chain -from .. import DOCS_HOME, PROJECT_HOME, log +from .. import DOCS_HOME, PROJECT_HOME, exceptions, log from ..doubleclick import click from ..storage import storage_names from ..sync import StorageEmpty, SyncConflict @@ -167,22 +167,27 @@ def _get_coll(pair_name, storage_name, collection, discovered, config): try: return discovered[collection] except KeyError: + return _handle_collection_not_found(config, collection) + + +def _handle_collection_not_found(config, collection): + storage_name = config.get('instance_name', None) + cli_logger.error('No collection {} found for storage {}.' + .format(collection, storage_name)) + if click.confirm('Should vdirsyncer attempt to create it?'): storage_type = config['type'] cls, config = storage_class_from_config(config) try: - args = cls.join_collection(collection=collection, **config) + args = cls.create_collection(collection=collection, **config) args['type'] = storage_type return args except NotImplementedError as e: cli_logger.error(e) - raise CliError( - '{pair}: Unable to find or create collection {collection!r} ' - 'for storage {storage!r}. A same-named collection was found ' - 'for the other storage, and vdirsyncer is configured to ' - 'synchronize these two collections. Please create the ' - 'collection yourself.' - .format(pair=pair_name, collection=collection, - storage=storage_name)) + + raise CliError('Unable to find or create collection {collection!r} for ' + 'storage {storage!r}. Please create the collection ' + 'yourself.'.format(collection=collection, + storage=storage_name)) def _collections_for_pair_impl(status_path, name_a, name_b, pair_name, @@ -353,19 +358,25 @@ def storage_class_from_config(config): return cls, config -def storage_instance_from_config(config): +def storage_instance_from_config(config, create=True): ''' :param config: A configuration dictionary to pass as kwargs to the class corresponding to config['type'] - :param description: A name for the storage for debugging purposes ''' - cls, config = storage_class_from_config(config) + cls, new_config = storage_class_from_config(config) try: - return cls(**config) + return cls(**new_config) + except exceptions.CollectionNotFound as e: + cli_logger.error(str(e)) + if create: + _handle_collection_not_found(config, None) + return storage_instance_from_config(config, create=False) + else: + raise except Exception: - handle_storage_init_error(cls, config) + return handle_storage_init_error(cls, new_config) def handle_storage_init_error(cls, config): diff --git a/vdirsyncer/exceptions.py b/vdirsyncer/exceptions.py index a77e77d..b9f8cdf 100644 --- a/vdirsyncer/exceptions.py +++ b/vdirsyncer/exceptions.py @@ -23,6 +23,10 @@ class Error(Exception): super(Error, self).__init__(*args) +class CollectionNotFound(Error): + '''Collection not found''' + + class PreconditionFailed(Error): ''' - The item doesn't exist although it should diff --git a/vdirsyncer/storage/base.py b/vdirsyncer/storage/base.py index 0a3fd64..0ff4aee 100644 --- a/vdirsyncer/storage/base.py +++ b/vdirsyncer/storage/base.py @@ -109,10 +109,11 @@ class Storage(with_metaclass(StorageMeta)): raise NotImplementedError() @classmethod - def join_collection(cls, collection, **kwargs): - '''Append the collection to the URL or path specified in ``**kwargs`` - and return the new arguments. - ''' + def create_collection(cls, collection, **kwargs): + '''Create the specified collection and return the new arguments. + + ``collection=None`` means the arguments are already pointing to a + possible collection location.''' raise NotImplementedError() def __repr__(self): diff --git a/vdirsyncer/storage/dav.py b/vdirsyncer/storage/dav.py index 4c78087..f78bfe0 100644 --- a/vdirsyncer/storage/dav.py +++ b/vdirsyncer/storage/dav.py @@ -65,6 +65,11 @@ def _fuzzy_matches_mimetype(strict, weak): return False +def _get_collection_from_url(url): + _, collection = url.rstrip('/').rsplit('/', 1) + return collection + + def _catch_generator_exceptions(f): @functools.wraps(f) def inner(*args, **kwargs): @@ -334,14 +339,16 @@ class DavStorage(Storage): d = cls.discovery_class(cls._get_session(**kwargs)) for c in d.discover(): url = c['href'] - _, collection = url.rstrip('/').rsplit('/', 1) + collection = _get_collection_from_url(url) storage_args = dict(kwargs) storage_args.update({'url': url, 'collection': collection, 'collection_human': c['displayname']}) yield storage_args @classmethod - def join_collection(cls, collection, **kwargs): + def create_collection(cls, collection, **kwargs): + if collection is None: + collection = _get_collection_from_url(kwargs['url']) session = cls._get_session(**kwargs) d = cls.discovery_class(session) diff --git a/vdirsyncer/storage/filesystem.py b/vdirsyncer/storage/filesystem.py index 6a17c59..670d3e0 100644 --- a/vdirsyncer/storage/filesystem.py +++ b/vdirsyncer/storage/filesystem.py @@ -33,16 +33,15 @@ class FilesystemStorage(Storage): trigger a re-download of everything (but *should* not cause data-loss of any kind). :param encoding: File encoding for items. - :param create: Create directories if they don't exist. ''' storage_name = 'filesystem' _repr_attributes = ('path',) - def __init__(self, path, fileext, encoding='utf-8', create=True, **kwargs): + def __init__(self, path, fileext, encoding='utf-8', **kwargs): super(FilesystemStorage, self).__init__(**kwargs) path = expand_path(path) - checkdir(path, create=create) + checkdir(path, create=False) self.path = path self.encoding = encoding self.fileext = fileext @@ -66,8 +65,10 @@ class FilesystemStorage(Storage): yield args @classmethod - def join_collection(cls, collection, **kwargs): - kwargs['path'] = os.path.join(kwargs['path'], collection) + def create_collection(cls, collection, **kwargs): + if collection is not None: + kwargs['path'] = os.path.join(kwargs['path'], collection) + checkdir(kwargs['path'], create=True) return kwargs def _get_filepath(self, href): diff --git a/vdirsyncer/storage/singlefile.py b/vdirsyncer/storage/singlefile.py index 76dffc4..705e9b6 100644 --- a/vdirsyncer/storage/singlefile.py +++ b/vdirsyncer/storage/singlefile.py @@ -38,7 +38,6 @@ class SingleFileStorage(Storage): :param path: The filepath to the file to be written to. :param encoding: Which encoding the file should use. Defaults to UTF-8. - :param create: Create the file if it does not exist. Example for syncing with :py:class:`vdirsyncer.storage.CaldavStorage`:: @@ -68,7 +67,7 @@ class SingleFileStorage(Storage): _items = None _last_mtime = None - def __init__(self, path, encoding='utf-8', create=True, **kwargs): + def __init__(self, path, encoding='utf-8', **kwargs): super(SingleFileStorage, self).__init__(**kwargs) path = expand_path(path) @@ -77,15 +76,19 @@ class SingleFileStorage(Storage): raise ValueError('collection is not a valid argument for {}' .format(type(self).__name__)) - checkfile(path, create=create) - - if create: - self._write_mode = 'wb+' - self._append_mode = 'ab+' + checkfile(path, create=False) self.path = path self.encoding = encoding - self.create = create + + @classmethod + def create_collection(cls, collection, **kwargs): + if collection is not None: + raise ValueError('collection is not a valid argument for {}' + .format(cls.__name__)) + + checkfile(kwargs['path'], create=True) + return kwargs def list(self): self._items = collections.OrderedDict() diff --git a/vdirsyncer/utils/__init__.py b/vdirsyncer/utils/__init__.py index f917717..90f5f8d 100644 --- a/vdirsyncer/utils/__init__.py +++ b/vdirsyncer/utils/__init__.py @@ -325,10 +325,8 @@ def checkdir(path, create=False, mode=0o750): if create: os.makedirs(path, mode) else: - raise IOError('Directory {} does not exist. Use create = ' - 'True in your configuration to automatically ' - 'create it, or create it ' - 'yourself.'.format(path)) + raise exceptions.CollectionNotFound('Directory {} does not exist.' + .format(path)) def checkfile(path, create=False): @@ -343,10 +341,8 @@ def checkfile(path, create=False): if os.path.exists(path): raise IOError('{} is not a file.'.format(path)) if not create: - raise IOError('File {} does not exist. Use create = ' - 'True in your configuration to automatically ' - 'create it, or create it ' - 'yourself.'.format(path)) + raise exceptions.CollectionNotFound('File {} does not exist.' + .format(path)) class cached_property(object):