Merge branch 'master' of http://git.gitorious.org/mediagoblin/mediagoblin

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.
 

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.

docs/contributinghowto.rst

@@ -4,6 +4,10 @@
  Contributing HOWTO
 ====================
 
+.. contents:: Sections
+   :local:
+
+
 .. _join-the-community-section:
 
 Join the community!

docs/designdecisions.rst

@@ -4,6 +4,10 @@
  Design Decisions
 ==================
 
+.. contents:: Sections
+   :local:
+
+
 This chapter talks a bit about design decisions.
 
 

docs/git.rst

@@ -2,11 +2,21 @@
  Git, Cloning and Patches
 ==========================
 
-GNU MediaGoblin uses git for all our version control and we have
+.. contents:: Sections
-the repositories hosted on `Gitorious <http://gitorious.org/>`_.
+   :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>`_.
+<http://bugs.foocorp.net/projects/mediagoblin/issues>`_.  That makes
-That makes it a lot easier for everyone to track proposed changes and
+it a lot easier for everyone to track proposed changes and make sure
-make sure your hard work doesn't get dropped on the floor!
+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
-If there isn't an issue for what you're working on, please create
+description of what it is you're trying to fix/implement, the better
-one.  The better the description of what it is you're trying to
+everyone else is able to understand why you're doing what you're
-fix/implement, the better everyone else is able to understand why
+doing.
-you're doing what you're doing.
-
-There are two ways you could send in a patch.
 
 
-How to send in a patch from a publicly available clone
+Use bugfix branches to make changes
-------------------------------------------------------
+-----------------------------------
 
-Add a comment to the issue you're working on with the following bits
+The best way to isolate your changes is to create a branch based off
-of information:
+of the MediaGoblin repository master branch, do the changes related to
+that one issue there, and then let us know how to get it.
 
-* the url for your clone
+It's much easier on us if you isolate your changes to a branch focused
-* the revs you want looked at
+on the issue.  Then we don't have to sift through things.
-* any details, questions, or other things that should be known
+
+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.
 
 
-How to send in a patch if you don't have a publicly available clone
+Properly document your changes
--------------------------------------------------------------------
+------------------------------
 
-Assuming that the remote is our repository on gitorious and the branch
+Include comments in the code.
-to compare against is master, do the following:
 
-1. checkout the branch you did your work in
+Write comprehensive commit messages.  The better your commit message
-2. do::
+is at describing what you did and why, the easier it is for us to
+quickly accept your patch.
 
-      git format-patch -o patches origin/master
+Write comprehensive comments in the issue tracker about what you're
+doing and why.
 
-3. either:
 
-   * tar up and attach the tarball to the issue you're working on, OR
+How to send us your changes
-   * attach the patch files to the issue you're working on one at a
+---------------------------
-     time
+
+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::
+
+       git checkout issue_42
+
+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``)::
+
+       git push origin issue_42
+
+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.
+
+
+What happens next
+-----------------
+
+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.
+
+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.
+
+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.
+
+Slartibartfast is notified of this.  Slartibartfast does a::
+
+   git fetch --all
+
+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

docs/hackinghowto.rst

@@ -4,6 +4,10 @@
  Hacking HOWTO
 ===============
 
+.. contents:: Sections
+   :local:
+
+
 So you want to hack on GNU MediaGoblin?
 =======================================
 

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
 

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

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,

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.

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):

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:

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):

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,
+            'mg_globals': mg_globals,
-            'db': mgoblin_globals.database})
+            'db': mg_globals.database})

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)

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(
+    queued_filename = workbench.localized_file(
-        workbench, mg_globals.queue_store, queued_filepath,
+        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(
+        thumb_filepath = create_pub_filepath(entry, 'thumbnail.jpg')
-            ['media_entries',
-             unicode(entry['_id']),
-             '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(
+        main_filepath = create_pub_filepath(entry, queued_filepath[-1])
-            ['media_entries',
-             unicode(entry['_id']),
-             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()

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"

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'

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(

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

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>')

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 os.path.isdir(workbench.dir)
-        assert workbench.startswith(self.workbench_manager.base_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'))
+        assert os.path.exists(tmpfile_name)
 
-        self.workbench_manager.destroy_workbench(this_workbench)
+        wb_dir = this_workbench.dir
-        assert not os.path.exists(os.path.join(this_workbench, 'temp.txt'))
+        this_workbench.destroy_self()
-        assert not os.path.exists(this_workbench)
+        assert not os.path.exists(tmpfile_name)
-
+        assert not os.path.exists(wb_dir)
-        # make sure we can't kill other stuff though
-        dont_kill_this = tempfile.mkdtemp()
-
-        assert_raises(
-            workbench.WorkbenchOutsideScope,
-            self.workbench_manager.destroy_workbench,
-            dont_kill_this)
 
     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(
+        filename = this_workbench.localized_file(this_storage, filepath)
-            this_workbench, 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(
+        filename = this_workbench.localized_file(this_storage, filepath)
-            this_workbench, 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(
+        filename = this_workbench.localized_file(
-            this_workbench, this_storage, filepath, 'thisfile')
+            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(
+        filename = this_workbench.localized_file(
-            this_workbench, this_storage, filepath, 'thisfile.text', False)
+            this_storage, filepath, 'thisfile.text', False)
         assert filename == os.path.join(
-            this_workbench, 'thisfile.text')
+            this_workbench.dir, 'thisfile.text')

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,
+        mg_globals.translations.gettext,
-        mgoblin_globals.translations.ngettext)
+        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)
 
 

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
+    WARNING: DO NOT create Workbench objects on your own,
-    for during the processing stage.
+    let the WorkbenchManager do that for you!
     """
-
+    def __init__(self, dir):
-    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).
+        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):
+    def __unicode__(self):
-        """
+        return unicode(self.dir)
-        Destroy this workbench!  Deletes the directory and all its contents!
+    def __str__(self):
+        return str(self.dir)
+    def __repr__(self):
+        return repr(self.dir)
 
-        Makes sure the workbench actually belongs to this manager though.
+    def joinpath(self, *args):
-        """
+        return os.path.join(self.dir, *args)
-        # just in case
-        workbench = os.path.abspath(workbench)
 
-        if not workbench.startswith(self.base_workbench_dir):
+    def localized_file(self, storage, filepath,
-            raise WorkbenchOutsideScope(
-                "Can't destroy workbench outside the base workbench dir")
-
-        shutil.rmtree(workbench)
-
-    def localized_file(self, workbench, 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))