diff --git a/docs/api.rst b/docs/api.rst index 86ad02d..10474ef 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -21,7 +21,7 @@ General Section next sync. The data is needed to determine whether a new item means it has been added on one side or deleted on the other. -- ``processes``: Optional, defines the amount of maximal connections to use for +- ``processes``: Optional, defines the maximal amount of threads to use for syncing. By default there is no limit, which means vdirsyncer will try to open a connection for each collection to be synced. The value ``0`` is ignored. Setting this to ``1`` will only synchronize one collection at a @@ -32,6 +32,12 @@ General Section Raspberry Pi is so slow that multiple connections don't help much, since the CPU and not the network is the bottleneck. + .. note:: + + Due to restrictions in Python's threading module, setting ``processes`` + to anything else than ``1`` will mean that you can't properly abort the + program with ``^C`` anymore. + .. _pair_config: Pair Section diff --git a/docs/keyring.rst b/docs/keyring.rst index 74c4db5..1c4ccd8 100644 --- a/docs/keyring.rst +++ b/docs/keyring.rst @@ -26,14 +26,8 @@ To use it, you must install keyring_. .. _keyring: https://bitbucket.org/kang/python-keyring-lib -*vdirsyncer* will use the full resource URL as the key when saving. - -When retrieving the key, it will try to remove segments of the URL's path until -it finds a password. For example, if you save a password under the key -``vdirsyncer:http://example.com``, it will be used as a fallback for all -resources on ``example.com``. If you additionally save a password under the key -``vdirsyncer:http://example.com/special/``, that password will be used for all -resources on ``example.com`` whose path starts with ``/special/``. +*vdirsyncer* will use the hostname as key prefixed with ``vdirsyncer:`` when +saving and fetching, e.g. ``vdirsyncer:owncloud.example.com``. *keyring* support these keyrings: diff --git a/tests/test_doubleclick.py b/tests/test_doubleclick.py new file mode 100644 index 0000000..3f1c8fc --- /dev/null +++ b/tests/test_doubleclick.py @@ -0,0 +1,23 @@ +# -*- coding: utf-8 -*- +''' + tests.test_doubleclick + ~~~~~~~~~~~~~~~~~~~~~~ + + :copyright: (c) 2014 Markus Unterwaditzer & contributors + :license: MIT, see LICENSE for more details. +''' +from click.testing import CliRunner + +from vdirsyncer.doubleclick import _ctx_stack, click, ctx as global_ctx + + +def test_simple(): + @click.command() + @click.pass_context + def cli(ctx): + assert global_ctx + assert _ctx_stack.top is ctx + + assert not global_ctx + runner = CliRunner() + runner.invoke(cli) diff --git a/tests/utils/test_main.py b/tests/utils/test_main.py index 929a48a..3d7b7f2 100644 --- a/tests/utils/test_main.py +++ b/tests/utils/test_main.py @@ -8,14 +8,31 @@ ''' import click + from click.testing import CliRunner import pytest import vdirsyncer.utils as utils +import vdirsyncer.doubleclick as doubleclick from vdirsyncer.utils.vobject import split_collection from .. import blow_up, normalize_item, SIMPLE_TEMPLATE, BARE_EVENT_TEMPLATE +class EmptyNetrc(object): + def authenticators(self, hostname): + return None + +class EmptyKeyring(object): + def get_password(self, *a, **kw): + return None + + +@pytest.fixture(autouse=True) +def empty_password_storages(monkeypatch): + monkeypatch.setattr('netrc.netrc', EmptyNetrc) + monkeypatch.setattr(utils, 'keyring', EmptyKeyring()) + + def test_parse_options(): o = { 'foo': 'yes', @@ -67,31 +84,17 @@ def test_get_password_from_netrc(monkeypatch): assert calls == [hostname] -@pytest.mark.parametrize('resources_to_test', range(1, 8)) -def test_get_password_from_system_keyring(monkeypatch, resources_to_test): +def test_get_password_from_system_keyring(monkeypatch): username = 'foouser' password = 'foopass' resource = 'http://example.com/path/to/whatever/' hostname = 'example.com' class KeyringMock(object): - def __init__(self): - p = utils.password_key_prefix - self.resources = [ - p + 'http://example.com/path/to/whatever/', - p + 'http://example.com/path/to/whatever', - p + 'http://example.com/path/to/', - p + 'http://example.com/path/to', - p + 'http://example.com/path/', - p + 'http://example.com/path', - p + 'http://example.com/', - ][:resources_to_test] - def get_password(self, resource, _username): assert _username == username - assert resource == self.resources.pop(0) - if not self.resources: - return password + assert resource == utils.password_key_prefix + hostname + return password monkeypatch.setattr(utils, 'keyring', KeyringMock()) @@ -110,20 +113,9 @@ def test_get_password_from_system_keyring(monkeypatch, resources_to_test): assert netrc_calls == [hostname] -def test_get_password_from_prompt(monkeypatch): +def test_get_password_from_prompt(): getpass_calls = [] - class Netrc(object): - def authenticators(self, hostname): - return None - - class Keyring(object): - def get_password(self, *a, **kw): - return None - - monkeypatch.setattr('netrc.netrc', Netrc) - monkeypatch.setattr(utils, 'keyring', Keyring()) - user = 'my_user' resource = 'http://example.com' @@ -136,8 +128,35 @@ def test_get_password_from_prompt(monkeypatch): result = runner.invoke(fake_app, input='my_password\n\n') assert not result.exception assert result.output.splitlines() == [ - 'Server password for {} at the resource {}: '.format(user, resource), + 'Server password for {} at host {}: '.format(user, 'example.com'), + 'Password is my_password' + ] + + +def test_get_password_from_cache(monkeypatch): + user = 'my_user' + resource = 'http://example.com' + + @doubleclick.click.command() + @doubleclick.click.pass_context + def fake_app(ctx): + ctx.obj = {} + x = utils.get_password(user, resource) + click.echo('Password is {}'.format(x)) + monkeypatch.setattr(doubleclick.click, 'prompt', blow_up) + + assert (user, 'example.com') in ctx.obj['passwords'] + x = utils.get_password(user, resource) + click.echo('Password is {}'.format(x)) + + runner = CliRunner() + result = runner.invoke(fake_app, input='my_password\n') + assert not result.exception + assert result.output.splitlines() == [ + 'Server password for {} at host {}: '.format(user, 'example.com'), 'Save this password in the keyring? [y/N]: ', + 'Password is my_password', + 'debug: Got password for my_user from internal cache', 'Password is my_password' ] diff --git a/vdirsyncer/cli.py b/vdirsyncer/cli.py index 526c272..6078495 100644 --- a/vdirsyncer/cli.py +++ b/vdirsyncer/cli.py @@ -350,13 +350,14 @@ def _create_app(): cli_logger.debug('Using {} processes.'.format(processes)) if processes == 1: - cli_logger.debug('Not using multiprocessing.') + cli_logger.debug('Not using threads.') rv = (_sync_collection(x) for x in actions) else: - cli_logger.debug('Using multiprocessing.') - from multiprocessing import Pool + cli_logger.debug('Using threads.') + from multiprocessing.dummy import Pool p = Pool(processes=general.get('processes', 0) or len(actions)) - rv = p.map_async(_sync_collection, actions).get(10**9) + + rv = p.imap_unordered(_sync_collection, actions) if not all(rv): sys.exit(1) @@ -377,13 +378,15 @@ def sync_collection(config_a, config_b, pair_name, collection, pair_options, collection_description = pair_name if collection is None \ else '{} from {}'.format(collection, pair_name) - a = storage_instance_from_config(config_a) - b = storage_instance_from_config(config_b) - - cli_logger.info('Syncing {}'.format(collection_description)) - status = load_status(general['status_path'], status_name) rv = True try: + cli_logger.info('Syncing {}'.format(collection_description)) + + a = storage_instance_from_config(config_a) + b = storage_instance_from_config(config_b) + + status = load_status(general['status_path'], status_name) + cli_logger.debug('Loaded status for {}'.format(collection_description)) sync( a, b, status, conflict_resolution=pair_options.get('conflict_resolution', None), @@ -414,10 +417,13 @@ def sync_collection(config_a, config_b, pair_name, collection, pair_options, 'Item href on side B: {e.href_b}\n' .format(collection=collection_description, e=e, docs=DOCS_HOME) ) - except Exception: + except (click.Abort, KeyboardInterrupt): + rv = False + except Exception as e: rv = False cli_logger.exception('Unhandled exception occured while syncing {}.' .format(collection_description)) - save_status(general['status_path'], status_name, status) + if rv: + save_status(general['status_path'], status_name, status) return rv diff --git a/vdirsyncer/doubleclick.py b/vdirsyncer/doubleclick.py index c8aed1d..0529981 100644 --- a/vdirsyncer/doubleclick.py +++ b/vdirsyncer/doubleclick.py @@ -3,53 +3,134 @@ vdirsyncer.utils.doubleclick ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - Utilities for writing multiprocessing applications with click. + Utilities for writing threaded applications with click. - Currently the only relevant object here is the ``click`` object, which - provides everything importable from click. It also wraps some UI functions - such that they don't produce overlapping output or prompt the user at the - same time. + Two objects are useful: + + - There is a global ``ctx`` object to be used. + + - The ``click`` object's attributes are supposed to be used instead of the + click package's content. + + - It wraps some UI functions such that they don't produce overlapping + output or prompt the user at the same time. + + - It wraps BaseCommand subclasses such that their invocation changes the + ctx global, and also changes the shortcut decorators to use the new + classes. :copyright: (c) 2014 Markus Unterwaditzer & contributors :license: MIT, see LICENSE for more details. ''' import functools -import multiprocessing - -UI_FUNCTIONS = frozenset(['echo', 'echo_via_pager', 'prompt', 'clear', 'edit', - 'launch', 'getchar', 'pause']) +import threading -_ui_lock = multiprocessing.Lock() +class _ClickProxy(object): + def __init__(self, wrappers, click=None): + if click is None: + import click + self._click = click + self._cache = {} + self._wrappers = dict(wrappers) + + def __getattr__(self, name): + if name not in self._cache: + f = getattr(self._click, name) + f = self._wrappers.get(name, lambda x: x)(f) + self._cache[name] = f + + return self._cache[name] + + +_ui_lock = threading.Lock() def _ui_function(f): @functools.wraps(f) def inner(*a, **kw): - _ui_lock.acquire() - try: - return f(*a, **kw) - finally: - _ui_lock.release() + with _ui_lock: + rv = f(*a, **kw) + return rv return inner -class _ClickProxy(object): - def __init__(self, needs_wrapper, click=None): - if click is None: - import click - self._click = click - self._cache = {} - self._needs_wrapper = frozenset(needs_wrapper) +class _Stack(object): + def __init__(self): + self._stack = [] + + @property + def top(self): + return self._stack[-1] + + def push(self, value): + self._stack.append(value) + + def pop(self): + return self._stack.pop() + + +class _StackProxy(object): + def __init__(self, stack): + self._doubleclick_stack = stack + + def __bool__(self): + try: + self._doubleclick_stack.top + except IndexError: + return False + else: + return True + + __nonzero__ = __bool__ def __getattr__(self, name): - if name not in self._cache: - f = getattr(self._click, name) - if name in self._needs_wrapper: - f = _ui_function(f) - self._cache[name] = f + return getattr(self._doubleclick_stack.top, name) - return self._cache[name] -click = _ClickProxy(UI_FUNCTIONS) +_ctx_stack = _Stack() +ctx = _StackProxy(_ctx_stack) + + +def _ctx_pushing_class(cls): + class ContextPusher(cls): + def invoke(self, ctx): + _ctx_stack.push(ctx) + try: + cls.invoke(self, ctx) + finally: + _ctx_stack.pop() + + return ContextPusher + + +def _command_class_wrapper(cls_name): + def inner(f): + def wrapper(name=None, **attrs): + attrs.setdefault('cls', getattr(click, cls_name)) + return f(name, **attrs) + return wrapper + return inner + + +WRAPPERS = { + 'echo': _ui_function, + 'echo_via_pager': _ui_function, + 'prompt': _ui_function, + 'confirm': _ui_function, + 'clear': _ui_function, + 'edit': _ui_function, + 'launch': _ui_function, + 'getchar': _ui_function, + 'pause': _ui_function, + 'BaseCommand': _ctx_pushing_class, + 'Command': _ctx_pushing_class, + 'MultiCommand': _ctx_pushing_class, + 'Group': _ctx_pushing_class, + 'CommandCollection': _ctx_pushing_class, + 'command': _command_class_wrapper('Command'), + 'group': _command_class_wrapper('Group') +} + +click = _ClickProxy(WRAPPERS) diff --git a/vdirsyncer/utils/__init__.py b/vdirsyncer/utils/__init__.py index 75bc384..20f9e00 100644 --- a/vdirsyncer/utils/__init__.py +++ b/vdirsyncer/utils/__init__.py @@ -8,11 +8,12 @@ ''' import os +import threading import requests from .. import exceptions, log -from ..doubleclick import click +from ..doubleclick import click, ctx from .compat import iteritems, urlparse @@ -88,7 +89,7 @@ def parse_options(items, section=None): .format(section, key, e)) -def get_password(username, resource): +def get_password(username, resource, _lock=threading.Lock()): """tries to access saved password or asks user for it will try the following in this order: @@ -110,71 +111,58 @@ def get_password(username, resource): """ - for func in (_password_from_netrc, _password_from_keyring): - password = func(username, resource) - if password is not None: - logger.debug('Got password for {} from {}' - .format(username, func.__doc__)) - return password + if ctx: + password_cache = ctx.obj.setdefault('passwords', {}) - prompt = ('Server password for {} at the resource {}' - .format(username, resource)) - password = click.prompt(prompt, hide_input=True) + with _lock: + host = urlparse.urlsplit(resource).hostname + for func in (_password_from_cache, _password_from_netrc, + _password_from_keyring): + password = func(username, host) + if password is not None: + logger.debug('Got password for {} from {}' + .format(username, func.__doc__)) + return password - if keyring is not None and \ - click.confirm('Save this password in the keyring?', default=False): - keyring.set_password(password_key_prefix + resource, - username, password) + prompt = ('Server password for {} at host {}'.format(username, host)) + password = click.prompt(prompt, hide_input=True) - return password + if ctx and func is not _password_from_cache: + password_cache[(username, host)] = password + if keyring is not None and \ + click.confirm('Save this password in the keyring?', + default=False): + keyring.set_password(password_key_prefix + resource, + username, password) + + return password -def _password_from_netrc(username, resource): +def _password_from_cache(username, host): + '''internal cache''' + if ctx: + return ctx.obj['passwords'].get((username, host), None) + + +def _password_from_netrc(username, host): '''.netrc''' from netrc import netrc - hostname = urlparse.urlsplit(resource).hostname try: netrc_user, account, password = \ - netrc().authenticators(hostname) or (None, None, None) + netrc().authenticators(host) or (None, None, None) if netrc_user == username: return password except IOError: pass -def _password_from_keyring(username, resource): +def _password_from_keyring(username, host): '''system keyring''' if keyring is None: return None - key = resource - password = None - - while True: - password = keyring.get_password(password_key_prefix + key, username) - if password is not None: - return password - - parsed = urlparse.urlsplit(key) - path = parsed.path - if not path: - return None - elif path.endswith('/'): - path = path.rstrip('/') - else: - path = path.rsplit('/', 1)[0] + '/' - - new_key = urlparse.urlunsplit(( - parsed.scheme, - parsed.netloc, - path, - parsed.query, - parsed.fragment - )) - if new_key == key: - return None - key = new_key + return keyring.get_password(password_key_prefix + host, username) def request(method, url, data=None, headers=None, auth=None, verify=None,