diff --git a/vcsserver/exceptions.py b/vcsserver/exceptions.py --- a/vcsserver/exceptions.py +++ b/vcsserver/exceptions.py @@ -71,7 +71,7 @@ def VcsException(org_exc=None): return _make_exception_wrapper -def RepositoryLockedException(org_exc=None): +def LockedRepoException(org_exc=None): def _make_exception_wrapper(*args): return _make_exception('repo_locked', org_exc, *args) return _make_exception_wrapper diff --git a/vcsserver/hooks.py b/vcsserver/hooks.py --- a/vcsserver/hooks.py +++ b/vcsserver/hooks.py @@ -41,54 +41,6 @@ celery_app = Celery('__vcsserver__') log = logging.getLogger(__name__) -class HooksHttpClient: - proto = 'msgpack.v1' - connection = None - - def __init__(self, hooks_uri): - self.hooks_uri = hooks_uri - - def __repr__(self): - return f'{self.__class__}(hook_uri={self.hooks_uri}, proto={self.proto})' - - def __call__(self, method, extras): - connection = http.client.HTTPConnection(self.hooks_uri) - # binary msgpack body - headers, body = self._serialize(method, extras) - log.debug('Doing a new hooks call using HTTPConnection to %s', self.hooks_uri) - - try: - try: - connection.request('POST', '/', body, headers) - except Exception as error: - log.error('Hooks calling Connection failed on %s, org error: %s', connection.__dict__, error) - raise - - response = connection.getresponse() - try: - return msgpack.load(response) - except Exception: - response_data = response.read() - log.exception('Failed to decode hook response json data. ' - 'response_code:%s, raw_data:%s', - response.status, response_data) - raise - finally: - connection.close() - - @classmethod - def _serialize(cls, hook_name, extras): - data = { - 'method': hook_name, - 'extras': extras - } - headers = { - "rc-hooks-protocol": cls.proto, - "Connection": "keep-alive" - } - return headers, msgpack.packb(data) - - class HooksCeleryClient: TASK_TIMEOUT = 60 # time in seconds @@ -160,17 +112,20 @@ class SvnMessageWriter(RemoteMessageWrit def _maybe_handle_exception(result): + + exception_class = result.get('exception') exception_traceback = result.get('exception_traceback') - if not (exception_class and exception_traceback): + if not exception_class: return + log.debug('Handling hook-call exception: %s', exception_class) if exception_traceback: log.error('Got traceback from remote call:%s', exception_traceback) - if exception_class == 'HTTPLockedRC': - raise exceptions.RepositoryLockedException()(*result['exception_args']) + if exception_class == 'HTTPLockedRepo': + raise exceptions.LockedRepoException()(*result['exception_args']) elif exception_class == 'ClientNotSupportedError': raise exceptions.ClientNotSupportedException()(*result['exception_args']) elif exception_class == 'HTTPBranchProtected': @@ -184,14 +139,11 @@ def _maybe_handle_exception(result): def _get_hooks_client(extras): - hooks_uri = extras.get('hooks_uri') task_queue = extras.get('task_queue') task_backend = extras.get('task_backend') is_shadow_repo = extras.get('is_shadow_repo') - if hooks_uri: - return HooksHttpClient(hooks_uri) - elif task_queue and task_backend: + if task_queue and task_backend: return HooksCeleryClient(task_queue, task_backend) elif is_shadow_repo: return HooksShadowRepoClient() diff --git a/vcsserver/tests/test_hooks.py b/vcsserver/tests/test_hooks.py --- a/vcsserver/tests/test_hooks.py +++ b/vcsserver/tests/test_hooks.py @@ -15,17 +15,11 @@ # along with this program; if not, write to the Free Software Foundation, # Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA -import threading -import msgpack - -from http.server import BaseHTTPRequestHandler -from socketserver import TCPServer +import pytest import mercurial.ui import mock -import pytest -from vcsserver.hooks import HooksHttpClient from vcsserver.lib.ext_json import json from vcsserver import hooks @@ -41,7 +35,6 @@ def get_hg_ui(extras=None): 'make_lock': '', 'action': '', 'ip': '', - 'hooks_uri': 'fake_hooks_uri', } required_extras.update(extras) hg_ui = mercurial.ui.ui() @@ -123,15 +116,6 @@ def test_git_post_pull_is_disabled(): class TestGetHooksClient: - def test_returns_http_client_when_protocol_matches(self): - hooks_uri = 'localhost:8000' - result = hooks._get_hooks_client({ - 'hooks_uri': hooks_uri, - 'hooks_protocol': 'http' - }) - assert isinstance(result, hooks.HooksHttpClient) - assert result.hooks_uri == hooks_uri - def test_return_celery_client_when_queue_and_backend_provided(self): task_queue = 'redis://task_queue:0' task_backend = task_queue @@ -142,116 +126,10 @@ class TestGetHooksClient: assert isinstance(result, hooks.HooksCeleryClient) -class TestHooksHttpClient: - def test_init_sets_hooks_uri(self): - uri = 'localhost:3000' - client = hooks.HooksHttpClient(uri) - assert client.hooks_uri == uri - - def test_serialize_returns_serialized_string(self): - client = hooks.HooksHttpClient('localhost:3000') - hook_name = 'test' - extras = { - 'first': 1, - 'second': 'two' - } - hooks_proto, result = client._serialize(hook_name, extras) - expected_result = msgpack.packb({ - 'method': hook_name, - 'extras': extras, - }) - assert hooks_proto == {'rc-hooks-protocol': 'msgpack.v1', 'Connection': 'keep-alive'} - assert result == expected_result - - def test_call_queries_http_server(self, http_mirror): - client = hooks.HooksHttpClient(http_mirror.uri) - hook_name = 'test' - extras = { - 'first': 1, - 'second': 'two' - } - result = client(hook_name, extras) - expected_result = msgpack.unpackb(msgpack.packb({ - 'method': hook_name, - 'extras': extras - }), raw=False) - assert result == expected_result - - -@pytest.fixture -def http_mirror(request): - server = MirrorHttpServer() - request.addfinalizer(server.stop) - return server - - -class MirrorHttpHandler(BaseHTTPRequestHandler): - - def do_POST(self): - length = int(self.headers['Content-Length']) - body = self.rfile.read(length) - self.send_response(200) - self.end_headers() - self.wfile.write(body) - - -class MirrorHttpServer: - ip_address = '127.0.0.1' - port = 0 +class TestHooksCeleryClient: - def __init__(self): - self._daemon = TCPServer((self.ip_address, 0), MirrorHttpHandler) - _, self.port = self._daemon.server_address - self._thread = threading.Thread(target=self._daemon.serve_forever) - self._thread.daemon = True - self._thread.start() - - def stop(self): - self._daemon.shutdown() - self._thread.join() - self._daemon = None - self._thread = None - - @property - def uri(self): - return '{}:{}'.format(self.ip_address, self.port) - - -def test_hooks_http_client_init(): - hooks_uri = 'http://localhost:8000' - client = HooksHttpClient(hooks_uri) - assert client.hooks_uri == hooks_uri - - -def test_hooks_http_client_call(): - hooks_uri = 'http://localhost:8000' - - method = 'test_method' - extras = {'key': 'value'} - - with \ - mock.patch('http.client.HTTPConnection') as mock_connection,\ - mock.patch('msgpack.load') as mock_load: - - client = HooksHttpClient(hooks_uri) - - mock_load.return_value = {'result': 'success'} - response = mock.MagicMock() - response.status = 200 - mock_connection.request.side_effect = None - mock_connection.getresponse.return_value = response - - result = client(method, extras) - - mock_connection.assert_called_with(hooks_uri) - mock_connection.return_value.request.assert_called_once() - assert result == {'result': 'success'} - - -def test_hooks_http_client_serialize(): - method = 'test_method' - extras = {'key': 'value'} - headers, body = HooksHttpClient._serialize(method, extras) - - assert headers == {'rc-hooks-protocol': HooksHttpClient.proto, 'Connection': 'keep-alive'} - assert msgpack.unpackb(body) == {'method': method, 'extras': extras} + def test_hooks_http_client_init(self): + queue = 'redis://redis:6379/0' + backend = 'redis://redis:6379/0' + client = hooks.HooksCeleryClient(queue, backend) + assert client.celery_app.conf.broker_url == queue