Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Zope: CMF

allowedContentTypes improvement

 

 

Zope cmf RSS feed   Index | Next | Previous | View Threaded


vincent.fretin at gmail

Jun 22, 2009, 2:13 PM

Post #1 of 3 (893 views)
Permalink
allowedContentTypes improvement

Hi CMF people,

I patched the getDefaultAddableTypes method in Products.ATContentTypes, see:
https://dev.plone.org/collective/changeset/88621
https://dev.plone.org/collective/changeset/88622

and I see there is almost the same code at
Products.CMFCore.PortalFolder.PortalFolderBase.allowedContentTypes
which is used by ATContentTypes.allowedContentTypes if mode=DISABLED.

If anyone want to patch CMFCore, please do so.

Please see the story below:
--
Vincent Fretin


On Mon, Jun 22, 2009 at 8:52 PM, Alexander Limi<limi [at] plone> wrote:
> That's a nice optimization, kudos!
>
>
> On Mon, 22 Jun 2009 10:32:15 -0700, Vincent Fretin
> <vincent.fretin [at] gmail> wrote:
>
>> Hi,
>>
>> About this change, it speed sup a lot the creation of a Folder.
>> I have a case where a user belong to 44 groups and there are a lot of
>> local
>> roles involved.
>> The creation of a Folder took 4minutes, yes 4 minutes.
>>
>> At the creation of a Folder, for each field of the Schema, we execute
>> setDefault.
>> setDefault for locallyAllowedTypes field call getDefaultAddableTypes.
>> getDefaultAddableTypes takes all the types from portal_types and check if
>> you can add it to the not yet created Folder. Then it filters for allowed
>> types specified by the FTI.
>> My patch do the contrary, filter the allowed types specified by the FTI,
>> then check isConstructionAllowed.
>>
>> The problem was that the execution of isConstructionAllowed call at the
>> end
>> checkLocalRolesAllowed from borg/localrole/workspace.py, and the
>> execution
>> of this method to check if I can add CMFTopic for example takes 2-3
>> seconds,
>> simply because it need the Manager role, so the algorithm execute until
>> the
>> end.
>> An unnecessary check because CMFTopic is filtered afterwards.
>>
>> Now my Folder creation take 10s with this change.
>>
>>
>> On Mon, Jun 22, 2009 at 7:06 PM, Vincent Fretin
>> <svn-changes [at] plone>wrote:
>>
>>> Author: vincentfretin
>>> Date: Mon Jun 22 17:06:49 2009
>>> New Revision: 88621
>>>
>>> Modified:
>>>
>>> Products.ATContentTypes/branches/1.3/Products/ATContentTypes/lib/constraintypes.py
>>>   Products.ATContentTypes/branches/1.3/docs/HISTORY.txt
>>> Log:
>>> Modified lib/constraintypes.py:getDefaultAddableTypes method to check
>>> isConstructionAllowed only for allowed types, not for all content types
>>> in portal_types. isConstructionAllowed was called twice for each types.
>>>
>>>
>>> Modified:
>>> Products.ATContentTypes/branches/1.3/Products/ATContentTypes/lib/constraintypes.py
>>>
>>> ==============================================================================
>>> ---
>>> Products.ATContentTypes/branches/1.3/Products/ATContentTypes/lib/constraintypes.py
>>>  (original)
>>> +++
>>> Products.ATContentTypes/branches/1.3/Products/ATContentTypes/lib/constraintypes.py
>>>  Mon Jun 22 17:06:49 2009
>>> @@ -297,15 +297,15 @@
>>>         if context is None:
>>>             context = self
>>>
>>> -        result = []
>>>         portal_types = getToolByName(self, 'portal_types')
>>>         myType = portal_types.getTypeInfo(self)
>>> -        if myType is not None:
>>> -            for contentType in portal_types.listTypeInfo(context):
>>> -                if myType.allowType( contentType.getId() ):
>>> -                    result.append( contentType )
>>> -        else:
>>>             result = portal_types.listTypeInfo()
>>> +        # Don't give parameter context to portal_types.listTypeInfo().
>>> If
>>> we do that,
>>> +        # listTypeInfo will execute t.isConstructionAllowed(context)
>>> for
>>> each content type
>>> +        # in portal_types.
>>> +        # The isConstructionAllowed should be done only on allowed
>>> types.
>>> +        if myType is not None:
>>> +            return [.t for t in result if myType.allowType(t.getId())
>>> and
>>> t.isConstructionAllowed(context)]
>>>
>>>         return [ t for t in result if t.isConstructionAllowed(context) ]
>>>
>>>
>>> Modified: Products.ATContentTypes/branches/1.3/docs/HISTORY.txt
>>>
>>> ==============================================================================
>>> --- Products.ATContentTypes/branches/1.3/docs/HISTORY.txt
>>> (original)
>>> +++ Products.ATContentTypes/branches/1.3/docs/HISTORY.txt       Mon Jun
>>> 22
>>> 17:06:49 2009
>>> @@ -1,6 +1,11 @@
>>>  1.3.3 unreleased
>>>  ================
>>>
>>> +* Modified lib/constraintypes.py:getDefaultAddableTypes method to check
>>> +  isConstructionAllowed only for allowed types, not for all content
>>> types
>>> +  in portal_types. isConstructionAllowed was called twice for each
>>> types.
>>> +  [vincentfretin]
>>> +
>>>  * Fix XHTML error in criterion_edit_form.cpt
>>>   [davisagli]
>>>
>>>
>>>
>>> ------------------------------------------------------------------------------
>>> Are you an open source citizen? Join us for the Open Source Bridge
>>> conference!
>>> Portland, OR, June 17-19. Two days of sessions, one day of unconference:
>>> $250.
>>> Need another reason to go? 24-hour hacker lounge. Register today!
>>>
>>> http://ad.doubleclick.net/clk;215844324;13503038;v?http://opensourcebridge.org
>>> _______________________________________________
>>> Collective-checkins mailing list
>>> Collective-checkins [at] lists
>>> https://lists.sourceforge.net/lists/listinfo/collective-checkins
>>>
>>
>>
>>
>
>
>
> --
> Alexander Limi · http://limi.net
>
>
> ------------------------------------------------------------------------------
> Are you an open source citizen? Join us for the Open Source Bridge conference!
> Portland, OR, June 17-19. Two days of sessions, one day of unconference: $250.
> Need another reason to go? 24-hour hacker lounge. Register today!
> http://ad.doubleclick.net/clk;215844324;13503038;v?http://opensourcebridge.org
> _______________________________________________
> Plone-developers mailing list
> Plone-developers [at] lists
> https://lists.sourceforge.net/lists/listinfo/plone-developers
>
_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests


vincent.fretin at gmail

Jun 28, 2009, 12:46 AM

Post #2 of 3 (828 views)
Permalink
Re: allowedContentTypes improvement [In reply to]

Hi,

I patched Products.CMFCore trunk 2.2 as well. I hope I didn't break anything.

Vincent Fretin



On Mon, Jun 22, 2009 at 11:13 PM, Vincent
Fretin<vincent.fretin [at] gmail> wrote:
> Hi CMF people,
>
> I patched the getDefaultAddableTypes method in Products.ATContentTypes, see:
> https://dev.plone.org/collective/changeset/88621
> https://dev.plone.org/collective/changeset/88622
>
> and I see there is almost the same code at
> Products.CMFCore.PortalFolder.PortalFolderBase.allowedContentTypes
> which is used by ATContentTypes.allowedContentTypes if mode=DISABLED.
>
> If anyone want to patch CMFCore, please do so.
>
> Please see the story below:
> --
> Vincent Fretin
>
>
> On Mon, Jun 22, 2009 at 8:52 PM, Alexander Limi<limi [at] plone> wrote:
>> That's a nice optimization, kudos!
>>
>>
>> On Mon, 22 Jun 2009 10:32:15 -0700, Vincent Fretin
>> <vincent.fretin [at] gmail> wrote:
>>
>>> Hi,
>>>
>>> About this change, it speed sup a lot the creation of a Folder.
>>> I have a case where a user belong to 44 groups and there are a lot of
>>> local
>>> roles involved.
>>> The creation of a Folder took 4minutes, yes 4 minutes.
>>>
>>> At the creation of a Folder, for each field of the Schema, we execute
>>> setDefault.
>>> setDefault for locallyAllowedTypes field call getDefaultAddableTypes.
>>> getDefaultAddableTypes takes all the types from portal_types and check if
>>> you can add it to the not yet created Folder. Then it filters for allowed
>>> types specified by the FTI.
>>> My patch do the contrary, filter the allowed types specified by the FTI,
>>> then check isConstructionAllowed.
>>>
>>> The problem was that the execution of isConstructionAllowed call at the
>>> end
>>> checkLocalRolesAllowed from borg/localrole/workspace.py, and the
>>> execution
>>> of this method to check if I can add CMFTopic for example takes 2-3
>>> seconds,
>>> simply because it need the Manager role, so the algorithm execute until
>>> the
>>> end.
>>> An unnecessary check because CMFTopic is filtered afterwards.
>>>
>>> Now my Folder creation take 10s with this change.
>>>
>>>
>>> On Mon, Jun 22, 2009 at 7:06 PM, Vincent Fretin
>>> <svn-changes [at] plone>wrote:
>>>
>>>> Author: vincentfretin
>>>> Date: Mon Jun 22 17:06:49 2009
>>>> New Revision: 88621
>>>>
>>>> Modified:
>>>>
>>>> Products.ATContentTypes/branches/1.3/Products/ATContentTypes/lib/constraintypes.py
>>>>   Products.ATContentTypes/branches/1.3/docs/HISTORY.txt
>>>> Log:
>>>> Modified lib/constraintypes.py:getDefaultAddableTypes method to check
>>>> isConstructionAllowed only for allowed types, not for all content types
>>>> in portal_types. isConstructionAllowed was called twice for each types.
>>>>
>>>>
>>>> Modified:
>>>> Products.ATContentTypes/branches/1.3/Products/ATContentTypes/lib/constraintypes.py
>>>>
>>>> ==============================================================================
>>>> ---
>>>> Products.ATContentTypes/branches/1.3/Products/ATContentTypes/lib/constraintypes.py
>>>>  (original)
>>>> +++
>>>> Products.ATContentTypes/branches/1.3/Products/ATContentTypes/lib/constraintypes.py
>>>>  Mon Jun 22 17:06:49 2009
>>>> @@ -297,15 +297,15 @@
>>>>         if context is None:
>>>>             context = self
>>>>
>>>> -        result = []
>>>>         portal_types = getToolByName(self, 'portal_types')
>>>>         myType = portal_types.getTypeInfo(self)
>>>> -        if myType is not None:
>>>> -            for contentType in portal_types.listTypeInfo(context):
>>>> -                if myType.allowType( contentType.getId() ):
>>>> -                    result.append( contentType )
>>>> -        else:
>>>>             result = portal_types.listTypeInfo()
>>>> +        # Don't give parameter context to portal_types.listTypeInfo().
>>>> If
>>>> we do that,
>>>> +        # listTypeInfo will execute t.isConstructionAllowed(context)
>>>> for
>>>> each content type
>>>> +        # in portal_types.
>>>> +        # The isConstructionAllowed should be done only on allowed
>>>> types.
>>>> +        if myType is not None:
>>>> +            return [.t for t in result if myType.allowType(t.getId())
>>>> and
>>>> t.isConstructionAllowed(context)]
>>>>
>>>>         return [ t for t in result if t.isConstructionAllowed(context) ]
>>>>
>>>>
>>>> Modified: Products.ATContentTypes/branches/1.3/docs/HISTORY.txt
>>>>
>>>> ==============================================================================
>>>> --- Products.ATContentTypes/branches/1.3/docs/HISTORY.txt
>>>> (original)
>>>> +++ Products.ATContentTypes/branches/1.3/docs/HISTORY.txt       Mon Jun
>>>> 22
>>>> 17:06:49 2009
>>>> @@ -1,6 +1,11 @@
>>>>  1.3.3 unreleased
>>>>  ================
>>>>
>>>> +* Modified lib/constraintypes.py:getDefaultAddableTypes method to check
>>>> +  isConstructionAllowed only for allowed types, not for all content
>>>> types
>>>> +  in portal_types. isConstructionAllowed was called twice for each
>>>> types.
>>>> +  [vincentfretin]
>>>> +
>>>>  * Fix XHTML error in criterion_edit_form.cpt
>>>>   [davisagli]
>>>>
>>>>
>>>>
>>>> ------------------------------------------------------------------------------
>>>> Are you an open source citizen? Join us for the Open Source Bridge
>>>> conference!
>>>> Portland, OR, June 17-19. Two days of sessions, one day of unconference:
>>>> $250.
>>>> Need another reason to go? 24-hour hacker lounge. Register today!
>>>>
>>>> http://ad.doubleclick.net/clk;215844324;13503038;v?http://opensourcebridge.org
>>>> _______________________________________________
>>>> Collective-checkins mailing list
>>>> Collective-checkins [at] lists
>>>> https://lists.sourceforge.net/lists/listinfo/collective-checkins
>>>>
>>>
>>>
>>>
>>
>>
>>
>> --
>> Alexander Limi · http://limi.net
>>
>>
>> ------------------------------------------------------------------------------
>> Are you an open source citizen? Join us for the Open Source Bridge conference!
>> Portland, OR, June 17-19. Two days of sessions, one day of unconference: $250.
>> Need another reason to go? 24-hour hacker lounge. Register today!
>> http://ad.doubleclick.net/clk;215844324;13503038;v?http://opensourcebridge.org
>> _______________________________________________
>> Plone-developers mailing list
>> Plone-developers [at] lists
>> https://lists.sourceforge.net/lists/listinfo/plone-developers
>>
>
_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests


charlie at begeistert

Jun 28, 2009, 5:35 AM

Post #3 of 3 (833 views)
Permalink
Re: allowedContentTypes improvement [In reply to]

Am 28.06.2009, 09:46 Uhr, schrieb Vincent Fretin
<vincent.fretin [at] gmail>:

> Hi,
> I patched Products.CMFCore trunk 2.2 as well. I hope I didn't break
> anything.

Difficult to know as there doesn't seem to be any test for this. I think
it would be a good idea to have one

Apart from the use of list comprehension, the change seems to be:

portal_types.listTypeInfo(self)

versus

portal_types.listTypeInfo()

Where ther is type information which leads to isConstructionAllowed()
being called twice. The change shouldn't have any unpleasant side effects
and is certainly easier to understand.

I would add that the changed code exceeds the recommended maxium line
length.

Charlie
--
Charlie Clark
Helmholtzstr. 20
Düsseldorf
D- 40215
Tel: +49-211-938-5360
GSM: +49-178-782-6226
_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

Zope cmf RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.