From 4502dffa13d2ed78552c0a80ea766feef13bec2d Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 4 Jul 2018 14:54:27 +0100 Subject: [PATCH 1/2] include date being processed in the migrate nightly stats command --- app/commands.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/commands.py b/app/commands.py index b130d97a8..64f0c4661 100644 --- a/app/commands.py +++ b/app/commands.py @@ -616,8 +616,11 @@ def migrate_data_to_ft_notification_status(start_date, end_date): """ result = db.session.execute(sql, {"start": process_date, "end": process_date + timedelta(days=1)}) db.session.commit() - print('ft_notification_status: --- Completed took {}ms. Migrated {} rows.'.format(datetime.now() - start_time, - result.rowcount)) + print('ft_notification_status: --- Completed took {}ms. Migrated {} rows for {}.'.format( + datetime.now() - start_time, + result.rowcount, + process_date + )) process_date += timedelta(days=1) total_updated += result.rowcount From 32415b3b14938c5c16d6f423e21f76623e56cabf Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 5 Jul 2018 11:09:17 +0100 Subject: [PATCH 2/2] add delete functions for inbound and callback api objects Both service api tasks work fine if the object is unexpectedly deleted halfway through - they both check to see if the api details are still in the DB before trying to send the request. --- app/dao/service_callback_api_dao.py | 5 + app/dao/service_inbound_api_dao.py | 5 + app/service/callback_rest.py | 35 ++++- tests/app/conftest.py | 9 +- tests/app/service/test_callback_rest.py | 175 +++++++++++++++--------- 5 files changed, 155 insertions(+), 74 deletions(-) diff --git a/app/dao/service_callback_api_dao.py b/app/dao/service_callback_api_dao.py index 2bc7cef76..e7d991adb 100644 --- a/app/dao/service_callback_api_dao.py +++ b/app/dao/service_callback_api_dao.py @@ -32,3 +32,8 @@ def get_service_callback_api(service_callback_api_id, service_id): def get_service_callback_api_for_service(service_id): return ServiceCallbackApi.query.filter_by(service_id=service_id).first() + + +@transactional +def delete_service_callback_api(service_callback_api): + db.session.delete(service_callback_api) diff --git a/app/dao/service_inbound_api_dao.py b/app/dao/service_inbound_api_dao.py index 8c7f2c422..6b8cc973c 100644 --- a/app/dao/service_inbound_api_dao.py +++ b/app/dao/service_inbound_api_dao.py @@ -33,3 +33,8 @@ def get_service_inbound_api(service_inbound_api_id, service_id): def get_service_inbound_api_for_service(service_id): return ServiceInboundApi.query.filter_by(service_id=service_id).first() + + +@transactional +def delete_service_inbound_api(service_inbound_api): + db.session.delete(service_inbound_api) diff --git a/app/service/callback_rest.py b/app/service/callback_rest.py index ca52a7814..26d0b6e2d 100644 --- a/app/service/callback_rest.py +++ b/app/service/callback_rest.py @@ -6,7 +6,8 @@ from flask import ( from sqlalchemy.exc import SQLAlchemyError from app.errors import ( - register_errors + register_errors, + InvalidRequest ) from app.models import ( ServiceInboundApi, @@ -20,12 +21,14 @@ from app.service.service_callback_api_schema import ( from app.dao.service_inbound_api_dao import ( save_service_inbound_api, get_service_inbound_api, - reset_service_inbound_api + reset_service_inbound_api, + delete_service_inbound_api, ) from app.dao.service_callback_api_dao import ( save_service_callback_api, get_service_callback_api, - reset_service_callback_api + reset_service_callback_api, + delete_service_callback_api, ) service_callback_blueprint = Blueprint('service_callback', __name__, url_prefix='/service/') @@ -61,13 +64,25 @@ def update_service_inbound_api(service_id, inbound_api_id): return jsonify(data=to_update.serialize()), 200 -@service_callback_blueprint.route('/inbound-api/', methods=["GET"]) +@service_callback_blueprint.route('/inbound-api/', methods=['GET']) def fetch_service_inbound_api(service_id, inbound_api_id): inbound_api = get_service_inbound_api(inbound_api_id, service_id) return jsonify(data=inbound_api.serialize()), 200 +@service_callback_blueprint.route('/inbound-api/', methods=['DELETE']) +def remove_service_inbound_api(service_id, inbound_api_id): + inbound_api = get_service_inbound_api(inbound_api_id, service_id) + + if not inbound_api: + error = 'Service inbound API not found' + raise InvalidRequest(error, status_code=404) + + delete_service_inbound_api(inbound_api) + return '', 204 + + @service_callback_blueprint.route('/delivery-receipt-api', methods=['POST']) def create_service_callback_api(service_id): data = request.get_json() @@ -103,6 +118,18 @@ def fetch_service_callback_api(service_id, callback_api_id): return jsonify(data=callback_api.serialize()), 200 +@service_callback_blueprint.route('/delivery-receipt-api/', methods=['DELETE']) +def remove_service_callback_api(service_id, callback_api_id): + callback_api = get_service_callback_api(callback_api_id, service_id) + + if not callback_api: + error = 'Service delivery receipt callback API not found' + raise InvalidRequest(error, status_code=404) + + delete_service_callback_api(callback_api) + return '', 204 + + def handle_sql_error(e, table_name): if hasattr(e, 'orig') and hasattr(e.orig, 'pgerror') and e.orig.pgerror \ and ('duplicate key value violates unique constraint "ix_{}_service_id"'.format(table_name) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index ed6d432c8..fc27f1500 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -1106,7 +1106,7 @@ def admin_request(client): url_for(endpoint, **(endpoint_kwargs or {})), headers=[create_authorization_header()] ) - json_resp = json.loads(resp.get_data(as_text=True)) + json_resp = resp.json assert resp.status_code == _expected_status return json_resp @@ -1118,7 +1118,7 @@ def admin_request(client): headers=[('Content-Type', 'application/json'), create_authorization_header()] ) if resp.get_data(): - json_resp = json.loads(resp.get_data(as_text=True)) + json_resp = resp.json else: json_resp = None assert resp.status_code == _expected_status @@ -1130,7 +1130,10 @@ def admin_request(client): url_for(endpoint, **(endpoint_kwargs or {})), headers=[create_authorization_header()] ) - json_resp = json.loads(resp.get_data(as_text=True)) + if resp.get_data(): + json_resp = resp.json + else: + json_resp = None assert resp.status_code == _expected_status, json_resp return json_resp diff --git a/tests/app/service/test_callback_rest.py b/tests/app/service/test_callback_rest.py index bd2dcf223..33e92b14f 100644 --- a/tests/app/service/test_callback_rest.py +++ b/tests/app/service/test_callback_rest.py @@ -1,28 +1,27 @@ -import json import uuid -from tests import create_authorization_header - from tests.app.db import ( create_service_inbound_api, create_service_callback_api ) +from app.models import ServiceInboundApi, ServiceCallbackApi -def test_create_service_inbound_api(client, sample_service): + +def test_create_service_inbound_api(admin_request, sample_service): data = { "url": "https://some_service/inbound-sms", "bearer_token": "some-unique-string", "updated_by_id": str(sample_service.users[0].id) } - response = client.post( - '/service/{}/inbound-api'.format(sample_service.id), - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), create_authorization_header()] + resp_json = admin_request.post( + 'service_callback.create_service_inbound_api', + service_id=sample_service.id, + _data=data, + _expected_status=201 ) - assert response.status_code == 201 - resp_json = json.loads(response.get_data(as_text=True))["data"] + resp_json = resp_json["data"] assert resp_json["id"] assert resp_json["service_id"] == str(sample_service.id) assert resp_json["url"] == "https://some_service/inbound-sms" @@ -31,22 +30,22 @@ def test_create_service_inbound_api(client, sample_service): assert not resp_json["updated_at"] -def test_set_service_inbound_api_raises_404_when_service_does_not_exist(client): +def test_set_service_inbound_api_raises_404_when_service_does_not_exist(admin_request): data = { "url": "https://some_service/inbound-sms", "bearer_token": "some-unique-string", "updated_by_id": str(uuid.uuid4()) } - response = client.post( - '/service/{}/inbound-api'.format(uuid.uuid4()), - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), create_authorization_header()] + response = admin_request.post( + 'service_callback.create_service_inbound_api', + service_id=uuid.uuid4(), + _data=data, + _expected_status=404 ) - assert response.status_code == 404 - assert json.loads(response.get_data(as_text=True))['message'] == 'No result found' + assert response['message'] == 'No result found' -def test_update_service_inbound_api_updates_url(client, sample_service): +def test_update_service_inbound_api_updates_url(admin_request, sample_service): service_inbound_api = create_service_inbound_api(service=sample_service, url="https://original_url.com") @@ -54,53 +53,74 @@ def test_update_service_inbound_api_updates_url(client, sample_service): "url": "https://another_url.com", "updated_by_id": str(sample_service.users[0].id) } - response = client.post("/service/{}/inbound-api/{}".format(sample_service.id, service_inbound_api.id), - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), create_authorization_header()]) - assert response.status_code == 200 - resp_json = json.loads(response.get_data(as_text=True))["data"] - assert resp_json["url"] == "https://another_url.com" + + response = admin_request.post( + 'service_callback.update_service_inbound_api', + service_id=sample_service.id, + inbound_api_id=service_inbound_api.id, + _data=data + ) + + assert response["data"]["url"] == "https://another_url.com" assert service_inbound_api.url == "https://another_url.com" -def test_update_service_inbound_api_updates_bearer_token(client, sample_service): +def test_update_service_inbound_api_updates_bearer_token(admin_request, sample_service): service_inbound_api = create_service_inbound_api(service=sample_service, bearer_token="some_super_secret") data = { "bearer_token": "different_token", "updated_by_id": str(sample_service.users[0].id) } - response = client.post("/service/{}/inbound-api/{}".format(sample_service.id, service_inbound_api.id), - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), create_authorization_header()]) - assert response.status_code == 200 + + admin_request.post( + 'service_callback.update_service_inbound_api', + service_id=sample_service.id, + inbound_api_id=service_inbound_api.id, + _data=data + ) assert service_inbound_api.bearer_token == "different_token" -def test_fetch_service_inbound_api(client, sample_service): +def test_fetch_service_inbound_api(admin_request, sample_service): service_inbound_api = create_service_inbound_api(service=sample_service) - response = client.get("/service/{}/inbound-api/{}".format(sample_service.id, service_inbound_api.id), - headers=[create_authorization_header()]) - - assert response.status_code == 200 - assert json.loads(response.get_data(as_text=True))["data"] == service_inbound_api.serialize() + response = admin_request.get( + 'service_callback.fetch_service_inbound_api', + service_id=sample_service.id, + inbound_api_id=service_inbound_api.id, + ) + assert response["data"] == service_inbound_api.serialize() -def test_create_service_callback_api(client, sample_service): +def test_delete_service_inbound_api(admin_request, sample_service): + service_inbound_api = create_service_inbound_api(sample_service) + + response = admin_request.delete( + 'service_callback.remove_service_inbound_api', + service_id=sample_service.id, + inbound_api_id=service_inbound_api.id, + ) + + assert response is None + assert ServiceInboundApi.query.count() == 0 + + +def test_create_service_callback_api(admin_request, sample_service): data = { "url": "https://some_service/delivery-receipt-endpoint", "bearer_token": "some-unique-string", "updated_by_id": str(sample_service.users[0].id) } - response = client.post( - '/service/{}/delivery-receipt-api'.format(sample_service.id), - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), create_authorization_header()] - ) - assert response.status_code == 201 - resp_json = json.loads(response.get_data(as_text=True))["data"] + resp_json = admin_request.post( + 'service_callback.create_service_callback_api', + service_id=sample_service.id, + _data=data, + _expected_status=201 + ) + + resp_json = resp_json["data"] assert resp_json["id"] assert resp_json["service_id"] == str(sample_service.id) assert resp_json["url"] == "https://some_service/delivery-receipt-endpoint" @@ -109,22 +129,23 @@ def test_create_service_callback_api(client, sample_service): assert not resp_json["updated_at"] -def test_set_service_callback_api_raises_404_when_service_does_not_exist(client, notify_db_session): +def test_set_service_callback_api_raises_404_when_service_does_not_exist(admin_request, notify_db_session): data = { "url": "https://some_service/delivery-receipt-endpoint", "bearer_token": "some-unique-string", "updated_by_id": str(uuid.uuid4()) } - response = client.post( - '/service/{}/delivery-receipt-api'.format(uuid.uuid4()), - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), create_authorization_header()] + + resp_json = admin_request.post( + 'service_callback.create_service_callback_api', + service_id=uuid.uuid4(), + _data=data, + _expected_status=404 ) - assert response.status_code == 404 - assert json.loads(response.get_data(as_text=True))['message'] == 'No result found' + assert resp_json['message'] == 'No result found' -def test_update_service_callback_api_updates_url(client, sample_service): +def test_update_service_callback_api_updates_url(admin_request, sample_service): service_callback_api = create_service_callback_api(service=sample_service, url="https://original_url.com") @@ -132,34 +153,54 @@ def test_update_service_callback_api_updates_url(client, sample_service): "url": "https://another_url.com", "updated_by_id": str(sample_service.users[0].id) } - response = client.post("/service/{}/delivery-receipt-api/{}".format(sample_service.id, service_callback_api.id), - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), create_authorization_header()]) - assert response.status_code == 200 - resp_json = json.loads(response.get_data(as_text=True))["data"] - assert resp_json["url"] == "https://another_url.com" + + resp_json = admin_request.post( + 'service_callback.update_service_callback_api', + service_id=sample_service.id, + callback_api_id=service_callback_api.id, + _data=data + ) + assert resp_json["data"]["url"] == "https://another_url.com" assert service_callback_api.url == "https://another_url.com" -def test_update_service_callback_api_updates_bearer_token(client, sample_service): +def test_update_service_callback_api_updates_bearer_token(admin_request, sample_service): service_callback_api = create_service_callback_api(service=sample_service, bearer_token="some_super_secret") data = { "bearer_token": "different_token", "updated_by_id": str(sample_service.users[0].id) } - response = client.post("/service/{}/delivery-receipt-api/{}".format(sample_service.id, service_callback_api.id), - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), create_authorization_header()]) - assert response.status_code == 200 + + admin_request.post( + 'service_callback.update_service_callback_api', + service_id=sample_service.id, + callback_api_id=service_callback_api.id, + _data=data + ) assert service_callback_api.bearer_token == "different_token" -def test_fetch_service_callback_api(client, sample_service): +def test_fetch_service_callback_api(admin_request, sample_service): service_callback_api = create_service_callback_api(service=sample_service) - response = client.get("/service/{}/delivery-receipt-api/{}".format(sample_service.id, service_callback_api.id), - headers=[create_authorization_header()]) + response = admin_request.get( + 'service_callback.fetch_service_callback_api', + service_id=sample_service.id, + callback_api_id=service_callback_api.id, + ) - assert response.status_code == 200 - assert json.loads(response.get_data(as_text=True))["data"] == service_callback_api.serialize() + assert response["data"] == service_callback_api.serialize() + + +def test_delete_service_callback_api(admin_request, sample_service): + service_callback_api = create_service_callback_api(sample_service) + + response = admin_request.delete( + 'service_callback.remove_service_callback_api', + service_id=sample_service.id, + callback_api_id=service_callback_api.id, + ) + + assert response is None + assert ServiceCallbackApi.query.count() == 0