From 2d73983e8ca353f47ad25642e1e0c292d94b20d5 Mon Sep 17 00:00:00 2001 From: Jessica Tallon Date: Mon, 25 May 2015 17:22:32 +0200 Subject: [PATCH] Fix some problems with activity mixins and migrations --- mediagoblin/db/migrations.py | 6 +-- mediagoblin/db/mixin.py | 9 ++--- mediagoblin/db/models.py | 27 ++++++------- mediagoblin/tests/test_modelmethods.py | 52 -------------------------- mediagoblin/tools/federation.py | 6 +-- 5 files changed, 24 insertions(+), 76 deletions(-) diff --git a/mediagoblin/db/migrations.py b/mediagoblin/db/migrations.py index 1cc4b625..9ecd6e62 100644 --- a/mediagoblin/db/migrations.py +++ b/mediagoblin/db/migrations.py @@ -36,7 +36,7 @@ from mediagoblin.db.extratypes import JSONEncoded, MutationDict from mediagoblin.db.migration_tools import ( RegisterMigration, inspect_table, replace_table_hack) from mediagoblin.db.models import (MediaEntry, Collection, MediaComment, User, - Privilege, Generator, GenericForeignKey) + Privilege, Generator) from mediagoblin.db.extratypes import JSONEncoded, MutationDict @@ -1289,10 +1289,10 @@ def add_foreign_key_fields(db): activity_table = inspect_table(metadata, "core__activities") # Create column and add to model. - object_column = Column("temp_object", Integer, GenericForeignKey()) + object_column = Column("temp_object", Integer, ForeignKey(GenericModelReference_V0)) object_column.create(activity_table) - target_column = Column("temp_target", Integer, GenericForeignKey()) + target_column = Column("temp_target", Integer, ForeignKey(GenericModelReference_V0)) target_column.create(activity_table) # Commit this to the database diff --git a/mediagoblin/db/mixin.py b/mediagoblin/db/mixin.py index 4602c709..88df1f6b 100644 --- a/mediagoblin/db/mixin.py +++ b/mediagoblin/db/mixin.py @@ -432,13 +432,12 @@ class ActivityMixin(object): "audio": _("audio"), "person": _("a person"), } - - obj = self.get_object - target = self.get_target + obj = self.object_helper.get_object() + target = None if self.target_helper is None else self.target_helper.get_object() actor = self.get_actor content = verb_to_content.get(self.verb, None) - if content is None or obj is None: + if content is None or self.object is None: return # Decide what to fill the object with @@ -452,7 +451,7 @@ class ActivityMixin(object): # Do we want to add a target (indirect object) to content? if target is not None and "targetted" in content: if hasattr(target, "title") and target.title.strip(" "): - target_value = target.title + target_value = terget.title elif target.object_type in object_map: target_value = object_map[target.object_type] else: diff --git a/mediagoblin/db/models.py b/mediagoblin/db/models.py index 6fdf0a0f..054e1677 100644 --- a/mediagoblin/db/models.py +++ b/mediagoblin/db/models.py @@ -61,8 +61,10 @@ class GenericModelReference(Base): model_type = Column(Unicode, nullable=False) # Constrain it so obj_pk and model_type have to be unique + # They should be this order as the index is generated, "model_type" will be + # the major order as it's put first. __table_args__ = ( - UniqueConstraint("obj_pk", "model_type"), + UniqueConstraint("model_type", "obj_pk"), {}) def get_object(self): @@ -108,11 +110,11 @@ class GenericModelReference(Base): # the class so it can be shared between all instances # to prevent circular imports do import here - registry = Base._decl_class_registry + registry = dict(Base._decl_class_registry).values() self._TYPE_MAP = dict( - ((m.__tablename__, m) for m in six.itervalues(registry)) + ((m.__tablename__, m) for m in registry if hasattr(m, "__tablename__")) ) - setattr(self.__class__, "_TYPE_MAP", self._TYPE_MAP) + setattr(type(self), "_TYPE_MAP", self._TYPE_MAP) return self.__class__._TYPE_MAP[model_type] @@ -133,16 +135,16 @@ class GenericModelReference(Base): @classmethod def find_or_new(cls, obj): """ Finds an existing GMR or creates a new one for the object """ - obj = cls.find_for_obj(obj) + gmr = cls.find_for_obj(obj) # If there isn't one already create one - if obj is None: - obj = cls( + if gmr is None: + gmr = cls( obj_pk=obj.id, model_type=type(obj).__tablename__ ) - return obj + return gmr class Location(Base): """ Represents a physical location """ @@ -582,7 +584,6 @@ class MediaEntry(Base, MediaEntryMixin): return import_component(self.media_type + '.models:BACKREF_NAME') def __repr__(self): - return "" if six.PY2: # obj.__repr__() should return a str on Python 2 safe_title = self.title.encode('utf-8', 'replace') @@ -715,7 +716,7 @@ class MediaEntry(Base, MediaEntryMixin): self.license = data["license"] if "location" in data: - Licence.create(data["location"], self) + License.create(data["location"], self) return True @@ -1373,13 +1374,13 @@ class Activity(Base, ActivityMixin): # Create the generic foreign keys for the object object_id = Column(Integer, ForeignKey(GenericModelReference.id), nullable=False) - object_helper = relationship(GenericModelReference) + object_helper = relationship(GenericModelReference, foreign_keys=[object_id]) object = association_proxy("object_helper", "get_object", creator=GenericModelReference.find_or_new) # Create the generic foreign Key for the target - target_id = Column(Integer, ForeignKey(GenericModelReference), nullable=True) - target_helper = relationship(GenericModelReference) + target_id = Column(Integer, ForeignKey(GenericModelReference.id), nullable=True) + target_helper = relationship(GenericModelReference, foreign_keys=[target_id]) taget = association_proxy("target_helper", "get_target", creator=GenericModelReference.find_or_new) diff --git a/mediagoblin/tests/test_modelmethods.py b/mediagoblin/tests/test_modelmethods.py index 1ab0c476..0706aae4 100644 --- a/mediagoblin/tests/test_modelmethods.py +++ b/mediagoblin/tests/test_modelmethods.py @@ -232,55 +232,3 @@ class TestUserUrlForSelf(MGClientTestCase): self.user(u'lindsay').url_for_self(fake_urlgen()) assert excinfo.errisinstance(TypeError) assert 'object is not callable' in str(excinfo) - -class TestActivitySetGet(object): - """ Test methods on the Activity and ActivityIntermediator models """ - - @pytest.fixture(autouse=True) - def setup(self, test_app): - self.app = test_app - self.user = fixture_add_user() - self.obj = fixture_media_entry() - self.target = fixture_media_entry() - - def test_set_activity_object(self): - """ Activity.set_object should produce ActivityIntermediator """ - # The fixture will set self.obj as the object on the activity. - activity = fixture_add_activity(self.obj, actor=self.user) - - # Assert the media has been associated with an AI - assert self.obj.activity is not None - - # Assert the AI on the media and object are the same - assert activity.object == self.obj.activity - - def test_activity_set_target(self): - """ Activity.set_target should produce ActivityIntermediator """ - # This should set everything needed on the target - activity = fixture_add_activity(self.obj, actor=self.user) - activity.set_target(self.target) - - # Assert the media has been associated with the AI - assert self.target.activity is not None - - # assert the AI on the media and target are the same - assert activity.target == self.target.activity - - def test_get_activity_object(self): - """ Activity.get_object should return a set object """ - activity = fixture_add_activity(self.obj, actor=self.user) - - print("self.obj.activity = {0}".format(self.obj.activity)) - - # check we now can get the object - assert activity.get_object is not None - assert activity.get_object.id == self.obj.id - - def test_get_activity_target(self): - """ Activity.set_target should return a set target """ - activity = fixture_add_activity(self.obj, actor=self.user) - activity.set_target(self.target) - - # check we can get the target - assert activity.get_target is not None - assert activity.get_target.id == self.target.id diff --git a/mediagoblin/tools/federation.py b/mediagoblin/tools/federation.py index e8d6fc36..e7593a92 100644 --- a/mediagoblin/tools/federation.py +++ b/mediagoblin/tools/federation.py @@ -26,7 +26,7 @@ def create_generator(request): return None client = request.access_token.get_requesttoken.get_client - + # Check if there is a generator already generator = Generator.query.filter_by( name=client.application_name, @@ -40,8 +40,8 @@ def create_generator(request): generator.save() return generator - - + + def create_activity(verb, obj, actor, target=None, generator=None): """