diff --git a/ckan/lib/dictization/model_save.py b/ckan/lib/dictization/model_save.py index 1df8b662584..b292b4bab51 100644 --- a/ckan/lib/dictization/model_save.py +++ b/ckan/lib/dictization/model_save.py @@ -96,16 +96,14 @@ def package_resource_list_save(res_dicts, package, context): resource_list.append(resource) -def package_extras_save(extra_dicts, obj, context): +def package_extras_save(extra_dicts, pkg, context): allow_partial_update = context.get("allow_partial_update", False) if extra_dicts is None and allow_partial_update: return - model = context["model"] session = context["session"] - extras_list = obj.extras_list - old_extras = dict((extra.key, extra) for extra in extras_list) + old_extras = pkg._extras new_extras = {} for extra_dict in extra_dicts or []: @@ -116,28 +114,22 @@ def package_extras_save(extra_dicts, obj, context): pass else: new_extras[extra_dict["key"]] = extra_dict["value"] + #new for key in set(new_extras.keys()) - set(old_extras.keys()): - state = 'active' - extra = model.PackageExtra(state=state, key=key, value=new_extras[key]) - session.add(extra) - extras_list.append(extra) + pkg.extras[key] = new_extras[key] #changed for key in set(new_extras.keys()) & set(old_extras.keys()): extra = old_extras[key] - if new_extras[key] == extra.value and extra.state != 'deleted': + if new_extras[key] == extra.value: continue - state = 'active' extra.value = new_extras[key] - extra.state = state session.add(extra) #deleted for key in set(old_extras.keys()) - set(new_extras.keys()): extra = old_extras[key] - if extra.state == 'deleted': - continue - state = 'deleted' - extra.state = state + extra.delete() + def package_tag_list_save(tag_dicts, package, context): allow_partial_update = context.get("allow_partial_update", False) @@ -305,7 +297,7 @@ def package_dict_save(pkg_dict, context): objects = pkg_dict.get('relationships_as_object') relationship_list_save(objects, pkg, 'relationships_as_object', context) - extras = package_extras_save(pkg_dict.get("extras"), pkg, context) + package_extras_save(pkg_dict.get("extras"), pkg, context) return pkg diff --git a/ckan/migration/versions/077_add_revisions_to_system_info.py b/ckan/migration/versions/077_add_revisions_to_system_info.py index 9516a9127c0..daa979164a2 100644 --- a/ckan/migration/versions/077_add_revisions_to_system_info.py +++ b/ckan/migration/versions/077_add_revisions_to_system_info.py @@ -1,6 +1,6 @@ # encoding: utf-8 -import vdm.sqlalchemy +from ckan import model def upgrade(migrate_engine): @@ -21,5 +21,5 @@ def upgrade(migrate_engine): ALTER TABLE system_info_revision DROP CONSTRAINT "system_info_revision_key_key"; - '''.format(state=vdm.sqlalchemy.State.ACTIVE) + '''.format(state=model.State.ACTIVE) ) 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 new file mode 100644 index 00000000000..7cf4a2c3752 --- /dev/null +++ b/ckan/migration/versions/088_delete_extras_which_are_deleted_state.py @@ -0,0 +1,14 @@ +# encoding: utf-8 + + +def upgrade(migrate_engine): + migrate_engine.execute( + ''' +ALTER TABLE "package_extra_revision" + DROP CONSTRAINT IF EXISTS package_extra_revision_continuity_id_fkey; +ALTER TABLE "group_extra_revision" + DROP CONSTRAINT IF EXISTS group_extra_revision_continuity_id_fkey; +DELETE FROM "package_extra" WHERE state='deleted'; +DELETE FROM "group_extra" WHERE state='deleted'; + ''' + ) diff --git a/ckan/migration/versions/088_package_activity_migration_check.py b/ckan/migration/versions/089_package_activity_migration_check.py similarity index 100% rename from ckan/migration/versions/088_package_activity_migration_check.py rename to ckan/migration/versions/089_package_activity_migration_check.py diff --git a/ckan/model/core.py b/ckan/model/core.py index 767dd851d8c..596f86d5099 100644 --- a/ckan/model/core.py +++ b/ckan/model/core.py @@ -2,13 +2,14 @@ import datetime +from sqlalchemy import Column, DateTime, Text, Boolean + import domain_object import meta -import vdm.sqlalchemy -from sqlalchemy import Column, DateTime, Text, Boolean -__all__ = ['System', 'State'] +__all__ = ['System', 'State', 'StatefulObjectMixin'] +log = __import__('logging').getLogger(__name__) class System(domain_object.DomainObject): @@ -27,5 +28,22 @@ def by_name(cls, name): # VDM-specific domain objects -State = vdm.sqlalchemy.State -State.all = [State.ACTIVE, State.DELETED] +class State(object): + ACTIVE = u'active' + DELETED = u'deleted' + PENDING = u'pending' + + +class StatefulObjectMixin(object): + __stateful__ = True + + def delete(self): + log.debug('Running delete on %s', self) + self.state = State.DELETED + + def undelete(self): + self.state = State.ACTIVE + + def is_active(self): + # also support None in case this object is not yet refreshed ... + return self.state is None or self.state == State.ACTIVE diff --git a/ckan/model/domain_object.py b/ckan/model/domain_object.py index 9761042a553..c7b5ae1cf16 100644 --- a/ckan/model/domain_object.py +++ b/ckan/model/domain_object.py @@ -79,7 +79,7 @@ def remove(self): def delete(self): # stateful objects have this method overridden - see - # vmd.base.StatefulObjectMixin + # core.StatefulObjectMixin self.Session.delete(self) def purge(self): diff --git a/ckan/model/group.py b/ckan/model/group.py index b3459ae022f..fcb1e33ea89 100644 --- a/ckan/model/group.py +++ b/ckan/model/group.py @@ -27,9 +27,11 @@ Column('capacity', types.UnicodeText, nullable=False), Column('group_id', types.UnicodeText, - ForeignKey('group.id')),) + ForeignKey('group.id')), + Column('state', types.UnicodeText, + default=core.State.ACTIVE), + ) -vdm.sqlalchemy.make_table_stateful(member_table) group_table = Table('group', meta.metadata, Column('id', types.UnicodeText, @@ -46,12 +48,13 @@ default=datetime.datetime.now), Column('is_organization', types.Boolean, default=False), Column('approval_status', types.UnicodeText, - default=u"approved")) + default=u"approved"), + Column('state', types.UnicodeText, + default=core.State.ACTIVE), + ) -vdm.sqlalchemy.make_table_stateful(group_table) - -class Member(vdm.sqlalchemy.StatefulObjectMixin, +class Member(core.StatefulObjectMixin, domain_object.DomainObject): '''A Member object represents any other object being a 'member' of a particular Group. @@ -108,7 +111,7 @@ def __unicode__(self): table_info, self.capacity, self.state) -class Group(vdm.sqlalchemy.StatefulObjectMixin, +class Group(core.StatefulObjectMixin, domain_object.DomainObject): def __init__(self, name=u'', title=u'', description=u'', image_url=u'', diff --git a/ckan/model/group_extra.py b/ckan/model/group_extra.py index 6895125d865..989b9b5f37b 100644 --- a/ckan/model/group_extra.py +++ b/ckan/model/group_extra.py @@ -1,8 +1,8 @@ # encoding: utf-8 import vdm.sqlalchemy -import vdm.sqlalchemy.stateful from sqlalchemy import orm, types, Column, Table, ForeignKey +from sqlalchemy.ext.associationproxy import association_proxy from six import text_type import group @@ -19,14 +19,12 @@ Column('group_id', types.UnicodeText, ForeignKey('group.id')), Column('key', types.UnicodeText), Column('value', types.UnicodeText), + Column('state', types.UnicodeText, default=core.State.ACTIVE), ) -vdm.sqlalchemy.make_table_stateful(group_extra_table) - -class GroupExtra( - vdm.sqlalchemy.StatefulObjectMixin, - domain_object.DomainObject): +class GroupExtra(core.StatefulObjectMixin, + domain_object.DomainObject): pass meta.mapper(GroupExtra, group_extra_table, properties={ @@ -40,12 +38,8 @@ class GroupExtra( order_by=[group_extra_table.c.group_id, group_extra_table.c.key], ) - def _create_extra(key, value): return GroupExtra(key=text_type(key), value=value) -_extras_active = vdm.sqlalchemy.stateful.DeferredProperty('_extras', - vdm.sqlalchemy.stateful.StatefulDict) -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/package.py b/ckan/model/package.py index 0c2189ecfff..fe91b8d4911 100644 --- a/ckan/model/package.py +++ b/ckan/model/package.py @@ -7,7 +7,6 @@ from sqlalchemy import orm from sqlalchemy import types, Column, Table from ckan.common import config -import vdm.sqlalchemy import meta import core @@ -52,17 +51,15 @@ Column('metadata_created', types.DateTime, default=datetime.datetime.utcnow), Column('metadata_modified', types.DateTime, default=datetime.datetime.utcnow), Column('private', types.Boolean, default=False), + Column('state', types.UnicodeText, default=core.State.ACTIVE), ) -vdm.sqlalchemy.make_table_stateful(package_table) - ## ------------------- ## Mapped classes -class Package( - vdm.sqlalchemy.StatefulObjectMixin, - domain_object.DomainObject): +class Package(core.StatefulObjectMixin, + domain_object.DomainObject): text_search_fields = ['name', 'title'] diff --git a/ckan/model/package_extra.py b/ckan/model/package_extra.py index 3bc70399f0d..81c95dad2ab 100644 --- a/ckan/model/package_extra.py +++ b/ckan/model/package_extra.py @@ -2,8 +2,8 @@ from six import text_type import vdm.sqlalchemy -import vdm.sqlalchemy.stateful from sqlalchemy import orm, types, Column, Table, ForeignKey +from sqlalchemy.ext.associationproxy import association_proxy import meta import core @@ -22,12 +22,12 @@ Column('package_id', types.UnicodeText, ForeignKey('package.id')), Column('key', types.UnicodeText), Column('value', types.UnicodeText), + Column('state', types.UnicodeText, default=core.State.ACTIVE), ) -vdm.sqlalchemy.make_table_stateful(package_extra_table) class PackageExtra( - vdm.sqlalchemy.StatefulObjectMixin, + core.StatefulObjectMixin, domain_object.DomainObject): def related_packages(self): @@ -41,11 +41,6 @@ def related_packages(self): cascade='all, delete, delete-orphan', ), ), - 'package_no_state': orm.relation(_package.Package, - backref=orm.backref('extras_list', - cascade='all, delete, delete-orphan', - ), - ) }, order_by=[package_extra_table.c.package_id, package_extra_table.c.key], extension=[extension.PluginMapperExtension()], @@ -55,8 +50,5 @@ def related_packages(self): def _create_extra(key, value): return PackageExtra(key=text_type(key), value=value) -_extras_active = vdm.sqlalchemy.stateful.DeferredProperty('_extras', - vdm.sqlalchemy.stateful.StatefulDict) -setattr(_package.Package, 'extras_active', _extras_active) -_package.Package.extras = vdm.sqlalchemy.stateful.OurAssociationProxy('extras_active', 'value', - creator=_create_extra) +_package.Package.extras = association_proxy( + '_extras', 'value', creator=_create_extra) diff --git a/ckan/model/package_relationship.py b/ckan/model/package_relationship.py index 386e19cc300..ad9fec017e1 100644 --- a/ckan/model/package_relationship.py +++ b/ckan/model/package_relationship.py @@ -27,11 +27,11 @@ def _(txt): Column('object_package_id', types.UnicodeText, ForeignKey('package.id')), Column('type', types.UnicodeText), Column('comment', types.UnicodeText), + Column('state', types.UnicodeText, default=core.State.ACTIVE), ) -vdm.sqlalchemy.make_table_stateful(package_relationship_table) -class PackageRelationship(vdm.sqlalchemy.StatefulObjectMixin, +class PackageRelationship(core.StatefulObjectMixin, domain_object.DomainObject): '''The rule with PackageRelationships is that they are stored in the model always as the "forward" relationship - i.e. "child_of" but never diff --git a/ckan/model/resource.py b/ckan/model/resource.py index 3b977bdec42..f7a0ab4c65f 100644 --- a/ckan/model/resource.py +++ b/ckan/model/resource.py @@ -8,7 +8,6 @@ from sqlalchemy import orm from ckan.common import config import vdm.sqlalchemy -import vdm.sqlalchemy.stateful from sqlalchemy import types, func, Column, Table, ForeignKey, and_ import meta @@ -53,12 +52,11 @@ Column('cache_last_updated', types.DateTime), Column('url_type', types.UnicodeText), Column('extras', _types.JsonDictType), + Column('state', types.UnicodeText, default=core.State.ACTIVE), ) -vdm.sqlalchemy.make_table_stateful(resource_table) - -class Resource(vdm.sqlalchemy.StatefulObjectMixin, +class Resource(core.StatefulObjectMixin, domain_object.DomainObject): extra_columns = None diff --git a/ckan/model/system_info.py b/ckan/model/system_info.py index 2941aa479ee..2c282172d8d 100644 --- a/ckan/model/system_info.py +++ b/ckan/model/system_info.py @@ -23,12 +23,11 @@ Column('id', types.Integer(), primary_key=True, nullable=False), Column('key', types.Unicode(100), unique=True, nullable=False), Column('value', types.UnicodeText), + Column('state', types.UnicodeText, default=core.State.ACTIVE), ) -vdm.sqlalchemy.make_table_stateful(system_info_table) - -class SystemInfo(vdm.sqlalchemy.StatefulObjectMixin, +class SystemInfo(core.StatefulObjectMixin, domain_object.DomainObject): def __init__(self, key, value): diff --git a/ckan/model/tag.py b/ckan/model/tag.py index 60e1c9c637b..736daf3df11 100644 --- a/ckan/model/tag.py +++ b/ckan/model/tag.py @@ -34,10 +34,9 @@ Column('id', types.UnicodeText, primary_key=True, default=_types.make_uuid), Column('package_id', types.UnicodeText, ForeignKey('package.id')), Column('tag_id', types.UnicodeText, ForeignKey('tag.id')), + Column('state', types.UnicodeText, default=core.State.ACTIVE), ) -vdm.sqlalchemy.make_table_stateful(package_tag_table) - class Tag(domain_object.DomainObject): def __init__(self, name='', vocabulary_id=None): @@ -215,9 +214,8 @@ def packages(self): def __repr__(self): return '' % self.name -class PackageTag( - vdm.sqlalchemy.StatefulObjectMixin, - domain_object.DomainObject): +class PackageTag(core.StatefulObjectMixin, + domain_object.DomainObject): def __init__(self, package=None, tag=None, state=None, **kwargs): self.package = package self.tag = tag diff --git a/ckan/model/user.py b/ckan/model/user.py index 2daced5dcb3..a02885a1218 100644 --- a/ckan/model/user.py +++ b/ckan/model/user.py @@ -11,7 +11,6 @@ from sqlalchemy.orm import synonym from sqlalchemy import types, Column, Table, func from six import text_type -import vdm.sqlalchemy import meta import core @@ -33,12 +32,11 @@ Column('activity_streams_email_notifications', types.Boolean, default=False), Column('sysadmin', types.Boolean, default=False), + Column('state', types.UnicodeText, default=core.State.ACTIVE), ) -vdm.sqlalchemy.make_table_stateful(user_table) - -class User(vdm.sqlalchemy.StatefulObjectMixin, +class User(core.StatefulObjectMixin, domain_object.DomainObject): VALID_NAME = re.compile(r"^[a-zA-Z0-9_\-]{3,255}$") diff --git a/ckan/tests/legacy/lib/test_dictization.py b/ckan/tests/legacy/lib/test_dictization.py index ce2993ac74d..08dfe296877 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 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', u'capacity': u'public', diff --git a/ckan/tests/legacy/models/test_extras.py b/ckan/tests/legacy/models/test_extras.py index 226376e518c..0afe9475e3d 100644 --- a/ckan/tests/legacy/models/test_extras.py +++ b/ckan/tests/legacy/models/test_extras.py @@ -19,7 +19,7 @@ def test_1(self): assert pkg is not None pkg._extras[u'country'] = model.PackageExtra(key=u'country', value='us') - pkg.extras_active[u'xxx'] = model.PackageExtra(key=u'xxx', value='yyy') + pkg.extras[u'xxx'] = u'yyy' pkg.extras[u'format'] = u'rdf' model.repo.commit_and_remove() @@ -27,35 +27,18 @@ def test_1(self): rev1 = model.repo.youngest_revision().id samepkg = model.Package.by_name(u'warandpeace') assert len(samepkg._extras) == 3, samepkg._extras - assert samepkg.extras_active[u'country'].value == 'us', samepkg.extras_active assert samepkg.extras[u'country'] == 'us' assert samepkg.extras[u'format'] == 'rdf' model.Session.remove() # now delete and extras - samepkg = model.Package.by_name(u'warandpeace')() + samepkg = model.Package.by_name(u'warandpeace') del samepkg.extras[u'country'] model.repo.commit_and_remove() samepkg = model.Package.by_name(u'warandpeace') - assert len(samepkg._extras) == 3 + assert len(samepkg._extras) == 2 assert len(samepkg.extras) == 2 extra = model.Session.query(model.PackageExtra).filter_by(key=u'country').first() - assert extra and extra.state == model.State.DELETED, extra + assert not extra, extra model.Session.remove() - - samepkg = model.Package.by_name(u'warandpeace') - samepkg.get_as_of(model.Session.query(model.Revision).get(rev1)) - assert len(samepkg.extras) == 3, len(samepkg.extras) - model.Session.remove() - - # now restore it ...() - samepkg = model.Package.by_name(u'warandpeace') - samepkg.extras[u'country'] = 'uk' - model.repo.commit_and_remove() - - samepkg = model.Package.by_name(u'warandpeace') - assert len(samepkg.extras) == 3 - assert len(samepkg._extras) == 3 - assert samepkg.extras[u'country'] == 'uk' - diff --git a/ckan/tests/logic/action/test_delete.py b/ckan/tests/logic/action/test_delete.py index a3527d2dfc3..aeea79b5394 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