From c07e804319ef200b15e03e27fe57c70eb1703a27 Mon Sep 17 00:00:00 2001 From: venusbb Date: Wed, 29 Nov 2017 15:58:11 +0000 Subject: [PATCH 1/8] Created service_callback_api daos --- app/dao/service_callback_api_dao.py | 35 +++++ .../app/dao/test_service_callback_api_dao.py | 129 ++++++++++++++++++ tests/app/db.py | 16 +++ 3 files changed, 180 insertions(+) create mode 100644 app/dao/service_callback_api_dao.py create mode 100644 tests/app/dao/test_service_callback_api_dao.py diff --git a/app/dao/service_callback_api_dao.py b/app/dao/service_callback_api_dao.py new file mode 100644 index 000000000..7db4435ba --- /dev/null +++ b/app/dao/service_callback_api_dao.py @@ -0,0 +1,35 @@ +from datetime import datetime + +from app import db, create_uuid +from app.dao.dao_utils import transactional, version_class +from app.models import ServiceCallbackApi + + +@transactional +@version_class(ServiceCallbackApi) +def save_service_callback_api(service_callback_api): + service_callback_api.id = create_uuid() + service_callback_api.created_at == datetime.utcnow() + service_callback_api.bearer_token = service_callback_api.bearer_token + db.session.add(service_callback_api) + + +@transactional +@version_class(ServiceCallbackApi) +def reset_service_callback_api(service_callback_api, updated_by_id, url=None, bearer_token=None): + if url: + service_callback_api.url = url + if bearer_token: + service_callback_api.bearer_token = bearer_token + service_callback_api.updated_by_id = updated_by_id + service_callback_api.updated_at = datetime.utcnow() + + db.session.add(service_callback_api) + + +def get_service_callback_api(service_callback_api_id, service_id): + return ServiceCallbackApi.query.filter_by(id=service_callback_api_id, service_id=service_id).first() + + +def get_service_callback_api_for_service(service_id): + return ServiceCallbackApi.query.filter_by(service_id=service_id).first() diff --git a/tests/app/dao/test_service_callback_api_dao.py b/tests/app/dao/test_service_callback_api_dao.py new file mode 100644 index 000000000..4f064a8a0 --- /dev/null +++ b/tests/app/dao/test_service_callback_api_dao.py @@ -0,0 +1,129 @@ +import uuid + +import pytest +from sqlalchemy.exc import SQLAlchemyError + +from app import encryption +from app.dao.service_callback_api_dao import ( + save_service_callback_api, + reset_service_callback_api, + get_service_callback_api, + get_service_callback_api_for_service) +from app.models import ServiceCallbackApi +from tests.app.db import create_service_callback_api + + +def test_save_service_callback_api(sample_service): + service_callback_api = ServiceCallbackApi( + service_id=sample_service.id, + url="https://some_service/callback_endpoint", + bearer_token="some_unique_string", + updated_by_id=sample_service.users[0].id + ) + + save_service_callback_api(service_callback_api) + + results = ServiceCallbackApi.query.all() + assert len(results) == 1 + callback_api = results[0] + assert callback_api.id is not None + assert callback_api.service_id == sample_service.id + assert callback_api.updated_by_id == sample_service.users[0].id + assert callback_api.url == "https://some_service/callback_endpoint" + assert callback_api.bearer_token == "some_unique_string" + assert callback_api._bearer_token != "some_unique_string" + assert callback_api.updated_at is None + + versioned = ServiceCallbackApi.get_history_model().query.filter_by(id=callback_api.id).one() + assert versioned.id == callback_api.id + assert versioned.service_id == sample_service.id + assert versioned.updated_by_id == sample_service.users[0].id + assert versioned.url == "https://some_service/callback_endpoint" + assert encryption.decrypt(versioned._bearer_token) == "some_unique_string" + assert versioned.updated_at is None + assert versioned.version == 1 + + +def test_save_service_callback_api_fails_if_service_does_not_exist(notify_db, notify_db_session): + service_callback_api = ServiceCallbackApi( + service_id=uuid.uuid4(), + url="https://some_service/callback_endpoint", + bearer_token="some_unique_string", + updated_by_id=uuid.uuid4() + ) + + with pytest.raises(SQLAlchemyError): + save_service_callback_api(service_callback_api) + + +def test_update_service_callback_api(sample_service): + service_callback_api = ServiceCallbackApi( + service_id=sample_service.id, + url="https://some_service/callback_endpoint", + bearer_token="some_unique_string", + updated_by_id=sample_service.users[0].id + ) + + save_service_callback_api(service_callback_api) + results = ServiceCallbackApi.query.all() + assert len(results) == 1 + saved_callback_api = results[0] + + reset_service_callback_api(saved_callback_api, updated_by_id=sample_service.users[0].id, + url="https://some_service/changed_url") + updated_results = ServiceCallbackApi.query.all() + assert len(updated_results) == 1 + updated = updated_results[0] + assert updated.id is not None + assert updated.service_id == sample_service.id + assert updated.updated_by_id == sample_service.users[0].id + assert updated.url == "https://some_service/changed_url" + assert updated.bearer_token == "some_unique_string" + assert updated._bearer_token != "some_unique_string" + assert updated.updated_at is not None + + versioned_results = ServiceCallbackApi.get_history_model().query.filter_by(id=saved_callback_api.id).all() + assert len(versioned_results) == 2 + for x in versioned_results: + if x.version == 1: + assert x.url == "https://some_service/callback_endpoint" + assert not x.updated_at + elif x.version == 2: + assert x.url == "https://some_service/changed_url" + assert x.updated_at + else: + pytest.fail("version should not exist") + assert x.id is not None + assert x.service_id == sample_service.id + assert x.updated_by_id == sample_service.users[0].id + assert encryption.decrypt(x._bearer_token) == "some_unique_string" + + +def test_get_service_callback_api(sample_service): + service_callback_api = ServiceCallbackApi( + service_id=sample_service.id, + url="https://some_service/callback_endpoint", + bearer_token="some_unique_string", + updated_by_id=sample_service.users[0].id + ) + save_service_callback_api(service_callback_api) + + callback_api = get_service_callback_api(service_callback_api.id, sample_service.id) + assert callback_api.id is not None + assert callback_api.service_id == sample_service.id + assert callback_api.updated_by_id == sample_service.users[0].id + assert callback_api.url == "https://some_service/callback_endpoint" + assert callback_api.bearer_token == "some_unique_string" + assert callback_api._bearer_token != "some_unique_string" + assert callback_api.updated_at is None + + +def test_get_service_callback_api_for_service(sample_service): + service_callback_api = create_service_callback_api(service=sample_service) + result = get_service_callback_api_for_service(sample_service.id) + assert result.id == service_callback_api.id + assert result.url == service_callback_api.url + assert result.bearer_token == service_callback_api.bearer_token + assert result.created_at == service_callback_api.created_at + assert result.updated_at == service_callback_api.updated_at + assert result.updated_by_id == service_callback_api.updated_by_id diff --git a/tests/app/db.py b/tests/app/db.py index 41b6691ca..504245d86 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -4,6 +4,7 @@ import uuid from app import db from app.dao.jobs_dao import dao_create_job from app.dao.service_inbound_api_dao import save_service_inbound_api +from app.dao.service_callback_api_dao import save_service_callback_api from app.dao.service_sms_sender_dao import update_existing_sms_sender_with_inbound_number, dao_update_service_sms_sender from app.models import ( ApiKey, @@ -18,6 +19,7 @@ from app.models import ( Service, ServiceEmailReplyTo, ServiceInboundApi, + ServiceCallbackApi, ServiceLetterContact, ScheduledNotification, ServicePermission, @@ -301,6 +303,20 @@ def create_service_inbound_api( return service_inbound_api +def create_service_callback_api( + service, + url="https://something.com", + bearer_token="some_super_secret", +): + service_callback_api = ServiceCallbackApi(service_id=service.id, + url=url, + bearer_token=bearer_token, + updated_by_id=service.users[0].id + ) + save_service_callback_api(service_callback_api) + return service_callback_api + + def create_organisation(colour='blue', logo='test_x2.png', name='test_org_1'): data = { 'colour': colour, From 0304af08df7314e20118c5f2ef6ba2fae7d33f21 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 29 Nov 2017 16:28:01 +0000 Subject: [PATCH 2/8] move service_inbound_api endpoints to their own blueprint try and reduce the size of the service blueprint :) --- app/__init__.py | 4 + app/service/callback_rest.py | 79 +++++++++++++++++++ app/service/rest.py | 68 +--------------- ...hema.py => service_callback_api_schema.py} | 12 +-- 4 files changed, 90 insertions(+), 73 deletions(-) create mode 100644 app/service/callback_rest.py rename app/service/{service_inbound_api_schema.py => service_callback_api_schema.py} (66%) diff --git a/app/__init__.py b/app/__init__.py index de0f50aad..870972817 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -82,6 +82,7 @@ def create_app(application): def register_blueprint(application): from app.service.rest import service_blueprint + from app.service.callback_rest import service_callback_blueprint from app.user.rest import user_blueprint from app.template.rest import template_blueprint from app.status.healthcheck import status as status_blueprint @@ -171,6 +172,9 @@ def register_blueprint(application): billing_blueprint.before_request(requires_admin_auth) application.register_blueprint(billing_blueprint) + service_callback_blueprint.before_request(requires_admin_auth) + application.register_blueprint(service_callback_blueprint) + def register_v2_blueprints(application): from app.v2.inbound_sms.get_inbound_sms import v2_inbound_sms_blueprint as get_inbound_sms diff --git a/app/service/callback_rest.py b/app/service/callback_rest.py new file mode 100644 index 000000000..95098e989 --- /dev/null +++ b/app/service/callback_rest.py @@ -0,0 +1,79 @@ +from flask import ( + Blueprint, + jsonify, + request, +) +from sqlalchemy.exc import SQLAlchemyError + +from app.dao.service_inbound_api_dao import ( + save_service_inbound_api, + reset_service_inbound_api, + get_service_inbound_api +) +from app.errors import ( + register_errors +) +from app.models import ( + ServiceInboundApi, +) +from app.schema_validation import validate +from app.service.service_callback_api_schema import ( + create_service_callback_api_schema, + update_service_callback_api_schema +) + +service_callback_blueprint = Blueprint('service_callback', __name__, url_prefix='/service/') + +register_errors(service_callback_blueprint) + + +@service_callback_blueprint.route('/inbound-api', methods=['POST']) +def create_service_inbound_api(service_id): + data = request.get_json() + validate(data, create_service_callback_api_schema) + data["service_id"] = service_id + inbound_api = ServiceInboundApi(**data) + try: + save_service_inbound_api(inbound_api) + except SQLAlchemyError as e: + return handle_sql_error(e) + + return jsonify(data=inbound_api.serialize()), 201 + + +@service_callback_blueprint.route('/inbound-api/', methods=['POST']) +def update_service_inbound_api(service_id, inbound_api_id): + data = request.get_json() + validate(data, update_service_callback_api_schema) + + to_update = get_service_inbound_api(inbound_api_id, service_id) + + reset_service_inbound_api(service_inbound_api=to_update, + updated_by_id=data["updated_by_id"], + url=data.get("url", None), + bearer_token=data.get("bearer_token", None)) + return jsonify(data=to_update.serialize()), 200 + + +@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 + + +def handle_sql_error(e): + if hasattr(e, 'orig') and hasattr(e.orig, 'pgerror') and e.orig.pgerror \ + and ('duplicate key value violates unique constraint "ix_service_inbound_api_service_id"' + in e.orig.pgerror): + return jsonify( + result='error', + message={'name': ["You can only have one URL and bearer token for your service."]} + ), 400 + elif hasattr(e, 'orig') and hasattr(e.orig, 'pgerror') and e.orig.pgerror \ + and ('insert or update on table "service_inbound_api" violates ' + 'foreign key constraint "service_inbound_api_service_id_fkey"' + in e.orig.pgerror): + return jsonify(result='error', message="No result found"), 404 + else: + raise e diff --git a/app/service/rest.py b/app/service/rest.py index a662b8526..efb7044a3 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -7,7 +7,6 @@ from flask import ( current_app, Blueprint ) -from sqlalchemy.exc import SQLAlchemyError from sqlalchemy.orm.exc import NoResultFound from app.dao import notifications_dao @@ -18,11 +17,6 @@ from app.dao.api_key_dao import ( get_unsigned_secret, expire_api_key) from app.dao.inbound_numbers_dao import dao_allocate_number_for_service -from app.dao.service_inbound_api_dao import ( - save_service_inbound_api, - reset_service_inbound_api, - get_service_inbound_api -) from app.dao.service_sms_sender_dao import ( dao_add_sms_sender_for_service, dao_update_service_sms_sender, @@ -72,17 +66,9 @@ from app.errors import ( InvalidRequest, register_errors ) - -from app.models import ( - Service, - ServiceInboundApi -) +from app.models import Service from app.schema_validation import validate from app.service import statistics -from app.service.service_inbound_api_schema import ( - service_inbound_api, - update_service_inbound_api_schema -) from app.service.service_senders_schema import ( add_service_email_reply_to_request, add_service_letter_contact_block_request, @@ -540,58 +526,6 @@ def get_monthly_template_usage(service_id): raise InvalidRequest('Year must be a number', status_code=400) -@service_blueprint.route('//inbound-api', methods=['POST']) -def create_service_inbound_api(service_id): - data = request.get_json() - validate(data, service_inbound_api) - data["service_id"] = service_id - inbound_api = ServiceInboundApi(**data) - try: - save_service_inbound_api(inbound_api) - except SQLAlchemyError as e: - return handle_sql_errror(e) - - return jsonify(data=inbound_api.serialize()), 201 - - -@service_blueprint.route('//inbound-api/', methods=['POST']) -def update_service_inbound_api(service_id, inbound_api_id): - data = request.get_json() - validate(data, update_service_inbound_api_schema) - - to_update = get_service_inbound_api(inbound_api_id, service_id) - - reset_service_inbound_api(service_inbound_api=to_update, - updated_by_id=data["updated_by_id"], - url=data.get("url", None), - bearer_token=data.get("bearer_token", None)) - return jsonify(data=to_update.serialize()), 200 - - -@service_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 - - -def handle_sql_errror(e): - if hasattr(e, 'orig') and hasattr(e.orig, 'pgerror') and e.orig.pgerror \ - and ('duplicate key value violates unique constraint "ix_service_inbound_api_service_id"' - in e.orig.pgerror): - return jsonify( - result='error', - message={'name': ["You can only have one URL and bearer token for your service."]} - ), 400 - elif hasattr(e, 'orig') and hasattr(e.orig, 'pgerror') and e.orig.pgerror \ - and ('insert or update on table "service_inbound_api" violates ' - 'foreign key constraint "service_inbound_api_service_id_fkey"' - in e.orig.pgerror): - return jsonify(result='error', message="No result found"), 404 - else: - raise e - - @service_blueprint.route('//send-notification', methods=['POST']) def create_one_off_notification(service_id): resp = send_one_off_notification(service_id, request.get_json()) diff --git a/app/service/service_inbound_api_schema.py b/app/service/service_callback_api_schema.py similarity index 66% rename from app/service/service_inbound_api_schema.py rename to app/service/service_callback_api_schema.py index 88978c949..138dc3cee 100644 --- a/app/service/service_inbound_api_schema.py +++ b/app/service/service_callback_api_schema.py @@ -1,10 +1,10 @@ from app.schema_validation.definitions import uuid, https_url -service_inbound_api = { +create_service_callback_api_schema = { "$schema": "http://json-schema.org/draft-04/schema#", - "description": "POST service inbound api schema", + "description": "POST service callback/inbound api schema", "type": "object", - "title": "Create service inbound api", + "title": "Create service callback/inbound api", "properties": { "url": https_url, "bearer_token": {"type": "string", "minLength": 10}, @@ -13,11 +13,11 @@ service_inbound_api = { "required": ["url", "bearer_token", "updated_by_id"] } -update_service_inbound_api_schema = { +update_service_callback_api_schema = { "$schema": "http://json-schema.org/draft-04/schema#", - "description": "POST service inbound api schema", + "description": "POST service callback/inbound api schema", "type": "object", - "title": "Create service inbound api", + "title": "Create service callback/inbound api", "properties": { "url": https_url, "bearer_token": {"type": "string", "minLength": 10}, From 8dc0455f705e4822363490a30a88e2485a9d34e2 Mon Sep 17 00:00:00 2001 From: venusbb Date: Wed, 29 Nov 2017 16:42:37 +0000 Subject: [PATCH 3/8] move inbound_api tests into test_callback_rest.py --- app/service/callback_rest.py | 10 +-- tests/app/service/test_callback_rest.py | 86 +++++++++++++++++++++++++ tests/app/service/test_rest.py | 78 ---------------------- 3 files changed, 91 insertions(+), 83 deletions(-) create mode 100644 tests/app/service/test_callback_rest.py diff --git a/app/service/callback_rest.py b/app/service/callback_rest.py index 95098e989..4c177bab4 100644 --- a/app/service/callback_rest.py +++ b/app/service/callback_rest.py @@ -5,11 +5,6 @@ from flask import ( ) from sqlalchemy.exc import SQLAlchemyError -from app.dao.service_inbound_api_dao import ( - save_service_inbound_api, - reset_service_inbound_api, - get_service_inbound_api -) from app.errors import ( register_errors ) @@ -21,6 +16,11 @@ from app.service.service_callback_api_schema import ( create_service_callback_api_schema, update_service_callback_api_schema ) +from app.dao.service_inbound_api_dao import ( + save_service_inbound_api, + get_service_inbound_api, + reset_service_inbound_api +) service_callback_blueprint = Blueprint('service_callback', __name__, url_prefix='/service/') diff --git a/tests/app/service/test_callback_rest.py b/tests/app/service/test_callback_rest.py new file mode 100644 index 000000000..0937bc9f3 --- /dev/null +++ b/tests/app/service/test_callback_rest.py @@ -0,0 +1,86 @@ +import json +import uuid + +from tests import create_authorization_header + +from tests.app.db import ( + create_service_inbound_api, +) + + +def test_create_service_inbound_api(client, 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()] + ) + assert response.status_code == 201 + + resp_json = json.loads(response.get_data(as_text=True))["data"] + assert resp_json["id"] + assert resp_json["service_id"] == str(sample_service.id) + assert resp_json["url"] == "https://some_service/inbound-sms" + assert resp_json["updated_by_id"] == str(sample_service.users[0].id) + assert resp_json["created_at"] + assert not resp_json["updated_at"] + + +def test_set_service_inbound_api_raises_404_when_service_does_not_exist(client): + 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()] + ) + assert response.status_code == 404 + assert json.loads(response.get_data(as_text=True))['message'] == 'No result found' + + +def test_update_service_inbound_api_updates_url(client, sample_service): + service_inbound_api = create_service_inbound_api(service=sample_service, + url="https://original_url.com") + + data = { + "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" + assert service_inbound_api.url == "https://another_url.com" + + +def test_update_service_inbound_api_updates_bearer_token(client, 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 + assert service_inbound_api.bearer_token == "different_token" + + +def test_fetch_service_inbound_api(client, 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() diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 109430a3c..a90278f0b 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -2225,84 +2225,6 @@ def test_search_for_notification_by_to_field_returns_content( assert notifications[0]['template']['content'] == 'Hello (( Name))\nYour thing is due soon' -def test_create_service_inbound_api(client, 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()] - ) - assert response.status_code == 201 - - resp_json = json.loads(response.get_data(as_text=True))["data"] - assert resp_json["id"] - assert resp_json["service_id"] == str(sample_service.id) - assert resp_json["url"] == "https://some_service/inbound-sms" - assert resp_json["updated_by_id"] == str(sample_service.users[0].id) - assert resp_json["created_at"] - assert not resp_json["updated_at"] - - -def test_set_service_inbound_api_raises_404_when_service_does_not_exist(client): - 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()] - ) - assert response.status_code == 404 - assert json.loads(response.get_data(as_text=True))['message'] == 'No result found' - - -def test_update_service_inbound_api_updates_url(client, sample_service): - service_inbound_api = create_service_inbound_api(service=sample_service, - url="https://original_url.com") - - data = { - "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" - assert service_inbound_api.url == "https://another_url.com" - - -def test_update_service_inbound_api_updates_bearer_token(client, 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 - assert service_inbound_api.bearer_token == "different_token" - - -def test_fetch_service_inbound_api(client, 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() - - def test_send_one_off_notification(admin_request, mocker): service = create_service() template = create_template(service=service) From a628834a2bdbe32dedd4c0eea22403264456249b Mon Sep 17 00:00:00 2001 From: venusbb Date: Wed, 29 Nov 2017 16:51:39 +0000 Subject: [PATCH 4/8] Add service callback endpoints --- app/service/callback_rest.py | 47 +++++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/app/service/callback_rest.py b/app/service/callback_rest.py index 4c177bab4..7a71c34ae 100644 --- a/app/service/callback_rest.py +++ b/app/service/callback_rest.py @@ -10,6 +10,7 @@ from app.errors import ( ) from app.models import ( ServiceInboundApi, + ServiceCallbackApi ) from app.schema_validation import validate from app.service.service_callback_api_schema import ( @@ -21,6 +22,11 @@ from app.dao.service_inbound_api_dao import ( get_service_inbound_api, reset_service_inbound_api ) +from app.dao.service_callback_api_dao import ( + save_service_callback_api, + get_service_callback_api, + reset_service_callback_api +) service_callback_blueprint = Blueprint('service_callback', __name__, url_prefix='/service/') @@ -62,17 +68,52 @@ def fetch_service_inbound_api(service_id, inbound_api_id): return jsonify(data=inbound_api.serialize()), 200 +@service_callback_blueprint.route('/service-callback-api', methods=['POST']) +def create_service_callback_api(service_id): + data = request.get_json() + validate(data, create_service_callback_api_schema) + data["service_id"] = service_id + callback_api = ServiceCallbackApi(**data) + try: + save_service_callback_api(callback_api) + except SQLAlchemyError as e: + return handle_sql_error(e) + + return jsonify(data=callback_api.serialize()), 201 + + +@service_callback_blueprint.route('/service-callback-api/', methods=['POST']) +def update_service_callback_api(service_id, callback_api_id): + data = request.get_json() + validate(data, update_service_callback_api_schema) + + to_update = get_service_callback_api(callback_api_id, service_id) + + reset_service_callback_api(service_callback_api=to_update, + updated_by_id=data["updated_by_id"], + url=data.get("url", None), + bearer_token=data.get("bearer_token", None)) + return jsonify(data=to_update.serialize()), 200 + + +@service_callback_blueprint.route('/service-callback-api/', methods=["GET"]) +def fetch_service_callback_api(service_id, callback_api_id): + callback_api = get_service_callback_api(callback_api_id, service_id) + + return jsonify(data=callback_api.serialize()), 200 + + def handle_sql_error(e): if hasattr(e, 'orig') and hasattr(e.orig, 'pgerror') and e.orig.pgerror \ - and ('duplicate key value violates unique constraint "ix_service_inbound_api_service_id"' + and ('duplicate key value violates unique constraint "ix_service_callback_api_service_id"' in e.orig.pgerror): return jsonify( result='error', message={'name': ["You can only have one URL and bearer token for your service."]} ), 400 elif hasattr(e, 'orig') and hasattr(e.orig, 'pgerror') and e.orig.pgerror \ - and ('insert or update on table "service_inbound_api" violates ' - 'foreign key constraint "service_inbound_api_service_id_fkey"' + and ('insert or update on table "service_callback_api" violates ' + 'foreign key constraint "service_callback_api_service_id_fkey"' in e.orig.pgerror): return jsonify(result='error', message="No result found"), 404 else: From 02f8ad4db2ad94b6604c6cf26d7c14e05e6ca3d1 Mon Sep 17 00:00:00 2001 From: venusbb Date: Wed, 29 Nov 2017 17:27:57 +0000 Subject: [PATCH 5/8] added test for service_callback_api rest --- app/service/callback_rest.py | 12 ++-- tests/app/service/test_callback_rest.py | 79 +++++++++++++++++++++++++ tests/app/service/test_schema.py | 15 ++--- 3 files changed, 93 insertions(+), 13 deletions(-) diff --git a/app/service/callback_rest.py b/app/service/callback_rest.py index 7a71c34ae..2b824620f 100644 --- a/app/service/callback_rest.py +++ b/app/service/callback_rest.py @@ -42,7 +42,7 @@ def create_service_inbound_api(service_id): try: save_service_inbound_api(inbound_api) except SQLAlchemyError as e: - return handle_sql_error(e) + return handle_sql_error(e, 'service_inbound_api') return jsonify(data=inbound_api.serialize()), 201 @@ -77,7 +77,7 @@ def create_service_callback_api(service_id): try: save_service_callback_api(callback_api) except SQLAlchemyError as e: - return handle_sql_error(e) + return handle_sql_error(e, 'service_callback_api') return jsonify(data=callback_api.serialize()), 201 @@ -103,17 +103,17 @@ def fetch_service_callback_api(service_id, callback_api_id): return jsonify(data=callback_api.serialize()), 200 -def handle_sql_error(e): +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_callback_api_service_id"' + and ('duplicate key value violates unique constraint "ix_{}_service_id"'.format(table_name) in e.orig.pgerror): return jsonify( result='error', message={'name': ["You can only have one URL and bearer token for your service."]} ), 400 elif hasattr(e, 'orig') and hasattr(e.orig, 'pgerror') and e.orig.pgerror \ - and ('insert or update on table "service_callback_api" violates ' - 'foreign key constraint "service_callback_api_service_id_fkey"' + and ('insert or update on table "{0}" violates ' + 'foreign key constraint "{0}_service_id_fkey"'.format(table_name) in e.orig.pgerror): return jsonify(result='error', message="No result found"), 404 else: diff --git a/tests/app/service/test_callback_rest.py b/tests/app/service/test_callback_rest.py index 0937bc9f3..f22bc0903 100644 --- a/tests/app/service/test_callback_rest.py +++ b/tests/app/service/test_callback_rest.py @@ -5,6 +5,7 @@ from tests import create_authorization_header from tests.app.db import ( create_service_inbound_api, + create_service_callback_api ) @@ -84,3 +85,81 @@ def test_fetch_service_inbound_api(client, sample_service): assert response.status_code == 200 assert json.loads(response.get_data(as_text=True))["data"] == service_inbound_api.serialize() + + +def test_create_service_callback_api(client, sample_service): + data = { + "url": "https://some_service/callback-endpoint", + "bearer_token": "some-unique-string", + "updated_by_id": str(sample_service.users[0].id) + } + response = client.post( + '/service/{}/service-callback-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"] + assert resp_json["id"] + assert resp_json["service_id"] == str(sample_service.id) + assert resp_json["url"] == "https://some_service/callback-endpoint" + assert resp_json["updated_by_id"] == str(sample_service.users[0].id) + assert resp_json["created_at"] + assert not resp_json["updated_at"] + + +def test_set_service_callback_api_raises_404_when_service_does_not_exist(client, notify_db_session): + data = { + "url": "https://some_service/service-callback-endpoint", + "bearer_token": "some-unique-string", + "updated_by_id": str(uuid.uuid4()) + } + response = client.post( + '/service/{}/service-callback-api'.format(uuid.uuid4()), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), create_authorization_header()] + ) + assert response.status_code == 404 + assert json.loads(response.get_data(as_text=True))['message'] == 'No result found' + + +def test_update_service_callback_api_updates_url(client, sample_service): + service_callback_api = create_service_callback_api(service=sample_service, + url="https://original_url.com") + + data = { + "url": "https://another_url.com", + "updated_by_id": str(sample_service.users[0].id) + } + response = client.post("/service/{}/service-callback-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" + assert service_callback_api.url == "https://another_url.com" + + +def test_update_service_callback_api_updates_bearer_token(client, 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/{}/service-callback-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 + assert service_callback_api.bearer_token == "different_token" + + +def test_fetch_service_callback_api(client, sample_service): + service_callback_api = create_service_callback_api(service=sample_service) + + response = client.get("/service/{}/service-callback-api/{}".format(sample_service.id, service_callback_api.id), + headers=[create_authorization_header()]) + + assert response.status_code == 200 + assert json.loads(response.get_data(as_text=True))["data"] == service_callback_api.serialize() diff --git a/tests/app/service/test_schema.py b/tests/app/service/test_schema.py index 23fa186de..886a030e8 100644 --- a/tests/app/service/test_schema.py +++ b/tests/app/service/test_schema.py @@ -5,41 +5,42 @@ import pytest from jsonschema import ValidationError from app.schema_validation import validate -from app.service.service_inbound_api_schema import service_inbound_api +from app.service.service_callback_api_schema import ( + update_service_callback_api_schema) -def test_service_inbound_api_schema_validates(): +def test_service_callback_api_schema_validates(): under_test = {"url": "https://some_url.for_service", "bearer_token": "something_ten_chars", "updated_by_id": str(uuid.uuid4()) } - validated = validate(under_test, service_inbound_api) + validated = validate(under_test, update_service_callback_api_schema) assert validated == under_test @pytest.mark.parametrize("url", ["not a url", "https not a url", "http://valid.com"]) -def test_service_inbound_api_schema_errors_for_url_not_valid_url(url): +def test_service_callback_api_schema_errors_for_url_not_valid_url(url): under_test = {"url": url, "bearer_token": "something_ten_chars", "updated_by_id": str(uuid.uuid4()) } with pytest.raises(ValidationError) as e: - validate(under_test, service_inbound_api) + validate(under_test, update_service_callback_api_schema) errors = json.loads(str(e.value)).get('errors') assert len(errors) == 1 assert errors[0]['message'] == "url is not a valid https url" -def test_service_inbound_api_schema_bearer_token_under_ten_char(): +def test_service_callback_api_schema_bearer_token_under_ten_char(): under_test = {"url": "https://some_url.for_service", "bearer_token": "shorty", "updated_by_id": str(uuid.uuid4()) } with pytest.raises(ValidationError) as e: - validate(under_test, service_inbound_api) + validate(under_test, update_service_callback_api_schema) errors = json.loads(str(e.value)).get('errors') assert len(errors) == 1 assert errors[0]['message'] == "bearer_token shorty is too short" From d03280776d9e3a315aebb06db0c4eff784632f0a Mon Sep 17 00:00:00 2001 From: venusbb Date: Thu, 30 Nov 2017 09:38:57 +0000 Subject: [PATCH 6/8] CodeStyle correction --- tests/app/dao/test_service_callback_api_dao.py | 2 +- tests/app/db.py | 8 ++++---- tests/app/service/test_callback_rest.py | 4 ++-- tests/app/service/test_rest.py | 1 - 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/tests/app/dao/test_service_callback_api_dao.py b/tests/app/dao/test_service_callback_api_dao.py index 4f064a8a0..884c87bac 100644 --- a/tests/app/dao/test_service_callback_api_dao.py +++ b/tests/app/dao/test_service_callback_api_dao.py @@ -70,7 +70,7 @@ def test_update_service_callback_api(sample_service): saved_callback_api = results[0] reset_service_callback_api(saved_callback_api, updated_by_id=sample_service.users[0].id, - url="https://some_service/changed_url") + url="https://some_service/changed_url") updated_results = ServiceCallbackApi.query.all() assert len(updated_results) == 1 updated = updated_results[0] diff --git a/tests/app/db.py b/tests/app/db.py index d3131e479..879d9ee39 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -308,10 +308,10 @@ def create_service_callback_api( bearer_token="some_super_secret", ): service_callback_api = ServiceCallbackApi(service_id=service.id, - url=url, - bearer_token=bearer_token, - updated_by_id=service.users[0].id - ) + url=url, + bearer_token=bearer_token, + updated_by_id=service.users[0].id + ) save_service_callback_api(service_callback_api) return service_callback_api diff --git a/tests/app/service/test_callback_rest.py b/tests/app/service/test_callback_rest.py index f22bc0903..a0e8c8722 100644 --- a/tests/app/service/test_callback_rest.py +++ b/tests/app/service/test_callback_rest.py @@ -126,7 +126,7 @@ def test_set_service_callback_api_raises_404_when_service_does_not_exist(client, def test_update_service_callback_api_updates_url(client, sample_service): service_callback_api = create_service_callback_api(service=sample_service, - url="https://original_url.com") + url="https://original_url.com") data = { "url": "https://another_url.com", @@ -143,7 +143,7 @@ def test_update_service_callback_api_updates_url(client, sample_service): def test_update_service_callback_api_updates_bearer_token(client, sample_service): service_callback_api = create_service_callback_api(service=sample_service, - bearer_token="some_super_secret") + bearer_token="some_super_secret") data = { "bearer_token": "different_token", "updated_by_id": str(sample_service.users[0].id) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 6bc8da4fc..73679001f 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -30,7 +30,6 @@ from tests.app.conftest import ( from tests.app.db import ( create_service, create_template, - create_service_inbound_api, create_notification, create_reply_to_email, create_letter_contact, From bf8fe099083303b2951b89f3a11df5eee27229a2 Mon Sep 17 00:00:00 2001 From: venusbb Date: Thu, 30 Nov 2017 11:27:07 +0000 Subject: [PATCH 7/8] changed name of endpoint from service_callback to delivery_receipt --- app/service/callback_rest.py | 6 +++--- tests/app/service/test_callback_rest.py | 16 ++++++++-------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/app/service/callback_rest.py b/app/service/callback_rest.py index 2b824620f..ca52a7814 100644 --- a/app/service/callback_rest.py +++ b/app/service/callback_rest.py @@ -68,7 +68,7 @@ def fetch_service_inbound_api(service_id, inbound_api_id): return jsonify(data=inbound_api.serialize()), 200 -@service_callback_blueprint.route('/service-callback-api', methods=['POST']) +@service_callback_blueprint.route('/delivery-receipt-api', methods=['POST']) def create_service_callback_api(service_id): data = request.get_json() validate(data, create_service_callback_api_schema) @@ -82,7 +82,7 @@ def create_service_callback_api(service_id): return jsonify(data=callback_api.serialize()), 201 -@service_callback_blueprint.route('/service-callback-api/', methods=['POST']) +@service_callback_blueprint.route('/delivery-receipt-api/', methods=['POST']) def update_service_callback_api(service_id, callback_api_id): data = request.get_json() validate(data, update_service_callback_api_schema) @@ -96,7 +96,7 @@ def update_service_callback_api(service_id, callback_api_id): return jsonify(data=to_update.serialize()), 200 -@service_callback_blueprint.route('/service-callback-api/', methods=["GET"]) +@service_callback_blueprint.route('/delivery-receipt-api/', methods=["GET"]) def fetch_service_callback_api(service_id, callback_api_id): callback_api = get_service_callback_api(callback_api_id, service_id) diff --git a/tests/app/service/test_callback_rest.py b/tests/app/service/test_callback_rest.py index a0e8c8722..bd2dcf223 100644 --- a/tests/app/service/test_callback_rest.py +++ b/tests/app/service/test_callback_rest.py @@ -89,12 +89,12 @@ def test_fetch_service_inbound_api(client, sample_service): def test_create_service_callback_api(client, sample_service): data = { - "url": "https://some_service/callback-endpoint", + "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/{}/service-callback-api'.format(sample_service.id), + '/service/{}/delivery-receipt-api'.format(sample_service.id), data=json.dumps(data), headers=[('Content-Type', 'application/json'), create_authorization_header()] ) @@ -103,7 +103,7 @@ def test_create_service_callback_api(client, sample_service): resp_json = json.loads(response.get_data(as_text=True))["data"] assert resp_json["id"] assert resp_json["service_id"] == str(sample_service.id) - assert resp_json["url"] == "https://some_service/callback-endpoint" + assert resp_json["url"] == "https://some_service/delivery-receipt-endpoint" assert resp_json["updated_by_id"] == str(sample_service.users[0].id) assert resp_json["created_at"] assert not resp_json["updated_at"] @@ -111,12 +111,12 @@ def test_create_service_callback_api(client, sample_service): def test_set_service_callback_api_raises_404_when_service_does_not_exist(client, notify_db_session): data = { - "url": "https://some_service/service-callback-endpoint", + "url": "https://some_service/delivery-receipt-endpoint", "bearer_token": "some-unique-string", "updated_by_id": str(uuid.uuid4()) } response = client.post( - '/service/{}/service-callback-api'.format(uuid.uuid4()), + '/service/{}/delivery-receipt-api'.format(uuid.uuid4()), data=json.dumps(data), headers=[('Content-Type', 'application/json'), create_authorization_header()] ) @@ -132,7 +132,7 @@ 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/{}/service-callback-api/{}".format(sample_service.id, service_callback_api.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 @@ -148,7 +148,7 @@ def test_update_service_callback_api_updates_bearer_token(client, sample_service "bearer_token": "different_token", "updated_by_id": str(sample_service.users[0].id) } - response = client.post("/service/{}/service-callback-api/{}".format(sample_service.id, service_callback_api.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 @@ -158,7 +158,7 @@ def test_update_service_callback_api_updates_bearer_token(client, sample_service def test_fetch_service_callback_api(client, sample_service): service_callback_api = create_service_callback_api(service=sample_service) - response = client.get("/service/{}/service-callback-api/{}".format(sample_service.id, service_callback_api.id), + response = client.get("/service/{}/delivery-receipt-api/{}".format(sample_service.id, service_callback_api.id), headers=[create_authorization_header()]) assert response.status_code == 200 From f32051194d40b37d0bc3c417bf98d79ee250cd74 Mon Sep 17 00:00:00 2001 From: venusbb Date: Thu, 30 Nov 2017 12:39:19 +0000 Subject: [PATCH 8/8] Bugs rectified --- app/dao/service_callback_api_dao.py | 3 +-- app/dao/service_inbound_api_dao.py | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/app/dao/service_callback_api_dao.py b/app/dao/service_callback_api_dao.py index 7db4435ba..2bc7cef76 100644 --- a/app/dao/service_callback_api_dao.py +++ b/app/dao/service_callback_api_dao.py @@ -9,8 +9,7 @@ from app.models import ServiceCallbackApi @version_class(ServiceCallbackApi) def save_service_callback_api(service_callback_api): service_callback_api.id = create_uuid() - service_callback_api.created_at == datetime.utcnow() - service_callback_api.bearer_token = service_callback_api.bearer_token + service_callback_api.created_at = datetime.utcnow() db.session.add(service_callback_api) diff --git a/app/dao/service_inbound_api_dao.py b/app/dao/service_inbound_api_dao.py index 15dad1415..8c7f2c422 100644 --- a/app/dao/service_inbound_api_dao.py +++ b/app/dao/service_inbound_api_dao.py @@ -9,8 +9,7 @@ from app.models import ServiceInboundApi @version_class(ServiceInboundApi) def save_service_inbound_api(service_inbound_api): service_inbound_api.id = create_uuid() - service_inbound_api.created_at == datetime.utcnow() - service_inbound_api.bearer_token = service_inbound_api.bearer_token + service_inbound_api.created_at = datetime.utcnow() db.session.add(service_inbound_api)