From 5bead9f44c78fe2d3e9d651e11e46bf8e0efc200 Mon Sep 17 00:00:00 2001 From: ale-rt Date: Wed, 18 May 2022 13:27:27 +0200 Subject: [PATCH] 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 ac95233e..a0f7e1c5 100644 --- a/setup.py +++ b/setup.py @@ -34,6 +34,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 777b503a..8cda9bce 100644 --- a/src/plone/api/content.py +++ b/src/plone/api/content.py @@ -8,8 +8,13 @@ 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.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 @@ -18,7 +23,6 @@ from zope.interface import providedBy import random -import transaction _marker = [] @@ -27,13 +31,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 @@ -61,14 +60,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 @@ -88,20 +112,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 03200cf6..89c357c3 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