Unify collection creation

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
 
@@ -276,7 +282,7 @@ def test_collections_cache_invalidation(tmpdir, runner):
     bar = tmpdir.mkdir('bar')
     foo.mkdir('a').join('itemone.txt').write('UID:itemone')
 
-    result = runner.invoke(['sync'])
+    result = runner.invoke(['sync'], input='y\n' * 5)
     assert not result.exception
 
     rv = bar.join('a').listdir()
@@ -302,7 +308,7 @@ def test_collections_cache_invalidation(tmpdir, runner):
 
     tmpdir.join('status').remove()
     bar2 = tmpdir.mkdir('bar2')
-    result = runner.invoke(['sync'])
+    result = runner.invoke(['sync'], input='y\n' * 3)
     assert not result.exception
 
     rv = bar.join('a').listdir()
@@ -329,8 +335,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
@@ -362,7 +370,7 @@ def test_discover_command(tmpdir, runner):
     foo.mkdir('b')
     foo.mkdir('c')
 
-    result = runner.invoke(['sync'])
+    result = runner.invoke(['sync'], input='y\n' * 3)
     assert not result.exception
     lines = result.output.splitlines()
     assert lines[0].startswith('Discovering')
@@ -375,7 +383,7 @@ def test_discover_command(tmpdir, runner):
     assert not result.exception
     assert 'Syncing foobar/d' not in result.output
 
-    result = runner.invoke(['discover'])
+    result = runner.invoke(['discover'], input='y\n')
     assert not result.exception
 
     result = runner.invoke(['sync'])
@@ -403,12 +411,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):

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,6 +167,14 @@ 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:
@@ -175,14 +183,11 @@ def _get_coll(pair_name, storage_name, collection, discovered, config):
             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,24 @@ 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:
+        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/storage/base.py

@@ -110,7 +110,10 @@ class Storage(with_metaclass(StorageMeta)):
 
     @classmethod
     def create_collection(cls, collection, **kwargs):
-        '''Create the specified collection and return the new arguments.'''
+        '''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,7 +339,7 @@ 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']})
@@ -342,6 +347,8 @@ class DavStorage(Storage):
 
     @classmethod
     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

@@ -67,8 +67,9 @@ class FilesystemStorage(Storage):
 
     @classmethod
     def create_collection(cls, collection, **kwargs):
-        kwargs['path'] = path = os.path.join(kwargs['path'], collection)
-        checkdir(path, create=True)
+        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

@@ -68,7 +68,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 +77,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(type(self).__name__))
+
+        checkfile(kwargs['path'], create=True)
+        return kwargs
 
     def list(self):
         self._items = collections.OrderedDict()