From 9ae6e14140aa90c1b15b1b47525fef402f2b85d8 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 10 Nov 2016 17:07:02 +0000 Subject: [PATCH] move deactivate functionality into one database transactions this means that any errors will cause the entire thing to roll back unfortunately, to do this we have to circumvent our regular code, which calls commit a lot, and lazily loads a lot of things, which will flush, and cause the version decorators to fail. so we have to write a lot of stuff by hand and re-select the service (even though it's already been queried) just to populate the api_keys and templates relationship on it --- app/dao/dao_utils.py | 2 -- app/dao/services_dao.py | 29 +++++++++++++++++++++++++++- app/models.py | 2 +- app/service/rest.py | 18 +++-------------- tests/app/service/test_deactivate.py | 2 +- 5 files changed, 33 insertions(+), 20 deletions(-) diff --git a/app/dao/dao_utils.py b/app/dao/dao_utils.py index bdaafc074..96628be06 100644 --- a/app/dao/dao_utils.py +++ b/app/dao/dao_utils.py @@ -9,7 +9,6 @@ def transactional(func): @wraps(func) def commit_or_rollback(*args, **kwargs): from flask import current_app - from app import db try: res = func(*args, **kwargs) db.session.commit() @@ -27,7 +26,6 @@ def version_class(model_class, history_cls=None): def versioned(func): @wraps(func) def record_version(*args, **kwargs): - from app import db func(*args, **kwargs) history_objects = [create_hist(obj) for obj in itertools.chain(db.session.new, db.session.dirty) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index dde38af32..604f0dd25 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -1,5 +1,5 @@ import uuid -from datetime import date +from datetime import date, datetime from sqlalchemy import asc, func from sqlalchemy.orm import joinedload @@ -69,6 +69,33 @@ def dao_fetch_all_services_by_user(user_id, only_active=False): return query.all() +@transactional +@version_class(Service) +@version_class(Template, TemplateHistory) +@version_class(ApiKey) +def dao_deactive_service(service_id): + # have to eager load templates and api keys so that we don't flush when we loop through them + service = Service.query.options( + joinedload('templates'), + joinedload('api_keys'), + ).filter(Service.id == service_id).one() + + service.active = False + service.name = '_archived_' + service.name + service.email_from = '_archived_' + service.email_from + + + for template in service.templates: + template.archived = True + + for api_key in service.api_keys: + api_key.expiry_date = datetime.utcnow() + + db.session.add(service) + db.session.add_all(service.templates) + db.session.add_all(service.api_keys) + + def dao_fetch_service_by_id_and_user(service_id, user_id): return Service.query.filter( Service.users.any(id=user_id), diff --git a/app/models.py b/app/models.py index 75b9615fc..2f2c6163d 100644 --- a/app/models.py +++ b/app/models.py @@ -275,7 +275,7 @@ class Template(db.Model): content = db.Column(db.Text, index=False, unique=False, nullable=False) archived = db.Column(db.Boolean, index=False, nullable=False, default=False) service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, unique=False, nullable=False) - service = db.relationship('Service', backref=db.backref('templates', lazy='dynamic')) + service = db.relationship('Service', backref='templates') subject = db.Column(db.Text, index=False, unique=False, nullable=True) created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), index=True, nullable=False) created_by = db.relationship('User') diff --git a/app/service/rest.py b/app/service/rest.py index 78138b434..312839435 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -25,16 +25,14 @@ from app.dao.services_dao import ( dao_fetch_stats_for_service, dao_fetch_todays_stats_for_service, dao_fetch_weekly_historical_stats_for_service, - dao_fetch_todays_stats_for_all_services + dao_fetch_todays_stats_for_all_services, + dao_deactive_service ) from app.dao.service_whitelist_dao import ( dao_fetch_service_whitelist, dao_add_and_commit_whitelisted_contacts, dao_remove_service_whitelist ) -from app.dao.templates_dao import ( - dao_update_template -) from app.dao import notifications_dao from app.dao.provider_statistics_dao import get_fragment_count from app.dao.users_dao import get_model_users @@ -326,17 +324,7 @@ def deactivate_service(service_id): # assume already inactive, don't change service name return '', 204 - service.active = False - service.name = '_archived_' + service.name - service.email_from = '_archived_' + service.email_from - dao_update_service(service) - - for template in service.templates: - template.archived = True - dao_update_template(template) - - for api_key in service.api_keys: - expire_api_key(service.id, api_key.id) + dao_deactive_service(service.id) return '', 204 diff --git a/tests/app/service/test_deactivate.py b/tests/app/service/test_deactivate.py index d81de0e10..df39fecac 100644 --- a/tests/app/service/test_deactivate.py +++ b/tests/app/service/test_deactivate.py @@ -57,7 +57,7 @@ def test_deactivating_service_revokes_api_keys(deactivated_service): def test_deactivating_service_archives_templates(deactivated_service): - assert deactivated_service.templates.count() == 2 + assert len(deactivated_service.templates) == 2 for template in deactivated_service.templates: assert template.archived is True assert template.version == 2