From ad7420281cdaf5846d8754a29e5006bde82d87f9 Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Wed, 5 Dec 2012 13:37:19 +0100 Subject: Create a active_user_from_url decorator This can be used for URL patterns containing a element. It will look up the corresponding user among all active users and return a 404 NOT FOUND page if there is no such active user. It then passes the User() instance as url_user keyword argument to the decorated view function. Signed-off-by: Sebastian Spaeth --- mediagoblin/decorators.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'mediagoblin/decorators.py') diff --git a/mediagoblin/decorators.py b/mediagoblin/decorators.py index 9be9d4cc..90b36771 100644 --- a/mediagoblin/decorators.py +++ b/mediagoblin/decorators.py @@ -22,6 +22,7 @@ from urllib import urlencode from webob import exc from mediagoblin.db.util import ObjectId, InvalidId +from mediagoblin.db.sql.models import User from mediagoblin.tools.response import redirect, render_404 @@ -52,6 +53,20 @@ def require_active_login(controller): return new_controller_func +def active_user_from_url(controller): + """Retrieve User() from URL pattern and pass in as url_user=... + + Returns a 404 if no such active user has been found""" + @wraps(controller) + def wrapper(request, *args, **kwargs): + user = User.query.filter_by(username=request.matchdict['user']).first() + if user is None: + return render_404(request) + + return controller(request, *args, url_user=user, **kwargs) + + return wrapper + def user_may_delete_media(controller): """ -- cgit v1.2.3 From 5c2b84869fe3f4bfe41a31ff3968bb13c6d7f868 Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Fri, 30 Nov 2012 10:49:06 +0100 Subject: Move DBModel._id -> DBModel.id We were refering to model._id in most of the code base as this is what Mongo uses. However, each use of _id required a) fixup of queries: e.g. what we did in our find() and find_one() functions moving all '_id' to 'id'. It also required using AliasFields to make the ._id attribute available. This all means lots of superfluous fixing and transitioning in a SQL world. It will also not work in the long run. Much newer code already refers to the objects by model.id (e.g. in the oauth plugin), which will break with Mongo. So let's be honest, rip out the _id mongoism and live with .id as the one canonical way to address objects. This commit modifies all users and providers of model._id to use model.id instead. This patch works with or without Mongo removed first, but will break Mongo usage (even more than before) I have not bothered to fixup db.mongo.* and db.sql.convert (which converts from Mongo to SQL) Signed-off-by: Sebastian Spaeth --- mediagoblin/decorators.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'mediagoblin/decorators.py') diff --git a/mediagoblin/decorators.py b/mediagoblin/decorators.py index 90b36771..2955c927 100644 --- a/mediagoblin/decorators.py +++ b/mediagoblin/decorators.py @@ -75,9 +75,9 @@ def user_may_delete_media(controller): @wraps(controller) def wrapper(request, *args, **kwargs): uploader_id = request.db.MediaEntry.find_one( - {'_id': ObjectId(request.matchdict['media'])}).uploader + {'id': ObjectId(request.matchdict['media'])}).uploader if not (request.user.is_admin or - request.user._id == uploader_id): + request.user.id == uploader_id): return exc.HTTPForbidden() return controller(request, *args, **kwargs) @@ -94,7 +94,7 @@ def user_may_alter_collection(controller): creator_id = request.db.User.find_one( {'username': request.matchdict['user']}).id if not (request.user.is_admin or - request.user._id == creator_id): + request.user.id == creator_id): return exc.HTTPForbidden() return controller(request, *args, **kwargs) @@ -134,15 +134,15 @@ def get_user_media_entry(controller): media = request.db.MediaEntry.find_one( {'slug': request.matchdict['media'], 'state': u'processed', - 'uploader': user._id}) + 'uploader': user.id}) # no media via slug? Grab it via ObjectId if not media: try: media = request.db.MediaEntry.find_one( - {'_id': ObjectId(request.matchdict['media']), + {'id': ObjectId(request.matchdict['media']), 'state': u'processed', - 'uploader': user._id}) + 'uploader': user.id}) except InvalidId: return render_404(request) @@ -169,7 +169,7 @@ def get_user_collection(controller): collection = request.db.Collection.find_one( {'slug': request.matchdict['collection'], - 'creator': user._id}) + 'creator': user.id}) # Still no collection? Okay, 404. if not collection: @@ -194,10 +194,10 @@ def get_user_collection_item(controller): collection = request.db.Collection.find_one( {'slug': request.matchdict['collection'], - 'creator': user._id}) + 'creator': user.id}) collection_item = request.db.CollectionItem.find_one( - {'_id': request.matchdict['collection_item'] }) + {'id': request.matchdict['collection_item'] }) # Still no collection item? Okay, 404. if not collection_item: @@ -216,7 +216,7 @@ def get_media_entry_by_id(controller): def wrapper(request, *args, **kwargs): try: media = request.db.MediaEntry.find_one( - {'_id': ObjectId(request.matchdict['media']), + {'id': ObjectId(request.matchdict['media']), 'state': u'processed'}) except InvalidId: return render_404(request) -- cgit v1.2.3 From 059eaee4dfa40e3c2e67b7d638f49955b68d9c31 Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Thu, 15 Nov 2012 16:49:51 +0100 Subject: Remove webobisms from decorators.py Use our own redirect function rather than webobs HttpFound Also replace HttpForbidden() with webob's Forbidden() --- mediagoblin/decorators.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) (limited to 'mediagoblin/decorators.py') diff --git a/mediagoblin/decorators.py b/mediagoblin/decorators.py index 2955c927..e45d3272 100644 --- a/mediagoblin/decorators.py +++ b/mediagoblin/decorators.py @@ -17,9 +17,8 @@ from functools import wraps from urlparse import urljoin -from urllib import urlencode - -from webob import exc +from werkzeug.exceptions import Forbidden +from werkzeug.urls import url_quote from mediagoblin.db.util import ObjectId, InvalidId from mediagoblin.db.sql.models import User @@ -43,11 +42,8 @@ def require_active_login(controller): qualified=True), request.url) - return exc.HTTPFound( - location='?'.join([ - request.urlgen('mediagoblin.auth.login'), - urlencode({ - 'next': next_url})])) + return redirect(request, 'mediagoblin.auth.login', + next=url_quote(next_url)) return controller(request, *args, **kwargs) @@ -78,7 +74,7 @@ def user_may_delete_media(controller): {'id': ObjectId(request.matchdict['media'])}).uploader if not (request.user.is_admin or request.user.id == uploader_id): - return exc.HTTPForbidden() + return Forbidden() return controller(request, *args, **kwargs) @@ -95,7 +91,7 @@ def user_may_alter_collection(controller): {'username': request.matchdict['user']}).id if not (request.user.is_admin or request.user.id == creator_id): - return exc.HTTPForbidden() + return Forbidden() return controller(request, *args, **kwargs) -- cgit v1.2.3 From cfa922295e5ddfaab336a3c2c0403422f77758b6 Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Sun, 23 Dec 2012 11:58:51 +0100 Subject: Convert return HttpException to raise HttpException controllers (view function) raise HttpException's and do not return them. Signed-off-by: Sebastian Spaeth --- mediagoblin/decorators.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'mediagoblin/decorators.py') diff --git a/mediagoblin/decorators.py b/mediagoblin/decorators.py index e45d3272..0903dd41 100644 --- a/mediagoblin/decorators.py +++ b/mediagoblin/decorators.py @@ -74,7 +74,7 @@ def user_may_delete_media(controller): {'id': ObjectId(request.matchdict['media'])}).uploader if not (request.user.is_admin or request.user.id == uploader_id): - return Forbidden() + raise Forbidden() return controller(request, *args, **kwargs) @@ -91,7 +91,7 @@ def user_may_alter_collection(controller): {'username': request.matchdict['user']}).id if not (request.user.is_admin or request.user.id == creator_id): - return Forbidden() + raise Forbidden() return controller(request, *args, **kwargs) -- cgit v1.2.3 From 7c029a1f338bd918b80a4ad071ad05180647750f Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Tue, 25 Dec 2012 20:28:19 +0100 Subject: Remove InvalidId It was a NoOp in our Non-mongo world. So it is safe to remove. Signed-off-by: Sebastian Spaeth --- mediagoblin/decorators.py | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) (limited to 'mediagoblin/decorators.py') diff --git a/mediagoblin/decorators.py b/mediagoblin/decorators.py index 0903dd41..ce5cb59c 100644 --- a/mediagoblin/decorators.py +++ b/mediagoblin/decorators.py @@ -20,7 +20,7 @@ from urlparse import urljoin from werkzeug.exceptions import Forbidden from werkzeug.urls import url_quote -from mediagoblin.db.util import ObjectId, InvalidId +from mediagoblin.db.util import ObjectId from mediagoblin.db.sql.models import User from mediagoblin.tools.response import redirect, render_404 @@ -134,14 +134,10 @@ def get_user_media_entry(controller): # no media via slug? Grab it via ObjectId if not media: - try: - media = request.db.MediaEntry.find_one( + media = request.db.MediaEntry.find_one( {'id': ObjectId(request.matchdict['media']), 'state': u'processed', 'uploader': user.id}) - except InvalidId: - return render_404(request) - # Still no media? Okay, 404. if not media: return render_404(request) @@ -210,13 +206,9 @@ def get_media_entry_by_id(controller): """ @wraps(controller) def wrapper(request, *args, **kwargs): - try: - media = request.db.MediaEntry.find_one( + media = request.db.MediaEntry.find_one( {'id': ObjectId(request.matchdict['media']), 'state': u'processed'}) - except InvalidId: - return render_404(request) - # Still no media? Okay, 404. if not media: return render_404(request) -- cgit v1.2.3 From 71717fd5316607500159f782b10ca91cf9684bfd Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Tue, 25 Dec 2012 20:52:25 +0100 Subject: Remove ObjectId from the tree This was one of the last remaining Mongo holdouts and has been removed from the tree herewith. Good bye, ObjectId. Signed-off-by: Sebastian Spaeth --- mediagoblin/decorators.py | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) (limited to 'mediagoblin/decorators.py') diff --git a/mediagoblin/decorators.py b/mediagoblin/decorators.py index ce5cb59c..f6169060 100644 --- a/mediagoblin/decorators.py +++ b/mediagoblin/decorators.py @@ -20,8 +20,7 @@ from urlparse import urljoin from werkzeug.exceptions import Forbidden from werkzeug.urls import url_quote -from mediagoblin.db.util import ObjectId -from mediagoblin.db.sql.models import User +from mediagoblin.db.sql.models import MediaEntry, User from mediagoblin.tools.response import redirect, render_404 @@ -70,8 +69,7 @@ def user_may_delete_media(controller): """ @wraps(controller) def wrapper(request, *args, **kwargs): - uploader_id = request.db.MediaEntry.find_one( - {'id': ObjectId(request.matchdict['media'])}).uploader + uploader_id = MediaEntry.query.get(request.matchdict['media']).uploader if not (request.user.is_admin or request.user.id == uploader_id): raise Forbidden() @@ -132,12 +130,12 @@ def get_user_media_entry(controller): 'state': u'processed', 'uploader': user.id}) - # no media via slug? Grab it via ObjectId + # no media via slug? Grab it via object id if not media: - media = request.db.MediaEntry.find_one( - {'id': ObjectId(request.matchdict['media']), - 'state': u'processed', - 'uploader': user.id}) + media = MediaEntry.query.filter_by( + id=request.matchdict['media'], + state=u'processed', + uploader=user.id).first() # Still no media? Okay, 404. if not media: return render_404(request) @@ -206,9 +204,9 @@ def get_media_entry_by_id(controller): """ @wraps(controller) def wrapper(request, *args, **kwargs): - media = request.db.MediaEntry.find_one( - {'id': ObjectId(request.matchdict['media']), - 'state': u'processed'}) + media = MediaEntry.query.filter_by( + id=request.matchdict['media'], + state=u'processed').first() # Still no media? Okay, 404. if not media: return render_404(request) -- cgit v1.2.3 From b0c8328e547288028e7e43f0ceb1fa9f7c8dac4a Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Fri, 30 Nov 2012 10:10:35 +0100 Subject: Move db.sql.models* to db.models* --- mediagoblin/decorators.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'mediagoblin/decorators.py') diff --git a/mediagoblin/decorators.py b/mediagoblin/decorators.py index f6169060..4ae1b867 100644 --- a/mediagoblin/decorators.py +++ b/mediagoblin/decorators.py @@ -20,7 +20,7 @@ from urlparse import urljoin from werkzeug.exceptions import Forbidden from werkzeug.urls import url_quote -from mediagoblin.db.sql.models import MediaEntry, User +from mediagoblin.db.models import MediaEntry, User from mediagoblin.tools.response import redirect, render_404 -- cgit v1.2.3 From 7f4e42b0b15aefcc885c9aacefac3a76f2f7b5ad Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Tue, 8 Jan 2013 08:59:32 +0100 Subject: Fix slug lookup regression (#587) Removing the Mongo InvalidID legacy code removed an explicit check for "int" for the id lookup. This led the @get_user_media_entry decorator to fail if we looked up a nonexisting non-numerical slug (it tried to query the id with a string, which failed). Cast id to int and return 404 in case it is non-numeric which fixes the regression. It does not fix the underlying problem of slug_or_id lookups that were discussed. Signed-off-by: Sebastian Spaeth --- mediagoblin/decorators.py | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) (limited to 'mediagoblin/decorators.py') diff --git a/mediagoblin/decorators.py b/mediagoblin/decorators.py index 4ae1b867..5533e81d 100644 --- a/mediagoblin/decorators.py +++ b/mediagoblin/decorators.py @@ -17,7 +17,7 @@ from functools import wraps from urlparse import urljoin -from werkzeug.exceptions import Forbidden +from werkzeug.exceptions import Forbidden, NotFound from werkzeug.urls import url_quote from mediagoblin.db.models import MediaEntry, User @@ -120,25 +120,29 @@ def get_user_media_entry(controller): """ @wraps(controller) def wrapper(request, *args, **kwargs): - user = request.db.User.find_one( - {'username': request.matchdict['user']}) - + user = User.query.filter_by(username=request.matchdict['user']).first() if not user: - return render_404(request) - media = request.db.MediaEntry.find_one( - {'slug': request.matchdict['media'], - 'state': u'processed', - 'uploader': user.id}) + raise NotFound() + + media = MediaEntry.query.filter_by( + slug = request.matchdict['media'], + state = u'processed', + uploader = user.id).first() - # no media via slug? Grab it via object id if not media: - media = MediaEntry.query.filter_by( - id=request.matchdict['media'], - state=u'processed', - uploader=user.id).first() - # Still no media? Okay, 404. - if not media: - return render_404(request) + # no media via slug? Grab it via object id + try: + media = MediaEntry.query.filter_by( + id = int(request.matchdict['media']), + state = u'processed', + uploader = user.id).first() + except ValueError: + # media "id" was no int + raise NotFound() + + if not media: + # no media by that id? Okay, 404. + raise NotFound() return controller(request, media=media, *args, **kwargs) -- cgit v1.2.3 From 461dd9717cce6c5b4d40bb4e76ca65d9d898d1df Mon Sep 17 00:00:00 2001 From: Elrond Date: Fri, 11 Jan 2013 14:18:27 +0100 Subject: Start to use the media_id in "admin" URLs. We have a bunch of URLs that are more for internal use. At least they're definitely not intended to be posted somewhere for long term useage. When those things affect a media, it's much better to reference the media by its id. This can't change, ever. This is better for races. Like someone posting a comment while the owner corrects a typo in the slug. --- mediagoblin/decorators.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'mediagoblin/decorators.py') diff --git a/mediagoblin/decorators.py b/mediagoblin/decorators.py index 5533e81d..a40f1d5a 100644 --- a/mediagoblin/decorators.py +++ b/mediagoblin/decorators.py @@ -69,7 +69,7 @@ def user_may_delete_media(controller): """ @wraps(controller) def wrapper(request, *args, **kwargs): - uploader_id = MediaEntry.query.get(request.matchdict['media']).uploader + uploader_id = kwargs['media'].uploader if not (request.user.is_admin or request.user.id == uploader_id): raise Forbidden() @@ -209,12 +209,16 @@ def get_media_entry_by_id(controller): @wraps(controller) def wrapper(request, *args, **kwargs): media = MediaEntry.query.filter_by( - id=request.matchdict['media'], + id=request.matchdict['media_id'], state=u'processed').first() # Still no media? Okay, 404. if not media: return render_404(request) + given_username = request.matchdict.get('user') + if given_username and (given_username != media.get_uploader.username): + return render_404(request) + return controller(request, media=media, *args, **kwargs) return wrapper -- cgit v1.2.3 From f91dcc9d963c87d6b6752cb397e1ff9fdf57ad28 Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Wed, 12 Dec 2012 13:53:56 +0100 Subject: Implement @get_workbench decorator This passes in a Workbench() via the 'workbench' keyword argument, and conveniently cleans it up after the function has finished. 2 out of our 5 backends forgot to clean up their workbench, so this is clearly needed :-). Signed-off-by: Sebastian Spaeth --- mediagoblin/decorators.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'mediagoblin/decorators.py') diff --git a/mediagoblin/decorators.py b/mediagoblin/decorators.py index a40f1d5a..3655d758 100644 --- a/mediagoblin/decorators.py +++ b/mediagoblin/decorators.py @@ -20,6 +20,7 @@ from urlparse import urljoin from werkzeug.exceptions import Forbidden, NotFound from werkzeug.urls import url_quote +from mediagoblin import mg_globals as mgg from mediagoblin.db.models import MediaEntry, User from mediagoblin.tools.response import redirect, render_404 @@ -222,3 +223,14 @@ def get_media_entry_by_id(controller): return controller(request, media=media, *args, **kwargs) return wrapper + + +def get_workbench(func): + """Decorator, passing in a workbench as kwarg which is cleaned up afterwards""" + + @wraps(func) + def new_func(*args, **kwargs): + with mgg.workbench_manager.create_workbench() as workbench: + return func(*args, workbench=workbench, **kwargs) + + return new_func -- cgit v1.2.3 From bd6fe9774693a241e99138c987f71b80d968bdc3 Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Wed, 12 Dec 2012 14:08:38 +0100 Subject: Shorten Workbench(Manager) method names 1) destroy_self() is a horrible function name, make it "destroy". workbench.destroy() is descriptive enough. 2) WorkbenchManager.create_workbench() -> WorkbenchManager.create() We use the pattern "with workbench_manager.create() as workbench:" No need to mention workbenches three times in a row... Signed-off-by: Sebastian Spaeth --- mediagoblin/decorators.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'mediagoblin/decorators.py') diff --git a/mediagoblin/decorators.py b/mediagoblin/decorators.py index 3655d758..804fab7e 100644 --- a/mediagoblin/decorators.py +++ b/mediagoblin/decorators.py @@ -230,7 +230,7 @@ def get_workbench(func): @wraps(func) def new_func(*args, **kwargs): - with mgg.workbench_manager.create_workbench() as workbench: + with mgg.workbench_manager.create() as workbench: return func(*args, workbench=workbench, **kwargs) return new_func -- cgit v1.2.3 From 066d49b2c1c8319a5b7805b4a68a8ee2b1c59ad1 Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Tue, 22 Jan 2013 22:18:08 +0100 Subject: user.get('moo') -> user.moo User fields are always existent, so there is no need to .get() them, just use them directly. Signed-off-by: Sebastian Spaeth --- mediagoblin/decorators.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'mediagoblin/decorators.py') diff --git a/mediagoblin/decorators.py b/mediagoblin/decorators.py index 804fab7e..09235614 100644 --- a/mediagoblin/decorators.py +++ b/mediagoblin/decorators.py @@ -32,11 +32,11 @@ def require_active_login(controller): @wraps(controller) def new_controller_func(request, *args, **kwargs): if request.user and \ - request.user.get('status') == u'needs_email_verification': + request.user.status == u'needs_email_verification': return redirect( request, 'mediagoblin.user_pages.user_home', user=request.user.username) - elif not request.user or request.user.get('status') != u'active': + elif not request.user or request.user.status != u'active': next_url = urljoin( request.urlgen('mediagoblin.auth.login', qualified=True), -- cgit v1.2.3 From 92fae6d8cd7901e74befcda219d509ca8558bd6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A1s=20Veres-Szentkir=C3=A1lyi?= Date: Thu, 21 Feb 2013 10:48:52 +0100 Subject: removed unnecessary collection lookup --- mediagoblin/decorators.py | 4 ---- 1 file changed, 4 deletions(-) (limited to 'mediagoblin/decorators.py') diff --git a/mediagoblin/decorators.py b/mediagoblin/decorators.py index 09235614..42406b7d 100644 --- a/mediagoblin/decorators.py +++ b/mediagoblin/decorators.py @@ -187,10 +187,6 @@ def get_user_collection_item(controller): if not user: return render_404(request) - collection = request.db.Collection.find_one( - {'slug': request.matchdict['collection'], - 'creator': user.id}) - collection_item = request.db.CollectionItem.find_one( {'id': request.matchdict['collection_item'] }) -- cgit v1.2.3 From 67c7c81162c1114093d4588668d5ba4cf83cf902 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Tue, 26 Feb 2013 10:33:51 -0600 Subject: Small PEP-8 compliance fix. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit sponsored by Mats Sjöberg. Thanks! --- mediagoblin/decorators.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'mediagoblin/decorators.py') diff --git a/mediagoblin/decorators.py b/mediagoblin/decorators.py index 42406b7d..85883c1a 100644 --- a/mediagoblin/decorators.py +++ b/mediagoblin/decorators.py @@ -126,9 +126,9 @@ def get_user_media_entry(controller): raise NotFound() media = MediaEntry.query.filter_by( - slug = request.matchdict['media'], - state = u'processed', - uploader = user.id).first() + slug=request.matchdict['media'], + state=u'processed', + uploader=user.id).first() if not media: # no media via slug? Grab it via object id -- cgit v1.2.3 From 7de20e52345949fa9d377f06ab0241d98063d9c9 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Tue, 26 Feb 2013 13:38:11 -0600 Subject: Media URLs with ids in them are now like /u/cwebber/m/id:4112/ rather than /u/cwebber/m/4112/ This avoids some potential name collision issues. This commit sponsored by Asokan Pichai. Thank you! --- mediagoblin/decorators.py | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-) (limited to 'mediagoblin/decorators.py') diff --git a/mediagoblin/decorators.py b/mediagoblin/decorators.py index 85883c1a..14b4f8fc 100644 --- a/mediagoblin/decorators.py +++ b/mediagoblin/decorators.py @@ -125,24 +125,31 @@ def get_user_media_entry(controller): if not user: raise NotFound() - media = MediaEntry.query.filter_by( - slug=request.matchdict['media'], - state=u'processed', - uploader=user.id).first() - - if not media: - # no media via slug? Grab it via object id - try: - media = MediaEntry.query.filter_by( - id = int(request.matchdict['media']), - state = u'processed', - uploader = user.id).first() - except ValueError: - # media "id" was no int - raise NotFound() + media = None + + # might not be a slug, might be an id, but whatever + media_slug = request.matchdict['media'] + + if u":" in request.matchdict['media']: + # okay, it's not actually a slug, it's some kind of identifier, + # probably id: + if media_slug.startswith(u'id:'): + try: + media = MediaEntry.query.filter_by( + id=int(media_slug[3:]), + state=u'processed', + uploader=user.id).first() + except ValueError: + raise NotFound() + else: + # no magical id: stuff? It's a slug! + media = MediaEntry.query.filter_by( + slug=request.matchdict['media'], + state=u'processed', + uploader=user.id).first() if not media: - # no media by that id? Okay, 404. + # Didn't find anything? Okay, 404. raise NotFound() return controller(request, media=media, *args, **kwargs) -- cgit v1.2.3 From 697c74c2de0cc940b45014f9becdfa55f961d193 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Tue, 26 Feb 2013 13:54:19 -0600 Subject: Replacing several request.matchdict['media'] -> media_slug ... shorter! Thanks for pointing this out, Elrond ;) This commit sponsored by Gerardo Joven Valdivia. Thank you! --- mediagoblin/decorators.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'mediagoblin/decorators.py') diff --git a/mediagoblin/decorators.py b/mediagoblin/decorators.py index 14b4f8fc..b6f6f909 100644 --- a/mediagoblin/decorators.py +++ b/mediagoblin/decorators.py @@ -130,7 +130,7 @@ def get_user_media_entry(controller): # might not be a slug, might be an id, but whatever media_slug = request.matchdict['media'] - if u":" in request.matchdict['media']: + if u":" in media_slug: # okay, it's not actually a slug, it's some kind of identifier, # probably id: if media_slug.startswith(u'id:'): @@ -144,7 +144,7 @@ def get_user_media_entry(controller): else: # no magical id: stuff? It's a slug! media = MediaEntry.query.filter_by( - slug=request.matchdict['media'], + slug=media_slug, state=u'processed', uploader=user.id).first() -- cgit v1.2.3 From e4e50a27653bd582e853e3f328ebc67cdd07b0e7 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Tue, 26 Feb 2013 14:04:26 -0600 Subject: Simplifying the "id:" url detection, per Elrond's suggestion. As pointed out, we didn't need that nested if. This commit sponsored by Paul Kuriakose. Thank you! --- mediagoblin/decorators.py | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) (limited to 'mediagoblin/decorators.py') diff --git a/mediagoblin/decorators.py b/mediagoblin/decorators.py index b6f6f909..fbf7b188 100644 --- a/mediagoblin/decorators.py +++ b/mediagoblin/decorators.py @@ -130,17 +130,15 @@ def get_user_media_entry(controller): # might not be a slug, might be an id, but whatever media_slug = request.matchdict['media'] - if u":" in media_slug: - # okay, it's not actually a slug, it's some kind of identifier, - # probably id: - if media_slug.startswith(u'id:'): - try: - media = MediaEntry.query.filter_by( - id=int(media_slug[3:]), - state=u'processed', - uploader=user.id).first() - except ValueError: - raise NotFound() + # if it starts with id: it actually isn't a slug, it's an id. + if media_slug.startswith(u'id:'): + try: + media = MediaEntry.query.filter_by( + id=int(media_slug[3:]), + state=u'processed', + uploader=user.id).first() + except ValueError: + raise NotFound() else: # no magical id: stuff? It's a slug! media = MediaEntry.query.filter_by( -- cgit v1.2.3 From 56c113c77005a889834f43fb13549d531f391be4 Mon Sep 17 00:00:00 2001 From: Joar Wandborg Date: Sat, 2 Mar 2013 23:17:50 +0100 Subject: Do not encode the next kwarg twice --- mediagoblin/decorators.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'mediagoblin/decorators.py') diff --git a/mediagoblin/decorators.py b/mediagoblin/decorators.py index fbf7b188..f3535fcf 100644 --- a/mediagoblin/decorators.py +++ b/mediagoblin/decorators.py @@ -43,7 +43,7 @@ def require_active_login(controller): request.url) return redirect(request, 'mediagoblin.auth.login', - next=url_quote(next_url)) + next=next_url) return controller(request, *args, **kwargs) -- cgit v1.2.3