diff --git a/ckan/lib/dictization/model_dictize.py b/ckan/lib/dictization/model_dictize.py index 0315eada73d..f7a1e145f08 100644 --- a/ckan/lib/dictization/model_dictize.py +++ b/ckan/lib/dictization/model_dictize.py @@ -228,7 +228,7 @@ def package_dictize(pkg, context): #extras - no longer revisioned, so always provide latest extra = model.package_extra_table q = select([extra]).where(extra.c.package_id == pkg.id) - result = execute(q, extra, context) + result = _execute(q, extra, context) result_dict["extras"] = extras_list_dictize(result, context) #groups diff --git a/ckan/migration/versions/088_delete_extras_which_are_deleted_state.py b/ckan/migration/versions/088_delete_extras_which_are_deleted_state.py index 99b08c6bba7..fe10df86cd9 100644 --- a/ckan/migration/versions/088_delete_extras_which_are_deleted_state.py +++ b/ckan/migration/versions/088_delete_extras_which_are_deleted_state.py @@ -6,6 +6,9 @@ def upgrade(migrate_engine): ''' ALTER TABLE "package_extra_revision" DROP CONSTRAINT package_extra_revision_continuity_id_fkey; + ALTER TABLE "group_extra_revision" + DROP CONSTRAINT group_extra_revision_continuity_id_fkey; DELETE FROM "package_extra" WHERE state='deleted'; + DELETE FROM "group_extra" WHERE state='deleted'; ''' ) diff --git a/ckan/model/__init__.py b/ckan/model/__init__.py index 280bb342dd5..c00755c0b65 100644 --- a/ckan/model/__init__.py +++ b/ckan/model/__init__.py @@ -60,7 +60,6 @@ from group_extra import ( GroupExtra, group_extra_table, - GroupExtraRevision, ) from package_extra import ( PackageExtra, diff --git a/ckan/model/group.py b/ckan/model/group.py index a27a00e1503..ee82c0d03dc 100644 --- a/ckan/model/group.py +++ b/ckan/model/group.py @@ -357,12 +357,11 @@ def all_related_revisions(self): this group. Ordered by most recent first. ''' results = {} - from group_extra import GroupExtra for grp_rev in self.all_revisions: if not grp_rev.revision in results: results[grp_rev.revision] = [] results[grp_rev.revision].append(grp_rev) - for class_ in [Member, GroupExtra]: + for class_ in [Member]: # GroupExtra is not revisioned any more rev_class = class_.__revision_class__ obj_revisions = meta.Session.query(rev_class).\ filter_by(group_id=self.id).all() diff --git a/ckan/model/group_extra.py b/ckan/model/group_extra.py index fd4fb9ba6ea..3d3fa4612b4 100644 --- a/ckan/model/group_extra.py +++ b/ckan/model/group_extra.py @@ -2,6 +2,7 @@ import vdm.sqlalchemy from sqlalchemy import orm, types, Column, Table, ForeignKey +from sqlalchemy.ext.associationproxy import association_proxy from six import text_type import group @@ -11,7 +12,7 @@ import domain_object -__all__ = ['GroupExtra', 'group_extra_table', 'GroupExtraRevision'] +__all__ = ['GroupExtra', 'group_extra_table'] group_extra_table = Table('group_extra', meta.metadata, Column('id', types.UnicodeText, primary_key=True, default=_types.make_uuid), @@ -21,11 +22,13 @@ Column('state', types.UnicodeText, default=core.State.ACTIVE), ) -group_extra_revision_table = core.make_revisioned_table(group_extra_table) +# Define the group_extra_revision table, but no need to map it, as it is only +# used by migrate_package_activity.py +group_extra_revision_table = \ + core.make_revisioned_table(group_extra_table, frozen=True) -class GroupExtra(vdm.sqlalchemy.RevisionedObjectMixin, - core.StatefulObjectMixin, +class GroupExtra(core.StatefulObjectMixin, domain_object.DomainObject): pass @@ -38,18 +41,10 @@ class GroupExtra(vdm.sqlalchemy.RevisionedObjectMixin, ) }, order_by=[group_extra_table.c.group_id, group_extra_table.c.key], - extension=[vdm.sqlalchemy.Revisioner(group_extra_revision_table),], ) -vdm.sqlalchemy.modify_base_object_mapper(GroupExtra, core.Revision, core.State) -GroupExtraRevision = vdm.sqlalchemy.create_object_version(meta.mapper, GroupExtra, - group_extra_revision_table) - def _create_extra(key, value): return GroupExtra(key=text_type(key), value=value) -_extras_active = vdm.sqlalchemy.stateful.DeferredProperty('_extras', - vdm.sqlalchemy.stateful.StatefulDict, base_modifier=lambda x: x.get_as_of()) -setattr(group.Group, 'extras_active', _extras_active) -group.Group.extras = vdm.sqlalchemy.stateful.OurAssociationProxy('extras_active', 'value', - creator=_create_extra) +group.Group.extras = association_proxy( + '_extras', 'value', creator=_create_extra) diff --git a/ckan/model/revision.py b/ckan/model/revision.py index 8e989ecc654..7ebd56f64d2 100644 --- a/ckan/model/revision.py +++ b/ckan/model/revision.py @@ -15,9 +15,9 @@ def make_revisioned_table(base_table, frozen=False): @return revision table. ''' base_table.append_column( - Column('revision_id', UnicodeText, ForeignKey('revision.id')) + Column(u'revision_id', UnicodeText, ForeignKey(u'revision.id')) ) - newtable = Table(base_table.name + '_revision', base_table.metadata) + newtable = Table(base_table.name + u'_revision', base_table.metadata) copy_table(base_table, newtable) # create foreign key 'continuity' constraint @@ -28,17 +28,17 @@ def make_revisioned_table(base_table, frozen=False): if col.primary_key: pkcols.append(col) assert len(pkcols) <= 1,\ - 'Do not support versioning objects with multiple primary keys' - fk_name = base_table.name + '.' + pkcols[0].name + u'Do not support versioning objects with multiple primary keys' + fk_name = base_table.name + u'.' + pkcols[0].name newtable.append_column( - Column('continuity_id', pkcols[0].type, + Column(u'continuity_id', pkcols[0].type, None if frozen else ForeignKey(fk_name)) ) # TODO: why do we iterate all the way through rather than just using dict # functionality ...? Surely we always have a revision here ... for col in newtable.c: - if col.name == 'revision_id': + if col.name == u'revision_id': col.primary_key = True newtable.primary_key.columns.add(col) return newtable diff --git a/ckan/tests/legacy/lib/test_dictization.py b/ckan/tests/legacy/lib/test_dictization.py index 21387459861..96bfbe5dba9 100644 --- a/ckan/tests/legacy/lib/test_dictization.py +++ b/ckan/tests/legacy/lib/test_dictization.py @@ -41,11 +41,11 @@ def setup_class(cls): u'author_email': None, u'creator_user_id': None, 'extras': [ - # 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'} + # extras are no longer revisioned so we get the latest version + {'key': u'david', 'state': u'active', 'value': u'new_value'}, + {'key': u'genre', 'state': u'active', 'value': u'new_value'}, + {'key': u'original media', 'state': u'active', + 'value': u'book'} ], 'groups': [{ u'name': u'david', @@ -441,7 +441,6 @@ 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'] = [] # extras are no longer revisioned third_dictized['state'] = 'active' third_dictized['state'] = 'active' diff --git a/ckan/tests/logic/action/test_delete.py b/ckan/tests/logic/action/test_delete.py index 1fced4211de..a4daac6fd3a 100644 --- a/ckan/tests/logic/action/test_delete.py +++ b/ckan/tests/logic/action/test_delete.py @@ -242,8 +242,7 @@ def test_purged_group_leaves_no_trace_in_the_model(self): assert_equals(sorted( [gr.name for gr in model.Session.query(model.GroupRevision)]), ['child', 'parent']) - assert_equals(model.Session.query(model.GroupExtraRevision).all(), - []) + # GroupExtra is not revisioned # Member is not revisioned # No Revision objects were purged, in fact 1 is created for the purge @@ -345,8 +344,7 @@ def test_purged_organization_leaves_no_trace_in_the_model(self): assert_equals(sorted( [gr.name for gr in model.Session.query(model.GroupRevision)]), ['child', 'parent']) - assert_equals(model.Session.query(model.GroupExtraRevision).all(), - []) + # GroupExtra is not revisioned # Member is not revisioned # No Revision objects were purged, in fact 1 is created for the purge