From 049514d4b2f02f8a5f191bd1c62229721ff20f9d Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 2 Aug 2016 16:23:14 +0100 Subject: [PATCH] remove history-meta for templates, replace with hand-made history table history-meta's dynamic magic is insufficient for templates, where we need to be able to refer to the specific history table to take advantage of sqlalchemy's relationship management (2/3rds of an ORM). So replace it with a custom made version table. Had to change the version decorator slightly for this --- app/dao/dao_utils.py | 13 ++++++++----- app/dao/services_dao.py | 3 ++- app/dao/templates_dao.py | 21 ++++++++++++--------- app/history_meta.py | 14 +++++--------- app/models.py | 22 +++++++++++++++++++++- app/service/rest.py | 2 +- tests/app/dao/test_services_dao.py | 2 +- tests/app/dao/test_templates_dao.py | 14 +++++++------- 8 files changed, 57 insertions(+), 34 deletions(-) diff --git a/app/dao/dao_utils.py b/app/dao/dao_utils.py index f5179e8b5..d5ec27fb5 100644 --- a/app/dao/dao_utils.py +++ b/app/dao/dao_utils.py @@ -1,6 +1,7 @@ import itertools -from functools import wraps -from app.history_meta import versioned_objects, create_history +from functools import wraps, partial + +from app.history_meta import create_history def transactional(func): @@ -19,14 +20,16 @@ def transactional(func): return commit_or_rollback -def version_class(model_class): +def version_class(model_class, history_cls=None): + create_hist = partial(create_history, history_cls=history_cls) + def versioned(func): @wraps(func) def record_version(*args, **kwargs): from app import db func(*args, **kwargs) - history_objects = [create_history(obj) for obj in - versioned_objects(itertools.chain(db.session.new, db.session.dirty)) + history_objects = [create_hist(obj) for obj in + itertools.chain(db.session.new, db.session.dirty) if isinstance(obj, model_class)] for h_obj in history_objects: db.session.add(h_obj) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 5bd2e975a..eff4679dd 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -16,6 +16,7 @@ from app.models import ( VerifyCode, ApiKey, Template, + TemplateHistory, Job, NotificationHistory, Notification, @@ -122,7 +123,7 @@ def delete_service_and_all_associated_db_objects(service): _delete_commit(Notification.query.filter_by(service=service)) _delete_commit(Job.query.filter_by(service=service)) _delete_commit(Template.query.filter_by(service=service)) - _delete_commit(Template.get_history_model().query.filter_by(service_id=service.id)) + _delete_commit(TemplateHistory.query.filter_by(service_id=service.id)) verify_codes = VerifyCode.query.join(User).filter(User.id.in_([x.id for x in service.users])) list(map(db.session.delete, verify_codes)) diff --git a/app/dao/templates_dao.py b/app/dao/templates_dao.py index d69b530e0..f7f4c5e4d 100644 --- a/app/dao/templates_dao.py +++ b/app/dao/templates_dao.py @@ -1,8 +1,9 @@ import uuid -from app import db -from app.models import (Template, Service) + from sqlalchemy import (asc, desc) +from app import db +from app.models import (Template, TemplateHistory) from app.dao.dao_utils import ( transactional, version_class @@ -10,7 +11,7 @@ from app.dao.dao_utils import ( @transactional -@version_class(Template) +@version_class(Template, TemplateHistory) def dao_create_template(template): template.id = uuid.uuid4() # must be set now so version history model can use same id template.archived = False @@ -18,14 +19,14 @@ def dao_create_template(template): @transactional -@version_class(Template) +@version_class(Template, TemplateHistory) def dao_update_template(template): db.session.add(template) def dao_get_template_by_id_and_service_id(template_id, service_id, version=None): if version is not None: - return Template.get_history_model().query.filter_by( + return TemplateHistory.query.filter_by( id=template_id, service_id=service_id, version=version).one() @@ -34,7 +35,7 @@ def dao_get_template_by_id_and_service_id(template_id, service_id, version=None) def dao_get_template_by_id(template_id, version=None): if version is not None: - return Template.get_history_model().query.filter_by( + return TemplateHistory.query.filter_by( id=template_id, version=version).one() return Template.query.filter_by(id=template_id).one() @@ -50,6 +51,8 @@ def dao_get_all_templates_for_service(service_id): def dao_get_template_versions(service_id, template_id): - history_model = Template.get_history_model() - return history_model.query.filter_by(service_id=service_id, id=template_id).order_by( - desc(history_model.version)).all() + return TemplateHistory.query.filter_by( + service_id=service_id, id=template_id + ).order_by( + desc(TemplateHistory.version) + ).all() diff --git a/app/history_meta.py b/app/history_meta.py index 9cbbe07f1..49a420572 100644 --- a/app/history_meta.py +++ b/app/history_meta.py @@ -171,17 +171,13 @@ class Versioned(object): return history_mapper.class_ -def versioned_objects(iter): - for obj in iter: - if hasattr(obj, '__history_mapper__'): - yield obj +def create_history(obj, history_cls=None): + if not history_cls: + history_mapper = obj.__history_mapper__ + history_cls = history_mapper.class_ - -def create_history(obj): - obj_mapper = object_mapper(obj) - history_mapper = obj.__history_mapper__ - history_cls = history_mapper.class_ history = history_cls() + obj_mapper = object_mapper(obj) obj_state = attributes.instance_state(obj) data = {} diff --git a/app/models.py b/app/models.py index e3aeb3d13..09fcaf05a 100644 --- a/app/models.py +++ b/app/models.py @@ -206,7 +206,7 @@ TEMPLATE_TYPES = [SMS_TYPE, EMAIL_TYPE, LETTER_TYPE] template_types = db.Enum(*TEMPLATE_TYPES, name='template_type') -class Template(db.Model, Versioned): +class Template(db.Model): __tablename__ = 'templates' id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) @@ -231,6 +231,26 @@ class Template(db.Model, Versioned): 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') + version = db.Column(db.Integer, default=1) + + +class TemplateHistory(db.Model): + __tablename__ = 'templates_history' + + id = db.Column(UUID(as_uuid=True), primary_key=True) + name = db.Column(db.String(255), nullable=False) + template_type = db.Column(template_types, nullable=False) + created_at = db.Column(db.DateTime, nullable=False) + updated_at = db.Column(db.DateTime) + content = db.Column(db.Text, nullable=False) + archived = db.Column(db.Boolean, nullable=False, default=False) + service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, nullable=False) + service = db.relationship('Service') + subject = db.Column(db.Text) + created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), index=True, nullable=False) + created_by = db.relationship('User') + version = db.Column(db.Integer) + MMG_PROVIDER = "mmg" FIRETEXT_PROVIDER = "firetext" diff --git a/app/service/rest.py b/app/service/rest.py index bf84fb859..776ab6400 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -189,7 +189,7 @@ def get_service_history(service_id): api_key_history = ApiKey.get_history_model().query.filter_by(service_id=service_id).all() api_keys_data = api_key_history_schema.dump(api_key_history, many=True).data - template_history = Template.get_history_model().query.filter_by(service_id=service_id).all() + template_history = TemplateHistory.query.filter_by(service_id=service_id).all() template_data, errors = template_history_schema.dump(template_history, many=True) events = Event.query.all() diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 1af238b4d..26494dda9 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -331,7 +331,7 @@ def test_delete_service_and_associated_objects(notify_db, assert ApiKey.query.count() == 0 assert ApiKey.get_history_model().query.count() == 0 assert Template.query.count() == 0 - assert Template.get_history_model().query.count() == 0 + assert TemplateHistory.query.count() == 0 assert Job.query.count() == 0 assert Notification.query.count() == 0 assert Permission.query.count() == 0 diff --git a/tests/app/dao/test_templates_dao.py b/tests/app/dao/test_templates_dao.py index 82b943963..b2a29e1b5 100644 --- a/tests/app/dao/test_templates_dao.py +++ b/tests/app/dao/test_templates_dao.py @@ -185,7 +185,7 @@ def test_get_template_by_id_and_service_returns_none_if_no_template(sample_servi def test_create_template_creates_a_history_record_with_current_data(sample_service, sample_user): assert Template.query.count() == 0 - assert Template.get_history_model().query.count() == 0 + assert TemplateHistory.query.count() == 0 data = { 'name': 'Sample Template', 'template_type': "email", @@ -200,7 +200,7 @@ def test_create_template_creates_a_history_record_with_current_data(sample_servi assert Template.query.count() == 1 template_from_db = Template.query.first() - template_history = Template.get_history_model().query.first() + template_history = TemplateHistory.query.first() assert template_from_db.id == template_history.id assert template_from_db.name == template_history.name @@ -212,7 +212,7 @@ def test_create_template_creates_a_history_record_with_current_data(sample_servi def test_update_template_creates_a_history_record_with_current_data(sample_service, sample_user): assert Template.query.count() == 0 - assert Template.get_history_model().query.count() == 0 + assert TemplateHistory.query.count() == 0 data = { 'name': 'Sample Template', 'template_type': "email", @@ -228,20 +228,20 @@ def test_update_template_creates_a_history_record_with_current_data(sample_servi assert created.name == 'Sample Template' assert Template.query.count() == 1 assert Template.query.first().version == 1 - assert Template.get_history_model().query.count() == 1 + assert TemplateHistory.query.count() == 1 created.name = 'new name' dao_update_template(created) assert Template.query.count() == 1 - assert Template.get_history_model().query.count() == 2 + assert TemplateHistory.query.count() == 2 template_from_db = Template.query.first() assert template_from_db.version == 2 - assert Template.get_history_model().query.filter_by(name='Sample Template').one().version == 1 - assert Template.get_history_model().query.filter_by(name='new name').one().version == 2 + assert TemplateHistory.query.filter_by(name='Sample Template').one().version == 1 + assert TemplateHistory.query.filter_by(name='new name').one().version == 2 def test_get_template_history_version(sample_user, sample_service, sample_template):