Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds django storage backend for ORA2 file uploads. #1018

Merged
merged 1 commit into from
Jul 14, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions openassessment/fileupload/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@

from . import backends


def get_upload_url(key, content_type):
"""
Returns a url (absolute or relative, depending on the endpoint) which can be used to upload a file to.
"""
return backends.get_backend().get_upload_url(key, content_type)


def get_download_url(key):
"""
Returns the url at which the file that corresponds to the key can be downloaded.
Expand Down
3 changes: 3 additions & 0 deletions openassessment/fileupload/backends/__init__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from . import s3
from . import filesystem
from . import swift
from . import django_storage

from django.conf import settings

Expand All @@ -14,5 +15,7 @@ def get_backend():
return filesystem.Backend()
elif backend_setting == "swift":
return swift.Backend()
elif backend_setting == "django":
return django_storage.Backend()
else:
raise ValueError("Invalid ORA2_FILEUPLOAD_BACKEND setting value: %s" % backend_setting)
2 changes: 0 additions & 2 deletions openassessment/fileupload/backends/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ def _retrieve_parameters(self, key):
raise FileUploadRequestError("Key required for URL request")
return Settings.get_bucket_name(), self._get_key_name(key)


def _get_key_name(self, key):
"""Construct a key name with the given string and configured prefix.

Expand All @@ -140,4 +139,3 @@ def _get_key_name(self, key):
prefix=Settings.get_prefix(),
key=key
)

67 changes: 67 additions & 0 deletions openassessment/fileupload/backends/django_storage.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import os
from .base import BaseBackend

from django.core.files.storage import default_storage
from django.core.files.base import ContentFile
from django.core.urlresolvers import reverse


class Backend(BaseBackend):
"""
Manage openassessment student files uploaded using the default django storage settings.
"""
def get_upload_url(self, key, content_type):
"""
Return the URL pointing to the ORA2 django storage upload endpoint.
"""
return reverse("openassessment-django-storage", kwargs={'key': key})

def get_download_url(self, key):
"""
Return the django storage download URL for the given key.

Returns None if no file exists at that location.
"""
path = self._get_file_path(key)
if default_storage.exists(path):
return default_storage.url(path)
return None

def upload_file(self, key, content):
"""
Upload the given file content to the keyed location.
"""
path = self._get_file_path(key)
saved_path = default_storage.save(path, ContentFile(content))
return saved_path

def remove_file(self, key):
"""
Remove the file at the given keyed location.

Returns True if the file exists, and was removed.
Returns False if the file does not exist, and so was not removed.
"""
path = self._get_file_path(key)
if default_storage.exists(path):
default_storage.delete(path)
return True
return False

def _get_file_name(self, key):
"""
Returns the name of the keyed file.

Since the backend storage may be folders, or it may use pseudo-folders,
make sure the filename doesn't include any path separators.
"""
file_name = key.replace("..", "").strip("/ ")
file_name = file_name.replace(os.sep, "_")
return file_name

def _get_file_path(self, key):
"""
Returns the path to the keyed file, including the storage prefix.
"""
path = self._get_key_name(self._get_file_name(key))
return path
5 changes: 5 additions & 0 deletions openassessment/fileupload/backends/filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ def make_upload_url_available(url_key_name, timeout):
1, timeout
)


def make_download_url_available(url_key_name, timeout):
"""
Authorize a download URL.
Expand All @@ -93,20 +94,24 @@ def make_download_url_available(url_key_name, timeout):
1, timeout
)


def is_upload_url_available(url_key_name):
"""
Return True if the corresponding upload URL is available.
"""
return get_cache().get(smart_text(get_upload_cache_key(url_key_name))) is not None


def is_download_url_available(url_key_name):
"""
Return True if the corresponding download URL is available.
"""
return get_cache().get(smart_text(get_download_cache_key(url_key_name))) is not None


def get_upload_cache_key(url_key_name):
return "upload/" + url_key_name


def get_download_cache_key(url_key_name):
return "download/" + url_key_name
5 changes: 1 addition & 4 deletions openassessment/fileupload/backends/s3.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@
import logging
from django.conf import settings

logger = logging.getLogger("openassessment.fileupload.api")

from .base import BaseBackend
from ..exceptions import FileUploadInternalError
logger = logging.getLogger("openassessment.fileupload.api")


class Backend(BaseBackend):
Expand Down Expand Up @@ -70,5 +69,3 @@ def _connect_to_s3():
aws_access_key_id=aws_access_key_id,
aws_secret_access_key=aws_secret_access_key
)


4 changes: 1 addition & 3 deletions openassessment/fileupload/backends/swift.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,9 @@
import urlparse
import requests


logger = logging.getLogger("openassessment.fileupload.api")

from .base import BaseBackend
from ..exceptions import FileUploadInternalError
logger = logging.getLogger("openassessment.fileupload.api")


class Backend(BaseBackend):
Expand Down
96 changes: 96 additions & 0 deletions openassessment/fileupload/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@
import os
import shutil
import tempfile
import urllib
from urlparse import urlparse

from django.conf import settings
from django.test import TestCase
from django.test.utils import override_settings
from django.core.urlresolvers import reverse
from django.contrib.auth import get_user_model

from moto import mock_s3
from mock import patch
Expand Down Expand Up @@ -364,3 +366,97 @@ def test_get_download_url_no_object(self, requests_get_mock):
requests_get_mock.return_value = fake_resp
url = self.backend.get_download_url('foo')
self.assertEqual(url, '')


@override_settings(
ORA2_FILEUPLOAD_BACKEND="django",
DEFAULT_FILE_STORAGE="django.core.files.storage.FileSystemStorage",
FILE_UPLOAD_STORAGE_PREFIX="submissions",
)
@ddt.ddt
class TestFileUploadServiceWithDjangoStorageBackend(TestCase):
"""
Test open assessment file upload using django default storage backend.

For testing purposes, the django filesystem storage class is used.
"""

def setUp(self):
self.backend = api.backends.get_backend()
self.username = 'test_user'
self.password = 'password'
self.user = get_user_model().objects.create_user(username=self.username, password=self.password)

self.content = tempfile.TemporaryFile()
self.content.write("foobar content")
self.content.seek(0)

self.key = "myfile.txt"
self.content_type = "text/plain"
self.tearDown()

def tearDown(self):
self.backend.remove_file(self.key)

def test_get_backend(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please include docstrings on these tests

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed by bb88ed1

"""
Ensure the django storage backend is returned when ORA2_FILEUPLOAD_BACKEND="django".
"""
self.assertTrue(isinstance(self.backend, api.backends.django_storage.Backend))

def test_upload_login_required(self):
"""
Ensure the django file upload endpoint requires authentication.
"""
upload_url = self.backend.get_upload_url(self.key, "bar")
response = self.client.put(upload_url, data={"attachment": self.content})
self.assertEqual(302, response.status_code)

@ddt.data(u"noël.txt", "myfile.txt")
def test_upload_download(self, key):
"""
Test that uploaded files can be downloaded again.
"""
# Download URL is None until the file is uploaded
self.key = key
download_url = self.backend.get_download_url(self.key)
self.assertIsNone(download_url)

# Upload file
self.client.login(username=self.username, password=self.password)
upload_url = self.backend.get_upload_url(self.key, "bar")
response = self.client.put(upload_url, data=self.content.read(), content_type=self.content_type)
self.assertEqual(200, response.status_code)

# Check updated download URL
download_url = self.backend.get_download_url(self.key)
encoded_key = urllib.quote(self.key.encode('utf-8'))
self.assertEqual(u"submissions/{}".format(encoded_key), download_url)

@ddt.data(u"noël.txt", "myfile.txt")
def test_remove(self, key):
"""
Test that uploaded files can be removed.
"""
self.key = key

# Remove file returns False if file does not exist
self.assertFalse(self.backend.remove_file(self.key))

# Upload file
self.client.login(username=self.username, password=self.password)
upload_url = self.backend.get_upload_url(self.key, "bar")
response = self.client.put(upload_url, data=self.content.read(), content_type=self.content_type)
self.assertEqual(200, response.status_code)

# File exists now
download_url = self.backend.get_download_url(self.key)
encoded_key = urllib.quote(self.key.encode('utf-8'))
self.assertEqual(u"submissions/{}".format(encoded_key), download_url)

# Remove file returns True now, and removes the file
self.assertTrue(self.backend.remove_file(self.key))

# File no longer exists
download_url = self.backend.get_download_url(self.key)
self.assertIsNone(download_url)
7 changes: 6 additions & 1 deletion openassessment/fileupload/urls.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
from django.conf.urls import patterns, url

urlpatterns = patterns(
'openassessment.fileupload.views_django_storage',
url(r'^django/(?P<key>.+)/$', 'django_storage', name='openassessment-django-storage'),
)

urlpatterns = patterns('openassessment.fileupload.views_filesystem',
urlpatterns += patterns(
'openassessment.fileupload.views_filesystem',
url(r'^(?P<key>.+)/$', 'filesystem_storage', name='openassessment-filesystem-storage'),
)
18 changes: 18 additions & 0 deletions openassessment/fileupload/views_django_storage.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
"""
Provides the upload endpoint for the django storage backend.
"""
from django.contrib.auth.decorators import login_required
from django.shortcuts import HttpResponse
from django.views.decorators.http import require_http_methods

from .backends.django_storage import Backend


@login_required()
@require_http_methods(["PUT"])
def django_storage(request, key):
"""
Upload files using django storage backend.
"""
Backend().upload_file(key, request.body)
return HttpResponse()
2 changes: 2 additions & 0 deletions openassessment/fileupload/views_filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ def save_to_file(key, content, metadata=None):
safe_remove(metadata_path)
raise


def safe_save(path, content):
"""
Save content to path. Creates the appropriate directories, if required.
Expand All @@ -107,6 +108,7 @@ def safe_save(path, content):
with open(path, 'w') as f:
f.write(content)


def safe_remove(path):
"""Remove a file if it exists.

Expand Down