From 44e2da2fe60a3b8765d0fef5a9ce0c3e5997dd01 Mon Sep 17 00:00:00 2001 From: Joar Wandborg Date: Sun, 12 Jun 2011 03:24:31 +0200 Subject: [PATCH 01/43] Added Markdown rendering for `media_entry` --- mediagoblin/db/models.py | 3 ++- mediagoblin/edit/views.py | 9 ++++++++- mediagoblin/submit/views.py | 7 +++++++ mediagoblin/templates/mediagoblin/user_pages/media.html | 4 +++- mediagoblin/user_pages/views.py | 4 ++-- mediagoblin/util.py | 3 +-- setup.py | 1 + 7 files changed, 24 insertions(+), 7 deletions(-) diff --git a/mediagoblin/db/models.py b/mediagoblin/db/models.py index 3da97a49..0b092d60 100644 --- a/mediagoblin/db/models.py +++ b/mediagoblin/db/models.py @@ -73,7 +73,8 @@ class MediaEntry(Document): 'title': unicode, 'slug': unicode, 'created': datetime.datetime, - 'description': unicode, + 'description': unicode, # May contain markdown/up + 'description_html': unicode, # May contain plaintext, or HTML 'media_type': unicode, 'media_data': dict, # extra data relevant to this media_type 'plugin_data': dict, # plugins can dump stuff here. diff --git a/mediagoblin/edit/views.py b/mediagoblin/edit/views.py index c5f0f435..2bc53a54 100644 --- a/mediagoblin/edit/views.py +++ b/mediagoblin/edit/views.py @@ -47,7 +47,14 @@ def edit_media(request, media): u'An entry with that slug already exists for this user.') else: media['title'] = request.POST['title'] - media['description'] = request.POST['description'] + media['description'] = request.POST.get('description') + + import markdown + md = markdown.Markdown( + safe_mode = 'escape') + media['description_html'] = md.convert( + media['description']) + media['slug'] = request.POST['slug'] media.save() diff --git a/mediagoblin/submit/views.py b/mediagoblin/submit/views.py index e9b5c37e..21562e6f 100644 --- a/mediagoblin/submit/views.py +++ b/mediagoblin/submit/views.py @@ -48,6 +48,13 @@ def submit_start(request): entry = request.db.MediaEntry() entry['title'] = request.POST['title'] or unicode(splitext(filename)[0]) entry['description'] = request.POST.get('description') + + import markdown + md = markdown.Markdown( + safe_mode = 'escape') + entry['description_html'] = md.convert( + entry['description']) + entry['media_type'] = u'image' # heh entry['uploader'] = request.user['_id'] diff --git a/mediagoblin/templates/mediagoblin/user_pages/media.html b/mediagoblin/templates/mediagoblin/user_pages/media.html index 200f13cd..44bc38b8 100644 --- a/mediagoblin/templates/mediagoblin/user_pages/media.html +++ b/mediagoblin/templates/mediagoblin/user_pages/media.html @@ -25,7 +25,9 @@ -

{{ media.description }}

+ {% autoescape False %} +

{{ media.description_html }}

+ {% endautoescape %}

Uploaded on {{ "%4d-%02d-%02d"|format(media.created.year, media.created.month, media.created.day) }} diff --git a/mediagoblin/user_pages/views.py b/mediagoblin/user_pages/views.py index 323c3e54..4ec0f0aa 100644 --- a/mediagoblin/user_pages/views.py +++ b/mediagoblin/user_pages/views.py @@ -81,10 +81,10 @@ def atom_feed(request): feed = AtomFeed(request.matchdict['user'], feed_url=request.url, url=request.host_url) - + for entry in cursor: feed.add(entry.get('title'), - entry.get('description'), + entry.get('description_html'), content_type='html', author=request.matchdict['user'], updated=entry.get('created'), diff --git a/mediagoblin/util.py b/mediagoblin/util.py index 64e21ca9..1e8fa095 100644 --- a/mediagoblin/util.py +++ b/mediagoblin/util.py @@ -34,7 +34,6 @@ from webob import Response, exc from mediagoblin import globals as mgoblin_globals from mediagoblin.db.util import ObjectId - TESTS_ENABLED = False def _activate_testing(): """ @@ -99,7 +98,7 @@ def get_jinja_env(template_loader, locale): template_env = jinja2.Environment( loader=template_loader, autoescape=True, - extensions=['jinja2.ext.i18n']) + extensions=['jinja2.ext.i18n', 'jinja2.ext.autoescape']) template_env.install_gettext_callables( mgoblin_globals.translations.gettext, diff --git a/setup.py b/setup.py index 46da7276..fb86d600 100644 --- a/setup.py +++ b/setup.py @@ -42,6 +42,7 @@ setup( 'translitcodec', 'argparse', 'webtest', + 'Markdown', ], test_suite='nose.collector', From 49285baf2755e4cead741dd5c615a65736a5dc08 Mon Sep 17 00:00:00 2001 From: Elrond Date: Sun, 12 Jun 2011 17:36:49 +0200 Subject: [PATCH 02/43] Let setup_globals check for known globals To avoid typos in calling setup_globals(), only allow globals, which are already known to the system. Plugins should have their own globals. --- mediagoblin/globals.py | 11 +++++++++++ mediagoblin/tests/test_globals.py | 7 +++++++ 2 files changed, 18 insertions(+) diff --git a/mediagoblin/globals.py b/mediagoblin/globals.py index 80d1f01d..49a513a2 100644 --- a/mediagoblin/globals.py +++ b/mediagoblin/globals.py @@ -19,6 +19,15 @@ database = None public_store = None queue_store = None +# Dump mail to stdout instead of sending it: +email_debug_mode = False + +# Address for sending out mails +email_sender_address = None + +# A WorkBenchManager +workbench_manager = None + # gettext translations = gettext.find( 'mediagoblin', @@ -30,4 +39,6 @@ def setup_globals(**kwargs): from mediagoblin import globals as mg_globals for key, value in kwargs.iteritems(): + if not hasattr(mg_globals, key): + raise AssertionError("Global %s not known" % key) setattr(mg_globals, key, value) diff --git a/mediagoblin/tests/test_globals.py b/mediagoblin/tests/test_globals.py index 6d2e01da..b285cdf5 100644 --- a/mediagoblin/tests/test_globals.py +++ b/mediagoblin/tests/test_globals.py @@ -14,6 +14,8 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . +from nose.tools import assert_raises + from mediagoblin import globals as mg_globals def test_setup_globals(): @@ -27,3 +29,8 @@ def test_setup_globals(): assert mg_globals.database == 'my favorite database!' assert mg_globals.public_store == 'my favorite public_store!' assert mg_globals.queue_store == 'my favorite queue_store!' + + assert_raises( + AssertionError, + mg_globals.setup_globals, + no_such_global_foo = "Dummy") From 44e51d3464e719e596e1480b7af2957742a9085b Mon Sep 17 00:00:00 2001 From: Joar Wandborg Date: Wed, 15 Jun 2011 23:07:54 +0200 Subject: [PATCH 03/43] Made changes according to http://bugs.foocorp.net/issues/363#note-5 --- mediagoblin/edit/views.py | 10 ++++++---- mediagoblin/submit/views.py | 10 ++++++---- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/mediagoblin/edit/views.py b/mediagoblin/edit/views.py index 2bc53a54..6c16a61e 100644 --- a/mediagoblin/edit/views.py +++ b/mediagoblin/edit/views.py @@ -17,11 +17,13 @@ from webob import exc -from mediagoblin.util import render_to_response, redirect +from mediagoblin.util import render_to_response, redirect, clean_html from mediagoblin.edit import forms from mediagoblin.edit.lib import may_edit_media from mediagoblin.decorators import require_active_login, get_user_media_entry +import markdown + @get_user_media_entry @require_active_login @@ -49,11 +51,11 @@ def edit_media(request, media): media['title'] = request.POST['title'] media['description'] = request.POST.get('description') - import markdown md = markdown.Markdown( safe_mode = 'escape') - media['description_html'] = md.convert( - media['description']) + media['description_html'] = clean_html( + md.convert( + media['description'])) media['slug'] = request.POST['slug'] media.save() diff --git a/mediagoblin/submit/views.py b/mediagoblin/submit/views.py index 21562e6f..437a5a51 100644 --- a/mediagoblin/submit/views.py +++ b/mediagoblin/submit/views.py @@ -19,11 +19,13 @@ from cgi import FieldStorage from werkzeug.utils import secure_filename -from mediagoblin.util import render_to_response, redirect +from mediagoblin.util import render_to_response, redirect, clean_html from mediagoblin.decorators import require_active_login from mediagoblin.submit import forms as submit_forms, security from mediagoblin.process_media import process_media_initial +import markdown + @require_active_login def submit_start(request): @@ -49,11 +51,11 @@ def submit_start(request): entry['title'] = request.POST['title'] or unicode(splitext(filename)[0]) entry['description'] = request.POST.get('description') - import markdown md = markdown.Markdown( safe_mode = 'escape') - entry['description_html'] = md.convert( - entry['description']) + entry['description_html'] = clean_html( + md.convert( + entry['description'])) entry['media_type'] = u'image' # heh entry['uploader'] = request.user['_id'] From f970e6e5df915081ca9c2951a4f8f0984aaec0e8 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Wed, 15 Jun 2011 21:14:00 -0500 Subject: [PATCH 04/43] Require ConfigObj --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index 46da7276..37144b5b 100644 --- a/setup.py +++ b/setup.py @@ -42,6 +42,7 @@ setup( 'translitcodec', 'argparse', 'webtest', + 'ConfigObj', ], test_suite='nose.collector', From 0fcfff5a3a12fe7eeb0cabb862242776334879de Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Wed, 15 Jun 2011 21:17:55 -0500 Subject: [PATCH 05/43] Basic config "requirements" file. Not used yet, but this will be used by ConfigObj to transform values, set defaults, etc. --- mediagoblin/config_spec.ini | 69 +++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 mediagoblin/config_spec.ini diff --git a/mediagoblin/config_spec.ini b/mediagoblin/config_spec.ini new file mode 100644 index 00000000..f91d1c1b --- /dev/null +++ b/mediagoblin/config_spec.ini @@ -0,0 +1,69 @@ +[mediagoblin] +# +queuestore_base_dir = string(default="%(here)s/user_dev/media/queue") +publicstore_base_dir = string(default="%(here)s/user_dev/media/public") + +# Where temporary files used in processing and etc are kept +workbench_base_dir = string(default="%(here)s/user_dev/media/workbench") + +# +publicstore_base_url = string(default="/mgoblin_media/") + +# Where mediagoblin-builtin static assets are kept +direct_remote_path = string(default="/mgoblin_static/") + +# set to false to enable sending notices +email_debug_mode = boolean(default=True) +email_sender_address = string(default="notice@mediagoblin.example.org") + +local_templates = string(default="%(here)s/user_dev/templates/") + +# Whether or not celery is set up via an environment variable or +# something else (and thus mediagoblin should not attempt to set it up +# itself) +celery_setup_elsewhere = boolean(default=False) + +[celery] +# known booleans +celery_result_persistent = boolean() +celery_create_missing_queues = boolean() +broker_use_ssl = boolean() +broker_connection_retry = boolean() +celery_always_eager = boolean() +celery_eager_propagates_exceptions = boolean() +celery_ignore_result = boolean() +celery_track_started = boolean() +celery_disable_rate_limits = boolean() +celery_acks_late = boolean() +celery_store_errors_even_if_ignored = boolean() +celery_send_task_error_emails = boolean() +celery_send_events = boolean() +celery_send_task_sent_event = boolean() +celeryd_log_color = boolean() +celery_redirect_stdouts = boolean() + +# known ints +celeryd_concurrency = integer() +celeryd_prefetch_multiplier = integer() +celery_amqp_task_result_expires = integer() +celery_amqp_task_result_connection_max = integer() +redis_port = integer() +redis_db = integer() +broker_port = integer() +broker_connection_timeout = integer() +celery_broker_connection_max_retries = integer() +celery_task_result_expires = integer() +celery_max_cached_results = integer() +celery_default_rate_limit = integer() +celeryd_max_tasks_per_child = integer() +celeryd_task_time_limit = integer() +celeryd_task_soft_time_limit = integer() +mail_port = integer() +celerybeat_max_loop_interval = integer() + +# known floats +celeryd_eta_scheduler_precision = float() + +# known lists +celery_routes = list() +celery_imports = list() \ No newline at end of file From e2c6436e3e7c654a8d4196ac378debf185c74fa4 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Thu, 16 Jun 2011 08:21:51 -0500 Subject: [PATCH 06/43] Configuration file loading via ConfigObj. Uses ConfigObj to open the config file. Also does validation via the config spec, so defaults are provided, strings are interpolated, types are converted. --- mediagoblin/config.py | 71 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 mediagoblin/config.py diff --git a/mediagoblin/config.py b/mediagoblin/config.py new file mode 100644 index 00000000..533dbe19 --- /dev/null +++ b/mediagoblin/config.py @@ -0,0 +1,71 @@ +# GNU MediaGoblin -- federated, autonomous media hosting +# Copyright (C) 2011 Free Software Foundation, Inc +# +# 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 . + +import os +import pkg_resources + +from configobj import ConfigObj +from validate import Validator + + +CONFIG_SPEC_PATH = pkg_resources.resource_filename( + 'mediagoblin', 'config_spec.ini') + + +def _setup_defaults(config, config_path): + """ + Setup DEFAULTS in a config object from an (absolute) config_path. + """ + config.setdefault('DEFAULT', {}) + config['DEFAULT']['here'] = os.path.dirname(config_path) + config['DEFAULT']['__file__'] = config_path + + +def read_mediagoblin_config(config_path, config_spec=CONFIG_SPEC_PATH): + """ + Read a config object from config_path. + + Does automatic value transformation based on the config_spec. + Also provides %(__file__)s and %(here)s values of this file and + its directory respectively similar to paste deploy. + + Args: + - config_path: path to the config file + - config_spec: config file that provides defaults and value types + for validation / conversion. Defaults to mediagoblin/config_spec.ini + + Returns: + A read ConfigObj object. + """ + config_path = os.path.abspath(config_path) + + config_spec = ConfigObj( + CONFIG_SPEC_PATH, + encoding='UTF8', list_values=False, _inspec=True) + + _setup_defaults(config_spec, config_path) + + conf = ConfigObj( + config_path, + configspec=config_spec, + interpolation='ConfigParser') + + _setup_defaults(conf, config_path) + + conf.validate(Validator()) + + return conf + From efc8f1a0d0872b602f4d00e10dea0682c3afb787 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sat, 18 Jun 2011 14:08:58 -0500 Subject: [PATCH 07/43] Let's specifically import string_list()s in the config_spec. --- mediagoblin/config_spec.ini | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mediagoblin/config_spec.ini b/mediagoblin/config_spec.ini index f91d1c1b..2e782101 100644 --- a/mediagoblin/config_spec.ini +++ b/mediagoblin/config_spec.ini @@ -65,5 +65,5 @@ celerybeat_max_loop_interval = integer() celeryd_eta_scheduler_precision = float() # known lists -celery_routes = list() -celery_imports = list() \ No newline at end of file +celery_routes = string_list() +celery_imports = string_list() \ No newline at end of file From 4fd487f72e26dfc8390756e4cfea367fe470558b Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sat, 18 Jun 2011 15:01:32 -0500 Subject: [PATCH 08/43] Validation error reporting functionality. Changed a few things so we can report errors to users properly in the config loading system. - We now return from read_mediagoblin_config both a loaded config and the validation results - We now have a helper function generate_validation_report that can generate a proper validation report saying if there are errors in a way that's useful to users. - Moved conf->config in the read_mediagoblin_config function, which looks nicer IMO. --- mediagoblin/config.py | 63 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 57 insertions(+), 6 deletions(-) diff --git a/mediagoblin/config.py b/mediagoblin/config.py index 533dbe19..2e457e44 100644 --- a/mediagoblin/config.py +++ b/mediagoblin/config.py @@ -17,7 +17,7 @@ import os import pkg_resources -from configobj import ConfigObj +from configobj import ConfigObj, flatten_errors from validate import Validator @@ -42,13 +42,18 @@ def read_mediagoblin_config(config_path, config_spec=CONFIG_SPEC_PATH): Also provides %(__file__)s and %(here)s values of this file and its directory respectively similar to paste deploy. + This function doesn't itself raise any exceptions if validation + fails, you'll have to do something + Args: - config_path: path to the config file - config_spec: config file that provides defaults and value types for validation / conversion. Defaults to mediagoblin/config_spec.ini Returns: - A read ConfigObj object. + A tuple like: (config, validation_result) + ... where 'conf' is the parsed config object and 'validation_result' + is the information from the validation process. """ config_path = os.path.abspath(config_path) @@ -58,14 +63,60 @@ def read_mediagoblin_config(config_path, config_spec=CONFIG_SPEC_PATH): _setup_defaults(config_spec, config_path) - conf = ConfigObj( + config = ConfigObj( config_path, configspec=config_spec, interpolation='ConfigParser') - _setup_defaults(conf, config_path) + _setup_defaults(config, config_path) - conf.validate(Validator()) + # For now the validator just works with the default functions, + # but in the future if we want to add additional validation/configuration + # functions we'd add them to validator.functions here. + # + # See also: + # http://www.voidspace.org.uk/python/validate.html#adding-functions + validator = Validator() + validation_result = config.validate(validator, preserve_errors=True) - return conf + return config, validation_result + +REPORT_HEADER = """\ +There were validation problems loading this config file: +-------------------------------------------------------- +""" + + +def generate_validation_report(config, validation_result): + """ + Generate a report if necessary of problems while validating. + + Returns: + Either a string describing for a user the problems validating + this config or None if there are no problems. + """ + report = [] + + # Organize the report + for entry in flatten_errors(config, validation_result): + # each entry is a tuple + section_list, key, error = entry + + if key is not None: + section_list.append(key) + else: + section_list.append(u'[missing section]') + + section_string = u':'.join(section_list) + + if error == False: + # We don't care about missing values for now. + continue + + report.append(u"%s = %s" % (section_string, error)) + + if report: + return REPORT_HEADER + u"\n".join(report) + else: + return None From 9dba44de84561c72831dad179db30e4dfb3923fd Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sat, 18 Jun 2011 15:18:25 -0500 Subject: [PATCH 09/43] Make REPORT_HEADER a unicode string also. Unicode everywhere, ideally! --- mediagoblin/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mediagoblin/config.py b/mediagoblin/config.py index 2e457e44..4f6d9f2e 100644 --- a/mediagoblin/config.py +++ b/mediagoblin/config.py @@ -82,7 +82,7 @@ def read_mediagoblin_config(config_path, config_spec=CONFIG_SPEC_PATH): return config, validation_result -REPORT_HEADER = """\ +REPORT_HEADER = u"""\ There were validation problems loading this config file: -------------------------------------------------------- """ From e5aa1ec6b4507fc1eb9b11428dad824b4285522d Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sat, 18 Jun 2011 16:51:35 -0500 Subject: [PATCH 10/43] CONFIG_SPEC_PATH should be config_spec here, fixing. --- mediagoblin/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mediagoblin/config.py b/mediagoblin/config.py index 4f6d9f2e..2f93d32c 100644 --- a/mediagoblin/config.py +++ b/mediagoblin/config.py @@ -58,7 +58,7 @@ def read_mediagoblin_config(config_path, config_spec=CONFIG_SPEC_PATH): config_path = os.path.abspath(config_path) config_spec = ConfigObj( - CONFIG_SPEC_PATH, + config_spec, encoding='UTF8', list_values=False, _inspec=True) _setup_defaults(config_spec, config_path) From d5234024b03f9187c3ad52afdb55af5422a1a5d0 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sat, 18 Jun 2011 16:52:40 -0500 Subject: [PATCH 11/43] Tests for mediagoblin.config functions Tests for: - read_mediagoblin_config() - generate_validation_report() --- mediagoblin/tests/fake_carrot_conf_bad.ini | 14 +++ mediagoblin/tests/fake_carrot_conf_empty.ini | 0 mediagoblin/tests/fake_carrot_conf_good.ini | 13 +++ mediagoblin/tests/fake_config_spec.ini | 10 ++ mediagoblin/tests/test_config.py | 97 ++++++++++++++++++++ 5 files changed, 134 insertions(+) create mode 100644 mediagoblin/tests/fake_carrot_conf_bad.ini create mode 100644 mediagoblin/tests/fake_carrot_conf_empty.ini create mode 100644 mediagoblin/tests/fake_carrot_conf_good.ini create mode 100644 mediagoblin/tests/fake_config_spec.ini create mode 100644 mediagoblin/tests/test_config.py diff --git a/mediagoblin/tests/fake_carrot_conf_bad.ini b/mediagoblin/tests/fake_carrot_conf_bad.ini new file mode 100644 index 00000000..0c79b354 --- /dev/null +++ b/mediagoblin/tests/fake_carrot_conf_bad.ini @@ -0,0 +1,14 @@ +[carrotapp] +# Whether or not our carrots are going to be turned into cake. +## These should throw errors +carrotcake = slobber +num_carrots = GROSS + +# A message encouraging our users to eat their carrots. +encouragement_phrase = 586956856856 # shouldn't throw error + +# Something extra! +blah_blah = "blah!" # shouldn't throw error either + +[celery] +eat_celery_with_carrots = pants # yeah that's def an error right there. diff --git a/mediagoblin/tests/fake_carrot_conf_empty.ini b/mediagoblin/tests/fake_carrot_conf_empty.ini new file mode 100644 index 00000000..e69de29b diff --git a/mediagoblin/tests/fake_carrot_conf_good.ini b/mediagoblin/tests/fake_carrot_conf_good.ini new file mode 100644 index 00000000..fed14d07 --- /dev/null +++ b/mediagoblin/tests/fake_carrot_conf_good.ini @@ -0,0 +1,13 @@ +[carrotapp] +# Whether or not our carrots are going to be turned into cake. +carrotcake = true +num_carrots = 88 + +# A message encouraging our users to eat their carrots. +encouragement_phrase = "I'd love it if you eat your carrots!" + +# Something extra! +blah_blah = "blah!" + +[celery] +eat_celery_with_carrots = False diff --git a/mediagoblin/tests/fake_config_spec.ini b/mediagoblin/tests/fake_config_spec.ini new file mode 100644 index 00000000..9421ce36 --- /dev/null +++ b/mediagoblin/tests/fake_config_spec.ini @@ -0,0 +1,10 @@ +[carrotapp] +# Whether or not our carrots are going to be turned into cake. +carrotcake = boolean(default=False) +num_carrots = integer(default=1) + +# A message encouraging our users to eat their carrots. +encouragement_phrase = string() + +[celery] +eat_celery_with_carrots = boolean(default=True) \ No newline at end of file diff --git a/mediagoblin/tests/test_config.py b/mediagoblin/tests/test_config.py new file mode 100644 index 00000000..244f05e5 --- /dev/null +++ b/mediagoblin/tests/test_config.py @@ -0,0 +1,97 @@ +# GNU MediaGoblin -- federated, autonomous media hosting +# Copyright (C) 2011 Free Software Foundation, Inc +# +# 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 . + +import pkg_resources + +from mediagoblin import config + + +CARROT_CONF_GOOD = pkg_resources.resource_filename( + 'mediagoblin.tests', 'fake_carrot_conf_good.ini') +CARROT_CONF_EMPTY = pkg_resources.resource_filename( + 'mediagoblin.tests', 'fake_carrot_conf_empty.ini') +CARROT_CONF_BAD = pkg_resources.resource_filename( + 'mediagoblin.tests', 'fake_carrot_conf_bad.ini') +FAKE_CONFIG_SPEC = pkg_resources.resource_filename( + 'mediagoblin.tests', 'fake_config_spec.ini') + + +def test_read_mediagoblin_config(): + # An empty file + this_conf, validation_results = config.read_mediagoblin_config( + CARROT_CONF_EMPTY, FAKE_CONFIG_SPEC) + + assert this_conf['carrotapp']['carrotcake'] == False + assert this_conf['carrotapp']['num_carrots'] == 1 + assert not this_conf['carrotapp'].has_key('encouragement_phrase') + assert this_conf['celery']['eat_celery_with_carrots'] == True + + # A good file + this_conf, validation_results = config.read_mediagoblin_config( + CARROT_CONF_GOOD, FAKE_CONFIG_SPEC) + + assert this_conf['carrotapp']['carrotcake'] == True + assert this_conf['carrotapp']['num_carrots'] == 88 + assert this_conf['carrotapp']['encouragement_phrase'] == \ + "I'd love it if you eat your carrots!" + assert this_conf['carrotapp']['blah_blah'] == "blah!" + assert this_conf['celery']['eat_celery_with_carrots'] == False + + # A bad file + this_conf, validation_results = config.read_mediagoblin_config( + CARROT_CONF_BAD, FAKE_CONFIG_SPEC) + + # These should still open but will have errors that we'll test for + # in test_generate_validation_report() + assert this_conf['carrotapp']['carrotcake'] == 'slobber' + assert this_conf['carrotapp']['num_carrots'] == 'GROSS' + assert this_conf['carrotapp']['encouragement_phrase'] == \ + "586956856856" + assert this_conf['carrotapp']['blah_blah'] == "blah!" + assert this_conf['celery']['eat_celery_with_carrots'] == "pants" + + +def test_generate_validation_report(): + # Empty + this_conf, validation_results = config.read_mediagoblin_config( + CARROT_CONF_EMPTY, FAKE_CONFIG_SPEC) + report = config.generate_validation_report(this_conf, validation_results) + assert report is None + + # Good + this_conf, validation_results = config.read_mediagoblin_config( + CARROT_CONF_GOOD, FAKE_CONFIG_SPEC) + report = config.generate_validation_report(this_conf, validation_results) + assert report is None + + # Bad + this_conf, validation_results = config.read_mediagoblin_config( + CARROT_CONF_BAD, FAKE_CONFIG_SPEC) + report = config.generate_validation_report(this_conf, validation_results) + + assert report.startswith("""\ +There were validation problems loading this config file: +--------------------------------------------------------""") + + expected_warnings = [ + 'carrotapp:carrotcake = the value "slobber" is of the wrong type.', + 'carrotapp:num_carrots = the value "GROSS" is of the wrong type.', + 'celery:eat_celery_with_carrots = the value "pants" is of the wrong type.'] + warnings = report.splitlines()[2:] + + assert len(warnings) == 3 + for warning in expected_warnings: + assert warning in warnings From 3f5cf663c0c8c2c06c9185af82b5f75423fce7f3 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sat, 18 Jun 2011 17:59:38 -0500 Subject: [PATCH 12/43] Move entire app structure over to using the new config system. This is a huge change! This means several things. - From the python point of view, launching the application is a heck of a lot cleaner. You just need to pass in the config file path to MediaGoblinApp's __init__() and whether or not this funtion should setup celery and you're good. - There are now two separate config files, separating the server setup from the application setup. - server.ini: the paste deploy config file, which configures the applications and server setup but *NOT* the mediagoblin application itself. - mediagoblin.ini: where you configure mediagoblin (and possibly celery) - Launching the application is now different. Instead of: ./bin/paster serve mediagoblin.ini --reload We launch like: ./bin/paster serve server.ini --reload --- mediagoblin/app.py | 143 +++++++++++++++++++--------------- mediagoblin/config_spec.ini | 6 +- mediagoblin.ini => server.ini | 12 +-- 3 files changed, 88 insertions(+), 73 deletions(-) rename mediagoblin.ini => server.ini (60%) diff --git a/mediagoblin/app.py b/mediagoblin/app.py index a1c6b512..da1aba47 100644 --- a/mediagoblin/app.py +++ b/mediagoblin/app.py @@ -18,14 +18,15 @@ import os import urllib import routes -from paste.deploy.converters import asbool from webob import Request, exc from mediagoblin import routing, util, storage, staticdirect +from mediagoblin.config import ( + read_mediagoblin_config, generate_validation_report) from mediagoblin.db.open import setup_connection_and_db_from_config from mediagoblin.mg_globals import setup_globals from mediagoblin.celery_setup import setup_celery_from_config -from mediagoblin.workbench import WorkbenchManager, DEFAULT_WORKBENCH_DIR +from mediagoblin.workbench import WorkbenchManager class Error(Exception): pass @@ -34,42 +35,101 @@ class ImproperlyConfigured(Error): pass class MediaGoblinApp(object): """ - Really basic wsgi app using routes and WebOb. + WSGI application of MediaGoblin + + ... this is the heart of the program! """ - def __init__(self, connection, db, - public_store, queue_store, - staticdirector, - email_sender_address, email_debug_mode, - user_template_path=None, - workbench_path=DEFAULT_WORKBENCH_DIR): + def __init__(self, config_path, setup_celery=True): + """ + Initialize the application based on a configuration file. + + Arguments: + - config_path: path to the configuration file we're opening. + - setup_celery: whether or not to setup celery during init. + (Note: setting 'celery_setup_elsewhere' also disables + setting up celery.) + """ + ############## + # Setup config + ############## + + # Open and setup the config + global_config, validation_result = read_mediagoblin_config(config_path) + app_config = global_config['mediagoblin'] + # report errors if necessary + validation_report = generate_validation_report( + global_config, validation_result) + if validation_report: + raise ImproperlyConfigured(validation_report) + + ########################################## + # Setup other connections / useful objects + ########################################## + + # Set up the database + self.connection, self.db = setup_connection_and_db_from_config( + app_config) + # Get the template environment - self.template_loader = util.get_jinja_loader(user_template_path) + self.template_loader = util.get_jinja_loader( + app_config.get('user_template_path')) # Set up storage systems - self.public_store = public_store - self.queue_store = queue_store - - # Set up database - self.connection = connection - self.db = db + self.public_store = storage.storage_system_from_paste_config( + app_config, 'publicstore') + self.queue_store = storage.storage_system_from_paste_config( + app_config, 'queuestore') # set up routing self.routing = routing.get_mapper() # set up staticdirector tool - self.staticdirector = staticdirector + if app_config.has_key('direct_remote_path'): + self.staticdirector = staticdirect.RemoteStaticDirect( + app_config['direct_remote_path'].strip()) + elif app_config.has_key('direct_remote_paths'): + direct_remote_path_lines = app_config[ + 'direct_remote_paths'].strip().splitlines() + self.staticdirector = staticdirect.MultiRemoteStaticDirect( + dict([line.strip().split(' ', 1) + for line in direct_remote_path_lines])) + else: + raise ImproperlyConfigured( + "One of direct_remote_path or " + "direct_remote_paths must be provided") + # Setup celery, if appropriate + if setup_celery and not app_config.get('celery_setup_elsewhere'): + if os.environ.get('CELERY_ALWAYS_EAGER'): + setup_celery_from_config( + app_config, global_config, + force_celery_always_eager=True) + else: + setup_celery_from_config(app_config, global_config) + + ####################################################### + # Insert appropriate things into mediagoblin.mg_globals + # # certain properties need to be accessed globally eg from # validators, etc, which might not access to the request # object. + ####################################################### + setup_globals( - email_sender_address=email_sender_address, - email_debug_mode=email_debug_mode, - db_connection=connection, + app_config=app_config, + global_config=global_config, + + # TODO: No need to set these two up as globals, we could + # just read them out of mg_globals.app_config + email_sender_address=app_config['email_sender_address'], + email_debug_mode=app_config['email_debug_mode'], + + # Actual, useful to everyone objects + db_connection=self.connection, database=self.db, public_store=self.public_store, queue_store=self.queue_store, - workbench_manager=WorkbenchManager(workbench_path)) + workbench_manager=WorkbenchManager(app_config['workbench_path'])) def __call__(self, environ, start_response): request = Request(environ) @@ -119,45 +179,6 @@ class MediaGoblinApp(object): def paste_app_factory(global_config, **app_config): - # Get the database connection - connection, db = setup_connection_and_db_from_config(app_config) - - # Set up the storage systems. - public_store = storage.storage_system_from_paste_config( - app_config, 'publicstore') - queue_store = storage.storage_system_from_paste_config( - app_config, 'queuestore') - - # Set up the staticdirect system - if app_config.has_key('direct_remote_path'): - staticdirector = staticdirect.RemoteStaticDirect( - app_config['direct_remote_path'].strip()) - elif app_config.has_key('direct_remote_paths'): - direct_remote_path_lines = app_config[ - 'direct_remote_paths'].strip().splitlines() - staticdirector = staticdirect.MultiRemoteStaticDirect( - dict([line.strip().split(' ', 1) - for line in direct_remote_path_lines])) - else: - raise ImproperlyConfigured( - "One of direct_remote_path or direct_remote_paths must be provided") - - if not asbool(app_config.get('celery_setup_elsewhere')): - if asbool(os.environ.get('CELERY_ALWAYS_EAGER')): - setup_celery_from_config( - app_config, global_config, - force_celery_always_eager=True) - else: - setup_celery_from_config(app_config, global_config) - - mgoblin_app = MediaGoblinApp( - connection, db, - public_store=public_store, queue_store=queue_store, - staticdirector=staticdirector, - email_sender_address=app_config.get( - 'email_sender_address', 'notice@mediagoblin.example.org'), - email_debug_mode=asbool(app_config.get('email_debug_mode')), - user_template_path=app_config.get('local_templates'), - workbench_path=app_config.get('workbench_path', DEFAULT_WORKBENCH_DIR)) + mgoblin_app = MediaGoblinApp(app_config['config']) return mgoblin_app diff --git a/mediagoblin/config_spec.ini b/mediagoblin/config_spec.ini index 2e782101..52e3fdfd 100644 --- a/mediagoblin/config_spec.ini +++ b/mediagoblin/config_spec.ini @@ -4,7 +4,7 @@ queuestore_base_dir = string(default="%(here)s/user_dev/media/queue") publicstore_base_dir = string(default="%(here)s/user_dev/media/public") # Where temporary files used in processing and etc are kept -workbench_base_dir = string(default="%(here)s/user_dev/media/workbench") +workbench_path = string(default="%(here)s/user_dev/media/workbench") # publicstore_base_url = string(default="/mgoblin_media/") @@ -16,7 +16,9 @@ direct_remote_path = string(default="/mgoblin_static/") email_debug_mode = boolean(default=True) email_sender_address = string(default="notice@mediagoblin.example.org") -local_templates = string(default="%(here)s/user_dev/templates/") +# By default not set, but you might want something like: +# "%(here)s/user_dev/templates/" +local_templates = string() # Whether or not celery is set up via an environment variable or # something else (and thus mediagoblin should not attempt to set it up diff --git a/mediagoblin.ini b/server.ini similarity index 60% rename from mediagoblin.ini rename to server.ini index b85f22ea..73fbe8e8 100644 --- a/mediagoblin.ini +++ b/server.ini @@ -10,19 +10,11 @@ use = egg:Paste#urlmap [app:mediagoblin] use = egg:mediagoblin#app filter-with = beaker -queuestore_base_dir = %(here)s/user_dev/media/queue -publicstore_base_dir = %(here)s/user_dev/media/public -publicstore_base_url = /mgoblin_media/ -direct_remote_path = /mgoblin_static/ -email_sender_address = "notice@mediagoblin.example.org" -# set to false to enable sending notices -email_debug_mode = true -## Uncomment this to put some user-overriding templates here -#local_templates = %(here)s/user_dev/templates/ +config = %(here)s/mediagoblin.ini [app:publicstore_serve] use = egg:Paste#static -document_root = %(here)s/user_dev/media/public +document_root = %(here)s/user_dev/media/public/ [app:mediagoblin_static] use = egg:Paste#static From 8d808c1fa176715722719ee287a91e5976373caa Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sat, 18 Jun 2011 18:04:14 -0500 Subject: [PATCH 13/43] Erk! Forgot to include the new mediagoblin.ini. --- mediagoblin.ini | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 mediagoblin.ini diff --git a/mediagoblin.ini b/mediagoblin.ini new file mode 100644 index 00000000..596107dc --- /dev/null +++ b/mediagoblin.ini @@ -0,0 +1,15 @@ +[mediagoblin] +queuestore_base_dir = %(here)s/user_dev/media/queue +publicstore_base_dir = %(here)s/user_dev/media/public +publicstore_base_url = /mgoblin_media/ +direct_remote_path = /mgoblin_static/ +email_sender_address = "notice@mediagoblin.example.org" + +# set to false to enable sending notices +email_debug_mode = true + +## Uncomment this to put some user-overriding templates here +#local_templates = %(here)s/user_dev/templates/ + +[celery] +# Put celery stuff here From d01744dd750e7f20d11fabe24b553281a1eff2f1 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sat, 18 Jun 2011 18:30:14 -0500 Subject: [PATCH 14/43] Removing type conversions from setup_celery_from_config. These simply aren't needed any more, not now that the config validator stuff does type conversion for us. Also fixed the docstring to explain force_celery_always_eager. --- mediagoblin/celery_setup/__init__.py | 69 ++-------------------------- 1 file changed, 5 insertions(+), 64 deletions(-) diff --git a/mediagoblin/celery_setup/__init__.py b/mediagoblin/celery_setup/__init__.py index d4f25b07..3a9efeaf 100644 --- a/mediagoblin/celery_setup/__init__.py +++ b/mediagoblin/celery_setup/__init__.py @@ -17,58 +17,6 @@ import os import sys -from paste.deploy.converters import asbool, asint, aslist - - -KNOWN_CONFIG_BOOLS = [ - 'CELERY_RESULT_PERSISTENT', - 'CELERY_CREATE_MISSING_QUEUES', - 'BROKER_USE_SSL', 'BROKER_CONNECTION_RETRY', - 'CELERY_ALWAYS_EAGER', 'CELERY_EAGER_PROPAGATES_EXCEPTIONS', - 'CELERY_IGNORE_RESULT', 'CELERY_TRACK_STARTED', - 'CELERY_DISABLE_RATE_LIMITS', 'CELERY_ACKS_LATE', - 'CELERY_STORE_ERRORS_EVEN_IF_IGNORED', - 'CELERY_SEND_TASK_ERROR_EMAILS', - 'CELERY_SEND_EVENTS', 'CELERY_SEND_TASK_SENT_EVENT', - 'CELERYD_LOG_COLOR', 'CELERY_REDIRECT_STDOUTS', - ] - -KNOWN_CONFIG_INTS = [ - 'CELERYD_CONCURRENCY', - 'CELERYD_PREFETCH_MULTIPLIER', - 'CELERY_AMQP_TASK_RESULT_EXPIRES', - 'CELERY_AMQP_TASK_RESULT_CONNECTION_MAX', - 'REDIS_PORT', 'REDIS_DB', - 'BROKER_PORT', 'BROKER_CONNECTION_TIMEOUT', - 'CELERY_BROKER_CONNECTION_MAX_RETRIES', - 'CELERY_TASK_RESULT_EXPIRES', 'CELERY_MAX_CACHED_RESULTS', - 'CELERY_DEFAULT_RATE_LIMIT', # ?? - 'CELERYD_MAX_TASKS_PER_CHILD', 'CELERYD_TASK_TIME_LIMIT', - 'CELERYD_TASK_SOFT_TIME_LIMIT', - 'MAIL_PORT', 'CELERYBEAT_MAX_LOOP_INTERVAL', - ] - -KNOWN_CONFIG_FLOATS = [ - 'CELERYD_ETA_SCHEDULER_PRECISION', - ] - -KNOWN_CONFIG_LISTS = [ - 'CELERY_ROUTES', 'CELERY_IMPORTS', - ] - - -## Needs special processing: -# ADMINS, ??? -# there are a lot more; we should list here or process specially. - - -def asfloat(obj): - try: - return float(obj) - except (TypeError, ValueError), e: - raise ValueError( - "Bad float value: %r" % obj) - MANDATORY_CELERY_IMPORTS = ['mediagoblin.process_media'] @@ -86,11 +34,12 @@ def setup_celery_from_config(app_config, global_config, - app_config: the application config section - global_config: the entire paste config, all sections - settings_module: the module to populate, as a string - - + - force_celery_always_eager: whether or not to force celery into + always eager mode; good for development and small installs - set_environ: if set, this will CELERY_CONFIG_MODULE to the settings_module """ - if asbool(app_config.get('use_celery_environment_var')) == True: + if app_config.get('celery_setup_elsewhere') == True: # Don't setup celery based on our config file. return @@ -114,9 +63,9 @@ def setup_celery_from_config(app_config, global_config, if celery_settings['BROKER_BACKEND'] == 'mongodb': celery_settings['BROKER_HOST'] = app_config['db_host'] if app_config.has_key('db_port'): - celery_mongo_settings['port'] = asint(app_config['db_port']) + celery_mongo_settings['port'] = app_config['db_port'] if celery_settings['BROKER_BACKEND'] == 'mongodb': - celery_settings['BROKER_PORT'] = asint(app_config['db_port']) + celery_settings['BROKER_PORT'] = app_config['db_port'] celery_mongo_settings['database'] = app_config.get('db_name', 'mediagoblin') celery_settings['CELERY_MONGODB_BACKEND_SETTINGS'] = celery_mongo_settings @@ -124,14 +73,6 @@ def setup_celery_from_config(app_config, global_config, # Add anything else for key, value in celery_conf.iteritems(): key = key.upper() - if key in KNOWN_CONFIG_BOOLS: - value = asbool(value) - elif key in KNOWN_CONFIG_INTS: - value = asint(value) - elif key in KNOWN_CONFIG_FLOATS: - value = asfloat(value) - elif key in KNOWN_CONFIG_LISTS: - value = aslist(value) celery_settings[key] = value # add mandatory celery imports From 52f20ff33715225d45aead9678701915f3e32706 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sat, 18 Jun 2011 18:31:56 -0500 Subject: [PATCH 15/43] Removing option to set celery config section. There's no real reason to support setting the celery config section; so we'll just make it always ['celery']. --- mediagoblin/celery_setup/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mediagoblin/celery_setup/__init__.py b/mediagoblin/celery_setup/__init__.py index 3a9efeaf..e9195ab2 100644 --- a/mediagoblin/celery_setup/__init__.py +++ b/mediagoblin/celery_setup/__init__.py @@ -22,6 +22,7 @@ MANDATORY_CELERY_IMPORTS = ['mediagoblin.process_media'] DEFAULT_SETTINGS_MODULE = 'mediagoblin.celery_setup.dummy_settings_module' + def setup_celery_from_config(app_config, global_config, settings_module=DEFAULT_SETTINGS_MODULE, force_celery_always_eager=False, @@ -43,9 +44,8 @@ def setup_celery_from_config(app_config, global_config, # Don't setup celery based on our config file. return - celery_conf_section = app_config.get('celery_section', 'celery') - if global_config.has_key(celery_conf_section): - celery_conf = global_config[celery_conf_section] + if global_config.has_key('celery'): + celery_conf = global_config['celery'] else: celery_conf = {} From 924c586b72a2313029cc9b8529504060b30e5c55 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sat, 18 Jun 2011 18:59:42 -0500 Subject: [PATCH 16/43] Updating celery_setup.from_celery to use new config loading / app init - The code for this is significantly simpler now. The app sets up everything but celery, and from_celery finishes the job. - There's no more specifying the mediagoblin section in the file, which doesn't make sense anymore and was already confusing. --- mediagoblin/celery_setup/from_celery.py | 66 +++++-------------------- 1 file changed, 12 insertions(+), 54 deletions(-) diff --git a/mediagoblin/celery_setup/from_celery.py b/mediagoblin/celery_setup/from_celery.py index c8ccebc8..45e65e52 100644 --- a/mediagoblin/celery_setup/from_celery.py +++ b/mediagoblin/celery_setup/from_celery.py @@ -16,14 +16,8 @@ import os -from paste.deploy.loadwsgi import NicerConfigParser -from paste.deploy.converters import asbool - -from mediagoblin import storage -from mediagoblin.db.open import setup_connection_and_db_from_config +from mediagoblin import app, mg_globals from mediagoblin.celery_setup import setup_celery_from_config -from mediagoblin.mg_globals import setup_globals -from mediagoblin.workbench import WorkbenchManager, DEFAULT_WORKBENCH_DIR OUR_MODULENAME = __name__ @@ -33,64 +27,28 @@ def setup_self(): """ Transform this module into a celery config module by reading the mediagoblin config file. Set the environment variable - MEDIAGOBLIN_CONFIG to specify where this config file is at and - what section it uses. + MEDIAGOBLIN_CONFIG to specify where this config file is. - By default it defaults to 'mediagoblin.ini:app:mediagoblin'. + By default it defaults to 'mediagoblin.ini'. - The first colon ":" is a delimiter between the filename and the - config section, so in this case the filename is 'mediagoblin.ini' - and the section where mediagoblin is defined is 'app:mediagoblin'. - - Args: - - 'setup_globals_func': this is for testing purposes only. Don't - set this! + Note that if celery_setup_elsewhere is set in your config file, + this simply won't work. """ - mgoblin_conf_file, mgoblin_section = os.environ.get( - 'MEDIAGOBLIN_CONFIG', 'mediagoblin.ini:app:mediagoblin').split(':', 1) + mgoblin_conf_file = os.path.abspath( + os.environ.get('MEDIAGOBLIN_CONFIG', 'mediagoblin.ini')) if not os.path.exists(mgoblin_conf_file): raise IOError( "MEDIAGOBLIN_CONFIG not set or file does not exist") - parser = NicerConfigParser(mgoblin_conf_file) - parser.read(mgoblin_conf_file) - parser._defaults.setdefault( - 'here', os.path.dirname(os.path.abspath(mgoblin_conf_file))) - parser._defaults.setdefault( - '__file__', os.path.abspath(mgoblin_conf_file)) - - mgoblin_section = dict(parser.items(mgoblin_section)) - mgoblin_conf = dict( - [(section_name, dict(parser.items(section_name))) - for section_name in parser.sections()]) + # By setting the environment variable here we should ensure that + # this is the module that gets set up. + os.environ['CELERY_CONFIG_MODULE'] = OUR_MODULENAME + app.MediaGoblinApp(mgoblin_conf_file, setup_celery=False) setup_celery_from_config( - mgoblin_section, mgoblin_conf, + mg_globals.app_config, mg_globals.global_config, settings_module=OUR_MODULENAME, set_environ=False) - connection, db = setup_connection_and_db_from_config(mgoblin_section) - - # Set up the storage systems. - public_store = storage.storage_system_from_paste_config( - mgoblin_section, 'publicstore') - queue_store = storage.storage_system_from_paste_config( - mgoblin_section, 'queuestore') - - workbench_manager = WorkbenchManager( - mgoblin_section.get( - 'workbench_path', DEFAULT_WORKBENCH_DIR)) - - setup_globals( - db_connection=connection, - database=db, - public_store=public_store, - email_debug_mode=asbool(mgoblin_section.get('email_debug_mode')), - email_sender_address=mgoblin_section.get( - 'email_sender_address', - 'notice@mediagoblin.example.org'), - queue_store=queue_store, - workbench_manager=workbench_manager) - if os.environ['CELERY_CONFIG_MODULE'] == OUR_MODULENAME: setup_self() From becb77ee8cf4cc92d87d656117d6bac96544f168 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sat, 18 Jun 2011 19:00:01 -0500 Subject: [PATCH 17/43] It's a good idea for us to pass the application itself into mg_globals :) --- mediagoblin/app.py | 1 + 1 file changed, 1 insertion(+) diff --git a/mediagoblin/app.py b/mediagoblin/app.py index da1aba47..d2a0695e 100644 --- a/mediagoblin/app.py +++ b/mediagoblin/app.py @@ -125,6 +125,7 @@ class MediaGoblinApp(object): email_debug_mode=app_config['email_debug_mode'], # Actual, useful to everyone objects + app=self, db_connection=self.connection, database=self.db, public_store=self.public_store, From 908d045939dcbbba9404fdec8f3c1c584189bd76 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sat, 18 Jun 2011 20:02:59 -0500 Subject: [PATCH 18/43] Only kill the database if it's really set up. --- mediagoblin/tests/__init__.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/mediagoblin/tests/__init__.py b/mediagoblin/tests/__init__.py index e9e2a59a..1f1e23e9 100644 --- a/mediagoblin/tests/__init__.py +++ b/mediagoblin/tests/__init__.py @@ -21,6 +21,7 @@ def setup_package(): pass def teardown_package(): - print "Killing db ..." - mg_globals.db_connection.drop_database(mg_globals.database.name) - print "... done" + if mg_globals.db_connection: + print "Killing db ..." + mg_globals.db_connection.drop_database(mg_globals.database.name) + print "... done" From 623bee73b1632c313e95c828d4a39bea935760d0 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sat, 18 Jun 2011 20:14:33 -0500 Subject: [PATCH 19/43] Nosetests should now be able to run using the new configobj / app init setup Lots of changes: - CELERY_CONFIG_FILE does not need to be set to the from_tests module to run tests anymore, in fact it *should not be set at all* and is specifically forbidden. - moved around the configuration to the new 2-file format - and generally adjusting the code appropriately. --- mediagoblin/celery_setup/from_tests.py | 42 ----------------- mediagoblin/tests/test_mgoblin_app.ini | 12 +++++ .../{mgoblin_test_app.ini => test_server.ini} | 11 +---- mediagoblin/tests/tools.py | 45 ++++++++++++------- 4 files changed, 42 insertions(+), 68 deletions(-) delete mode 100644 mediagoblin/celery_setup/from_tests.py create mode 100644 mediagoblin/tests/test_mgoblin_app.ini rename mediagoblin/tests/{mgoblin_test_app.ini => test_server.ini} (66%) diff --git a/mediagoblin/celery_setup/from_tests.py b/mediagoblin/celery_setup/from_tests.py deleted file mode 100644 index 70814075..00000000 --- a/mediagoblin/celery_setup/from_tests.py +++ /dev/null @@ -1,42 +0,0 @@ -# GNU MediaGoblin -- federated, autonomous media hosting -# Copyright (C) 2011 Free Software Foundation, Inc -# -# 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 . - -import os - -from mediagoblin.tests.tools import TEST_APP_CONFIG -from mediagoblin import util -from mediagoblin.celery_setup import setup_celery_from_config - - -OUR_MODULENAME = __name__ - - -def setup_self(): - """ - Set up celery for testing's sake, which just needs to set up - celery and celery only. - """ - mgoblin_conf = util.read_config_file(TEST_APP_CONFIG) - mgoblin_section = mgoblin_conf['app:mediagoblin'] - - setup_celery_from_config( - mgoblin_section, mgoblin_conf, - settings_module=OUR_MODULENAME, - set_environ=False) - - -if os.environ.get('CELERY_CONFIG_MODULE') == OUR_MODULENAME: - setup_self() diff --git a/mediagoblin/tests/test_mgoblin_app.ini b/mediagoblin/tests/test_mgoblin_app.ini new file mode 100644 index 00000000..94eafb5a --- /dev/null +++ b/mediagoblin/tests/test_mgoblin_app.ini @@ -0,0 +1,12 @@ +[mediagoblin] +queuestore_base_dir = %(here)s/test_user_dev/media/queue +publicstore_base_dir = %(here)s/test_user_dev/media/public +publicstore_base_url = /mgoblin_media/ +direct_remote_path = /mgoblin_static/ +email_sender_address = "notice@mediagoblin.example.org" +email_debug_mode = true +db_name = __mediagoblin_tests__ + +# Celery shouldn't be set up by the paste app factory as it's set up +# elsewhere +celery_setup_elsewhere = true diff --git a/mediagoblin/tests/mgoblin_test_app.ini b/mediagoblin/tests/test_server.ini similarity index 66% rename from mediagoblin/tests/mgoblin_test_app.ini rename to mediagoblin/tests/test_server.ini index abed2615..929a1ccf 100644 --- a/mediagoblin/tests/mgoblin_test_app.ini +++ b/mediagoblin/tests/test_server.ini @@ -10,16 +10,7 @@ use = egg:Paste#urlmap [app:mediagoblin] use = egg:mediagoblin#app filter-with = beaker -queuestore_base_dir = %(here)s/test_user_dev/media/queue -publicstore_base_dir = %(here)s/test_user_dev/media/public -publicstore_base_url = /mgoblin_media/ -direct_remote_path = /mgoblin_static/ -email_sender_address = "notice@mediagoblin.example.org" -email_debug_mode = true -db_name = __mediagoblin_tests__ -# Celery shouldn't be set up by the paste app factory as it's set up -# elsewhere -celery_setup_elsewhere = true +config = %(here)s/test_mgoblin_app.ini [app:publicstore_serve] use = egg:Paste#static diff --git a/mediagoblin/tests/tools.py b/mediagoblin/tests/tools.py index 342b54b7..b1fe56a0 100644 --- a/mediagoblin/tests/tools.py +++ b/mediagoblin/tests/tools.py @@ -18,20 +18,25 @@ import pkg_resources import os, shutil -from paste.deploy import appconfig, loadapp +from paste.deploy import loadapp from webtest import TestApp -from mediagoblin import util +from mediagoblin import util, mg_globals +from mediagoblin.config import read_mediagoblin_config +from mediagoblin.celery_setup import setup_celery_from_config from mediagoblin.decorators import _make_safe from mediagoblin.db.open import setup_connection_and_db_from_config MEDIAGOBLIN_TEST_DB_NAME = '__mediagoblinunittests__' +TEST_SERVER_CONFIG = pkg_resources.resource_filename( + 'mediagoblin.tests', 'test_server.ini') TEST_APP_CONFIG = pkg_resources.resource_filename( - 'mediagoblin.tests', 'mgoblin_test_app.ini') + 'mediagoblin.tests', 'test_mgoblin_app.ini') TEST_USER_DEV = pkg_resources.resource_filename( 'mediagoblin.tests', 'test_user_dev') MGOBLIN_APP = None +CELERY_SETUP = False USER_DEV_DIRECTORIES_TO_SETUP = [ 'media/public', 'media/queue', @@ -42,12 +47,13 @@ class BadCeleryEnviron(Exception): pass def get_test_app(dump_old_app=True): - if not os.environ.get('CELERY_CONFIG_MODULE') == \ - 'mediagoblin.celery_setup.from_tests': + if os.environ.get('CELERY_CONFIG_MODULE'): raise BadCeleryEnviron( - u"Sorry, you *absolutely* must run nosetests with the\n" - u"mediagoblin.celery_setup.from_tests module. Like so:\n" - u"$ CELERY_CONFIG_MODULE=mediagoblin.celery_setup.from_tests ./bin/nosetests") + u"Sorry, you *ABSOLUTELY MUST *NOT* run nosetests with the\n" + u"CELERY_CONFIG_MODULE set to anything.") + + global MGOBLIN_APP + global CELERY_SETUP # Just return the old app if that exists and it's okay to set up # and return @@ -63,16 +69,13 @@ def get_test_app(dump_old_app=True): os.makedirs(full_dir) # Get app config - config = appconfig( - 'config:' + os.path.basename(TEST_APP_CONFIG), - relative_to=os.path.dirname(TEST_APP_CONFIG), - name='mediagoblin') + global_config, validation_result = read_mediagoblin_config(TEST_APP_CONFIG) + app_config = global_config['mediagoblin'] # Wipe database # @@: For now we're dropping collections, but we could also just # collection.remove() ? - connection, db = setup_connection_and_db_from_config( - config.local_conf) + connection, db = setup_connection_and_db_from_config(app_config) collections_to_wipe = [ collection @@ -90,9 +93,19 @@ def get_test_app(dump_old_app=True): # setup app and return test_app = loadapp( - 'config:' + TEST_APP_CONFIG) + 'config:' + TEST_SERVER_CONFIG) - return TestApp(test_app) + app = TestApp(test_app) + MGOBLIN_APP = app + + # setup celery + if not CELERY_SETUP: + setup_celery_from_config( + mg_globals.app_config, mg_globals.global_config, + set_environ=True) + CELERY_SETUP = True + + return app def setup_fresh_app(func): From 72c3ae046a1bb2644c7f0eaaf138f75c4697555f Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sat, 18 Jun 2011 20:14:51 -0500 Subject: [PATCH 20/43] ./bin/gmg commands upgraded to work with the new setup. --- mediagoblin/gmg_commands/migrate.py | 3 --- mediagoblin/gmg_commands/shell.py | 3 --- mediagoblin/gmg_commands/util.py | 20 +------------------- 3 files changed, 1 insertion(+), 25 deletions(-) diff --git a/mediagoblin/gmg_commands/migrate.py b/mediagoblin/gmg_commands/migrate.py index 3ce25701..9e01d51c 100644 --- a/mediagoblin/gmg_commands/migrate.py +++ b/mediagoblin/gmg_commands/migrate.py @@ -23,9 +23,6 @@ def migrate_parser_setup(subparser): subparser.add_argument( '-cf', '--conf_file', default='mediagoblin.ini', help="Config file used to set up environment") - subparser.add_argument( - '-cs', '--app_section', default='app:mediagoblin', - help="Section of the config file where the app config is stored.") def migrate(args): diff --git a/mediagoblin/gmg_commands/shell.py b/mediagoblin/gmg_commands/shell.py index 16caf398..dc1621d1 100644 --- a/mediagoblin/gmg_commands/shell.py +++ b/mediagoblin/gmg_commands/shell.py @@ -25,9 +25,6 @@ def shell_parser_setup(subparser): subparser.add_argument( '-cf', '--conf_file', default='mediagoblin.ini', help="Config file used to set up environment") - subparser.add_argument( - '-cs', '--app_section', default='app:mediagoblin', - help="Section of the config file where the app config is stored.") SHELL_BANNER = """\ diff --git a/mediagoblin/gmg_commands/util.py b/mediagoblin/gmg_commands/util.py index 41a21a1e..8dcac913 100644 --- a/mediagoblin/gmg_commands/util.py +++ b/mediagoblin/gmg_commands/util.py @@ -15,10 +15,6 @@ # along with this program. If not, see . -import os - -from paste.deploy.loadwsgi import NicerConfigParser - from mediagoblin import app @@ -26,20 +22,6 @@ def setup_app(args): """ Setup the application after reading the mediagoblin config files """ - # Duplicated from from_celery.py, remove when we have the generic util - parser = NicerConfigParser(args.conf_file) - parser.read(args.conf_file) - parser._defaults.setdefault( - 'here', os.path.dirname(os.path.abspath(args.conf_file))) - parser._defaults.setdefault( - '__file__', os.path.abspath(args.conf_file)) - - mgoblin_section = dict(parser.items(args.app_section)) - mgoblin_conf = dict( - [(section_name, dict(parser.items(section_name))) - for section_name in parser.sections()]) - - mgoblin_app = app.paste_app_factory( - mgoblin_conf, **mgoblin_section) + mgoblin_app = app.MediaGoblinApp(args.conf_file) return mgoblin_app From 8abeaf2fb60fe5242e6fb569bf2542c88fcd8c30 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sat, 18 Jun 2011 20:15:46 -0500 Subject: [PATCH 21/43] Preparing celery unit tests for new setup. Instead of the previous passing in of dictionaries, we're actually checking some example config files. --- mediagoblin/config_spec.ini | 5 +++ mediagoblin/tests/fake_celery_conf.ini | 9 ++++++ mediagoblin/tests/fake_celery_conf_mgdb.ini | 14 +++++++++ mediagoblin/tests/test_celery_setup.py | 34 ++++++++++----------- 4 files changed, 45 insertions(+), 17 deletions(-) create mode 100644 mediagoblin/tests/fake_celery_conf.ini create mode 100644 mediagoblin/tests/fake_celery_conf_mgdb.ini diff --git a/mediagoblin/config_spec.ini b/mediagoblin/config_spec.ini index 52e3fdfd..aadf5c21 100644 --- a/mediagoblin/config_spec.ini +++ b/mediagoblin/config_spec.ini @@ -1,4 +1,9 @@ [mediagoblin] +# database stuff +db_host = string() +db_name = string() +db_port = integer() + # queuestore_base_dir = string(default="%(here)s/user_dev/media/queue") publicstore_base_dir = string(default="%(here)s/user_dev/media/public") diff --git a/mediagoblin/tests/fake_celery_conf.ini b/mediagoblin/tests/fake_celery_conf.ini new file mode 100644 index 00000000..3e52ac3a --- /dev/null +++ b/mediagoblin/tests/fake_celery_conf.ini @@ -0,0 +1,9 @@ +['mediagoblin'] +# I got nothin' in this file! + +['celery'] +some_variable = floop +mail_port = 2000 +celeryd_eta_scheduler_precision = 1.3 +celery_result_persistent = true +celery_imports = foo.bar.baz, this.is.an.import diff --git a/mediagoblin/tests/fake_celery_conf_mgdb.ini b/mediagoblin/tests/fake_celery_conf_mgdb.ini new file mode 100644 index 00000000..52671c14 --- /dev/null +++ b/mediagoblin/tests/fake_celery_conf_mgdb.ini @@ -0,0 +1,14 @@ +['mediagoblin'] +db_host = mongodb.example.org +db_port = 8080 +db_name = captain_lollerskates + +['something'] +or = other + +['celery'] +some_variable = poolf +mail_port = 2020 +celeryd_eta_scheduler_precision = 3.1 +celery_result_persistent = false +celery_imports = baz.bar.foo, import.is.a.this diff --git a/mediagoblin/tests/test_celery_setup.py b/mediagoblin/tests/test_celery_setup.py index 558eb458..8bf97ae4 100644 --- a/mediagoblin/tests/test_celery_setup.py +++ b/mediagoblin/tests/test_celery_setup.py @@ -17,6 +17,13 @@ import pkg_resources from mediagoblin import celery_setup +from mediagoblin.config import read_mediagoblin_config + + +TEST_CELERY_CONF_NOSPECIALDB = pkg_resources.resource_filename( + 'mediagoblin.tests', 'fake_celery_conf.ini') +TEST_CELERY_CONF_MGSPECIALDB = pkg_resources.resource_filename( + 'mediagoblin.tests', 'fake_celery_conf_mgdb.ini') def test_setup_celery_from_config(): @@ -27,14 +34,12 @@ def test_setup_celery_from_config(): for var in vars_to_wipe: delattr(module, var) + global_config, validation_result = read_mediagoblin_config( + TEST_CELERY_CONF_NOSPECIALDB) + app_config = global_config['mediagoblin'] + celery_setup.setup_celery_from_config( - {}, - {'something': {'or': 'other'}, - 'celery': {'some_variable': 'floop', - 'mail_port': '2000', - 'CELERYD_ETA_SCHEDULER_PRECISION': '1.3', - 'celery_result_persistent': 'true', - 'celery_imports': 'foo.bar.baz this.is.an.import'}}, + app_config, global_config, 'mediagoblin.tests.fake_celery_module', set_environ=False) from mediagoblin.tests import fake_celery_module @@ -53,17 +58,12 @@ def test_setup_celery_from_config(): _wipe_testmodule_clean(fake_celery_module) + global_config, validation_result = read_mediagoblin_config( + TEST_CELERY_CONF_MGSPECIALDB) + app_config = global_config['mediagoblin'] + celery_setup.setup_celery_from_config( - {'db_host': 'mongodb.example.org', - 'db_port': '8080', - 'db_name': 'captain_lollerskates', - 'celery_section': 'vegetable'}, - {'something': {'or': 'other'}, - 'vegetable': {'some_variable': 'poolf', - 'mail_port': '2020', - 'CELERYD_ETA_SCHEDULER_PRECISION': '3.1', - 'celery_result_persistent': 'false', - 'celery_imports': 'baz.bar.foo import.is.a.this'}}, + app_config, global_config, 'mediagoblin.tests.fake_celery_module', set_environ=False) from mediagoblin.tests import fake_celery_module From 0a4cecdc6603b02534c591d065ec772a0f723c8b Mon Sep 17 00:00:00 2001 From: Chris Moylan Date: Sun, 19 Jun 2011 00:22:47 -0500 Subject: [PATCH 22/43] Added tests for all sorts of login form abuse. Added tests for log out --- mediagoblin/tests/test_auth.py | 82 +++++++++++++++++++++++++++++++--- 1 file changed, 76 insertions(+), 6 deletions(-) diff --git a/mediagoblin/tests/test_auth.py b/mediagoblin/tests/test_auth.py index b8389f8d..1b3b5082 100644 --- a/mediagoblin/tests/test_auth.py +++ b/mediagoblin/tests/test_auth.py @@ -242,17 +242,69 @@ def test_authentication_views(test_app): test_user.save() # Get login + # --------- test_app.get('/auth/login/') - # Make sure it rendered with the appropriate template assert util.TEMPLATE_TEST_CONTEXT.has_key( 'mediagoblin/auth/login.html') - # Log in as that user + # Failed login - blank form + # ------------------------- + util.clear_test_template_context() + response = test_app.post('/auth/login/') + context = util.TEMPLATE_TEST_CONTEXT['mediagoblin/auth/login.html'] + form = context['login_form'] + assert form.username.errors == [u'This field is required.'] + assert form.password.errors == [u'This field is required.'] + + # Failed login - blank user + # ------------------------- + util.clear_test_template_context() + response = test_app.post( + '/auth/login/', { + 'password': u'toast'}) + context = util.TEMPLATE_TEST_CONTEXT['mediagoblin/auth/login.html'] + form = context['login_form'] + assert form.username.errors == [u'This field is required.'] + + # Failed login - blank password + # ----------------------------- + util.clear_test_template_context() + response = test_app.post( + '/auth/login/', { + 'username': u'chris'}) + context = util.TEMPLATE_TEST_CONTEXT['mediagoblin/auth/login.html'] + form = context['login_form'] + assert form.password.errors == [u'This field is required.'] + + # Failed login - bad user + # ----------------------- + util.clear_test_template_context() + response = test_app.post( + '/auth/login/', { + 'username': u'steve', + 'password': 'toast'}) + context = util.TEMPLATE_TEST_CONTEXT['mediagoblin/auth/login.html'] + assert context['login_failed'] + + # Failed login - bad password + # --------------------------- + util.clear_test_template_context() + response = test_app.post( + '/auth/login/', { + 'username': u'chris', + 'password': 'jam'}) + context = util.TEMPLATE_TEST_CONTEXT['mediagoblin/auth/login.html'] + assert context['login_failed'] + + # Successful login + # ---------------- util.clear_test_template_context() response = test_app.post( '/auth/login/', { 'username': u'chris', 'password': 'toast'}) + + # User should be redirected response.follow() assert_equal( urlparse.urlsplit(response.location)[2], @@ -260,10 +312,28 @@ def test_authentication_views(test_app): assert util.TEMPLATE_TEST_CONTEXT.has_key( 'mediagoblin/root.html') - # Make sure we're in the session or something - session = util.TEMPLATE_TEST_CONTEXT['mediagoblin/root.html']['request'].session + # Make sure user is in the session + context = util.TEMPLATE_TEST_CONTEXT['mediagoblin/root.html'] + session = context['request'].session assert session['user_id'] == unicode(test_user['_id']) - # Log out as that user - # Make sure we're not in the session + # TODO: test custom redirect when next=True + + # Successful logout + # ----------------- + util.clear_test_template_context() + response = test_app.get('/auth/logout/') + + # Should be redirected to index page + response.follow() + assert_equal( + urlparse.urlsplit(response.location)[2], + '/') + assert util.TEMPLATE_TEST_CONTEXT.has_key( + 'mediagoblin/root.html') + + # Make sure the user is not in the session + context = util.TEMPLATE_TEST_CONTEXT['mediagoblin/root.html'] + session = context['request'].session + assert session.has_key('user_id') == False From 0a7805f9130308e264ac0ea77c93f3554c5a66cc Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 19 Jun 2011 11:10:45 -0500 Subject: [PATCH 23/43] util.read_config_file() no longer needed; removing. --- mediagoblin/util.py | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/mediagoblin/util.py b/mediagoblin/util.py index fc380f41..865add79 100644 --- a/mediagoblin/util.py +++ b/mediagoblin/util.py @@ -352,28 +352,6 @@ def get_locale_from_request(request): return locale_to_lower_upper(target_lang) -def read_config_file(conf_file): - """ - Read a paste deploy style config file and process it. - """ - if not os.path.exists(conf_file): - raise IOError( - "MEDIAGOBLIN_CONFIG not set or file does not exist") - - parser = NicerConfigParser(conf_file) - parser.read(conf_file) - parser._defaults.setdefault( - 'here', os.path.dirname(os.path.abspath(conf_file))) - parser._defaults.setdefault( - '__file__', os.path.abspath(conf_file)) - - mgoblin_conf = dict( - [(section_name, dict(parser.items(section_name))) - for section_name in parser.sections()]) - - return mgoblin_conf - - # A super strict version of the lxml.html cleaner class HTML_CLEANER = Cleaner( scripts=True, From 3c7d11ff2840fdc40be8cf916aef1ae7f532c309 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 19 Jun 2011 11:36:52 -0500 Subject: [PATCH 24/43] renaming storage_system_from_paste_config()->storage_system_from_config() As Elrond points out, this name doesn't make sense anymore since this isn't based on the paste config. Thanks Elrond! --- mediagoblin/app.py | 4 ++-- mediagoblin/storage.py | 4 ++-- mediagoblin/tests/test_storage.py | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/mediagoblin/app.py b/mediagoblin/app.py index d2a0695e..b27b5761 100644 --- a/mediagoblin/app.py +++ b/mediagoblin/app.py @@ -75,9 +75,9 @@ class MediaGoblinApp(object): app_config.get('user_template_path')) # Set up storage systems - self.public_store = storage.storage_system_from_paste_config( + self.public_store = storage.storage_system_from_config( app_config, 'publicstore') - self.queue_store = storage.storage_system_from_paste_config( + self.queue_store = storage.storage_system_from_config( app_config, 'queuestore') # set up routing diff --git a/mediagoblin/storage.py b/mediagoblin/storage.py index ba6ac017..5d6faa4c 100644 --- a/mediagoblin/storage.py +++ b/mediagoblin/storage.py @@ -247,7 +247,7 @@ def clean_listy_filepath(listy_filepath): return cleaned_filepath -def storage_system_from_paste_config(paste_config, storage_prefix): +def storage_system_from_config(paste_config, storage_prefix): """ Utility for setting up a storage system from the paste app config. @@ -266,7 +266,7 @@ def storage_system_from_paste_config(paste_config, storage_prefix): An instantiated storage system. Example: - storage_system_from_paste_config( + storage_system_from_config( {'publicstore_base_url': '/media/', 'publicstore_base_dir': '/var/whatever/media/'}, 'publicstore') diff --git a/mediagoblin/tests/test_storage.py b/mediagoblin/tests/test_storage.py index 55b66e84..1800c29d 100644 --- a/mediagoblin/tests/test_storage.py +++ b/mediagoblin/tests/test_storage.py @@ -58,8 +58,8 @@ class FakeRemoteStorage(storage.BasicFileStorage): local_storage = False -def test_storage_system_from_paste_config(): - this_storage = storage.storage_system_from_paste_config( +def test_storage_system_from_config(): + this_storage = storage.storage_system_from_config( {'somestorage_base_url': 'http://example.org/moodia/', 'somestorage_base_dir': '/tmp/', 'somestorage_garbage_arg': 'garbage_arg', @@ -69,7 +69,7 @@ def test_storage_system_from_paste_config(): assert this_storage.base_dir == '/tmp/' assert this_storage.__class__ is storage.BasicFileStorage - this_storage = storage.storage_system_from_paste_config( + this_storage = storage.storage_system_from_config( {'somestorage_foobie': 'eiboof', 'somestorage_blech': 'hcelb', 'somestorage_garbage_arg': 'garbage_arg', From a32f17c969b68b830d3d497284ccb61bfad79109 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 19 Jun 2011 11:39:35 -0500 Subject: [PATCH 25/43] Removing "paste config" phrasing where we're no longer using paste's config --- mediagoblin/celery_setup/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mediagoblin/celery_setup/__init__.py b/mediagoblin/celery_setup/__init__.py index e9195ab2..b6e35e99 100644 --- a/mediagoblin/celery_setup/__init__.py +++ b/mediagoblin/celery_setup/__init__.py @@ -28,12 +28,12 @@ def setup_celery_from_config(app_config, global_config, force_celery_always_eager=False, set_environ=True): """ - Take a mediagoblin app config and the global config from a paste - factory and try to set up a celery settings module from this. + Take a mediagoblin app config and try to set up a celery settings + module from this. Args: - app_config: the application config section - - global_config: the entire paste config, all sections + - global_config: the entire ConfigObj loaded config, all sections - settings_module: the module to populate, as a string - force_celery_always_eager: whether or not to force celery into always eager mode; good for development and small installs From 07f35dad60ddeae8785157ceae22c239a62f3158 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 19 Jun 2011 11:41:48 -0500 Subject: [PATCH 26/43] Removing a couple of unused imports from util.py --- mediagoblin/util.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/mediagoblin/util.py b/mediagoblin/util.py index 865add79..349bc027 100644 --- a/mediagoblin/util.py +++ b/mediagoblin/util.py @@ -18,7 +18,6 @@ from email.MIMEText import MIMEText import gettext import pkg_resources import smtplib -import os import sys import re import urllib @@ -28,7 +27,6 @@ import copy from babel.localedata import exists import jinja2 import translitcodec -from paste.deploy.loadwsgi import NicerConfigParser from webob import Response, exc from lxml.html.clean import Cleaner From 77edca545b46ddbbd3ff79f4b8ee7b1bee12b43a Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 19 Jun 2011 11:50:05 -0500 Subject: [PATCH 27/43] Adjusting hackinghowto docs to reflect change in server/tests commands --- docs/hackinghowto.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/hackinghowto.rst b/docs/hackinghowto.rst index d8bb9330..e1cae992 100644 --- a/docs/hackinghowto.rst +++ b/docs/hackinghowto.rst @@ -125,7 +125,7 @@ Running the server Run:: - ./bin/paster serve mediagoblin.ini --reload + ./bin/paster serve server.ini --reload Running celeryd @@ -143,7 +143,7 @@ Run:: Too much work? Don't want to run an http server and celeryd at the same time? For development purposes there's a shortcut:: - CELERY_ALWAYS_EAGER=true ./bin/paster serve mediagoblin.ini --reload + CELERY_ALWAYS_EAGER=true ./bin/paster serve server.ini --reload This way the web server will block on processing items until they are done, but you don't need to run celery separately (which is probably @@ -156,7 +156,7 @@ Running the test suite Run:: - CELERY_CONFIG_MODULE=mediagoblin.celery_setup.from_tests ./bin/nosetests + ./bin/nosetests Running a shell From 12c231c8acc2036330143baa17610b6faa901ad6 Mon Sep 17 00:00:00 2001 From: Chris Moylan Date: Sun, 19 Jun 2011 12:28:53 -0500 Subject: [PATCH 28/43] added test coverage for redirecting after login with the next param --- mediagoblin/tests/test_auth.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/mediagoblin/tests/test_auth.py b/mediagoblin/tests/test_auth.py index 1b3b5082..3a13cbb1 100644 --- a/mediagoblin/tests/test_auth.py +++ b/mediagoblin/tests/test_auth.py @@ -317,8 +317,6 @@ def test_authentication_views(test_app): session = context['request'].session assert session['user_id'] == unicode(test_user['_id']) - # TODO: test custom redirect when next=True - # Successful logout # ----------------- util.clear_test_template_context() @@ -337,3 +335,15 @@ def test_authentication_views(test_app): session = context['request'].session assert session.has_key('user_id') == False + # User is redirected to custom URL if POST['next'] is set + # ------------------------------------------------------- + util.clear_test_template_context() + response = test_app.post( + '/auth/login/', { + 'username': u'chris', + 'password': 'toast', + 'next' : '/u/chris/'}) + assert_equal( + urlparse.urlsplit(response.location)[2], + '/u/chris/') + From 78e1c5f1edf7c5e651f40921bd69448a4ceec9eb Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 19 Jun 2011 13:00:49 -0500 Subject: [PATCH 29/43] Actually we need pkg_resources again in test_celery_setup :) This got removed in master at the same time that it got used in the configobj branch... --- mediagoblin/tests/test_celery_setup.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mediagoblin/tests/test_celery_setup.py b/mediagoblin/tests/test_celery_setup.py index 33c50e40..8bf97ae4 100644 --- a/mediagoblin/tests/test_celery_setup.py +++ b/mediagoblin/tests/test_celery_setup.py @@ -14,6 +14,8 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . +import pkg_resources + from mediagoblin import celery_setup from mediagoblin.config import read_mediagoblin_config From 678a504a59217da6e9f0f0c7b0a6750eae8a650c Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 19 Jun 2011 13:38:33 -0500 Subject: [PATCH 30/43] ./lazyserver.sh: launch a server in the most basic way possible Looks for paster either in buildout or in virtualenv / on the path, wherever it seems to be. --- lazyserver.sh | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100755 lazyserver.sh diff --git a/lazyserver.sh b/lazyserver.sh new file mode 100755 index 00000000..bd93b109 --- /dev/null +++ b/lazyserver.sh @@ -0,0 +1,30 @@ +#!/bin/sh + +# GNU MediaGoblin -- federated, autonomous media hosting +# Copyright (C) 2011 Free Software Foundation, Inc +# +# 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 . + +if [ -f ./bin/paster ]; then + echo "Using ./bin/paster"; + export PASTER="./bin/paster"; +elif which paster > /dev/null; then + echo "Using paster from \$PATH"; + export PASTER="paster"; +else + echo "No paster found, exiting! X_X"; + exit 1 +fi + +CELERY_ALWAYS_EAGER=true $PASTER serve server.ini --reload From f1fadb641d043bd260d9c0f5d2f9f3fddfcaae98 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 19 Jun 2011 14:03:08 -0500 Subject: [PATCH 31/43] Updated hacking howto to reflect our new, easier to run lazyserver.sh command. --- docs/hackinghowto.rst | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/docs/hackinghowto.rst b/docs/hackinghowto.rst index e1cae992..fcab5844 100644 --- a/docs/hackinghowto.rst +++ b/docs/hackinghowto.rst @@ -123,7 +123,18 @@ To do this, do:: Running the server ================== -Run:: +If you want to get things running quickly and without hassle, just +run:: + + ./lazyserver.sh + +This will start up a python server where you can begin playing with +mediagoblin. It will also run celery in "always eager" mode so you +don't have to start a separate process for it. + +This is fine in development, but if you want to actually run celery +separately for testing (or deployment purposes), you'll want to run +the server independently:: ./bin/paster serve server.ini --reload @@ -131,26 +142,17 @@ Run:: Running celeryd =============== -You need to do this if you want your media to process and actually -show up. It's probably a good idea in development to have the web -server (above) running in one terminal and celeryd in another window. +If you aren't using ./lazyserver.sh or otherwise aren't running celery +in always eager mode, you'll need to do this if you want your media to +process and actually show up. It's probably a good idea in +development to have the web server (above) running in one terminal and +celeryd in another window. Run:: CELERY_CONFIG_MODULE=mediagoblin.celery_setup.from_celery ./bin/celeryd -Too much work? Don't want to run an http server and celeryd at the -same time? For development purposes there's a shortcut:: - - CELERY_ALWAYS_EAGER=true ./bin/paster serve server.ini --reload - -This way the web server will block on processing items until they are -done, but you don't need to run celery separately (which is probably -good enough for development purposes, but something you almost -certainly shouldn't do in production). - - Running the test suite ====================== From d45e39664a1e6b9f1448302d5c2c3e12a603ead4 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 19 Jun 2011 15:35:19 -0500 Subject: [PATCH 32/43] Re-commenting-out lxml from setup.py Clarifying that this is something to install from the package manager and not via python setuptools itself. --- setup.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 1ca12426..cd0e7f0b 100644 --- a/setup.py +++ b/setup.py @@ -43,7 +43,9 @@ setup( 'argparse', 'webtest', 'ConfigObj', - 'lxml', + ## For now we're expecting that users will install this from + ## their package managers. + # 'lxml', ], test_suite='nose.collector', From 668e8c26ad61b0ff1ee1beeb81d729934dd298f7 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 19 Jun 2011 16:03:13 -0500 Subject: [PATCH 33/43] Reset the globals parameters while testing parameters (This way we can be sure that the database is torn down if necessary but this was the only test that passed last.) --- mediagoblin/tests/test_globals.py | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/mediagoblin/tests/test_globals.py b/mediagoblin/tests/test_globals.py index 59d217f3..1acea328 100644 --- a/mediagoblin/tests/test_globals.py +++ b/mediagoblin/tests/test_globals.py @@ -16,14 +16,23 @@ from mediagoblin import mg_globals -def test_setup_globals(): - mg_globals.setup_globals( - db_connection='my favorite db_connection!', - database='my favorite database!', - public_store='my favorite public_store!', - queue_store='my favorite queue_store!') - - assert mg_globals.db_connection == 'my favorite db_connection!' - assert mg_globals.database == 'my favorite database!' - assert mg_globals.public_store == 'my favorite public_store!' - assert mg_globals.queue_store == 'my favorite queue_store!' +class TestGlobals(object): + def setUp(self): + self.old_connection = mg_globals.db_connection + self.old_database = mg_globals.database + + def tearDown(self): + mg_globals.db_connection = self.old_connection + mg_globals.database = self.old_database + + def test_setup_globals(self): + mg_globals.setup_globals( + db_connection='my favorite db_connection!', + database='my favorite database!', + public_store='my favorite public_store!', + queue_store='my favorite queue_store!') + + assert mg_globals.db_connection == 'my favorite db_connection!' + assert mg_globals.database == 'my favorite database!' + assert mg_globals.public_store == 'my favorite public_store!' + assert mg_globals.queue_store == 'my favorite queue_store!' From 218b8124ca5a6bd747a442c49c75a4bea37c75bf Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 19 Jun 2011 16:23:17 -0500 Subject: [PATCH 34/43] Document our new global objects added during the configobj branch --- mediagoblin/mg_globals.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/mediagoblin/mg_globals.py b/mediagoblin/mg_globals.py index 3d0ed61d..b01be8d3 100644 --- a/mediagoblin/mg_globals.py +++ b/mediagoblin/mg_globals.py @@ -5,6 +5,7 @@ In some places, we need to access the database, public_store, queue_store import gettext import pkg_resources + ############################# # General mediagoblin globals ############################# @@ -34,6 +35,13 @@ translations = gettext.find( pkg_resources.resource_filename( 'mediagoblin', 'translations'), ['en']) +# app and global config objects +app_config = None +global_config = None + +# The actual app object +app = None + def setup_globals(**kwargs): from mediagoblin import mg_globals From 68bf5b19420c5a5e28e33e57b2690442893adc33 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 19 Jun 2011 16:24:31 -0500 Subject: [PATCH 35/43] Documenting the setup_globals function. --- mediagoblin/mg_globals.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/mediagoblin/mg_globals.py b/mediagoblin/mg_globals.py index b01be8d3..739f44ee 100644 --- a/mediagoblin/mg_globals.py +++ b/mediagoblin/mg_globals.py @@ -44,6 +44,12 @@ app = None def setup_globals(**kwargs): + """ + Sets up a bunch of globals in this module. + + Takes the globals to setup as keyword arguments. If globals are + specified that aren't set as variables above, then throw an error. + """ from mediagoblin import mg_globals for key, value in kwargs.iteritems(): From eaca78748cd705b8ac7d987fc7e8a852eb690129 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 19 Jun 2011 16:43:23 -0500 Subject: [PATCH 36/43] Need to run nosetests with CELERY_CONFIG_MODULE set to from_tests again. Basically, if we don't do this celery sets itself up before it should and improperly. :\ --- mediagoblin/celery_setup/from_celery.py | 14 ++++++++----- mediagoblin/celery_setup/from_tests.py | 26 +++++++++++++++++++++++++ mediagoblin/tests/tools.py | 8 +++++--- 3 files changed, 40 insertions(+), 8 deletions(-) create mode 100644 mediagoblin/celery_setup/from_tests.py diff --git a/mediagoblin/celery_setup/from_celery.py b/mediagoblin/celery_setup/from_celery.py index 45e65e52..046aaa50 100644 --- a/mediagoblin/celery_setup/from_celery.py +++ b/mediagoblin/celery_setup/from_celery.py @@ -23,7 +23,7 @@ from mediagoblin.celery_setup import setup_celery_from_config OUR_MODULENAME = __name__ -def setup_self(): +def setup_self(check_environ_for_conf=True, module_name=OUR_MODULENAME): """ Transform this module into a celery config module by reading the mediagoblin config file. Set the environment variable @@ -34,19 +34,23 @@ def setup_self(): Note that if celery_setup_elsewhere is set in your config file, this simply won't work. """ - mgoblin_conf_file = os.path.abspath( - os.environ.get('MEDIAGOBLIN_CONFIG', 'mediagoblin.ini')) + if check_environ_for_conf: + mgoblin_conf_file = os.path.abspath( + os.environ.get('MEDIAGOBLIN_CONFIG', 'mediagoblin.ini')) + else: + mgoblin_conf_file = 'mediagoblin.ini' + if not os.path.exists(mgoblin_conf_file): raise IOError( "MEDIAGOBLIN_CONFIG not set or file does not exist") # By setting the environment variable here we should ensure that # this is the module that gets set up. - os.environ['CELERY_CONFIG_MODULE'] = OUR_MODULENAME + os.environ['CELERY_CONFIG_MODULE'] = module_name app.MediaGoblinApp(mgoblin_conf_file, setup_celery=False) setup_celery_from_config( mg_globals.app_config, mg_globals.global_config, - settings_module=OUR_MODULENAME, + settings_module=module_name, set_environ=False) diff --git a/mediagoblin/celery_setup/from_tests.py b/mediagoblin/celery_setup/from_tests.py new file mode 100644 index 00000000..43032f41 --- /dev/null +++ b/mediagoblin/celery_setup/from_tests.py @@ -0,0 +1,26 @@ +# GNU MediaGoblin -- federated, autonomous media hosting +# Copyright (C) 2011 Free Software Foundation, Inc +# +# 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 . + +import os + +from mediagoblin.celery_setup.from_celery import setup_self + + +OUR_MODULENAME = __name__ + + +if os.environ.get('CELERY_CONFIG_MODULE') == OUR_MODULENAME: + setup_self(check_environ_for_conf=False, module_name=OUR_MODULENAME) diff --git a/mediagoblin/tests/tools.py b/mediagoblin/tests/tools.py index b1fe56a0..515bccd4 100644 --- a/mediagoblin/tests/tools.py +++ b/mediagoblin/tests/tools.py @@ -47,10 +47,12 @@ class BadCeleryEnviron(Exception): pass def get_test_app(dump_old_app=True): - if os.environ.get('CELERY_CONFIG_MODULE'): + if not os.environ.get('CELERY_CONFIG_MODULE') == \ + 'mediagoblin.celery_setup.from_tests': raise BadCeleryEnviron( - u"Sorry, you *ABSOLUTELY MUST *NOT* run nosetests with the\n" - u"CELERY_CONFIG_MODULE set to anything.") + u"Sorry, you *absolutely* must run nosetests with the\n" + u"mediagoblin.celery_setup.from_tests module. Like so:\n" + u"$ CELERY_CONFIG_MODULE=mediagoblin.celery_setup.from_tests ./bin/nosetests") global MGOBLIN_APP global CELERY_SETUP From 188240e312b8c5ff50bef276c97b36e5b3835f1e Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 19 Jun 2011 16:51:02 -0500 Subject: [PATCH 37/43] ./runtests.sh: run unit tests without having to remember the long command. --- runtests.sh | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100755 runtests.sh diff --git a/runtests.sh b/runtests.sh new file mode 100755 index 00000000..9b96b17c --- /dev/null +++ b/runtests.sh @@ -0,0 +1,30 @@ +#!/bin/sh + +# GNU MediaGoblin -- federated, autonomous media hosting +# Copyright (C) 2011 Free Software Foundation, Inc +# +# 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 . + +if [ -f ./bin/nosetests ]; then + echo "Using ./bin/nosetests"; + export NOSETESTS="./bin/nosetests"; +elif which nosetests > /dev/null; then + echo "Using nosetests from \$PATH"; + export NOSETESTS="nosetests"; +else + echo "No nosetests found, exiting! X_X"; + exit 1 +fi + +CELERY_CONFIG_MODULE=mediagoblin.celery_setup.from_tests $NOSETESTS $@ From 8b7663a0bd04da856d36b21d219c396c4111a06b Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 19 Jun 2011 16:52:27 -0500 Subject: [PATCH 38/43] Making ./runtests.sh the official way to run the tests. --- docs/hackinghowto.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/hackinghowto.rst b/docs/hackinghowto.rst index fcab5844..b03efa91 100644 --- a/docs/hackinghowto.rst +++ b/docs/hackinghowto.rst @@ -158,7 +158,7 @@ Running the test suite Run:: - ./bin/nosetests + ./runtests.sh Running a shell From 4bf8e8888c7df6717eb43487136cc9d5c155bc6c Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 19 Jun 2011 20:41:40 -0500 Subject: [PATCH 39/43] Adds util.cleaned_markdown_conversion() and uses it in the submission process This simplifies the markdown processing & html cleaning of descritions and etc by providing a wrapper function that we can use in multiple locations. --- mediagoblin/submit/views.py | 18 ++++++++---------- mediagoblin/util.py | 11 +++++++++++ 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/mediagoblin/submit/views.py b/mediagoblin/submit/views.py index 437a5a51..6139614e 100644 --- a/mediagoblin/submit/views.py +++ b/mediagoblin/submit/views.py @@ -19,13 +19,12 @@ from cgi import FieldStorage from werkzeug.utils import secure_filename -from mediagoblin.util import render_to_response, redirect, clean_html +from mediagoblin.util import ( + render_to_response, redirect, cleaned_markdown_conversion) from mediagoblin.decorators import require_active_login from mediagoblin.submit import forms as submit_forms, security from mediagoblin.process_media import process_media_initial -import markdown - @require_active_login def submit_start(request): @@ -48,14 +47,13 @@ def submit_start(request): # create entry and save in database entry = request.db.MediaEntry() - entry['title'] = request.POST['title'] or unicode(splitext(filename)[0]) + entry['title'] = ( + request.POST['title'] + or unicode(splitext(filename)[0])) + entry['description'] = request.POST.get('description') - - md = markdown.Markdown( - safe_mode = 'escape') - entry['description_html'] = clean_html( - md.convert( - entry['description'])) + entry['description_html'] = cleaned_markdown_conversion( + entry['description']) entry['media_type'] = u'image' # heh entry['uploader'] = request.user['_id'] diff --git a/mediagoblin/util.py b/mediagoblin/util.py index 4d625728..0e43a1f5 100644 --- a/mediagoblin/util.py +++ b/mediagoblin/util.py @@ -29,6 +29,7 @@ import jinja2 import translitcodec from webob import Response, exc from lxml.html.clean import Cleaner +import markdown from mediagoblin import mg_globals from mediagoblin.db.util import ObjectId @@ -375,6 +376,16 @@ def clean_html(html): return HTML_CLEANER.clean_html(html) +MARKDOWN_INSTANCE = markdown.Markdown(safe_mode='escape') + + +def cleaned_markdown_conversion(text): + """ + Take a block of text, run it through MarkDown, and clean its HTML. + """ + return clean_html(MARKDOWN_INSTANCE.convert(text)) + + SETUP_GETTEXTS = {} def setup_gettext(locale): From a01d04a017cbf6009e7122b123aaef7697e299c4 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 19 Jun 2011 20:42:48 -0500 Subject: [PATCH 40/43] Provide a migration to add description_html to MediaEntries that don't have it --- mediagoblin/db/migrations.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/mediagoblin/db/migrations.py b/mediagoblin/db/migrations.py index f1f625b7..b87988fe 100644 --- a/mediagoblin/db/migrations.py +++ b/mediagoblin/db/migrations.py @@ -14,6 +14,8 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . +from mediagoblin.util import cleaned_markdown_conversion + from mongokit import DocumentMigration @@ -33,5 +35,19 @@ class MediaEntryMigration(DocumentMigration): self.collection.update( self.target, self.update, multi=True, safe=True) + def allmigration02_add_description_html(self): + """ + Now that we can have rich descriptions via Markdown, we should + update all existing entries to record the rich description versions. + """ + self.target = {'description_html': {'$exists': False}} + + if not self.status: + for doc in self.collection.find(self.target): + self.update = { + '$set': { + 'description_html': cleaned_markdown_conversion( + doc['description'])}} + MIGRATE_CLASSES = ['MediaEntry'] From fcdd172264fcc8b312fe449301471fe1761e2ed3 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 19 Jun 2011 20:53:38 -0500 Subject: [PATCH 41/43] Updated git documentation to have more useful branch names. I haven't discussed this with Will yet... if he gets unhappy we can roll back this documentation change :) --- docs/git.rst | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/docs/git.rst b/docs/git.rst index c3f7ccce..2836ecd8 100644 --- a/docs/git.rst +++ b/docs/git.rst @@ -108,8 +108,8 @@ 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. +decided that he is bored with fjords and wants to fix issue 42 (the +meaning of life bug) and send us the changes. Slartibartfast has cloned the MediaGoblin repository and his clone lives on gitorious. @@ -125,18 +125,18 @@ Slartibartfast does the following: 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``:: + remote is named ``gmg``) master branch called ``bug42_meaning_of_life``:: - git checkout -b issue_42 gmg/master + git checkout -b bug42_meaning_of_life gmg/master -3. Slartibartfast works hard on his changes in the ``issue_42`` +3. Slartibartfast works hard on his changes in the ``bug42_meaning_of_life`` 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 --set-upstream + git push origin bug42_meaning_of_life --set-upstream 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 @@ -155,19 +155,19 @@ 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:: +1. He checks out the ``bug42_meaning_of_life`` branch:: - git checkout issue_42 + git checkout bug42_meaning_of_life -2. He fixes the bug and checks it into the ``issue_42`` branch. +2. He fixes the bug and checks it into the ``bug42_meaning_of_life`` branch. 3. He pushes his changes to his clone (the remote is named ``origin``):: - git push origin issue_42 + git push origin bug42_meaning_of_life 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. + ``bug42_meaning_of_life`` branch of his publicly available clone. What happens next @@ -180,7 +180,7 @@ 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 +again, fixes the issue, pushes it to his ``bug42_meaning_of_life`` 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 @@ -192,8 +192,8 @@ 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. +Slartibartfast now deletes his ``bug42_meaning_of_life`` branch +because he doesn't need it anymore. How to learn git From 41f965c59d0f0673ac6e9f373e60226b3e768559 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Mon, 20 Jun 2011 08:55:58 -0500 Subject: [PATCH 42/43] Moving server.ini->paste.ini because willkg points out server.ini is ambiguous Updating the file, updating lazyserver.sh, updating docs. --- docs/hackinghowto.rst | 2 +- lazyserver.sh | 2 +- server.ini => paste.ini | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename server.ini => paste.ini (100%) diff --git a/docs/hackinghowto.rst b/docs/hackinghowto.rst index b03efa91..911f2340 100644 --- a/docs/hackinghowto.rst +++ b/docs/hackinghowto.rst @@ -136,7 +136,7 @@ This is fine in development, but if you want to actually run celery separately for testing (or deployment purposes), you'll want to run the server independently:: - ./bin/paster serve server.ini --reload + ./bin/paster serve paste.ini --reload Running celeryd diff --git a/lazyserver.sh b/lazyserver.sh index bd93b109..fdb03ba0 100755 --- a/lazyserver.sh +++ b/lazyserver.sh @@ -27,4 +27,4 @@ else exit 1 fi -CELERY_ALWAYS_EAGER=true $PASTER serve server.ini --reload +CELERY_ALWAYS_EAGER=true $PASTER serve paste.ini --reload diff --git a/server.ini b/paste.ini similarity index 100% rename from server.ini rename to paste.ini From 5c441e75ebc4ad63c3a5362d9bc451abe97984d2 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Mon, 20 Jun 2011 08:57:58 -0500 Subject: [PATCH 43/43] Also moving the test_server.ini to test_paste.ini to avoid ambiguity. --- mediagoblin/tests/{test_server.ini => test_paste.ini} | 0 mediagoblin/tests/tools.py | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename mediagoblin/tests/{test_server.ini => test_paste.ini} (100%) diff --git a/mediagoblin/tests/test_server.ini b/mediagoblin/tests/test_paste.ini similarity index 100% rename from mediagoblin/tests/test_server.ini rename to mediagoblin/tests/test_paste.ini diff --git a/mediagoblin/tests/tools.py b/mediagoblin/tests/tools.py index 515bccd4..9e36fc5c 100644 --- a/mediagoblin/tests/tools.py +++ b/mediagoblin/tests/tools.py @@ -30,7 +30,7 @@ from mediagoblin.db.open import setup_connection_and_db_from_config MEDIAGOBLIN_TEST_DB_NAME = '__mediagoblinunittests__' TEST_SERVER_CONFIG = pkg_resources.resource_filename( - 'mediagoblin.tests', 'test_server.ini') + 'mediagoblin.tests', 'test_paste.ini') TEST_APP_CONFIG = pkg_resources.resource_filename( 'mediagoblin.tests', 'test_mgoblin_app.ini') TEST_USER_DEV = pkg_resources.resource_filename(