Mailing-List: contact allura-commits-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: allura-dev@incubator.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: brondsem@apache.org
To: allura-commits@incubator.apache.org
Date: Wed, 21 Aug 2013 15:27:30 -0000
Message-Id: <d5cc3ef730004b428ee6e31e3f0f5bf3@git.apache.org>
In-Reply-To: <1151aa212a9a42cc81140ccd6acc492f@git.apache.org>
References: <1151aa212a9a42cc81140ccd6acc492f@git.apache.org>
Subject: [48/50] git commit: [#3154] call bulk_export_filename() just once per
 export; better status check; include filename & `c` in config templates

[#3154] call bulk_export_filename() just once per export; better status check; include filename & `c` in config templates


Project: http://git-wip-us.apache.org/repos/asf/incubator-allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-allura/commit/a885568c
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/a885568c
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/a885568c

Branch: refs/heads/db/3154b
Commit: a885568cf6fcfa1e13dca1cdaf4e6cdd91ad3ab0
Parents: 2dae15a
Author: Dave Brondsema <dbrondsema@slashdotmedia.com>
Authored: Mon Aug 19 18:23:31 2013 +0000
Committer: Dave Brondsema <dbrondsema@slashdotmedia.com>
Committed: Wed Aug 21 15:26:27 2013 +0000

----------------------------------------------------------------------
 Allura/allura/ext/admin/admin_main.py         |  3 +-
 Allura/allura/ext/admin/templates/export.html | 12 ++---
 Allura/allura/model/project.py                | 27 +++++++---
 Allura/allura/tasks/export_tasks.py           | 58 ++++++++++++----------
 Allura/allura/tests/functional/test_admin.py  | 30 +++--------
 Allura/allura/tests/test_tasks.py             | 43 ++++------------
 6 files changed, 76 insertions(+), 97 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/a885568c/Allura/allura/ext/admin/admin_main.py
----------------------------------------------------------------------
diff --git a/Allura/allura/ext/admin/admin_main.py b/Allura/allura/ext/admin/admin_main.py
index cbb1e2c..0e33c94 100644
--- a/Allura/allura/ext/admin/admin_main.py
+++ b/Allura/allura/ext/admin/admin_main.py
@@ -652,9 +652,10 @@ class ProjectAdminController(BaseController):
             if c.project.bulk_export_status() == 'busy':
                 flash('Export for project %s already running' % c.project.shortname, 'info')
                 redirect('export')
-            export_tasks.bulk_export.post(c.project.shortname, tools, c.user.username, c.project.neighborhood.name)
+            export_tasks.bulk_export.post(tools)
             flash('Export scheduled', 'ok')
             redirect('export')
+
         return {
             'tools': exportable_tools,
             'status': c.project.bulk_export_status()

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/a885568c/Allura/allura/ext/admin/templates/export.html
----------------------------------------------------------------------
diff --git a/Allura/allura/ext/admin/templates/export.html b/Allura/allura/ext/admin/templates/export.html
index eff9d5a..f0429c2 100644
--- a/Allura/allura/ext/admin/templates/export.html
+++ b/Allura/allura/ext/admin/templates/export.html
@@ -24,12 +24,10 @@
 
 {% block content %}
 
-{% if status == 'ready' %}
-<div class="error">
-  <h2>Careful!</h2>
-  This project has been exported already.
-  Follow instructions in notification email to get the exported data.
-  If you run export again previous exported data will be lost.
+{% if status == 'busy' %}
+<div class="info">
+  <h2>Busy</h2>
+  This project is queued for export.  You can't start another export yet.
 </div>
 {% endif %}
 
@@ -42,7 +40,7 @@
         <label for="tool-{{ loop.index }}">{{ tool.options.mount_label }}</label> <a href="{{ tool.url() }}">{{ tool.url() }}</a>
       </div>
       {% endfor %}
-      <p><div class="grid-19"><input type="submit" value="Export"></div></p>
+      <p><div class="grid-19"><input type="submit" value="Export" {% if status == 'busy' %}disabled{% endif %}></div></p>
     </form>
   {% else %}
     There are no exportable tools in your project.

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/a885568c/Allura/allura/model/project.py
----------------------------------------------------------------------
diff --git a/Allura/allura/model/project.py b/Allura/allura/model/project.py
index 95ad80f..f57dc34 100644
--- a/Allura/allura/model/project.py
+++ b/Allura/allura/model/project.py
@@ -46,6 +46,7 @@ from .neighborhood import Neighborhood
 from .auth import ProjectRole, User
 from .timeline import ActivityNode, ActivityObject
 from .types import ACL, ACE
+from .monq_model import MonQTask
 
 from filesystem import File
 
@@ -855,9 +856,15 @@ class Project(MappedClass, ActivityNode, ActivityObject):
             shortname = self.shortname.split('/')[0]
         return config['bulk_export_path'].format(
                 nbhd=self.neighborhood.url_prefix.strip('/'),
-                project=shortname)
+                project=shortname,
+                c=c,
+        )
 
     def bulk_export_filename(self):
+        '''
+        Return a filename (configurable) for this project export.  The current timestamp
+        may be included, so only run this method once per export.
+        '''
         shortname = self.shortname
         if self.is_nbhd_project:
             shortname = self.url().strip('/')
@@ -869,13 +876,21 @@ class Project(MappedClass, ActivityNode, ActivityObject):
         return config['bulk_export_filename'].format(project=shortname, date=datetime.utcnow())
 
     def bulk_export_status(self):
-        fn = os.path.join(self.bulk_export_path(), self.bulk_export_filename())
-        tmpdir = os.path.join(self.bulk_export_path(), self.shortname)
-        if os.path.isfile(fn):
-            return 'ready'
-        elif os.path.exists(tmpdir):
+        '''
+        Returns 'busy' if an export is queued or in-progress.  Returns None otherwise
+        '''
+        q = {
+            'task_name': 'allura.tasks.export_tasks.bulk_export',
+            'state': {'$in': ['busy', 'ready']},
+            'context.project_id': self._id,
+        }
+        export_task = MonQTask.query.get(**q)
+        if not export_task:
+            return
+        if export_task.state in ('busy', 'ready'):
             return 'busy'
 
+
     def __json__(self):
         return dict(
             shortname=self.shortname,

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/a885568c/Allura/allura/tasks/export_tasks.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tasks/export_tasks.py b/Allura/allura/tasks/export_tasks.py
index b6a84ac..ee07057 100644
--- a/Allura/allura/tasks/export_tasks.py
+++ b/Allura/allura/tasks/export_tasks.py
@@ -15,14 +15,13 @@
 #       specific language governing permissions and limitations
 #       under the License.
 
-import json
 import os
 import logging
 import shutil
 from tempfile import mkstemp
 
 import tg
-from pylons import app_globals as g
+from pylons import app_globals as g, tmpl_context as c
 
 from allura import model as M
 from allura.tasks import mail_tasks
@@ -35,15 +34,19 @@ log = logging.getLogger(__name__)
 
 
 @task
-def bulk_export(project_shortname, tools, username, neighborhood):
-    neighborhood = M.Neighborhood.query.get(name=neighborhood)
-    project = M.Project.query.get(shortname=project_shortname, neighborhood_id=neighborhood._id)
-    if not project:
-        log.error('Project %s not found' % project_shortname)
-        return
-    if project.bulk_export_status() == 'busy':
-        log.info('Another export is running for project %s. Skipping.' % project_shortname)
-        return
+def bulk_export(tools):
+    '''
+    Export the current project data.  Send notification to current user
+
+    :param list tools: list of mount_points to export
+    '''
+    # it's very handy to use c.* within a @task,
+    # but let's be explicit and keep it separate from the main code
+    return _bulk_export(c.project, tools, c.user)
+
+
+def _bulk_export(project, tools, user):
+    export_filename = project.bulk_export_filename()
     not_exported_tools = []
     for tool in tools or []:
         app = project.app_instance(tool)
@@ -57,7 +60,7 @@ def bulk_export(project_shortname, tools, username, neighborhood):
             continue
         log.info('Exporting %s...' % tool)
         try:
-            path = create_export_dir(project)
+            path = create_export_dir(project, export_filename)
             temp_name = mkstemp(dir=path)[1]
             with open(temp_name, 'w') as f:
                 with h.push_context(project._id):
@@ -71,17 +74,17 @@ def bulk_export(project_shortname, tools, username, neighborhood):
     if tools and len(not_exported_tools) < len(tools):
         # If that fails, we need to let it fail
         # there won't be a valid zip file for the user to get.
-        zip_and_cleanup(project)
+        zip_and_cleanup(project, export_filename)
     else:
         log.error('Nothing to export')
+        None
 
-    user = M.User.by_username(username)
     if not user:
-        log.info('Can not find user %s. Skipping notification.' % username)
+        log.info('No user. Skipping notification.')
         return
     tmpl = g.jinja2_env.get_template('allura:templates/mail/bulk_export.html')
     instructions = tg.config.get('bulk_export_download_instructions', '')
-    instructions = instructions.format(project=project.shortname)
+    instructions = instructions.format(project=project.shortname, filename=export_filename, c=c)
     tmpl_context = {
         'instructions': instructions,
         'project': project,
@@ -93,34 +96,35 @@ def bulk_export(project_shortname, tools, username, neighborhood):
         'reply_to': unicode(user.email_address_header()),
         'message_id': h.gen_message_id(),
         'destinations': [unicode(user._id)],
-        'subject': u'Bulk export for project %s completed' % project_shortname,
+        'subject': u'Bulk export for project %s completed' % project.shortname,
         'text': tmpl.render(tmpl_context),
     }
     mail_tasks.sendmail.post(**email)
 
 
-def create_export_dir(project):
+def create_export_dir(project, export_filename):
     """Create temporary directory for export files"""
-    zip_fn = project.bulk_export_filename()
     # Name temporary directory after project shortname,
     # thus zipdir() will use proper prefix inside the archive.
-    tmp_dir_suffix = zip_fn.split('.')[0]
+    tmp_dir_suffix = export_filename.split('.')[0]
     path = os.path.join(project.bulk_export_path(), tmp_dir_suffix)
     if not os.path.exists(path):
         os.makedirs(path)
     return path
 
 
-def zip_and_cleanup(project):
-    """Zip exported data. Copy it to proper location. Remove temporary files."""
+def zip_and_cleanup(project, export_filename):
+    """
+    Zip exported data. Copy it to proper location. Remove temporary files.
+    Returns base filename of zip file
+    """
     path = project.bulk_export_path()
-    zip_fn = project.bulk_export_filename()
-    temp = os.path.join(path, zip_fn.split('.')[0])
-    zip_path_temp = os.path.join(temp, zip_fn)
-    zip_path = os.path.join(path, zip_fn)
+    temp = os.path.join(path, export_filename.split('.')[0])
+    zip_path_temp = os.path.join(temp, export_filename)
+    zip_path = os.path.join(path, export_filename)
 
     zipdir(temp, zip_path_temp)
 
     # cleanup
     shutil.move(zip_path_temp, zip_path)
-    shutil.rmtree(temp)
+    shutil.rmtree(temp)
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/a885568c/Allura/allura/tests/functional/test_admin.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/functional/test_admin.py b/Allura/allura/tests/functional/test_admin.py
index 669f113..9c5aed6 100644
--- a/Allura/allura/tests/functional/test_admin.py
+++ b/Allura/allura/tests/functional/test_admin.py
@@ -836,37 +836,19 @@ class TestExport(TestController):
     def test_selected_one_tool(self, export_tasks):
         r = self.app.post('/admin/export', {'tools': u'wiki'})
         assert_in('ok', self.webflash(r))
-        export_tasks.bulk_export.post.assert_called_once_with(
-            'test', [u'wiki'], u'test-admin', u'Projects')
+        export_tasks.bulk_export.post.assert_called_once_with([u'wiki'])
 
     @mock.patch('allura.ext.admin.admin_main.export_tasks')
     def test_selected_multiple_tools(self, export_tasks):
         r = self.app.post('/admin/export', {'tools': [u'wiki', u'wiki2']})
         assert_in('ok', self.webflash(r))
-        export_tasks.bulk_export.post.assert_called_once_with(
-            'test', [u'wiki', u'wiki2'], u'test-admin', u'Projects')
+        export_tasks.bulk_export.post.assert_called_once_with([u'wiki', u'wiki2'])
 
-    @mock.patch('allura.ext.admin.admin_main.export_tasks')
-    def test_export_in_progress(self, export_tasks):
-        p = M.Project.query.get(shortname='test')
-        tmpdir = os.path.join(p.bulk_export_path(), p.shortname)
-        shutil.rmtree(p.bulk_export_path(), ignore_errors=True)
-        os.makedirs(tmpdir)
-        r = self.app.post('/admin/export', {'tools': [u'wiki', u'wiki2']})
-        assert_in('info', self.webflash(r))
-        assert_equals(export_tasks.bulk_export.post.call_count, 0)
-        shutil.rmtree(p.bulk_export_path(), ignore_errors=True)
-
-    def test_export_done(self):
-        p = M.Project.query.get(shortname='test')
-        shutil.rmtree(p.bulk_export_path(), ignore_errors=True)
-        os.makedirs(p.bulk_export_path())
-        fn = os.path.join(p.bulk_export_path(), p.bulk_export_filename())
-        with open(fn, 'w') as f:
-            f.write('Pretending I am zip archive')
+    def test_export_in_progress(self):
+        from allura.tasks import export_tasks
+        export_tasks.bulk_export.post(['wiki'])
         r = self.app.get('/admin/export')
-        assert_in('<h2>Careful!</h2>', r)
-        shutil.rmtree(p.bulk_export_path(), ignore_errors=True)
+        assert_in('<h2>Busy</h2>', r.body)
 
     @td.with_user_project('test-user')
     def test_bulk_export_path_for_user_project(self):

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/a885568c/Allura/allura/tests/test_tasks.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/test_tasks.py b/Allura/allura/tests/test_tasks.py
index a1e09c3..9f307ee 100644
--- a/Allura/allura/tests/test_tasks.py
+++ b/Allura/allura/tests/test_tasks.py
@@ -341,13 +341,8 @@ class TestExportTasks(unittest.TestCase):
         shutil.rmtree(project.bulk_export_path(), ignore_errors=True)
 
     @mock.patch('allura.tasks.export_tasks.log')
-    def test_bulk_export_invalid_project(self, log):
-        export_tasks.bulk_export('bad', [u'wiki'], 'test-admin', 'Projects')
-        log.error.assert_called_once_with('Project bad not found')
-
-    @mock.patch('allura.tasks.export_tasks.log')
     def test_bulk_export_invalid_tool(self, log):
-        export_tasks.bulk_export('test', [u'bugs', u'blog'], 'test-admin', 'Projects')
+        export_tasks.bulk_export([u'bugs', u'blog'])
         assert_equal(log.info.call_count, 2)
         assert_equal(log.info.call_args_list, [
             mock.call('Can not load app for bugs mount point. Skipping.'),
@@ -360,7 +355,7 @@ class TestExportTasks(unittest.TestCase):
     @td.with_tool('test', 'Blog', 'blog')
     def test_bulk_export_not_exportable_tool(self, mail_tasks, app, log):
         app.return_value.exportable = False
-        export_tasks.bulk_export('test', [u'bugs', u'blog'], 'test-admin', 'Projects')
+        export_tasks.bulk_export([u'bugs', u'blog'])
         assert_equal(log.info.call_count, 2)
         assert_equal(log.info.call_args_list, [
             mock.call('Tool bugs is not exportable. Skipping.'),
@@ -374,7 +369,7 @@ class TestExportTasks(unittest.TestCase):
     @td.with_wiki
     def test_bulk_export(self, log, wiki_bulk_export, zipdir, shutil, project_json):
         M.MonQTask.query.remove()
-        export_tasks.bulk_export('test', [u'wiki'], 'test-admin', 'Projects')
+        export_tasks.bulk_export([u'wiki'])
         assert_equal(log.info.call_count, 1)
         assert_equal(log.info.call_args_list, [
             mock.call('Exporting wiki...')])
@@ -396,21 +391,11 @@ class TestExportTasks(unittest.TestCase):
         assert_in('The following tools were exported:\n- wiki', text)
         assert_in('Sample instructions for test', text)
 
-    @mock.patch('forgewiki.wiki_main.ForgeWikiApp.bulk_export')
-    @mock.patch('allura.tasks.export_tasks.log')
-    @td.with_wiki
-    def test_bulk_export_quits_if_another_export_is_running(self, log, wiki_bulk_export):
-        project = M.Project.query.get(shortname='test')
-        export_tasks.create_export_dir(project)
-        assert_equal(project.bulk_export_status(), 'busy')
-        export_tasks.bulk_export('test', [u'wiki'], 'test-admin', 'Projects')
-        log.info.assert_called_once_with('Another export is running for project test. Skipping.')
-        assert_equal(wiki_bulk_export.call_count, 0)
-
     def test_create_export_dir(self):
         project = M.Project.query.get(shortname='test')
         export_path = project.bulk_export_path()
-        path = export_tasks.create_export_dir(project)
+        export_filename = project.bulk_export_filename()
+        path = export_tasks.create_export_dir(project, export_filename)
         assert_equal(path, '/tmp/bulk_export/p/test/test')
         assert os.path.exists(os.path.join(export_path, project.shortname))
 
@@ -418,22 +403,16 @@ class TestExportTasks(unittest.TestCase):
     def test_zip_and_cleanup(self):
         project = M.Project.query.get(shortname='test')
         export_path = project.bulk_export_path()
-        path = export_tasks.create_export_dir(project)
-        export_tasks.zip_and_cleanup(project)
+        export_filename = project.bulk_export_filename()
+        path = export_tasks.create_export_dir(project, export_filename)
+        export_tasks.zip_and_cleanup(project, export_filename)
         assert not os.path.exists(path)
         assert os.path.exists(os.path.join(export_path, 'test.zip'))
 
     def test_bulk_export_status(self):
-        project = M.Project.query.get(shortname='test')
-        assert_equal(project.bulk_export_status(), None)
-
-        export_tasks.create_export_dir(project)
-        assert_equal(project.bulk_export_status(), 'busy')
-
-        with open(os.path.join(project.bulk_export_path(),
-                               project.bulk_export_filename()), 'w') as f:
-            f.write('just test')
-        assert_equal(project.bulk_export_status(), 'ready')
+        assert_equal(c.project.bulk_export_status(), None)
+        export_tasks.bulk_export.post(['wiki'])
+        assert_equal(c.project.bulk_export_status(), 'busy')
 
 
 Mapper.compile_all()