Inconsistent UUID format handling

Submission.uuid is now being serialized as a hex string w/o hyphens,
but old data exists in the database with them.

This commit will make those old submissions accessible again and, as
a bonus, fix them to be new-style uuids on first load. In addition,
a management command has been added to asynchronously update all old
values to new ones.

setup.py

@@ -33,7 +33,7 @@ def load_requirements(*requirements_paths):
 
 setup(
     name='edx-submissions',
-    version='2.0.8',
+    version='2.0.9',
     author='edX',
     description='An API for creating submissions and scores.',
     url='http://github.com/edx/edx-submissions.git',

submissions/api.py

@@ -204,9 +204,36 @@ def _get_submission_model(uuid, read_replica=False):
     submission_qs = Submission.objects
     if read_replica:
         submission_qs = _use_read_replica(submission_qs)
-
-    query_regex = "^{}$|^{}$".format(uuid, uuid.replace("-",""))
-    submission = submission_qs.get(uuid__regex=query_regex)
+    try:
+        submission = submission_qs.get(uuid=uuid)
+    except Submission.DoesNotExist:
+        try:
+            hyphenated_value = unicode(UUID(uuid))
+            query = """
+                SELECT
+                    `submissions_submission`.`id`,
+                    `submissions_submission`.`uuid`,
+                    `submissions_submission`.`student_item_id`,
+                    `submissions_submission`.`attempt_number`,
+                    `submissions_submission`.`submitted_at`,
+                    `submissions_submission`.`created_at`,
+                    `submissions_submission`.`raw_answer`,
+                    `submissions_submission`.`status`
+                FROM
+                    `submissions_submission`
+                WHERE (
+                    NOT (`submissions_submission`.`status` = 'D')
+                    AND `submissions_submission`.`uuid` = '{}'
+                )
+            """
+            query = query.replace("{}", hyphenated_value)
+
+            # We can use Submission.objects instead of the SoftDeletedManager, we'll include that logic manually
+            submission = Submission.objects.raw(query)[0]
+        except IndexError:
+            raise Submission.DoesNotExist()
+        # Avoid the extra hit next time
+        submission.save(update_fields=['uuid'])
     return submission
 
 

submissions/management/commands/update_submissions_uuids.py

+"""
+Command to update all instances of old-style (hyphenated) uuid values in the
+submissions_submission table.
+
+This command takes a long time to execute, please run it on a long-lived
+background worker. The model code is resilient to both styles of uuid, this
+command just standardizes them all to be similar.
+
+EDUCATOR-1090
+"""
+
+import logging
+
+from django.core.management.base import BaseCommand
+from django.db import transaction
+
+from submissions.models import Submission
+
+log = logging.getLogger(__name__)
+
+class Command(BaseCommand):
+    """
+    Example usage: ./manage.py lms --settings=devstack update_submissions_uuids.py
+    """
+    help = 'Loads and saves all Submissions objects to force new non-hyphenated uuid values on disk.'
+
+    def handle(self, *args, **options):
+        """
+        By default, we're going to do this in chunks. This way, if there ends up being an error,
+        we can check log messages and continue from that point after fixing the issue.
+        """
+        START_VALUE = 0
+        CHUNK_SIZE = 1000
+        total_len = Submission.objects.count()
+        log.info("Beginning uuid update, {} rows exist in total")
+
+        current = START_VALUE;
+        while current < total_len:
+            end_chunk = current + CHUNK_SIZE
+            log.info("Updating entries {} to {}".format(current, end_chunk))
+            with transaction.atomic():
+                for submission in Submission.objects.filter(id__gte=current, id__lt=end_chunk).iterator():
+                    submission.save(update_fields=['uuid'])
+            current = current + CHUNK_SIZE

submissions/tests/test_api.py

@@ -9,7 +9,7 @@ from django.core.cache import cache
 from django.test import TestCase
 from freezegun import freeze_time
 from nose.tools import raises
-from mock import patch
+import mock
 import pytz
 
 from submissions import api as api
@@ -164,13 +164,46 @@ class TestSubmissionsApi(TestCase):
         with self.assertRaises(api.SubmissionNotFoundError):
             api.get_submission("deadbeef-1234-5678-9100-1234deadbeef")
 
-    @patch.object(Submission.objects, 'get')
+    @mock.patch.object(Submission.objects, 'get')
     @raises(api.SubmissionInternalError)
     def test_get_submission_deep_error(self, mock_get):
         # Test deep explosions are wrapped
         mock_get.side_effect = DatabaseError("Kaboom!")
         api.get_submission("000000000000000")
 
+    def test_get_old_submission(self):
+        # hack in an old-style submission, this can't be created with the ORM (EDUCATOR-1090)
+        with transaction.atomic():
+            student_item = StudentItem.objects.create()
+            connection.cursor().execute("""
+                INSERT INTO submissions_submission
+                    (id, uuid, attempt_number, submitted_at, created_at, raw_answer, student_item_id, status)
+                VALUES (
+                    {}, {}, {}, {}, {}, {}, {}, {}
+                );""".format(
+                    1,
+                    "\'deadbeef-1234-5678-9100-1234deadbeef\'",
+                    1,
+                    "\'2017-07-13 17:56:02.656129\'",
+                    "\'2017-07-13 17:56:02.656129\'",
+                    "\'{\"parts\":[{\"text\":\"raw answer text\"}]}\'",
+                    int(student_item.id),
+                    "\'A\'"
+                ), []
+            )
+
+        with mock.patch.object(
+            Submission.objects, 'raw',
+            wraps=Submission.objects.raw
+        ) as mock_raw:
+            _ = api.get_submission('deadbeef-1234-5678-9100-1234deadbeef')
+            self.assertEqual(1, mock_raw.call_count)
+
+            # On subsequent accesses we still get the submission, but raw() isn't needed
+            mock_raw.reset_mock()
+            _ = api.get_submission('deadbeef-1234-5678-9100-1234deadbeef')
+            self.assertEqual(0, mock_raw.call_count)
+
     def test_two_students(self):
         api.create_submission(STUDENT_ITEM, ANSWER_ONE)
         api.create_submission(SECOND_STUDENT_ITEM, ANSWER_TWO)
@@ -221,7 +254,7 @@ class TestSubmissionsApi(TestCase):
         # Attempt number should be >= 0
         api.create_submission(STUDENT_ITEM, ANSWER_ONE, None, -1)
 
-    @patch.object(Submission.objects, 'filter')
+    @mock.patch.object(Submission.objects, 'filter')
     @raises(api.SubmissionInternalError)
     def test_error_on_submission_creation(self, mock_filter):
         mock_filter.side_effect = DatabaseError("Bad things happened")
@@ -247,7 +280,7 @@ class TestSubmissionsApi(TestCase):
         with self.assertRaises(api.SubmissionInternalError):
             api.get_submission_and_student(sub_model.uuid)
 
-    @patch.object(StudentItemSerializer, 'save')
+    @mock.patch.object(StudentItemSerializer, 'save')
     @raises(api.SubmissionInternalError)
     def test_create_student_item_validation(self, mock_save):
         mock_save.side_effect = DatabaseError("Bad things happened")
@@ -302,7 +335,7 @@ class TestSubmissionsApi(TestCase):
         self.assertFalse(ScoreAnnotation.objects.all().exists())
 
     @freeze_time(datetime.datetime.now())
-    @patch.object(score_set, 'send')
+    @mock.patch.object(score_set, 'send')
     def test_set_score_signal(self, send_mock):
         submission = api.create_submission(STUDENT_ITEM, ANSWER_ONE)
         api.set_score(submission['uuid'], 11, 12)
@@ -688,7 +721,7 @@ class TestSubmissionsApi(TestCase):
             read_replica=False
         )
 
-    @patch.object(ScoreSummary.objects, 'filter')
+    @mock.patch.object(ScoreSummary.objects, 'filter')
     @raises(api.SubmissionInternalError)
     def test_error_on_get_top_submissions_db_error(self, mock_filter):
         mock_filter.side_effect = DatabaseError("Bad things happened")
@@ -700,7 +733,7 @@ class TestSubmissionsApi(TestCase):
             read_replica=False
         )
 
-    @patch.object(ScoreSummary.objects, 'filter')
+    @mock.patch.object(ScoreSummary.objects, 'filter')
     @raises(api.SubmissionInternalError)
     def test_error_on_get_scores(self, mock_filter):
         mock_filter.side_effect = DatabaseError("Bad things happened")