diff options
| -rw-r--r-- | docs/codebase.rst | 4 | ||||
| -rw-r--r-- | docs/conf.py | 4 | ||||
| -rw-r--r-- | docs/contributinghowto.rst | 4 | ||||
| -rw-r--r-- | docs/designdecisions.rst | 4 | ||||
| -rw-r--r-- | docs/git.rst | 198 | ||||
| -rw-r--r-- | docs/hackinghowto.rst | 4 | ||||
| -rw-r--r-- | mediagoblin/app.py | 2 | ||||
| -rw-r--r-- | mediagoblin/auth/lib.py | 4 | ||||
| -rw-r--r-- | mediagoblin/celery_setup/from_celery.py | 8 | ||||
| -rw-r--r-- | mediagoblin/celery_setup/from_tests.py | 5 | ||||
| -rw-r--r-- | mediagoblin/db/migrations.py | 2 | ||||
| -rw-r--r-- | mediagoblin/db/models.py | 4 | ||||
| -rw-r--r-- | mediagoblin/gmg_commands/migrate.py | 1 | ||||
| -rw-r--r-- | mediagoblin/gmg_commands/shell.py | 8 | ||||
| -rw-r--r-- | mediagoblin/mg_globals.py (renamed from mediagoblin/globals.py) | 2 | ||||
| -rw-r--r-- | mediagoblin/process_media/__init__.py | 39 | ||||
| -rw-r--r-- | mediagoblin/tests/__init__.py | 11 | ||||
| -rw-r--r-- | mediagoblin/tests/test_auth.py | 10 | ||||
| -rw-r--r-- | mediagoblin/tests/test_globals.py | 2 | ||||
| -rw-r--r-- | mediagoblin/tests/test_tests.py | 10 | ||||
| -rw-r--r-- | mediagoblin/tests/test_util.py | 19 | ||||
| -rw-r--r-- | mediagoblin/tests/test_workbench.py | 50 | ||||
| -rw-r--r-- | mediagoblin/util.py | 41 | ||||
| -rw-r--r-- | mediagoblin/workbench.py | 89 |
24 files changed, 372 insertions, 153 deletions
diff --git a/docs/codebase.rst b/docs/codebase.rst index 4f5f215f..898eadfe 100644 --- a/docs/codebase.rst +++ b/docs/codebase.rst @@ -4,6 +4,10 @@ Codebase Documentation ======================== +.. contents:: Sections + :local: + + This chapter covers the libraries that GNU MediaGoblin uses as well as various recipes for getting things done. diff --git a/docs/conf.py b/docs/conf.py index fedaf33c..0e75a617 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -48,9 +48,9 @@ copyright = u'2011, Free Software Foundation, Inc and contributors' # built documents. # # The short X.Y version. -version = '0.0.1' +version = '0.0.2' # The full version, including alpha/beta/rc tags. -release = '0.0.1' +release = '0.0.2' # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. diff --git a/docs/contributinghowto.rst b/docs/contributinghowto.rst index e980a5e0..06d2814e 100644 --- a/docs/contributinghowto.rst +++ b/docs/contributinghowto.rst @@ -4,6 +4,10 @@ Contributing HOWTO ==================== +.. contents:: Sections + :local: + + .. _join-the-community-section: Join the community! diff --git a/docs/designdecisions.rst b/docs/designdecisions.rst index 50dfe3e8..afa1e26b 100644 --- a/docs/designdecisions.rst +++ b/docs/designdecisions.rst @@ -4,6 +4,10 @@ Design Decisions ================== +.. contents:: Sections + :local: + + This chapter talks a bit about design decisions. diff --git a/docs/git.rst b/docs/git.rst index 0db1dacf..8eb038b2 100644 --- a/docs/git.rst +++ b/docs/git.rst @@ -2,11 +2,21 @@ Git, Cloning and Patches ========================== -GNU MediaGoblin uses git for all our version control and we have -the repositories hosted on `Gitorious <http://gitorious.org/>`_. +.. contents:: Sections + :local: -We have two repositories. One is for the project and the other is for -the project website. + +GNU MediaGoblin uses git for all our version control and we have the +repositories hosted on `Gitorious <http://gitorious.org/>`_. We have +two repositories: + +* MediaGoblin software: http://gitorious.org/mediagoblin/mediagoblin +* MediaGoblin website: http://gitorious.org/mediagoblin/mediagoblin-website + +It's most likely you want to look at the software repository--not the +website one. + +The rest of this chapter talks about using the software repository. How to clone the project @@ -17,49 +27,173 @@ Do:: git clone git://gitorious.org/mediagoblin/mediagoblin.git -How to send in patches -====================== +How to contribute changes +========================= + +Tie your changes to issues in the issue tracker +----------------------------------------------- All patches should be tied to issues in the `issue tracker -<http://bugs.foocorp.net/projects/mediagoblin/issues>`_. -That makes it a lot easier for everyone to track proposed changes and -make sure your hard work doesn't get dropped on the floor! +<http://bugs.foocorp.net/projects/mediagoblin/issues>`_. That makes +it a lot easier for everyone to track proposed changes and make sure +your hard work doesn't get dropped on the floor! If there isn't an +issue for what you're working on, please create one. The better the +description of what it is you're trying to fix/implement, the better +everyone else is able to understand why you're doing what you're +doing. + + +Use bugfix branches to make changes +----------------------------------- + +The best way to isolate your changes is to create a branch based off +of the MediaGoblin repository master branch, do the changes related to +that one issue there, and then let us know how to get it. + +It's much easier on us if you isolate your changes to a branch focused +on the issue. Then we don't have to sift through things. + +It's much easier on you if you isolate your changes to a branch +focused on the issue. Then when we merge your changes in, you just +have to do a ``git fetch`` and that's it. This is especially true if +we reject some of your changes, but accept others or otherwise tweak +your changes. + +Further, if you isolate your changes to a branch, then you can work on +multiple issues at the same time and they don't conflict with one +another. + + +Properly document your changes +------------------------------ + +Include comments in the code. + +Write comprehensive commit messages. The better your commit message +is at describing what you did and why, the easier it is for us to +quickly accept your patch. + +Write comprehensive comments in the issue tracker about what you're +doing and why. + + +How to send us your changes +--------------------------- + +There are three ways to let us know how to get it: + +1. (preferred) **push changes to publicly available git clone and let + us know where to find it** + + Push your feature/bugfix/issue branch to your publicly available + git clone and add a comment to the issue with the url for your + clone and the branch to look at. + +2. **attaching the patch files to the issue** + + Run:: + + git format-patch -o patches <remote>/master + + Then tar up the newly created ``patches`` directory and attach the + directory to the issue. + + +Example workflow +================ +Here's an example workflow. + + +Contributing changes +-------------------- + +Slartibartfast from the planet Magrathea far off in the universe has +decided that he is bored with fjords and wants to fix issue 42 and +send us the changes. + +Slartibartfast has cloned the MediaGoblin repository and his clone +lives on gitorious. + +Slartibartfast works locally. The remote named ``origin`` points to +his clone on gitorious. The remote named ``gmg`` points to the +MediaGoblin repository. + +Slartibartfast does the following: + +1. Fetches the latest from the MediaGoblin repository:: + + git fetch --all -p + +2. Creates a branch from the tip of the MediaGoblin repository (the + remote is named ``gmg``) master branch called ``issue_42``:: + + git checkout -b issue_42 gmg/master + +3. Slartibartfast works hard on his changes in the ``issue_42`` + branch. When done, he wants to notify us that he has made changes + he wants us to see. + +4. Slartibartfast pushes his changes to his clone (the remote is named + ``origin``):: + + git push origin issue_42 + +5. Slartibartfast adds a comment to issue 42 with the url for his + repository and the name of the branch he put the code in. He also + explains what he did and why it addresses the issue. + + +Updating a contribution +----------------------- + +Slartibartfast brushes his hands off with the sense of accomplishment +that comes with the knowledge of a job well done. He stands, wanders +over to get a cup of water, then realizes that he forgot to run the +unit tests! + +He runs the unit tests and discovers there's a bug in the code! + +Then he does this: + +1. He checks out the ``issue_42`` branch:: -If there isn't an issue for what you're working on, please create -one. The better the description of what it is you're trying to -fix/implement, the better everyone else is able to understand why -you're doing what you're doing. + git checkout issue_42 -There are two ways you could send in a patch. +2. He fixes the bug and checks it into the ``issue_42`` branch. +3. He pushes his changes to his clone (the remote is named ``origin``):: -How to send in a patch from a publicly available clone ------------------------------------------------------- + git push origin issue_42 -Add a comment to the issue you're working on with the following bits -of information: +4. He adds another comment to issue 42 explaining about the mistake + and how he fixed it and that he's pushed the new change to the + ``issue_42`` branch of his publicly available clone. -* the url for your clone -* the revs you want looked at -* any details, questions, or other things that should be known +What happens next +----------------- -How to send in a patch if you don't have a publicly available clone -------------------------------------------------------------------- +Slartibartfast is once again happy with his work. He finds issue 42 +in the issue tracker and adds a comment saying he submitted a merge +request with his changes and explains what they are. -Assuming that the remote is our repository on gitorious and the branch -to compare against is master, do the following: +Later, someone checks out his code and finds a problem with it. He +adds a comment to the issue tracker specifying the problem and asks +Slartibartfast to fix it. Slartibartfst goes through the above steps +again, fixes the issue, pushes it to his ``issue_42`` branch and adds +another comment to the issue tracker about how he fixed it. -1. checkout the branch you did your work in -2. do:: +Later, someone checks out his code and is happy with it. Someone +pulls it into the master branch of the MediaGoblin repository and adds +another comment to the issue and probably closes the issue out. - git format-patch -o patches origin/master +Slartibartfast is notified of this. Slartibartfast does a:: -3. either: + git fetch --all - * tar up and attach the tarball to the issue you're working on, OR - * attach the patch files to the issue you're working on one at a - time +The changes show up in the ``master`` branch of the ``gmg`` remote. +Slartibartfast now deletes his ``issue_42`` branch because he doesn't +need it anymore. How to learn git diff --git a/docs/hackinghowto.rst b/docs/hackinghowto.rst index a9aadb62..d8bb9330 100644 --- a/docs/hackinghowto.rst +++ b/docs/hackinghowto.rst @@ -4,6 +4,10 @@ Hacking HOWTO =============== +.. contents:: Sections + :local: + + So you want to hack on GNU MediaGoblin? ======================================= diff --git a/mediagoblin/app.py b/mediagoblin/app.py index 5d594039..a1c6b512 100644 --- a/mediagoblin/app.py +++ b/mediagoblin/app.py @@ -23,7 +23,7 @@ from webob import Request, exc from mediagoblin import routing, util, storage, staticdirect from mediagoblin.db.open import setup_connection_and_db_from_config -from mediagoblin.globals import setup_globals +from mediagoblin.mg_globals import setup_globals from mediagoblin.celery_setup import setup_celery_from_config from mediagoblin.workbench import WorkbenchManager, DEFAULT_WORKBENCH_DIR diff --git a/mediagoblin/auth/lib.py b/mediagoblin/auth/lib.py index f40e560f..08bbdd16 100644 --- a/mediagoblin/auth/lib.py +++ b/mediagoblin/auth/lib.py @@ -20,7 +20,7 @@ import random import bcrypt from mediagoblin.util import send_email, render_template -from mediagoblin import globals as mgoblin_globals +from mediagoblin import mg_globals def bcrypt_check_password(raw_pass, stored_hash, extra_salt=None): @@ -112,7 +112,7 @@ def send_verification_email(user, request): # TODO: There is no error handling in place send_email( - mgoblin_globals.email_sender_address, + mg_globals.email_sender_address, [user['email']], # TODO # Due to the distributed nature of GNU MediaGoblin, we should diff --git a/mediagoblin/celery_setup/from_celery.py b/mediagoblin/celery_setup/from_celery.py index 5fa9ba76..c8ccebc8 100644 --- a/mediagoblin/celery_setup/from_celery.py +++ b/mediagoblin/celery_setup/from_celery.py @@ -22,14 +22,14 @@ from paste.deploy.converters import asbool from mediagoblin import storage from mediagoblin.db.open import setup_connection_and_db_from_config from mediagoblin.celery_setup import setup_celery_from_config -from mediagoblin.globals import setup_globals +from mediagoblin.mg_globals import setup_globals from mediagoblin.workbench import WorkbenchManager, DEFAULT_WORKBENCH_DIR -OUR_MODULENAME = 'mediagoblin.celery_setup.from_celery' +OUR_MODULENAME = __name__ -def setup_self(setup_globals_func=setup_globals): +def setup_self(): """ Transform this module into a celery config module by reading the mediagoblin config file. Set the environment variable @@ -80,7 +80,7 @@ def setup_self(setup_globals_func=setup_globals): mgoblin_section.get( 'workbench_path', DEFAULT_WORKBENCH_DIR)) - setup_globals_func( + setup_globals( db_connection=connection, database=db, public_store=public_store, diff --git a/mediagoblin/celery_setup/from_tests.py b/mediagoblin/celery_setup/from_tests.py index fe7d7314..70814075 100644 --- a/mediagoblin/celery_setup/from_tests.py +++ b/mediagoblin/celery_setup/from_tests.py @@ -19,13 +19,12 @@ import os from mediagoblin.tests.tools import TEST_APP_CONFIG from mediagoblin import util from mediagoblin.celery_setup import setup_celery_from_config -from mediagoblin.globals import setup_globals -OUR_MODULENAME = 'mediagoblin.celery_setup.from_tests' +OUR_MODULENAME = __name__ -def setup_self(setup_globals_func=setup_globals): +def setup_self(): """ Set up celery for testing's sake, which just needs to set up celery and celery only. diff --git a/mediagoblin/db/migrations.py b/mediagoblin/db/migrations.py index d035b15b..f1f625b7 100644 --- a/mediagoblin/db/migrations.py +++ b/mediagoblin/db/migrations.py @@ -16,8 +16,6 @@ from mongokit import DocumentMigration -from mediagoblin import globals as mediagoblin_globals - class MediaEntryMigration(DocumentMigration): def allmigration01_uploader_to_reference(self): diff --git a/mediagoblin/db/models.py b/mediagoblin/db/models.py index 0b092d60..e034cc29 100644 --- a/mediagoblin/db/models.py +++ b/mediagoblin/db/models.py @@ -20,7 +20,7 @@ from mongokit import Document, Set from mediagoblin import util from mediagoblin.auth import lib as auth_lib -from mediagoblin import globals as mediagoblin_globals +from mediagoblin import mg_globals from mediagoblin.db import migrations from mediagoblin.db.util import ObjectId @@ -115,7 +115,7 @@ class MediaEntry(Document): def generate_slug(self): self['slug'] = util.slugify(self['title']) - duplicate = mediagoblin_globals.database.media_entries.find_one( + duplicate = mg_globals.database.media_entries.find_one( {'slug': self['slug']}) if duplicate: diff --git a/mediagoblin/gmg_commands/migrate.py b/mediagoblin/gmg_commands/migrate.py index e04fb343..3ce25701 100644 --- a/mediagoblin/gmg_commands/migrate.py +++ b/mediagoblin/gmg_commands/migrate.py @@ -17,7 +17,6 @@ from mediagoblin.db import migrations from mediagoblin.gmg_commands import util as commands_util -from mediagoblin import globals as mgoblin_globals def migrate_parser_setup(subparser): diff --git a/mediagoblin/gmg_commands/shell.py b/mediagoblin/gmg_commands/shell.py index 9c0259de..16caf398 100644 --- a/mediagoblin/gmg_commands/shell.py +++ b/mediagoblin/gmg_commands/shell.py @@ -17,7 +17,7 @@ import code -from mediagoblin import globals as mgoblin_globals +from mediagoblin import mg_globals from mediagoblin.gmg_commands import util as commands_util @@ -35,7 +35,7 @@ GNU MediaGoblin shell! ---------------------- Available vars: - mgoblin_app: instantiated mediagoblin application - - mgoblin_globals: mediagoblin.globals + - mg_globals: mediagoblin.globals - db: database instance """ @@ -50,5 +50,5 @@ def shell(args): banner=SHELL_BANNER, local={ 'mgoblin_app': mgoblin_app, - 'mgoblin_globals': mgoblin_globals, - 'db': mgoblin_globals.database}) + 'mg_globals': mg_globals, + 'db': mg_globals.database}) diff --git a/mediagoblin/globals.py b/mediagoblin/mg_globals.py index 80d1f01d..2fca3c0a 100644 --- a/mediagoblin/globals.py +++ b/mediagoblin/mg_globals.py @@ -27,7 +27,7 @@ translations = gettext.find( def setup_globals(**kwargs): - from mediagoblin import globals as mg_globals + from mediagoblin import mg_globals for key, value in kwargs.iteritems(): setattr(mg_globals, key, value) diff --git a/mediagoblin/process_media/__init__.py b/mediagoblin/process_media/__init__.py index 531eb16d..0dce1418 100644 --- a/mediagoblin/process_media/__init__.py +++ b/mediagoblin/process_media/__init__.py @@ -18,22 +18,29 @@ import Image from mediagoblin.db.util import ObjectId from celery.task import task -from mediagoblin import globals as mg_globals +from mediagoblin import mg_globals as mgg THUMB_SIZE = 200, 200 +def create_pub_filepath(entry, filename): + return mgg.public_store.get_unique_filepath( + ['media_entries', + unicode(entry['_id']), + filename]) + + @task def process_media_initial(media_id): - workbench = mg_globals.workbench_manager.create_workbench() + workbench = mgg.workbench_manager.create_workbench() - entry = mg_globals.database.MediaEntry.one( + entry = mgg.database.MediaEntry.one( {'_id': ObjectId(media_id)}) queued_filepath = entry['queued_media_file'] - queued_filename = mg_globals.workbench_manager.localized_file( - workbench, mg_globals.queue_store, queued_filepath, + queued_filename = workbench.localized_file( + mgg.queue_store, queued_filepath, 'source') queued_file = file(queued_filename, 'r') @@ -41,13 +48,13 @@ def process_media_initial(media_id): with queued_file: thumb = Image.open(queued_file) thumb.thumbnail(THUMB_SIZE, Image.ANTIALIAS) + # ensure color mode is compatible with jpg + if thumb.mode != "RGB": + thumb = thumb.convert("RGB") - thumb_filepath = mg_globals.public_store.get_unique_filepath( - ['media_entries', - unicode(entry['_id']), - 'thumbnail.jpg']) + thumb_filepath = create_pub_filepath(entry, 'thumbnail.jpg') - thumb_file = mg_globals.public_store.get_file(thumb_filepath, 'w') + thumb_file = mgg.public_store.get_file(thumb_filepath, 'w') with thumb_file: thumb.save(thumb_file, "JPEG") @@ -56,15 +63,13 @@ def process_media_initial(media_id): queued_file = file(queued_filename, 'rb') with queued_file: - main_filepath = mg_globals.public_store.get_unique_filepath( - ['media_entries', - unicode(entry['_id']), - queued_filepath[-1]]) + main_filepath = create_pub_filepath(entry, queued_filepath[-1]) - with mg_globals.public_store.get_file(main_filepath, 'wb') as main_file: + with mgg.public_store.get_file(main_filepath, 'wb') as main_file: main_file.write(queued_file.read()) - mg_globals.queue_store.delete_file(queued_filepath) + mgg.queue_store.delete_file(queued_filepath) + entry['queued_media_file'] = [] media_files_dict = entry.setdefault('media_files', {}) media_files_dict['thumb'] = thumb_filepath media_files_dict['main'] = main_filepath @@ -72,4 +77,4 @@ def process_media_initial(media_id): entry.save() # clean up workbench - mg_globals.workbench_manager.destroy_workbench(workbench) + workbench.destroy_self() diff --git a/mediagoblin/tests/__init__.py b/mediagoblin/tests/__init__.py index c129cbf8..e9e2a59a 100644 --- a/mediagoblin/tests/__init__.py +++ b/mediagoblin/tests/__init__.py @@ -13,3 +13,14 @@ # # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see <http://www.gnu.org/licenses/>. + +from mediagoblin import mg_globals + + +def setup_package(): + pass + +def teardown_package(): + print "Killing db ..." + mg_globals.db_connection.drop_database(mg_globals.database.name) + print "... done" diff --git a/mediagoblin/tests/test_auth.py b/mediagoblin/tests/test_auth.py index cdfeccab..3d569093 100644 --- a/mediagoblin/tests/test_auth.py +++ b/mediagoblin/tests/test_auth.py @@ -20,7 +20,7 @@ from nose.tools import assert_equal from mediagoblin.auth import lib as auth_lib from mediagoblin.tests.tools import setup_fresh_app -from mediagoblin import globals as mgoblin_globals +from mediagoblin import mg_globals from mediagoblin import util @@ -137,7 +137,7 @@ def test_register_views(test_app): u'Passwords must match.'] ## At this point there should be no users in the database ;) - assert not mgoblin_globals.database.User.find().count() + assert not mg_globals.database.User.find().count() # Successful register # ------------------- @@ -158,7 +158,7 @@ def test_register_views(test_app): 'mediagoblin/auth/register_success.html') ## Make sure user is in place - new_user = mgoblin_globals.database.User.find_one( + new_user = mg_globals.database.User.find_one( {'username': 'happygirl'}) assert new_user assert new_user['status'] == u'needs_email_verification' @@ -191,7 +191,7 @@ def test_register_views(test_app): context = util.TEMPLATE_TEST_CONTEXT[ 'mediagoblin/auth/verify_email.html'] assert context['verification_successful'] == False - new_user = mgoblin_globals.database.User.find_one( + new_user = mg_globals.database.User.find_one( {'username': 'happygirl'}) assert new_user assert new_user['status'] == u'needs_email_verification' @@ -203,7 +203,7 @@ def test_register_views(test_app): context = util.TEMPLATE_TEST_CONTEXT[ 'mediagoblin/auth/verify_email.html'] assert context['verification_successful'] == True - new_user = mgoblin_globals.database.User.find_one( + new_user = mg_globals.database.User.find_one( {'username': 'happygirl'}) assert new_user assert new_user['status'] == u'active' diff --git a/mediagoblin/tests/test_globals.py b/mediagoblin/tests/test_globals.py index 6d2e01da..59d217f3 100644 --- a/mediagoblin/tests/test_globals.py +++ b/mediagoblin/tests/test_globals.py @@ -14,7 +14,7 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see <http://www.gnu.org/licenses/>. -from mediagoblin import globals as mg_globals +from mediagoblin import mg_globals def test_setup_globals(): mg_globals.setup_globals( diff --git a/mediagoblin/tests/test_tests.py b/mediagoblin/tests/test_tests.py index 3ecbfac7..8ac7f0a4 100644 --- a/mediagoblin/tests/test_tests.py +++ b/mediagoblin/tests/test_tests.py @@ -16,7 +16,7 @@ from mediagoblin.tests.tools import get_test_app -from mediagoblin import globals as mgoblin_globals +from mediagoblin import mg_globals def test_get_test_app_wipes_db(): @@ -24,15 +24,15 @@ def test_get_test_app_wipes_db(): Make sure we get a fresh database on every wipe :) """ get_test_app() - assert mgoblin_globals.database.User.find().count() == 0 + assert mg_globals.database.User.find().count() == 0 - new_user = mgoblin_globals.database.User() + new_user = mg_globals.database.User() new_user['username'] = u'lolcat' new_user['email'] = u'lol@cats.example.org' new_user['pw_hash'] = u'pretend_this_is_a_hash' new_user.save() - assert mgoblin_globals.database.User.find().count() == 1 + assert mg_globals.database.User.find().count() == 1 get_test_app() - assert mgoblin_globals.database.User.find().count() == 0 + assert mg_globals.database.User.find().count() == 0 diff --git a/mediagoblin/tests/test_util.py b/mediagoblin/tests/test_util.py index 7b00a074..75e28aca 100644 --- a/mediagoblin/tests/test_util.py +++ b/mediagoblin/tests/test_util.py @@ -103,3 +103,22 @@ def test_locale_to_lower_lower(): # crazy renditions. Useful? assert util.locale_to_lower_lower('en-US') == 'en-us' assert util.locale_to_lower_lower('en_us') == 'en-us' + + +def test_html_cleaner(): + # Remove images + result = util.clean_html( + '<p>Hi everybody! ' + '<img src="http://example.org/huge-purple-barney.png" /></p>\n' + '<p>:)</p>') + assert result == ( + '<div>' + '<p>Hi everybody! </p>\n' + '<p>:)</p>' + '</div>') + + # Remove evil javascript + result = util.clean_html( + '<p><a href="javascript:nasty_surprise">innocent link!</a></p>') + assert result == ( + '<p><a href="">innocent link!</a></p>') diff --git a/mediagoblin/tests/test_workbench.py b/mediagoblin/tests/test_workbench.py index 89f2ef33..953835a1 100644 --- a/mediagoblin/tests/test_workbench.py +++ b/mediagoblin/tests/test_workbench.py @@ -30,29 +30,29 @@ class TestWorkbench(object): def test_create_workbench(self): workbench = self.workbench_manager.create_workbench() - assert os.path.isdir(workbench) - assert workbench.startswith(self.workbench_manager.base_workbench_dir) + assert os.path.isdir(workbench.dir) + assert workbench.dir.startswith(self.workbench_manager.base_workbench_dir) + + def test_joinpath(self): + this_workbench = self.workbench_manager.create_workbench() + tmpname = this_workbench.joinpath('temp.txt') + assert tmpname == os.path.join(this_workbench.dir, 'temp.txt') + this_workbench.destroy_self() def test_destroy_workbench(self): # kill a workbench this_workbench = self.workbench_manager.create_workbench() - tmpfile = file(os.path.join(this_workbench, 'temp.txt'), 'w') + tmpfile_name = this_workbench.joinpath('temp.txt') + tmpfile = file(tmpfile_name, 'w') with tmpfile: tmpfile.write('lollerskates') - assert os.path.exists(os.path.join(this_workbench, 'temp.txt')) - - self.workbench_manager.destroy_workbench(this_workbench) - assert not os.path.exists(os.path.join(this_workbench, 'temp.txt')) - assert not os.path.exists(this_workbench) - - # make sure we can't kill other stuff though - dont_kill_this = tempfile.mkdtemp() + assert os.path.exists(tmpfile_name) - assert_raises( - workbench.WorkbenchOutsideScope, - self.workbench_manager.destroy_workbench, - dont_kill_this) + wb_dir = this_workbench.dir + this_workbench.destroy_self() + assert not os.path.exists(tmpfile_name) + assert not os.path.exists(wb_dir) def test_localized_file(self): tmpdir, this_storage = get_tmp_filestorage() @@ -65,8 +65,7 @@ class TestWorkbench(object): our_file.write('Our file') # with a local file storage - filename = self.workbench_manager.localized_file( - this_workbench, this_storage, filepath) + filename = this_workbench.localized_file(this_storage, filepath) assert filename == os.path.join( tmpdir, 'dir1/dir2/ourfile.txt') @@ -77,20 +76,19 @@ class TestWorkbench(object): with this_storage.get_file(filepath, 'w') as our_file: our_file.write('Our file') - filename = self.workbench_manager.localized_file( - this_workbench, this_storage, filepath) + filename = this_workbench.localized_file(this_storage, filepath) assert filename == os.path.join( - this_workbench, 'ourfile.txt') + this_workbench.dir, 'ourfile.txt') # fake remote file storage, filename_if_copying set - filename = self.workbench_manager.localized_file( - this_workbench, this_storage, filepath, 'thisfile') + filename = this_workbench.localized_file( + this_storage, filepath, 'thisfile') assert filename == os.path.join( - this_workbench, 'thisfile.txt') + this_workbench.dir, 'thisfile.txt') # fake remote file storage, filename_if_copying set, # keep_extension_if_copying set to false - filename = self.workbench_manager.localized_file( - this_workbench, this_storage, filepath, 'thisfile.text', False) + filename = this_workbench.localized_file( + this_storage, filepath, 'thisfile.text', False) assert filename == os.path.join( - this_workbench, 'thisfile.text') + this_workbench.dir, 'thisfile.text') diff --git a/mediagoblin/util.py b/mediagoblin/util.py index 1e8fa095..eb131830 100644 --- a/mediagoblin/util.py +++ b/mediagoblin/util.py @@ -30,8 +30,9 @@ import jinja2 import translitcodec from paste.deploy.loadwsgi import NicerConfigParser from webob import Response, exc +from lxml.html.clean import Cleaner -from mediagoblin import globals as mgoblin_globals +from mediagoblin import mg_globals from mediagoblin.db.util import ObjectId TESTS_ENABLED = False @@ -101,8 +102,8 @@ def get_jinja_env(template_loader, locale): extensions=['jinja2.ext.i18n', 'jinja2.ext.autoescape']) template_env.install_gettext_callables( - mgoblin_globals.translations.gettext, - mgoblin_globals.translations.ngettext) + mg_globals.translations.gettext, + mg_globals.translations.ngettext) if exists(locale): SETUP_JINJA_ENVS[locale] = template_env @@ -263,9 +264,9 @@ def send_email(from_addr, to_addrs, subject, message_body): - message_body: email body text """ # TODO: make a mock mhost if testing is enabled - if TESTS_ENABLED or mgoblin_globals.email_debug_mode: + if TESTS_ENABLED or mg_globals.email_debug_mode: mhost = FakeMhost() - elif not mgoblin_globals.email_debug_mode: + elif not mg_globals.email_debug_mode: mhost = smtplib.SMTP() mhost.connect() @@ -278,7 +279,7 @@ def send_email(from_addr, to_addrs, subject, message_body): if TESTS_ENABLED: EMAIL_TEST_INBOX.append(message) - if getattr(mgoblin_globals, 'email_debug_mode', False): + if getattr(mg_globals, 'email_debug_mode', False): print u"===== Email =====" print u"From address: %s" % message['From'] print u"To addresses: %s" % message['To'] @@ -372,6 +373,32 @@ def read_config_file(conf_file): return mgoblin_conf +# A super strict version of the lxml.html cleaner class +HTML_CLEANER = Cleaner( + scripts=True, + javascript=True, + comments=True, + style=True, + links=True, + page_structure=True, + processing_instructions=True, + embedded=True, + frames=True, + forms=True, + annoying_tags=True, + allow_tags=[ + 'div', 'b', 'i', 'em', 'strong', 'p', 'ul', 'ol', 'li', 'a', 'br'], + remove_unknown_tags=False, # can't be used with allow_tags + safe_attrs_only=True, + add_nofollow=True, # for now + host_whitelist=(), + whitelist_tags=set([])) + + +def clean_html(html): + return HTML_CLEANER.clean_html(html) + + SETUP_GETTEXTS = {} def setup_gettext(locale): @@ -392,7 +419,7 @@ def setup_gettext(locale): if exists(locale): SETUP_GETTEXTS[locale] = this_gettext - mgoblin_globals.setup_globals( + mg_globals.setup_globals( translations=this_gettext) diff --git a/mediagoblin/workbench.py b/mediagoblin/workbench.py index d7252623..f83c4fa0 100644 --- a/mediagoblin/workbench.py +++ b/mediagoblin/workbench.py @@ -23,54 +23,34 @@ DEFAULT_WORKBENCH_DIR = os.path.join( tempfile.gettempdir(), u'mgoblin_workbench') -# Exception(s) -# ------------ - -class WorkbenchOutsideScope(Exception): - """ - Raised when a workbench is outside a WorkbenchManager scope. - """ - pass - - # Actual workbench stuff # ---------------------- -class WorkbenchManager(object): +class Workbench(object): """ - A system for generating and destroying workbenches. + Represent the directory for the workbench - Workbenches are actually just subdirectories of a temporary storage space - for during the processing stage. + WARNING: DO NOT create Workbench objects on your own, + let the WorkbenchManager do that for you! """ - - def __init__(self, base_workbench_dir): - self.base_workbench_dir = os.path.abspath(base_workbench_dir) - if not os.path.exists(self.base_workbench_dir): - os.makedirs(self.base_workbench_dir) - - def create_workbench(self): + def __init__(self, dir): """ - Create and return the path to a new workbench (directory). + WARNING: DO NOT create Workbench objects on your own, + let the WorkbenchManager do that for you! """ - return tempfile.mkdtemp(dir=self.base_workbench_dir) + self.dir = dir - def destroy_workbench(self, workbench): - """ - Destroy this workbench! Deletes the directory and all its contents! + def __unicode__(self): + return unicode(self.dir) + def __str__(self): + return str(self.dir) + def __repr__(self): + return repr(self.dir) - Makes sure the workbench actually belongs to this manager though. - """ - # just in case - workbench = os.path.abspath(workbench) - - if not workbench.startswith(self.base_workbench_dir): - raise WorkbenchOutsideScope( - "Can't destroy workbench outside the base workbench dir") - - shutil.rmtree(workbench) + def joinpath(self, *args): + return os.path.join(self.dir, *args) - def localized_file(self, workbench, storage, filepath, + def localized_file(self, storage, filepath, filename_if_copying=None, keep_extension_if_copying=True): """ @@ -126,10 +106,43 @@ class WorkbenchManager(object): dest_filename = filename_if_copying full_dest_filename = os.path.join( - workbench, dest_filename) + self.dir, dest_filename) # copy it over storage.copy_locally( filepath, full_dest_filename) return full_dest_filename + + def destroy_self(self): + """ + Destroy this workbench! Deletes the directory and all its contents! + + WARNING: Does no checks for a sane value in self.dir! + """ + # just in case + workbench = os.path.abspath(self.dir) + + shutil.rmtree(workbench) + + del self.dir + + +class WorkbenchManager(object): + """ + A system for generating and destroying workbenches. + + Workbenches are actually just subdirectories of a temporary storage space + for during the processing stage. + """ + + def __init__(self, base_workbench_dir): + self.base_workbench_dir = os.path.abspath(base_workbench_dir) + if not os.path.exists(self.base_workbench_dir): + os.makedirs(self.base_workbench_dir) + + def create_workbench(self): + """ + Create and return the path to a new workbench (directory). + """ + return Workbench(tempfile.mkdtemp(dir=self.base_workbench_dir)) |