From eebd0063feb19ea696f632dbb60588f0591c037b Mon Sep 17 00:00:00 2001 From: Jessica Tallon Date: Tue, 12 Jan 2016 11:41:21 +0000 Subject: [PATCH] Fix #5382 - Add migration and fix so tombstones are removed from collections When an item is deleted it should be removed from all collections, this commit makes that happen. It's got two changes: 1. Adds the code so when an object is soft deleted, it's automatically removed from all collection items 2. Add a migration to fix this issue for those who have tombstones (Graveyard objects) in their collections because of this bug. This commit requires you to run a migration --- mediagoblin/db/base.py | 26 ++++++ ...510e3a713_removes_graveyard_items_from_.py | 84 +++++++++++++++++++ 2 files changed, 110 insertions(+) create mode 100644 mediagoblin/db/migrations/versions/101510e3a713_removes_graveyard_items_from_.py diff --git a/mediagoblin/db/base.py b/mediagoblin/db/base.py index 11afbcec..af2337df 100644 --- a/mediagoblin/db/base.py +++ b/mediagoblin/db/base.py @@ -92,6 +92,31 @@ class GMGTableBase(object): if deletion is None: deletion = self.deletion_mode + # If the item is in any collection then it should be removed, this will + # cause issues if it isn't. See #5382. + # Import here to prevent cyclic imports. + from mediagoblin.db.models import CollectionItem, GenericModelReference + + # Some of the models don't have an "id" field which means they can't be + # used with GMR, these models won't be in collections because they + # can't be. We can skip all of this. + if hasattr(self, "id"): + # First find the GenericModelReference for this object + gmr = GenericModelReference.query.filter_by( + obj_pk=self.id, + model_type=self.__tablename__ + ).first() + + # If there is no gmr then we've got lucky, a GMR is a requirement of + # being in a collection. + if gmr is not None: + items = CollectionItem.query.filter_by( + object_id=gmr.id + ) + + # Delete any CollectionItems found. + items.delete() + # Hand off to the correct deletion function. if deletion == self.HARD_DELETE: return self.hard_delete(commit=commit) @@ -132,6 +157,7 @@ class GMGTableBase(object): "model_type": tombstone.__tablename__, }) + # Now we can go ahead and actually delete the model. return self.hard_delete(commit=commit) diff --git a/mediagoblin/db/migrations/versions/101510e3a713_removes_graveyard_items_from_.py b/mediagoblin/db/migrations/versions/101510e3a713_removes_graveyard_items_from_.py new file mode 100644 index 00000000..d730e808 --- /dev/null +++ b/mediagoblin/db/migrations/versions/101510e3a713_removes_graveyard_items_from_.py @@ -0,0 +1,84 @@ +"""#5382 Removes graveyard items from collections + +Revision ID: 101510e3a713 +Revises: 52bf0ccbedc1 +Create Date: 2016-01-12 10:46:26.486610 + +""" + +# revision identifiers, used by Alembic. +revision = '101510e3a713' +down_revision = '52bf0ccbedc1' + +from alembic import op + +import sqlalchemy as sa +from sqlalchemy.sql import and_ + +# Create the tables to query +gmr_table = sa.Table( + "core__generic_model_reference", + sa.MetaData(), + sa.Column("id", sa.Integer, primary_key=True), + sa.Column("obj_pk", sa.Integer), + sa.Column("model_type", sa.Unicode) +) + +graveyard_table = sa.Table( + "core__graveyard", + sa.MetaData(), + sa.Column("id", sa.Integer, primary_key=True), + sa.Column("public_id", sa.Unicode, unique=True), + sa.Column("deleted", sa.DateTime, nullable=False), + sa.Column("object_type", sa.Unicode, nullable=False), + sa.Column("actor_id", sa.Integer) +) + +collection_items_table = sa.Table( + "core__collection_items", + sa.MetaData(), + sa.Column("id", sa.Integer, primary_key=True), + sa.Column("collection", sa.Integer, nullable=False), + sa.Column("note", sa.UnicodeText), + sa.Column("added", sa.DateTime, nullable=False), + sa.Column("position", sa.Integer), + sa.Column("object_id", sa.Integer, index=True) +) + +def upgrade(): + """ + The problem is deletions are occuring and as we expect the + GenericModelReference objects are being updated to point to the tombstone + object. The issue is that collections now contain deleted items, this + causes problems when it comes to rendering them for example. + + This migration is to remove any Graveyard objects (tombstones) from any + Collection. + """ + connection = op.get_bind() + + for tombstone in connection.execute(graveyard_table.select()): + # Get GMR for tombstone + gmr = connection.execute(gmr_table.select().where(and_( + gmr_table.c.obj_pk == tombstone.id, + gmr_table.c.model_type == "core__graveyard" + ))).first() + + # If there is no GMR, we're all good as it's required to be in a + # collection + if gmr is None: + continue + + # Delete all the CollectionItem objects for this GMR + connection.execute(collection_items_table.delete().where( + collection_items_table.c.object_id == gmr.id + )) + + +def downgrade(): + """ + Nothing to do here, the migration just deletes objects from collections. + There are no schema changes that have occured. This can be reverted without + any problems. + """ + pass -- 2.25.1