Skip to content

Commit ee29b2e

Browse files
committed
feat: implement submission file workflow with comprehensive testing
- Create Submission file model - Develop SubmissionFileManager for centralized DB query handling - Add classes to handle file errors - Add TestSubmissionFileManager with extensive test coverage - Test file processing with different input types (bytes, file objects) - Implement error handling tests for IO errors and invalid files - Verify complete file processing workflow - Validation tests for file addition in create_external_grader_detail - Document architectural decisions with ADR - Add test queue folder in .gitignore
1 parent 2413e6a commit ee29b2e

9 files changed

Lines changed: 668 additions & 2 deletions

File tree

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,3 +63,4 @@ test.txt.tmp
6363

6464
# VSCode
6565
.vscode
66+
test_queue/*
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
2. File Handling Implementation for Submission System
2+
#####################################################
3+
4+
Status
5+
******
6+
7+
**Provisional** *2025-02-14*
8+
9+
Implemented by https://github.com/openedx/edx-submissions/pull/286
10+
11+
Context
12+
*******
13+
14+
As part of the XQueue migration effort detailed in ADR 0001, we need to implement a file handling system within edx-submissions. Currently, XQueue manages file submissions through a tightly coupled approach.
15+
16+
### Current Limitations
17+
18+
1. **Inadequate File Management**: XQueue's approach relies on JSON strings in character fields, with size constraints and manual URL manipulation for file handling.
19+
20+
2. **Process Inefficiencies**: The current system uses synchronous HTTP for file retrieval, lacks proper validation, and tightly couples submission processing with file handling.
21+
22+
3. **Integration Challenges**: External graders depend on specific URL formats with HTTP-based retrieval, embedding file information directly in submission payloads.
23+
24+
Decision
25+
********
26+
27+
We will implement a dedicated file management system for the assessment submission process, focusing on workflow and educational needs:
28+
29+
1. **Centralized Storage**: Create a unified repository for student-submitted files, ensuring they are properly associated with their assessments and accessible throughout the grading process.
30+
31+
2. **Streamlined Workflow**: Design a clear process where files are automatically processed during submission creation, securely stored, and efficiently delivered to grading systems.
32+
33+
3. **Consistent Experience**: Maintain compatibility with existing systems to ensure a smooth transition, allowing instructors and external graders to access files without changes to their established workflows.
34+
35+
Consequences
36+
************
37+
38+
Positive:
39+
---------
40+
41+
1. **Architecture**: Clean separation of concerns, improved maintainability, better error handling, optimized database access
42+
43+
2. **Integration**: Seamless xqueue-watcher compatibility, support for existing workflows, minimal client code changes
44+
45+
3. **Operations**: Robust file validation, improved tracking, better error visibility, simplified lifecycle management
46+
47+
Negative:
48+
---------
49+
50+
1. **Technical**: Additional database structures
51+
52+
2. **Migration**: Temporary system complexity, additional testing needs
53+
54+
3. **Performance**: File processing overhead
55+
56+
References
57+
**********
58+
59+
Current System Documentation:
60+
* XQueue Repository: https://github.com/openedx/xqueue
61+
* XQueue Watcher Repository: https://github.com/openedx/xqueue-watcher
62+
63+
Related Repositories:
64+
* edx-submissions: https://github.com/openedx/edx-submissions
65+
* edx-platform: https://github.com/openedx/edx-platform
66+
* XQueue Repository: https://github.com/openedx/xqueue
67+
68+
Related Documentation:
69+
* ADR 0001: Creation of ExternalGraderDetail Model for XQueue Migration
70+
71+
Future Event Integration:
72+
* Open edX Events Framework: https://github.com/openedx/openedx-events
73+
* Event Bus Documentation: https://openedx.atlassian.net/wiki/spaces/AC/pages/124125264/Event+Bus
74+
75+
Related Architecture Documents:
76+
* Open edX Architecture Guidelines: https://openedx.atlassian.net/wiki/spaces/AC/pages/124125264/Architecture+Guidelines
77+

submissions/api.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
ScoreSummary,
2929
StudentItem,
3030
Submission,
31+
SubmissionFileManager,
3132
score_reset,
3233
score_set
3334
)
@@ -49,7 +50,6 @@
4950
TOP_SUBMISSIONS_CACHE_TIMEOUT = 300
5051

5152

52-
# pylint: disable=unused-argument
5353
def create_external_grader_detail(student_item_dict,
5454
answer,
5555
queue_name: str,
@@ -96,6 +96,12 @@ def create_external_grader_detail(student_item_dict,
9696
points_possible=points_possible,
9797

9898
)
99+
100+
files_dict = external_grader_additional_data.get('files')
101+
if files_dict:
102+
file_manager = SubmissionFileManager(instance)
103+
file_manager.process_files(files_dict)
104+
99105
return instance
100106

101107
except DatabaseError as error:

submissions/errors.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,27 @@ class ExternalGraderQueueEmptyError(SubmissionError):
4040
"""
4141

4242

43+
class InvalidFileTypeError(SubmissionError):
44+
"""
45+
Exception raised when a file object has an unsupported or invalid type.
46+
47+
This exception is typically raised during file processing when the system
48+
encounters a file object that doesn't match any of the expected types
49+
(bytes, ContentFile, SimpleUploadedFile) and doesn't have a 'read' method.
50+
"""
51+
52+
53+
class FileProcessingError(SubmissionError):
54+
"""
55+
Exception raised when there's an error reading or processing a file.
56+
57+
This exception is raised when file operations fail, such as:
58+
- I/O errors when reading file content
59+
- OS errors during file operations
60+
- Unicode decoding errors when processing file content
61+
"""
62+
63+
4364
class SubmissionRequestError(SubmissionError):
4465
"""
4566
This error is raised when there was a request-specific error
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# Generated by Django 4.2.19 on 2025-04-04 16:05
2+
3+
from django.db import migrations, models
4+
import django.db.models.deletion
5+
import django.utils.timezone
6+
import submissions.models
7+
import uuid
8+
9+
10+
class Migration(migrations.Migration):
11+
12+
dependencies = [
13+
('submissions', '0004_externalgraderdetail'),
14+
]
15+
16+
operations = [
17+
migrations.CreateModel(
18+
name='SubmissionFile',
19+
fields=[
20+
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
21+
('uid', models.UUIDField(default=uuid.uuid4, editable=False)),
22+
('file', models.FileField(max_length=512, upload_to=submissions.models.submission_file_path)),
23+
('original_filename', models.CharField(max_length=255)),
24+
('created_at', models.DateTimeField(default=django.utils.timezone.now)),
25+
('external_grader', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL,
26+
related_name='files', to='submissions.externalgraderdetail')),
27+
],
28+
options={
29+
'indexes': [models.Index(fields=['external_grader', 'uid'], name='submissions_externa_657c60_idx')],
30+
},
31+
),
32+
]

submissions/models.py

Lines changed: 143 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,28 @@
1010
"""
1111

1212
import logging
13+
import os
1314
from datetime import timedelta
1415
from uuid import uuid4
1516

1617
from django.conf import settings
1718
from django.contrib import auth
19+
from django.core.files.base import ContentFile
20+
from django.core.files.uploadedfile import SimpleUploadedFile
1821
from django.db import DatabaseError, models, transaction
1922
from django.db.models.signals import post_save, pre_save
2023
from django.dispatch import Signal, receiver
2124
from django.utils.timezone import now
2225
from jsonfield import JSONField
2326
from model_utils.models import TimeStampedModel
2427

25-
from submissions.errors import DuplicateTeamSubmissionsError, TeamSubmissionInternalError, TeamSubmissionNotFoundError
28+
from submissions.errors import (
29+
DuplicateTeamSubmissionsError,
30+
FileProcessingError,
31+
InvalidFileTypeError,
32+
TeamSubmissionInternalError,
33+
TeamSubmissionNotFoundError
34+
)
2635

2736
logger = logging.getLogger(__name__)
2837
User = auth.get_user_model()
@@ -693,3 +702,136 @@ def update_status(self, new_status):
693702
def create_from_uuid(cls, submission_uuid, **kwargs):
694703
submission = Submission.objects.get(uuid=submission_uuid)
695704
return cls.objects.create(submission=submission, **kwargs)
705+
706+
707+
def submission_file_path(instance, _):
708+
"""
709+
Generate file path for submission files.
710+
Format: queue_name/uuid
711+
The filename is replaced with the UUID to ensure uniqueness without preserving extension.
712+
"""
713+
return os.path.join(
714+
instance.external_grader.queue_name,
715+
f"{instance.uid}"
716+
)
717+
718+
719+
class SubmissionFile(models.Model):
720+
"""
721+
Model to handle files associated with submissions
722+
"""
723+
uid = models.UUIDField(default=uuid4, editable=False) # legacy S3 key
724+
external_grader = models.ForeignKey(
725+
'submissions.ExternalGraderDetail',
726+
on_delete=models.SET_NULL,
727+
related_name='files',
728+
null=True,
729+
)
730+
file = models.FileField(
731+
upload_to=submission_file_path,
732+
max_length=512
733+
)
734+
original_filename = models.CharField(max_length=255) # This is necessary to send file name to xqueue-watcher
735+
created_at = models.DateTimeField(default=now)
736+
737+
class Meta:
738+
indexes = [
739+
models.Index(fields=['external_grader', 'uid']),
740+
]
741+
742+
@property
743+
def xqueue_url(self):
744+
"""
745+
Returns URL in xqueue format: /queue_name/uid
746+
"""
747+
return f"/{self.external_grader.queue_name}/{self.uid}"
748+
749+
750+
class SubmissionFileManager:
751+
"""
752+
Manages file operations for submissions
753+
"""
754+
755+
def __init__(self, external_grader):
756+
self.external_grader = external_grader
757+
758+
def process_files(self, files_dict):
759+
"""
760+
Process uploaded files from an Open edX environment and store them as SubmissionFile objects.
761+
762+
This method handles various file object types that might be received from Open edX, including:
763+
- Native Open edX FileObjForWebobFiles objects
764+
- Bytes objects
765+
- ContentFile objects
766+
- SimpleUploadedFile objects
767+
- Any object with a 'read' method
768+
769+
The method performs the following operations:
770+
1. Validates each file object type
771+
2. Reads content from file-like objects
772+
3. Converts byte content to ContentFile objects
773+
4. Creates SubmissionFile records in the database
774+
5. Returns URLs in xqueue-compatible format
775+
776+
Args:
777+
files_dict (dict): Dictionary mapping filenames to file objects.
778+
Format: {filename: file_object, ...}
779+
780+
Returns:
781+
dict: Dictionary mapping original filenames to xqueue URLs.
782+
Format: {filename: "/queue_name/uuid", ...}
783+
784+
Raises:
785+
InvalidFileTypeError: If a file object has an unsupported type.
786+
FileProcessingError: If there's an error reading from a file object,
787+
including I/O errors or Unicode decoding errors.
788+
789+
Example:
790+
>>> file_manager = SubmissionFileManager(external_grader)
791+
>>> files = {'assignment.py': file_obj}
792+
>>> urls = file_manager.process_files(files)
793+
>>> print(urls)
794+
{'assignment.py': '/my_queue/550e8400-e29b-41d4-a716-446655440000'}
795+
"""
796+
files_urls = {}
797+
for filename, file_obj in files_dict.items():
798+
# Validate file object type
799+
if not (isinstance(file_obj, (bytes, ContentFile, SimpleUploadedFile)) or hasattr(file_obj, 'read')):
800+
logger.error(f"Invalid file object type for {filename}")
801+
raise InvalidFileTypeError(f"Invalid file object type for {filename}")
802+
803+
# Handle file-like objects by reading their content
804+
# This returns a bytes object that will be converted to ContentFile later
805+
if hasattr(file_obj, 'read'):
806+
try:
807+
file_obj = file_obj.read() # read() returns a bytes object
808+
except (IOError, OSError) as e:
809+
logger.error(f"Error reading file {filename}: {e}")
810+
raise FileProcessingError(f"Error reading file {filename}: {e}") from e
811+
except UnicodeDecodeError as e:
812+
logger.error(f"Error decoding file {filename}: {e}")
813+
raise FileProcessingError(f"Error decoding file {filename}: {e}") from e
814+
815+
# Convert bytes to ContentFile
816+
# The read() method from file-like objects returns bytes, which we handle here
817+
if isinstance(file_obj, bytes):
818+
file_obj = ContentFile(file_obj, name=filename)
819+
820+
# Create a SubmissionFile record for storage and retrieval
821+
submission_file = SubmissionFile.objects.create(
822+
external_grader=self.external_grader,
823+
file=file_obj,
824+
original_filename=filename
825+
)
826+
files_urls[filename] = submission_file.xqueue_url
827+
828+
return files_urls
829+
830+
def get_files_for_grader(self):
831+
"""
832+
Returns files in format expected by xqueue-watcher
833+
"""
834+
return {
835+
file.original_filename: file.file.url
836+
for file in self.external_grader.files.all()
837+
}

0 commit comments

Comments
 (0)