From f69c959f2a91555f4469050e2cd217e317d98d44 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Wed, 19 Jul 2023 14:16:57 +0200 Subject: [PATCH 1/2] Use ns time for geojson and gzipped geojson mtime --- umap/tests/test_datalayer_views.py | 13 +++++++++++++ umap/utils.py | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/umap/tests/test_datalayer_views.py b/umap/tests/test_datalayer_views.py index e2c4e925..3693725e 100644 --- a/umap/tests/test_datalayer_views.py +++ b/umap/tests/test_datalayer_views.py @@ -1,4 +1,5 @@ import json +from pathlib import Path import pytest from django.core.files.base import ContentFile @@ -33,6 +34,17 @@ def test_get(client, settings, datalayer): assert j["type"] == "FeatureCollection" +def test_gzip_should_be_created_if_accepted(client, datalayer, map, post_data): + url = reverse("datalayer_view", args=(datalayer.pk,)) + response = client.get(url, headers={"ACCEPT_ENCODING": "gzip"}) + assert response.status_code == 200 + flat = datalayer.geojson.path + gzipped = datalayer.geojson.path + ".gz" + assert Path(flat).exists() + assert Path(gzipped).exists() + assert Path(flat).stat().st_mtime_ns == Path(gzipped).stat().st_mtime_ns + + def test_update(client, datalayer, map, post_data): url = reverse("datalayer_update", args=(map.pk, datalayer.pk)) client.login(username=map.owner.username, password="123123") @@ -49,6 +61,7 @@ def test_update(client, datalayer, map, post_data): j = json.loads(response.content.decode()) assert "id" in j assert datalayer.pk == j["id"] + assert Path(modified_datalayer.geojson.path).exists() def test_should_not_be_possible_to_update_with_wrong_map_id_in_url( diff --git a/umap/utils.py b/umap/utils.py index 0890dbfd..29417150 100644 --- a/umap/utils.py +++ b/umap/utils.py @@ -112,7 +112,7 @@ def gzip_file(from_path, to_path): with open(from_path, "rb") as f_in: with gzip.open(to_path, "wb") as f_out: f_out.writelines(f_in) - os.utime(to_path, (stat.st_mtime, stat.st_mtime)) + os.utime(to_path, ns=(stat.st_mtime_ns, stat.st_mtime_ns)) def is_ajax(request): From 13a1c3bd5c69711dbd415b273b63c9bab24ae042 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Wed, 19 Jul 2023 14:17:40 +0200 Subject: [PATCH 2/2] Use gzip mtime for Last-Modified comparison when in gzip mode Prior to 1.3.0, uMap was not setting the gzip mtime, so it was whatever the time it get requested at first. Since 1.3.0: - when creating the geojson.gzip, we also force its mtime to be the geojson one - we replaced If-Match by If-Unmodified, which relies on Last-Modified When uMap is served by a proxy like Nginx (and X-Accel-Redirect), and if user accepts gzip, then the Last-Modified would be the gzip one, not the flat geojson one. So when comparing that value in a subsequent update, we need to compare with the correct value. fix #1212 --- umap/views.py | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/umap/views.py b/umap/views.py index 3459e49e..b79240c5 100644 --- a/umap/views.py +++ b/umap/views.py @@ -784,11 +784,28 @@ class GZipMixin(object): def path(self): return self.object.geojson.path + @property + def gzip_path(self): + return Path(f"{self.path}{self.EXT}") + @property def last_modified(self): - stat = os.stat(self.path) + # Prior to 1.3.0 we did not set gzip mtime as geojson mtime, + # but we switched from If-Match header to IF-Unmodified-Since + # and when users accepts gzip their last modified value is the gzip + # (when umap is served by nginx and X-Accel-Redirect) + # one, so we need to compare with that value in that case. + # cf https://github.com/umap-project/umap/issues/1212 + path = self.gzip_path if self.accepts_gzip else self.path + stat = os.stat(path) return http_date(stat.st_mtime) + @property + def accepts_gzip(self): + return settings.UMAP_GZIP and re_accepts_gzip.search( + self.request.META.get("HTTP_ACCEPT_ENCODING", "") + ) + class DataLayerView(GZipMixin, BaseDetailView): model = DataLayer @@ -797,13 +814,9 @@ class DataLayerView(GZipMixin, BaseDetailView): response = None path = self.path # Generate gzip if needed - accepts_gzip = re_accepts_gzip.search( - self.request.META.get("HTTP_ACCEPT_ENCODING", "") - ) - if accepts_gzip and settings.UMAP_GZIP: - gzip_path = Path(f"{path}{self.EXT}") - if not gzip_path.exists(): - gzip_file(path, gzip_path) + if self.accepts_gzip: + if not self.gzip_path.exists(): + gzip_file(path, self.gzip_path) if getattr(settings, "UMAP_XSENDFILE_HEADER", None): response = HttpResponse()