diff options
| -rw-r--r-- | mediagoblin/api/views.py | 4 | ||||
| -rw-r--r-- | mediagoblin/db/migrations.py | 29 | ||||
| -rw-r--r-- | mediagoblin/db/mixin.py | 60 | ||||
| -rw-r--r-- | mediagoblin/db/models.py | 4 | ||||
| -rw-r--r-- | mediagoblin/submit/lib.py | 12 | ||||
| -rw-r--r-- | mediagoblin/tests/test_submission.py | 10 | ||||
| -rw-r--r-- | mediagoblin/user_pages/views.py | 15 |
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) |