From 492e12b7e65aacb606b0a23ac7afd2a6997f06c1 Mon Sep 17 00:00:00 2001 From: Joohwan Oh Date: Sun, 25 Aug 2019 04:32:05 -0700 Subject: [PATCH] Properly populate exceptions from bulk operations --- arango/collection.py | 95 ++++++++++++++++-------------------------- arango/connection.py | 25 +++++++++++ arango/exceptions.py | 1 - arango/version.py | 2 +- tests/test_document.py | 41 +++++++++++++----- 5 files changed, 92 insertions(+), 72 deletions(-) diff --git a/arango/collection.py b/arango/collection.py index dcc08243..c45c0d14 100644 --- a/arango/collection.py +++ b/arango/collection.py @@ -37,7 +37,6 @@ IndexLoadError, ) from arango.request import Request -from arango.response import Response from arango.utils import ( get_doc_id, is_none_or_int, @@ -1648,20 +1647,13 @@ def response_handler(resp): return True results = [] - for result in resp.body: - if '_id' in result: - if '_oldRev' in result: - result['_old_rev'] = result.pop('_oldRev') - results.append(result) + for body in resp.body: + if '_id' in body: + if '_oldRev' in body: + body['_old_rev'] = body.pop('_oldRev') + results.append(body) else: - sub_resp = Response( - method=resp.method, - url=resp.url, - headers=resp.headers, - status_code=resp.status_code, - status_text=resp.status_text, - raw_body=result - ) + sub_resp = self._conn.build_error_response(resp, body) results.append(DocumentInsertError(sub_resp, request)) return results @@ -1814,23 +1806,17 @@ def response_handler(resp): return True results = [] - for result in resp.body: - if '_id' not in result: - sub_resp = Response( - method='patch', - url=resp.url, - headers=resp.headers, - status_code=resp.status_code, - status_text=resp.status_text, - raw_body=result, - ) - if result['errorNum'] == 1200: - result = DocumentRevisionError(sub_resp, request) - else: # pragma: no cover - result = DocumentUpdateError(sub_resp, request) + for body in resp.body: + if '_id' in body: + body['_old_rev'] = body.pop('_oldRev') + results.append(body) else: - result['_old_rev'] = result.pop('_oldRev') - results.append(result) + sub_resp = self._conn.build_error_response(resp, body) + if sub_resp.error_code == 1200: + error = DocumentRevisionError(sub_resp, request) + else: # pragma: no cover + error = DocumentUpdateError(sub_resp, request) + results.append(error) return results @@ -2019,23 +2005,17 @@ def response_handler(resp): return True results = [] - for result in resp.body: - if '_id' not in result: - sub_resp = Response( - method=resp.method, - url=resp.url, - headers=resp.headers, - status_code=resp.status_code, - status_text=resp.status_text, - raw_body=result - ) - if result['errorNum'] == 1200: - result = DocumentRevisionError(sub_resp, request) - else: # pragma: no cover - result = DocumentReplaceError(sub_resp, request) + for body in resp.body: + if '_id' in body: + body['_old_rev'] = body.pop('_oldRev') + results.append(body) else: - result['_old_rev'] = result.pop('_oldRev') - results.append(result) + sub_resp = self._conn.build_error_response(resp, body) + if sub_resp.error_code == 1200: + error = DocumentRevisionError(sub_resp, request) + else: # pragma: no cover + error = DocumentReplaceError(sub_resp, request) + results.append(error) return results @@ -2212,21 +2192,16 @@ def response_handler(resp): return True results = [] - for result in resp.body: - if '_id' not in result: - sub_resp = Response( - method=resp.method, - url=resp.url, - headers=resp.headers, - status_code=resp.status_code, - status_text=resp.status_text, - raw_body=result - ) - if result['errorNum'] == 1200: - result = DocumentRevisionError(sub_resp, request) + for body in resp.body: + if '_id' in body: + results.append(body) + else: + sub_resp = self._conn.build_error_response(resp, body) + if sub_resp.error_code == 1200: + error = DocumentRevisionError(sub_resp, request) else: - result = DocumentDeleteError(sub_resp, request) - results.append(result) + error = DocumentDeleteError(sub_resp, request) + results.append(error) return results diff --git a/arango/connection.py b/arango/connection.py index 915041f2..00bc7a7a 100644 --- a/arango/connection.py +++ b/arango/connection.py @@ -3,6 +3,7 @@ from six import string_types from arango.exceptions import ServerConnectionError +from arango.response import Response __all__ = ['Connection'] @@ -105,6 +106,30 @@ def prep_response(self, response): response.is_success = http_ok and response.error_code is None return response + def build_error_response(self, parent_response, body): + """Build and return a bulk error response. + + :param parent_response: Parent response. + :type parent_response: arango.response.Response + :param body: Error response body. + :type body: dict + :return: Child bulk error response. + :rtype: arango.response.Response + """ + response = Response( + method=parent_response.method, + url=parent_response.url, + headers=parent_response.headers, + status_code=parent_response.status_code, + status_text=parent_response.status_text, + raw_body=self.serialize(body), + ) + response.body = body + response.error_code = body['errorNum'] + response.error_message = body['errorMessage'] + response.is_success = False + return response + def send_request(self, request): """Send an HTTP request to ArangoDB server. diff --git a/arango/exceptions.py b/arango/exceptions.py index ba4883e1..9b54a7dc 100644 --- a/arango/exceptions.py +++ b/arango/exceptions.py @@ -80,7 +80,6 @@ def __init__(self, resp, request, msg=None): self.http_code = resp.status_code self.http_headers = resp.headers - ################## # AQL Exceptions # ################## diff --git a/arango/version.py b/arango/version.py index a0f66580..a5e45131 100644 --- a/arango/version.py +++ b/arango/version.py @@ -1 +1 @@ -__version__ = '5.0.0' +__version__ = '5.1.0' diff --git a/tests/test_document.py b/tests/test_document.py index effba66d..ca8c8c8d 100644 --- a/tests/test_document.py +++ b/tests/test_document.py @@ -189,8 +189,12 @@ def test_document_insert_many(col, bad_col, docs): # Test insert_many duplicate documents results = col.insert_many(docs, return_new=False) - for result, doc in zip(results, docs): - assert isinstance(result, DocumentInsertError) + for error, doc in zip(results, docs): + assert isinstance(error, DocumentInsertError) + assert error.error_code in {1210} + assert 'unique constraint violated' in error.error_message + assert error.http_code == 202 + assert '[HTTP 202][ERR 1210]' in error.message # Test insert_many with overwrite and return_old set to True results = col.insert_many(docs, overwrite=True, return_old=True) @@ -455,8 +459,13 @@ def test_document_update_many(col, bad_col, docs): doc['val'] = 5 doc['_rev'] = old_revs[doc['_key']] + '0' results = col.update_many(docs, check_rev=True) - for result, doc_key in zip(results, doc_keys): - assert isinstance(result, DocumentRevisionError) + for error, doc_key in zip(results, doc_keys): + assert isinstance(error, DocumentRevisionError) + assert error.error_code in {1200} + assert 'conflict' in error.error_message + assert error.http_code == 202 + assert '[HTTP 202][ERR 1200]' in error.message + for doc in col: assert doc['val'] == 4 @@ -729,8 +738,12 @@ def test_document_replace_many(col, bad_col, docs): doc.pop('baz') doc['_rev'] = old_revs[doc['_key']] + '0' results = col.replace_many(docs, check_rev=True) - for result, doc_key in zip(results, doc_keys): - assert isinstance(result, DocumentRevisionError) + for error, doc_key in zip(results, doc_keys): + assert isinstance(error, DocumentRevisionError) + assert error.error_code in {1200} + assert 'conflict' in error.error_message + assert error.http_code == 202 + assert '[HTTP 202][ERR 1200]' in error.message for doc in col: assert 'foo' not in doc assert doc['baz'] == 4 @@ -957,8 +970,12 @@ def test_document_delete_many(col, bad_col, docs): for doc in docs: doc['_rev'] = old_revs[doc['_key']] + '0' results = col.delete_many(docs, check_rev=True) - for result, doc in zip(results, docs): - assert isinstance(result, DocumentRevisionError) + for error, doc in zip(results, docs): + assert isinstance(error, DocumentRevisionError) + assert error.error_code in {1200} + assert 'conflict' in error.error_message + assert error.http_code == 202 + assert '[HTTP 202][ERR 1200]' in error.message assert len(col) == 6 # Test delete_many (documents) with missing documents @@ -968,8 +985,12 @@ def test_document_delete_many(col, bad_col, docs): {'_key': generate_doc_key()}, {'_key': generate_doc_key()} ]) - for result, doc in zip(results, docs): - assert isinstance(result, DocumentDeleteError) + for error, doc in zip(results, docs): + assert isinstance(error, DocumentDeleteError) + assert error.error_code in {1202} + assert 'document not found' in error.error_message + assert error.http_code == 202 + assert '[HTTP 202][ERR 1202]' in error.message assert len(col) == 0 # Test delete_many with bad database