Kill special codepath for null-collection

tests/cli/test_discover.py

@@ -57,27 +57,6 @@ def test_discover_command(tmpdir, runner):
         .read()
 
 
-def test_discover_on_unsupported_storage(tmpdir, runner):
-    runner.write_with_general(dedent('''
-    [storage foo]
-    type = http
-    url = https://example.com/foo.ics
-
-    [storage bar]
-    type = memory
-    fileext = .txt
-
-    [pair foobar]
-    a = foo
-    b = bar
-    collections = ["from a"]
-    ''').format(str(tmpdir)))
-
-    result = runner.invoke(['discover'])
-    assert result.exception
-    assert 'doesn\'t support collection discovery' in result.output
-
-
 def test_discover_different_collection_names(tmpdir, runner):
     foo = tmpdir.mkdir('foo')
     bar = tmpdir.mkdir('bar')
@@ -144,11 +123,47 @@ def test_discover_direct_path(tmpdir, runner):
     collections = null
     ''').format(foo=str(foo), bar=str(bar)))
 
-    result = runner.invoke(['discover'])
+    result = runner.invoke(['discover'], input='y\n' * 2)
     assert not result.exception
 
-    result = runner.invoke(['sync'], input='y\n' * 2)
+    result = runner.invoke(['sync'])
     assert not result.exception
 
     assert foo.exists()
     assert bar.exists()
+
+
+def test_null_collection_with_named_collection(tmpdir, runner):
+    runner.write_with_general(dedent('''
+    [pair foobar]
+    a = foo
+    b = bar
+    collections = [["baz", "baz", null]]
+
+    [storage foo]
+    type = filesystem
+    path = {base}/foo/
+    fileext = .txt
+
+    [storage bar]
+    type = singlefile
+    path = {base}/bar.txt
+    '''.format(base=str(tmpdir))))
+
+    result = runner.invoke(['discover'], input='y\n' * 2)
+    assert not result.exception
+
+    foo = tmpdir.join('foo')
+    foobaz = foo.join('baz')
+    assert foo.exists()
+    assert foobaz.exists()
+
+    bar = tmpdir.join('bar.txt')
+    assert bar.exists()
+
+    foobaz.join('lol.txt').write('BEGIN:VCARD\nUID:HAHA\nEND:VCARD')
+
+    result = runner.invoke(['sync'])
+    assert not result.exception
+
+    assert 'HAHA' in bar.read()

vdirsyncer/cli/utils.py

@@ -250,23 +250,20 @@ def _discover_from_config(config):
     storage_type = config['type']
     cls, config = storage_class_from_config(config)
 
+    discovered = []
+
     try:
-        try:
+        discovered.extend(cls.discover(**config))
-            discovered = list(cls.discover(**config))
+    except NotImplementedError:
-        except NotImplementedError:
+        pass
-            raise exceptions.UserError(
-                'The storage {} (type {}) doesn\'t support collection '
-                'discovery. You can only use `collections = null` with it.'
-                .format(config.get('instance_name', '???'), storage_type)
-            )
     except Exception:
         return handle_storage_init_error(cls, config)
-    else:
+
-        rv = {}
+    rv = {}
-        for args in discovered:
+    for args in discovered:
-            args['type'] = storage_type
+        args['type'] = storage_type
-            rv[args['collection']] = args
+        rv[args['collection']] = args
-        return rv
+    return rv
 
 
 def _handle_collection_not_found(config, collection, e=None):
@@ -298,17 +295,21 @@ def _print_collections(base_config, discovered):
     instance_name = base_config['instance_name']
     cli_logger.info('{}:'.format(coerce_native(instance_name)))
     for args in discovered.values():
+        collection = args['collection']
+        if collection is None:
+            continue
+
         args['instance_name'] = instance_name
         try:
-            storage = storage_instance_from_config(args)
+            storage = storage_instance_from_config(args, create=False)
             displayname = storage.get_meta('displayname')
         except Exception:
             displayname = u''
 
         cli_logger.info('  - {}{}'.format(
-            storage.collection,
+            json.dumps(collection),
             ' ("{}")'.format(coerce_native(displayname))
-            if displayname and displayname != storage.collection
+            if displayname and displayname != collection
             else ''
         ))
 
@@ -318,52 +319,52 @@ def _collections_for_pair_impl(status_path, pair, list_collections=False):
 
     shortcuts = pair.options['collections']
     if shortcuts is None:
-        yield None, (pair.config_a, pair.config_b)
+        shortcuts = [None]
-    else:
-        a_discovered = _discover_from_config(pair.config_a)
-        b_discovered = _discover_from_config(pair.config_b)
 
-        if list_collections:
+    a_discovered = _discover_from_config(pair.config_a)
-            _print_collections(pair.config_a, a_discovered)
+    b_discovered = _discover_from_config(pair.config_b)
-            _print_collections(pair.config_b, b_discovered)
 
-        for shortcut in shortcuts:
+    if list_collections:
-            if shortcut == 'from a':
+        _print_collections(pair.config_a, a_discovered)
-                collections = a_discovered
+        _print_collections(pair.config_b, b_discovered)
-            elif shortcut == 'from b':
+
-                collections = b_discovered
+    for shortcut in shortcuts:
+        if shortcut == 'from a':
+            collections = a_discovered
+        elif shortcut == 'from b':
+            collections = b_discovered
+        else:
+            collections = [shortcut]
+
+        for collection in collections:
+            if isinstance(collection, list):
+                collection, collection_a, collection_b = collection
             else:
-                collections = [shortcut]
+                collection_a = collection_b = collection
 
-            for collection in collections:
+            if collection in handled_collections:
-                if isinstance(collection, list):
+                continue
-                    try:
+            handled_collections.add(collection)
-                        collection, collection_a, collection_b = collection
-                    except ValueError:
-                        raise exceptions.UserError(
-                            'Expected string or list of length 3, '
-                            '{} found instead.'
-                            .format(collection))
-                else:
-                    collection_a = collection_b = collection
 
-                if collection in handled_collections:
+            a_args = _collection_from_discovered(a_discovered, collection_a,
-                    continue
+                                                 pair.config_a)
-                handled_collections.add(collection)
+            b_args = _collection_from_discovered(b_discovered, collection_b,
+                                                 pair.config_b)
 
-                try:
+            yield collection, (a_args, b_args)
-                    a_args = a_discovered[collection_a]
-                except KeyError:
-                    a_args = _handle_collection_not_found(pair.config_a,
-                                                          collection_a)
 
-                try:
-                    b_args = b_discovered[collection_b]
-                except KeyError:
-                    b_args = _handle_collection_not_found(pair.config_b,
-                                                          collection_b)
 
-                yield collection, (a_args, b_args)
+def _collection_from_discovered(discovered, collection, config):
+    if collection is None:
+        args = dict(config)
+        args['collection'] = None
+        storage_instance_from_config(args)
+        return args
+
+    try:
+        return discovered[collection]
+    except KeyError:
+        return _handle_collection_not_found(config, collection)
 
 
 def load_status(base_path, pair, collection=None, data_type=None):