Merge branch 'unify_creation'

fix #157

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
 =============
 

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

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')
 

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'

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)

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):

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):

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

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):

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)
 

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):

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()

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):