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/src/plone/api/content.py b/src/plone/api/content.py index f0a9c850..0ddbf699 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 @@ -8,9 +9,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 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 ComponentLookupError from zope.component import getMultiAdapter from zope.component import getSiteManager @@ -20,7 +25,6 @@ from zope.interface import providedBy import random -import transaction _marker = [] @@ -63,14 +67,50 @@ 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 + if not fti.isConstructionAllowed(container): + raise Unauthorized(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( + f"Subobject type disallowed by IConstrainTypes adapter: {type}" + ) + if id: + kwargs["id"] = id + content = createContent(type, **kwargs) + + 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) except UnicodeDecodeError: # UnicodeDecodeError is a subclass of ValueError, # so will be swallowed below unless we re-raise it here @@ -92,20 +132,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 44109d95..222092bd 100644 --- a/src/plone/api/tests/test_content.py +++ b/src/plone/api/tests/test_content.py @@ -1,6 +1,7 @@ """Tests for plone.api.content.""" from Acquisition import aq_base +from contextlib import AbstractContextManager from OFS.CopySupport import CopyError from OFS.event import ObjectWillBeMovedEvent from OFS.interfaces import IObjectWillBeMovedEvent @@ -21,6 +22,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 +32,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 +83,6 @@ def setUp(self): id="events", container=self.portal, ) - self.team = api.content.create( container=self.about, type="Document", @@ -252,6 +268,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 @@ -325,6 +379,26 @@ 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_anonymous_unauthorized(self): + from AccessControl import Unauthorized + from plone.app.testing import logout + + logout() + with self.assertRaises(Unauthorized): + 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()