From f42e49c3ad1c446e7899e7b76cd7fa381d5bba7c Mon Sep 17 00:00:00 2001 From: Elrond Date: Thu, 5 Jan 2012 00:18:17 +0100 Subject: [PATCH 1/6] Add DB Mixin classes and use them A bunch of functions on the db objects are really more like "utility functions": They could live outside the classes and be called "by hand" passing the appropiate reference. They usually only use the public API of the object and rarely use database related stuff. Goals: - First, simple: Share the code with the SQL objects, so that the code doesn't need to be duplicated. - Second, it might unclutter the db models and make them more into "model only" stuff. - Doesn't really hurt. --- mediagoblin/db/mixin.py | 90 ++++++++++++++++++++++++++++++++++ mediagoblin/db/mongo/models.py | 63 ++---------------------- mediagoblin/db/sql/models.py | 5 +- 3 files changed, 97 insertions(+), 61 deletions(-) create mode 100644 mediagoblin/db/mixin.py diff --git a/mediagoblin/db/mixin.py b/mediagoblin/db/mixin.py new file mode 100644 index 00000000..4fb325d2 --- /dev/null +++ b/mediagoblin/db/mixin.py @@ -0,0 +1,90 @@ +# GNU MediaGoblin -- federated, autonomous media hosting +# Copyright (C) 2011,2012 MediaGoblin contributors. See AUTHORS. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +""" +This module contains some Mixin classes for the db objects. + +A bunch of functions on the db objects are really more like +"utility functions": They could live outside the classes +and be called "by hand" passing the appropiate reference. +They usually only use the public API of the object and +rarely use database related stuff. + +These functions now live here and get "mixed in" into the +real objects. +""" + +from mediagoblin.auth import lib as auth_lib +from mediagoblin.tools import common + + +class UserMixin(object): + def check_login(self, password): + """ + See if a user can login with this password + """ + return auth_lib.bcrypt_check_password( + password, self.pw_hash) + + +class MediaEntryMixin(object): + def get_display_media(self, media_map, + fetch_order=common.DISPLAY_IMAGE_FETCHING_ORDER): + """ + Find the best media for display. + + Args: + - media_map: a dict like + {u'image_size': [u'dir1', u'dir2', u'image.jpg']} + - fetch_order: the order we should try fetching images in + + Returns: + (media_size, media_path) + """ + media_sizes = media_map.keys() + + for media_size in common.DISPLAY_IMAGE_FETCHING_ORDER: + if media_size in media_sizes: + return media_map[media_size] + + def main_mediafile(self): + pass + + def url_for_self(self, urlgen): + """ + Generate an appropriate url for ourselves + + Use a slug if we have one, else use our '_id'. + """ + uploader = self.get_uploader + + if self.get('slug'): + return urlgen( + 'mediagoblin.user_pages.media_home', + user=uploader.username, + media=self.slug) + else: + return urlgen( + 'mediagoblin.user_pages.media_home', + user=uploader.username, + media=unicode(self._id)) + + def get_fail_exception(self): + """ + Get the exception that's appropriate for this error + """ + if self['fail_error']: + return common.import_component(self['fail_error']) diff --git a/mediagoblin/db/mongo/models.py b/mediagoblin/db/mongo/models.py index 5de59c12..906d2849 100644 --- a/mediagoblin/db/mongo/models.py +++ b/mediagoblin/db/mongo/models.py @@ -18,12 +18,12 @@ import datetime from mongokit import Document -from mediagoblin.auth import lib as auth_lib from mediagoblin import mg_globals from mediagoblin.db.mongo import migrations from mediagoblin.db.mongo.util import ASCENDING, DESCENDING, ObjectId from mediagoblin.tools.pagination import Pagination -from mediagoblin.tools import url, common +from mediagoblin.tools import url +from mediagoblin.db.mixin import UserMixin, MediaEntryMixin ################### # Custom validators @@ -34,7 +34,7 @@ from mediagoblin.tools import url, common ######## -class User(Document): +class User(Document, UserMixin): """ A user of MediaGoblin. @@ -89,15 +89,8 @@ class User(Document): 'status': u'needs_email_verification', 'is_admin': False} - def check_login(self, password): - """ - See if a user can login with this password - """ - return auth_lib.bcrypt_check_password( - password, self.pw_hash) - -class MediaEntry(Document): +class MediaEntry(Document, MediaEntryMixin): """ Record of a piece of media. @@ -224,28 +217,6 @@ class MediaEntry(Document): return self.db.MediaComment.find({ 'media_entry': self._id}).sort('created', order) - def get_display_media(self, media_map, - fetch_order=common.DISPLAY_IMAGE_FETCHING_ORDER): - """ - Find the best media for display. - - Args: - - media_map: a dict like - {u'image_size': [u'dir1', u'dir2', u'image.jpg']} - - fetch_order: the order we should try fetching images in - - Returns: - (media_size, media_path) - """ - media_sizes = media_map.keys() - - for media_size in common.DISPLAY_IMAGE_FETCHING_ORDER: - if media_size in media_sizes: - return media_map[media_size] - - def main_mediafile(self): - pass - def generate_slug(self): self.slug = url.slugify(self.title) @@ -255,25 +226,6 @@ class MediaEntry(Document): if duplicate: self.slug = "%s-%s" % (self._id, self.slug) - def url_for_self(self, urlgen): - """ - Generate an appropriate url for ourselves - - Use a slug if we have one, else use our '_id'. - """ - uploader = self.get_uploader - - if self.get('slug'): - return urlgen( - 'mediagoblin.user_pages.media_home', - user=uploader.username, - media=self.slug) - else: - return urlgen( - 'mediagoblin.user_pages.media_home', - user=uploader.username, - media=unicode(self._id)) - def url_to_prev(self, urlgen): """ Provide a url to the previous entry from this user, if there is one @@ -301,13 +253,6 @@ class MediaEntry(Document): def get_uploader(self): return self.db.User.find_one({'_id': self.uploader}) - def get_fail_exception(self): - """ - Get the exception that's appropriate for this error - """ - if self['fail_error']: - return common.import_component(self['fail_error']) - class MediaComment(Document): """ diff --git a/mediagoblin/db/sql/models.py b/mediagoblin/db/sql/models.py index 31a6ed3b..95821b4f 100644 --- a/mediagoblin/db/sql/models.py +++ b/mediagoblin/db/sql/models.py @@ -7,6 +7,7 @@ from sqlalchemy import ( from sqlalchemy.orm import relationship from mediagoblin.db.sql.base import GMGTableBase +from mediagoblin.db.mixin import UserMixin, MediaEntryMixin Base = declarative_base(cls=GMGTableBase) @@ -24,7 +25,7 @@ class SimpleFieldAlias(object): setattr(instance, self.fieldname, val) -class User(Base): +class User(Base, UserMixin): __tablename__ = "users" id = Column(Integer, primary_key=True) @@ -48,7 +49,7 @@ class User(Base): _id = SimpleFieldAlias("id") -class MediaEntry(Base): +class MediaEntry(Base, MediaEntryMixin): __tablename__ = "media_entries" id = Column(Integer, primary_key=True) From 7cbbf3e75b2dc67068ec270e53249d95224a86cc Mon Sep 17 00:00:00 2001 From: Elrond Date: Mon, 9 Jan 2012 14:22:28 +0100 Subject: [PATCH 2/6] Create a default logging config paste uses paste.ini to configure python's logging module. Until now, there was NO config, not even a useful default one. This means: any messages went away unseen. Not good. The new default logs everything to stderr at level INFO and higher. Maybe not the best, but a good starting point. --- paste.ini | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/paste.ini b/paste.ini index c729e41d..13c15209 100644 --- a/paste.ini +++ b/paste.ini @@ -19,6 +19,28 @@ use = egg:mediagoblin#app filter-with = beaker config = %(here)s/mediagoblin_local.ini %(here)s/mediagoblin.ini +[loggers] +keys = root + +[handlers] +keys = console + +[formatters] +keys = generic + +[logger_root] +level = INFO +handlers = console + +[handler_console] +class = StreamHandler +args = (sys.stderr,) +level = NOTSET +formatter = generic + +[formatter_generic] +format = %(asctime)s %(levelname)-7.7s [%(name)s] %(message)s + [app:publicstore_serve] use = egg:Paste#static document_root = %(here)s/user_dev/media/public/ From 1b876ac2ea58830b187463dd3de75299fa257212 Mon Sep 17 00:00:00 2001 From: Elrond Date: Mon, 9 Jan 2012 14:26:01 +0100 Subject: [PATCH 3/6] Warn about unknown staticdirect paths. Use pkg_resource to check for the existence of any files referenced by staticdirect. If they don't exist, warn about this. This might raise false warnings in the future for more advanced setups. --- mediagoblin/staticdirect.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/mediagoblin/staticdirect.py b/mediagoblin/staticdirect.py index c6d2b374..2bddb160 100644 --- a/mediagoblin/staticdirect.py +++ b/mediagoblin/staticdirect.py @@ -14,9 +14,6 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . -import pkg_resources -import urlparse - #################################### # Staticdirect infrastructure. # Borrowed largely from cc.engine @@ -26,7 +23,9 @@ import urlparse #################################### import pkg_resources -import urlparse +import logging + +_log = logging.getLogger(__name__) class StaticDirect(object): @@ -37,6 +36,10 @@ class StaticDirect(object): if filepath in self.cache: return self.cache[filepath] + if not pkg_resources.resource_exists('mediagoblin', + 'static' + filepath): + _log.info("StaticDirect resource %r not found locally", + filepath) static_direction = self.cache[filepath] = self.get(filepath) return static_direction From 1dc7f28d2476135f9548a92ec1147659f1a4e810 Mon Sep 17 00:00:00 2001 From: Elrond Date: Mon, 9 Jan 2012 14:33:57 +0100 Subject: [PATCH 4/6] Fix reset.css reference and drop link to video-js.css 1. reset.css was moved to /css/extlib/ some time ago. So update the staticdirect link to it. 2. We don't have video-js.css (any more?). Drop link to it. --- mediagoblin/templates/mediagoblin/base.html | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/mediagoblin/templates/mediagoblin/base.html b/mediagoblin/templates/mediagoblin/base.html index 82ee41b7..5335ebe3 100644 --- a/mediagoblin/templates/mediagoblin/base.html +++ b/mediagoblin/templates/mediagoblin/base.html @@ -22,11 +22,9 @@ {% block title %}{{ app_config['html_title'] }}{% endblock %} + href="{{ request.staticdirect('/css/extlib/reset.css') }}"/> - From c2d6792ddb8d968e0c93a7cdd1da7bdae3b5fa36 Mon Sep 17 00:00:00 2001 From: Elrond Date: Tue, 10 Jan 2012 12:52:01 +0100 Subject: [PATCH 5/6] Test Suite: Enable attachments, add failing test attachments are an optional part. But it doesn't hurt to enable them in the test suite at all. Also (with enabled attachmemtns) the main media view fails, if one isn't logged in (joar found it!). So add a simple (currently failing) test for this. --- mediagoblin/tests/test_mgoblin_app.ini | 3 +++ mediagoblin/tests/test_submission.py | 14 ++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/mediagoblin/tests/test_mgoblin_app.ini b/mediagoblin/tests/test_mgoblin_app.ini index 2525a4f9..c91ed92b 100644 --- a/mediagoblin/tests/test_mgoblin_app.ini +++ b/mediagoblin/tests/test_mgoblin_app.ini @@ -7,6 +7,9 @@ db_name = __mediagoblin_tests__ # tag parsing tags_max_length = 50 +# So we can start to test attachments: +allow_attachments = True + # Celery shouldn't be set up by the application as it's setup via # mediagoblin.init.celery.from_celery celery_setup_elsewhere = true diff --git a/mediagoblin/tests/test_submission.py b/mediagoblin/tests/test_submission.py index 2b17c515..b3c11249 100644 --- a/mediagoblin/tests/test_submission.py +++ b/mediagoblin/tests/test_submission.py @@ -51,11 +51,17 @@ class TestSubmission: self.test_user = test_user + self.login() + + def login(self): self.test_app.post( '/auth/login/', { 'username': u'chris', 'password': 'toast'}) + def logout(self): + self.test_app.get('/auth/logout/') + def test_missing_fields(self): # Test blank form # --------------- @@ -95,6 +101,14 @@ class TestSubmission: assert template.TEMPLATE_TEST_CONTEXT.has_key( 'mediagoblin/user_pages/user.html') + # Make sure the media view is at least reachable, logged in... + self.test_app.get('/u/chris/m/normal-upload-1/') + # ... and logged out too. + self.logout() + self.test_app.get('/u/chris/m/normal-upload-1/') + # Log back in for the remaining tests. + self.login() + # Test PNG # -------- template.clear_test_template_context() From 914b8bcde3e01c2dd3e5679fb7733fc194b34d68 Mon Sep 17 00:00:00 2001 From: Joar Wandborg Date: Tue, 10 Jan 2012 13:12:14 +0100 Subject: [PATCH 6/6] Added check for request.user to media.html attachment-related conditional --- mediagoblin/templates/mediagoblin/user_pages/media.html | 1 + 1 file changed, 1 insertion(+) diff --git a/mediagoblin/templates/mediagoblin/user_pages/media.html b/mediagoblin/templates/mediagoblin/user_pages/media.html index 10525f4c..583e4ebd 100644 --- a/mediagoblin/templates/mediagoblin/user_pages/media.html +++ b/mediagoblin/templates/mediagoblin/user_pages/media.html @@ -158,6 +158,7 @@ {% endif %} {% if app_config['allow_attachments'] + and request.user and (media.uploader == request.user._id or request.user.is_admin) %}