Replaced first queries with one queries, which throws a NoResultFound.

Able to remove many of the None checks as a result of this.
Fixed the tests were needed.
This commit is contained in:
Rebecca Law
2016-03-11 15:34:20 +00:00
parent ef5969be77
commit 123b0ead3a
14 changed files with 26 additions and 66 deletions

View File

@@ -34,8 +34,4 @@ def get_invited_user_by_token(token):
invited_user = get_invited_user_by_id(invited_user_id) 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 return jsonify(data=invited_user_schema.dump(invited_user).data), 200

View File

@@ -10,11 +10,11 @@ def save_invited_user(invited_user):
def get_invited_user(service_id, invited_user_id): 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): 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): def get_invited_users_for_service(service_id):

View File

@@ -11,7 +11,7 @@ def dao_get_jobs_by_service_id(service_id):
def dao_get_job_by_id(job_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): def dao_create_job(job):

View File

@@ -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): 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): def dao_create_service(service, user):

View File

@@ -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): 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): def dao_get_all_templates_for_service(service_id):

View File

@@ -43,16 +43,13 @@ def get_invited_users_by_service(service_id):
@invite.route('/<invited_user_id>', methods=['GET']) @invite.route('/<invited_user_id>', methods=['GET'])
def get_invited_user_by_service_and_id(service_id, invited_user_id): 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) 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 return jsonify(data=invited_user_schema.dump(invited_user).data), 200
@invite.route('/<invited_user_id>', methods=['POST']) @invite.route('/<invited_user_id>', methods=['POST'])
def update_invited_user(service_id, invited_user_id): def update_invited_user(service_id, invited_user_id):
fetched = get_invited_user(service_id=service_id, invited_user_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 = dict(invited_user_schema.dump(fetched).data.items())
current_data.update(request.get_json()) 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 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): def _create_invitation(invited_user):
from utils.url_safe_token import generate_token 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']) token = generate_token(str(invited_user.id), current_app.config['SECRET_KEY'], current_app.config['DANGEROUS_SALT'])

View File

@@ -29,8 +29,6 @@ register_errors(job)
@job.route('/<job_id>', methods=['GET']) @job.route('/<job_id>', methods=['GET'])
def get_job_by_service_and_job_id(service_id, job_id): 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) 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) data, errors = job_schema.dump(job)
return jsonify(data=data) return jsonify(data=data)
@@ -44,7 +42,6 @@ def get_jobs_by_service(service_id):
@job.route('', methods=['POST']) @job.route('', methods=['POST'])
def create_job(service_id): def create_job(service_id):
dao_fetch_service_by_id(service_id) dao_fetch_service_by_id(service_id)
data = request.get_json() data = request.get_json()

View File

@@ -19,9 +19,7 @@ def get_permissions():
@permission.route('/<permission_id>', methods=['GET']) @permission.route('/<permission_id>', methods=['GET'])
def get_permission(permission_id): def get_permission(permission_id):
inst = permission_dao.get_query(filter_by_dict={'id': permission_id}).first() inst = permission_dao.get_query(filter_by_dict={'id': permission_id}).one()
if not inst:
abort(404, 'Permission not found for id: {permission_id}'.format(permission_id))
data, errors = permission_schema.dump(inst) data, errors = permission_schema.dump(inst)
if errors: if errors:
abort(500, errors) abort(500, errors)

View File

@@ -57,11 +57,7 @@ def get_service_by_id(service_id):
fetched = dao_fetch_service_by_id_and_user(service_id, user_id) fetched = dao_fetch_service_by_id_and_user(service_id, user_id)
else: else:
fetched = dao_fetch_service_by_id(service_id) 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) data, errors = service_schema.dump(fetched)
return jsonify(data=data) return jsonify(data=data)
@@ -72,10 +68,7 @@ def create_service():
if not data.get('user_id', None): if not data.get('user_id', None):
return jsonify(result="error", message={'user_id': ['Missing data for required field.']}), 400 return jsonify(result="error", message={'user_id': ['Missing data for required field.']}), 400
try: user = get_model_users(data['user_id'])
user = get_model_users(data['user_id'])
except NoResultFound:
return jsonify(result="error", message={'user_id': ['not found']}), 400
data.pop('user_id', None) data.pop('user_id', None)
if 'name' in data: if 'name' in data:
@@ -93,8 +86,6 @@ def create_service():
@service.route('/<uuid:service_id>', methods=['POST']) @service.route('/<uuid:service_id>', methods=['POST'])
def update_service(service_id): def update_service(service_id):
fetched_service = dao_fetch_service_by_id(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 = dict(service_schema.dump(fetched_service).data.items())
current_data.update(request.get_json()) current_data.update(request.get_json())
@@ -108,8 +99,6 @@ def update_service(service_id):
@service.route('/<uuid:service_id>/api-key', methods=['POST']) @service.route('/<uuid:service_id>/api-key', methods=['POST'])
def renew_api_key(service_id=None): def renew_api_key(service_id=None):
fetched_service = dao_fetch_service_by_id(service_id=service_id) 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 # create a new one
# TODO: what validation should be done here? # TODO: what validation should be done here?
@@ -132,9 +121,8 @@ def revoke_api_key(service_id, api_key_id):
@service.route('/<uuid:service_id>/api-keys', methods=['GET']) @service.route('/<uuid:service_id>/api-keys', methods=['GET'])
@service.route('/<uuid:service_id>/api-keys/<int:key_id>', methods=['GET']) @service.route('/<uuid:service_id>/api-keys/<int:key_id>', methods=['GET'])
def get_api_keys(service_id, key_id=None): def get_api_keys(service_id, key_id=None):
fetched_service = dao_fetch_service_by_id(service_id=service_id) dao_fetch_service_by_id(service_id=service_id)
if not fetched_service:
return _service_not_found(service_id)
try: try:
if key_id: if key_id:
api_keys = [get_model_api_keys(service_id=service_id, id=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('/<uuid:service_id>/users', methods=['GET']) @service.route('/<uuid:service_id>/users', methods=['GET'])
def get_users_for_service(service_id): def get_users_for_service(service_id):
fetched = dao_fetch_service_by_id(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) result = user_schema.dump(fetched.users, many=True)
return jsonify(data=result.data) return jsonify(data=result.data)
@@ -162,14 +145,8 @@ def get_users_for_service(service_id):
@service.route('/<uuid:service_id>/users/<user_id>', methods=['POST']) @service.route('/<uuid:service_id>/users/<user_id>', methods=['POST'])
def add_user_to_service(service_id, user_id): def add_user_to_service(service_id, user_id):
service = dao_fetch_service_by_id(service_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) 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: if user in service.users:
return jsonify(result='error', return jsonify(result='error',
message='User id: {} already part of service id: {}'.format(user_id, service_id)), 400 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 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): def _process_permissions(user, service, permission_groups):
from app.permissions_utils import get_permissions_by_group from app.permissions_utils import get_permissions_by_group
from app.dao.permissions_dao import permission_dao from app.dao.permissions_dao import permission_dao

View File

@@ -156,7 +156,7 @@ def get_user(user_id=None):
return jsonify(data=result.data) return jsonify(data=result.data)
@user.route('/<int:user_id>/service/<service_id>/permission', methods=['POST']) @user.route('/<int:user_id>/service/<uuid:service_id>/permission', methods=['POST'])
def set_permissions(user_id, service_id): def set_permissions(user_id, service_id):
# TODO fix security hole, how do we verify that the user # TODO fix security hole, how do we verify that the user
# who is making this request has permission to make the request. # who is making this request has permission to make the request.

View File

@@ -1,5 +1,9 @@
from datetime import datetime, timedelta from datetime import datetime, timedelta
import uuid import uuid
import pytest
from sqlalchemy.orm.exc import NoResultFound
from app import db from app import db
from app.models import InvitedUser 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): def test_get_unknown_invited_user_returns_none(notify_db, notify_db_session, sample_service):
unknown_id = uuid.uuid4() unknown_id = uuid.uuid4()
unknown = get_invited_user(sample_service.id, unknown_id) with pytest.raises(NoResultFound) as e:
assert unknown is None 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): def test_get_invited_users_for_service(notify_db, notify_db_session, sample_service):

View File

@@ -171,4 +171,6 @@ def test_cannot_get_service_by_id_and_owned_by_different_user(service_factory, s
save_model_user(new_user) save_model_user(new_user)
service2 = service_factory.get('service 2', 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 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)

View File

@@ -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]) headers=[('Content-Type', 'application/json'), auth_header])
assert response.status_code == 404 assert response.status_code == 404
json_response = json.loads(response.get_data(as_text=True))['message'] 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: {}'\ assert json_response == 'No result found'
.format(bad_service_id, sample_invited_user.id)
def test_update_invited_user_for_invalid_data_returns_400(notify_api, sample_invited_user): def test_update_invited_user_for_invalid_data_returns_400(notify_api, sample_invited_user):

View File

@@ -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 assert resp.status_code == 404
json_resp = json.loads(resp.get_data(as_text=True)) json_resp = json.loads(resp.get_data(as_text=True))
assert json_resp['result'] == 'error' assert json_resp['result'] == 'error'
assert json_resp['message'] == \ assert json_resp['message'] == 'No result found'
'Service not found for service id: {0} and for user id: {1}'.format(service_id, sample_user.id)
def test_create_service(notify_api, sample_user): 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), data=json.dumps(data),
headers=headers) headers=headers)
json_resp = json.loads(resp.get_data(as_text=True)) 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 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): def test_should_not_create_service_if_missing_data(notify_api, sample_user):