From 3cff1f406cca023f1d1114f0c5f890dbc591160d Mon Sep 17 00:00:00 2001 From: J-E Castagnede Date: Wed, 3 Apr 2024 11:57:14 +0200 Subject: [PATCH 01/12] use storage and don't explicit use another MEDIA_ROOT in tests --- mapentity/helpers.py | 14 ++++----- mapentity/models.py | 16 +++++------ mapentity/templatetags/mapentity_tags.py | 16 +++++------ mapentity/tests/__init__.py | 29 ++++++------------- mapentity/views/generic.py | 25 ++++++++-------- test_app/tests/test_templatetags.py | 36 +++++++++++------------- test_app/tests/test_views.py | 25 +++++----------- 7 files changed, 67 insertions(+), 94 deletions(-) diff --git a/mapentity/helpers.py b/mapentity/helpers.py index ddb07a98b..2fbaf59d7 100644 --- a/mapentity/helpers.py +++ b/mapentity/helpers.py @@ -1,16 +1,15 @@ import json import logging import math -import os import string import time -from datetime import datetime from mimetypes import types_map from urllib.parse import urljoin, quote import bs4 import requests from django.conf import settings +from django.core.files.storage import default_storage from django.contrib.gis.gdal.error import GDALException from django.contrib.gis.geos import GEOSException, fromstr from django.core.files.uploadedfile import SimpleUploadedFile @@ -18,8 +17,8 @@ from django.template.exceptions import TemplateDoesNotExist from django.template.loader import get_template from django.urls import resolve -from django.utils import timezone from django.utils.translation import get_language + from .settings import app_settings, API_SRID from .tokens import TokenManager @@ -66,19 +65,18 @@ def smart_urljoin(base, path): def is_file_uptodate(path, date_update, delete_empty=True): - if not os.path.exists(path): + if not default_storage.exists(path): return False if date_update is None: return False - if os.path.getsize(path) == 0: + if default_storage.size(path) == 0: if delete_empty: - os.remove(path) + default_storage.delete(path) return False - modified = datetime.utcfromtimestamp(os.path.getmtime(path)) - modified = modified.replace(tzinfo=timezone.utc) + modified = default_storage.get_modified_time(path) return modified > date_update diff --git a/mapentity/models.py b/mapentity/models.py index 324aad629..31863e106 100755 --- a/mapentity/models.py +++ b/mapentity/models.py @@ -9,6 +9,7 @@ from django.contrib.contenttypes.fields import GenericRelation from django.contrib.contenttypes.models import ContentType from django.core.exceptions import FieldError, ObjectDoesNotExist +from django.core.files.storage import default_storage from django.db import models, transaction from django.db.utils import OperationalError from django.urls import reverse, NoReverseMatch @@ -203,9 +204,7 @@ def get_geom(self): def delete(self, *args, **kwargs): # Delete map image capture when delete object - image_path = self.get_map_image_path() - if os.path.exists(image_path): - os.unlink(image_path) + default_storage.delete(self.get_map_image_path()) super().delete(*args, **kwargs) @classmethod @@ -299,14 +298,15 @@ def prepare_map_image(self, rooturl): else: size = app_settings['MAP_CAPTURE_SIZE'] printcontext = self.get_printcontext() if hasattr(self, 'get_printcontext') else None - capture_map_image(url, path, size=size, waitfor=self.capture_map_image_waitfor, printcontext=printcontext) + capture_map_image(url, + default_storage.path(path), + size=size, + waitfor=self.capture_map_image_waitfor, + printcontext=printcontext) return True def get_map_image_path(self): - basefolder = os.path.join(settings.MEDIA_ROOT, 'maps') - if not os.path.exists(basefolder): - os.makedirs(basefolder) - return os.path.join(basefolder, '%s-%s.png' % (self._meta.model_name, self.pk)) + return os.path.join('maps', '%s-%s.png' % (self._meta.model_name, self.pk)) def get_attributes_html(self, request): return extract_attributes_html(self.get_detail_url(), request) diff --git a/mapentity/templatetags/mapentity_tags.py b/mapentity/templatetags/mapentity_tags.py index 471cd71bd..05a8cc690 100644 --- a/mapentity/templatetags/mapentity_tags.py +++ b/mapentity/templatetags/mapentity_tags.py @@ -1,9 +1,9 @@ -import os - from django import template from django.conf import settings from django.contrib.gis.geos import GEOSGeometry +from django.contrib.staticfiles.storage import staticfiles_storage from django.core.exceptions import FieldDoesNotExist +from django.core.files.storage import default_storage from django.template import Context from django.template.exceptions import TemplateDoesNotExist from django.utils.timezone import now @@ -75,16 +75,16 @@ def field_verbose_name(obj, field): @register.simple_tag() def media_static_fallback(media_file, static_file, *args, **kwarg): - if os.path.exists(os.path.join(settings.MEDIA_ROOT, media_file)): - return os.path.join(settings.MEDIA_URL, media_file) - return os.path.join(settings.STATIC_URL, static_file) + if default_storage.exists(media_file): + return default_storage.url(media_file) + return staticfiles_storage.url(static_file) @register.simple_tag() def media_static_fallback_path(media_file, static_file, *args, **kwarg): - if os.path.exists(os.path.join(settings.MEDIA_ROOT, media_file)): - return os.path.join(settings.MEDIA_ROOT, media_file) - return os.path.join(settings.STATIC_ROOT, static_file) + if default_storage.exists(media_file): + return default_storage.path(media_file) + return staticfiles_storage.path(static_file) @register.filter(name='timesince') diff --git a/mapentity/tests/__init__.py b/mapentity/tests/__init__.py index 4a842a10b..e195b7ef9 100644 --- a/mapentity/tests/__init__.py +++ b/mapentity/tests/__init__.py @@ -1,23 +1,19 @@ import csv import hashlib import logging -import os -import shutil import time from datetime import datetime from io import StringIO -from tempfile import TemporaryDirectory from unittest.mock import patch from urllib.parse import quote from bs4 import BeautifulSoup -from django.conf import settings from django.contrib import messages from django.contrib.auth.models import Permission from django.contrib.contenttypes.models import ContentType -from django.test import TestCase, LiveServerTestCase +from django.core.files.storage import default_storage +from django.test import LiveServerTestCase, TestCase from django.test.testcases import to_list -from django.test.utils import override_settings from django.urls import reverse from django.utils import html from django.utils.encoding import force_str @@ -25,14 +21,13 @@ from django.utils.timezone import utc from django.utils.translation import gettext_lazy as _ from freezegun import freeze_time +from paperclip.settings import get_attachment_model -from .factories import AttachmentFactory, SuperUserFactory, UserFactory from ..forms import MapEntityForm from ..helpers import smart_urljoin from ..models import ENTITY_MARKUP from ..settings import app_settings - -from paperclip.settings import get_attachment_model +from .factories import AttachmentFactory, SuperUserFactory, UserFactory class AdjustDebugLevel: @@ -48,7 +43,6 @@ def __exit__(self, exc_type, exc_value, traceback): self.logger.setLevel(self.old_level) -@override_settings(MEDIA_ROOT=TemporaryDirectory().name) class MapEntityTest(TestCase): model = None modelfactory = None @@ -66,15 +60,9 @@ def get_expected_datatables_attrs(self): return {} def setUp(self): - if os.path.exists(settings.MEDIA_ROOT): - self.tearDown() - os.makedirs(settings.MEDIA_ROOT) if self.user: self.client.force_login(user=self.user) - def tearDown(self): - shutil.rmtree(settings.MEDIA_ROOT) - @classmethod def setUpTestData(cls): if cls.userfactory: @@ -426,7 +414,6 @@ def test_api_geojson_list_for_model(self): }) -@override_settings(MEDIA_ROOT='/tmp/mapentity-media') class MapEntityLiveTest(LiveServerTestCase): model = None userfactory = None @@ -527,9 +514,9 @@ def test_map_image(self, mock_requests): # Initially, map image does not exists image_path = obj.get_map_image_path() - if os.path.exists(image_path): - os.remove(image_path) - self.assertFalse(os.path.exists(image_path)) + if default_storage.exists(image_path): + default_storage.delete(image_path) + self.assertFalse(default_storage.exists(image_path)) # Mock Screenshot response mock_requests.get.return_value.status_code = 200 @@ -537,7 +524,7 @@ def test_map_image(self, mock_requests): response = self.client.get(obj.map_image_url) self.assertEqual(response.status_code, 200) - self.assertTrue(os.path.exists(image_path)) + self.assertTrue(default_storage.exists(image_path)) mapimage_url = '%s%s?context' % (self.live_server_url, obj.get_detail_url()) screenshot_url = 'http://0.0.0.0:8001/?url=%s' % quote(mapimage_url) diff --git a/mapentity/views/generic.py b/mapentity/views/generic.py index 1ed6f5af8..929461698 100755 --- a/mapentity/views/generic.py +++ b/mapentity/views/generic.py @@ -7,7 +7,9 @@ from django.conf import settings from django.contrib import messages from django.contrib.auth.decorators import login_required +from django.contrib.staticfiles.storage import staticfiles_storage from django.core.exceptions import PermissionDenied +from django.core.files.storage import default_storage from django.http import HttpResponse, HttpResponseBadRequest, HttpResponseRedirect from django.template.defaultfilters import slugify from django.template.exceptions import TemplateDoesNotExist @@ -196,13 +198,14 @@ def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) context['datetime'] = datetime.now() context['objecticon'] = os.path.join(settings.STATIC_ROOT, self.get_entity().icon_big) - context['logo_path'] = os.path.join(settings.MEDIA_ROOT, 'upload/logo-header.png') - if not os.path.exists(context['logo_path']): - context['logo_path'] = os.path.join(settings.STATIC_ROOT, 'images/logo-header.png') - context['STATIC_URL'] = self.request.build_absolute_uri(settings.STATIC_URL) - context['STATIC_ROOT'] = settings.STATIC_ROOT - context['MEDIA_URL'] = self.request.build_absolute_uri(settings.MEDIA_URL) - context['MEDIA_ROOT'] = settings.MEDIA_ROOT + if default_storage.exists('upload/logo-header.png'): + context['logo_path'] = default_storage.path('upload/logo-header.png') + else: + context['logo_path'] = staticfiles_storage.path('upload/logo-header.png') + context['STATIC_URL'] = staticfiles_storage.base_url + context['STATIC_ROOT'] = staticfiles_storage.location + context['MEDIA_URL'] = default_storage.base_url + context['MEDIA_ROOT'] = default_storage.location return context @@ -240,10 +243,10 @@ def get_entity_kind(cls): def patch_static_file_paths(self, content): """ Patch weasyprint renderer content to switch file://xxx paths to html scheme """ - file_paths_media = f"file://{settings.MEDIA_ROOT}" - new_content = content.replace(file_paths_media, settings.MEDIA_URL) - file_paths_static = f"file://{settings.STATIC_ROOT}" - return new_content.replace(file_paths_static, settings.STATIC_URL) + file_paths_media = f"file://{default_storage.location}" + new_content = content.replace(file_paths_media, default_storage.base_url) + file_paths_static = f"file://{staticfiles_storage.location}" + return new_content.replace(file_paths_static, staticfiles_storage.base_url) def render_to_response(self, context, **response_kwargs): response = super().render_to_response(context, **response_kwargs) diff --git a/test_app/tests/test_templatetags.py b/test_app/tests/test_templatetags.py index 80ef24a72..4f5eca196 100644 --- a/test_app/tests/test_templatetags.py +++ b/test_app/tests/test_templatetags.py @@ -245,16 +245,14 @@ def test_media_static_fallback_path(self): self.assertEqual(os.path.join(d.name, 'foo.png'), out) def test_media_static_find_path(self): - d = TemporaryDirectory() - with override_settings(MEDIA_ROOT=d.name): - with open(os.path.join(settings.MEDIA_ROOT, 'exist.png'), mode='wb') as f: - f.write(b'iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/' - b'w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg==') - out = Template( - '{% load mapentity_tags %}' - '{% media_static_fallback_path "exist.png" "foo.png" %}' - ).render(Context({})) - self.assertEqual(out, f.name) + with open(os.path.join(settings.MEDIA_ROOT, 'exist.png'), mode='wb') as f: + f.write(b'iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/' + b'w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg==') + out = Template( + '{% load mapentity_tags %}' + '{% media_static_fallback_path "exist.png" "foo.png" %}' + ).render(Context({})) + self.assertEqual(out, f.name) class MediaStaticFallbackTest(TestCase): @@ -267,16 +265,14 @@ def test_media_static_fallback(self): self.assertEqual(out, "/static/foo.png") def test_media_static_find(self): - d = TemporaryDirectory() - with override_settings(MEDIA_ROOT=d.name): - with open(os.path.join(settings.MEDIA_ROOT, 'exist.png'), mode='wb') as f: - f.write(b'iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/' - b'w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg==') - out = Template( - '{% load mapentity_tags %}' - '{% media_static_fallback "exist.png" "foo.png" %}' - ).render(Context({})) - self.assertEqual(out, '/media/exist.png') + with open(os.path.join(settings.MEDIA_ROOT, 'exist.png'), mode='wb') as f: + f.write(b'iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/' + b'w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg==') + out = Template( + '{% load mapentity_tags %}' + '{% media_static_fallback "exist.png" "foo.png" %}' + ).render(Context({})) + self.assertEqual(out, '/media/exist.png') class SmartIncludeTest(TestCase): diff --git a/test_app/tests/test_views.py b/test_app/tests/test_views.py index c969d33a5..0daba9cbb 100644 --- a/test_app/tests/test_views.py +++ b/test_app/tests/test_views.py @@ -1,15 +1,13 @@ import os -import shutil from unittest import mock import django import factory -from django.conf import settings from django.contrib.auth import get_user_model from django.contrib.auth.models import Permission from django.core.files.uploadedfile import SimpleUploadedFile from django.core.management import call_command -from django.test import TestCase, RequestFactory +from django.test import RequestFactory, TestCase from django.test.utils import override_settings from django.utils.encoding import force_str from faker import Faker @@ -18,12 +16,13 @@ from mapentity.models import LogEntry from mapentity.registry import app_settings -from mapentity.tests import MapEntityTest, MapEntityLiveTest -from mapentity.tests.factories import SuperUserFactory, AttachmentFactory -from mapentity.views import ServeAttachment, Convert, JSSettings -from .factories import DummyModelFactory +from mapentity.tests import MapEntityLiveTest, MapEntityTest +from mapentity.tests.factories import AttachmentFactory, SuperUserFactory +from mapentity.views import Convert, JSSettings, ServeAttachment + from ..models import DummyModel, FileType -from ..views import DummyList, DummyDetail +from ..views import DummyDetail, DummyList +from .factories import DummyModelFactory fake = Faker('fr_FR') fake.add_provider(geo) @@ -161,25 +160,15 @@ def test_convert_view_builds_absolute_url_from_relative(self, token_mocked, get_ headers={}) -@override_settings(MEDIA_ROOT='/tmp/mapentity-media') class AttachmentTest(BaseTest): def setUp(self): app_settings['SENDFILE_HTTP_HEADER'] = 'X-Accel-Redirect' self.obj = DummyModelFactory.create() - """ - if os.path.exists(settings.MEDIA_ROOT): - self.tearDown() - os.makedirs(os.path.join(settings.MEDIA_ROOT, 'paperclip/test_app_dummymodel/{}'.format(self.obj.pk))) - self.file = os.path.join(settings.MEDIA_ROOT, 'paperclip/test_app_dummymodel/{}/file.pdf'.format(self.obj.pk)) - self.url = '/media/paperclip/test_app_dummymodel/{}/file.pdf'.format(self.obj.pk) - open(self.file, 'wb').write(b'*' * 300) - """ self.attachment = AttachmentFactory.create(content_object=self.obj) self.url = "/media/%s" % self.attachment.attachment_file call_command('update_permissions_mapentity') def tearDown(self): - shutil.rmtree(settings.MEDIA_ROOT) app_settings['SENDFILE_HTTP_HEADER'] = None def download(self, url): From c1e84f38cc4b667da576f9cd8a54e29507fd441f Mon Sep 17 00:00:00 2001 From: J-E Castagnede Date: Wed, 3 Apr 2024 15:45:40 +0200 Subject: [PATCH 02/12] use storage to handle map capture --- mapentity/helpers.py | 55 +++++++++------------------------- mapentity/views/base.py | 11 ++++--- mapentity/views/generic.py | 11 +++---- test_app/tests/test_helpers.py | 14 ++++----- 4 files changed, 32 insertions(+), 59 deletions(-) diff --git a/mapentity/helpers.py b/mapentity/helpers.py index 2fbaf59d7..3197a215c 100644 --- a/mapentity/helpers.py +++ b/mapentity/helpers.py @@ -4,22 +4,22 @@ import string import time from mimetypes import types_map -from urllib.parse import urljoin, quote +from urllib.parse import quote, urljoin import bs4 import requests from django.conf import settings -from django.core.files.storage import default_storage from django.contrib.gis.gdal.error import GDALException from django.contrib.gis.geos import GEOSException, fromstr +from django.core.files.base import ContentFile +from django.core.files.storage import default_storage from django.core.files.uploadedfile import SimpleUploadedFile -from django.http import HttpResponse from django.template.exceptions import TemplateDoesNotExist from django.template.loader import get_template from django.urls import resolve from django.utils.translation import get_language -from .settings import app_settings, API_SRID +from .settings import API_SRID, app_settings from .tokens import TokenManager logger = logging.getLogger(__name__) @@ -89,12 +89,11 @@ def get_source(url, headers): content_error = 'Request on %s returned empty content' % url assert len(source.content) > 0, content_error - return source + return source.content -def download_to_stream(url, stream, silent=False, headers=None): - """ Download url and writes response to stream. - """ +def download_content(url, silent=False, headers=None): + """ Download URL and return content.""" source = None try: try: @@ -109,24 +108,7 @@ def download_to_stream(url, stream, silent=False, headers=None): logger.info('Response: %s' % source.text[:150]) if not silent: - raise - - if source is None: - return source - - try: - stream.write(source.content) - stream.flush() - except IOError as e: - logger.exception(e) - if not silent: - raise - - if isinstance(stream, HttpResponse): - stream.status_code = source.status_code - # Copy headers - for header, value in source.headers.items(): - stream[header] = value + raise e return source @@ -148,16 +130,9 @@ def convertit_url(url, from_type=None, to_type=None, proxy=False): return url -def convertit_download(url, destination, from_type=None, to_type='application/pdf', headers=None): - # Mock for tests - if getattr(settings, 'TEST', False): - with open(destination, 'w') as out_file: - out_file.write("Mock\n") - return - +def convertit_download(url, from_type=None, to_type='application/pdf', headers=None): url = convertit_url(url, from_type, to_type) - fd = open(destination, 'wb') if isinstance(destination, str) else destination - download_to_stream(url, fd, headers=headers) + return download_content(url, headers=headers) def capture_url(url, width=None, height=None, selector=None, waitfor=None): @@ -178,10 +153,10 @@ def capture_url(url, width=None, height=None, selector=None, waitfor=None): return final_url -def capture_image(url, stream, **kwargs): +def capture_image(url, **kwargs): """ Capture url to stream. """ url = capture_url(url, **kwargs) - download_to_stream(url, stream) + return download_content(url) def capture_map_image(url, destination, size=None, aspect=1.0, waitfor='.leaflet-tile-loaded', printcontext=None): @@ -204,10 +179,8 @@ def capture_map_image(url, destination, size=None, aspect=1.0, waitfor='.leaflet # Run head-less capture (takes time) auth_token = TokenManager.generate_token() url += f"?lang={get_language()}&auth_token={auth_token}&context={quote(serialized)}" - with open(destination, 'wb') as fd: - capture_image(url, fd, - selector='.map-panel', - waitfor=waitfor) + map_image = capture_image(url, selector='.map-panel', waitfor=waitfor) + default_storage.save(destination, ContentFile(map_image)) def extract_attributes_html(url, request): diff --git a/mapentity/views/base.py b/mapentity/views/base.py index f42638ea1..6e5c0ad5e 100644 --- a/mapentity/views/base.py +++ b/mapentity/views/base.py @@ -3,8 +3,7 @@ import mimetypes import os import re -from datetime import datetime -from io import BytesIO + from urllib.parse import quote from django.conf import settings @@ -14,6 +13,7 @@ from django.http import (HttpResponse, HttpResponseBadRequest, Http404) from django.shortcuts import get_object_or_404 from django.urls import reverse +from django.utils import timezone from django.views import static, View from django.views.decorators.csrf import csrf_exempt from django.views.decorators.http import require_http_methods @@ -169,10 +169,9 @@ def map_screenshot(request): width = context.get('viewport', {}).get('width') height = context.get('viewport', {}).get('height') - stream = BytesIO() - capture_image(map_url, stream, width=width, height=height, selector=selector) - response = HttpResponse(stream.getvalue(), content_type='image/png') - response['Content-Disposition'] = 'attachment; filename=%s.png' % datetime.now().strftime('%Y%m%d-%H%M%S') + map_image = capture_image(map_url, width=width, height=height, selector=selector) + response = HttpResponse(map_image, content_type='image/png') + response['Content-Disposition'] = 'attachment; filename=%s.png' % timezone.now().isoformat() return response except Exception as exc: diff --git a/mapentity/views/generic.py b/mapentity/views/generic.py index 929461698..6b5b98fef 100755 --- a/mapentity/views/generic.py +++ b/mapentity/views/generic.py @@ -33,7 +33,7 @@ from .. import serializers as mapentity_serializers from ..decorators import save_history, view_permission_required from ..forms import AttachmentForm -from ..helpers import convertit_url, download_to_stream, user_has_perm +from ..helpers import convertit_url, download_content, user_has_perm from ..helpers import suffix_for, name_for, smart_get_template from ..models import LogEntry, ADDITION, CHANGE, DELETION from ..settings import app_settings @@ -322,11 +322,12 @@ def get(self, request, *args, **kwargs): url = convertit_url(source_url, from_type=fromtype, to_type=format) response = HttpResponse() - received = download_to_stream(url, response, - silent=True, - headers=self.request_headers()) + received = download_content(url, + silent=True, + headers=self.request_headers()) if received: - filename = os.path.basename(received.url) + response = HttpResponse(received) + filename = os.path.basename(url) response['Content-Disposition'] = 'attachment; filename=%s' % filename return response diff --git a/test_app/tests/test_helpers.py b/test_app/tests/test_helpers.py index a7dfe99a2..31f4ce75a 100644 --- a/test_app/tests/test_helpers.py +++ b/test_app/tests/test_helpers.py @@ -1,14 +1,14 @@ -import os from unittest import mock +from django.core.files.storage import default_storage from django.test import TestCase from mapentity.helpers import ( capture_map_image, capture_url, convertit_url, + download_content, user_has_perm, - download_to_stream ) from mapentity.registry import app_settings @@ -26,11 +26,11 @@ def test_capture_url_is_escaped(self): url = capture_url('http://geotrek.fr') self.assertIn('http%3A//geotrek.fr', url) - @mock.patch('mapentity.helpers.open') @mock.patch('mapentity.helpers.capture_image') - def test_capture_url_has_auth_token(self, mocked_capture, mocked_open): - capture_map_image('', '') - url, _ = mocked_capture.call_args[0] + def test_capture_url_has_auth_token(self, mocked_capture): + mocked_capture.return_value = b'*' + capture_map_image('', default_storage.get_available_name('test.png')) + url = mocked_capture.call_args[0][0] self.assertIn("auth_token", url) def test_capture_url_with_no_params(self): @@ -98,5 +98,5 @@ def test_headers_can_be_specified_for_download(self, get_mocked): # Required to specified language for example get_mocked.return_value.status_code = 200 get_mocked.return_value.content = "x" - download_to_stream('http://google.com', open(os.devnull, 'w'), silent=True, headers={'Accept-language': 'fr'}) + download_content('http://google.com', silent=True, headers={'Accept-language': 'fr'}) get_mocked.assert_called_with('http://google.com', headers={'Accept-language': 'fr'}) From 0c4e5c7976d72c6ba42fdeb33f35d5fae6a42528 Mon Sep 17 00:00:00 2001 From: J-E Castagnede Date: Wed, 3 Apr 2024 15:45:54 +0200 Subject: [PATCH 03/12] use temp dir for MEDIA in tests --- test_project/settings.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test_project/settings.py b/test_project/settings.py index ac79fdbf4..0ff4e232d 100644 --- a/test_project/settings.py +++ b/test_project/settings.py @@ -12,6 +12,9 @@ # Build paths inside the project like this: os.path.join(BASE_DIR, ...) import os +import sys +from tempfile import TemporaryDirectory + BASE_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) @@ -200,3 +203,6 @@ LOGOUT_REDIRECT_URL = '/login/' MODELTRANSLATION_LANGUAGES = ('en', 'fr', 'zh-hant') + +if 'test' in sys.argv: + MEDIA_ROOT = TemporaryDirectory().name From 3556de6ec2a8c3849500b4d34b5d35f8d70f9c91 Mon Sep 17 00:00:00 2001 From: J-E Castagnede Date: Thu, 4 Apr 2024 14:41:24 +0200 Subject: [PATCH 04/12] fix storage deletion --- mapentity/models.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mapentity/models.py b/mapentity/models.py index 31863e106..d4266f734 100755 --- a/mapentity/models.py +++ b/mapentity/models.py @@ -204,7 +204,9 @@ def get_geom(self): def delete(self, *args, **kwargs): # Delete map image capture when delete object - default_storage.delete(self.get_map_image_path()) + image_path = self.get_map_image_path() + if default_storage.exists(image_path): + default_storage.delete(image_path) super().delete(*args, **kwargs) @classmethod From d98dd11fd2662ce0c5241fc93dccae3eef806bf8 Mon Sep 17 00:00:00 2001 From: J-E Castagnede Date: Fri, 5 Apr 2024 14:51:58 +0200 Subject: [PATCH 05/12] improve internal user cache --- .../commands/update_permissions_mapentity.py | 4 -- mapentity/middleware.py | 40 ++++++++++--------- test_app/tests/test_middleware.py | 3 +- test_app/tests/test_permissions.py | 3 +- 4 files changed, 23 insertions(+), 27 deletions(-) diff --git a/mapentity/management/commands/update_permissions_mapentity.py b/mapentity/management/commands/update_permissions_mapentity.py index 5238faefb..f42f7ec30 100644 --- a/mapentity/management/commands/update_permissions_mapentity.py +++ b/mapentity/management/commands/update_permissions_mapentity.py @@ -8,7 +8,6 @@ from django.contrib.contenttypes.models import ContentType from django.core.management.base import BaseCommand -from mapentity.middleware import clear_internal_user_cache from mapentity.registry import create_mapentity_model_permissions, registry logger = logging.getLogger(__name__) @@ -30,9 +29,6 @@ def execute(self, *args, **options): # Make sure apps are registered at this point import_module(settings.ROOT_URLCONF) - # Tests reset DB so we have to reset this cache too - clear_internal_user_cache() - # For all models registered, add missing bits for model in registry.registry.keys(): if not model._meta.abstract: diff --git a/mapentity/middleware.py b/mapentity/middleware.py index e61ada7cc..4b19a4899 100644 --- a/mapentity/middleware.py +++ b/mapentity/middleware.py @@ -1,7 +1,8 @@ import logging +from hashlib import sha256 -from django.conf import settings from django.contrib.auth import get_user_model, login +from django.core.cache import cache from .settings import app_settings from .tokens import TokenManager @@ -10,24 +11,25 @@ def get_internal_user(): - if not hasattr(get_internal_user, 'instance'): - username = app_settings['INTERNAL_USER'] - User = get_user_model() - - internal_user, created = User.objects.get_or_create( - username=username, - defaults={'password': settings.SECRET_KEY, - 'is_active': True, - 'is_staff': False} - ) - - get_internal_user.instance = internal_user - return get_internal_user.instance - - -def clear_internal_user_cache(): - if hasattr(get_internal_user, 'instance'): - del get_internal_user.instance + User = get_user_model() + cache_key = sha256(app_settings['INTERNAL_USER'].encode()).hexdigest() + + id = 0 + + if cache_key in cache: + id = int(cache.get(cache_key)) + + internal_user, created = User.objects.get_or_create( + id=int(id), + defaults={ + 'username': app_settings['INTERNAL_USER'], + 'password': '', + 'is_active': True + } + ) + if created: + cache.set(cache_key, internal_user.pk) + return internal_user class AutoLoginMiddleware: diff --git a/test_app/tests/test_middleware.py b/test_app/tests/test_middleware.py index 6dbd9f7b9..8b7940882 100644 --- a/test_app/tests/test_middleware.py +++ b/test_app/tests/test_middleware.py @@ -2,7 +2,7 @@ from django.core.management import call_command from django.test import TestCase -from mapentity.middleware import get_internal_user, clear_internal_user_cache +from mapentity.middleware import get_internal_user from mapentity.tests.factories import SuperUserFactory from mapentity.tokens import TokenManager @@ -15,7 +15,6 @@ def setUpTestData(cls): cls.user = SuperUserFactory() def setUp(self): - clear_internal_user_cache() call_command('update_permissions_mapentity') def test_user_authenticated_no_token(self): diff --git a/test_app/tests/test_permissions.py b/test_app/tests/test_permissions.py index 3b777871a..911ee4a22 100644 --- a/test_app/tests/test_permissions.py +++ b/test_app/tests/test_permissions.py @@ -4,7 +4,7 @@ from django.test import TestCase from mapentity.helpers import user_has_perm -from mapentity.middleware import get_internal_user, clear_internal_user_cache +from mapentity.middleware import get_internal_user from mapentity.tests.factories import UserFactory from ..models import DummyModel @@ -43,7 +43,6 @@ def test_internal_user_permissions_work_as_others(self): class NavBarPermissionsTest(TestCase): def setUp(self): - clear_internal_user_cache() call_command('update_permissions_mapentity') def test_navbar_permissions(self): From 2830a3c7c717ed1120d9c606039df827e256241b Mon Sep 17 00:00:00 2001 From: J-E Castagnede Date: Fri, 5 Apr 2024 15:47:40 +0200 Subject: [PATCH 06/12] fix local map path --- mapentity/views/generic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mapentity/views/generic.py b/mapentity/views/generic.py index 6b5b98fef..a0538b79c 100755 --- a/mapentity/views/generic.py +++ b/mapentity/views/generic.py @@ -225,7 +225,7 @@ def __init__(self, *args, **kwargs): def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) - context['map_path'] = self.get_object().get_map_image_path() + context['map_path'] = default_storage.path(self.get_object().get_map_image_path()) context['template_attributes'] = self.template_attributes context['template_css'] = self.template_css return context From 58e99553b9f79712cf6038fb81c3e9337c919984 Mon Sep 17 00:00:00 2001 From: J-E Castagnede Date: Fri, 5 Apr 2024 15:57:23 +0200 Subject: [PATCH 07/12] fix local map path --- mapentity/helpers.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mapentity/helpers.py b/mapentity/helpers.py index 3197a215c..7dea0d19d 100644 --- a/mapentity/helpers.py +++ b/mapentity/helpers.py @@ -180,6 +180,8 @@ def capture_map_image(url, destination, size=None, aspect=1.0, waitfor='.leaflet auth_token = TokenManager.generate_token() url += f"?lang={get_language()}&auth_token={auth_token}&context={quote(serialized)}" map_image = capture_image(url, selector='.map-panel', waitfor=waitfor) + if default_storage.exists(destination): + default_storage.delete(destination) default_storage.save(destination, ContentFile(map_image)) From 74021d451f9f756dc941a64a53d6a532df65925c Mon Sep 17 00:00:00 2001 From: J-E Castagnede Date: Wed, 10 Apr 2024 09:34:43 +0200 Subject: [PATCH 08/12] fix local map path --- .../management/commands/update_permissions_mapentity.py | 5 +++-- mapentity/registry.py | 4 +--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/mapentity/management/commands/update_permissions_mapentity.py b/mapentity/management/commands/update_permissions_mapentity.py index f42f7ec30..8917779bd 100644 --- a/mapentity/management/commands/update_permissions_mapentity.py +++ b/mapentity/management/commands/update_permissions_mapentity.py @@ -8,6 +8,7 @@ from django.contrib.contenttypes.models import ContentType from django.core.management.base import BaseCommand +from mapentity.middleware import get_internal_user from mapentity.registry import create_mapentity_model_permissions, registry logger = logging.getLogger(__name__) @@ -28,11 +29,11 @@ def execute(self, *args, **options): # Make sure apps are registered at this point import_module(settings.ROOT_URLCONF) - + internal_user = get_internal_user() # For all models registered, add missing bits for model in registry.registry.keys(): if not model._meta.abstract: - create_mapentity_model_permissions(model) + create_mapentity_model_permissions(model, internal_user) logger.info("Done.") diff --git a/mapentity/registry.py b/mapentity/registry.py index 02eefd7b6..ea097e8cc 100644 --- a/mapentity/registry.py +++ b/mapentity/registry.py @@ -17,7 +17,6 @@ from rest_framework.serializers import ModelSerializer from mapentity import models as mapentity_models -from mapentity.middleware import get_internal_user from mapentity.serializers import MapentityGeojsonModelSerializer from mapentity.settings import app_settings @@ -232,7 +231,7 @@ def entities(self): registry = Registry() -def create_mapentity_model_permissions(model): +def create_mapentity_model_permissions(model, internal_user): """ Create all the necessary permissions for the specified model. @@ -254,7 +253,6 @@ def create_mapentity_model_permissions(model): db = DEFAULT_DB_ALIAS - internal_user = get_internal_user() perms_manager = Permission.objects.using(db) permissions = set() From 0b501b0f47d338a7eb10953637817007506e8faa Mon Sep 17 00:00:00 2001 From: J-E Castagnede Date: Wed, 10 Apr 2024 10:03:14 +0200 Subject: [PATCH 09/12] fix local map path --- .../management/commands/update_permissions_mapentity.py | 5 ++--- mapentity/middleware.py | 3 +++ mapentity/registry.py | 4 ++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/mapentity/management/commands/update_permissions_mapentity.py b/mapentity/management/commands/update_permissions_mapentity.py index 8917779bd..f42f7ec30 100644 --- a/mapentity/management/commands/update_permissions_mapentity.py +++ b/mapentity/management/commands/update_permissions_mapentity.py @@ -8,7 +8,6 @@ from django.contrib.contenttypes.models import ContentType from django.core.management.base import BaseCommand -from mapentity.middleware import get_internal_user from mapentity.registry import create_mapentity_model_permissions, registry logger = logging.getLogger(__name__) @@ -29,11 +28,11 @@ def execute(self, *args, **options): # Make sure apps are registered at this point import_module(settings.ROOT_URLCONF) - internal_user = get_internal_user() + # For all models registered, add missing bits for model in registry.registry.keys(): if not model._meta.abstract: - create_mapentity_model_permissions(model, internal_user) + create_mapentity_model_permissions(model) logger.info("Done.") diff --git a/mapentity/middleware.py b/mapentity/middleware.py index 4b19a4899..92347b7d2 100644 --- a/mapentity/middleware.py +++ b/mapentity/middleware.py @@ -32,6 +32,9 @@ def get_internal_user(): return internal_user +internal_user = get_internal_user() + + class AutoLoginMiddleware: def __init__(self, get_response): self.get_response = get_response diff --git a/mapentity/registry.py b/mapentity/registry.py index ea097e8cc..e64e578d2 100644 --- a/mapentity/registry.py +++ b/mapentity/registry.py @@ -17,6 +17,7 @@ from rest_framework.serializers import ModelSerializer from mapentity import models as mapentity_models +from mapentity.middleware import internal_user from mapentity.serializers import MapentityGeojsonModelSerializer from mapentity.settings import app_settings @@ -231,7 +232,7 @@ def entities(self): registry = Registry() -def create_mapentity_model_permissions(model, internal_user): +def create_mapentity_model_permissions(model): """ Create all the necessary permissions for the specified model. @@ -252,7 +253,6 @@ def create_mapentity_model_permissions(model, internal_user): return db = DEFAULT_DB_ALIAS - perms_manager = Permission.objects.using(db) permissions = set() From e137c8bfa87de8b4a8708fe0b50fe68dc17543fa Mon Sep 17 00:00:00 2001 From: J-E Castagnede Date: Wed, 10 Apr 2024 10:10:14 +0200 Subject: [PATCH 10/12] fix local map path --- mapentity/middleware.py | 3 --- mapentity/registry.py | 24 ++++++++++++++++-------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/mapentity/middleware.py b/mapentity/middleware.py index 92347b7d2..4b19a4899 100644 --- a/mapentity/middleware.py +++ b/mapentity/middleware.py @@ -32,9 +32,6 @@ def get_internal_user(): return internal_user -internal_user = get_internal_user() - - class AutoLoginMiddleware: def __init__(self, get_response): self.get_response = get_response diff --git a/mapentity/registry.py b/mapentity/registry.py index e64e578d2..2c5914c2e 100644 --- a/mapentity/registry.py +++ b/mapentity/registry.py @@ -17,7 +17,7 @@ from rest_framework.serializers import ModelSerializer from mapentity import models as mapentity_models -from mapentity.middleware import internal_user +from mapentity.middleware import get_internal_user from mapentity.serializers import MapentityGeojsonModelSerializer from mapentity.settings import app_settings @@ -253,6 +253,7 @@ def create_mapentity_model_permissions(model): return db = DEFAULT_DB_ALIAS + internal_user = get_internal_user() perms_manager = Permission.objects.using(db) permissions = set() @@ -280,15 +281,22 @@ def create_mapentity_model_permissions(model): content_type=ctype) if not internal_user_permission.exists(): - permission = perms_manager.get(codename=codename, content_type=ctype) - internal_user.user_permissions.add(permission) - logger.info("Added permission %s to internal user %s" % (codename, - internal_user)) + try: + permission = perms_manager.get(codename=codename, content_type=ctype) + internal_user.user_permissions.add(permission) + logger.info("Added permission %s to internal user %s" % (codename, + internal_user)) + except Exception: + pass attachmenttype = ContentType.objects.db_manager(db).get_for_model(get_attachment_model()) read_perm = dict(codename='read_attachment', content_type=attachmenttype) if not internal_user.user_permissions.filter(**read_perm).exists(): permission = perms_manager.get(**read_perm) - internal_user.user_permissions.add(permission) - logger.info("Added permission %s to internal user %s" % (permission.codename, - internal_user)) + try: + internal_user.user_permissions.add(permission) + logger.info("Added permission %s to internal user %s" % (permission.codename, + internal_user)) + + except Exception: + pass From 22809b15ae31bbf04f87bf8141bf4f3565b6f26f Mon Sep 17 00:00:00 2001 From: J-E Castagnede Date: Wed, 10 Apr 2024 14:10:23 +0200 Subject: [PATCH 11/12] fix coverage --- .github/workflows/python-ci.yml | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/.github/workflows/python-ci.yml b/.github/workflows/python-ci.yml index 98e4cf093..f8b6524de 100644 --- a/.github/workflows/python-ci.yml +++ b/.github/workflows/python-ci.yml @@ -37,6 +37,10 @@ jobs: python-version: [ '3.8', '3.11' ] django-version: [ '3.2.*', '4.2.*' ] + env: + PYTHON: ${{ matrix.python-version }} + DJANGO: ${{ matrix.django-version }} + steps: - uses: actions/checkout@v4 @@ -62,13 +66,16 @@ jobs: - name: Test with coverage run: | - coverage run ./manage.py test -v3 - coverage report -m + coverage run --parallel-mode --concurrency=multiprocessing ./manage.py test --parallel -v 3 + coverage combine + coverage xml -o coverage.xml - - name: Coverage upload - run: | - pip install codecov - codecov + - uses: codecov/codecov-action@v3.1.4 + with: + files: ./coverage.xml + env_vars: PYTHON,DJANGO + token: ${{ secrets.CODECOV_TOKEN }} # not usually required for public repos + fail_ci_if_error: true # optional (default = false) publish: needs: [lint, test] From ca55a4cf6592ab8fa06b4662a0cc9b36cf91f2ac Mon Sep 17 00:00:00 2001 From: J-E Castagnede Date: Wed, 10 Apr 2024 14:10:32 +0200 Subject: [PATCH 12/12] fix coverage --- .github/workflows/python-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python-ci.yml b/.github/workflows/python-ci.yml index f8b6524de..2caf8fa2a 100644 --- a/.github/workflows/python-ci.yml +++ b/.github/workflows/python-ci.yml @@ -70,7 +70,7 @@ jobs: coverage combine coverage xml -o coverage.xml - - uses: codecov/codecov-action@v3.1.4 + - uses: codecov/codecov-action@v4 with: files: ./coverage.xml env_vars: PYTHON,DJANGO