diff --git a/kolibri/core/content/api.py b/kolibri/core/content/api.py index 0865670fe16..d9cf961058f 100644 --- a/kolibri/core/content/api.py +++ b/kolibri/core/content/api.py @@ -5,7 +5,6 @@ import requests from django.core.cache import cache -from django.core.urlresolvers import reverse from django.db.models import Exists from django.db.models import OuterRef from django.db.models import Q @@ -52,7 +51,6 @@ get_channel_stats_from_studio, ) from kolibri.core.content.utils.paths import get_channel_lookup_url -from kolibri.core.content.utils.paths import get_content_file_name from kolibri.core.content.utils.paths import get_info_url from kolibri.core.content.utils.paths import get_local_content_storage_file_url from kolibri.core.content.utils.stopwords import stopwords_set @@ -231,25 +229,18 @@ def map_lang(obj): return output -def map_file(file, obj): - url_lookup = { - "available": file["available"], - "id": file["checksum"], - "extension": file["extension"], - } - download_filename = models.get_download_filename( - obj["title"], - models.PRESET_LOOKUP.get(file["preset"], _("Unknown format")), - file["extension"], - ) - file["download_url"] = reverse( - "kolibri:core:downloadcontent", - kwargs={ - "filename": get_content_file_name(url_lookup), - "new_filename": download_filename, - }, +def map_file(file): + file["checksum"] = file.pop("local_file__id") + file["available"] = file.pop("local_file__available") + file["file_size"] = file.pop("local_file__file_size") + file["extension"] = file.pop("local_file__extension") + file["storage_url"] = get_local_content_storage_file_url( + { + "available": file["available"], + "id": file["checksum"], + "extension": file["extension"], + } ) - file["storage_url"] = get_local_content_storage_file_url(url_lookup) file["lang"] = map_lang(file) return file @@ -331,11 +322,7 @@ def consolidate(self, items, queryset): ): if f["contentnode"] not in files: files[f["contentnode"]] = [] - f["checksum"] = f.pop("local_file__id") - f["available"] = f.pop("local_file__available") - f["file_size"] = f.pop("local_file__file_size") - f["extension"] = f.pop("local_file__extension") - files[f["contentnode"]].append(f) + files[f["contentnode"]].append(map_file(f)) ancestors = queryset.get_ancestors().values( "id", "title", "lft", "rght", "tree_id" @@ -356,10 +343,8 @@ def consolidate(self, items, queryset): for item in items: item["assessmentmetadata"] = assessmentmetadata.get(item["id"]) - item["files"] = list( - map(lambda x: map_file(x, item), files.get(item["id"], [])) - ) item["tags"] = tags.get(item["id"], []) + item["files"] = files.get(item["id"], []) lft = item.pop("lft") rght = item.pop("rght") diff --git a/kolibri/core/content/management/commands/exportcontent.py b/kolibri/core/content/management/commands/exportcontent.py index b61c1dfd717..193c000184a 100644 --- a/kolibri/core/content/management/commands/exportcontent.py +++ b/kolibri/core/content/management/commands/exportcontent.py @@ -1,6 +1,8 @@ import logging import os +from django.core.management.base import CommandError + from ...utils import paths from ...utils import transfer from kolibri.core.content.errors import InvalidStorageFilenameError @@ -60,6 +62,8 @@ def update_job_metadata(self, total_bytes_to_transfer, total_resource_count): job.save_meta() def handle_async(self, *args, **options): + if paths.using_remote_storage(): + raise CommandError("Cannot export files when using remote file storage") channel_id = options["channel_id"] data_dir = os.path.realpath(options["destination"]) node_ids = options["node_ids"] diff --git a/kolibri/core/content/management/commands/importcontent.py b/kolibri/core/content/management/commands/importcontent.py index f85ba6ee2dd..ed997d8d5ac 100644 --- a/kolibri/core/content/management/commands/importcontent.py +++ b/kolibri/core/content/management/commands/importcontent.py @@ -294,8 +294,10 @@ def _transfer( # noqa: max-complexity=16 overall_progress_update(f.file_size) continue - # if the file already exists, add its size to our overall progress, and skip - if os.path.isfile(dest) and os.path.getsize(dest) == f.file_size: + # if the file already exists, or we are using remote storage, add its size to our overall progress, and skip + if paths.using_remote_storage() or ( + os.path.isfile(dest) and os.path.getsize(dest) == f.file_size + ): overall_progress_update(f.file_size) file_checksums_to_annotate.append(f.id) transferred_file_size += f.file_size diff --git a/kolibri/core/content/models.py b/kolibri/core/content/models.py index dfb2d029d9c..122cc54134b 100644 --- a/kolibri/core/content/models.py +++ b/kolibri/core/content/models.py @@ -25,14 +25,12 @@ import os from gettext import gettext as _ -from django.core.urlresolvers import reverse from django.db import connection from django.db import models from django.db.models import Min from django.db.models import Q from django.db.models import QuerySet from django.utils.encoding import python_2_unicode_compatible -from django.utils.text import get_valid_filename from le_utils.constants import content_kinds from le_utils.constants import format_presets from mptt.managers import TreeManager @@ -194,15 +192,6 @@ def __str__(self): return self.lang_name or "" -def get_download_filename(title, preset, extension): - """ - Return a valid filename to be downloaded as. - """ - filename = "{} ({}).{}".format(title, preset, extension) - valid_filename = get_valid_filename(filename) - return valid_filename - - class File(base_models.File): """ The second to bottom layer of the contentDB schema, defines the basic building brick for content. @@ -230,27 +219,6 @@ def get_preset(self): """ return PRESET_LOOKUP.get(self.preset, _("Unknown format")) - def get_download_filename(self): - """ - Return a valid filename to be downloaded as. - """ - return get_download_filename( - self.contentnode.title, self.get_preset(), self.get_extension() - ) - - def get_download_url(self): - """ - Return the download url. - """ - new_filename = self.get_download_filename() - return reverse( - "kolibri:core:downloadcontent", - kwargs={ - "filename": self.local_file.get_filename(), - "new_filename": new_filename, - }, - ) - class LocalFileQueryset(models.QuerySet, FilterByUUIDQuerysetMixin): def delete_unused_files(self): diff --git a/kolibri/core/content/serializers.py b/kolibri/core/content/serializers.py index b565014834a..ee7e67b7a9b 100644 --- a/kolibri/core/content/serializers.py +++ b/kolibri/core/content/serializers.py @@ -133,7 +133,6 @@ class Meta: class FileSerializer(serializers.ModelSerializer): checksum = serializers.CharField(source="local_file_id") storage_url = serializers.SerializerMethodField() - download_url = serializers.SerializerMethodField() extension = serializers.SerializerMethodField() file_size = serializers.SerializerMethodField() lang = LanguageSerializer() @@ -142,9 +141,6 @@ class FileSerializer(serializers.ModelSerializer): def get_storage_url(self, target_node): return target_node.get_storage_url() - def get_download_url(self, target_node): - return target_node.get_download_url() - def get_extension(self, target_node): return target_node.get_extension() @@ -165,7 +161,6 @@ class Meta: "lang", "supplementary", "thumbnail", - "download_url", ) diff --git a/kolibri/core/content/test/test_downloadcontent.py b/kolibri/core/content/test/test_downloadcontent.py deleted file mode 100644 index 1a2c692dab9..00000000000 --- a/kolibri/core/content/test/test_downloadcontent.py +++ /dev/null @@ -1,82 +0,0 @@ -import hashlib -import mimetypes -import os -import tempfile - -from django.test import Client -from django.test import TestCase -from le_utils.constants import file_formats -from le_utils.constants import format_presets - -from kolibri.core.auth.test.helpers import provision_device -from kolibri.core.content.models import ContentNode -from kolibri.core.content.models import File -from kolibri.core.content.models import LocalFile -from kolibri.core.content.utils.paths import get_content_storage_file_path -from kolibri.utils.tests.helpers import override_option - - -@override_option("Paths", "CONTENT_DIR", tempfile.mkdtemp()) -class DownloadContentTestCase(TestCase): - """ - Test case for the downloadcontent endpoint. - """ - - def setUp(self): - provision_device() - - self.client = Client() - self.hash = hashlib.md5("DUMMYDATA".encode()).hexdigest() - self.extension = file_formats.PDF - self.filename = "{}.{}".format(self.hash, self.extension) - self.title = "abc123!@#$%^&*();'[],./?><" - self.contentnode = ContentNode(title=self.title) - self.available = True - self.preset = format_presets.DOCUMENT - self.local_file = LocalFile( - id=self.hash, extension=self.extension, available=self.available - ) - self.file = File( - local_file=self.local_file, contentnode=self.contentnode, preset=self.preset - ) - - self.path = get_content_storage_file_path(self.filename) - path_dir = os.path.dirname(self.path) - if not os.path.exists(path_dir): - os.makedirs(path_dir) - tempfile = open(self.path, "w") - tempfile.write("test") - tempfile.close() - - def test_generate_download_filename(self): - self.assertEqual( - self.file.get_download_filename(), - "abc123._Document.{}".format(self.extension), - ) - - def test_generate_download_url(self): - self.assertEqual( - self.file.get_download_url(), - "/downloadcontent/{}/{}".format( - self.filename, self.file.get_download_filename() - ), - ) - - def test_download_existing_file(self): - response = self.client.get(self.file.get_download_url()) - self.assertEqual(response.status_code, 200) - - def test_download_non_existing_file(self): - bad_download_url = self.file.get_download_url().replace( - self.file.get_download_url()[25:25], "aaaaa" - ) - response = self.client.get(bad_download_url) - self.assertEqual(response.status_code, 404) - - def test_download_headers(self): - response = self.client.get(self.file.get_download_url()) - self.assertEqual( - response["Content-Type"], mimetypes.guess_type(self.filename)[0] - ) - self.assertEqual(response["Content-Disposition"], "attachment;") - self.assertEqual(response["Content-Length"], str(os.path.getsize(self.path))) diff --git a/kolibri/core/content/urls.py b/kolibri/core/content/urls.py index ed5630ab5a6..a912644f3b8 100644 --- a/kolibri/core/content/urls.py +++ b/kolibri/core/content/urls.py @@ -1,14 +1,7 @@ from django.conf.urls import url from .views import ContentPermalinkRedirect -from .views import DownloadContentView urlpatterns = [ - url( - r"^downloadcontent/(?P[^/]+)/(?P.*)", - DownloadContentView.as_view(), - {}, - "downloadcontent", - ), url(r"^viewcontent$", ContentPermalinkRedirect.as_view(), name="contentpermalink"), ] diff --git a/kolibri/core/content/utils/annotation.py b/kolibri/core/content/utils/annotation.py index c61e08035c2..f54a41bdc43 100644 --- a/kolibri/core/content/utils/annotation.py +++ b/kolibri/core/content/utils/annotation.py @@ -18,6 +18,7 @@ from .paths import get_content_file_name from .paths import get_content_storage_file_path +from .paths import using_remote_storage from .sqlalchemybridge import Bridge from .sqlalchemybridge import filter_by_uuids from kolibri.core.content.apps import KolibriContentConfig @@ -434,7 +435,7 @@ def _check_file_availability(files): for file in files: try: # Update if the file exists, *and* the localfile is set as unavailable. - if os.path.exists( + if using_remote_storage() or os.path.exists( get_content_storage_file_path( get_content_file_name({"id": file[0], "extension": file[2]}) ) diff --git a/kolibri/core/content/utils/paths.py b/kolibri/core/content/utils/paths.py index bfab772eeee..e9cb07179de 100644 --- a/kolibri/core/content/utils/paths.py +++ b/kolibri/core/content/utils/paths.py @@ -187,6 +187,10 @@ def get_content_storage_file_path(filename, datafolder=None, contentfolder=None) return backup_path or primary_path +def using_remote_storage(): + return conf.OPTIONS["Deployment"]["REMOTE_CONTENT"] + + # URL PATHS diff --git a/kolibri/core/content/views.py b/kolibri/core/content/views.py index a7f484a5678..4ff31ace151 100644 --- a/kolibri/core/content/views.py +++ b/kolibri/core/content/views.py @@ -3,51 +3,17 @@ from __future__ import unicode_literals import logging -import mimetypes -import os from django.http import Http404 from django.http import HttpResponseRedirect -from django.http.response import FileResponse from django.views.generic.base import View from .models import ContentNode -from .utils.paths import get_content_storage_file_path from kolibri.core.content.hooks import ContentNodeDisplayHook logger = logging.getLogger(__name__) -class DownloadContentView(View): - def get(self, request, filename, new_filename): - """ - Handles GET requests and serves a static file as an attachment. - """ - - # calculate the local file path of the file - path = get_content_storage_file_path(filename) - - # if the file does not exist on disk, return a 404 - if not os.path.exists(path): - raise Http404( - '"%(filename)s" does not exist locally' % {"filename": filename} - ) - - # generate a file response - response = FileResponse(open(path, "rb")) - - # set the content-type by guessing from the filename - response["Content-Type"] = mimetypes.guess_type(filename)[0] - - # set the content-disposition as attachment to force download - response["Content-Disposition"] = "attachment;" - - # set the content-length to the file size - response["Content-Length"] = os.path.getsize(path) - - return response - - def get_by_node_id(node_id): """ Function to return a content node based on a node id diff --git a/kolibri/utils/options.py b/kolibri/utils/options.py index bc1ddb6e9f1..d1ac6622879 100644 --- a/kolibri/utils/options.py +++ b/kolibri/utils/options.py @@ -449,6 +449,15 @@ def url_prefix(value): retrieving alternate origin URLs. """, }, + "REMOTE_CONTENT": { + "type": "boolean", + "default": False, + "description": """ + Boolean flag that causes content import processes to skip trying to import any + content, as it is assumed that the remote source has everything available. + Server configuration should handle ensuring that the files are properly served. + """, + }, }, "Python": { "PICKLE_PROTOCOL": {