-
-
Notifications
You must be signed in to change notification settings - Fork 75
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Branch: refs/heads/master Date: 2022-05-17T11:13:16+02:00 Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org> Commit: plone/plone.namedfile@2e54b2c Only look at the width when checking if a HiDPI image would be larger than the original. Otherwise HiDPI srcsets are never included when the scale is defined with a height of 65536. Fixes plone/plone.namedfile#114 Files changed: A news/114.bugfix M plone/namedfile/scaling.py Repository: plone.namedfile Branch: refs/heads/master Date: 2022-05-17T22:27:23+02:00 Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org> Commit: plone/plone.namedfile@8f910cd Merge pull request #115 from plone/maurits-ignore-height-check-for-hidpi Only look at width when checking if HiDPI image would be too large Files changed: A news/114.bugfix M plone/namedfile/scaling.py
- Loading branch information
1 parent
408b0ea
commit c200c04
Showing
1 changed file
with
17 additions
and
48 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,68 +1,37 @@ | ||
| Repository: plone.scale | ||
| Repository: plone.namedfile | ||
|
|
||
|
|
||
| Branch: refs/heads/master | ||
| Date: 2022-05-16T14:24:05+02:00 | ||
| Date: 2022-05-17T11:13:16+02:00 | ||
| Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org> | ||
| Commit: https://github.com/plone/plone.scale/commit/6cc4b95ea0a18b828e6539fdb55b00cdf3b7f72a | ||
| Commit: https://github.com/plone/plone.namedfile/commit/2e54b2c949adcdcda8afd57e73898a781224de69 | ||
|
|
||
| Add more tests for cleaning up old items. | ||
| Only look at the width when checking if a HiDPI image would be larger than the original. | ||
|
|
||
| Files changed: | ||
| M plone/scale/tests/test_storage.py | ||
|
|
||
| b'diff --git a/plone/scale/tests/test_storage.py b/plone/scale/tests/test_storage.py\nindex 17c2f2b..50fa17d 100644\n--- a/plone/scale/tests/test_storage.py\n+++ b/plone/scale/tests/test_storage.py\n@@ -148,7 +148,7 @@ def testDeleteRemovesItemAndIndex(self):\n del storage[scale["uid"]]\n self.assertEqual(len(storage), 0)\n \n- def testCleanUpOldItems(self):\n+ def testCleanUpOldItemsForSameParameters(self):\n self._provide_dummy_scale_adapter()\n storage = self.storage\n scale_old = storage.scale(foo=23, bar=42)\n@@ -158,17 +158,32 @@ def testCleanUpOldItems(self):\n self.assertEqual(len(storage), 1)\n self.assertIn(scale_new["uid"], storage)\n self.assertNotIn(scale_old["uid"], storage)\n+ del storage[scale_new["uid"]]\n+ self.assertEqual(len(storage), 0)\n+\n+ def testCleanUpOldItemsForDifferentParameters(self):\n+ self._provide_dummy_scale_adapter()\n+ storage = self.storage\n+ scale_old = storage.scale(foo=23, bar=42)\n+ orig_modified = storage.modified()\n+ next_modified = orig_modified + 60000\n+ storage.modified = lambda: next_modified\n+ scale_new = storage.scale(foo=23, bar=50)\n+ self.assertEqual(len(storage), 2)\n+ self.assertIn(scale_new["uid"], storage)\n+ self.assertIn(scale_old["uid"], storage)\n \n # When modification time is older than a day, too old scales\n # get purged.\n- next_modified = storage.modified() + 24 * 60 * 60 * 1000 + 1\n+ next_modified = orig_modified + 24 * 60 * 60 * 1000 + 1\n storage.modified = lambda: next_modified\n- scale_newer = storage.scale(foo=23, bar=42)\n+ scale_newer = storage.scale(foo=23, bar=70)\n \n self.assertIn(scale_newer["uid"], storage)\n- self.assertNotIn(scale_new["uid"], storage)\n+ self.assertIn(scale_new["uid"], storage)\n self.assertNotIn(scale_old["uid"], storage)\n del storage[scale_newer["uid"]]\n+ del storage[scale_new["uid"]]\n self.assertEqual(len(storage), 0)\n \n def testClear(self):\n' | ||
|
|
||
| Repository: plone.scale | ||
|
|
||
|
|
||
| Branch: refs/heads/master | ||
| Date: 2022-05-16T15:34:19+02:00 | ||
| Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org> | ||
| Commit: https://github.com/plone/plone.scale/commit/3b61587dd914946d15194aec47376a916703c8f3 | ||
|
|
||
| Test _modified_since method. | ||
|
|
||
| Files changed: | ||
| M plone/scale/tests/test_storage.py | ||
|
|
||
| b'diff --git a/plone/scale/tests/test_storage.py b/plone/scale/tests/test_storage.py\nindex 50fa17d..351acdf 100644\n--- a/plone/scale/tests/test_storage.py\n+++ b/plone/scale/tests/test_storage.py\n@@ -148,6 +148,20 @@ def testDeleteRemovesItemAndIndex(self):\n del storage[scale["uid"]]\n self.assertEqual(len(storage), 0)\n \n+ def test_modified_since(self):\n+ self._provide_dummy_scale_adapter()\n+ storage = self.storage\n+ self.assertEqual(storage.modified(), 42)\n+\n+ self.assertTrue(storage._modified_since(41))\n+ self.assertFalse(storage._modified_since(42))\n+ self.assertFalse(storage._modified_since(43))\n+\n+ self.assertFalse(storage._modified_since(41, offset=1))\n+ self.assertTrue(storage._modified_since(40, offset=1))\n+ self.assertFalse(storage._modified_since(32, offset=10))\n+ self.assertTrue(storage._modified_since(32, offset=9))\n+\n def testCleanUpOldItemsForSameParameters(self):\n self._provide_dummy_scale_adapter()\n storage = self.storage\n' | ||
|
|
||
| Repository: plone.scale | ||
|
|
||
|
|
||
| Branch: refs/heads/master | ||
| Date: 2022-05-16T16:13:13+02:00 | ||
| Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org> | ||
| Commit: https://github.com/plone/plone.scale/commit/afbad8258e8d0d5a7ba817ec5da9da2a8a373dbe | ||
|
|
||
| Fix cleanup of scales: only throw away outdated scales of the same field. | ||
|
|
||
| Fixes https://github.com/plone/plone.scale/issues/55 | ||
| Otherwise HiDPI srcsets are never included when the scale is defined with a height of 65536. | ||
| Fixes https://github.com/plone/plone.namedfile/issues/114 | ||
|
|
||
| Files changed: | ||
| A news/55.bugfix | ||
| M plone/scale/storage.py | ||
| M plone/scale/tests/test_storage.py | ||
| A news/114.bugfix | ||
| M plone/namedfile/scaling.py | ||
|
|
||
| b'diff --git a/news/55.bugfix b/news/55.bugfix\nnew file mode 100644\nindex 0000000..261a4b6\n--- /dev/null\n+++ b/news/55.bugfix\n@@ -0,0 +1,2 @@\n+Fix cleanup of scales: only throw away outdated scales of the same field.\n+[maurits]\ndiff --git a/plone/scale/storage.py b/plone/scale/storage.py\nindex 6cab18a..87cedad 100644\n--- a/plone/scale/storage.py\n+++ b/plone/scale/storage.py\n@@ -192,7 +192,8 @@ def scale(self, **parameters):\n if result is not None:\n # storage will be modified:\n # good time to also cleanup\n- self._cleanup()\n+ fieldname = parameters.get("fieldname")\n+ self._cleanup(fieldname=fieldname)\n data, format_, dimensions = result\n width, height = dimensions\n uid = str(uuid4())\n@@ -205,12 +206,14 @@ def scale(self, **parameters):\n key=key,\n modified=self.modified_time,\n )\n+ if fieldname:\n+ info["fieldname"] = fieldname\n if outdated_uid:\n del self[outdated_uid]\n storage[uid] = info\n return info\n \n- def _cleanup(self):\n+ def _cleanup(self, fieldname=None):\n storage = self.storage\n modified_time = self.modified_time\n if modified_time is None:\n@@ -223,6 +226,10 @@ def _cleanup(self):\n # before refactoring\n if isinstance(key, tuple):\n del self[key]\n+ if fieldname and "fieldname" in value and value["fieldname"] != fieldname:\n+ # Leave scales for other fieldnames alone.\n+ # self.modified may have nothing to do with that field.\n+ continue\n # clear cache from scales older than one day\n elif self._modified_since(value["modified"], offset=KEEP_SCALE_MILLIS):\n del self[key]\n@@ -239,7 +246,7 @@ def __delitem__(self, uid):\n except KeyError:\n # This should not happen, but it apparently can happen in corner\n # cases. See https://github.com/plone/plone.scale/issues/15\n- logger.warn("Could not delete key %s from storage.", uid)\n+ logger.warning("Could not delete key %s from storage.", uid)\n \n def __iter__(self):\n return iter(self.storage)\ndiff --git a/plone/scale/tests/test_storage.py b/plone/scale/tests/test_storage.py\nindex 351acdf..0f883ca 100644\n--- a/plone/scale/tests/test_storage.py\n+++ b/plone/scale/tests/test_storage.py\n@@ -200,6 +200,35 @@ def testCleanUpOldItemsForDifferentParameters(self):\n del storage[scale_new["uid"]]\n self.assertEqual(len(storage), 0)\n \n+ def testCleanUpOldItemsForDifferentFieldname(self):\n+ self._provide_dummy_scale_adapter()\n+ storage = self.storage\n+ scale_image = storage.scale(fieldname="image", bar=42)\n+ next_modified = storage.modified() + 60000\n+ storage.modified = lambda: next_modified\n+ scale_leadimage_old = storage.scale(fieldname="leadimage", bar=50)\n+ self.assertEqual(len(storage), 2)\n+ self.assertIn(scale_leadimage_old["uid"], storage)\n+ self.assertIn(scale_image["uid"], storage)\n+\n+ # When modification time is older than a day, too old scales\n+ # get purged. But only for the current fieldname.\n+ next_modified = storage.modified() + 24 * 60 * 60 * 1000 + 1\n+ storage.modified = lambda: next_modified\n+ scale_leadimage_new = storage.scale(fieldname="leadimage", bar=70)\n+\n+ self.assertIn(scale_leadimage_new["uid"], storage)\n+ self.assertNotIn(scale_leadimage_old["uid"], storage)\n+ self.assertIn(scale_image["uid"], storage)\n+\n+ # If we manually call cleanup without a fieldname,\n+ # all items are checked.\n+ storage._cleanup()\n+ self.assertIn(scale_leadimage_new["uid"], storage)\n+ self.assertNotIn(scale_image["uid"], storage)\n+ del storage[scale_leadimage_new["uid"]]\n+ self.assertEqual(len(storage), 0)\n+\n def testClear(self):\n self._provide_dummy_scale_adapter()\n storage = self.storage\n' | ||
| b'diff --git a/news/114.bugfix b/news/114.bugfix\nnew file mode 100644\nindex 0000000..a9127a9\n--- /dev/null\n+++ b/news/114.bugfix\n@@ -0,0 +1,3 @@\n+Only look at the width when checking if a HiDPI image would be larger than the original.\n+Otherwise HiDPI srcsets are never included when the scale is defined with a height of 65536.\n+[maurits]\ndiff --git a/plone/namedfile/scaling.py b/plone/namedfile/scaling.py\nindex cf4133a..e8fb860 100644\n--- a/plone/namedfile/scaling.py\n+++ b/plone/namedfile/scaling.py\n@@ -538,9 +538,8 @@ def calculate_srcset(\n (orig_width, orig_height) = self.getImageSize(fieldname)\n for hdScale in self.getHighPixelDensityScales():\n # Don\'t create retina scales larger than the source image.\n- if (height and orig_height and orig_height < height * hdScale["scale"]) or (\n- width and orig_width and orig_width < width * hdScale["scale"]\n- ):\n+ # We only care about the width, because height might be 65536.\n+ if width and orig_width and orig_width < width * hdScale["scale"]:\n continue\n parameters["quality"] = hdScale["quality"]\n scale_src = storage.scale(\n' | ||
|
|
||
| Repository: plone.scale | ||
| Repository: plone.namedfile | ||
|
|
||
|
|
||
| Branch: refs/heads/master | ||
| Date: 2022-05-17T22:27:00+02:00 | ||
| Date: 2022-05-17T22:27:23+02:00 | ||
| Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org> | ||
| Commit: https://github.com/plone/plone.scale/commit/7caa0c4a7b6dec3c2b835384f4b0614e060e6229 | ||
| Commit: https://github.com/plone/plone.namedfile/commit/8f910cd1fd15115d32cbc21da121e31e85b6c5aa | ||
|
|
||
| Merge pull request #56 from plone/maurits-test-cleanup | ||
| Merge pull request #115 from plone/maurits-ignore-height-check-for-hidpi | ||
|
|
||
| Only throw away outdated scales of the same field. | ||
| Only look at width when checking if HiDPI image would be too large | ||
|
|
||
| Files changed: | ||
| A news/55.bugfix | ||
| M plone/scale/storage.py | ||
| M plone/scale/tests/test_storage.py | ||
| A news/114.bugfix | ||
| M plone/namedfile/scaling.py | ||
|
|
||
| b'diff --git a/news/55.bugfix b/news/55.bugfix\nnew file mode 100644\nindex 0000000..261a4b6\n--- /dev/null\n+++ b/news/55.bugfix\n@@ -0,0 +1,2 @@\n+Fix cleanup of scales: only throw away outdated scales of the same field.\n+[maurits]\ndiff --git a/plone/scale/storage.py b/plone/scale/storage.py\nindex 6cab18a..87cedad 100644\n--- a/plone/scale/storage.py\n+++ b/plone/scale/storage.py\n@@ -192,7 +192,8 @@ def scale(self, **parameters):\n if result is not None:\n # storage will be modified:\n # good time to also cleanup\n- self._cleanup()\n+ fieldname = parameters.get("fieldname")\n+ self._cleanup(fieldname=fieldname)\n data, format_, dimensions = result\n width, height = dimensions\n uid = str(uuid4())\n@@ -205,12 +206,14 @@ def scale(self, **parameters):\n key=key,\n modified=self.modified_time,\n )\n+ if fieldname:\n+ info["fieldname"] = fieldname\n if outdated_uid:\n del self[outdated_uid]\n storage[uid] = info\n return info\n \n- def _cleanup(self):\n+ def _cleanup(self, fieldname=None):\n storage = self.storage\n modified_time = self.modified_time\n if modified_time is None:\n@@ -223,6 +226,10 @@ def _cleanup(self):\n # before refactoring\n if isinstance(key, tuple):\n del self[key]\n+ if fieldname and "fieldname" in value and value["fieldname"] != fieldname:\n+ # Leave scales for other fieldnames alone.\n+ # self.modified may have nothing to do with that field.\n+ continue\n # clear cache from scales older than one day\n elif self._modified_since(value["modified"], offset=KEEP_SCALE_MILLIS):\n del self[key]\n@@ -239,7 +246,7 @@ def __delitem__(self, uid):\n except KeyError:\n # This should not happen, but it apparently can happen in corner\n # cases. See https://github.com/plone/plone.scale/issues/15\n- logger.warn("Could not delete key %s from storage.", uid)\n+ logger.warning("Could not delete key %s from storage.", uid)\n \n def __iter__(self):\n return iter(self.storage)\ndiff --git a/plone/scale/tests/test_storage.py b/plone/scale/tests/test_storage.py\nindex 17c2f2b..0f883ca 100644\n--- a/plone/scale/tests/test_storage.py\n+++ b/plone/scale/tests/test_storage.py\n@@ -148,7 +148,21 @@ def testDeleteRemovesItemAndIndex(self):\n del storage[scale["uid"]]\n self.assertEqual(len(storage), 0)\n \n- def testCleanUpOldItems(self):\n+ def test_modified_since(self):\n+ self._provide_dummy_scale_adapter()\n+ storage = self.storage\n+ self.assertEqual(storage.modified(), 42)\n+\n+ self.assertTrue(storage._modified_since(41))\n+ self.assertFalse(storage._modified_since(42))\n+ self.assertFalse(storage._modified_since(43))\n+\n+ self.assertFalse(storage._modified_since(41, offset=1))\n+ self.assertTrue(storage._modified_since(40, offset=1))\n+ self.assertFalse(storage._modified_since(32, offset=10))\n+ self.assertTrue(storage._modified_since(32, offset=9))\n+\n+ def testCleanUpOldItemsForSameParameters(self):\n self._provide_dummy_scale_adapter()\n storage = self.storage\n scale_old = storage.scale(foo=23, bar=42)\n@@ -158,17 +172,61 @@ def testCleanUpOldItems(self):\n self.assertEqual(len(storage), 1)\n self.assertIn(scale_new["uid"], storage)\n self.assertNotIn(scale_old["uid"], storage)\n+ del storage[scale_new["uid"]]\n+ self.assertEqual(len(storage), 0)\n+\n+ def testCleanUpOldItemsForDifferentParameters(self):\n+ self._provide_dummy_scale_adapter()\n+ storage = self.storage\n+ scale_old = storage.scale(foo=23, bar=42)\n+ orig_modified = storage.modified()\n+ next_modified = orig_modified + 60000\n+ storage.modified = lambda: next_modified\n+ scale_new = storage.scale(foo=23, bar=50)\n+ self.assertEqual(len(storage), 2)\n+ self.assertIn(scale_new["uid"], storage)\n+ self.assertIn(scale_old["uid"], storage)\n \n # When modification time is older than a day, too old scales\n # get purged.\n- next_modified = storage.modified() + 24 * 60 * 60 * 1000 + 1\n+ next_modified = orig_modified + 24 * 60 * 60 * 1000 + 1\n storage.modified = lambda: next_modified\n- scale_newer = storage.scale(foo=23, bar=42)\n+ scale_newer = storage.scale(foo=23, bar=70)\n \n self.assertIn(scale_newer["uid"], storage)\n- self.assertNotIn(scale_new["uid"], storage)\n+ self.assertIn(scale_new["uid"], storage)\n self.assertNotIn(scale_old["uid"], storage)\n del storage[scale_newer["uid"]]\n+ del storage[scale_new["uid"]]\n+ self.assertEqual(len(storage), 0)\n+\n+ def testCleanUpOldItemsForDifferentFieldname(self):\n+ self._provide_dummy_scale_adapter()\n+ storage = self.storage\n+ scale_image = storage.scale(fieldname="image", bar=42)\n+ next_modified = storage.modified() + 60000\n+ storage.modified = lambda: next_modified\n+ scale_leadimage_old = storage.scale(fieldname="leadimage", bar=50)\n+ self.assertEqual(len(storage), 2)\n+ self.assertIn(scale_leadimage_old["uid"], storage)\n+ self.assertIn(scale_image["uid"], storage)\n+\n+ # When modification time is older than a day, too old scales\n+ # get purged. But only for the current fieldname.\n+ next_modified = storage.modified() + 24 * 60 * 60 * 1000 + 1\n+ storage.modified = lambda: next_modified\n+ scale_leadimage_new = storage.scale(fieldname="leadimage", bar=70)\n+\n+ self.assertIn(scale_leadimage_new["uid"], storage)\n+ self.assertNotIn(scale_leadimage_old["uid"], storage)\n+ self.assertIn(scale_image["uid"], storage)\n+\n+ # If we manually call cleanup without a fieldname,\n+ # all items are checked.\n+ storage._cleanup()\n+ self.assertIn(scale_leadimage_new["uid"], storage)\n+ self.assertNotIn(scale_image["uid"], storage)\n+ del storage[scale_leadimage_new["uid"]]\n self.assertEqual(len(storage), 0)\n \n def testClear(self):\n' | ||
| b'diff --git a/news/114.bugfix b/news/114.bugfix\nnew file mode 100644\nindex 0000000..a9127a9\n--- /dev/null\n+++ b/news/114.bugfix\n@@ -0,0 +1,3 @@\n+Only look at the width when checking if a HiDPI image would be larger than the original.\n+Otherwise HiDPI srcsets are never included when the scale is defined with a height of 65536.\n+[maurits]\ndiff --git a/plone/namedfile/scaling.py b/plone/namedfile/scaling.py\nindex cf4133a..e8fb860 100644\n--- a/plone/namedfile/scaling.py\n+++ b/plone/namedfile/scaling.py\n@@ -538,9 +538,8 @@ def calculate_srcset(\n (orig_width, orig_height) = self.getImageSize(fieldname)\n for hdScale in self.getHighPixelDensityScales():\n # Don\'t create retina scales larger than the source image.\n- if (height and orig_height and orig_height < height * hdScale["scale"]) or (\n- width and orig_width and orig_width < width * hdScale["scale"]\n- ):\n+ # We only care about the width, because height might be 65536.\n+ if width and orig_width and orig_width < width * hdScale["scale"]:\n continue\n parameters["quality"] = hdScale["quality"]\n scale_src = storage.scale(\n' | ||
|
|