From 6194344bf9a27d580f21b061a6e4b7c3d17ec4a9 Mon Sep 17 00:00:00 2001 From: Elrond Date: Tue, 22 Jan 2013 22:00:41 +0100 Subject: Use better relationships to delete collections. When deleting a User, his/her collections can be deleted by sqlalchemy: Collections do not need any special code to be executed on deletion. --- mediagoblin/db/models.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'mediagoblin/db/models.py') diff --git a/mediagoblin/db/models.py b/mediagoblin/db/models.py index 7e2cc7d2..de491e96 100644 --- a/mediagoblin/db/models.py +++ b/mediagoblin/db/models.py @@ -84,9 +84,7 @@ class User(Base, UserMixin): def delete(self, **kwargs): """Deletes a User and all related entries/comments/files/...""" - # Delete this user's Collections and all contained CollectionItems - for collection in self.collections: - collection.delete(commit=False) + # Collections get deleted by relationships. media_entries = MediaEntry.query.filter(MediaEntry.uploader == self.id) for media in media_entries: @@ -415,7 +413,10 @@ class Collection(Base, CollectionMixin): # TODO: No of items in Collection. Badly named, can we migrate to num_items? items = Column(Integer, default=0) - get_creator = relationship(User, backref="collections") + # Cascade: Collections are owned by their creator. So do the full thing. + get_creator = relationship(User, + backref=backref("collections", + cascade="all, delete-orphan")) def get_collection_items(self, ascending=False): #TODO, is this still needed with self.collection_items being available? @@ -436,7 +437,9 @@ class CollectionItem(Base, CollectionItemMixin): note = Column(UnicodeText, nullable=True) added = Column(DateTime, nullable=False, default=datetime.datetime.now) position = Column(Integer) - in_collection = relationship("Collection", + + # Cascade: CollectionItems are owned by their Collection. So do the full thing. + in_collection = relationship(Collection, backref=backref( "collection_items", cascade="all, delete-orphan")) -- cgit v1.2.3 From ff68ca9fc2b329d1c2ec395abf50358845c4e5fc Mon Sep 17 00:00:00 2001 From: Elrond Date: Tue, 29 Jan 2013 21:23:21 +0100 Subject: Fix issue 611: Proper (back)relationship on MediaComment. well, fix the relationship on the comments. --- mediagoblin/db/models.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'mediagoblin/db/models.py') diff --git a/mediagoblin/db/models.py b/mediagoblin/db/models.py index de491e96..101e7cee 100644 --- a/mediagoblin/db/models.py +++ b/mediagoblin/db/models.py @@ -393,7 +393,13 @@ class MediaComment(Base, MediaCommentMixin): created = Column(DateTime, nullable=False, default=datetime.datetime.now) content = Column(UnicodeText, nullable=False) - get_author = relationship(User) + # Cascade: Comments are owned by their creator. So do the full thing. + # lazy=dynamic: People might post a *lot* of comments, so make + # the "posted_comments" a query-like thing. + get_author = relationship(User, + backref=backref("posted_comments", + lazy="dynamic", + cascade="all, delete-orphan")) class Collection(Base, CollectionMixin): -- cgit v1.2.3 From 57f8d263e1773be7458f09f9b3f1b7571cb0e026 Mon Sep 17 00:00:00 2001 From: Elrond Date: Fri, 1 Feb 2013 15:42:44 +0100 Subject: Rewrite media_data handling to use relationships Instead of doing query by hand, use the relationships on the models to find the media_data. Is is made possible by the BACKREF_NAME in each models.py, which lets us know the local attr to ask for. Also initialize the relationship attribute on new media_data instead of the media_id. Also do not add it to the session. This gives us: - This automatically initializes the other side of the relationship, which will allow later acces via that way. - If the media_data is too early in the session, when the (new) media_entry is not yet in there, this could get conflicts. Avoid those by not adding to session. - Uses cascading to commit media_data together with the media_entry. --- mediagoblin/db/models.py | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) (limited to 'mediagoblin/db/models.py') diff --git a/mediagoblin/db/models.py b/mediagoblin/db/models.py index 101e7cee..bdd957dd 100644 --- a/mediagoblin/db/models.py +++ b/mediagoblin/db/models.py @@ -20,7 +20,7 @@ TODO: indexes on foreignkeys, where useful. import logging import datetime -import sys +from collections import Sequence from sqlalchemy import Column, Integer, Unicode, UnicodeText, DateTime, \ Boolean, ForeignKey, UniqueConstraint, PrimaryKeyConstraint, \ @@ -32,9 +32,10 @@ from sqlalchemy.ext.associationproxy import association_proxy from sqlalchemy.util import memoized_property from mediagoblin.db.extratypes import PathTupleWithSlashes, JSONEncoded -from mediagoblin.db.base import Base, DictReadAttrProxy, Session +from mediagoblin.db.base import Base, DictReadAttrProxy from mediagoblin.db.mixin import UserMixin, MediaEntryMixin, MediaCommentMixin, CollectionMixin, CollectionItemMixin from mediagoblin.tools.files import delete_media_files +from mediagoblin.tools.common import import_component # It's actually kind of annoying how sqlalchemy-migrate does this, if # I understand it right, but whatever. Anyway, don't remove this :P @@ -165,7 +166,6 @@ class MediaEntry(Base, MediaEntryMixin): collections = association_proxy("collections_helper", "in_collection") ## TODO - # media_data # fail_error def get_comments(self, ascending=False): @@ -195,40 +195,41 @@ class MediaEntry(Base, MediaEntryMixin): if media is not None: return media.url_for_self(urlgen) - #@memoized_property @property def media_data(self): - session = Session() - - return session.query(self.media_data_table).filter_by( - media_entry=self.id).first() + r = getattr(self, self.media_data_ref, None) + if isinstance(r, Sequence): + assert len(r) < 2 + if r: + return r[0] + else: + return None + return r def media_data_init(self, **kwargs): """ Initialize or update the contents of a media entry's media_data row """ - session = Session() - - media_data = session.query(self.media_data_table).filter_by( - media_entry=self.id).first() + media_data = self.media_data - # No media data, so actually add a new one if media_data is None: + # No media data, so actually add a new one media_data = self.media_data_table( - media_entry=self.id, **kwargs) - session.add(media_data) - # Update old media data + # Get the relationship set up. + media_data.get_media_entry = self else: + # Update old media data for field, value in kwargs.iteritems(): setattr(media_data, field, value) @memoized_property def media_data_table(self): - # TODO: memoize this - models_module = self.media_type + '.models' - __import__(models_module) - return sys.modules[models_module].DATA_MODEL + return import_component(self.media_type + '.models:DATA_MODEL') + + @memoized_property + def media_data_ref(self): + return import_component(self.media_type + '.models:BACKREF_NAME') def __repr__(self): safe_title = self.title.encode('ascii', 'replace') -- cgit v1.2.3 From 139c6c099fdbf139d2441db0c1a774081394a47d Mon Sep 17 00:00:00 2001 From: Elrond Date: Fri, 1 Feb 2013 16:33:53 +0100 Subject: Drop media_data_table property. Only when creating a new media_data row, we need the table. So load that locally in media_data_init(). --- mediagoblin/db/models.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'mediagoblin/db/models.py') diff --git a/mediagoblin/db/models.py b/mediagoblin/db/models.py index bdd957dd..c9bc3c11 100644 --- a/mediagoblin/db/models.py +++ b/mediagoblin/db/models.py @@ -213,9 +213,10 @@ class MediaEntry(Base, MediaEntryMixin): media_data = self.media_data if media_data is None: + # Get the correct table: + table = import_component(self.media_type + '.models:DATA_MODEL') # No media data, so actually add a new one - media_data = self.media_data_table( - **kwargs) + media_data = table(**kwargs) # Get the relationship set up. media_data.get_media_entry = self else: @@ -223,10 +224,6 @@ class MediaEntry(Base, MediaEntryMixin): for field, value in kwargs.iteritems(): setattr(media_data, field, value) - @memoized_property - def media_data_table(self): - return import_component(self.media_type + '.models:DATA_MODEL') - @memoized_property def media_data_ref(self): return import_component(self.media_type + '.models:BACKREF_NAME') -- cgit v1.2.3 From 485404a9c42b09f0fda38aeb8d1242f24ccfa143 Mon Sep 17 00:00:00 2001 From: Elrond Date: Fri, 1 Feb 2013 16:33:53 +0100 Subject: Drop backward compatibility for media_data backref. Now we only support media_type backrefs with uselist=False. --- mediagoblin/db/models.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) (limited to 'mediagoblin/db/models.py') diff --git a/mediagoblin/db/models.py b/mediagoblin/db/models.py index c9bc3c11..10e0c33f 100644 --- a/mediagoblin/db/models.py +++ b/mediagoblin/db/models.py @@ -20,7 +20,6 @@ TODO: indexes on foreignkeys, where useful. import logging import datetime -from collections import Sequence from sqlalchemy import Column, Integer, Unicode, UnicodeText, DateTime, \ Boolean, ForeignKey, UniqueConstraint, PrimaryKeyConstraint, \ @@ -197,14 +196,7 @@ class MediaEntry(Base, MediaEntryMixin): @property def media_data(self): - r = getattr(self, self.media_data_ref, None) - if isinstance(r, Sequence): - assert len(r) < 2 - if r: - return r[0] - else: - return None - return r + return getattr(self, self.media_data_ref) def media_data_init(self, **kwargs): """ -- cgit v1.2.3 From df5b142ab9bfc590f17768079104f6cfa2cd7bba Mon Sep 17 00:00:00 2001 From: Elrond Date: Mon, 18 Feb 2013 14:46:28 +0100 Subject: Fix deleting media with attachments. If one deletes a media with attachments, there have been various problems: 1) If the file in the storage did not exist any more (maybe because due to a previous deletion attempt?), the error propagation failed, because the wrong thing was gathered. 2) The attachment database entries were not deleted. Using cascade for this, for now. Also add a simple unit test, that tests both by having a broken attachment on a media. --- mediagoblin/db/models.py | 1 + 1 file changed, 1 insertion(+) (limited to 'mediagoblin/db/models.py') diff --git a/mediagoblin/db/models.py b/mediagoblin/db/models.py index 10e0c33f..2f58503f 100644 --- a/mediagoblin/db/models.py +++ b/mediagoblin/db/models.py @@ -145,6 +145,7 @@ class MediaEntry(Base, MediaEntryMixin): ) attachment_files_helper = relationship("MediaAttachmentFile", + cascade="all, delete-orphan", order_by="MediaAttachmentFile.created" ) attachment_files = association_proxy("attachment_files_helper", "dict_view", -- cgit v1.2.3