Merge pull request #470 from pimutils/collections-null-fix

Kill special codepath for `null`-collection
This commit is contained in:
Markus Unterwaditzer 2016-06-16 15:30:16 +02:00 committed by GitHub
commit 8d0b055110
4 changed files with 97 additions and 81 deletions

View file

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

View file

@ -234,7 +234,7 @@ def test_invalid_pairs_as_cli_arg(tmpdir, runner):
result = runner.invoke(['sync', 'foobar/d'])
assert result.exception
assert 'pair foobar: collection d not found' in result.output.lower()
assert 'pair foobar: collection "d" not found' in result.output.lower()
def test_multiple_pairs(tmpdir, runner):

View file

@ -33,7 +33,7 @@ def prepare_pair(wq, pair_name, collections, config, callback, **kwargs):
'Pair {}: Collection {} not found. These are the '
'configured collections:\n{}'
.format(pair_name,
coerce_native(collection_name),
json.dumps(collection_name),
list(all_collections)))
new_workers += 1

View file

@ -250,23 +250,20 @@ def _discover_from_config(config):
storage_type = config['type']
cls, config = storage_class_from_config(config)
discovered = []
try:
try:
discovered = list(cls.discover(**config))
except NotImplementedError:
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)
)
discovered.extend(cls.discover(**config))
except NotImplementedError:
pass
except Exception:
return handle_storage_init_error(cls, config)
else:
rv = {}
for args in discovered:
args['type'] = storage_type
rv[args['collection']] = args
return rv
rv = {}
for args in discovered:
args['type'] = storage_type
rv[args['collection']] = args
return rv
def _handle_collection_not_found(config, collection, e=None):
@ -274,7 +271,7 @@ def _handle_collection_not_found(config, collection, e=None):
cli_logger.error('{}No collection {} found for storage {}.'
.format('{}\n'.format(e) if e else '',
coerce_native(collection), storage_name))
json.dumps(collection), storage_name))
if click.confirm('Should vdirsyncer attempt to create it?'):
storage_type = config['type']
@ -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)
else:
a_discovered = _discover_from_config(pair.config_a)
b_discovered = _discover_from_config(pair.config_b)
shortcuts = [None]
if list_collections:
_print_collections(pair.config_a, a_discovered)
_print_collections(pair.config_b, b_discovered)
a_discovered = _discover_from_config(pair.config_a)
b_discovered = _discover_from_config(pair.config_b)
for shortcut in shortcuts:
if shortcut == 'from a':
collections = a_discovered
elif shortcut == 'from b':
collections = b_discovered
if list_collections:
_print_collections(pair.config_a, a_discovered)
_print_collections(pair.config_b, 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 isinstance(collection, list):
try:
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:
continue
handled_collections.add(collection)
if collection in handled_collections:
continue
handled_collections.add(collection)
a_args = _collection_from_discovered(a_discovered, collection_a,
pair.config_a)
b_args = _collection_from_discovered(b_discovered, collection_b,
pair.config_b)
try:
a_args = a_discovered[collection_a]
except KeyError:
a_args = _handle_collection_not_found(pair.config_a,
collection_a)
yield collection, (a_args, b_args)
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):