diff --git a/client_src/test/test_integration.py b/client_src/test/test_integration.py index 43d757e0..40cf1bdb 100644 --- a/client_src/test/test_integration.py +++ b/client_src/test/test_integration.py @@ -175,7 +175,6 @@ def test_save_docs_unknown_coll(self): { 'error': { 'message': 'Not found', - 'status': 404, 'details': "Collection 'xyz123' does not exist.", 'name': 'xyz123', } diff --git a/dev-requirements.txt b/dev-requirements.txt index ab184106..399563a0 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -3,4 +3,4 @@ bandit==1.5.1 mccabe==0.6.1 flake8==3.5.0 grequests==0.3.0 -coverage==4.5.1 +coverage==5.2.1 diff --git a/relation_engine_server/README.md b/relation_engine_server/README.md index 7198b7e1..1689158d 100644 --- a/relation_engine_server/README.md +++ b/relation_engine_server/README.md @@ -13,6 +13,21 @@ The API is a small, rest-ish service where all data is in JSON format. Replace t * Staging: `https://ci.kbase.us/services/relation_engine_api` * App-dev: `https://appdev.kbase.us/services/relation_engine_api` +### Error responses + +The majority of errors returned from the server have explanatory information in the response content in the following format: + +```json + +{ + "error": { + "message": "A brief message explaining the error", + } +} +``` + +Specific errors may have other fields giving more details, e.g. JSON parsing errors have `source_json`, `pos`, `lineno`, and `colno` describing the error; ArangoDB errors have an `arango_message` field. + ### GET / Returns server status info @@ -168,7 +183,7 @@ _Response JSON schema_ If you try to update a collection and it fails validation against a JSON schema found in the [relation engine spec](spec/), then you will get a JSON error response with the following fields: -* `"error"` - Human readable message explaining the error +* `"message"` - Human readable message explaining the error * `"failed_validator"` - The name of the validator that failed (eg. "required") * `"value"` - The (possibly nested) value in your data that failed validation * `"path"` - The path into your data where you can find the value that failed validation @@ -428,9 +443,9 @@ curl -X PUT -H "Authorization: " \ ## Deprecated Endpoints -#### GET `/api/v1/specs/schemas` (replaced by `/api/v1/specs/schemas`) +#### GET `/api/v1/specs/schemas` (replaced by `/api/v1/specs/collections`) -This endpoint has been deprecated; queries should use `/api/v1/specs/schemas` instead. +This endpoint has been deprecated; queries should use `/api/v1/specs/collections` instead. ## Development diff --git a/relation_engine_server/exceptions.py b/relation_engine_server/exceptions.py index 095c2dc4..5b18f839 100644 --- a/relation_engine_server/exceptions.py +++ b/relation_engine_server/exceptions.py @@ -24,7 +24,7 @@ def __str__(self): class UnauthorizedAccess(Exception): - "Authentication failed for an authorization header.""" + """Authentication failed for an authorization header.""" def __init__(self, auth_url, response): self.auth_url = auth_url diff --git a/relation_engine_server/main.py b/relation_engine_server/main.py index 9425d42a..00a0c2f8 100644 --- a/relation_engine_server/main.py +++ b/relation_engine_server/main.py @@ -18,6 +18,23 @@ app.register_blueprint(api_v1, url_prefix='/api/v1') +def return_error(error_dict, code): + """return the appropriate error structure and code + + Errors returned by the server have the basic format + + 'error': { + 'message': , + } + + The 'error' dictionary may have extra keys if there is additional information. + + This helper wraps the whole structure in an extra dict under the key 'error'. + + """ + return (flask.jsonify({'error': error_dict}), code) + + @app.route('/', methods=['GET']) def root(): """Server status.""" @@ -40,29 +57,32 @@ def root(): def json_decode_error(err): """A problem parsing json.""" resp = { - 'error': 'Unable to parse JSON', + 'message': 'Unable to parse JSON', 'source_json': err.doc, 'pos': err.pos, 'lineno': err.lineno, - 'colno': err.colno + 'colno': err.colno, } - return (flask.jsonify(resp), 400) + return return_error(resp, 400) @app.errorhandler(arango_client.ArangoServerError) def arango_server_error(err): resp = { - 'error': str(err), - 'arango_message': err.resp_json['errorMessage'] + 'message': str(err), + 'arango_message': err.resp_json['errorMessage'], } - return (flask.jsonify(resp), 400) + return return_error(resp, 400) +# Invalid request body json params or missing headers +@app.errorhandler(MissingHeader) @app.errorhandler(InvalidParameters) -def invalid_params(err): - """Invalid request body json params.""" - resp = {'error': str(err)} - return (flask.jsonify(resp), 400) +def generic_400(err): + resp = { + 'message': str(err), + } + return return_error(resp, 400) @app.errorhandler(ValidationError) @@ -71,63 +91,52 @@ def validation_error(err): # Refer to the documentation on jsonschema.exceptions.ValidationError: # https://python-jsonschema.readthedocs.io/en/stable/errors/ resp = { - 'error': err.message, + 'message': err.message, 'failed_validator': err.validator, 'value': err.instance, 'path': list(err.absolute_path), } - return (flask.jsonify(resp), 400) + return return_error(resp, 400) @app.errorhandler(UnauthorizedAccess) def unauthorized_access(err): resp = { - 'error': { - 'status': 403, - 'message': 'Unauthorized', - 'auth_url': err.auth_url, - 'auth_response': err.response, - }, + 'message': 'Unauthorized', + 'auth_url': err.auth_url, + 'auth_response': err.response, } - return (flask.jsonify(resp), 403) + return return_error(resp, 403) @app.errorhandler(SchemaNonexistent) def schema_does_not_exist(err): """General error cases.""" resp = { - 'error': { - 'message': 'Not found', - 'status': 404, - 'details': str(err), - 'name': err.name, - } + 'message': 'Not found', + 'details': str(err), + 'name': err.name, } - return (flask.jsonify(resp), 404) + return return_error(resp, 404) @app.errorhandler(NotFound) @app.errorhandler(404) def page_not_found(err): resp = { - 'error': { - 'message': 'Not found', - 'status': 404, - } + 'message': 'Not found', } if hasattr(err, 'details'): - resp['error']['details'] = err.details - return (flask.jsonify(resp), 404) + resp['details'] = err.details + return return_error(resp, 404) @app.errorhandler(405) def method_not_allowed(err): - return (flask.jsonify({'error': {'message': 'Method not allowed', 'status': 405}}), 405) - - -@app.errorhandler(MissingHeader) -def generic_400(err): - return (flask.jsonify({'error': {'message': str(err), 'status': 400}}), 400) + resp = { + 'message': 'Method not allowed', + } + return return_error(resp, 405) # Any other unhandled exceptions -> 500 @@ -139,11 +148,13 @@ def server_error(err): print('-' * 80) traceback.print_exc() print('=' * 80) - resp = {'error': {'status': 500, 'message': 'Unexpected server error'}} + resp = { + 'message': 'Unexpected server error' + } # TODO only set below two fields in dev mode - resp['error']['class'] = err.__class__.__name__ - resp['error']['details'] = str(err) - return (flask.jsonify(resp), 500) + resp['class'] = err.__class__.__name__ + resp['details'] = str(err) + return return_error(resp, 500) @app.after_request diff --git a/relation_engine_server/test/data/test_file.md b/relation_engine_server/test/data/test_file.md new file mode 100644 index 00000000..e69de29b diff --git a/relation_engine_server/test/test_api_v1.py b/relation_engine_server/test/test_api_v1.py index 97532572..21c50ded 100644 --- a/relation_engine_server/test/test_api_v1.py +++ b/relation_engine_server/test/test_api_v1.py @@ -63,19 +63,22 @@ def setUpClass(cls): def test_request(self, url=None, params=None, data=None, headers=None, method='get', status_code=200, resp_json=None, resp_test=None): - '''test a get request to the server + '''test a request to the server arguments: url url to be appended to API_URL (i.e. request will be made to API_URL + url) params request parameters + data query data, encoded as JSON method HTTP method; defaults to 'get' status_code expected response status; defaults to 200 resp_json expected response content (JSON) - resp_test a function to perform on the response to test it is as expected + resp_test a function to perform on the response to test that it is as expected ''' + # this method should only be run from another test method if url is None: - self.skipTest('No arguments provided') + self.assertTrue(True) + return resp = requests.request( method, @@ -96,20 +99,20 @@ def test_request(self, url=None, params=None, data=None, headers=None, method='g def test_root(self): """Test root path for api.""" - resp = requests.get(URL + '/').json() - self.assertEqual(resp['arangodb_status'], 'connected_authorized') - self.assertTrue(resp['commit_hash']) - self.assertTrue(resp['repo_url']) + resp_json = requests.get(URL + '/').json() + self.assertEqual(resp_json['arangodb_status'], 'connected_authorized') + self.assertTrue(resp_json['commit_hash']) + self.assertTrue(resp_json['repo_url']) def test_config(self): """Test config fetch.""" - resp = requests.get(API_URL + '/config').json() - self.assertTrue(len(resp['auth_url'])) - self.assertTrue(len(resp['workspace_url'])) - self.assertTrue(len(resp['kbase_endpoint'])) - self.assertTrue(len(resp['db_url'])) - self.assertTrue(len(resp['db_name'])) - self.assertTrue(len(resp['spec_url'])) + resp_json = requests.get(API_URL + '/config').json() + self.assertTrue(len(resp_json['auth_url'])) + self.assertTrue(len(resp_json['workspace_url'])) + self.assertTrue(len(resp_json['kbase_endpoint'])) + self.assertTrue(len(resp_json['db_url'])) + self.assertTrue(len(resp_json['db_name'])) + self.assertTrue(len(resp_json['spec_url'])) def test_update_specs(self): """Test the endpoint that triggers an update on the specs.""" @@ -268,7 +271,6 @@ def test_fetch_invalid_data_source(self): status_code=404, resp_json={ 'error': { - 'status': 404, 'message': 'Not found', 'details': f"Data source '{name}' does not exist.", 'name': name, @@ -291,7 +293,6 @@ def test_fetch_invalid_collections_and_documents(self): status_code=404, resp_json={ 'error': { - 'status': 404, 'message': 'Not found', 'details': f"Collection '{name}' does not exist.", 'name': name, @@ -309,7 +310,6 @@ def test_fetch_invalid_stored_queries(self): status_code=404, resp_json={ 'error': { - 'status': 404, 'message': 'Not found', 'details': f"Stored query '{name}' does not exist.", 'name': name, @@ -363,7 +363,6 @@ def test_show_data_source_unknown(self): status_code=404, resp_json={ 'error': { - 'status': 404, 'message': 'Not found', 'details': f"Data source '{name}' does not exist.", 'name': name, @@ -371,55 +370,76 @@ def test_show_data_source_unknown(self): } ) - resp = requests.get(f"{API_URL}/data_sources/{name}") - self.assertEqual(resp.status_code, 404) - resp_json = resp.json() - self.assertEqual(resp_json, { - 'error': { - 'message': 'Not found', - 'status': 404, - 'name': name, - 'details': f"Data source '{name}' does not exist.", - } - }) - def test_save_documents_missing_auth(self): """Test an invalid attempt to save a doc with a missing auth token.""" - resp = requests.put( - API_URL + '/documents?on_duplicate=error&overwrite=true&collection' - ).json() - self.assertEqual(resp['error'], {'message': 'Missing header: Authorization', 'status': 400}) + self.test_request( + '/documents?on_duplicate=error&overwrite=true&collection', + method='put', + status_code=400, + resp_json={'error': {'message': 'Missing header: Authorization'}}, + ) def test_save_documents_invalid_auth(self): """Test an invalid attempt to save a doc with a bad auth token.""" - resp = requests.put( - API_URL + '/documents?on_duplicate=error&overwrite=true&collection', - headers={'Authorization': 'Bearer ' + INVALID_TOKEN} - ).json() - self.assertEqual(resp['error']['message'], 'Unauthorized') - self.assertEqual(resp['error']['status'], 403) + + # see ./mock_auth/auth_invalid.json for the response + auth_response = { + "error": { + "httpcode": 401, + "httpstatus": "Unauthorized", + "appcode": 10020, + "apperror": "Invalid token", + "message": "10020 Invalid token", + "callid": "1757210147564211", + "time": 1542737889450 + } + } + + self.test_request( + '/documents?on_duplicate=error&overwrite=true&collection', + headers={'Authorization': 'Bearer ' + INVALID_TOKEN}, + method='put', + status_code=403, + resp_json={'error': { + 'message': 'Unauthorized', + 'auth_url': 'http://auth:5000', + 'auth_response': json.dumps(auth_response) + }}, + ) def test_save_documents_non_admin(self): """Test an invalid attempt to save a doc as a non-admin.""" - resp = requests.put( - API_URL + '/documents?on_duplicate=error&overwrite=true&collection', - headers=HEADERS_NON_ADMIN - ).json() - self.assertEqual(resp['error']['message'], 'Unauthorized') - self.assertEqual(resp['error']['status'], 403) + self.test_request( + '/documents?on_duplicate=error&overwrite=true&collection', + headers=HEADERS_NON_ADMIN, + method='put', + status_code=403, + resp_json={ + 'error': { + 'auth_response': 'Missing role', + 'auth_url': 'http://auth:5000', + 'message': 'Unauthorized' + } + }, + ) def test_save_documents_invalid_schema(self): """Test the case where some documents fail against their schema.""" - resp = requests.put( - API_URL + '/documents', + + self.test_request( + '/documents', params={'on_duplicate': 'ignore', 'collection': 'test_vertex'}, data='{"name": "x"}\n{"name": "y"}', - headers=HEADERS_ADMIN - ).json() - self.assertEqual(resp['error'], "'_key' is a required property") - self.assertEqual(resp['value'], {'name': 'x'}) - self.assertEqual(resp['path'], []) - self.assertEqual(resp['failed_validator'], 'required') + headers=HEADERS_ADMIN, + method='put', + status_code=400, + resp_json={'error': { + 'message': "'_key' is a required property", + 'value': {'name': 'x'}, + 'path': [], + 'failed_validator': 'required', + }}, + ) def test_save_documents_missing_schema(self): """Test the case where the collection/schema does not exist.""" @@ -434,7 +454,6 @@ def test_save_documents_missing_schema(self): status_code=404, resp_json={ 'error': { - 'status': 404, 'message': 'Not found', 'details': f"Collection '{name}' does not exist.", 'name': name, @@ -444,15 +463,15 @@ def test_save_documents_missing_schema(self): def test_save_documents_invalid_json(self): """Test an attempt to save documents with an invalid JSON body.""" - resp = requests.put( + resp_json = requests.put( API_URL + '/documents', params={'collection': 'test_vertex'}, data='\n', headers=HEADERS_ADMIN ).json() - self.assertTrue('Unable to parse' in resp['error']) - self.assertEqual(resp['pos'], 1) - self.assertEqual(resp['source_json'], '\n') + self.assertTrue('Unable to parse' in resp_json['error']['message']) + self.assertEqual(resp_json['error']['pos'], 1) + self.assertEqual(resp_json['error']['source_json'], '\n') def test_create_documents(self): """Test all valid cases for saving documents.""" @@ -468,14 +487,14 @@ def test_create_edges(self): def test_update_documents(self): """Test updating existing documents.""" - resp = requests.put( + resp_json = requests.put( API_URL + '/documents', params={'on_duplicate': 'update', 'collection': 'test_vertex'}, data=create_test_docs(3), headers=HEADERS_ADMIN ).json() expected = {'created': 0, 'errors': 0, 'empty': 0, 'updated': 3, 'ignored': 0, 'error': False} - self.assertEqual(resp, expected) + self.assertEqual(resp_json, expected) def test_update_edge(self): """Test updating existing edge.""" @@ -487,116 +506,147 @@ def test_update_edge(self): headers=HEADERS_ADMIN ) self.assertTrue(resp.ok) - resp = requests.put( + resp_json = requests.put( API_URL + '/documents', params={'on_duplicate': 'update', 'collection': 'test_edge'}, data=edges, headers=HEADERS_ADMIN ).json() expected = {'created': 0, 'errors': 0, 'empty': 0, 'updated': 3, 'ignored': 0, 'error': False} - self.assertEqual(resp, expected) + self.assertEqual(resp_json, expected) def test_replace_documents(self): """Test replacing of existing documents.""" - resp = requests.put( + resp_json = requests.put( API_URL + '/documents', params={'on_duplicate': 'replace', 'collection': 'test_vertex'}, data=create_test_docs(3), headers=HEADERS_ADMIN ).json() expected = {'created': 0, 'errors': 0, 'empty': 0, 'updated': 3, 'ignored': 0, 'error': False} - self.assertEqual(resp, expected) + self.assertEqual(resp_json, expected) def test_save_documents_dupe_errors(self): """Test where we want to raise errors on duplicate documents.""" save_test_docs(3) - resp = requests.put( + resp_json = requests.put( API_URL + '/documents', params={'on_duplicate': 'error', 'collection': 'test_vertex', 'display_errors': '1'}, data=create_test_docs(3), headers=HEADERS_ADMIN ).json() - self.assertEqual(resp['created'], 0) - self.assertEqual(resp['errors'], 3) - self.assertTrue(resp['details']) + self.assertEqual(resp_json['created'], 0) + self.assertEqual(resp_json['errors'], 3) + self.assertTrue(resp_json['details']) def test_save_documents_ignore_dupes(self): """Test ignoring duplicate, existing documents when saving.""" - resp = requests.put( + resp_json = requests.put( API_URL + '/documents', params={'on_duplicate': 'ignore', 'collection': 'test_vertex'}, data=create_test_docs(3), headers=HEADERS_ADMIN ).json() expected = {'created': 0, 'errors': 0, 'empty': 0, 'updated': 0, 'ignored': 3, 'error': False} - self.assertEqual(resp, expected) + self.assertEqual(resp_json, expected) def test_admin_query(self): """Test an ad-hoc query made by an admin.""" save_test_docs(1) query = 'for v in test_vertex sort rand() limit @count return v._id' - resp = requests.post( + resp_json = requests.post( API_URL + '/query_results', params={}, headers=HEADERS_ADMIN, data=json.dumps({'query': query, 'count': 1}) ).json() - self.assertEqual(resp['count'], 1) - self.assertEqual(len(resp['results']), 1) + self.assertEqual(resp_json['count'], 1) + self.assertEqual(len(resp_json['results']), 1) def test_admin_query_non_admin(self): """Test an ad-hoc query error as a non-admin.""" query = 'for v in test_vertex sort rand() limit @count return v._id' - resp = requests.post( - API_URL + '/query_results', + auth_response = '{"class":"Exception","error":"Unable to match endpoint: POST /"}\n' + self.test_request( + '/query_results', + method='post', params={}, headers=HEADERS_NON_ADMIN, - data=json.dumps({'query': query, 'count': 1}) - ).json() - self.assertEqual(resp['error']['message'], 'Unauthorized') - self.assertEqual(resp['error']['status'], 403) + data=json.dumps({'query': query, 'count': 1}), + status_code=403, + resp_json={'error': { + 'message': 'Unauthorized', + 'auth_url': 'http://workspace:5000', + 'auth_response': auth_response + }}, + ) def test_admin_query_invalid_auth(self): """Test the error response for an ad-hoc admin query without auth.""" + + # see ./mock_workspace/list_workspace_ids_invalid.json for response query = 'for v in test_vertex sort rand() limit @count return v._id' - resp = requests.post( - API_URL + '/query_results', + self.test_request( + '/query_results', + method='post', params={}, headers={'Authorization': INVALID_TOKEN}, - data=json.dumps({'query': query, 'count': 1}) - ).json() - self.assertEqual(resp['error']['message'], 'Unauthorized') - self.assertEqual(resp['error']['status'], 403) + data=json.dumps({'query': query, 'count': 1}), + status_code=403, + resp_json={ + 'error': { + 'message': 'Unauthorized', + 'auth_url': 'http://workspace:5000', + 'auth_response': json.dumps({ + "version": "1.1", + "error": { + "name": "JSONRPCError", + "code": -32400, + "message": "Token validation failed!", + "error": "..." + } + }) + } + } + ) def test_query_with_cursor(self): """Test getting more data via a query cursor and setting batch size.""" save_test_docs(count=20) - resp = requests.post( + resp_json = requests.post( API_URL + '/query_results', params={'stored_query': 'list_test_vertices', 'batch_size': 10, 'full_count': True} ).json() - self.assertTrue(resp['cursor_id']) - self.assertEqual(resp['has_more'], True) - self.assertEqual(resp['count'], 20) - self.assertEqual(resp['stats']['fullCount'], 20) - self.assertTrue(len(resp['results']), 10) - cursor_id = resp['cursor_id'] - resp = requests.post( + self.assertTrue(resp_json['cursor_id']) + self.assertEqual(resp_json['has_more'], True) + self.assertEqual(resp_json['count'], 20) + self.assertEqual(resp_json['stats']['fullCount'], 20) + self.assertTrue(len(resp_json['results']), 10) + + cursor_id = resp_json['cursor_id'] + resp_json = requests.post( API_URL + '/query_results', params={'cursor_id': cursor_id} ).json() - self.assertEqual(resp['count'], 20) - self.assertEqual(resp['stats']['fullCount'], 20) - self.assertEqual(resp['has_more'], False) - self.assertEqual(resp['cursor_id'], None) - self.assertTrue(len(resp['results']), 10) + self.assertEqual(resp_json['count'], 20) + self.assertEqual(resp_json['stats']['fullCount'], 20) + self.assertEqual(resp_json['has_more'], False) + self.assertEqual(resp_json['cursor_id'], None) + self.assertTrue(len(resp_json['results']), 10) + # Try to get the same cursor again - resp = requests.post( - API_URL + '/query_results', - params={'cursor_id': cursor_id} - ).json() - self.assertTrue(resp['error']) - self.assertEqual(resp['arango_message'], 'cursor not found') + self.test_request( + '/query_results', + method='post', + params={'cursor_id': cursor_id}, + status_code=400, + resp_json={ + 'error': { + 'message': 'ArangoDB server error.', + 'arango_message': 'cursor not found', + } + } + ) def test_query_no_name(self): """Test a query error with a stored query name that does not exist.""" @@ -609,7 +659,6 @@ def test_query_no_name(self): status_code=404, resp_json={ 'error': { - 'status': 404, 'message': 'Not found', 'details': f"Stored query '{name}' does not exist.", 'name': name, @@ -619,13 +668,21 @@ def test_query_no_name(self): def test_query_missing_bind_var(self): """Test a query error with a missing bind variable.""" - resp = requests.post( - API_URL + '/query_results', + + arango_msg = "AQL: bind parameter 'xyz' was not declared in the query (while parsing)" + self.test_request( + '/query_results', + method='post', params={'stored_query': 'list_test_vertices'}, - data=json.dumps({'xyz': 'test_vertex'}) - ).json() - self.assertEqual(resp['error'], 'ArangoDB server error.') - self.assertTrue(resp['arango_message']) + data=json.dumps({'xyz': 'test_vertex'}), + status_code=400, + resp_json={ + 'error': { + 'message': 'ArangoDB server error.', + 'arango_message': arango_msg, + } + } + ) def test_auth_query_with_access(self): """Test the case where we query a collection with specific workspace access.""" @@ -641,13 +698,13 @@ def test_auth_query_with_access(self): }), headers=HEADERS_ADMIN ) - resp = requests.post( + resp_json = requests.post( API_URL + '/query_results', params={'stored_query': 'list_test_vertices'}, headers={'Authorization': 'valid_token'} # see ./mock_workspace/endpoints.json ).json() - self.assertEqual(resp['count'], 1) - self.assertEqual(resp['results'][0]['ws_id'], ws_id) + self.assertEqual(resp_json['count'], 1) + self.assertEqual(resp_json['results'][0]['ws_id'], ws_id) def test_auth_query_no_access(self): """Test the case where we try to query a collection without the right workspace access.""" @@ -658,12 +715,12 @@ def test_auth_query_no_access(self): data='{"name": "requires_auth", "_key": "1", "ws_id": 9999}', headers=HEADERS_ADMIN ) - resp = requests.post( + resp_json = requests.post( API_URL + '/query_results', params={'stored_query': 'list_test_vertices'}, headers={'Authorization': 'valid_token'} # see ./mock_workspace/endpoints.json ).json() - self.assertEqual(resp['count'], 0) + self.assertEqual(resp_json['count'], 0) def test_query_cannot_pass_ws_ids(self): """Test that users cannot set the ws_ids param.""" @@ -674,13 +731,13 @@ def test_query_cannot_pass_ws_ids(self): data='{"name": "requires_auth", "_key": "1", "ws_id": 99}', headers=HEADERS_ADMIN ) - resp = requests.post( + resp_json = requests.post( API_URL + '/query_results', params={'view': 'list_test_vertices'}, data=json.dumps({'ws_ids': [ws_id]}), headers={'Authorization': 'valid_token'} ).json() - self.assertEqual(resp['count'], 0) + self.assertEqual(resp_json['count'], 0) def test_auth_query_invalid_token(self): """Test the case where we try to authorize a query using an invalid auth token.""" @@ -691,13 +748,30 @@ def test_auth_query_invalid_token(self): headers=HEADERS_ADMIN ) - resp = requests.post( - API_URL + '/query_results', + # see ./mock_workspace/list_workspace_ids_invalid.json for response + self.test_request( + '/query_results', params={'view': 'list_test_vertices'}, data=json.dumps({'ws_ids': [1]}), - headers={'Authorization': INVALID_TOKEN} + headers={'Authorization': INVALID_TOKEN}, + method='post', + status_code=403, + resp_json={ + 'error': { + 'message': 'Unauthorized', + 'auth_url': 'http://workspace:5000', + 'auth_response': json.dumps({ + "version": "1.1", + "error": { + "name": "JSONRPCError", + "code": -32400, + "message": "Token validation failed!", + "error": "..." + } + }) + } + } ) - self.assertEqual(resp.status_code, 403) def test_auth_adhoc_query(self): """Test that the 'ws_ids' bind-var is set for RE_ADMINs.""" @@ -710,12 +784,12 @@ def test_auth_adhoc_query(self): ) # This is the same query as list_test_vertices.aql in the spec query = 'for o in test_vertex filter o.is_public || o.ws_id IN ws_ids return o' - resp = requests.post( + resp_json = requests.post( API_URL + '/query_results', data=json.dumps({'query': query}), headers={'Authorization': ADMIN_TOKEN} # see ./mock_workspace/endpoints.json ).json() - self.assertEqual(resp['count'], 1) + self.assertEqual(resp_json['count'], 1) def test_save_docs_invalid(self): """Test that an invalid bulk save returns a 400 response""" diff --git a/relation_engine_server/test/test_json_validation.py b/relation_engine_server/test/test_json_validation.py index 649cba81..1a48fc1d 100644 --- a/relation_engine_server/test/test_json_validation.py +++ b/relation_engine_server/test/test_json_validation.py @@ -96,6 +96,12 @@ def test_non_validation_validator_errors(self): with self.assertRaisesRegex(ValueError, err_str): run_validator(schema={}, data=None, data_file=None) + # invalid file type + test_file = os_path.join(*(test_data_dirs + ['test_file.md'])) + err_msg = f'Unknown file type encountered: {test_file}' + with self.assertRaisesRegex(TypeError, err_msg): + run_validator(schema_file=test_file, data={}) + # invalid jsonpointer string - note the grammar error is from jsonpointer err_str = 'location must starts with /' json_loc = 'start validating here' diff --git a/relation_engine_server/test/test_spec_loader.py b/relation_engine_server/test/test_spec_loader.py index 8349b2db..d1da1df9 100644 --- a/relation_engine_server/test/test_spec_loader.py +++ b/relation_engine_server/test/test_spec_loader.py @@ -24,7 +24,8 @@ def test_get_names(self, schema_type_names=[], expected=[]): # this method should only be run from another test method if len(schema_type_names) == 0: - self.skipTest('No schema type names supplied. Skipping') + self.assertTrue(True) + return schema_type_singular = schema_type_names[0] schema_type_plural = schema_type_names[1] @@ -63,7 +64,8 @@ def test_run_spec_loading_tests(self, schema_type_names=[], test_name=None): # only run the test if it's being called from another test if test_name is None: - self.skipTest('No test name supplied') + self.assertTrue(True) + return print("running test_run_spec_loading_tests with schema_type " + schema_type_names[0]) method = getattr(spec_loader, 'get_' + schema_type_names[0]) @@ -127,9 +129,15 @@ def test_get_schemas_of_various_types(self): for schema in schema_type_list: self.test_run_spec_loading_tests(schema['schema_type_names'], schema['example']) - if schema['schema_type_names'][0] == 'collection': + if 'names' in schema: self.test_get_names(schema['schema_type_names'], schema['names']) + def test_non_existent_schema(self): + + err_msg = 'Reality does not exist' + with self.assertRaisesRegex(SchemaNonexistent, err_msg): + spec_loader.get_names('Reality') + def test_get_schema_for_doc(self): """test getting the schema for a specific document""" diff --git a/scripts/run_tests.sh b/scripts/run_tests.sh index 3d205ada..8f1f1ab9 100644 --- a/scripts/run_tests.sh +++ b/scripts/run_tests.sh @@ -5,6 +5,7 @@ set -e flake8 --max-complexity 15 /app mypy --ignore-missing-imports /app bandit -r /app +rm -rf /spec mkdir /spec mkdir /spec/repo cp -r /app/spec/* /spec/repo/ @@ -13,10 +14,12 @@ sh /app/scripts/start_server.sh & # spec validation python -m spec.validate && # spec stored query tests -python -m unittest discover spec/test && +coverage run --parallel-mode -m unittest discover spec/test && # importer tests -python -m unittest discover importers/test && +coverage run --parallel-mode -m unittest discover importers/test && # RE API tests -python -m unittest discover relation_engine_server/test && +coverage run --parallel-mode -m unittest discover relation_engine_server/test && # RE client tests -PYTHONPATH=client_src python -m unittest discover client_src/test +PYTHONPATH=client_src coverage run --parallel-mode -m unittest discover client_src/test +coverage combine +coverage html --omit=*/test_* diff --git a/spec/__init__.py b/spec/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/spec/stored_queries/djornl/djornl_fetch_clusters.yaml b/spec/stored_queries/djornl/djornl_fetch_clusters.yaml index 4aa6070e..498dc62b 100644 --- a/spec/stored_queries/djornl/djornl_fetch_clusters.yaml +++ b/spec/stored_queries/djornl/djornl_fetch_clusters.yaml @@ -8,8 +8,12 @@ params: type: array title: Cluster IDs description: Cluster IDs, in the form "clustering_system_name:cluster_id" - items: {type: string} + items: + type: string + format: regex + pattern: ^\w+:\d+$ examples: [['markov_i2:5', 'markov_i6:2'],['markov_i6:1']] + minItems: 1 distance: type: integer title: Traversal Distance diff --git a/spec/stored_queries/djornl/djornl_fetch_genes.yaml b/spec/stored_queries/djornl/djornl_fetch_genes.yaml index 42bbeeb5..6b8a8639 100644 --- a/spec/stored_queries/djornl/djornl_fetch_genes.yaml +++ b/spec/stored_queries/djornl/djornl_fetch_genes.yaml @@ -13,8 +13,10 @@ params: maximum: 100 keys: type: array - items: {type: string} + items: + type: string title: Gene Keys + minItems: 1 examples: [["AT1G01010"],["AT1G01020","AT1G01070"]] query: | LET node_ids = ( diff --git a/spec/stored_queries/djornl/djornl_fetch_phenotypes.yaml b/spec/stored_queries/djornl/djornl_fetch_phenotypes.yaml index 0e27ee4a..41482924 100644 --- a/spec/stored_queries/djornl/djornl_fetch_phenotypes.yaml +++ b/spec/stored_queries/djornl/djornl_fetch_phenotypes.yaml @@ -13,8 +13,10 @@ params: maximum: 100 keys: type: array - items: {type: string} + items: + type: string title: Phenotype Keys + minItems: 1 examples: [["As2"],["As2", "Na23"]] query: | LET node_ids = ( diff --git a/spec/test/djornl/results.json b/spec/test/djornl/results.json index fe9a613c..80c42a9c 100644 --- a/spec/test/djornl/results.json +++ b/spec/test/djornl/results.json @@ -56,297 +56,421 @@ {"_key": "SDV", "node_type": "pheno", "transcript": "", "gene_symbol": "", "gene_full_name": "", "gene_model_type": "", "tair_computational_description": "", "tair_curator_summary": "", "tair_short_description": "", "go_description": "", "go_terms": [], "mapman_bin": "", "mapman_name": "", "mapman_description": "", "pheno_aragwas_id": "10.21958/phenotype:104", "pheno_description": "Number of days following stratification to opening of first flower. The experiment was stopped at 200 d, and accessions that had not flowered at that point were assigned a value of 200", "pheno_pto_name": "days to flowering trait", "pheno_pto_description": "A flowering time trait (TO:0002616)which is the number of days required for an individual flower (PO:0009046), a whole plant (PO:0000003) or a plant population to reach flowering stage (PO:0007616) from a predetermined time point (e.g. the date of seed sowing, seedling transplant, or seedling emergence). [GR:pj, TO:cooperl]", "pheno_ref": "Atwell et. al, Nature 2010", "user_notes": ""} ] }, - "fetch_all": { - "nodes": [ - "As2", - "As75", - "AT1G01010", - "AT1G01020", - "AT1G01030", - "AT1G01040", - "AT1G01050", - "AT1G01060", - "AT1G01070", - "AT1G01080", - "AT1G01090", - "AT1G01100", - "Na23", - "SDV" + "queries": { + "djornl_fetch_phenotype": [ + { + "params": {"keys": ["A", "B", "C"]}, + "error": { + "details": "Stored query 'djornl_fetch_phenotype' does not exist.", + "message": "Not found", + "name": "djornl_fetch_phenotype" + } + } ], - "edges": [ - "As2__AT1G01020__AraGWAS-Phenotype_Associations__8.4", - "As2__AT1G01040__AraGWAS-Phenotype_Associations__5.4", - "As75__AT1G01020__AraGWAS-Phenotype_Associations__39.9", - "AT1G01010__AT1G01020__AraNetv2-HT_high-throughput-ppi__2.3", - "AT1G01010__AT1G01030__AraNetv2-HT_high-throughput-ppi__2.4", - "AT1G01010__AT1G01040__AraNetv2-DC_domain-co-occurrence__2.5", - "AT1G01010__AT1G01040__AraNetv2-LC_lit-curated-ppi__170.5", - "AT1G01030__AT1G01050__AraNetv2-CX_pairwise-gene-coexpression__2.6", - "AT1G01050__AT1G01060__AraNetv2-LC_lit-curated-ppi__2.7", - "AT1G01080__AT1G01090__AraNetv2-LC_lit-curated-ppi__2.8" - ] - }, - "fetch_genes": { - "keys": { - "Mary Poppins": { - "distance": { - "0": {"nodes": [], "edges": []}, - "1": {"nodes": [], "edges": []}, - "5": {"nodes": [], "edges": []} + "djornl_fetch_all": [ + { + "params": {}, + "results": { + "nodes": [ + "As2", + "As75", + "AT1G01010", + "AT1G01020", + "AT1G01030", + "AT1G01040", + "AT1G01050", + "AT1G01060", + "AT1G01070", + "AT1G01080", + "AT1G01090", + "AT1G01100", + "Na23", + "SDV" + ], + "edges": [ + "As2__AT1G01020__AraGWAS-Phenotype_Associations__8.4", + "As2__AT1G01040__AraGWAS-Phenotype_Associations__5.4", + "As75__AT1G01020__AraGWAS-Phenotype_Associations__39.9", + "AT1G01010__AT1G01020__AraNetv2-HT_high-throughput-ppi__2.3", + "AT1G01010__AT1G01030__AraNetv2-HT_high-throughput-ppi__2.4", + "AT1G01010__AT1G01040__AraNetv2-DC_domain-co-occurrence__2.5", + "AT1G01010__AT1G01040__AraNetv2-LC_lit-curated-ppi__170.5", + "AT1G01030__AT1G01050__AraNetv2-CX_pairwise-gene-coexpression__2.6", + "AT1G01050__AT1G01060__AraNetv2-LC_lit-curated-ppi__2.7", + "AT1G01080__AT1G01090__AraNetv2-LC_lit-curated-ppi__2.8" + ] + } + }, + { + "params": {"musical": "Mary Poppins"}, + "error": { + "message": "ArangoDB server error.", + "arango_message": "AQL: bind parameter 'musical' was not declared in the query (while parsing)" } + } + ], + "djornl_fetch_genes": [ + { + "params": {}, + "error": { + "failed_validator": "required", + "message": "'keys' is a required property", + "path": [], + "value": {"ws_ids": []} + } + }, + { + "params": {"keys": []}, + "error": { + "failed_validator": "minItems", + "message": "[] is too short", + "path": ["keys"], + "value": [] + } + }, + { + "params": { "keys": ["Mary Poppins"], "distance": 0 }, + "results": {"nodes": [], "edges": []} + }, + { + "params": { "keys": ["Mary Poppins"], "distance": 1 }, + "results": {"nodes": [], "edges": []} + }, + { + "params": { "keys": ["Mary Poppins"], "distance": 5 }, + "results": {"nodes": [], "edges": []} }, - "AT1G01010": { - "distance": { - "0": { - "nodes": ["AT1G01010"], - "edges": [] - }, - "1": { - "nodes": [ - "AT1G01010", - "AT1G01020", - "AT1G01030", - "AT1G01040" - ], - "edges": [ - "AT1G01010__AT1G01020__AraNetv2-HT_high-throughput-ppi__2.3", - "AT1G01010__AT1G01030__AraNetv2-HT_high-throughput-ppi__2.4", - "AT1G01010__AT1G01040__AraNetv2-DC_domain-co-occurrence__2.5", - "AT1G01010__AT1G01040__AraNetv2-LC_lit-curated-ppi__170.5" - ] - }, - "5": { - "nodes": ["As2", "As75", "AT1G01010", "AT1G01020", "AT1G01030", "AT1G01040", "AT1G01050", "AT1G01060"], - "edges": [ - "As2__AT1G01020__AraGWAS-Phenotype_Associations__8.4", - "As2__AT1G01040__AraGWAS-Phenotype_Associations__5.4", - "As75__AT1G01020__AraGWAS-Phenotype_Associations__39.9", - "AT1G01010__AT1G01020__AraNetv2-HT_high-throughput-ppi__2.3", - "AT1G01010__AT1G01030__AraNetv2-HT_high-throughput-ppi__2.4", - "AT1G01010__AT1G01040__AraNetv2-DC_domain-co-occurrence__2.5", - "AT1G01010__AT1G01040__AraNetv2-LC_lit-curated-ppi__170.5", - "AT1G01030__AT1G01050__AraNetv2-CX_pairwise-gene-coexpression__2.6", - "AT1G01050__AT1G01060__AraNetv2-LC_lit-curated-ppi__2.7" - ] - } + { + "params": { "keys": ["AT1G01010"], "distance": 0 }, + "results": { + "nodes": ["AT1G01010"], + "edges": [] } }, - "AT1G01020__AT1G01070": { - "distance": { - "0": { - "nodes": ["AT1G01020", "AT1G01070"], - "edges": [] - }, - "1": { - "nodes": ["As2", "As75", "AT1G01010", "AT1G01020", "AT1G01070"], - "edges": [ - "As2__AT1G01020__AraGWAS-Phenotype_Associations__8.4", - "As75__AT1G01020__AraGWAS-Phenotype_Associations__39.9", - "AT1G01010__AT1G01020__AraNetv2-HT_high-throughput-ppi__2.3" - ] - }, - "5": { - "nodes": ["As2", "As75", "AT1G01010", "AT1G01020", "AT1G01030", "AT1G01040", "AT1G01050", "AT1G01060", "AT1G01070"], - "edges": [ - "As2__AT1G01020__AraGWAS-Phenotype_Associations__8.4", - "As2__AT1G01040__AraGWAS-Phenotype_Associations__5.4", - "As75__AT1G01020__AraGWAS-Phenotype_Associations__39.9", - "AT1G01010__AT1G01020__AraNetv2-HT_high-throughput-ppi__2.3", - "AT1G01010__AT1G01030__AraNetv2-HT_high-throughput-ppi__2.4", - "AT1G01010__AT1G01040__AraNetv2-DC_domain-co-occurrence__2.5", - "AT1G01010__AT1G01040__AraNetv2-LC_lit-curated-ppi__170.5", - "AT1G01030__AT1G01050__AraNetv2-CX_pairwise-gene-coexpression__2.6", - "AT1G01050__AT1G01060__AraNetv2-LC_lit-curated-ppi__2.7" - ] - } + { + "params": { "keys": ["AT1G01010"], "distance": 1 }, + "results": { + "nodes": [ + "AT1G01010", + "AT1G01020", + "AT1G01030", + "AT1G01040" + ], + "edges": [ + "AT1G01010__AT1G01020__AraNetv2-HT_high-throughput-ppi__2.3", + "AT1G01010__AT1G01030__AraNetv2-HT_high-throughput-ppi__2.4", + "AT1G01010__AT1G01040__AraNetv2-DC_domain-co-occurrence__2.5", + "AT1G01010__AT1G01040__AraNetv2-LC_lit-curated-ppi__170.5" + ] + } + }, + { + "params": { "keys": ["AT1G01010"], "distance": 5 }, + "results": { + "nodes": ["As2", "As75", "AT1G01010", "AT1G01020", "AT1G01030", "AT1G01040", "AT1G01050", "AT1G01060"], + "edges": [ + "As2__AT1G01020__AraGWAS-Phenotype_Associations__8.4", + "As2__AT1G01040__AraGWAS-Phenotype_Associations__5.4", + "As75__AT1G01020__AraGWAS-Phenotype_Associations__39.9", + "AT1G01010__AT1G01020__AraNetv2-HT_high-throughput-ppi__2.3", + "AT1G01010__AT1G01030__AraNetv2-HT_high-throughput-ppi__2.4", + "AT1G01010__AT1G01040__AraNetv2-DC_domain-co-occurrence__2.5", + "AT1G01010__AT1G01040__AraNetv2-LC_lit-curated-ppi__170.5", + "AT1G01030__AT1G01050__AraNetv2-CX_pairwise-gene-coexpression__2.6", + "AT1G01050__AT1G01060__AraNetv2-LC_lit-curated-ppi__2.7" + ] + } + }, + { + "params": {"keys": ["AT1G01020", "AT1G01070"], "distance": 0 }, + "results": { + "nodes": ["AT1G01020", "AT1G01070"], + "edges": [] + } + }, + { + "params": {"keys": ["AT1G01020", "AT1G01070"], "distance": 1 }, + "results": { + "nodes": ["As2", "As75", "AT1G01010", "AT1G01020", "AT1G01070"], + "edges": [ + "As2__AT1G01020__AraGWAS-Phenotype_Associations__8.4", + "As75__AT1G01020__AraGWAS-Phenotype_Associations__39.9", + "AT1G01010__AT1G01020__AraNetv2-HT_high-throughput-ppi__2.3" + ] + } + }, + { + "params": {"keys": ["AT1G01020", "AT1G01070"], "distance": 5 }, + "results": { + "nodes": ["As2", "As75", "AT1G01010", "AT1G01020", "AT1G01030", "AT1G01040", "AT1G01050", "AT1G01060", "AT1G01070"], + "edges": [ + "As2__AT1G01020__AraGWAS-Phenotype_Associations__8.4", + "As2__AT1G01040__AraGWAS-Phenotype_Associations__5.4", + "As75__AT1G01020__AraGWAS-Phenotype_Associations__39.9", + "AT1G01010__AT1G01020__AraNetv2-HT_high-throughput-ppi__2.3", + "AT1G01010__AT1G01030__AraNetv2-HT_high-throughput-ppi__2.4", + "AT1G01010__AT1G01040__AraNetv2-DC_domain-co-occurrence__2.5", + "AT1G01010__AT1G01040__AraNetv2-LC_lit-curated-ppi__170.5", + "AT1G01030__AT1G01050__AraNetv2-CX_pairwise-gene-coexpression__2.6", + "AT1G01050__AT1G01060__AraNetv2-LC_lit-curated-ppi__2.7" + ] } } - } - }, - "fetch_phenotypes": { - "keys": { - "Mary Poppins": { - "distance": { - "0": {"nodes": [], "edges": []}, - "1": {"nodes": [], "edges": []}, - "5": {"nodes": [], "edges": []} + ], + "djornl_fetch_phenotypes": [ + { + "params": {"keys": "Mary Poppins"}, + "error": { + "failed_validator": "type", + "message": "'Mary Poppins' is not of type 'array'", + "path": ["keys"], + "value": "Mary Poppins" + } + }, + { + "params": {"keys": ["Mary Poppins"], "distance": 0}, + "results": {"nodes": [], "edges": []} + }, + { + "params": {"keys": ["Mary Poppins"], "distance": 1}, + "results": {"nodes": [], "edges": []} + }, + { + "params": {"keys": ["Mary Poppins"], "distance": 5}, + "results": {"nodes": [], "edges": []} + }, + { + "params": {"keys": ["As2"], "distance": 0}, + "results": { + "nodes": ["As2"], + "edges": [] } }, - "As2": { - "distance": { - "0": { - "nodes": ["As2"], - "edges": [] - }, - "1": { - "nodes": ["As2", "AT1G01020", "AT1G01040"], - "edges": [ - "As2__AT1G01020__AraGWAS-Phenotype_Associations__8.4", - "As2__AT1G01040__AraGWAS-Phenotype_Associations__5.4" - ] - }, - "5": { - "nodes": ["As2", "As75", "AT1G01010", "AT1G01020", "AT1G01030", "AT1G01040", "AT1G01050", "AT1G01060"], - "edges": [ - "As2__AT1G01020__AraGWAS-Phenotype_Associations__8.4", - "As2__AT1G01040__AraGWAS-Phenotype_Associations__5.4", - "As75__AT1G01020__AraGWAS-Phenotype_Associations__39.9", - "AT1G01010__AT1G01020__AraNetv2-HT_high-throughput-ppi__2.3", - "AT1G01010__AT1G01030__AraNetv2-HT_high-throughput-ppi__2.4", - "AT1G01010__AT1G01040__AraNetv2-DC_domain-co-occurrence__2.5", - "AT1G01010__AT1G01040__AraNetv2-LC_lit-curated-ppi__170.5", - "AT1G01030__AT1G01050__AraNetv2-CX_pairwise-gene-coexpression__2.6", - "AT1G01050__AT1G01060__AraNetv2-LC_lit-curated-ppi__2.7" - ] - } + { + "params": {"keys": ["As2"], "distance": 1}, + "results": { + "nodes": ["As2", "AT1G01020", "AT1G01040"], + "edges": [ + "As2__AT1G01020__AraGWAS-Phenotype_Associations__8.4", + "As2__AT1G01040__AraGWAS-Phenotype_Associations__5.4" + ] } }, - "As2__Na23": { - "distance": { - "0": { - "nodes": ["As2", "Na23"], - "edges": [] - }, - "1": { - "nodes": ["As2", "Na23", "AT1G01020", "AT1G01040"], - "edges": [ - "As2__AT1G01020__AraGWAS-Phenotype_Associations__8.4", - "As2__AT1G01040__AraGWAS-Phenotype_Associations__5.4" - ] - }, - "5": { - "nodes": ["As2", "As75", "AT1G01010", "AT1G01020", "AT1G01030", "AT1G01040", "AT1G01050", "AT1G01060", "Na23"], - "edges": [ - "As2__AT1G01020__AraGWAS-Phenotype_Associations__8.4", - "As2__AT1G01040__AraGWAS-Phenotype_Associations__5.4", - "As75__AT1G01020__AraGWAS-Phenotype_Associations__39.9", - "AT1G01010__AT1G01020__AraNetv2-HT_high-throughput-ppi__2.3", - "AT1G01010__AT1G01030__AraNetv2-HT_high-throughput-ppi__2.4", - "AT1G01010__AT1G01040__AraNetv2-DC_domain-co-occurrence__2.5", - "AT1G01010__AT1G01040__AraNetv2-LC_lit-curated-ppi__170.5", - "AT1G01030__AT1G01050__AraNetv2-CX_pairwise-gene-coexpression__2.6", - "AT1G01050__AT1G01060__AraNetv2-LC_lit-curated-ppi__2.7" - ] - } + { + "params": {"keys": ["As2"], "distance": 5}, + "results": { + "nodes": ["As2", "As75", "AT1G01010", "AT1G01020", "AT1G01030", "AT1G01040", "AT1G01050", "AT1G01060"], + "edges": [ + "As2__AT1G01020__AraGWAS-Phenotype_Associations__8.4", + "As2__AT1G01040__AraGWAS-Phenotype_Associations__5.4", + "As75__AT1G01020__AraGWAS-Phenotype_Associations__39.9", + "AT1G01010__AT1G01020__AraNetv2-HT_high-throughput-ppi__2.3", + "AT1G01010__AT1G01030__AraNetv2-HT_high-throughput-ppi__2.4", + "AT1G01010__AT1G01040__AraNetv2-DC_domain-co-occurrence__2.5", + "AT1G01010__AT1G01040__AraNetv2-LC_lit-curated-ppi__170.5", + "AT1G01030__AT1G01050__AraNetv2-CX_pairwise-gene-coexpression__2.6", + "AT1G01050__AT1G01060__AraNetv2-LC_lit-curated-ppi__2.7" + ] + } + }, + { + "params": {"keys": ["As2", "Na23"], "distance": 0}, + "results": { + "nodes": ["As2", "Na23"], + "edges": [] + } + }, + { + "params": {"keys": ["As2", "Na23"], "distance": 1}, + "results": { + "nodes": ["As2", "Na23", "AT1G01020", "AT1G01040"], + "edges": [ + "As2__AT1G01020__AraGWAS-Phenotype_Associations__8.4", + "As2__AT1G01040__AraGWAS-Phenotype_Associations__5.4" + ] + } + }, + { + "params": {"keys": ["As2", "Na23"], "distance": 5}, + "results": { + "nodes": ["As2", "As75", "AT1G01010", "AT1G01020", "AT1G01030", "AT1G01040", "AT1G01050", "AT1G01060", "Na23"], + "edges": [ + "As2__AT1G01020__AraGWAS-Phenotype_Associations__8.4", + "As2__AT1G01040__AraGWAS-Phenotype_Associations__5.4", + "As75__AT1G01020__AraGWAS-Phenotype_Associations__39.9", + "AT1G01010__AT1G01020__AraNetv2-HT_high-throughput-ppi__2.3", + "AT1G01010__AT1G01030__AraNetv2-HT_high-throughput-ppi__2.4", + "AT1G01010__AT1G01040__AraNetv2-DC_domain-co-occurrence__2.5", + "AT1G01010__AT1G01040__AraNetv2-LC_lit-curated-ppi__170.5", + "AT1G01030__AT1G01050__AraNetv2-CX_pairwise-gene-coexpression__2.6", + "AT1G01050__AT1G01060__AraNetv2-LC_lit-curated-ppi__2.7" + ] } } - } - }, - "search_nodes": { - "search_text": { - "Mary Poppins": { - "distance": { - "0": {"nodes": [], "edges": []}, - "1": {"nodes": [], "edges": []}, - "5": {"nodes": [], "edges": []} + ], + "djornl_search_nodes": [ + { + "params": {"search_text": "Mary Poppins", "distance": 0}, + "results": {"nodes": [], "edges": []} + }, + { + "params": {"search_text": "Mary Poppins", "distance": 1}, + "results": {"nodes": [], "edges": []} + }, + { + "params": {"search_text": "Mary Poppins", "distance": 5}, + "results": {"nodes": [], "edges": []} + }, + { + "params": {"search_text": "GO:0005515", "distance": 0}, + "results": { + "nodes": ["AT1G01040", "AT1G01090"], + "edges": [] + } + }, + { + "params": {"search_text": "GO:0005515", "distance": 1}, + "results": { + "nodes": ["As2", "AT1G01010", "AT1G01040", "AT1G01080", "AT1G01090"], + "edges": [ + "As2__AT1G01040__AraGWAS-Phenotype_Associations__5.4", + "AT1G01010__AT1G01040__AraNetv2-DC_domain-co-occurrence__2.5", + "AT1G01010__AT1G01040__AraNetv2-LC_lit-curated-ppi__170.5", + "AT1G01080__AT1G01090__AraNetv2-LC_lit-curated-ppi__2.8" + ] } }, - "GO:0005515": { - "distance": { - "0": { - "nodes": ["AT1G01040", "AT1G01090"], - "edges": [] - }, - "1": { - "nodes": ["As2", "AT1G01010", "AT1G01040", "AT1G01080", "AT1G01090"], - "edges": [ - "As2__AT1G01040__AraGWAS-Phenotype_Associations__5.4", - "AT1G01010__AT1G01040__AraNetv2-DC_domain-co-occurrence__2.5", - "AT1G01010__AT1G01040__AraNetv2-LC_lit-curated-ppi__170.5", - "AT1G01080__AT1G01090__AraNetv2-LC_lit-curated-ppi__2.8" - ] - }, - "5": { - "nodes": ["As2", "As75", "AT1G01010", "AT1G01020", "AT1G01030", "AT1G01040", "AT1G01050", "AT1G01060", "AT1G01080", "AT1G01090"], - "edges": [ - "As2__AT1G01020__AraGWAS-Phenotype_Associations__8.4", - "As2__AT1G01040__AraGWAS-Phenotype_Associations__5.4", - "As75__AT1G01020__AraGWAS-Phenotype_Associations__39.9", - "AT1G01010__AT1G01020__AraNetv2-HT_high-throughput-ppi__2.3", - "AT1G01010__AT1G01030__AraNetv2-HT_high-throughput-ppi__2.4", - "AT1G01010__AT1G01040__AraNetv2-DC_domain-co-occurrence__2.5", - "AT1G01010__AT1G01040__AraNetv2-LC_lit-curated-ppi__170.5", - "AT1G01030__AT1G01050__AraNetv2-CX_pairwise-gene-coexpression__2.6", - "AT1G01050__AT1G01060__AraNetv2-LC_lit-curated-ppi__2.7", - "AT1G01080__AT1G01090__AraNetv2-LC_lit-curated-ppi__2.8" - ] - } + { + "params": {"search_text": "GO:0005515", "distance": 5}, + "results": { + "nodes": ["As2", "As75", "AT1G01010", "AT1G01020", "AT1G01030", "AT1G01040", "AT1G01050", "AT1G01060", "AT1G01080", "AT1G01090"], + "edges": [ + "As2__AT1G01020__AraGWAS-Phenotype_Associations__8.4", + "As2__AT1G01040__AraGWAS-Phenotype_Associations__5.4", + "As75__AT1G01020__AraGWAS-Phenotype_Associations__39.9", + "AT1G01010__AT1G01020__AraNetv2-HT_high-throughput-ppi__2.3", + "AT1G01010__AT1G01030__AraNetv2-HT_high-throughput-ppi__2.4", + "AT1G01010__AT1G01040__AraNetv2-DC_domain-co-occurrence__2.5", + "AT1G01010__AT1G01040__AraNetv2-LC_lit-curated-ppi__170.5", + "AT1G01030__AT1G01050__AraNetv2-CX_pairwise-gene-coexpression__2.6", + "AT1G01050__AT1G01060__AraNetv2-LC_lit-curated-ppi__2.7", + "AT1G01080__AT1G01090__AraNetv2-LC_lit-curated-ppi__2.8" + ] } } - } - }, - - "fetch_clusters": { - "cluster_ids": { - "Mary Poppins": { - "distance": { - "0": {"nodes": [], "edges": []}, - "1": {"nodes": [], "edges": []}, - "5": {"nodes": [], "edges": []} + ], + "djornl_fetch_clusters": [ + { + "params": {"cluster_ids": "Mary Poppins"}, + "error": { + "failed_validator": "type", + "message": "'Mary Poppins' is not of type 'array'", + "path": ["cluster_ids"], + "value": "Mary Poppins" } }, - "markov_i6:1": { - "distance": { - "0": { - "nodes": ["AT1G01040", "AT1G01090"], - "edges": [] - }, - "1": { - "nodes": ["As2", "AT1G01010", "AT1G01040", "AT1G01080", "AT1G01090"], - "edges": [ - "As2__AT1G01040__AraGWAS-Phenotype_Associations__5.4", - "AT1G01010__AT1G01040__AraNetv2-DC_domain-co-occurrence__2.5", - "AT1G01010__AT1G01040__AraNetv2-LC_lit-curated-ppi__170.5", - "AT1G01080__AT1G01090__AraNetv2-LC_lit-curated-ppi__2.8" - ] - }, - "5": { - "nodes": ["As2", "As75", "AT1G01010", "AT1G01020", "AT1G01030", "AT1G01040", "AT1G01050", "AT1G01060", "AT1G01080", "AT1G01090"], - "edges": [ - "As2__AT1G01020__AraGWAS-Phenotype_Associations__8.4", - "As2__AT1G01040__AraGWAS-Phenotype_Associations__5.4", - "As75__AT1G01020__AraGWAS-Phenotype_Associations__39.9", - "AT1G01010__AT1G01020__AraNetv2-HT_high-throughput-ppi__2.3", - "AT1G01010__AT1G01030__AraNetv2-HT_high-throughput-ppi__2.4", - "AT1G01010__AT1G01040__AraNetv2-DC_domain-co-occurrence__2.5", - "AT1G01010__AT1G01040__AraNetv2-LC_lit-curated-ppi__170.5", - "AT1G01030__AT1G01050__AraNetv2-CX_pairwise-gene-coexpression__2.6", - "AT1G01050__AT1G01060__AraNetv2-LC_lit-curated-ppi__2.7", - "AT1G01080__AT1G01090__AraNetv2-LC_lit-curated-ppi__2.8" - ] - } + { + "params": {"cluster_ids": ["Mary Poppins"]}, + "error": { + "failed_validator": "pattern", + "message": "'Mary Poppins' does not match '^\\\\w+:\\\\d+$'", + "path": ["cluster_ids", 0], + "value": "Mary Poppins" } }, - "markov_i2:5__markov_i6:2": { - "distance": { - "0": { - "nodes": ["AT1G01020", "AT1G01070"], - "edges": [] - }, - "1": { - "nodes": ["As2", "As75", "AT1G01010", "AT1G01020", "AT1G01070"], - "edges": [ - "As2__AT1G01020__AraGWAS-Phenotype_Associations__8.4", - "As75__AT1G01020__AraGWAS-Phenotype_Associations__39.9", - "AT1G01010__AT1G01020__AraNetv2-HT_high-throughput-ppi__2.3" - ] - }, - "5": { - "nodes": ["As2", "As75", "AT1G01010", "AT1G01020", "AT1G01030", "AT1G01040", "AT1G01050", "AT1G01060", "AT1G01070"], - "edges": [ - "As2__AT1G01020__AraGWAS-Phenotype_Associations__8.4", - "As2__AT1G01040__AraGWAS-Phenotype_Associations__5.4", - "As75__AT1G01020__AraGWAS-Phenotype_Associations__39.9", - "AT1G01010__AT1G01020__AraNetv2-HT_high-throughput-ppi__2.3", - "AT1G01010__AT1G01030__AraNetv2-HT_high-throughput-ppi__2.4", - "AT1G01010__AT1G01040__AraNetv2-DC_domain-co-occurrence__2.5", - "AT1G01010__AT1G01040__AraNetv2-LC_lit-curated-ppi__170.5", - "AT1G01030__AT1G01050__AraNetv2-CX_pairwise-gene-coexpression__2.6", - "AT1G01050__AT1G01060__AraNetv2-LC_lit-curated-ppi__2.7" - ] - } + { + "params": {"cluster_ids": []}, + "error": { + "failed_validator": "minItems", + "message": "[] is too short", + "path": ["cluster_ids"], + "value": [] + } + }, + { + "params": {"cluster_ids": ["MaryPoppins:1"], "distance": 0}, + "results": {"nodes": [], "edges": []} + }, + { + "params": {"cluster_ids": ["MaryPoppins:1"], "distance": 1}, + "results": {"nodes": [], "edges": []} + }, + { + "params": {"cluster_ids": ["MaryPoppins:1"], "distance": 5}, + "results": {"nodes": [], "edges": []} + }, + { + "params": {"cluster_ids": ["markov_i6:1"], "distance": 0}, + "results": { + "nodes": ["AT1G01040", "AT1G01090"], + "edges": [] + } + }, + { + "params": {"cluster_ids": ["markov_i6:1"], "distance": 1}, + "results": { + "nodes": ["As2", "AT1G01010", "AT1G01040", "AT1G01080", "AT1G01090"], + "edges": [ + "As2__AT1G01040__AraGWAS-Phenotype_Associations__5.4", + "AT1G01010__AT1G01040__AraNetv2-DC_domain-co-occurrence__2.5", + "AT1G01010__AT1G01040__AraNetv2-LC_lit-curated-ppi__170.5", + "AT1G01080__AT1G01090__AraNetv2-LC_lit-curated-ppi__2.8" + ] + } + }, + { + "params": {"cluster_ids": ["markov_i6:1"], "distance": 5}, + "results": { + "nodes": ["As2", "As75", "AT1G01010", "AT1G01020", "AT1G01030", "AT1G01040", "AT1G01050", "AT1G01060", "AT1G01080", "AT1G01090"], + "edges": [ + "As2__AT1G01020__AraGWAS-Phenotype_Associations__8.4", + "As2__AT1G01040__AraGWAS-Phenotype_Associations__5.4", + "As75__AT1G01020__AraGWAS-Phenotype_Associations__39.9", + "AT1G01010__AT1G01020__AraNetv2-HT_high-throughput-ppi__2.3", + "AT1G01010__AT1G01030__AraNetv2-HT_high-throughput-ppi__2.4", + "AT1G01010__AT1G01040__AraNetv2-DC_domain-co-occurrence__2.5", + "AT1G01010__AT1G01040__AraNetv2-LC_lit-curated-ppi__170.5", + "AT1G01030__AT1G01050__AraNetv2-CX_pairwise-gene-coexpression__2.6", + "AT1G01050__AT1G01060__AraNetv2-LC_lit-curated-ppi__2.7", + "AT1G01080__AT1G01090__AraNetv2-LC_lit-curated-ppi__2.8" + ] + } + }, + { + "params": {"cluster_ids": ["markov_i2:5", "markov_i6:2"], "distance": 0}, + "results": { + "nodes": ["AT1G01020", "AT1G01070"], + "edges": [] + } + }, + { + "params": {"cluster_ids": ["markov_i2:5", "markov_i6:2"], "distance": 1}, + "results": { + "nodes": ["As2", "As75", "AT1G01010", "AT1G01020", "AT1G01070"], + "edges": [ + "As2__AT1G01020__AraGWAS-Phenotype_Associations__8.4", + "As75__AT1G01020__AraGWAS-Phenotype_Associations__39.9", + "AT1G01010__AT1G01020__AraNetv2-HT_high-throughput-ppi__2.3" + ] + } + }, + { + "params": {"cluster_ids": ["markov_i2:5", "markov_i6:2"], "distance": 5}, + "results": { + "nodes": ["As2", "As75", "AT1G01010", "AT1G01020", "AT1G01030", "AT1G01040", "AT1G01050", "AT1G01060", "AT1G01070"], + "edges": [ + "As2__AT1G01020__AraGWAS-Phenotype_Associations__8.4", + "As2__AT1G01040__AraGWAS-Phenotype_Associations__5.4", + "As75__AT1G01020__AraGWAS-Phenotype_Associations__39.9", + "AT1G01010__AT1G01020__AraNetv2-HT_high-throughput-ppi__2.3", + "AT1G01010__AT1G01030__AraNetv2-HT_high-throughput-ppi__2.4", + "AT1G01010__AT1G01040__AraNetv2-DC_domain-co-occurrence__2.5", + "AT1G01010__AT1G01040__AraNetv2-LC_lit-curated-ppi__170.5", + "AT1G01030__AT1G01050__AraNetv2-CX_pairwise-gene-coexpression__2.6", + "AT1G01050__AT1G01060__AraNetv2-LC_lit-curated-ppi__2.7" + ] } } - } + ] } } diff --git a/spec/test/sample_schemas/duplicate_names/ncbi/ncbi_taxon.yaml b/spec/test/sample_schemas/duplicate_names/ncbi/ncbi_taxon.yaml new file mode 100644 index 00000000..39c97168 --- /dev/null +++ b/spec/test/sample_schemas/duplicate_names/ncbi/ncbi_taxon.yaml @@ -0,0 +1,64 @@ +name: ncbi_taxon +type: vertex +delta: true + +indexes: + - type: fulltext + fields: [scientific_name] + - type: persistent + fields: [id, expired, created] + - type: persistent + fields: [expired, created, last_version] + +schema: + "$schema": http://json-schema.org/draft-07/schema# + type: object + description: Template for a vertex entry in the NCBI taxonomy tree. + required: [id, scientific_name, rank, strain] + properties: + id: + type: string + description: NCBI Taxon id (positive integer) + examples: ['1', '2053699'] + scientific_name: + type: string + title: Taxon name. + examples: ['Methylophilus methylotrophus', 'Bacteria', 'Firmicutes'] + aliases: + type: array + description: Aliases + examples: + - - category: authority + name: Borreliella burgdorferi (Johnson et al. 1984) Adeolu and Gupta 2015 + - category: genbank common name + name: Lyme disease spirochet + - category: synonym + name: Borrelia burgdorferi + - - category: common name + name: E. coli + - category: authority + name: '"Bacterium coli commune" Escherich 1885' + - category: synonym + name: Bacterium coli + items: + type: object + required: ['category', 'name'] + properties: + category: {type: string} + name: {type: string} + rank: + type: string + title: Taxonomic rank + examples: ["Domain", "Phylum", "no rank"] + strain: + type: boolean + title: Strain flag + description: Whether this node corresponds to a strain. Strains are considered to be nodes + that have a rank of "no rank" and whose parents' rank is either species or subspecies or + where the parent's strain flag is true. + ncbi_taxon_id: + type: integer + title: The NCBI taxon ID as a number + gencode: + type: integer + title: The numerc ID of the genetic code for this organism. diff --git a/spec/test/sample_schemas/duplicate_names/ncbi/test_vertex.yaml b/spec/test/sample_schemas/duplicate_names/ncbi/test_vertex.yaml new file mode 100644 index 00000000..b2d34668 --- /dev/null +++ b/spec/test/sample_schemas/duplicate_names/ncbi/test_vertex.yaml @@ -0,0 +1,11 @@ +name: test_vertex +type: vertex +schema: + "$schema": "http://json-schema.org/draft-07/schema#" + type: object + required: [_key] + description: An example vertex schema for testing + properties: + _key: {type: string} + is_public: {type: boolean} + ws_id: {type: integer} diff --git a/spec/test/sample_schemas/duplicate_names/test/test_edge.yaml b/spec/test/sample_schemas/duplicate_names/test/test_edge.yaml new file mode 100644 index 00000000..fab7ad6e --- /dev/null +++ b/spec/test/sample_schemas/duplicate_names/test/test_edge.yaml @@ -0,0 +1,10 @@ +name: test_edge +type: edge +schema: + "$schema": "http://json-schema.org/draft-07/schema#" + type: object + required: [_from, _to] + description: Example edge schema for testing. + properties: + _from: {type: string} + _to: {type: string} diff --git a/spec/test/sample_schemas/duplicate_names/test/test_vertex.yaml b/spec/test/sample_schemas/duplicate_names/test/test_vertex.yaml new file mode 100644 index 00000000..b2d34668 --- /dev/null +++ b/spec/test/sample_schemas/duplicate_names/test/test_vertex.yaml @@ -0,0 +1,11 @@ +name: test_vertex +type: vertex +schema: + "$schema": "http://json-schema.org/draft-07/schema#" + type: object + required: [_key] + description: An example vertex schema for testing + properties: + _key: {type: string} + is_public: {type: boolean} + ws_id: {type: integer} diff --git a/spec/test/stored_queries/test_djornl.py b/spec/test/stored_queries/test_djornl.py index fe14dd23..8c27e719 100644 --- a/spec/test/stored_queries/test_djornl.py +++ b/spec/test/stored_queries/test_djornl.py @@ -56,38 +56,72 @@ def setUpClass(cls): r = create_test_docs(node_name, cluster_data['nodes'], True) print_db_update(r, node_name) - def submit_query(self, query_name, query_data={}): - """submit a database query""" + def test_expected_results(self, query_name=None, test_data=None): - if _VERBOSE: - q_data_str = json.dumps(query_data) - print('query data string: ' + q_data_str) + # don't run the tests if they're being called automatically + if query_name is None: + self.assertTrue(True) + return + + # ensure we have either 'results' or 'error' in the test data + self.assertTrue('results' in test_data or 'error' in test_data) - return run_query(query_name, query_data) + params = {} + if 'params' in test_data: + params = test_data['params'] - def check_expected_results(self, description, response, expected): + response = run_query(query_name, params) if _VERBOSE: - print("Running test " + description) + print("Running query " + query_name) + if 'params' in test_data: + print({'params': params}) + + # expecting an error response + if 'error' in test_data: + if 'error' not in response: + print({'response': response}) + + self.assertIn('error', response) + self.assertEqual(response['error'], test_data['error']) + return response + + # expecting a valid response + if 'results' not in response: + print({'response': response}) + self.assertIn('results', response) results = response['results'][0] + self.assertEqual( set([n["_key"] for n in results['nodes']]), - set(expected['nodes']) + set(test_data['results']['nodes']) ) self.assertEqual( set([e["_key"] for e in results['edges']]), - set(expected['edges']) + set(test_data['results']['edges']) ) + return response - def test_fetch_all(self): + # indexing schema in results.json + # self.json_data['queries'][query_name] + # e.g. for fetch_clusters data: + # "djornl_fetch_clusters": { + # "params": { "cluster_ids": ["markov_i2:6", "markov_i4:3"], "distance": "1"}, + # "results": { + # "nodes": [ node IDs ], + # "edges": [ edge data ] + # } + # } + # nodes are represented as a list of node[_key] + # edges are objects with keys _to, _from, edge_type and score - response = self.submit_query('djornl_fetch_all') - self.check_expected_results( + def test_fetch_all(self): + '''Ensure that data returned by the fetch all query has all the information that we expect''' + response = self.test_expected_results( "djornl_fetch_all", - response, - self.json_data['fetch_all'] + self.json_data['queries']['djornl_fetch_all'][0] ) # ensure that all the cluster data is returned OK @@ -98,78 +132,10 @@ def test_fetch_all(self): {n['_key']: n['clusters'] for n in expected_node_data if 'clusters' in n}, ) - # indexing schema in results.json - # self.json_data[query_name][param_name][param_value]["distance"][distance_param] - # e.g. for fetch_clusters data: - # "fetch_clusters": { - # "cluster_ids": { - # "markov_i2:6__markov_i4:3": { - # "distance": { - # 1: { - # "nodes": [ node IDs ], - # "edges": [ edge data ], - # } - # } - # } - # } - # } - # if param_value is an array, join the array entities with "__" - # results are in the form {"nodes": [...], "edges": [...]} - # nodes are represented as a list of node[_key] - # edges are objects with keys _to, _from, edge_type and score + def test_queries(self): + '''Run parameterised queries and check for results or error messages''' - def test_fetch_phenotypes(self): - - for (fetch_args, key_data) in self.json_data['fetch_phenotypes']['keys'].items(): - for (distance, distance_data) in key_data['distance'].items(): - resp = self.submit_query('djornl_fetch_phenotypes', { - "keys": fetch_args.split('__'), - "distance": int(distance), - }) - self.check_expected_results( - "fetch phenotypes with args " + fetch_args + " and distance " + distance, - resp, - distance_data - ) - - def test_fetch_genes(self): - - for (fetch_args, key_data) in self.json_data['fetch_genes']['keys'].items(): - for (distance, distance_data) in key_data['distance'].items(): - resp = self.submit_query('djornl_fetch_genes', { - "keys": fetch_args.split('__'), - "distance": int(distance), - }) - self.check_expected_results( - "fetch genes with args " + fetch_args + " and distance " + distance, - resp, - distance_data - ) - - def test_fetch_clusters(self): - - for (fetch_args, cluster_data) in self.json_data['fetch_clusters']['cluster_ids'].items(): - for (distance, distance_data) in cluster_data['distance'].items(): - resp = self.submit_query('djornl_fetch_clusters', { - "cluster_ids": fetch_args.split('__'), - "distance": int(distance), - }) - self.check_expected_results( - "fetch clusters with args " + fetch_args + " and distance " + distance, - resp, - distance_data - ) - - def test_search_nodes(self): - - for (search_text, search_data) in self.json_data['search_nodes']['search_text'].items(): - for (distance, distance_data) in search_data['distance'].items(): - resp = self.submit_query('djornl_search_nodes', { - "search_text": search_text, - "distance": int(distance), - }) - self.check_expected_results( - "search nodes with args " + search_text + " and distance " + distance, - resp, - distance_data - ) + for query in self.json_data['queries'].keys(): + for test in self.json_data['queries'][query]: + with self.subTest(query=query, params=test['params']): + self.test_expected_results(query, test) diff --git a/spec/test/stored_queries/test_ncbi_tax.py b/spec/test/stored_queries/test_ncbi_tax.py index 7b6817e0..6e3a0435 100644 --- a/spec/test/stored_queries/test_ncbi_tax.py +++ b/spec/test/stored_queries/test_ncbi_tax.py @@ -187,7 +187,7 @@ def test_search_sciname_wrong_type(self): data=json.dumps({'ts': _NOW, 'search_text': 123}) ) self.assertEqual(resp.status_code, 400) - self.assertEqual(resp.json()['error'], "123 is not of type 'string'") + self.assertEqual(resp.json()['error']['message'], "123 is not of type 'string'") def test_search_sciname_missing_search(self): """Test a query to search sciname with the search_text param missing.""" @@ -197,7 +197,7 @@ def test_search_sciname_missing_search(self): data=json.dumps({'ts': _NOW}) ) self.assertEqual(resp.status_code, 400) - self.assertEqual(resp.json()['error'], "'search_text' is a required property") + self.assertEqual(resp.json()['error']['message'], "'search_text' is a required property") def test_search_sciname_more_complicated(self): """Test a query to search sciname with some more keyword options.""" @@ -222,7 +222,7 @@ def test_search_sciname_offset_max(self): data=json.dumps({'ts': _NOW, 'search_text': "prefix:bact", "offset": 100001}) ) self.assertEqual(resp.status_code, 400) - self.assertEqual(resp.json()['error'], "100001 is greater than the maximum of 100000") + self.assertEqual(resp.json()['error']['message'], "100001 is greater than the maximum of 100000") def test_search_sciname_limit_max(self): """Test a query to search sciname with an invalid offset (greater than max).""" @@ -232,7 +232,7 @@ def test_search_sciname_limit_max(self): data=json.dumps({'ts': _NOW, 'search_text': "prefix:bact", "limit": 1001}) ) self.assertEqual(resp.status_code, 400) - self.assertEqual(resp.json()['error'], "1001 is greater than the maximum of 1000") + self.assertEqual(resp.json()['error']['message'], "1001 is greater than the maximum of 1000") def test_search_sciname_limit_ranks_implicit_defaults(self): """ Test queries where the results are limited by the rank or strain flag. """ @@ -381,14 +381,14 @@ def test_fetch_taxon_by_sciname_failures(self): params={'stored_query': 'ncbi_fetch_taxon_by_sciname'}, data=json.dumps({'ts': _NOW}) ).json() - self.assertEqual(resp['error'], "'sciname' is a required property") + self.assertEqual(resp['error']['message'], "'sciname' is a required property") # No ts resp = requests.post( _CONF['re_api_url'] + '/api/v1/query_results', params={'stored_query': 'ncbi_fetch_taxon_by_sciname'}, data=json.dumps({'sciname': 'Deltaproteobacteria'}) ).json() - self.assertEqual(resp['error'], "'ts' is a required property") + self.assertEqual(resp['error']['message'], "'ts' is a required property") # sciname not found resp = requests.post( _CONF['re_api_url'] + '/api/v1/query_results', diff --git a/spec/test/stored_queries/test_taxonomy.py b/spec/test/stored_queries/test_taxonomy.py index e317d205..a6075548 100644 --- a/spec/test/stored_queries/test_taxonomy.py +++ b/spec/test/stored_queries/test_taxonomy.py @@ -271,7 +271,7 @@ def test_search_sciname_wrong_type(self): }) ) self.assertEqual(resp.status_code, 400) - self.assertEqual(resp.json()['error'], "123 is not of type 'string'") + self.assertEqual(resp.json()['error']['message'], "123 is not of type 'string'") def test_search_sciname_missing_search(self): """Test a query to search sciname with the search_text param missing.""" @@ -281,7 +281,7 @@ def test_search_sciname_missing_search(self): data=json.dumps({'ts': _NOW, '@taxon_coll': 'ncbi_taxon'}) ) self.assertEqual(resp.status_code, 400) - self.assertEqual(resp.json()['error'], "'search_text' is a required property") + self.assertEqual(resp.json()['error']['message'], "'search_text' is a required property") def test_search_sciname_more_complicated(self): """Test a query to search sciname with some more keyword options.""" @@ -314,7 +314,7 @@ def test_search_sciname_offset_max(self): }) ) self.assertEqual(resp.status_code, 400) - self.assertEqual(resp.json()['error'], "100001 is greater than the maximum of 100000") + self.assertEqual(resp.json()['error']['message'], "100001 is greater than the maximum of 100000") def test_search_sciname_limit_max(self): """Test a query to search sciname with an invalid offset (greater than max).""" @@ -330,7 +330,7 @@ def test_search_sciname_limit_max(self): }) ) self.assertEqual(resp.status_code, 400) - self.assertEqual(resp.json()['error'], "1001 is greater than the maximum of 1000") + self.assertEqual(resp.json()['error']['message'], "1001 is greater than the maximum of 1000") def test_search_sciname_limit_ranks_implicit_defaults(self): """ Test queries where the results are limited by the rank or strain flag. """ @@ -495,7 +495,7 @@ def test_fetch_taxon_by_sciname_failures(self): params={'stored_query': 'taxonomy_fetch_taxon_by_sciname'}, data=json.dumps({'ts': _NOW, 'sciname_field': 'scientific_name', '@taxon_coll': 'ncbi_taxon'}) ).json() - self.assertEqual(resp['error'], "'sciname' is a required property") + self.assertEqual(resp['error']['message'], "'sciname' is a required property") # No ts resp = requests.post( _CONF['re_api_url'] + '/api/v1/query_results', @@ -506,7 +506,7 @@ def test_fetch_taxon_by_sciname_failures(self): '@taxon_coll': 'ncbi_taxon' }) ).json() - self.assertEqual(resp['error'], "'ts' is a required property") + self.assertEqual(resp['error']['message'], "'ts' is a required property") # sciname not found resp = requests.post( _CONF['re_api_url'] + '/api/v1/query_results', diff --git a/spec/test/test_validate.py b/spec/test/test_validate.py index 1577147c..e104a6c5 100644 --- a/spec/test/test_validate.py +++ b/spec/test/test_validate.py @@ -13,7 +13,8 @@ validate_stored_query, validate_data_source, validate_view, - validate_all + validate_all, + validate_all_by_type, ) _TEST_DIR = '/app/spec/test/sample_schemas' @@ -28,7 +29,7 @@ def setUpClass(cls): def test_validate_schema(self): """Validate a single file using the generic validate_schema method""" - err_msg = 'No validation schema found for made-up_schema' + err_msg = "No validation schema found for 'made-up_schema'" with self.assertRaisesRegex(ValueError, err_msg): validate_schema('/path/to/file', 'made-up_schema') @@ -176,6 +177,16 @@ def test_validate_view(self): def test_validate_all(self): """test all the files in a directory""" + with self.assertRaisesRegex(ValueError, "No validation schema found for 'muffins'"): + validate_all('muffins') + + def validate_all_duplicate_names(self): + with self.assertRaisesRegex(ValidationError, "duplicate_names failed validation"): + validate_all('collection', os_path.join(_TEST_DIR, 'duplicate_names')) + + stdout = capture_stdout(validate_all_duplicate_names, self) + self.assertRegex(stdout, "Duplicate queries named 'test_vertex'") + sample_schemas = { 'collection': 'collections', 'stored_query': 'stored_queries', @@ -184,10 +195,20 @@ def test_validate_all(self): } for (schema_type, directory) in sample_schemas.items(): - # n.b. this assumes all the schemas in /spec are valid! stdout = capture_stdout(validate_all, schema_type) self.assertRegex(stdout, r'...all valid') with self.assertRaises(Exception): validate_all(schema_type, os_path.join(_TEST_DIR, directory)) + + def test_validate_all_by_type(self): + """test all files of all types from a root directory""" + + # use value from config + n_errors = validate_all_by_type() + self.assertEqual(n_errors, 0) + + # known dodgy dir + n_errors = validate_all_by_type(_TEST_DIR) + self.assertGreater(n_errors, 0) diff --git a/spec/validate.py b/spec/validate.py index dec6b6dc..286cdb58 100644 --- a/spec/validate.py +++ b/spec/validate.py @@ -36,12 +36,20 @@ def validate_all(schema_type, directory=None): - """Validate the syntax of all schemas of a certain type.""" - print(f'Validating {schema_type} schemas...') - + """ + Validate the syntax of all schemas of type schema_type in a specified directory + + :param schema_type: (string) the schema type to validate + :param directory: (string) the directory to look in. + If not specified, the default directory for the schema_type + will be used. + """ if schema_type not in _VALID_SCHEMA_TYPES.keys(): - raise ValueError('No validation schema found for ' + schema_type) + raise ValueError(f"No validation schema found for '{schema_type}'") + print(f'Validating {schema_type} schemas...') + + err_count = 0 names = set() # type: set if directory is None: type_dir_name = _VALID_SCHEMA_TYPES[schema_type]['plural'] @@ -49,23 +57,71 @@ def validate_all(schema_type, directory=None): for path in glob.iglob(os.path.join(directory, '**', '*.*'), recursive=True): if path.endswith('.yaml') or path.endswith('.json'): - data = validate_schema(path, schema_type) + try: + data = validate_schema(path, schema_type) + # Check for any duplicate schema names + name = data['name'] + if name in names: + raise ValueError(f"Duplicate queries named '{name}'") + else: + names.add(name) + + except Exception as err: + print(f"✕ {path} failed validation") + print(err) + err_count += 1 + + if err_count: + raise ValidationError(f'{directory} failed validation') + + # all's well + print('...all valid.') + return + + +def validate_all_by_type(validation_base_dir=None): + """ + Validate the syntax of all schemas of all types in validation_base_dir + + Assumes that the schemas will be set up in parent directories named with the plural form + of the schema type name, i.e. all collection schemas in the 'collections' dir, all views + in the 'views' dir, etc. + + :param validation_base_dir: (string) the directory to look in. + If not specified, the default directory from the config + will be used - # Check for any duplicate schema names - name = data['name'] - if name in names: - raise ValueError(f'Duplicate queries named {name}') + :return n_errors: (int) the number of errors encountered + + """ + + n_errors = 0 + for schema_type in sorted(_VALID_SCHEMA_TYPES.keys()): + try: + if validation_base_dir is None: + validate_all(schema_type) else: - names.add(name) + directory = os.path.join( + validation_base_dir, + _VALID_SCHEMA_TYPES[schema_type]['plural'] + ) + validate_all(schema_type, directory) + except Exception as err: + print(err) + n_errors += 1 + print("\n") - print('...all valid.') + if n_errors > 0: + print('Validation failed!\n') + + return n_errors def validate_schema(path, schema_type): """Validate a single file against its schema""" if schema_type not in _VALID_SCHEMA_TYPES.keys(): - raise ValueError('No validation schema found for ' + schema_type) + raise ValueError(f"No validation schema found for '{schema_type}'") return globals()["validate_" + schema_type](path) @@ -184,16 +240,13 @@ def validate_aql_on_arango(data): + f" Extra params in schema: {params - query_bind_vars}") -def _fatal(msg): - """Fatal error.""" - sys.stderr.write(str(msg) + '\n') - sys.exit(1) +if __name__ == '__main__': + validation_base_dir = None + if len(sys.argv) > 1: + validation_base_dir = sys.argv[1] -if __name__ == '__main__': wait_for_arangodb() - try: - for s in ['data_source', 'stored_query', 'view', 'collection']: - validate_all(s) - except Exception as err: - _fatal(err) + n_errors = validate_all_by_type(validation_base_dir) + exit_code = 0 if not n_errors else 1 + sys.exit(exit_code)