From 9fbb705a0beba57cdfd07452c0ee5eab7def220d Mon Sep 17 00:00:00 2001
From: Manon Blanco <blanco@teklia.com>
Date: Tue, 12 Sep 2023 14:30:40 +0000
Subject: [PATCH] Allow a worker to specify the needed size of the image

---
 arkindex_worker/cache.py  |  28 +++++---
 arkindex_worker/models.py |  31 +++++----
 tests/test_cache.py       | 133 ++++++++++++++++++++++++++++++++++----
 tests/test_element.py     |  51 ++++++++++-----
 4 files changed, 194 insertions(+), 49 deletions(-)

diff --git a/arkindex_worker/cache.py b/arkindex_worker/cache.py
index 42871350..233c7d93 100644
--- a/arkindex_worker/cache.py
+++ b/arkindex_worker/cache.py
@@ -102,14 +102,26 @@ class CachedElement(Model):
         database = db
         table_name = "elements"
 
-    def open_image(self, *args, max_size: Optional[int] = None, **kwargs) -> Image:
+    def open_image(
+        self,
+        *args,
+        max_width: Optional[int] = None,
+        max_height: Optional[int] = None,
+        **kwargs,
+    ) -> Image:
         """
         Open this element's image as a Pillow image.
         This does not crop the image to the element's polygon.
         IIIF servers with maxWidth, maxHeight or maxArea restrictions on image size are not supported.
 
+        Warns:
+        ----
+           If both, ``max_width`` and ``max_height`` are set, the image ratio is not preserved.
+
+
         :param *args: Positional arguments passed to [arkindex_worker.image.open_image][]
-        :param max_size: Subresolution of the image.
+        :param max_width: The maximum width of the image.
+        :param max_height: The maximum height of the image.
         :param **kwargs: Keyword arguments passed to [arkindex_worker.image.open_image][]
         :raises ValueError: When this element does not have an image ID or a polygon.
         :return: A Pillow image.
@@ -129,7 +141,7 @@ class CachedElement(Model):
         else:
             box = "full"
 
-        if max_size is None:
+        if max_width is None and max_height is None:
             resize = "full"
         else:
             # Do not resize for polygons that do not exactly match the images
@@ -141,14 +153,12 @@ class CachedElement(Model):
                 resize = "full"
 
             # Do not resize when the image is below the maximum size
-            elif self.image.width <= max_size and self.image.height <= max_size:
+            elif (max_width is None or self.image.width <= max_width) and (
+                max_height is None or self.image.height <= max_height
+            ):
                 resize = "full"
             else:
-                ratio = max_size / max(self.image.width, self.image.height)
-                new_width, new_height = int(self.image.width * ratio), int(
-                    self.image.height * ratio
-                )
-                resize = f"{new_width},{new_height}"
+                resize = f"{max_width or ''},{max_height or ''}"
 
         url = self.image.url
         if not url.endswith("/"):
diff --git a/arkindex_worker/models.py b/arkindex_worker/models.py
index 924610b6..d79495a6 100644
--- a/arkindex_worker/models.py
+++ b/arkindex_worker/models.py
@@ -124,9 +124,10 @@ class Element(MagicDict):
     def open_image(
         self,
         *args,
-        max_size: Optional[int] = None,
+        max_width: Optional[int] = None,
+        max_height: Optional[int] = None,
         use_full_image: Optional[bool] = False,
-        **kwargs
+        **kwargs,
     ) -> Image:
         """
         Open this element's image using Pillow, rotating and mirroring it according
@@ -149,7 +150,13 @@ class Element(MagicDict):
            ``rotation_angle=0, mirrored=False`` as keyword arguments.
 
 
-        :param max_size: The maximum size of the requested image.
+        Warns:
+        ----
+           If both, ``max_width`` and ``max_height`` are set, the image ratio is not preserved.
+
+
+        :param max_width: The maximum width of the image.
+        :param max_height: The maximum height of the image.
         :param use_full_image: Ignore the ``zone.polygon`` and always
            retrieve the image without cropping.
         :param *args: Positional arguments passed to [arkindex_worker.image.open_image][].
@@ -172,12 +179,14 @@ class Element(MagicDict):
             raise ValueError("Element {} has no zone".format(self.id))
 
         if self.requires_tiles:
-            if max_size is None:
+            if max_width is None and max_height is None:
                 return download_tiles(self.zone.image.url)
             else:
                 raise NotImplementedError
 
-        if max_size is not None:
+        if max_width is None and max_height is None:
+            resize = "full"
+        else:
             bounding_box = polygon_bounding_box(self.zone.polygon)
             original_size = {"w": self.zone.image.width, "h": self.zone.image.height}
             # No resizing if the element is smaller than the image.
@@ -191,15 +200,13 @@ class Element(MagicDict):
                     + "downloading full size image."
                 )
             # No resizing if the image is smaller than the wanted size.
-            elif original_size["w"] <= max_size and original_size["h"] <= max_size:
+            elif (max_width is None or original_size["w"] <= max_width) and (
+                max_height is None or original_size["h"] <= max_height
+            ):
                 resize = "full"
             # Resizing if the image is bigger than the wanted size.
             else:
-                ratio = max_size / max(original_size.values())
-                new_width, new_height = [int(x * ratio) for x in original_size.values()]
-                resize = "{},{}".format(new_width, new_height)
-        else:
-            resize = "full"
+                resize = f"{max_width or ''},{max_height or ''}"
 
         if use_full_image:
             url = self.image_url(resize)
@@ -212,7 +219,7 @@ class Element(MagicDict):
                 *args,
                 rotation_angle=self.rotation_angle,
                 mirrored=self.mirrored,
-                **kwargs
+                **kwargs,
             )
         except HTTPError as e:
             if (
diff --git a/tests/test_cache.py b/tests/test_cache.py
index 77c9ab4c..53430b30 100644
--- a/tests/test_cache.py
+++ b/tests/test_cache.py
@@ -148,10 +148,20 @@ def test_check_version_same_version(tmp_path):
 
 
 @pytest.mark.parametrize(
-    "image_width,image_height,polygon_x,polygon_y,polygon_width,polygon_height,max_size,expected_url",
+    "image_width,image_height,polygon_x,polygon_y,polygon_width,polygon_height,max_width,max_height,expected_url",
     [
         # No max_size: no resize
-        (400, 600, 0, 0, 400, 600, None, "http://something/full/full/0/default.jpg"),
+        (
+            400,
+            600,
+            0,
+            0,
+            400,
+            600,
+            None,
+            None,
+            "http://something/full/full/0/default.jpg",
+        ),
         # No max_size: resize on bbox
         (
             400,
@@ -161,6 +171,7 @@ def test_check_version_same_version(tmp_path):
             200,
             100,
             None,
+            None,
             "http://something/0,0,200,100/full/0/default.jpg",
         ),
         (
@@ -171,12 +182,43 @@ def test_check_version_same_version(tmp_path):
             200,
             100,
             None,
+            None,
             "http://something/50,50,200,100/full/0/default.jpg",
         ),
         # max_size equal to the image size, no resize
-        (400, 600, 0, 0, 400, 600, 600, "http://something/full/full/0/default.jpg"),
-        (600, 400, 0, 0, 600, 400, 600, "http://something/full/full/0/default.jpg"),
-        (400, 400, 0, 0, 400, 400, 400, "http://something/full/full/0/default.jpg"),
+        (
+            400,
+            600,
+            0,
+            0,
+            400,
+            600,
+            400,
+            None,
+            "http://something/full/full/0/default.jpg",
+        ),
+        (
+            600,
+            400,
+            0,
+            0,
+            600,
+            400,
+            None,
+            400,
+            "http://something/full/full/0/default.jpg",
+        ),
+        (
+            400,
+            400,
+            0,
+            0,
+            400,
+            400,
+            400,
+            400,
+            "http://something/full/full/0/default.jpg",
+        ),
         (
             400,
             400,
@@ -185,11 +227,32 @@ def test_check_version_same_version(tmp_path):
             200,
             100,
             200,
+            100,
             "http://something/50,50,200,100/full/0/default.jpg",
         ),
         # max_size is smaller than the image, resize
-        (400, 600, 0, 0, 400, 600, 400, "http://something/full/266,400/0/default.jpg"),
-        (600, 400, 0, 0, 600, 400, 400, "http://something/full/400,266/0/default.jpg"),
+        (
+            400,
+            600,
+            0,
+            0,
+            400,
+            600,
+            None,
+            400,
+            "http://something/full/,400/0/default.jpg",
+        ),
+        (
+            600,
+            400,
+            0,
+            0,
+            600,
+            400,
+            400,
+            None,
+            "http://something/full/400,/0/default.jpg",
+        ),
         (
             400,
             600,
@@ -198,6 +261,7 @@ def test_check_version_same_version(tmp_path):
             200,
             600,
             400,
+            600,
             "http://something/0,0,200,600/full/0/default.jpg",
         ),
         (
@@ -208,13 +272,54 @@ def test_check_version_same_version(tmp_path):
             200,
             600,
             400,
+            600,
             "http://something/50,50,200,600/full/0/default.jpg",
         ),
-        (400, 400, 0, 0, 400, 400, 200, "http://something/full/200,200/0/default.jpg"),
+        (
+            400,
+            400,
+            0,
+            0,
+            400,
+            400,
+            200,
+            200,
+            "http://something/full/200,200/0/default.jpg",
+        ),
         # max_size above the image size, no resize
-        (400, 600, 0, 0, 400, 600, 800, "http://something/full/full/0/default.jpg"),
-        (600, 400, 0, 0, 600, 400, 800, "http://something/full/full/0/default.jpg"),
-        (400, 400, 0, 0, 400, 400, 800, "http://something/full/full/0/default.jpg"),
+        (
+            400,
+            600,
+            0,
+            0,
+            400,
+            600,
+            800,
+            None,
+            "http://something/full/full/0/default.jpg",
+        ),
+        (
+            600,
+            400,
+            0,
+            0,
+            600,
+            400,
+            None,
+            800,
+            "http://something/full/full/0/default.jpg",
+        ),
+        (
+            400,
+            400,
+            0,
+            0,
+            400,
+            400,
+            800,
+            800,
+            "http://something/full/full/0/default.jpg",
+        ),
         (
             400,
             400,
@@ -223,6 +328,7 @@ def test_check_version_same_version(tmp_path):
             200,
             100,
             800,
+            800,
             "http://something/50,50,200,100/full/0/default.jpg",
         ),
     ],
@@ -235,7 +341,8 @@ def test_element_open_image(
     polygon_y,
     polygon_width,
     polygon_height,
-    max_size,
+    max_width,
+    max_height,
     expected_url,
 ):
     open_mock = mocker.patch(
@@ -261,7 +368,7 @@ def test_element_open_image(
         ],
     )
 
-    assert elt.open_image(max_size=max_size) == "an image!"
+    assert elt.open_image(max_width=max_width, max_height=max_height) == "an image!"
     assert open_mock.call_count == 1
     assert open_mock.call_args == mocker.call(
         expected_url, mirrored=False, rotation_angle=0
diff --git a/tests/test_element.py b/tests/test_element.py
index e26e2973..734d5c81 100644
--- a/tests/test_element.py
+++ b/tests/test_element.py
@@ -94,7 +94,7 @@ def test_open_image_resize_portrait(mocker):
         }
     )
     # Resize = original size
-    assert elt.open_image(max_size=600, use_full_image=True) == "an image!"
+    assert elt.open_image(max_height=600, use_full_image=True) == "an image!"
     assert open_mock.call_count == 1
     assert open_mock.call_args == mocker.call(
         "http://something/full/full/0/default.jpg",
@@ -102,15 +102,15 @@ def test_open_image_resize_portrait(mocker):
         mirrored=False,
     )
     # Resize = smaller height
-    assert elt.open_image(max_size=400, use_full_image=True) == "an image!"
+    assert elt.open_image(max_height=400, use_full_image=True) == "an image!"
     assert open_mock.call_count == 2
     assert open_mock.call_args == mocker.call(
-        "http://something/full/266,400/0/default.jpg",
+        "http://something/full/,400/0/default.jpg",
         rotation_angle=0,
         mirrored=False,
     )
     # Resize = bigger height
-    assert elt.open_image(max_size=800, use_full_image=True) == "an image!"
+    assert elt.open_image(max_height=800, use_full_image=True) == "an image!"
     assert open_mock.call_count == 3
     assert open_mock.call_args == mocker.call(
         "http://something/full/full/0/default.jpg",
@@ -138,7 +138,7 @@ def test_open_image_resize_partial_element(mocker):
             "mirrored": False,
         }
     )
-    assert elt.open_image(max_size=400, use_full_image=True) == "an image!"
+    assert elt.open_image(max_height=400, use_full_image=True) == "an image!"
     assert open_mock.call_count == 1
     assert open_mock.call_args == mocker.call(
         "http://something/full/full/0/default.jpg",
@@ -167,7 +167,7 @@ def test_open_image_resize_landscape(mocker):
         }
     )
     # Resize = original size
-    assert elt.open_image(max_size=600, use_full_image=True) == "an image!"
+    assert elt.open_image(max_width=600, use_full_image=True) == "an image!"
     assert open_mock.call_count == 1
     assert open_mock.call_args == mocker.call(
         "http://something/full/full/0/default.jpg",
@@ -175,15 +175,15 @@ def test_open_image_resize_landscape(mocker):
         mirrored=False,
     )
     # Resize = smaller width
-    assert elt.open_image(max_size=400, use_full_image=True) == "an image!"
+    assert elt.open_image(max_width=400, use_full_image=True) == "an image!"
     assert open_mock.call_count == 2
     assert open_mock.call_args == mocker.call(
-        "http://something/full/400,266/0/default.jpg",
+        "http://something/full/400,/0/default.jpg",
         rotation_angle=0,
         mirrored=False,
     )
     # Resize = bigger width
-    assert elt.open_image(max_size=800, use_full_image=True) == "an image!"
+    assert elt.open_image(max_width=800, use_full_image=True) == "an image!"
     assert open_mock.call_count == 3
     assert open_mock.call_args == mocker.call(
         "http://something/full/full/0/default.jpg",
@@ -212,7 +212,14 @@ def test_open_image_resize_square(mocker):
         }
     )
     # Resize = original size
-    assert elt.open_image(max_size=400, use_full_image=True) == "an image!"
+    assert (
+        elt.open_image(
+            max_width=400,
+            max_height=400,
+            use_full_image=True,
+        )
+        == "an image!"
+    )
     assert open_mock.call_count == 1
     assert open_mock.call_args == mocker.call(
         "http://something/full/full/0/default.jpg",
@@ -220,7 +227,14 @@ def test_open_image_resize_square(mocker):
         mirrored=False,
     )
     # Resize = smaller
-    assert elt.open_image(max_size=200, use_full_image=True) == "an image!"
+    assert (
+        elt.open_image(
+            max_width=200,
+            max_height=200,
+            use_full_image=True,
+        )
+        == "an image!"
+    )
     assert open_mock.call_count == 2
     assert open_mock.call_args == mocker.call(
         "http://something/full/200,200/0/default.jpg",
@@ -228,7 +242,14 @@ def test_open_image_resize_square(mocker):
         mirrored=False,
     )
     # Resize = bigger
-    assert elt.open_image(max_size=800, use_full_image=True) == "an image!"
+    assert (
+        elt.open_image(
+            max_width=800,
+            max_height=800,
+            use_full_image=True,
+        )
+        == "an image!"
+    )
     assert open_mock.call_count == 3
     assert open_mock.call_args == mocker.call(
         "http://something/full/full/0/default.jpg",
@@ -251,7 +272,7 @@ def test_open_image_resize_tiles(mocker):
         }
     )
     with pytest.raises(NotImplementedError):
-        elt.open_image(max_size=400)
+        elt.open_image(max_width=400)
 
 
 def test_open_image_requires_zone():
@@ -364,10 +385,10 @@ def test_open_image_resize_use_full_image_false(mocker):
         }
     )
     # Resize = smaller
-    assert elt.open_image(max_size=200, use_full_image=False) == "an image!"
+    assert elt.open_image(max_height=200, use_full_image=False) == "an image!"
     assert open_mock.call_count == 1
     assert open_mock.call_args == mocker.call(
-        "http://zoneurl/0,0,400,600/133,200/0/default.jpg",
+        "http://zoneurl/0,0,400,600/,200/0/default.jpg",
         rotation_angle=0,
         mirrored=False,
     )
-- 
GitLab