From b07aad729622dd0977e509a5e817aa8a8610b562 Mon Sep 17 00:00:00 2001 From: Dylan Jay Date: Fri, 24 Nov 2023 10:05:59 +0700 Subject: [PATCH 1/4] add recordsproxy to controlpanel serializer so it handles schemas with new fields by returning field default --- .../serializer/controlpanels/__init__.py | 47 ++++++++++++++++++- .../tests/test_services_controlpanels.py | 41 +++++++++++++++- 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/src/plone/restapi/serializer/controlpanels/__init__.py b/src/plone/restapi/serializer/controlpanels/__init__.py index addd732468..87389d30d5 100644 --- a/src/plone/restapi/serializer/controlpanels/__init__.py +++ b/src/plone/restapi/serializer/controlpanels/__init__.py @@ -1,5 +1,7 @@ from plone.dexterity.interfaces import IDexterityContent from plone.registry.interfaces import IRegistry +from plone.registry.interfaces import IRecordsProxy +from plone.registry.recordsproxy import RecordsProxy from plone.restapi.controlpanels import IControlpanel from plone.restapi.interfaces import IFieldSerializer from plone.restapi.interfaces import ISerializeToJson @@ -12,12 +14,15 @@ from zope.interface import alsoProvides from zope.interface import implementer from zope.interface import noLongerProvides +from zope.schema.interfaces import IField import zope.schema SERVICE_ID = "@controlpanels" +_marker = object() + @implementer(ISerializeToJsonSummary) @adapter(IControlpanel) @@ -82,7 +87,14 @@ def __call__(self): self.controlpanel, self.controlpanel.context, self.controlpanel.request ) - proxy = self.registry.forInterface(self.schema, prefix=self.schema_prefix) + # We use a special proxy and check=False just in case the schema has a new field before an upgrade happens + # Note this doesn't yet handle if a schema field changes type, or the options change + proxy = self.registry.forInterface( + self.schema, + prefix=self.schema_prefix, + check=False, + factory=DefaultRecordsProxy, + ) # Temporarily provide IDexterityContent, so we can use DX field # serializers @@ -113,3 +125,36 @@ def __call__(self): "schema": json_schema, "data": json_data, } + + +@implementer(IRecordsProxy) +class DefaultRecordsProxy(RecordsProxy): + """Modified RecordsProxy which returns defaults if values missing""" + + def __getattr__(self, name): + if not self.__dict__ or name in self.__dict__.keys(): + return super(RecordsProxy, self).__getattr__(name) + if name not in self.__schema__: + raise AttributeError(name) + value = self.__registry__.get(self.__prefix__ + name, _marker) + if value is _marker: + # Instead of returning missing_value we return the default + # so if this is an upgrade the registry will eventually be + # set with the default on next save. + field = self.__schema__[name] + if IField.providedBy(field): + field = field.bind(self) + value = field.default + else: + value = self.__schema__[name].default + + return value + + def __setattr__(self, name, value): + if name in self.__schema__: + full_name = self.__prefix__ + name + # if full_name not in self.__registry__: + # raise AttributeError(name) + self.__registry__[full_name] = value + else: + self.__dict__[name] = value diff --git a/src/plone/restapi/tests/test_services_controlpanels.py b/src/plone/restapi/tests/test_services_controlpanels.py index f23affcca5..7a332255de 100644 --- a/src/plone/restapi/tests/test_services_controlpanels.py +++ b/src/plone/restapi/tests/test_services_controlpanels.py @@ -4,12 +4,16 @@ from plone.app.testing import TEST_USER_ID from plone.restapi.testing import PLONE_RESTAPI_DX_FUNCTIONAL_TESTING from plone.restapi.testing import RelativeSession +from zope import schema +from zope.component import getUtility +from plone.registry.interfaces import IRegistry +import transaction + import unittest class TestControlpanelsEndpoint(unittest.TestCase): - layer = PLONE_RESTAPI_DX_FUNCTIONAL_TESTING def setUp(self): @@ -119,3 +123,38 @@ def test_get_usergroup_control_panel(self): # This control panel does not exist in Plone 5 response = self.api_session.get("/@controlpanels/usergroup") self.assertEqual(200, response.status_code) + + def test_get_schema_with_new_field(self): + # simulate startup with a change registry schema to ensure it doesn't break + + registry = getUtility(IRegistry) + registry.records._values[ + "plone.ext_editor" + ] = True # ensure it's not the default + transaction.commit() + + response = self.api_session.get("/@controlpanels/editing") + old_data = response.json()["data"] + self.assertEquals(old_data["ext_editor"], True) + + # It's too hard to add another field so lets delete the registry data to + # simulate what it's like starting when the schema has a field and no + # data in the registry for it + # del registry.records['plone.available_editors'] + del registry.records["plone.ext_editor"] + transaction.commit() + + response = self.api_session.get("/@controlpanels/editing") + old_data = response.json()["data"] + self.assertEquals(old_data["ext_editor"], False) + + # ensure there is no problem trying to set missing registry entries + new_values = { + "ext_editor": not old_data["ext_editor"], + "lock_on_ttw_edit": not old_data["lock_on_ttw_edit"], + } + response = self.api_session.patch("/@controlpanels/editing", json=new_values) + + # check if the values changed + response = self.api_session.get("/@controlpanels/editing") + self.assertNotEqual(response.json(), old_data) From 41acc1994cb0fef85f86d56faeb30b5aad8ed0b0 Mon Sep 17 00:00:00 2001 From: Dylan Jay Date: Fri, 24 Nov 2023 10:26:03 +0700 Subject: [PATCH 2/4] pep8 --- src/plone/restapi/serializer/controlpanels/__init__.py | 2 -- src/plone/restapi/tests/test_services_controlpanels.py | 1 - 2 files changed, 3 deletions(-) diff --git a/src/plone/restapi/serializer/controlpanels/__init__.py b/src/plone/restapi/serializer/controlpanels/__init__.py index 87389d30d5..9e722c4cf7 100644 --- a/src/plone/restapi/serializer/controlpanels/__init__.py +++ b/src/plone/restapi/serializer/controlpanels/__init__.py @@ -153,8 +153,6 @@ def __getattr__(self, name): def __setattr__(self, name, value): if name in self.__schema__: full_name = self.__prefix__ + name - # if full_name not in self.__registry__: - # raise AttributeError(name) self.__registry__[full_name] = value else: self.__dict__[name] = value diff --git a/src/plone/restapi/tests/test_services_controlpanels.py b/src/plone/restapi/tests/test_services_controlpanels.py index 7a332255de..e91c9f3572 100644 --- a/src/plone/restapi/tests/test_services_controlpanels.py +++ b/src/plone/restapi/tests/test_services_controlpanels.py @@ -4,7 +4,6 @@ from plone.app.testing import TEST_USER_ID from plone.restapi.testing import PLONE_RESTAPI_DX_FUNCTIONAL_TESTING from plone.restapi.testing import RelativeSession -from zope import schema from zope.component import getUtility from plone.registry.interfaces import IRegistry import transaction From 5edc89e958b9284b9c1e6dba24a1fc4e43ab937c Mon Sep 17 00:00:00 2001 From: Dylan Jay Date: Fri, 24 Nov 2023 11:26:25 +0700 Subject: [PATCH 3/4] fix py3.12 test error --- src/plone/restapi/tests/test_services_controlpanels.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/plone/restapi/tests/test_services_controlpanels.py b/src/plone/restapi/tests/test_services_controlpanels.py index e91c9f3572..1a679d1621 100644 --- a/src/plone/restapi/tests/test_services_controlpanels.py +++ b/src/plone/restapi/tests/test_services_controlpanels.py @@ -134,7 +134,7 @@ def test_get_schema_with_new_field(self): response = self.api_session.get("/@controlpanels/editing") old_data = response.json()["data"] - self.assertEquals(old_data["ext_editor"], True) + self.assertEqual(old_data["ext_editor"], True) # It's too hard to add another field so lets delete the registry data to # simulate what it's like starting when the schema has a field and no @@ -145,12 +145,11 @@ def test_get_schema_with_new_field(self): response = self.api_session.get("/@controlpanels/editing") old_data = response.json()["data"] - self.assertEquals(old_data["ext_editor"], False) + self.assertEqual(old_data["ext_editor"], False) # ensure there is no problem trying to set missing registry entries new_values = { "ext_editor": not old_data["ext_editor"], - "lock_on_ttw_edit": not old_data["lock_on_ttw_edit"], } response = self.api_session.patch("/@controlpanels/editing", json=new_values) From 0f345da66bdf6acea2f3f5d477b53a1f79d58b6a Mon Sep 17 00:00:00 2001 From: Dylan Jay Date: Wed, 29 Nov 2023 08:20:27 +0700 Subject: [PATCH 4/4] use missing value instead --- src/plone/restapi/serializer/controlpanels/__init__.py | 2 +- src/plone/restapi/tests/test_services_controlpanels.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plone/restapi/serializer/controlpanels/__init__.py b/src/plone/restapi/serializer/controlpanels/__init__.py index 9e722c4cf7..2bfff821b3 100644 --- a/src/plone/restapi/serializer/controlpanels/__init__.py +++ b/src/plone/restapi/serializer/controlpanels/__init__.py @@ -93,7 +93,7 @@ def __call__(self): self.schema, prefix=self.schema_prefix, check=False, - factory=DefaultRecordsProxy, + # factory=DefaultRecordsProxy, ) # Temporarily provide IDexterityContent, so we can use DX field diff --git a/src/plone/restapi/tests/test_services_controlpanels.py b/src/plone/restapi/tests/test_services_controlpanels.py index 1a679d1621..d3ebc960b7 100644 --- a/src/plone/restapi/tests/test_services_controlpanels.py +++ b/src/plone/restapi/tests/test_services_controlpanels.py @@ -145,7 +145,7 @@ def test_get_schema_with_new_field(self): response = self.api_session.get("/@controlpanels/editing") old_data = response.json()["data"] - self.assertEqual(old_data["ext_editor"], False) + self.assertEqual(old_data["ext_editor"], None) # ensure there is no problem trying to set missing registry entries new_values = {