From 1a36ddab49fa72ab291c080457396272db4f083b Mon Sep 17 00:00:00 2001 From: David Read Date: Fri, 15 Mar 2019 18:16:43 +0000 Subject: [PATCH] PackageExtra is no longer revisioned * We keep the PackageExtraRevision objects BUT they don't have a foreign key constraint/property to PackageExtra any more (achieved by removing vdm.sqlalchemy.modify_base_object_mapper) * Changes to PackageExtra are no longer written to package_extra_revision table (because we removed vdm.sqlalchemy.Revisioner(extra_revision_table)) * PackageExtra no longer has revision methods like .get_as_of() and .all_revisions (because we no longer inherit from RevisionedObjectMixin) --- ckan/controllers/revision.py | 8 -------- ckan/model/__init__.py | 2 +- ckan/model/package.py | 5 +++-- ckan/model/package_extra.py | 16 +++++++--------- ckan/tests/legacy/functional/test_revision.py | 1 - ckan/tests/legacy/lib/test_dictization.py | 15 +++++++-------- ckan/tests/legacy/models/test_package.py | 6 +----- 7 files changed, 19 insertions(+), 34 deletions(-) diff --git a/ckan/controllers/revision.py b/ckan/controllers/revision.py index ace9cb32805..7b5f94043ef 100644 --- a/ckan/controllers/revision.py +++ b/ckan/controllers/revision.py @@ -65,7 +65,6 @@ def list(self): package_indications = [] revision_changes = model.repo.list_changes(revision) resource_revisions = revision_changes[model.Resource] - package_extra_revisions = revision_changes[model.PackageExtra] for package in revision.packages: if not package: # package is None sometimes - I don't know why, @@ -93,13 +92,6 @@ def list(self): if resource_revision.package_id == package.id: transition += ':resources' break - for package_extra_revision in package_extra_revisions: - if package_extra_revision.package_id == \ - package.id: - if package_extra_revision.key == \ - 'date_updated': - transition += ':date_updated' - break indication = "%s:%s" % (package.name, transition) package_indications.append(indication) pkgs = u'[%s]' % ' '.join(package_indications) diff --git a/ckan/model/__init__.py b/ckan/model/__init__.py index 714d779c868..894a85873d6 100644 --- a/ckan/model/__init__.py +++ b/ckan/model/__init__.py @@ -351,7 +351,7 @@ def purge_revision(self, revision, leave_record=False): repo = Repository(meta.metadata, meta.Session, versioned_objects=[Package, PackageTag, Resource, - PackageExtra, Member, + Member, Group, SystemInfo] ) diff --git a/ckan/model/package.py b/ckan/model/package.py index 337b33ef460..ae0012c7a67 100644 --- a/ckan/model/package.py +++ b/ckan/model/package.py @@ -412,8 +412,9 @@ def diff(self, to_revision=None, from_revision=None): results = {} # field_name:diffs results.update(super(Package, self).diff(to_revision, from_revision)) - # Iterate over PackageTag, PackageExtra, Resources etc. - for obj_class in [Resource, PackageExtra, PackageTag]: + # Iterate over PackageTag, Resources etc. (NB PackageExtra is not + # revisioned any more, so the diff is incomplete) + for obj_class in [Resource, PackageTag]: obj_rev_class = obj_class.__revision_class__ # Query for object revisions related to this package obj_rev_query = meta.Session.query(obj_rev_class).\ diff --git a/ckan/model/package_extra.py b/ckan/model/package_extra.py index 957e96ce908..b36d721fa8d 100644 --- a/ckan/model/package_extra.py +++ b/ckan/model/package_extra.py @@ -28,9 +28,10 @@ ) -extra_revision_table= core.make_revisioned_table(package_extra_table) +extra_revision_table = core.make_revisioned_table(package_extra_table) -class PackageExtra(vdm.sqlalchemy.RevisionedObjectMixin, + +class PackageExtra( core.StatefulObjectMixin, domain_object.DomainObject): @@ -61,17 +62,14 @@ def activity_stream_detail(self, activity_id, activity_type): ), }, order_by=[package_extra_table.c.package_id, package_extra_table.c.key], - extension=[vdm.sqlalchemy.Revisioner(extra_revision_table), - extension.PluginMapperExtension(), - ], + extension=[extension.PluginMapperExtension()], ) -vdm.sqlalchemy.modify_base_object_mapper(PackageExtra, core.Revision, core.State) -PackageExtraRevision= vdm.sqlalchemy.create_object_version(meta.mapper, PackageExtra, +# Keep the PackageExtraRevision table for now, but it no longer has .continuity +# foreign key constraint to PackageExtra (modify_base_object_mapper did that) +PackageExtraRevision = vdm.sqlalchemy.create_object_version(meta.mapper, PackageExtra, extra_revision_table) -PackageExtraRevision.related_packages = lambda self: [self.continuity.package] - def _create_extra(key, value): return PackageExtra(key=text_type(key), value=value) diff --git a/ckan/tests/legacy/functional/test_revision.py b/ckan/tests/legacy/functional/test_revision.py index c7ed8b752ae..cd3a7e27571 100644 --- a/ckan/tests/legacy/functional/test_revision.py +++ b/ckan/tests/legacy/functional/test_revision.py @@ -149,6 +149,5 @@ def test_list_format_atom(self): # Tests for indications about what happened. assert 'warandpeace:created' in res, res assert 'annakarenina:created' in res, res - assert 'warandpeace:updated:date_updated' in res, res assert 'annakarenina:updated:resources' in res, res assert 'annakarenina:deleted' in res, res diff --git a/ckan/tests/legacy/lib/test_dictization.py b/ckan/tests/legacy/lib/test_dictization.py index 04b3b8d370b..21387459861 100644 --- a/ckan/tests/legacy/lib/test_dictization.py +++ b/ckan/tests/legacy/lib/test_dictization.py @@ -41,10 +41,12 @@ def setup_class(cls): u'author_email': None, u'creator_user_id': None, 'extras': [ - {u'key': u'genre', - u'state': u'active', - u'value': 'romantic novel'}, - {u'key': u'original media', u'state': u'active', u'value': u'book'}], + # extras are no longer revisioned + # {u'key': u'genre', + # u'state': u'active', + # u'value': 'romantic novel'}, + # {u'key': u'original media', u'state': u'active', u'value': u'book'} + ], 'groups': [{ u'name': u'david', u'capacity': u'public', @@ -410,7 +412,6 @@ def test_13_get_package_in_past(self): second_dictized['resources'][0]['url'] = u'http://new_url2' second_dictized['tags'][0]['name'] = u'new_tag' second_dictized['tags'][0]['display_name'] = u'new_tag' - second_dictized['extras'][0]['value'] = u'new_value' second_dictized['state'] = 'active' print('\n'.join(unified_diff(pformat(second_dictized).split('\n'), pformat(third_dictized).split('\n')))) @@ -440,9 +441,7 @@ def test_13_get_package_in_past(self): third_dictized['tags'].insert(1, {'name': u'newnew_tag', 'display_name': u'newnew_tag', 'state': 'active'}) third_dictized['num_tags'] = third_dictized['num_tags'] + 1 - third_dictized['extras'].insert(0, {'key': 'david', - 'value': u'new_value', - 'state': u'active'}) + third_dictized['extras'] = [] # extras are no longer revisioned third_dictized['state'] = 'active' third_dictized['state'] = 'active' diff --git a/ckan/tests/legacy/models/test_package.py b/ckan/tests/legacy/models/test_package.py index 582c709beb5..fab18dc88af 100644 --- a/ckan/tests/legacy/models/test_package.py +++ b/ckan/tests/legacy/models/test_package.py @@ -353,7 +353,7 @@ def teardown_class(self): def test_1_all_revisions(self): assert len(self.pkg1.all_revisions) == 3, self.pkg1.all_revisions - assert len(self.pkg1.all_related_revisions) == 7, self.pkg1.all_related_revisions + assert len(self.pkg1.all_related_revisions) == 6, self.pkg1.all_related_revisions def test_2_diff(self): rev_q = model.repo.history() @@ -365,10 +365,6 @@ def test_2_diff(self): assert diff['notes'] == '- None\n+ Changed notes', diff['notes'] assert diff.get('PackageTag-geo-state') == u'- \n+ active', diff assert diff.get('PackageTag-scientific-state') == u'- \n+ active', diff - assert diff.get('PackageExtra-a-value') == u'- \n+ b', diff - assert diff.get('PackageExtra-a-state') == u'- \n+ active', diff - assert diff.get('PackageExtra-c-value') == u'- \n+ d', diff - assert diff.get('PackageExtra-c-state') == u'- \n+ active', diff def test_res(diff, res, field, expected_value): key = 'Resource-%s-%s' % (res.id[:4], field) got_value = diff.get(key)