Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the dexterity machinery in api.content.create #484

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/439.changed
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ def read(*rnames):
"decorator",
"plone.app.uuid",
"plone.app.linkintegrity",
"plone.dexterity",
"plone.uuid",
"setuptools",
"zope.globalrequest",
Expand Down
71 changes: 46 additions & 25 deletions src/plone/api/content.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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 getMultiAdapter
from zope.component import getSiteManager
from zope.container.interfaces import INameChooser
Expand All @@ -19,7 +24,6 @@
from zope.interface import providedBy

import random
import transaction


_marker = []
Expand All @@ -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
Expand Down Expand Up @@ -62,14 +61,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
Expand All @@ -89,20 +124,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


Expand Down
76 changes: 75 additions & 1 deletion src/plone/api/tests/test_content.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -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."""

Expand Down Expand Up @@ -66,7 +83,6 @@ def setUp(self):
id="events",
container=self.portal,
)

self.team = api.content.create(
container=self.about,
type="Document",
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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):
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()
Expand Down