diff --git a/app/accept_invite/rest.py b/app/accept_invite/rest.py index e294465f5..2c9955afb 100644 --- a/app/accept_invite/rest.py +++ b/app/accept_invite/rest.py @@ -34,8 +34,4 @@ def get_invited_user_by_token(token): invited_user = get_invited_user_by_id(invited_user_id) - if not invited_user: - message = 'Invited user not found with id: {}'.format(invited_user_id) - return jsonify(result='error', message=message), 404 - return jsonify(data=invited_user_schema.dump(invited_user).data), 200 diff --git a/app/dao/invited_user_dao.py b/app/dao/invited_user_dao.py index 62fb0d25d..1e9ccd22d 100644 --- a/app/dao/invited_user_dao.py +++ b/app/dao/invited_user_dao.py @@ -10,11 +10,11 @@ def save_invited_user(invited_user): def get_invited_user(service_id, invited_user_id): - return InvitedUser.query.filter_by(service_id=service_id, id=invited_user_id).first() + return InvitedUser.query.filter_by(service_id=service_id, id=invited_user_id).one() def get_invited_user_by_id(invited_user_id): - return InvitedUser.query.filter_by(id=invited_user_id).first() + return InvitedUser.query.filter_by(id=invited_user_id).one() def get_invited_users_for_service(service_id): diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index c43d82b79..7e9408c94 100644 --- a/app/dao/jobs_dao.py +++ b/app/dao/jobs_dao.py @@ -11,7 +11,7 @@ def dao_get_jobs_by_service_id(service_id): def dao_get_job_by_id(job_id): - return Job.query.filter_by(id=job_id).first() + return Job.query.filter_by(id=job_id).one() def dao_create_job(job): diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 74f533b4c..9c66e30bc 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -16,7 +16,7 @@ def dao_fetch_all_services_by_user(user_id): def dao_fetch_service_by_id_and_user(service_id, user_id): - return Service.query.filter(Service.users.any(id=user_id)).filter_by(id=service_id).first() + return Service.query.filter(Service.users.any(id=user_id)).filter_by(id=service_id).one() def dao_create_service(service, user): diff --git a/app/dao/templates_dao.py b/app/dao/templates_dao.py index 09390d175..fe56972c8 100644 --- a/app/dao/templates_dao.py +++ b/app/dao/templates_dao.py @@ -18,7 +18,7 @@ def dao_get_template_by_id_and_service_id(template_id, service_id): def dao_get_template_by_id(template_id): - return Template.query.filter_by(id=template_id).first() + return Template.query.filter_by(id=template_id).one() def dao_get_all_templates_for_service(service_id): diff --git a/app/invite/rest.py b/app/invite/rest.py index d2618abe7..ceb24ba97 100644 --- a/app/invite/rest.py +++ b/app/invite/rest.py @@ -43,16 +43,13 @@ def get_invited_users_by_service(service_id): @invite.route('/', methods=['GET']) def get_invited_user_by_service_and_id(service_id, invited_user_id): invited_user = get_invited_user(service_id=service_id, invited_user_id=invited_user_id) - if not invited_user: - return _invited_user_not_found(service_id, invited_user_id) + return jsonify(data=invited_user_schema.dump(invited_user).data), 200 @invite.route('/', methods=['POST']) def update_invited_user(service_id, invited_user_id): fetched = get_invited_user(service_id=service_id, invited_user_id=invited_user_id) - if not fetched: - return _invited_user_not_found(service_id=service_id, invited_user_id=invited_user_id) current_data = dict(invited_user_schema.dump(fetched).data.items()) current_data.update(request.get_json()) @@ -63,12 +60,6 @@ def update_invited_user(service_id, invited_user_id): return jsonify(data=invited_user_schema.dump(fetched).data), 200 -def _invited_user_not_found(service_id, invited_user_id): - message = 'Invited user not found for service id: {} and invited user id: {}'.format(service_id, - invited_user_id) - return jsonify(result='error', message=message), 404 - - def _create_invitation(invited_user): from utils.url_safe_token import generate_token token = generate_token(str(invited_user.id), current_app.config['SECRET_KEY'], current_app.config['DANGEROUS_SALT']) diff --git a/app/job/rest.py b/app/job/rest.py index c7d38e8ce..78db23716 100644 --- a/app/job/rest.py +++ b/app/job/rest.py @@ -29,8 +29,6 @@ register_errors(job) @job.route('/', methods=['GET']) def get_job_by_service_and_job_id(service_id, job_id): job = dao_get_job_by_service_id_and_job_id(service_id, job_id) - if not job: - return jsonify(result="error", message="Job {} not found for service {}".format(job_id, service_id)), 404 data, errors = job_schema.dump(job) return jsonify(data=data) @@ -44,7 +42,6 @@ def get_jobs_by_service(service_id): @job.route('', methods=['POST']) def create_job(service_id): - dao_fetch_service_by_id(service_id) data = request.get_json() diff --git a/app/permission/rest.py b/app/permission/rest.py index ac10063d6..89266b88b 100644 --- a/app/permission/rest.py +++ b/app/permission/rest.py @@ -19,9 +19,7 @@ def get_permissions(): @permission.route('/', methods=['GET']) def get_permission(permission_id): - inst = permission_dao.get_query(filter_by_dict={'id': permission_id}).first() - if not inst: - abort(404, 'Permission not found for id: {permission_id}'.format(permission_id)) + inst = permission_dao.get_query(filter_by_dict={'id': permission_id}).one() data, errors = permission_schema.dump(inst) if errors: abort(500, errors) diff --git a/app/service/rest.py b/app/service/rest.py index cf5c440b7..0bead75a2 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -57,11 +57,7 @@ def get_service_by_id(service_id): fetched = dao_fetch_service_by_id_and_user(service_id, user_id) else: fetched = dao_fetch_service_by_id(service_id) - if not fetched: - message_with_user_id = 'and for user id: {}'.format(user_id) if user_id else '' - return jsonify(result="error", - message="Service not found for service id: {0} {1}".format(service_id, - message_with_user_id)), 404 + data, errors = service_schema.dump(fetched) return jsonify(data=data) @@ -72,10 +68,7 @@ def create_service(): if not data.get('user_id', None): return jsonify(result="error", message={'user_id': ['Missing data for required field.']}), 400 - try: - user = get_model_users(data['user_id']) - except NoResultFound: - return jsonify(result="error", message={'user_id': ['not found']}), 400 + user = get_model_users(data['user_id']) data.pop('user_id', None) if 'name' in data: @@ -93,8 +86,6 @@ def create_service(): @service.route('/', methods=['POST']) def update_service(service_id): fetched_service = dao_fetch_service_by_id(service_id) - if not fetched_service: - return _service_not_found(service_id) current_data = dict(service_schema.dump(fetched_service).data.items()) current_data.update(request.get_json()) @@ -108,8 +99,6 @@ def update_service(service_id): @service.route('//api-key', methods=['POST']) def renew_api_key(service_id=None): fetched_service = dao_fetch_service_by_id(service_id=service_id) - if not fetched_service: - return _service_not_found(service_id) # create a new one # TODO: what validation should be done here? @@ -132,9 +121,8 @@ def revoke_api_key(service_id, api_key_id): @service.route('//api-keys', methods=['GET']) @service.route('//api-keys/', methods=['GET']) def get_api_keys(service_id, key_id=None): - fetched_service = dao_fetch_service_by_id(service_id=service_id) - if not fetched_service: - return _service_not_found(service_id) + dao_fetch_service_by_id(service_id=service_id) + try: if key_id: api_keys = [get_model_api_keys(service_id=service_id, id=key_id)] @@ -149,11 +137,6 @@ def get_api_keys(service_id, key_id=None): @service.route('//users', methods=['GET']) def get_users_for_service(service_id): fetched = dao_fetch_service_by_id(service_id) - if not fetched: - return _service_not_found(service_id) - # TODO why is this code here, the same functionality exists without it? - if not fetched.users: - return jsonify(data=[]) result = user_schema.dump(fetched.users, many=True) return jsonify(data=result.data) @@ -162,14 +145,8 @@ def get_users_for_service(service_id): @service.route('//users/', methods=['POST']) def add_user_to_service(service_id, user_id): service = dao_fetch_service_by_id(service_id) - if not service: - return _service_not_found(service_id) user = get_model_users(user_id=user_id) - if not user: - return jsonify(result='error', - message='User not found for id: {}'.format(user_id)), 404 - if user in service.users: return jsonify(result='error', message='User id: {} already part of service id: {}'.format(user_id, service_id)), 400 @@ -182,10 +159,6 @@ def add_user_to_service(service_id, user_id): return jsonify(data=data), 201 -def _service_not_found(service_id): - return jsonify(result='error', message='Service not found for id: {}'.format(service_id)), 404 - - def _process_permissions(user, service, permission_groups): from app.permissions_utils import get_permissions_by_group from app.dao.permissions_dao import permission_dao diff --git a/app/user/rest.py b/app/user/rest.py index a98cfa8ef..cb574a909 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -156,7 +156,7 @@ def get_user(user_id=None): return jsonify(data=result.data) -@user.route('//service//permission', methods=['POST']) +@user.route('//service//permission', methods=['POST']) def set_permissions(user_id, service_id): # TODO fix security hole, how do we verify that the user # who is making this request has permission to make the request. diff --git a/tests/app/dao/test_invited_user_dao.py b/tests/app/dao/test_invited_user_dao.py index 0a3772ed7..d70632c6b 100644 --- a/tests/app/dao/test_invited_user_dao.py +++ b/tests/app/dao/test_invited_user_dao.py @@ -1,5 +1,9 @@ from datetime import datetime, timedelta import uuid + +import pytest +from sqlalchemy.orm.exc import NoResultFound + from app import db from app.models import InvitedUser @@ -50,8 +54,9 @@ def test_get_invited_user_by_id(notify_db, notify_db_session, sample_invited_use def test_get_unknown_invited_user_returns_none(notify_db, notify_db_session, sample_service): unknown_id = uuid.uuid4() - unknown = get_invited_user(sample_service.id, unknown_id) - assert unknown is None + with pytest.raises(NoResultFound) as e: + get_invited_user(sample_service.id, unknown_id) + assert 'No row was found for one()' in str(e.value) def test_get_invited_users_for_service(notify_db, notify_db_session, sample_service): diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 8ca09cdd0..abc946bc4 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -171,4 +171,6 @@ def test_cannot_get_service_by_id_and_owned_by_different_user(service_factory, s save_model_user(new_user) service2 = service_factory.get('service 2', new_user) assert dao_fetch_service_by_id_and_user(service1.id, sample_user.id).name == 'service 1' - assert not dao_fetch_service_by_id_and_user(service2.id, sample_user.id) + with pytest.raises(NoResultFound) as e: + dao_fetch_service_by_id_and_user(service2.id, sample_user.id) + assert 'No row was found for one()' in str(e) diff --git a/tests/app/invite/test_invite_rest.py b/tests/app/invite/test_invite_rest.py index 2731efc82..78c6223d4 100644 --- a/tests/app/invite/test_invite_rest.py +++ b/tests/app/invite/test_invite_rest.py @@ -233,8 +233,7 @@ def test_update_invited_user_for_wrong_service_returns_404(notify_api, sample_in headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 404 json_response = json.loads(response.get_data(as_text=True))['message'] - assert json_response == 'Invited user not found for service id: {} and invited user id: {}'\ - .format(bad_service_id, sample_invited_user.id) + assert json_response == 'No result found' def test_update_invited_user_for_invalid_data_returns_400(notify_api, sample_invited_user): diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 73d61bcd6..cc62b0e70 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -166,8 +166,7 @@ def test_get_service_by_id_should_404_if_no_service_for_user(notify_api, sample_ assert resp.status_code == 404 json_resp = json.loads(resp.get_data(as_text=True)) assert json_resp['result'] == 'error' - assert json_resp['message'] == \ - 'Service not found for service id: {0} and for user id: {1}'.format(service_id, sample_user.id) + assert json_resp['message'] == 'No result found' def test_create_service(notify_api, sample_user): @@ -257,9 +256,9 @@ def test_should_not_create_service_with_missing_if_user_id_is_not_in_database(no data=json.dumps(data), headers=headers) json_resp = json.loads(resp.get_data(as_text=True)) - assert resp.status_code == 400 + assert resp.status_code == 404 assert json_resp['result'] == 'error' - assert 'not found' in json_resp['message']['user_id'] + assert 'No result found' == json_resp['message'] def test_should_not_create_service_if_missing_data(notify_api, sample_user):