From 5ec99d9ffe15ca96c04ac48364dae93c95ece953 Mon Sep 17 00:00:00 2001 From: ale-rt Date: Wed, 18 May 2022 13:27:27 +0200 Subject: [PATCH 1/4] Use the dexterity machinery in api.content.create Fixes #439 --- news/439.changed | 1 + setup.py | 1 + src/plone/api/content.py | 60 +++++++++++++++++------------ src/plone/api/tests/test_content.py | 58 +++++++++++++++++++++++++++- 4 files changed, 94 insertions(+), 26 deletions(-) create mode 100644 news/439.changed diff --git a/news/439.changed b/news/439.changed new file mode 100644 index 00000000..59df0a45 --- /dev/null +++ b/news/439.changed @@ -0,0 +1 @@ +When creating dexterity object with api.content.create use the dexterity machinery to not have the object renamed after it is initially created. diff --git a/setup.py b/setup.py index 1f356a9c..9cbe4752 100644 --- a/setup.py +++ b/setup.py @@ -35,6 +35,7 @@ def read(*rnames): "decorator", "plone.app.uuid", "plone.app.linkintegrity", + "plone.dexterity", "plone.uuid", "setuptools", "zope.globalrequest", diff --git a/src/plone/api/content.py b/src/plone/api/content.py index c578d6b3..66ce4ed3 100644 --- a/src/plone/api/content.py +++ b/src/plone/api/content.py @@ -8,9 +8,14 @@ from plone.api.validation import required_parameters from plone.app.linkintegrity.exceptions import LinkIntegrityNotificationException from plone.app.uuid.utils import uuidToObject +from plone.dexterity.fti import DexterityFTI +from plone.dexterity.utils import addContentToContainer +from plone.dexterity.utils import createContent from plone.uuid.interfaces import IUUID from Products.CMFCore.DynamicType import DynamicType from Products.CMFCore.WorkflowCore import WorkflowException +from Products.CMFPlone.interfaces import IConstrainTypes +from zExceptions import BadRequest from zope.component import getMultiAdapter from zope.component import getSiteManager from zope.container.interfaces import INameChooser @@ -19,7 +24,6 @@ from zope.interface import providedBy import random -import transaction _marker = [] @@ -28,13 +32,8 @@ @required_parameters("container", "type") @at_least_one_of("id", "title") def create( - container=None, - type=None, - id=None, - title=None, - safe_id=False, - **kwargs # NOQA: C816, S101 -): + container=None, type=None, id=None, title=None, safe_id=False, **kwargs +): # NOQA: S101 """Create a new content item. :param container: [required] Container object in which to create the new @@ -62,14 +61,39 @@ def create( :class:`~plone.api.exc.InvalidParameterError` :Example: :ref:`content-create-example` """ - # Create a temporary id if the id is not given - content_id = not safe_id and id or str(random.randint(0, 99999999)) + fti = portal.get_tool("portal_types").get(type) if title: kwargs["title"] = title try: - container.invokeFactory(type, content_id, **kwargs) + if isinstance(fti, DexterityFTI): + # For dexterity objects we want to not use the invokeFactory + # method because we want to have the id generated by the name chooser + constraints = IConstrainTypes(container, None) + if constraints and fti not in constraints.allowedContentTypes(): + raise ValueError( + f"Subobject type disallowed by IConstrainTypes adapter: {type}" + ) + if id: + kwargs["id"] = id + content = createContent(type, **kwargs) + + if not content.id: + content.id = INameChooser(container).chooseName(title, content) + if not safe_id: + content.id = INameChooser(container).chooseName( + content.id or title, content + ) + if id and id != content.id: + raise BadRequest( + "The id you provided conflicts with an existing object or it is reserved" + ) + addContentToContainer(container, content) + content_id = content.id + else: + content_id = not safe_id and id or str(random.randint(0, 99999999)) + container.invokeFactory(type, content_id, **kwargs) except UnicodeDecodeError: # UnicodeDecodeError is a subclass of ValueError, # so will be swallowed below unless we re-raise it here @@ -89,20 +113,6 @@ def create( ) content = container[content_id] - if not id or (safe_id and id): - # Create a new id from title - chooser = INameChooser(container) - derived_id = id or title - new_id = chooser.chooseName(derived_id, content) - # kacee: we must do a partial commit, else the renaming fails because - # the object isn't in the zodb. - # Thus if it is not in zodb, there's nothing to move. We should - # choose a correct id when - # the object is created. - # maurits: tests run fine without this though. - transaction.savepoint(optimistic=True) - content.aq_parent.manage_renameObject(content_id, new_id) - return content diff --git a/src/plone/api/tests/test_content.py b/src/plone/api/tests/test_content.py index c02145dd..7c1f2384 100644 --- a/src/plone/api/tests/test_content.py +++ b/src/plone/api/tests/test_content.py @@ -1,6 +1,8 @@ """Tests for plone.api.content.""" from Acquisition import aq_base +from contextlib import AbstractContextManager +from gc import callbacks from OFS.CopySupport import CopyError from OFS.event import ObjectWillBeMovedEvent from OFS.interfaces import IObjectWillBeMovedEvent @@ -11,6 +13,7 @@ from plone.app.linkintegrity.exceptions import LinkIntegrityNotificationException from plone.app.textfield import RichTextValue from plone.indexer import indexer +from plone.namedfile import NamedBlobFile from plone.uuid.interfaces import IMutableUUID from plone.uuid.interfaces import IUUIDGenerator from Products.CMFCore.interfaces import IContentish @@ -21,6 +24,7 @@ from zope.component import getGlobalSiteManager from zope.component import getUtility from zope.container.contained import ContainerModifiedEvent +from zope.event import subscribers from zope.interface import alsoProvides from zope.lifecycleevent import IObjectModifiedEvent from zope.lifecycleevent import IObjectMovedEvent @@ -30,6 +34,21 @@ import unittest +class EventRecorder(AbstractContextManager): + def __init__(self): + self.events = [] + + def __enter__(self): + subscribers.append(self.record) + return self.events + + def record(self, event): + self.events.append(event.__class__.__name__) + + def __exit__(self, *exc_info): + subscribers.remove(self.record) + + class TestPloneApiContent(unittest.TestCase): """Unit tests for content manipulation using plone.api.""" @@ -66,7 +85,6 @@ def setUp(self): id="events", container=self.portal, ) - self.team = api.content.create( container=self.about, type="Document", @@ -252,6 +270,44 @@ def test_create_dexterity(self): ) self.verify_intids() + def test_create_dexterity_folder_events(self): + """Check the events that are fired when a folder is created. + Ensure the events fired are consistent whether or not an id is passed + and assert that the object is not moved in the creation process. + """ + container = self.portal + + # Create a folder passing an id and record the events + with EventRecorder() as events_id_yes: + foo = api.content.create( + id="foo", + container=container, + title="Bar", + type="Dexterity Folder", + ) + + self.assertEqual(foo.id, "foo") + + # Create a folder not passing an id and record the events + with EventRecorder() as events_id_not: + bar = api.content.create( + container=container, + title="Bar", + type="Dexterity Folder", + ) + + self.assertEqual(bar.id, "bar") + + self.assertListEqual( + events_id_yes, events_id_not, "The events fired should be consistent" + ) + + self.assertNotIn( + "ObjectMovedEvent", + events_id_yes, + "The object should be created in the proper place", + ) + def test_create_content(self): """Test create content.""" container = self.portal From b2e275941714c20299f1a6f6ab971ec8d5e29642 Mon Sep 17 00:00:00 2001 From: ale-rt Date: Wed, 18 May 2022 15:23:05 +0200 Subject: [PATCH 2/4] fixup! Use the dexterity machinery in api.content.create --- src/plone/api/content.py | 36 ++++++++++++++++++----------- src/plone/api/tests/test_content.py | 10 ++++++-- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/src/plone/api/content.py b/src/plone/api/content.py index 66ce4ed3..763bbddb 100644 --- a/src/plone/api/content.py +++ b/src/plone/api/content.py @@ -9,7 +9,6 @@ from plone.app.linkintegrity.exceptions import LinkIntegrityNotificationException from plone.app.uuid.utils import uuidToObject from plone.dexterity.fti import DexterityFTI -from plone.dexterity.utils import addContentToContainer from plone.dexterity.utils import createContent from plone.uuid.interfaces import IUUID from Products.CMFCore.DynamicType import DynamicType @@ -70,6 +69,12 @@ def create( if isinstance(fti, DexterityFTI): # For dexterity objects we want to not use the invokeFactory # method because we want to have the id generated by the name chooser + if not fti.isConstructionAllowed(container): + raise ValueError(f"Cannot create {type}") + + container_fti = container.getTypeInfo() + if container_fti is not None and not container_fti.allowType(type): + raise ValueError(f"Disallowed subobject type: {type}") constraints = IConstrainTypes(container, None) if constraints and fti not in constraints.allowedContentTypes(): raise ValueError( @@ -79,18 +84,23 @@ def create( kwargs["id"] = id content = createContent(type, **kwargs) - if not content.id: - content.id = INameChooser(container).chooseName(title, content) - if not safe_id: - content.id = INameChooser(container).chooseName( - content.id or title, content - ) - if id and id != content.id: - raise BadRequest( - "The id you provided conflicts with an existing object or it is reserved" - ) - addContentToContainer(container, content) - content_id = content.id + name_chooser = INameChooser(container) + if content.id: + # Check that the id we picked is valid + if not name_chooser.checkName(content.id, content): + if safe_id: + content.id = INameChooser(container).chooseName( + content.id, content + ) + else: + raise BadRequest( + "The id you provided conflicts with " + "an existing object or it is reserved" + ) + else: + content.id = name_chooser.chooseName(title, content) + + content_id = container._setObject(content.id, content) else: content_id = not safe_id and id or str(random.randint(0, 99999999)) container.invokeFactory(type, content_id, **kwargs) diff --git a/src/plone/api/tests/test_content.py b/src/plone/api/tests/test_content.py index 7c1f2384..2db7766b 100644 --- a/src/plone/api/tests/test_content.py +++ b/src/plone/api/tests/test_content.py @@ -2,7 +2,6 @@ from Acquisition import aq_base from contextlib import AbstractContextManager -from gc import callbacks from OFS.CopySupport import CopyError from OFS.event import ObjectWillBeMovedEvent from OFS.interfaces import IObjectWillBeMovedEvent @@ -13,7 +12,6 @@ from plone.app.linkintegrity.exceptions import LinkIntegrityNotificationException from plone.app.textfield import RichTextValue from plone.indexer import indexer -from plone.namedfile import NamedBlobFile from plone.uuid.interfaces import IMutableUUID from plone.uuid.interfaces import IUUIDGenerator from Products.CMFCore.interfaces import IContentish @@ -381,6 +379,14 @@ def test_create_with_safe_id(self): self.assertEqual(second_page.id, "test-document-1") self.assertEqual(second_page.portal_type, "Document") + def test_create_with_not_lowercase_id(self): + obj = api.content.create( + container=self.portal, + type="Dexterity Item", + id="Doc1", + ) + self.assertEqual(obj.id, "Doc1") + def test_create_raises_unicodedecodeerror(self): """Test that the create method raises UnicodeDecodeErrors correctly.""" site = getGlobalSiteManager() From 27d5f4419b8a79ef9c22eadda1e6b121976fe636 Mon Sep 17 00:00:00 2001 From: ale-rt Date: Thu, 19 May 2022 10:44:31 +0200 Subject: [PATCH 3/4] Raise Unauthorized when the user has no permission to create the content --- src/plone/api/content.py | 3 ++- src/plone/api/tests/test_content.py | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/plone/api/content.py b/src/plone/api/content.py index 763bbddb..c63d7f6c 100644 --- a/src/plone/api/content.py +++ b/src/plone/api/content.py @@ -1,5 +1,6 @@ """Module that provides functionality for content manipulation.""" +from AccessControl import Unauthorized from copy import copy as _copy from plone.api import portal from plone.api.exc import InvalidParameterError @@ -70,7 +71,7 @@ def create( # For dexterity objects we want to not use the invokeFactory # method because we want to have the id generated by the name chooser if not fti.isConstructionAllowed(container): - raise ValueError(f"Cannot create {type}") + raise Unauthorized(f"Cannot create {type}") container_fti = container.getTypeInfo() if container_fti is not None and not container_fti.allowType(type): diff --git a/src/plone/api/tests/test_content.py b/src/plone/api/tests/test_content.py index 2db7766b..d493cb67 100644 --- a/src/plone/api/tests/test_content.py +++ b/src/plone/api/tests/test_content.py @@ -387,6 +387,17 @@ def test_create_with_not_lowercase_id(self): ) self.assertEqual(obj.id, "Doc1") + def test_create_anonymous_unauthorized(self): + from AccessControl import Unauthorized + from plone.app.testing import logout + logout() + with self.assertRaises(Unauthorized): + obj = api.content.create( + container=self.portal, + type="Dexterity Item", + id="foo", + ) + def test_create_raises_unicodedecodeerror(self): """Test that the create method raises UnicodeDecodeErrors correctly.""" site = getGlobalSiteManager() From 3a186b6ce2ec7cf111ba6c41463a1b321328b299 Mon Sep 17 00:00:00 2001 From: ale-rt Date: Thu, 19 May 2022 11:02:10 +0200 Subject: [PATCH 4/4] fixup! Raise Unauthorized when the user has no permission to create the content --- src/plone/api/tests/test_content.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/plone/api/tests/test_content.py b/src/plone/api/tests/test_content.py index d493cb67..034f1b59 100644 --- a/src/plone/api/tests/test_content.py +++ b/src/plone/api/tests/test_content.py @@ -390,6 +390,7 @@ def test_create_with_not_lowercase_id(self): def test_create_anonymous_unauthorized(self): from AccessControl import Unauthorized from plone.app.testing import logout + logout() with self.assertRaises(Unauthorized): obj = api.content.create(