Skip to content

Commit

Permalink
PackageExtra is no longer revisioned
Browse files Browse the repository at this point in the history
* 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)
  • Loading branch information
David Read committed Mar 15, 2019
1 parent e094531 commit 1a36dda
Show file tree
Hide file tree
Showing 7 changed files with 19 additions and 34 deletions.
8 changes: 0 additions & 8 deletions ckan/controllers/revision.py
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion ckan/model/__init__.py
Expand Up @@ -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]
)

Expand Down
5 changes: 3 additions & 2 deletions ckan/model/package.py
Expand Up @@ -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).\
Expand Down
16 changes: 7 additions & 9 deletions ckan/model/package_extra.py
Expand Up @@ -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):

Expand Down Expand Up @@ -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)

Expand Down
1 change: 0 additions & 1 deletion ckan/tests/legacy/functional/test_revision.py
Expand Up @@ -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
15 changes: 7 additions & 8 deletions ckan/tests/legacy/lib/test_dictization.py
Expand Up @@ -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',
Expand Down Expand Up @@ -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'))))
Expand Down Expand Up @@ -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'

Expand Down
6 changes: 1 addition & 5 deletions ckan/tests/legacy/models/test_package.py
Expand Up @@ -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()
Expand All @@ -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)
Expand Down

0 comments on commit 1a36dda

Please sign in to comment.