From 5bfae689c2376a3ee3cac3d6cdd45abb811f9dd1 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Fri, 19 Feb 2016 15:53:15 +0000 Subject: [PATCH] Refactored the services dao to be a little cleaner - some things we don't need - bulk update of users - delete service Now returns None if can't find an object --- app/dao/services_dao.py | 70 +++++---- tests/app/dao/test_services_dao.py | 228 ++++++++++++++++++++--------- 2 files changed, 189 insertions(+), 109 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index c67b5b6f9..2df2ad173 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -1,48 +1,44 @@ import json -from datetime import datetime -from sqlalchemy.orm import load_only from . import DAOException from app import db -from app.models import (Service, User) +from app.models import Service +from sqlalchemy import asc -def save_model_service(service, update_dict=None): - users_list = update_dict.get('users', []) if update_dict else getattr(service, 'users', []) - if not users_list: - error_msg = {'users': ['Missing data for required attribute']} - raise DAOException(json.dumps(error_msg)) - if update_dict: - # Make sure the update_dict doesn't contain conflicting - update_dict.pop('id', None) - update_dict.pop('users', None) - # TODO optimize this algorithm - for i, x in enumerate(service.users): - if x not in users_list: - service.users.remove(x) - else: - users_list.remove(x) - for x in users_list: - service.users.append(x) - Service.query.filter_by(id=service.id).update(update_dict) - else: - db.session.add(service) +def dao_fetch_all_services(): + return Service.query.order_by(asc(Service.created_at)).all() + + +def dao_fetch_service_by_id(service_id): + return Service.query.filter_by(id=service_id).first() + + +def dao_fetch_all_services_by_user(user_id): + return Service.query.filter(Service.users.any(id=user_id)).order_by(asc(Service.created_at)).all() + + +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() + + +def dao_create_service(service, user): + service.users.append(user) + db.session.add(service) db.session.commit() -def delete_model_service(service): - db.session.delete(service) +def dao_update_service(service): + db.session.add(service) db.session.commit() -def get_model_services(service_id=None, user_id=None, _raise=True): - # TODO need better mapping from function params to sql query. - if user_id and service_id: - return Service.query.filter( - Service.users.any(id=user_id)).filter_by(id=service_id).one() - elif service_id: - result = Service.query.filter_by(id=service_id).one() if _raise else Service.query.filter_by( - id=service_id).first() - return result - elif user_id: - return Service.query.filter(Service.users.any(id=user_id)).all() - return Service.query.all() +def dao_add_user_to_service(service, user): + service.users.append(user) + db.session.add(service) + db.session.commit() + + +def dao_remove_user_from_service(service, user): + service.users.remove(user) + db.session.add(service) + db.session.commit() diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index ab5655025..9438103d1 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -1,88 +1,172 @@ +import uuid import pytest from app.dao.services_dao import ( - save_model_service, get_model_services, DAOException, delete_model_service) -from tests.app.conftest import sample_service as create_sample_service -from tests.app.conftest import sample_user as create_sample_user -from app.models import Service + dao_create_service, + dao_add_user_to_service, + dao_remove_user_from_service, + dao_fetch_all_services, + dao_fetch_service_by_id, + dao_fetch_all_services_by_user, + dao_fetch_service_by_id_and_user +) +from app.dao.users_dao import save_model_user +from app.models import Service, User +from sqlalchemy.orm.exc import FlushError +from sqlalchemy.exc import IntegrityError -def test_create_service(notify_api, notify_db, notify_db_session, sample_user): +def test_create_service(sample_user): assert Service.query.count() == 0 - service_name = 'Sample Service' - data = { - 'name': service_name, - 'users': [sample_user], - 'limit': 1000, - 'active': False, - 'restricted': False} - service = Service(**data) - save_model_service(service) + service = Service(name="service_name", email_from="email_from", limit=1000, active=True, restricted=False) + dao_create_service(service, sample_user) assert Service.query.count() == 1 - assert Service.query.first().name == service_name + assert Service.query.first().name == "service_name" assert Service.query.first().id == service.id + assert sample_user in Service.query.first().users -def test_get_services(notify_api, notify_db, notify_db_session, sample_user): - sample_service = create_sample_service(notify_db, - notify_db_session, - user=sample_user) - assert Service.query.count() == 1 - assert len(get_model_services()) == 1 - service_name = "Another service" - sample_service = create_sample_service(notify_db, - notify_db_session, - service_name=service_name, - user=sample_user) - assert Service.query.count() == 2 - assert len(get_model_services()) == 2 - - -def test_get_user_service(notify_api, notify_db, notify_db_session, sample_user): +def test_cannot_create_two_services_with_same_name(sample_user): assert Service.query.count() == 0 - service_name = "Random service" - sample_service = create_sample_service(notify_db, - notify_db_session, - service_name=service_name, - user=sample_user) - assert get_model_services(service_id=sample_service.id).name == service_name - assert Service.query.count() == 1 + service1 = Service(name="service_name", email_from="email_from1", limit=1000, active=True, restricted=False) + service2 = Service(name="service_name", email_from="email_from2", limit=1000, active=True, restricted=False) + with pytest.raises(IntegrityError) as excinfo: + dao_create_service(service1, sample_user) + dao_create_service(service2, sample_user) + assert 'duplicate key value violates unique constraint "services_name_key"' in str(excinfo.value) -def test_get_services_for_user(notify_api, notify_db, notify_db_session, sample_service): - assert Service.query.count() == 1 - service_name = "Random service" - second_user = create_sample_user(notify_db, notify_db_session, 'an@other.gov.uk') - create_sample_service(notify_db, notify_db_session, service_name='another service', user=second_user) - - sample_service = create_sample_service(notify_db, - notify_db_session, - service_name=service_name, - user=sample_service.users[0]) - assert Service.query.count() == 3 - services = get_model_services(user_id=sample_service.users[0].id) - assert len(services) == 2 - assert service_name in [x.name for x in services] - assert 'Sample service' in [x.name for x in services] - - -def test_missing_user_attribute(notify_api, notify_db, notify_db_session): +def test_cannot_create_two_services_with_same_email_from(sample_user): assert Service.query.count() == 0 - try: - service_name = 'Sample Service' - data = { - 'name': service_name, - 'limit': 1000, - 'active': False, - 'restricted': False} - - service = Service(**data) - save_model_service(service) - pytest.fail("DAOException not thrown") - except DAOException as e: - assert "Missing data for required attribute" in str(e) + service1 = Service(name="service_name1", email_from="email_from", limit=1000, active=True, restricted=False) + service2 = Service(name="service_name2", email_from="email_from", limit=1000, active=True, restricted=False) + with pytest.raises(IntegrityError) as excinfo: + dao_create_service(service1, sample_user) + dao_create_service(service2, sample_user) + assert 'duplicate key value violates unique constraint "services_email_from_key"' in str(excinfo.value) -def test_delete_service(notify_api, notify_db, notify_db_session, sample_service): - assert Service.query.count() == 1 - delete_model_service(sample_service) +def test_cannot_create_service_with_no_user(notify_db_session): assert Service.query.count() == 0 + service = Service(name="service_name", email_from="email_from", limit=1000, active=True, restricted=False) + with pytest.raises(FlushError) as excinfo: + dao_create_service(service, None) + assert "Can't flush None value found in collection Service.users" in str(excinfo.value) + + +def test_should_add_user_to_service(sample_user): + service = Service(name="service_name", email_from="email_from", limit=1000, active=True, restricted=False) + dao_create_service(service, sample_user) + assert sample_user in Service.query.first().users + new_user = User( + name='Test User', + email_address='new_user@digital.cabinet-office.gov.uk', + password='password', + mobile_number='+447700900986' + ) + save_model_user(new_user) + dao_add_user_to_service(service, new_user) + assert new_user in Service.query.first().users + + +def test_should_remove_user_from_service(sample_user): + service = Service(name="service_name", email_from="email_from", limit=1000, active=True, restricted=False) + dao_create_service(service, sample_user) + new_user = User( + name='Test User', + email_address='new_user@digital.cabinet-office.gov.uk', + password='password', + mobile_number='+447700900986' + ) + save_model_user(new_user) + dao_add_user_to_service(service, new_user) + assert new_user in Service.query.first().users + dao_remove_user_from_service(service, new_user) + assert new_user not in Service.query.first().users + + +def test_get_all_services(service_factory): + service_factory.get('service 1') + assert len(dao_fetch_all_services()) == 1 + assert dao_fetch_all_services()[0].name == 'service 1' + + service_factory.get('service 2') + assert len(dao_fetch_all_services()) == 2 + assert dao_fetch_all_services()[1].name == 'service 2' + + +def test_get_all_services_should_return_in_created_order(service_factory): + service_factory.get('service 1') + service_factory.get('service 2') + service_factory.get('service 3') + service_factory.get('service 4') + assert len(dao_fetch_all_services()) == 4 + assert dao_fetch_all_services()[0].name == 'service 1' + assert dao_fetch_all_services()[1].name == 'service 2' + assert dao_fetch_all_services()[2].name == 'service 3' + assert dao_fetch_all_services()[3].name == 'service 4' + + +def test_get_all_services_should_return_empty_list_if_no_services(): + assert len(dao_fetch_all_services()) == 0 + + +def test_get_all_services_for_user(service_factory, sample_user): + service_factory.get('service 1', sample_user) + service_factory.get('service 2', sample_user) + service_factory.get('service 3', sample_user) + assert len(dao_fetch_all_services_by_user(sample_user.id)) == 3 + assert dao_fetch_all_services_by_user(sample_user.id)[0].name == 'service 1' + assert dao_fetch_all_services_by_user(sample_user.id)[1].name == 'service 2' + assert dao_fetch_all_services_by_user(sample_user.id)[2].name == 'service 3' + + +def test_get_all_only_services_user_has_access_to(service_factory, sample_user): + service_factory.get('service 1', sample_user) + service_factory.get('service 2', sample_user) + service_3 = service_factory.get('service 3', sample_user) + new_user = User( + name='Test User', + email_address='new_user@digital.cabinet-office.gov.uk', + password='password', + mobile_number='+447700900986' + ) + save_model_user(new_user) + dao_add_user_to_service(service_3, new_user) + assert len(dao_fetch_all_services_by_user(sample_user.id)) == 3 + assert dao_fetch_all_services_by_user(sample_user.id)[0].name == 'service 1' + assert dao_fetch_all_services_by_user(sample_user.id)[1].name == 'service 2' + assert dao_fetch_all_services_by_user(sample_user.id)[2].name == 'service 3' + assert len(dao_fetch_all_services_by_user(new_user.id)) == 1 + assert dao_fetch_all_services_by_user(new_user.id)[0].name == 'service 3' + + +def test_get_all_user_services_should_return_empty_list_if_no_services_for_user(sample_user): + assert len(dao_fetch_all_services_by_user(sample_user.id)) == 0 + + +def test_get_service_by_id_returns_none_if_no_service(notify_db): + assert not dao_fetch_service_by_id(str(uuid.uuid4())) + + +def test_get_service_by_id_returns_service(service_factory): + service = service_factory.get('testing') + assert dao_fetch_service_by_id(service.id).name == 'testing' + + +def test_can_get_service_by_id_and_user(service_factory, sample_user): + service = service_factory.get('service 1', sample_user) + assert dao_fetch_service_by_id_and_user(service.id, sample_user.id).name == 'service 1' + + +def test_cannot_get_service_by_id_and_owned_by_different_user(service_factory, sample_user): + service1 = service_factory.get('service 1', sample_user) + new_user = User( + name='Test User', + email_address='new_user@digital.cabinet-office.gov.uk', + password='password', + mobile_number='+447700900986' + ) + save_model_user(new_user) + service2 = service_factory.get('service 2', new_user) + assert dao_fetch_service_by_id_and_user(service1.id, sample_user.id).name == 'service 1' + assert not dao_fetch_service_by_id_and_user(service2.id, sample_user.id)