From a1eb6715c2fd93cf409393692434ad72eae5dca3 Mon Sep 17 00:00:00 2001 From: Mikel Larreategi Date: Thu, 25 Jun 2026 11:30:58 +0200 Subject: [PATCH 1/5] Add test for redundant image PATCH fires no events --- .../restapi/tests/test_redundant_patch.py | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 src/plone/restapi/tests/test_redundant_patch.py diff --git a/src/plone/restapi/tests/test_redundant_patch.py b/src/plone/restapi/tests/test_redundant_patch.py new file mode 100644 index 000000000..c9cb77f82 --- /dev/null +++ b/src/plone/restapi/tests/test_redundant_patch.py @@ -0,0 +1,74 @@ +from plone.app.testing import login +from plone.app.testing import setRoles +from plone.app.testing import SITE_OWNER_NAME +from plone.app.testing import SITE_OWNER_PASSWORD +from plone.app.testing import TEST_USER_ID +from plone.restapi.testing import PLONE_RESTAPI_DX_FUNCTIONAL_TESTING +from zope.component import getGlobalSiteManager +from zope.lifecycleevent.interfaces import IObjectModifiedEvent + +import requests +import transaction +import unittest + +class TestRedundantPatch(unittest.TestCase): + + layer = PLONE_RESTAPI_DX_FUNCTIONAL_TESTING + + def setUp(self): + self.app = self.layer["app"] + self.portal = self.layer["portal"] + setRoles(self.portal, TEST_USER_ID, ["Member", "Manager"]) + login(self.portal, SITE_OWNER_NAME) + transaction.commit() + + def test_patch_image_redundant_no_event(self): + image_data = "R0lGODlhAQABAIAAAP///wAAACwAAAAAAQABAAACAkQBADs=" + response = requests.post( + self.portal.absolute_url(), + headers={"Accept": "application/json"}, + auth=(SITE_OWNER_NAME, SITE_OWNER_PASSWORD), + json={ + "@type": "Image", + "image": { + "data": image_data, + "encoding": "base64", + "content-type": "image/gif", + "filename": "test.gif" + }, + }, + ) + self.assertEqual(201, response.status_code) + image_url = response.json()["@id"] + transaction.commit() + + # Track ObjectModifiedEvent + sm = getGlobalSiteManager() + fired_events = [] + + def record_event(event): + fired_events.append(event) + + sm.registerHandler(record_event, (IObjectModifiedEvent,)) + + # Patch with same data + response = requests.patch( + image_url, + headers={"Accept": "application/json"}, + auth=(SITE_OWNER_NAME, SITE_OWNER_PASSWORD), + json={ + "image": { + "data": image_data, + "encoding": "base64", + "content-type": "image/gif", + "filename": "test.gif" + }, + }, + ) + self.assertEqual(204, response.status_code) + + # In current Plone, this will be 1 because of identity mismatch + # We want it to be 0 + self.assertEqual(len(fired_events), 0, "ObjectModifiedEvent was fired for redundant PATCH") + + sm.unregisterHandler(record_event, (IObjectModifiedEvent,)) From 5763d78ccc06f931ad11f632164f1e0d73695311 Mon Sep 17 00:00:00 2001 From: Mikel Larreategi Date: Thu, 25 Jun 2026 12:08:51 +0200 Subject: [PATCH 2/5] Implement hash-aware redundant PATCH suppression --- news/fix_image_scale_bloat.bugfix | 1 + src/plone/restapi/deserializer/dxcontent.py | 10 ++++++++++ 2 files changed, 11 insertions(+) create mode 100644 news/fix_image_scale_bloat.bugfix diff --git a/news/fix_image_scale_bloat.bugfix b/news/fix_image_scale_bloat.bugfix new file mode 100644 index 000000000..69e741cd9 --- /dev/null +++ b/news/fix_image_scale_bloat.bugfix @@ -0,0 +1 @@ +Optimization: Avoid redundant object updates in PATCH requests when image content is identical to existing content. diff --git a/src/plone/restapi/deserializer/dxcontent.py b/src/plone/restapi/deserializer/dxcontent.py index 2be6564cc..31d8d29d9 100644 --- a/src/plone/restapi/deserializer/dxcontent.py +++ b/src/plone/restapi/deserializer/dxcontent.py @@ -140,6 +140,16 @@ def get_schema_data(self, data, validate_all, create=False): current_value = dm.get() if value != current_value: should_change = True + if ( + getattr(value, "_hash", None) is not None + and getattr(value, "_hash", None) + == getattr(current_value, "_hash", None) + and getattr(value, "filename", None) + == getattr(current_value, "filename", None) + and getattr(value, "contentType", None) + == getattr(current_value, "contentType", None) + ): + should_change = False elif create and dm.field.defaultFactory: # During content creation we should set the value even if # it is the same from the dm if the current_value was From a2f75cad687a425fc511d7b886b5b39879ab3abf Mon Sep 17 00:00:00 2001 From: Mikel Larreategi Date: Thu, 25 Jun 2026 12:24:44 +0200 Subject: [PATCH 3/5] Simplify redundant PATCH suppression by relying on plone.namedfile equality --- src/plone/restapi/deserializer/dxcontent.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/plone/restapi/deserializer/dxcontent.py b/src/plone/restapi/deserializer/dxcontent.py index 31d8d29d9..2be6564cc 100644 --- a/src/plone/restapi/deserializer/dxcontent.py +++ b/src/plone/restapi/deserializer/dxcontent.py @@ -140,16 +140,6 @@ def get_schema_data(self, data, validate_all, create=False): current_value = dm.get() if value != current_value: should_change = True - if ( - getattr(value, "_hash", None) is not None - and getattr(value, "_hash", None) - == getattr(current_value, "_hash", None) - and getattr(value, "filename", None) - == getattr(current_value, "filename", None) - and getattr(value, "contentType", None) - == getattr(current_value, "contentType", None) - ): - should_change = False elif create and dm.field.defaultFactory: # During content creation we should set the value even if # it is the same from the dm if the current_value was From a38f37bc18fc6d07b2acec4c59316b5287b7f1d3 Mon Sep 17 00:00:00 2001 From: Mikel Larreategi Date: Thu, 25 Jun 2026 12:39:00 +0200 Subject: [PATCH 4/5] Consolidate redundant PATCH test and add HAS_PLONE_62 guard --- src/plone/restapi/tests/test_content_patch.py | 60 +++++++++++++++ .../restapi/tests/test_redundant_patch.py | 74 ------------------- 2 files changed, 60 insertions(+), 74 deletions(-) delete mode 100644 src/plone/restapi/tests/test_redundant_patch.py diff --git a/src/plone/restapi/tests/test_content_patch.py b/src/plone/restapi/tests/test_content_patch.py index 067b33c30..1483746ff 100644 --- a/src/plone/restapi/tests/test_content_patch.py +++ b/src/plone/restapi/tests/test_content_patch.py @@ -1,3 +1,4 @@ +from importlib import import_module from OFS.interfaces import IObjectWillBeAddedEvent from plone.app.testing import login from plone.app.testing import setRoles @@ -20,6 +21,11 @@ import unittest +HAS_PLONE_62 = getattr( + import_module("Products.CMFPlone.factory"), "PLONE62MARKER", False +) + + class TestContentPatch(unittest.TestCase): layer = PLONE_RESTAPI_DX_FUNCTIONAL_TESTING @@ -197,3 +203,57 @@ def test_patch_document_with_apostrophe_dont_return_500(self): self.assertEqual(204, response.status_code) transaction.begin() self.assertEqual("

example with '

", self.portal.doc1.text.raw) + + @unittest.skipUnless(HAS_PLONE_62, "Requires Plone 6.2+") + def test_patch_image_redundant_no_event(self): + image_data = "R0lGODlhAQABAIAAAP///wAAACwAAAAAAQABAAACAkQBADs=" + response = requests.post( + self.portal.absolute_url(), + headers={"Accept": "application/json"}, + auth=(SITE_OWNER_NAME, SITE_OWNER_PASSWORD), + json={ + "@type": "Image", + "image": { + "data": image_data, + "encoding": "base64", + "content-type": "image/gif", + "filename": "test.gif", + }, + }, + ) + self.assertEqual(201, response.status_code) + image_url = response.json()["@id"] + transaction.commit() + + # Track ObjectModifiedEvent + sm = getGlobalSiteManager() + fired_events = [] + + def record_event(event): + fired_events.append(event) + + sm.registerHandler(record_event, (IObjectModifiedEvent,)) + + # Patch with same data + response = requests.patch( + image_url, + headers={"Accept": "application/json"}, + auth=(SITE_OWNER_NAME, SITE_OWNER_PASSWORD), + json={ + "image": { + "data": image_data, + "encoding": "base64", + "content-type": "image/gif", + "filename": "test.gif", + }, + }, + ) + self.assertEqual(204, response.status_code) + + # In current Plone, this will be 1 because of identity mismatch + # We want it to be 0 + self.assertEqual( + len(fired_events), 0, "ObjectModifiedEvent was fired for redundant PATCH" + ) + + sm.unregisterHandler(record_event, (IObjectModifiedEvent,)) diff --git a/src/plone/restapi/tests/test_redundant_patch.py b/src/plone/restapi/tests/test_redundant_patch.py deleted file mode 100644 index c9cb77f82..000000000 --- a/src/plone/restapi/tests/test_redundant_patch.py +++ /dev/null @@ -1,74 +0,0 @@ -from plone.app.testing import login -from plone.app.testing import setRoles -from plone.app.testing import SITE_OWNER_NAME -from plone.app.testing import SITE_OWNER_PASSWORD -from plone.app.testing import TEST_USER_ID -from plone.restapi.testing import PLONE_RESTAPI_DX_FUNCTIONAL_TESTING -from zope.component import getGlobalSiteManager -from zope.lifecycleevent.interfaces import IObjectModifiedEvent - -import requests -import transaction -import unittest - -class TestRedundantPatch(unittest.TestCase): - - layer = PLONE_RESTAPI_DX_FUNCTIONAL_TESTING - - def setUp(self): - self.app = self.layer["app"] - self.portal = self.layer["portal"] - setRoles(self.portal, TEST_USER_ID, ["Member", "Manager"]) - login(self.portal, SITE_OWNER_NAME) - transaction.commit() - - def test_patch_image_redundant_no_event(self): - image_data = "R0lGODlhAQABAIAAAP///wAAACwAAAAAAQABAAACAkQBADs=" - response = requests.post( - self.portal.absolute_url(), - headers={"Accept": "application/json"}, - auth=(SITE_OWNER_NAME, SITE_OWNER_PASSWORD), - json={ - "@type": "Image", - "image": { - "data": image_data, - "encoding": "base64", - "content-type": "image/gif", - "filename": "test.gif" - }, - }, - ) - self.assertEqual(201, response.status_code) - image_url = response.json()["@id"] - transaction.commit() - - # Track ObjectModifiedEvent - sm = getGlobalSiteManager() - fired_events = [] - - def record_event(event): - fired_events.append(event) - - sm.registerHandler(record_event, (IObjectModifiedEvent,)) - - # Patch with same data - response = requests.patch( - image_url, - headers={"Accept": "application/json"}, - auth=(SITE_OWNER_NAME, SITE_OWNER_PASSWORD), - json={ - "image": { - "data": image_data, - "encoding": "base64", - "content-type": "image/gif", - "filename": "test.gif" - }, - }, - ) - self.assertEqual(204, response.status_code) - - # In current Plone, this will be 1 because of identity mismatch - # We want it to be 0 - self.assertEqual(len(fired_events), 0, "ObjectModifiedEvent was fired for redundant PATCH") - - sm.unregisterHandler(record_event, (IObjectModifiedEvent,)) From aaae3d946366944ee0d15eb68fa5aa4a54126893 Mon Sep 17 00:00:00 2001 From: Mikel Larreategi Date: Thu, 25 Jun 2026 12:52:37 +0200 Subject: [PATCH 5/5] whitespace --- src/plone/restapi/tests/test_content_patch.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/plone/restapi/tests/test_content_patch.py b/src/plone/restapi/tests/test_content_patch.py index 1483746ff..f01b4ff5e 100644 --- a/src/plone/restapi/tests/test_content_patch.py +++ b/src/plone/restapi/tests/test_content_patch.py @@ -20,7 +20,6 @@ import transaction import unittest - HAS_PLONE_62 = getattr( import_module("Products.CMFPlone.factory"), "PLONE62MARKER", False )