Merge pull request #550 from Cal-CS-61A-Staff/bug/alvin/deferredtask

Deferred Tasks

server/app/api.py

@@ -18,7 +18,7 @@
 from app import models, app, analytics
 from app.codereview import compare
 from app.needs import Need
-from app.utils import paginate, filter_query, create_zip, add_to_zip, start_zip, finish_zip, scores_to_gcs
+from app.utils import paginate, filter_query, create_zip, add_to_zip, start_zip, finish_zip, scores_to_gcs, subms_to_gcs, make_zip_filename
 from app.utils import add_to_grading_queues, parse_date, assign_submission
 from app.utils import merge_user
 
@@ -1398,38 +1398,35 @@ def check_permissions(self, user, data):
 
         if user.key not in course.staff and not user.is_admin:
             raise Need('get').exception()
+
+    @staticmethod
+    def results(data):
+        """ Returns results of query, limiting results accordingly """
+        results = SearchAPI.querify(data['query']).fetch()
+        if data.get('all', 'true').lower() != 'true':
+            start, end = SearchAPI.limits(data['page'], data['num_per_page'])
+            results = results[start:end]
+        return results
 
     def index(self, user, data):
+        """ Performs search query, with some extra information """
         self.check_permissions(user, data)
 
-        query = SearchAPI.querify(data['query'])
-        start, end = SearchAPI.limits(data['page'], data['num_per_page'])
-        results = query.fetch()[start:end]
+        results = self.results(data)
         return dict(data={
             'results': results,
             'more': len(results) >= data['num_per_page'],
             'query': data['query']
         })
 
     def download(self, user, data):
+        """ Sets up zip write to GCS """
         self.check_permissions(user, data)
 
-        results = SearchAPI.querify(data['query']).fetch()
-        if data.get('all', 'true').lower() != 'true':
-            start, end = SearchAPI.limits(data['page'], data['num_per_page'])
-            results = results[start:end]
-        zipfile_str, zipfile = start_zip()
-        subm = SubmissionAPI()
-        for result in results:
-            try:
-                if isinstance(result, models.Submission):
-                    result = result.backup.get()
-                name, file_contents = subm.data_for_zip(result)
-                zipfile = add_to_zip(zipfile, file_contents, name)
-            except BadValueError as e:
-                if str(e) != 'Submission has no contents to download':
-                    raise e
-        return subm.make_zip_response('query', finish_zip(zipfile_str, zipfile))
+        now = datetime.datetime.now()
+        deferred.defer(subms_to_gcs, SearchAPI, SubmissionAPI, models.Submission, user, data, now)
+
+        return [make_zip_filename(user, now)]
 
 
     @staticmethod

server/app/utils.py

@@ -25,6 +25,7 @@
 
 from app import app
 from app.constants import GRADES_BUCKET
+from app.exceptions import BadValueError
 
 # TODO Looks like this can be removed just by relocating parse_date
 # To deal with circular imports
@@ -154,13 +155,12 @@ def data_for_scores(assignment, user):
 
     return content
 
-def create_gcs_file(assignment, contents, info_type):
+def create_gcs_file(gcs_filename, contents, content_type):
     """ 
     Creates a GCS csv file with contents CONTENTS. 
     """
     try:
-        gcs_filename = '/{}/{}'.format(GRADES_BUCKET, make_filename(assignment, info_type))
-        gcs_file = gcs.open(gcs_filename, 'w', content_type='text/csv', options={'x-goog-acl':'project-private'})
+        gcs_file = gcs.open(gcs_filename, 'w', content_type=content_type, options={'x-goog-acl': 'project-private'})
         gcs_file.write(contents)
         gcs_file.close()
     except Exception as e:
@@ -171,7 +171,8 @@ def create_gcs_file(assignment, contents, info_type):
             logging.info("Could not delete file " + gcs_filename)
     logging.info("Created file " + gcs_filename)
 
-def make_filename(assignment, infotype):
+
+def make_csv_filename(assignment, infotype):
     """ Returns filename of format INFOTYPE_COURSE_ASSIGNMENT.csv """
     course_name = assignment.course.get().offering
     assign_name = assignment.display_name
@@ -500,9 +501,52 @@ def check_user(user_key):
 
     deferred.defer(deferred_check_user, user_key)
 
+
 def scores_to_gcs(assignment, user):
     """ Writes all final submission scores 
     for the given assignment to GCS csv file. """
     content = data_for_scores(assignment, user)
     csv_contents = create_csv_content(content)
-    create_gcs_file(assignment, csv_contents, 'scores')
+    csv_filename = '/{}/{}'.format(GRADES_BUCKET, make_csv_filename(assignment, 'scores'))
+    create_gcs_file(csv_filename, csv_contents, 'text/csv')
+
+
+def add_subm_to_zip(subm, Submission, zipfile, result):
+    """ Adds submission contents to a zipfile in-place, returns zipfile """
+    try:
+        if isinstance(result, Submission):
+            result = result.backup.get()
+        name, file_contents = subm.data_for_zip(result)
+        return add_to_zip(zipfile, file_contents, name)
+    except BadValueError as e:
+        if str(e) != 'Submission has no contents to download':
+            raise e
+
+
+def make_zip_filename(user, now):
+    """ Makes zip filename: query_USER EMAIL_DATETIME.zip """
+    outlawed = [' ', '.', ':', '@']
+    filename = '/{}/{}'.format(
+        GRADES_BUCKET, 
+        '%s_%s_%s' % (
+            'query', 
+            user.email[0], 
+            str(now)))
+    for outlaw in outlawed:
+        filename = filename.replace(outlaw, '-')
+    return filename+'.zip'
+
+
+def subms_to_gcs(SearchAPI, SubmissionAPI, Submission, user, data, datetime):
+    """
+    Writes all submissions for a given search query
+    to a GCS zip file.
+    """
+    results = SearchAPI.results(data)
+    zipfile_str, zipfile = start_zip()
+    subm = SubmissionAPI()
+    for result in results:
+        zipfile = add_subm_to_zip(subm, Submission, zipfile, result)
+    zip_contents = finish_zip(zipfile_str, zipfile)
+    zip_filename = make_zip_filename(user, datetime)
+    create_gcs_file(zip_filename, zip_contents, 'application/zip')

server/static/js/admin/controllers.js

@@ -533,6 +533,29 @@ app.controller("SubmissionListCtrl", ['$scope', '$stateParams', '$window', 'Sear
     $scope.search = function() {
       $scope.getPage($scope.currentPage)
     }
+
+    $scope.download_zip = function(query, all, courseId) {
+      filename = 'query_(your email)_(current time).zip'
+      Search.download_zip({
+        query: query,
+        all: all,
+        courseId: courseId
+      }, function(response) {
+        $window.swal({
+          title: 'Success',
+          text:'Saving submissions to ' + response[0] +
+          '\n Zip of submissions will be ready in Google Cloud Storage ok_grades_bucket in a few minutes',
+          type: 'success',
+          confirmButtonText: 'View zip',
+          cancelButtonText: 'Not now',
+          showCancelButton: true},
+          function() {
+            $window.location = 'https://console.developers.google.com/storage/browser'+response[0];
+          });
+      }, function(err) {
+        report_error($window, err);
+      });
+    }
   }]);
 
 

server/static/js/common/services.js

@@ -332,6 +332,11 @@ app.factory('Search', ['$resource',
           query: {
             url: '/api/v1/search',
             transformResponse: defaultTransformer
+          },
+          download_zip: {
+            isArray:true,
+            url: '/api/v1/search/download',
+            transformResponse: defaultTransformer
           }
         }
       )

server/static/partials/admin/submission.list.html

@@ -32,9 +32,9 @@
       <h3 class="box-title" ng-if="submissions.length > 0">All Submissions</h3>
       <h4 class="box-title" ng-if="submissions.length == 0" >No Submissions</h3>
       <div class="box-tools" style="float:right;">
-          <a href="/api/v1/search/download?courseId={{course.id}}&all=True&query={{ search_query }}"><button class="btn btn-primary btn-sm" ng-if="more || currentPage != 1" style="margin-right:5px"> <i class="fa fa-download"></i>  Download All</button></a>
-          <a href="/api/v1/search/download?courseId={{course.id}}&all=False&query={{ search_query }}"><button class="btn btn-primary btn-sm" ng-if="more || currentPage != 1"> <i class="fa fa-download"></i>  Download Page</button></a>
-          <a href="/api/v1/search/download?courseId={{course.id}}&all=True&query={{ search_query }}"><button class="btn btn-primary btn-sm" ng-if="!more && currentPage == 1"> <i class="fa fa-download"></i>  Download</button></a>
+          <button ng-click="download_zip(search_query, 'True', course.id)" class="btn btn-primary btn-sm" ng-if="more || currentPage != 1" style="margin-right:5px"> <i class="fa fa-download"></i>  Download All</button>
+          <button ng-click="download_zip(search_query, 'False', course.id)" class="btn btn-primary btn-sm" ng-if="more || currentPage != 1"> <i class="fa fa-download"></i>  Download Page</button>
+          <button ng-click="download_zip(search_query, 'True', course.id)" class="btn btn-primary btn-sm" ng-if="!more && currentPage == 1"> <i class="fa fa-download"></i>  Download</button>
       </div>
     </div>
     <!-- /.box-header -->

server/tests/integration/test_utils.py

@@ -0,0 +1,130 @@
+#!/usr/bin/env python
+# encoding: utf-8
+#pylint: disable=no-member, no-init, too-many-public-methods
+#pylint: disable=attribute-defined-outside-init
+# This disable is because the tests need to be name such that
+# you can understand what the test is doing from the method name.
+#pylint: disable=missing-docstring
+"""
+tests.py
+
+"""
+
+from test_base import APIBaseTestCase
+from test_base import utils, api
+from integration.test_api_base import APITest
+from test_base import make_fake_assignment, make_fake_course, make_fake_backup, make_fake_submission, make_fake_finalsubmission #pylint: disable=relative-import
+import datetime
+
+try:
+	from cStringIO import StringIO
+except:
+	from StringIO import StringIO
+
+class UtilsTestCase(APIBaseTestCase):
+
+	def setUp(self):
+		super(UtilsTestCase, self).setUp()
+		self.user = self.accounts['dummy_admin']
+		self.user1 = self.accounts['dummy_student']
+		self.user2 = self.accounts['dummy_student2']
+		self.user3 = self.accounts['dummy_student3']
+		self.assignment_name = 'Hog Project'
+		self._course = make_fake_course(self.user)
+		self._course.put()
+		self._assign = make_fake_assignment(self._course, self.user)
+		self._assign.name = self._assign.display_name = self.assignment_name
+		self._assign.put()
+		self._backup = make_fake_backup(self._assign, self.user2)
+		self._submission = make_fake_submission(self._backup)
+
+	def get_accounts(self):
+		return APITest().get_accounts()
+
+	##########################
+	# TEST ZIP FUNCTIONALITY #
+	##########################
+
+	def test_zip_filename_purified(self):
+		""" Test that filename doesn't contain weird chars """
+		user = lambda: '_'
+		user.email = ['test@example.com']
+		fn = utils.make_zip_filename(user, datetime.datetime.now())
+
+		assert fn.split('.')[1] == 'zip'
+		assert '@' not in fn
+		assert ' ' not in fn
+
+	def test_add_subm_to_zip(self):
+		""" Test that submission contents added to zip """
+		results = api.SearchAPI.results({
+			'query': ''
+		})
+		for result in results:
+			subm = api.SubmissionAPI()
+			zipfile_str, zipfile = utils.start_zip()
+			zipfile = utils.add_subm_to_zip(subm, result.__class__, zipfile, result)
+			assert zipfile is None or len(zipfile.infolist()) > 0
+
+	def test_start_zip_basic(self):
+		""" Test that a zip is started properly """
+		zipfile_str, zipfile = utils.start_zip()
+		assert zipfile_str is not None
+		assert zipfile is not None
+		return zipfile_str, zipfile
+
+	def test_start_zip_filecontents(self):
+		""" Test that zip is initialized with file contents dict properly """
+		file_contents = dict(a='file a contents', b='file b contents')
+		zipfile_str, zipfile = utils.start_zip(file_contents)
+		zipinfo = zipfile.infolist()
+		zipnames = [z.filename for z in zipinfo]
+		assert 'a' in zipnames
+		assert 'b' in zipnames
+		return zipfile_str, zipfile
+
+	def test_start_zip_dir(self):
+		""" Test that files are saved under specified directory """
+		file_contents, dir = dict(a='file a contents', b='file b contents'), 'dir'
+		zipfile_str, zipfile = utils.start_zip(file_contents, dir)
+		zipinfo = zipfile.infolist()
+		zipnames = [z.filename for z in zipinfo]
+		assert 'dir/a' in zipnames
+		assert 'dir/b' in zipnames
+		return zipfile_str, zipfile
+
+	def test_add_to_zip_basic(self):
+		""" Test that a zip is added to properly """
+		zipfile_str, zipfile = self.test_start_zip_basic()
+		zipfile = utils.add_to_zip(zipfile, dict(filename='file contents'))
+		assert len(zipfile.infolist()) == 1
+		return zipfile_str, zipfile
+
+	def test_add_to_zip_filecontents(self):
+		""" Test that zip is initialized with file contents dict properly """
+		zipfile_str, zipfile = self.test_start_zip_filecontents()
+		file_contents = dict(c='file c contents', d='file d contents')
+		zipfile = utils.add_to_zip(zipfile, file_contents)
+		zipinfo = zipfile.infolist()
+		zipnames = [z.filename for z in zipinfo]
+		assert 'c' in zipnames
+		assert 'c' in zipnames
+
+	def test_add_to_zip_dir(self):
+		""" Test that files are saved under specified directory """
+		zipfile_str, zipfile = self.test_start_zip_dir()
+		file_contents, dir = dict(c='file c contents', d='file d contents'), 'dir'
+		zipfile = utils.add_to_zip(zipfile, file_contents, dir)
+		zipinfo = zipfile.infolist()
+		zipnames = [z.filename for z in zipinfo]
+		assert 'dir/c' in zipnames
+		assert 'dir/d' in zipnames
+
+	def test_finish_zip_basic(self):
+		""" Test that zip is ready to go """
+		zipfile_str, zipfile = self.test_add_to_zip_basic()
+		assert utils.finish_zip(zipfile_str, zipfile) is not None
+
+	###########################
+	# TEST OK-GCS ABSTRACTION #
+	###########################

server/tests/test_base.py

@@ -34,7 +34,7 @@
 from app import models
 from app import auth
 from app.constants import API_PREFIX
-from app import api
+from app import api, utils
 from app.authenticator import Authenticator, AuthenticationException
 
 def make_fake_course(creator):