diff --git a/app/errors.py b/app/errors.py index 15f8977ad..37e447ed0 100644 --- a/app/errors.py +++ b/app/errors.py @@ -10,42 +10,42 @@ def register_errors(blueprint): @blueprint.app_errorhandler(400) def bad_request(e): - return jsonify(error=str(e.description)), 400 + return jsonify(result='error', message=str(e.description)), 400 @blueprint.app_errorhandler(401) def unauthorized(e): error_message = "Unauthorized, authentication token must be provided" - return jsonify(error=error_message), 401, [('WWW-Authenticate', 'Bearer')] + return jsonify(result='error', message=error_message), 401, [('WWW-Authenticate', 'Bearer')] @blueprint.app_errorhandler(403) def forbidden(e): error_message = "Forbidden, invalid authentication token provided" - return jsonify(error=error_message), 403 + return jsonify(result='error', message=error_message), 403 @blueprint.app_errorhandler(404) def page_not_found(e): - return jsonify(error=e.description or "Not found"), 404 + return jsonify(result='error', message=e.description or "Not found"), 404 @blueprint.app_errorhandler(429) def limit_exceeded(e): - return jsonify(error=str(e.description)), 429 + return jsonify(result='error', message=str(e.description)), 429 @blueprint.app_errorhandler(500) def internal_server_error(e): current_app.logger.exception(e) - return jsonify(error="Internal error"), 500 + return jsonify(result='error', message="Internal server error"), 500 @blueprint.app_errorhandler(NoResultFound) def no_result_found(e): current_app.logger.error(e) - return jsonify(error="No result found"), 404 + return jsonify(result='error', message="No result found"), 404 @blueprint.app_errorhandler(DataError) - def no_result_found(e): + def data_error(e): current_app.logger.error(e) - return jsonify(result="error", message="No result found"), 404 + return jsonify(result='error', message="No result found"), 404 @blueprint.app_errorhandler(SQLAlchemyError) def db_error(e): current_app.logger.error(e) - return jsonify(error="Database error occurred"), 500 + return jsonify(result='error', message=str(e)), 500 diff --git a/app/models.py b/app/models.py index ec6f54d8f..384bac10b 100644 --- a/app/models.py +++ b/app/models.py @@ -58,7 +58,10 @@ user_to_service = db.Table( 'user_to_service', db.Model.metadata, db.Column('user_id', db.Integer, db.ForeignKey('users.id')), - db.Column('service_id', UUID(as_uuid=True), db.ForeignKey('services.id')) + db.Column('service_id', UUID(as_uuid=True), db.ForeignKey('services.id')), + + UniqueConstraint('user_id', 'service_id', name='uix_user_to_service') + ) diff --git a/app/service/rest.py b/app/service/rest.py index 3363e210e..8be7442ac 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -1,14 +1,18 @@ from datetime import datetime +from flask import Blueprint from flask import ( jsonify, request ) -from sqlalchemy.exc import DataError from sqlalchemy.orm.exc import NoResultFound -from app.dao import DAOException -from app.dao.users_dao import get_model_users +from app import email_safe +from app.dao.api_key_dao import ( + save_model_api_key, + get_model_api_keys, + get_unsigned_secret +) from app.dao.services_dao import ( dao_fetch_service_by_id_and_user, dao_fetch_service_by_id, @@ -17,25 +21,17 @@ from app.dao.services_dao import ( dao_update_service, dao_fetch_all_services_by_user ) - -from app.dao.api_key_dao import ( - save_model_api_key, - get_model_api_keys, - get_unsigned_secret -) +from app.dao.users_dao import get_model_users from app.models import ApiKey from app.schemas import ( services_schema, service_schema, api_keys_schema, users_schema) -from app import email_safe - -from flask import Blueprint +from app.errors import register_errors service = Blueprint('service', __name__) -from app.errors import register_errors register_errors(service) @@ -59,7 +55,10 @@ def get_service_by_id(service_id): else: fetched = dao_fetch_service_by_id(service_id) if not fetched: - return jsonify(result="error", message="not found"), 404 + 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) @@ -91,7 +90,7 @@ def create_service(): def update_service(service_id): fetched_service = dao_fetch_service_by_id(service_id) if not fetched_service: - return jsonify(result="error", message="not found"), 404 + return _service_not_found(service_id) current_data = dict(service_schema.dump(fetched_service).data.items()) current_data.update(request.get_json()) @@ -107,28 +106,21 @@ def update_service(service_id): def renew_api_key(service_id=None): fetched_service = dao_fetch_service_by_id(service_id=service_id) if not fetched_service: - return jsonify(result="error", message="Service not found"), 404 + return _service_not_found(service_id) + + # create a new one + # TODO: what validation should be done here? + secret_name = request.get_json()['name'] + key = ApiKey(service=fetched_service, name=secret_name) + save_model_api_key(key) - try: - # create a new one - # TODO: what validation should be done here? - secret_name = request.get_json()['name'] - key = ApiKey(service=fetched_service, name=secret_name) - save_model_api_key(key) - except DAOException as e: - return jsonify(result='error', message=str(e)), 500 unsigned_api_key = get_unsigned_secret(key.id) return jsonify(data=unsigned_api_key), 201 @service.route('//api-key/revoke/', methods=['POST']) def revoke_api_key(service_id, api_key_id): - try: - service_api_key = get_model_api_keys(service_id=service_id, id=api_key_id) - except DataError: - return jsonify(result="error", message="Invalid api key for service"), 400 - except NoResultFound: - return jsonify(result="error", message="Api key not found for service"), 404 + service_api_key = get_model_api_keys(service_id=service_id, id=api_key_id) save_model_api_key(service_api_key, update_dict={'id': service_api_key.id, 'expiry_date': datetime.utcnow()}) return jsonify(), 202 @@ -137,22 +129,16 @@ 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): - try: - service = dao_fetch_service_by_id(service_id=service_id) - except DataError: - return jsonify(result="error", message="Invalid service id"), 400 - except NoResultFound: - return jsonify(result="error", message="Service not found"), 404 - + fetched_service = dao_fetch_service_by_id(service_id=service_id) + if not fetched_service: + return _service_not_found(service_id) try: if key_id: api_keys = [get_model_api_keys(service_id=service_id, id=key_id)] else: api_keys = get_model_api_keys(service_id=service_id) - except DAOException as e: - return jsonify(result='error', message=str(e)), 500 except NoResultFound: - return jsonify(result="error", message="API key not found"), 404 + return jsonify(result="error", message="API key not found for id: {}".format(service_id)), 404 return jsonify(apiKeys=api_keys_schema.dump(api_keys).data), 200 @@ -161,7 +147,13 @@ def get_api_keys(service_id, key_id=None): def get_users_for_service(service_id): fetched = dao_fetch_service_by_id(service_id) if not fetched: + return _service_not_found(service_id) + if not fetched.users: return jsonify(data=[]) result = users_schema.dump(fetched.users) return jsonify(data=result.data) + + +def _service_not_found(service_id): + return jsonify(result='error', message='Service not found for id: {}'.format(service_id)), 404 diff --git a/migrations/versions/0024_uix_user_to_service.py b/migrations/versions/0024_uix_user_to_service.py new file mode 100644 index 000000000..63b131c68 --- /dev/null +++ b/migrations/versions/0024_uix_user_to_service.py @@ -0,0 +1,26 @@ +"""empty message + +Revision ID: 0024_uix_user_to_service +Revises: 0023_drop_token +Create Date: 2016-02-25 11:15:06.088508 + +""" + +# revision identifiers, used by Alembic. +revision = '0024_uix_user_to_service' +down_revision = '0023_drop_token' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.create_unique_constraint('uix_user_to_service', 'user_to_service', ['user_id', 'service_id']) + ### end Alembic commands ### + + +def downgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.drop_constraint('uix_user_to_service', 'user_to_service', type_='unique') + ### end Alembic commands ### diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index bf1b74594..6e44f68d2 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1,10 +1,9 @@ import json import uuid - from app.dao.users_dao import save_model_user +from app.dao.services_dao import dao_remove_user_from_service from app.models import User from tests import create_authorization_header -from tests.app.conftest import sample_service as create_service def test_get_service_list(notify_api, service_factory): @@ -13,7 +12,6 @@ def test_get_service_list(notify_api, service_factory): service_factory.get('one') service_factory.get('two') service_factory.get('three') - auth_header = create_authorization_header( path='/service', method='GET' @@ -30,7 +28,8 @@ def test_get_service_list(notify_api, service_factory): assert json_resp['data'][2]['name'] == 'three' -def test_get_service_list_by_user(notify_api, service_factory, sample_user): +def test_get_service_list_by_user(notify_api, sample_user, service_factory): + with notify_api.test_request_context(): with notify_api.test_client() as client: service_factory.get('one', sample_user) @@ -81,7 +80,7 @@ def test_get_service_list_by_user_should_return_empty_list_if_no_services(notify assert len(json_resp['data']) == 0 -def test_get_service_list_should_return_empty_list_if_no_services(notify_api, notify_db): +def test_get_service_list_should_return_empty_list_if_no_services(notify_api, notify_db, notify_db_session): with notify_api.test_request_context(): with notify_api.test_client() as client: auth_header = create_authorization_header( @@ -129,7 +128,7 @@ def test_get_service_by_id_should_404_if_no_service(notify_api, notify_db): assert resp.status_code == 404 json_resp = json.loads(resp.get_data(as_text=True)) assert json_resp['result'] == 'error' - assert json_resp['message'] == 'not found' + assert json_resp['message'] == 'Service not found for service id: {} '.format(service_id) def test_get_service_by_id_and_user(notify_api, service_factory, sample_user): @@ -165,7 +164,8 @@ 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'] == 'not found' + assert json_resp['message'] == \ + '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): @@ -233,7 +233,7 @@ def test_should_not_create_service_with_missing_user_id_field(notify_api): assert 'Missing data for required field.' in json_resp['message']['user_id'] -def test_should_not_create_service_with_missing_if_user_id_is_not_in_database(notify_api, notify_db): +def test_should_not_create_service_with_missing_if_user_id_is_not_in_database(notify_api, notify_db, notify_db_session): with notify_api.test_request_context(): with notify_api.test_client() as client: data = { @@ -320,10 +320,9 @@ def test_update_service(notify_api, sample_service): assert result['data']['name'] == 'updated service name' -def test_update_service_should_404_if_id_is_invalid(notify_api, notify_db): +def test_update_service_should_404_if_id_is_invalid(notify_api, notify_db, notify_db_session): with notify_api.test_request_context(): with notify_api.test_client() as client: - data = { 'name': 'updated service name' } @@ -344,12 +343,10 @@ def test_update_service_should_404_if_id_is_invalid(notify_api, notify_db): assert resp.status_code == 404 -def test_get_users_by_service(notify_api, notify_db, notify_db_session, sample_user): +def test_get_users_by_service(notify_api, notify_db, notify_db_session, sample_service): with notify_api.test_request_context(): with notify_api.test_client() as client: - - sample_service = create_service(notify_db=notify_db, notify_db_session=notify_db_session, - service_name='Sample service', user=sample_user) + user_on_service = sample_service.users[0] auth_header = create_authorization_header( path='/service/{}/users'.format(sample_service.id), method='GET' @@ -363,19 +360,18 @@ def test_get_users_by_service(notify_api, notify_db, notify_db_session, sample_u assert resp.status_code == 200 result = json.loads(resp.get_data(as_text=True)) assert len(result['data']) == 1 - assert result['data'][0]['name'] == sample_user.name - assert result['data'][0]['email_address'] == sample_user.email_address - assert result['data'][0]['mobile_number'] == sample_user.mobile_number + assert result['data'][0]['name'] == user_on_service.name + assert result['data'][0]['email_address'] == user_on_service.email_address + assert result['data'][0]['mobile_number'] == user_on_service.mobile_number def test_get_users_for_service_returns_empty_list_if_no_users_associated_with_service(notify_api, notify_db, notify_db_session, - sample_service, - sample_user): + sample_service): with notify_api.test_request_context(): with notify_api.test_client() as client: - sample_service.users.remove(sample_user) + dao_remove_user_from_service(sample_service, sample_service.users[0]) auth_header = create_authorization_header( path='/service/{}/users'.format(sample_service.id), method='GET' @@ -388,3 +384,22 @@ def test_get_users_for_service_returns_empty_list_if_no_users_associated_with_se result = json.loads(response.get_data(as_text=True)) assert response.status_code == 200 assert result['data'] == [] + + +def test_get_users_for_service_returns_404_when_service_does_not_exist(notify_api, notify_db, notify_db_session): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + service_id = uuid.uuid4() + auth_header = create_authorization_header( + path='/service/{}/users'.format(service_id), + method='GET' + ) + + response = client.get( + '/service/{}/users'.format(service_id), + headers=[('Content-Type', 'application/json'), auth_header] + ) + assert response.status_code == 404 + result = json.loads(response.get_data(as_text=True)) + assert result['result'] == 'error' + assert result['message'] == 'Service not found for id: {}'.format(service_id) diff --git a/tests/conftest.py b/tests/conftest.py index 795a791df..55d72a2f2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -52,8 +52,8 @@ def notify_db_session(request): def teardown(): db.session.remove() for tbl in reversed(meta.sorted_tables): - if tbl.fullname not in ['roles']: - db.engine.execute(tbl.delete()) + db.engine.execute(tbl.delete()) + db.session.commit() meta = MetaData(bind=db.engine, reflect=True) request.addfinalizer(teardown)