From 49f427849bd73672845d039ca0090d321617b0e4 Mon Sep 17 00:00:00 2001 From: JensDiemer Date: Sun, 6 Dec 2020 15:01:17 +0100 Subject: [PATCH] Use "serve_media_app" to serve users uploads see: https://github.com/jedie/django-tools/tree/master/django_tools/serve_media_app --- README.creole | 1 + README.rst | 4 +- poetry.lock | 15 ++-- pyproject.toml | 2 +- .../0005_serve_uploads_by_django_tools.py | 74 +++++++++++++++++++ src/inventory/models/item.py | 13 +--- src/inventory/tests/test_item_images.py | 69 +++++++++-------- src/inventory/views/__init__.py | 0 src/inventory/views/media_files.py | 47 ------------ src/inventory_project/settings/base.py | 10 ++- src/inventory_project/urls.py | 4 +- 11 files changed, 130 insertions(+), 109 deletions(-) create mode 100644 src/inventory/migrations/0005_serve_uploads_by_django_tools.py delete mode 100644 src/inventory/views/__init__.py delete mode 100644 src/inventory/views/media_files.py diff --git a/README.creole b/README.creole index b71bc1c..4d8f3d1 100644 --- a/README.creole +++ b/README.creole @@ -187,6 +187,7 @@ Files are separated into: "/src/" and "/development/" == history * [[https://github.com/jedie/PyInventory/compare/v0.7.0...master|compare v0.7.0...master]] **dev** +** Outsource the "MEDIA file serve" part into [[https://github.com/jedie/django-tools/tree/master/django_tools/serve_media_app#readme|django.tools.serve_media_app]] ** tbc * [[https://github.com/jedie/PyInventory/compare/v0.6.0...v0.7.0|v0.7.0 - 23.11.2020]] ** Change deployment setup: diff --git a/README.rst b/README.rst index 896fd2b..45020ea 100644 --- a/README.rst +++ b/README.rst @@ -254,6 +254,8 @@ history * `compare v0.7.0...master `_ **dev** + * Outsource the "MEDIA file serve" part into `django.tools.serve_media_app `_ + * tbc * `v0.7.0 - 23.11.2020 `_ @@ -375,4 +377,4 @@ donation ------------ -``Note: this file is generated from README.creole 2020-11-23 17:53:10 with "python-creole"`` \ No newline at end of file +``Note: this file is generated from README.creole 2020-12-06 19:12:25 with "python-creole"`` \ No newline at end of file diff --git a/poetry.lock b/poetry.lock index 6b299e8..09354c1 100644 --- a/poetry.lock +++ b/poetry.lock @@ -361,14 +361,14 @@ django-reversion = "*" [[package]] name = "django-tagulous" -version = "1.0.0" +version = "1.1.0" description = "Fabulous Tagging for Django" category = "main" optional = false python-versions = "*" [package.dependencies] -Django = ">=1.11" +Django = ">=2.2" [package.extras] dev = ["tox", "jasmine"] @@ -377,7 +377,7 @@ i18n = ["unidecode"] [[package]] name = "django-tools" -version = "0.48.0" +version = "0.48.2" description = "miscellaneous tools for django" category = "main" optional = false @@ -1271,7 +1271,7 @@ postgres = ["psycopg2-binary"] [metadata] lock-version = "1.1" python-versions = ">=3.7,<4.0.0" -content-hash = "bb178e486925b95face37987e21f66c19a2e82194a0bc508d1c69eda4f105ce3" +content-hash = "53350bdeb15638ddf111098e991f6ef65c9426369c0c29385fa6bbd303b6d1ec" [metadata.files] appdirs = [ @@ -1494,11 +1494,12 @@ django-reversion-compare = [ {file = "django_reversion_compare-0.12.2-py3-none-any.whl", hash = "sha256:5ce8d402add477a3c38aae8335af22b3abdfffa83ef5333c06c865abb89e9cbd"}, ] django-tagulous = [ - {file = "django-tagulous-1.0.0.tar.gz", hash = "sha256:9b4fa1773845a1cf33d21b27f9cdafc6f3fe29a480428bdd8f8717e7d4742396"}, + {file = "django-tagulous-1.1.0.tar.gz", hash = "sha256:9bc9d1d066c486fac1a3ec351531e440bc239c459b043e9180d99d7846e45fd6"}, + {file = "django_tagulous-1.1.0-py3-none-any.whl", hash = "sha256:de2a56ed92374b79358275ac0b7910af2c3d2823f44a847bef91ca9e456353ba"}, ] django-tools = [ - {file = "django-tools-0.48.0.tar.gz", hash = "sha256:637e0137d232abaca9f5e1af44e63b299d1e561bd7881d791ce854cfc0e74031"}, - {file = "django_tools-0.48.0-py3-none-any.whl", hash = "sha256:26556cb0f03ea34d7c3a48a0ff69858f4e6600ea0c886652893d1d102cb8c5e2"}, + {file = "django-tools-0.48.2.tar.gz", hash = "sha256:76965bb71f70965fb7b56152836e76116c02e74b81635cf0eda2819d4ad594e9"}, + {file = "django_tools-0.48.2-py3-none-any.whl", hash = "sha256:0d4d141a5f20df79139c17279a95a022aad4f2fa4eb8236d8ab61c358c7206ef"}, ] docker = [ {file = "docker-4.4.0-py2.py3-none-any.whl", hash = "sha256:317e95a48c32de8c1aac92a48066a5b73e218ed096e03758bcdd799a7130a1a1"}, diff --git a/pyproject.toml b/pyproject.toml index 2366f22..623ad65 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -44,7 +44,7 @@ django-processinfo = "*" # https://github.com/jedie/django-processinfo/ django-debug-toolbar = "*" # http://django-debug-toolbar.readthedocs.io/en/stable/changes.html django-import-export = "*" # https://github.com/django-import-export/django-import-export django-dbbackup = "*" # https://github.com/django-dbbackup/django-dbbackup -django-tools = "*" # https://github.com/jedie/django-tools/ +django-tools = ">=0.48.2" # https://github.com/jedie/django-tools/ django-reversion-compare = "*" # https://github.com/jedie/django-reversion-compare/ django-ckeditor = "*" # https://github.com/django-ckeditor/django-ckeditor diff --git a/src/inventory/migrations/0005_serve_uploads_by_django_tools.py b/src/inventory/migrations/0005_serve_uploads_by_django_tools.py new file mode 100644 index 0000000..0f4c1d5 --- /dev/null +++ b/src/inventory/migrations/0005_serve_uploads_by_django_tools.py @@ -0,0 +1,74 @@ +# Generated by Django 2.2.17 on 2020-12-06 14:07 +import os +from pathlib import Path + +from django.conf import settings +from django.db import migrations +from django.utils import timezone + + +class Tee: + def __init__(self, f): + self.f = f + + def __enter__(self): + return self + + def __call__(self, line): + if not isinstance(line, str): + line = str(line) + print(line) + self.f.write(line) + self.f.write('\n') + + def __exit__(self, exc_type, exc_val, exc_tb): + pass + + +def forward_code(apps, schema_editor): + print() + log_file_path = Path(settings.MEDIA_ROOT, 'migrate.log') + print('Generate log file here:', log_file_path) + with log_file_path.open('w+') as log, Tee(log) as log: + log('-' * 100) + log(timezone.now()) + + from django_tools.serve_media_app.models import generate_media_path + ItemImageModel = apps.get_model('inventory', 'ItemImageModel') + + qs = ItemImageModel.objects.all() + + for instance in qs: + log('_' * 100) + log(f'Migrate {instance}') + user = instance.user + image = instance.image + + file_path = Path(str(image.file)) + log(f'Old path: {file_path}') + + media_path=generate_media_path(user, filename=file_path.name) + + new_file_path = Path(settings.MEDIA_ROOT, media_path) + log(f'New path: {new_file_path}') + + os.makedirs(new_file_path.parent, exist_ok=True) + os.link(file_path, new_file_path) + + instance.image = media_path + instance.save(update_fields=('image',)) + + log('All new path created via hardlinks!') + log('Old path can be deleted.') + + +class Migration(migrations.Migration): + + dependencies = [ + ('inventory', '0004_item_user_images'), + ('serve_media_app', '0001_initial'), + ] + + operations = [ + migrations.RunPython(forward_code, reverse_code=migrations.RunPython.noop), + ] diff --git a/src/inventory/models/item.py b/src/inventory/models/item.py index fca0df7..65a37c3 100644 --- a/src/inventory/models/item.py +++ b/src/inventory/models/item.py @@ -6,8 +6,8 @@ from bx_py_utils.filename import clean_filename from ckeditor_uploader.fields import RichTextUploadingField from django.db import models from django.urls import reverse -from django.utils.crypto import get_random_string from django.utils.translation import ugettext_lazy as _ +from django_tools.serve_media_app.models import user_directory_path from inventory.models.base import BaseModel from inventory.models.links import BaseLink @@ -177,17 +177,6 @@ class ItemLinkModel(BaseLink): ordering = ('position',) -def user_directory_path(instance, filename): - """ - Upload to /MEDIA_ROOT/... - """ - random_string = get_random_string() - filename = clean_filename(filename) - filename = f'user_{instance.user.id}/{random_string}/{filename}' - logger.info(f'Upload filename: {filename!r}') - return filename - - class ItemImageModel(BaseModel): """ Store Images to Items diff --git a/src/inventory/tests/test_item_images.py b/src/inventory/tests/test_item_images.py index 7fd386b..10e575c 100644 --- a/src/inventory/tests/test_item_images.py +++ b/src/inventory/tests/test_item_images.py @@ -3,6 +3,7 @@ from unittest import mock from django.http import FileResponse from django.test import TestCase, override_settings +from django_tools.serve_media_app.models import UserMediaTokenModel from model_bakery import baker from inventory.models import ItemImageModel @@ -11,53 +12,51 @@ from inventory.tests.fixtures.users import get_normal_pyinventory_user class ItemImagesTestCase(TestCase): def test_basics(self): - pyinventory_user1 = get_normal_pyinventory_user(id=1) - pyinventory_user2 = get_normal_pyinventory_user(id=2) + with mock.patch('secrets.token_urlsafe', return_value='user1token'): + pyinventory_user1 = get_normal_pyinventory_user(id=1) - with tempfile.TemporaryDirectory() as tmpdir, override_settings(MEDIA_ROOT=tmpdir): - print(tmpdir) + with mock.patch('secrets.token_urlsafe', return_value='user2token'): + pyinventory_user2 = get_normal_pyinventory_user(id=2) - with self.assertLogs('inventory') as logs: - with mock.patch('inventory.models.item.get_random_string', return_value='DrgCCsMrdIBJ'): + token1_instance = UserMediaTokenModel.objects.get(user=pyinventory_user1) + assert repr(token1_instance) == ( + f"" + ) + token2_instance = UserMediaTokenModel.objects.get(user=pyinventory_user2) + assert repr(token2_instance) == ( + f"" + ) + + with tempfile.TemporaryDirectory() as temp: + with override_settings(MEDIA_ROOT=temp): + with mock.patch('secrets.token_urlsafe', return_value='12345678901234567890'): image_instance = baker.make( ItemImageModel, user=pyinventory_user1, _create_files=True ) + assert image_instance.image is not None url = image_instance.image.url - # url = f'/media/{image_instance.image}' - assert url == '/media/user_1/DrgCCsMrdIBJ/mock_img.jpeg' - assert logs.output == [ - "INFO:inventory.models.item:" - "Upload filename: 'user_1/DrgCCsMrdIBJ/mock_img.jpeg'" - ] + assert url == '/media/user1token/12345678901234567890/mock_img.jpeg' - # Anonymous user can't access: - - with self.assertLogs('inventory') as logs, self.assertLogs('django'): - response = self.client.get(url) + # Anonymous has no access: + response = self.client.get('/media/user1token/12345678901234567890/mock_img.jpeg') assert response.status_code == 403 - assert logs.output == [ - 'ERROR:inventory.views.media_files:Anonymous try to access files from: 1' - ] - # Wrong user should not access: - - self.client.force_login(user=pyinventory_user2) - - with self.assertLogs('inventory') as logs, self.assertLogs('django'): - response = self.client.get(url) + # Can't access with wrong user: + self.client.force_login(pyinventory_user2) + response = self.client.get('/media/user1token/12345678901234567890/mock_img.jpeg') assert response.status_code == 403 - assert logs.output == [ - 'ERROR:inventory.views.media_files:Wrong user ID: 2 is not 1' - ] - # The right user should access: + # Can access with the right user: + self.client.force_login(pyinventory_user1) + response = self.client.get('/media/user1token/12345678901234567890/mock_img.jpeg') + assert response.status_code == 200 + assert isinstance(response, FileResponse) + assert response.getvalue() == image_instance.image.open('rb').read() - self.client.force_login(user=pyinventory_user1) - - response = self.client.get(url) - assert response.status_code == 200 - assert isinstance(response, FileResponse) - assert response.getvalue() == image_instance.image.read() + # Test whats happen, if token was deleted + UserMediaTokenModel.objects.all().delete() + response = self.client.get('/media/user1token/12345678901234567890/mock_img.jpeg') + assert response.status_code == 400 # SuspiciousOperation -> HttpResponseBadRequest diff --git a/src/inventory/views/__init__.py b/src/inventory/views/__init__.py deleted file mode 100644 index e69de29..0000000 diff --git a/src/inventory/views/media_files.py b/src/inventory/views/media_files.py deleted file mode 100644 index 875e4bf..0000000 --- a/src/inventory/views/media_files.py +++ /dev/null @@ -1,47 +0,0 @@ -import logging - -from django.conf import settings -from django.core.exceptions import PermissionDenied -from django.http import Http404 -from django.utils.translation import gettext as _ -from django.views.generic.base import View -from django.views.static import serve - -from inventory.models import ItemImageModel - - -logger = logging.getLogger(__name__) - - -class UserMediaView(View): - """ - Serve MEDIA_URL files, but check the current user: - """ - - def get(self, request, user_id, path): - media_path = f'user_{user_id}/{path}' - - if not request.user.is_superuser: - if request.user.id != user_id: - # A user tries to access a file from a other use? - if request.user.id is None: - logger.error(f'Anonymous try to access files from: {user_id!r}') - else: - logger.error(f'Wrong user ID: {request.user.id!r} is not {user_id!r}') - raise PermissionDenied - - # Check if the image really exists: - qs = ItemImageModel.objects.filter( - user_id=request.user.id, - image=media_path - ) - if not qs.exists(): - raise Http404(_('Image "%(path)s" does not exist') % {'path': media_path}) - - # Send the file to the user: - return serve( - request, - path=media_path, - document_root=settings.MEDIA_ROOT, - show_indexes=False - ) diff --git a/src/inventory_project/settings/base.py b/src/inventory_project/settings/base.py index 54ee983..00051bf 100644 --- a/src/inventory_project/settings/base.py +++ b/src/inventory_project/settings/base.py @@ -1,6 +1,6 @@ -""" +''' Base Django settings -""" +''' import logging from pathlib import Path as __Path @@ -76,6 +76,9 @@ INSTALLED_APPS = [ 'axes', # https://github.com/jazzband/django-axes 'django_processinfo', # https://github.com/jedie/django-processinfo/ + # https://github.com/jedie/django-tools/tree/master/django_tools/serve_media_app + 'django_tools.serve_media_app.apps.UserMediaFilesConfig', + 'inventory.apps.InventoryConfig', ] @@ -109,7 +112,7 @@ MIDDLEWARE = [ TEMPLATES = [ { 'BACKEND': 'django.template.backends.django.DjangoTemplates', - "DIRS": [str(__Path(PROJECT_PATH, 'inventory_project', 'templates'))], + 'DIRS': [str(__Path(PROJECT_PATH, 'inventory_project', 'templates'))], 'APP_DIRS': True, 'OPTIONS': { 'context_processors': [ @@ -342,6 +345,7 @@ LOGGING = { '': {'handlers': ['console'], 'level': 'DEBUG', 'propagate': False}, 'django': {'handlers': ['console'], 'level': 'INFO', 'propagate': False}, 'axes': {'handlers': ['console'], 'level': 'WARNING', 'propagate': False}, + 'django_tools': {'handlers': ['console'], 'level': 'INFO', 'propagate': False}, 'inventory': {'handlers': ['console'], 'level': 'DEBUG', 'propagate': False}, }, } diff --git a/src/inventory_project/urls.py b/src/inventory_project/urls.py index 064b2fb..9c8d8bd 100644 --- a/src/inventory_project/urls.py +++ b/src/inventory_project/urls.py @@ -4,8 +4,6 @@ from django.contrib import admin from django.urls import path from django.views.generic import RedirectView -from inventory.views.media_files import UserMediaView - admin.autodiscover() @@ -15,7 +13,7 @@ urlpatterns = [ # Don't use i18n_patterns() here url(r'^$', RedirectView.as_view(url='/admin/')), path('ckeditor/', include('ckeditor_uploader.urls')), # TODO: check permissions? - path('media/user_/', UserMediaView.as_view()) + path(settings.MEDIA_URL.lstrip('/'), include('django_tools.serve_media_app.urls')), ]