Add public_id fixes throughout the code

This adds several things, mainly code which checks for the public id and
if it doesn't exist generating it where it can. This is to because we
need to keep the public_id to be able to effectively soft delete models.

This also adds a public_id field to the Activity along with a migration.

7 files changed, 107 insertions, 27 deletions

diff --git a/mediagoblin/api/views.py b/mediagoblin/api/views.py
index d3eaacc9..c515a8fa 100644
--- a/mediagoblin/api/views.py
+++ b/mediagoblin/api/views.py
@@ -500,6 +500,10 @@ def feed_endpoint(request, outbox=None):
                         "No such 'image' with id '{0}'.".format(obj_id)
                     )
 
+                # Okay lets do our best to ensure there is a public_id for
+                # this image, there most likely is but it's important!
+                entry.get_public_id(request.urlgen)
+
                 # Make the delete activity
                 generator = create_generator(request)
                 activity = create_activity(
diff --git a/mediagoblin/db/migrations.py b/mediagoblin/db/migrations.py
index 36ad736a..2df06fc0 100644
--- a/mediagoblin/db/migrations.py
+++ b/mediagoblin/db/migrations.py
@@ -1693,6 +1693,15 @@ def federation_collection_schema(db):
     # Add the fields onto the Collection model, we need to set these as
     # not null to avoid DB integreity errors. We will add the not null
     # constraint later.
+    public_id_column = Column(
+        "public_id",
+        Unicode,
+        unique=True
+    )
+    public_id_column.create(
+        collection_table,
+        unique_name="collection_public_id")
+
     updated_column = Column(
         "updated",
         DateTime,
@@ -1846,3 +1855,23 @@ def federation_graveyard(db):
 
     # Commit changes to the db
     db.commit()
+
+@RegisterMigration(40, MIGRATIONS)
+def add_public_id(db):
+    metadata = MetaData(bind=db.bind)
+
+    # Get the table
+    activity_table = inspect_table(metadata, "core__activities")
+    activity_public_id = Column(
+        "public_id",
+        Unicode,
+        unique=True,
+        nullable=True
+    )
+    activity_public_id.create(
+        activity_table,
+        unique_name="activity_public_id"
+    )
+
+    # Commit this.
+    db.commit()
diff --git a/mediagoblin/db/mixin.py b/mediagoblin/db/mixin.py
index 7960061e..e6a2dc35 100644
--- a/mediagoblin/db/mixin.py
+++ b/mediagoblin/db/mixin.py
@@ -41,6 +41,40 @@ from mediagoblin.tools.text import cleaned_markdown_conversion
 from mediagoblin.tools.url import slugify
 from mediagoblin.tools.translate import pass_to_ugettext as _
 
+class GeneratePublicIDMixin(object):
+    """
+    Mixin that ensures that a the public_id field is populated.
+
+    The public_id is the ID that is used in the API, this must be globally
+    unique and dereferencable. This will be the URL for the API view of the
+    object. It's used in several places, not only is it used to give out via
+    the API but it's also vital information stored when a soft_deletion occurs
+    on the `Graveyard.public_id` field, this is needed to follow the spec which
+    says we have to be able to provide a shell of an object and return a 410
+    (rather than a 404) when a deleted object has been deleted.
+
+    This requires a the urlgen off the request object (`request.urlgen`) to be
+    provided as it's the ID is a URL.
+    """
+
+    def get_public_id(self, urlgen):
+        # Verify that the class this is on actually has a public_id field...
+        if "public_id" not in self.__table__.columns.keys():
+            raise Exception("Model has no public_id field")
+
+        # Great! the model has a public id, if it's None, let's create one!
+        if self.public_id is None:
+            # We need the internal ID for this so ensure we've been saved.
+            self.save(commit=False)
+
+            # Create the URL
+            self.public_id = urlgen(
+                "mediagoblin.api.object",
+                object_type=self.object_type,
+                id=self.id,
+                qualified=True
+            )
+        return self.public_id
 
 class UserMixin(object):
     object_type = "person"
@@ -52,6 +86,7 @@ class UserMixin(object):
     def url_for_self(self, urlgen, **kwargs):
         """Generate a URL for this User's home page."""
         return urlgen('mediagoblin.user_pages.user_home',
+
                       user=self.username, **kwargs)
 
 
@@ -128,7 +163,7 @@ class GenerateSlugMixin(object):
         self.slug = slug
 
 
-class MediaEntryMixin(GenerateSlugMixin):
+class MediaEntryMixin(GenerateSlugMixin, GeneratePublicIDMixin):
     def check_slug_used(self, slug):
         # import this here due to a cyclic import issue
         # (db.models -> db.mixin -> db.util -> db.models)
@@ -196,25 +231,6 @@ class MediaEntryMixin(GenerateSlugMixin):
             media=self.slug_or_id,
             **extra_args)
 
-    def get_public_id(self, request):
-        """ Returns the public_id of the MediaEntry
-
-        If the MediaEntry has no public ID one will be produced from the
-        current request.
-        """
-        if self.public_id is None:
-            self.public_id = request.urlgen(
-                "mediagoblin.api.object",
-                object_type=self.object_type,
-                id=self.id,
-                qualified=True
-            )
-        # We need to ensure this ID is reused once we've given it out.
-        self.save()
-
-        return self.public_id
-
-
     @property
     def thumb_url(self):
         """Return the thumbnail URL (for usage in templates)
@@ -352,7 +368,7 @@ class MediaCommentMixin(object):
             comment=self.content)
 
 
-class CollectionMixin(GenerateSlugMixin):
+class CollectionMixin(GenerateSlugMixin, GeneratePublicIDMixin):
     object_type = "collection"
 
     def check_slug_used(self, slug):
@@ -398,7 +414,7 @@ class CollectionItemMixin(object):
         """
         return cleaned_markdown_conversion(self.note)
 
-class ActivityMixin(object):
+class ActivityMixin(GeneratePublicIDMixin):
     object_type = "activity"
 
     VALID_VERBS = ["add", "author", "create", "delete", "dislike", "favorite",
diff --git a/mediagoblin/db/models.py b/mediagoblin/db/models.py
index 73f3c8ce..e52cab82 100644
--- a/mediagoblin/db/models.py
+++ b/mediagoblin/db/models.py
@@ -728,7 +728,7 @@ class MediaEntry(Base, MediaEntryMixin):
         author = self.get_actor
         published = UTC.localize(self.created)
         updated = UTC.localize(self.updated)
-        public_id = self.get_public_id(request)
+        public_id = self.get_public_id(request.urlgen)
         context = {
             "id": public_id,
             "author": author.serialize(request),
@@ -1034,6 +1034,7 @@ class Collection(Base, CollectionMixin):
     __tablename__ = "core__collections"
 
     id = Column(Integer, primary_key=True)
+    public_id = Column(Unicode, unique=True)
     title = Column(Unicode, nullable=False)
     slug = Column(Unicode)
     created = Column(DateTime, nullable=False, default=datetime.datetime.utcnow,
@@ -1492,6 +1493,7 @@ class Activity(Base, ActivityMixin):
     __tablename__ = "core__activities"
 
     id = Column(Integer, primary_key=True)
+    public_id = Column(Unicode, unique=True)
     actor = Column(Integer,
                    ForeignKey("core__users.id"),
                    nullable=False)
diff --git a/mediagoblin/submit/lib.py b/mediagoblin/submit/lib.py
index 77dd836b..eee5653f 100644
--- a/mediagoblin/submit/lib.py
+++ b/mediagoblin/submit/lib.py
@@ -104,9 +104,7 @@ def submit_media(mg_app, user, submitted_file, filename,
                  title=None, description=None,
                  license=None, metadata=None, tags_string=u"",
                  upload_limit=None, max_file_size=None,
-                 callback_url=None,
-                 # If provided we'll do the feed_url update, otherwise ignore
-                 urlgen=None,):
+                 callback_url=None, urlgen=None,):
     """
     Args:
      - mg_app: The MediaGoblinApp instantiated for this process
@@ -124,7 +122,8 @@ def submit_media(mg_app, user, submitted_file, filename,
      - upload_limit: size in megabytes that's the per-user upload limit
      - max_file_size: maximum size each file can be that's uploaded
      - callback_url: possible post-hook to call after submission
-     - urlgen: if provided, used to do the feed_url update
+     - urlgen: if provided, used to do the feed_url update and assign a public
+               ID used in the API (very important).
     """
     if upload_limit and user.uploaded >= upload_limit:
         raise UserPastUploadLimit()
@@ -189,6 +188,11 @@ def submit_media(mg_app, user, submitted_file, filename,
         metadata.save()
 
     if urlgen:
+        # Generate the public_id, this is very importent, especially relating
+        # to deletion, it allows the shell to be accessable post-delete!
+        entry.get_public_id(urlgen)
+
+        # Generate the feed URL
         feed_url = urlgen(
             'mediagoblin.user_pages.atom_feed',
             qualified=True, user=user.username)
diff --git a/mediagoblin/tests/test_submission.py b/mediagoblin/tests/test_submission.py
index 55394670..c0c3d3cf 100644
--- a/mediagoblin/tests/test_submission.py
+++ b/mediagoblin/tests/test_submission.py
@@ -153,6 +153,16 @@ class TestSubmission:
         # Reload user
         assert self.our_user().uploaded == file_size
 
+    def test_public_id_populated(self):
+        # Upload the image first.
+        response, request = self.do_post({'title': u'Balanced Goblin'},
+                                         *REQUEST_CONTEXT, do_follow=True,
+                                         **self.upload_data(GOOD_JPG))
+        media = self.check_media(request, {'title': u'Balanced Goblin'}, 1)
+
+        # Now check that the public_id attribute is set.
+        assert media.public_id != None
+
     def test_normal_png(self):
         self.check_normal_upload(u'Normal upload 2', GOOD_PNG)
 
diff --git a/mediagoblin/user_pages/views.py b/mediagoblin/user_pages/views.py
index 4ee601b3..f1c8a622 100644
--- a/mediagoblin/user_pages/views.py
+++ b/mediagoblin/user_pages/views.py
@@ -267,6 +267,7 @@ def media_collect(request, media):
         collection.actor = request.user.id
         collection.type = Collection.USER_DEFINED_TYPE
         collection.generate_slug()
+        collection.get_public_id(request.urlgen)
         create_activity("create", collection, collection.actor)
         collection.save()
 
@@ -318,6 +319,12 @@ def media_confirm_delete(request, media):
         if form.confirm.data is True:
             username = media.get_actor.username
 
+            # This probably is already filled but just in case it has slipped
+            # through the net somehow, we need to try and make sure the
+            # MediaEntry has a public ID so it gets properly soft-deleted.
+            media.get_public_id(request.urlgen)
+
+            # Decrement the users uploaded quota.
             media.get_actor.uploaded = media.get_actor.uploaded - \
                 media.file_size
             media.get_actor.save()
@@ -453,6 +460,10 @@ def collection_confirm_delete(request, collection):
         if form.confirm.data is True:
             collection_title = collection.title
 
+            # Firstly like with the MediaEntry delete, lets ensure the
+            # public_id is populated as this is really important!
+            collection.get_public_id(request.urlgen)
+
             # Delete all the associated collection items
             for item in collection.get_collection_items():
                 obj = item.get_object()
@@ -727,6 +738,10 @@ def activity_view(request):
         author=user.id
     ).first()
 
+    # There isn't many places to check that the public_id is filled so this
+    # will do, it really should be, lets try and fix that if it isn't.
+    activity.get_public_id(request.urlgen)
+
     if activity is None:
         return render_404(request)