From af7bd18a9b064c0e36345ea4b79a2c7119349483 Mon Sep 17 00:00:00 2001 From: Andrea Cecchi Date: Thu, 5 Dec 2024 09:13:33 +0100 Subject: [PATCH 1/7] move patches in separate file and patch absolutize_path method --- CHANGES.rst | 5 +- setup.py | 1 + src/redturtle/volto/__init__.py | 73 +------- src/redturtle/volto/patches.py | 169 ++++++++++++++++++ .../volto/restapi/services/search/get.py | 5 +- 5 files changed, 179 insertions(+), 74 deletions(-) create mode 100644 src/redturtle/volto/patches.py diff --git a/CHANGES.rst b/CHANGES.rst index d193557..0087d3e 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -4,7 +4,10 @@ Changelog 5.6.4 (unreleased) ------------------ -- Nothing changed yet. +- Add experimental.noacquisition as dependency. + [cekk] +- Patch absolutize_path method to disable acquisition when checking aliases. + [cekk] 5.6.3 (2024-12-02) diff --git a/setup.py b/setup.py index 8ed820e..a652160 100644 --- a/setup.py +++ b/setup.py @@ -63,6 +63,7 @@ "Products.PortalTransforms>=3.2.0", "collective.volto.sitesettings", "z3c.jbot", + "experimental.noacquisition", ], extras_require={ "advancedquery": [ diff --git a/src/redturtle/volto/__init__.py b/src/redturtle/volto/__init__.py index 86397e0..a4cd5e1 100644 --- a/src/redturtle/volto/__init__.py +++ b/src/redturtle/volto/__init__.py @@ -1,77 +1,6 @@ # -*- coding: utf-8 -*- -"""Init and utils.""" -from plone.app.content.browser.vocabulary import PERMISSIONS -from plone.folder.nogopip import GopipIndex -from Products.ZCatalog.Catalog import Catalog -from redturtle.volto.catalogplan import Catalog_sorted_search_indexes +from redturtle.volto import patches # noqa from zope.i18nmessageid import MessageFactory -from ZTUtils.Lazy import LazyCat -from ZTUtils.Lazy import LazyMap - -import logging - - -logger = logging.getLogger(__name__) _ = MessageFactory("redturtle.volto") - -PERMISSIONS["plone.app.vocabularies.Keywords"] = "View" - -# CATALOG PATCHES - -logger.info( - "install monkey patch for Products.ZCatalog.Catalog.Catalog._sorted_search_indexes #### WORK IN PROGRESS ####" -) -Catalog._orig_sorted_search_indexes = Catalog._sorted_search_indexes -Catalog._sorted_search_indexes = Catalog_sorted_search_indexes - -MAX_SORTABLE = 5000 - - -def Catalog_sortResults( - self, - rs, - sort_index, - reverse=False, - limit=None, - merge=True, - actual_result_count=None, - b_start=0, - b_size=None, -): - if MAX_SORTABLE > 0: - if actual_result_count is None: - actual_result_count = len(rs) - if actual_result_count >= MAX_SORTABLE and isinstance(sort_index, GopipIndex): - logger.warning( - "too many results %s disable GopipIndex sorting", actual_result_count - ) - switched_reverse = bool( - b_size and b_start and b_start > actual_result_count / 2 - ) - if hasattr(rs, "keys"): - sequence, slen = self._limit_sequence( - rs.keys(), actual_result_count, b_start, b_size, switched_reverse - ) - return LazyMap( - self.__getitem__, - sequence, - len(sequence), - actual_result_count=actual_result_count, - ) - else: - logger.error( - "too many results %s disable GopipIndex sorting results %s has no key", - actual_result_count, - type(rs), - ) - return LazyCat([], 0, actual_result_count) - return self._orig_sortResults( - rs, sort_index, reverse, limit, merge, actual_result_count, b_start, b_size - ) - - -logger.info("install monkey patch for Products.ZCatalog.Catalog.Catalog.sortResults") -Catalog._orig_sortResults = Catalog.sortResults -Catalog.sortResults = Catalog_sortResults diff --git a/src/redturtle/volto/patches.py b/src/redturtle/volto/patches.py new file mode 100644 index 0000000..b0ef529 --- /dev/null +++ b/src/redturtle/volto/patches.py @@ -0,0 +1,169 @@ +# These are patches not managed by collective.monkeypatcher +from Products.CMFPlone.controlpanel.browser import redirects +from plone.app.redirector.interfaces import IRedirectionStorage +from Products.CMFCore.utils import getToolByName +from urllib.parse import urlparse +from zope.component import getUtility +from zope.component.hooks import getSite +from zope.i18nmessageid import MessageFactory +from plone.app.content.browser.vocabulary import PERMISSIONS +from plone.folder.nogopip import GopipIndex +from Products.ZCatalog.Catalog import Catalog +from redturtle.volto.catalogplan import Catalog_sorted_search_indexes +from ZTUtils.Lazy import LazyCat +from ZTUtils.Lazy import LazyMap +from experimental.noacquisition import config + + +import logging + +logger = logging.getLogger(__name__) + +_ = MessageFactory("plone") + + +def absolutize_path_patched(path, is_source=True): + """Create path including the path of the portal root. + + The path must be absolute, so starting with a slash. + Or it can be a full url. + + If is_source is true, this is an alternative url + that will point to a target (unknown here). + + If is_source is true, path is the path of a target. + An object must exist at this path, unless it is a full url. + + Return a 2-tuple: (absolute redirection path, + an error message if something goes wrong and otherwise ''). + """ + + portal = getSite() + err = None + is_external_url = False + if not path: + if is_source: + err = _("You have to enter an alternative url.") + else: + err = _("You have to enter a target.") + elif not path.startswith("/"): + if is_source: + err = _("Alternative url path must start with a slash.") + else: + # For targets, we accept external urls. + # Do basic check. + parsed = urlparse(path) + if parsed.scheme in ("https", "http") and parsed.netloc: + is_external_url = True + else: + err = _("Target path must start with a slash.") + elif "@@" in path: + if is_source: + err = _("Alternative url path must not be a view.") + else: + err = _("Target path must not be a view.") + else: + context_path = "/".join(portal.getPhysicalPath()) + path = f"{context_path}{path}" + if not err and not is_external_url: + catalog = getToolByName(portal, "portal_catalog") + if is_source: + # Check whether already exists in storage + storage = getUtility(IRedirectionStorage) + if storage.get(path): + err = _("The provided alternative url already exists!") + else: + # Check whether obj exists at source path. + # A redirect would be useless then. + + ### THIS IS THE PATCH ### + # unrestrictedTraverse returns the object with acquisition, so if + # you have a content with path /Plone/aaa and try to call unrestrictedTraverse + # with /Plone/aaa/aaa/aaa/aaa it will always return /Plone/aaa object + # and this is not correct because we could want to create an alias for /Plone/aaa + # that is /Plone/aaa/aaa + # In Plone7 this will not be a problem anymore because of this: + # https://github.com/plone/Products.CMFPlone/issues/3871 + item = portal.unrestrictedTraverse(path, None) + # if item is not None: this is the original check + if item is not None and "/".join(item.getPhysicalPath()) == path: + # if paths are different, it's an acquisition false positive, + # so go on and let create the alias + err = _("Cannot use a working path as alternative url.") + ### END OF THE PATCH ### + else: + # Check whether obj exists at target path + result = catalog.searchResults(path={"query": path}) + if len(result) == 0: + err = _("The provided target object does not exist.") + + return path, err + + +MAX_SORTABLE = 5000 + + +def Catalog_sortResults( + self, + rs, + sort_index, + reverse=False, + limit=None, + merge=True, + actual_result_count=None, + b_start=0, + b_size=None, +): + if MAX_SORTABLE > 0: + if actual_result_count is None: + actual_result_count = len(rs) + if actual_result_count >= MAX_SORTABLE and isinstance(sort_index, GopipIndex): + logger.warning( + "too many results %s disable GopipIndex sorting", actual_result_count + ) + switched_reverse = bool( + b_size and b_start and b_start > actual_result_count / 2 + ) + if hasattr(rs, "keys"): + sequence, slen = self._limit_sequence( + rs.keys(), actual_result_count, b_start, b_size, switched_reverse + ) + return LazyMap( + self.__getitem__, + sequence, + len(sequence), + actual_result_count=actual_result_count, + ) + else: + logger.error( + "too many results %s disable GopipIndex sorting results %s has no key", + actual_result_count, + type(rs), + ) + return LazyCat([], 0, actual_result_count) + return self._orig_sortResults( + rs, sort_index, reverse, limit, merge, actual_result_count, b_start, b_size + ) + + +# apply patches +logger.info( + "install monkey patch for Products.ZCatalog.Catalog.Catalog._sorted_search_indexes #### WORK IN PROGRESS ####" +) +Catalog._orig_sorted_search_indexes = Catalog._sorted_search_indexes +Catalog._sorted_search_indexes = Catalog_sorted_search_indexes + +logger.info("install monkey patch for Products.ZCatalog.Catalog.Catalog.sortResults") +Catalog._orig_sortResults = Catalog.sortResults +Catalog.sortResults = Catalog_sortResults + +logger.info("install monkey patch for plone.app.content.browser.vocabulary.PERMISSIONS") +PERMISSIONS["plone.app.vocabularies.Keywords"] = "View" + +logger.info( + "install monkey patch for from Products.CMFPlone.controlpanel.browser.redirects.absolutize_path" +) +redirects.absolutize_path = absolutize_path_patched + +logger.info("enable experimental.noacquisition") +# config.DRYRUN = False diff --git a/src/redturtle/volto/restapi/services/search/get.py b/src/redturtle/volto/restapi/services/search/get.py index 5ba222f..dc90864 100644 --- a/src/redturtle/volto/restapi/services/search/get.py +++ b/src/redturtle/volto/restapi/services/search/get.py @@ -4,12 +4,15 @@ from plone.restapi.search.handler import SearchHandler as OriginalHandler from plone.restapi.search.utils import unflatten_dotted_dict from plone.restapi.services import Service -from redturtle.volto import logger from redturtle.volto.config import MAX_LIMIT from redturtle.volto.interfaces import IRedTurtleVoltoSettings from zope.component import getMultiAdapter +import logging + +logger = logging.getLogger(__name__) + # search for 'ranking' in 'SearchableText' and rank very high # when the term is in 'Subject' and high when it is in 'Title'. # print the id and the normalized rank From 66c11670694c7c35b53495219202bfbce2b08964 Mon Sep 17 00:00:00 2001 From: Andrea Cecchi Date: Thu, 5 Dec 2024 09:39:59 +0100 Subject: [PATCH 2/7] enable experimental.noacquisition --- src/redturtle/volto/patches.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/redturtle/volto/patches.py b/src/redturtle/volto/patches.py index b0ef529..bdd3617 100644 --- a/src/redturtle/volto/patches.py +++ b/src/redturtle/volto/patches.py @@ -166,4 +166,4 @@ def Catalog_sortResults( redirects.absolutize_path = absolutize_path_patched logger.info("enable experimental.noacquisition") -# config.DRYRUN = False +config.DRYRUN = False From ada901f152f0e42c936d274148a5fa2d738223d5 Mon Sep 17 00:00:00 2001 From: Andrea Cecchi Date: Thu, 5 Dec 2024 09:40:31 +0100 Subject: [PATCH 3/7] enable experimental.noacquisition --- src/redturtle/volto/patches.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/redturtle/volto/patches.py b/src/redturtle/volto/patches.py index bdd3617..b45d535 100644 --- a/src/redturtle/volto/patches.py +++ b/src/redturtle/volto/patches.py @@ -76,7 +76,7 @@ def absolutize_path_patched(path, is_source=True): # Check whether obj exists at source path. # A redirect would be useless then. - ### THIS IS THE PATCH ### + # THIS IS THE PATCH # unrestrictedTraverse returns the object with acquisition, so if # you have a content with path /Plone/aaa and try to call unrestrictedTraverse # with /Plone/aaa/aaa/aaa/aaa it will always return /Plone/aaa object @@ -90,7 +90,7 @@ def absolutize_path_patched(path, is_source=True): # if paths are different, it's an acquisition false positive, # so go on and let create the alias err = _("Cannot use a working path as alternative url.") - ### END OF THE PATCH ### + # END OF PATCH else: # Check whether obj exists at target path result = catalog.searchResults(path={"query": path}) From 96ab8fed585baff83cf69cc788ac3ae2019bb9b4 Mon Sep 17 00:00:00 2001 From: Andrea Cecchi Date: Thu, 5 Dec 2024 09:41:31 +0100 Subject: [PATCH 4/7] isort --- src/redturtle/volto/patches.py | 14 +++++++------- src/redturtle/volto/restapi/services/search/get.py | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/redturtle/volto/patches.py b/src/redturtle/volto/patches.py index b45d535..6d91618 100644 --- a/src/redturtle/volto/patches.py +++ b/src/redturtle/volto/patches.py @@ -1,22 +1,22 @@ # These are patches not managed by collective.monkeypatcher -from Products.CMFPlone.controlpanel.browser import redirects +from experimental.noacquisition import config +from plone.app.content.browser.vocabulary import PERMISSIONS from plone.app.redirector.interfaces import IRedirectionStorage +from plone.folder.nogopip import GopipIndex from Products.CMFCore.utils import getToolByName +from Products.CMFPlone.controlpanel.browser import redirects +from Products.ZCatalog.Catalog import Catalog +from redturtle.volto.catalogplan import Catalog_sorted_search_indexes from urllib.parse import urlparse from zope.component import getUtility from zope.component.hooks import getSite from zope.i18nmessageid import MessageFactory -from plone.app.content.browser.vocabulary import PERMISSIONS -from plone.folder.nogopip import GopipIndex -from Products.ZCatalog.Catalog import Catalog -from redturtle.volto.catalogplan import Catalog_sorted_search_indexes from ZTUtils.Lazy import LazyCat from ZTUtils.Lazy import LazyMap -from experimental.noacquisition import config - import logging + logger = logging.getLogger(__name__) _ = MessageFactory("plone") diff --git a/src/redturtle/volto/restapi/services/search/get.py b/src/redturtle/volto/restapi/services/search/get.py index dc90864..9de593a 100644 --- a/src/redturtle/volto/restapi/services/search/get.py +++ b/src/redturtle/volto/restapi/services/search/get.py @@ -8,9 +8,9 @@ from redturtle.volto.interfaces import IRedTurtleVoltoSettings from zope.component import getMultiAdapter - import logging + logger = logging.getLogger(__name__) # search for 'ranking' in 'SearchableText' and rank very high From 6019fc3c50eb312d98d3df38be7c8b3f7a5f5408 Mon Sep 17 00:00:00 2001 From: Andrea Cecchi Date: Thu, 5 Dec 2024 10:00:55 +0100 Subject: [PATCH 5/7] add tests --- src/redturtle/volto/testing.py | 3 ++ .../volto/tests/test_noacquisition.py | 39 +++++++++++++++++++ .../volto/tests/test_patch_absolutize_path.py | 32 +++++++++++++++ 3 files changed, 74 insertions(+) create mode 100644 src/redturtle/volto/tests/test_noacquisition.py create mode 100644 src/redturtle/volto/tests/test_patch_absolutize_path.py diff --git a/src/redturtle/volto/testing.py b/src/redturtle/volto/testing.py index c7719e8..b0ca7dc 100644 --- a/src/redturtle/volto/testing.py +++ b/src/redturtle/volto/testing.py @@ -10,6 +10,7 @@ import collective.volto.gdprcookie import collective.volto.sitesettings +import experimental.noacquisition import kitconcept.seo import plone.app.caching import plone.restapi @@ -31,6 +32,7 @@ def setUpZope(self, app, configurationContext): self.loadZCML(package=plone.volto) self.loadZCML(package=plone.app.caching) self.loadZCML(package=kitconcept.seo) + self.loadZCML(package=experimental.noacquisition) def setUpPloneSite(self, portal): applyProfile(portal, "plone.app.caching:default") @@ -75,6 +77,7 @@ def setUpZope(self, app, configurationContext): self.loadZCML(package=redturtle.volto) self.loadZCML(package=plone.app.caching) self.loadZCML(package=kitconcept.seo) + self.loadZCML(package=experimental.noacquisition) def setUpPloneSite(self, portal): applyProfile(portal, "plone.app.caching:default") diff --git a/src/redturtle/volto/tests/test_noacquisition.py b/src/redturtle/volto/tests/test_noacquisition.py new file mode 100644 index 0000000..3799f3e --- /dev/null +++ b/src/redturtle/volto/tests/test_noacquisition.py @@ -0,0 +1,39 @@ +from plone import api +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 RelativeSession +from redturtle.volto.testing import REDTURTLE_VOLTO_API_FUNCTIONAL_TESTING +from transaction import commit + +import unittest + + +class TestNoAcquisition(unittest.TestCase): + layer = REDTURTLE_VOLTO_API_FUNCTIONAL_TESTING + + def setUp(self): + self.app = self.layer["app"] + self.portal = self.layer["portal"] + self.request = self.layer["request"] + self.portal_url = self.portal.absolute_url() + setRoles(self.portal, TEST_USER_ID, ["Manager"]) + + self.api_session = RelativeSession(self.portal_url) + self.api_session.headers.update({"Accept": "application/json"}) + self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD) + + self.document = api.content.create( + container=self.portal, type="Document", title="aaa" + ) + + self.child = api.content.create( + container=self.document, type="Document", title="bbb" + ) + + commit() + + def test_with_noacquisition_enabled_get_not_found(self): + response = self.api_session.get(f"{self.document.absolute_url()}/aaa/aaa/bbb") + self.assertEqual(response.status_code, 404) diff --git a/src/redturtle/volto/tests/test_patch_absolutize_path.py b/src/redturtle/volto/tests/test_patch_absolutize_path.py new file mode 100644 index 0000000..ecfefaf --- /dev/null +++ b/src/redturtle/volto/tests/test_patch_absolutize_path.py @@ -0,0 +1,32 @@ +# -*- coding: utf-8 -*- +"""Setup tests for this package.""" +from plone import api +from plone.app.testing import setRoles +from plone.app.testing import TEST_USER_ID +from Products.CMFPlone.controlpanel.browser.redirects import absolutize_path +from redturtle.volto.testing import REDTURTLE_VOLTO_INTEGRATION_TESTING + +import unittest + + +class TestAbsolutizePath(unittest.TestCase): + """ """ + + layer = REDTURTLE_VOLTO_INTEGRATION_TESTING + + def setUp(self): + self.app = self.layer["app"] + self.portal = self.layer["portal"] + self.portal_url = self.portal.absolute_url() + setRoles(self.portal, TEST_USER_ID, ["Manager"]) + + self.foo = api.content.create( + container=self.portal, + type="Document", + title="Foo", + ) + + def test_patched_method_allows_to_create_alias_with_same_path(self): + # by default will return + # ('/plone/foo/foo', 'Cannot use a working path as alternative url.') + self.assertEqual(absolutize_path("/foo/foo"), ("/plone/foo/foo", None)) From ba88efa6c8cb1429d10293ae4149d68eb0688684 Mon Sep 17 00:00:00 2001 From: Andrea Cecchi Date: Thu, 5 Dec 2024 10:37:18 +0100 Subject: [PATCH 6/7] disable test for plone < 6 --- src/redturtle/volto/tests/test_blocks_linkintegrity.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/redturtle/volto/tests/test_blocks_linkintegrity.py b/src/redturtle/volto/tests/test_blocks_linkintegrity.py index e219c67..579e85f 100644 --- a/src/redturtle/volto/tests/test_blocks_linkintegrity.py +++ b/src/redturtle/volto/tests/test_blocks_linkintegrity.py @@ -1,4 +1,5 @@ # -*- coding: utf-8 -*- +from importlib import import_module from plone import api from plone.app.testing import setRoles from plone.app.testing import TEST_USER_ID @@ -9,6 +10,10 @@ import unittest +HAS_PLONE_6 = getattr( + import_module("Products.CMFPlone.factory"), "PLONE60MARKER", False +) + class TestBlocksLinkIntegrity(unittest.TestCase): layer = REDTURTLE_VOLTO_FUNCTIONAL_TESTING @@ -704,6 +709,10 @@ def test_count_down_countdown_text_link_integrity(self): self.assertEqual(reference["sources"][0]["uid"], self.document.UID()) self.assertEqual(reference["target"]["uid"], self.ref.UID()) + @unittest.skipIf( + not HAS_PLONE_6, + "This test is only intended to run for Plone 6 and DX site root enabled", + ) def test_linkintegrity_works_also_on_site_root(self): self.assertEqual(self.get_references(), []) self.portal.blocks = { From 8db060c324f9c48b4d8304d5b05410f0efcc69a7 Mon Sep 17 00:00:00 2001 From: Andrea Cecchi Date: Thu, 5 Dec 2024 10:54:48 +0100 Subject: [PATCH 7/7] isort --- src/redturtle/volto/tests/test_blocks_linkintegrity.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/redturtle/volto/tests/test_blocks_linkintegrity.py b/src/redturtle/volto/tests/test_blocks_linkintegrity.py index 579e85f..39a3f76 100644 --- a/src/redturtle/volto/tests/test_blocks_linkintegrity.py +++ b/src/redturtle/volto/tests/test_blocks_linkintegrity.py @@ -10,6 +10,7 @@ import unittest + HAS_PLONE_6 = getattr( import_module("Products.CMFPlone.factory"), "PLONE60MARKER", False )