Re: [Zope-CMF] Extending FTI.isConstructionAllowed
Previously yuppie wrote: After (re)reading all the comments and having a closer look at the code I came to these conclusions: 1.) CMF 2.1 checks two different restrictions: allowType() and isConstructionAllowed(). PortalFolderBase._verifyObjectPaste just checks allowType() because in CMF 2.1 isConstructionAllowed() does basically the same permission check as CopyContainer._verifyObjectPaste. Changing isConstructionAllowed() without changing PortalFolderBase._verifyObjectPaste creates inconsistent behavior. The _checkWorkflowAllowed change and your branch are both broken. That sounds like there is some opportunity for code sharing there. 2.) The distinction between allowType() and isConstructionAllowed() was clear in CMF 2.1: allowType() checked a cheap, not permission related CMF specific restriction. isConstructionAllowed() checked generic permission related restictions. The new restrictions _checkWorkflowAllowed and ITypeConstructionFilter don't fit in one of these two categories. Is there a reason that the two have to be separate? What is the downside of one call that does all necessary checks? 3.) I was wrong about comparing isConstructionAllowed with checkFactory and checkObject. These are used for checking general container constraints, not for checking user specific permissions. checkFactory doesn't work for CMF because it doesn't take the portal type as argument. Right. My conclusion: allowType() and isConstructionAllowed() are both the wrong place for checking additional restrictions. But allowType() could become part of a more general precondition that could be checked by checkObject and a new checkPortalType (=CMF specific checkFactory) function. How do you see this working? If it's simple enough I might have enough time to work on it this week. Wichert. -- Wichert Akkerman wich...@wiggy.netIt is simple to make things. http://www.wiggy.net/ It is hard to make things simple. ___ Zope-CMF maillist - Zope-CMF@lists.zope.org http://mail.zope.org/mailman/listinfo/zope-cmf See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
Re: [Zope-CMF] Extending FTI.isConstructionAllowed
Hi Wichert! Wichert Akkerman wrote: Previously yuppie wrote: 2.) The distinction between allowType() and isConstructionAllowed() was clear in CMF 2.1: allowType() checked a cheap, not permission related CMF specific restriction. isConstructionAllowed() checked generic permission related restictions. The new restrictions _checkWorkflowAllowed and ITypeConstructionFilter don't fit in one of these two categories. Is there a reason that the two have to be separate? I don't know the reasons, I just can guess. AFAICS it's not absolutely necessary, but using one method would require several changes. What is the downside of one call that does all necessary checks? These come to my mind: - You no longer can use TypeInformation.constructInstance to bypass the allowType() check. PortalFolderBase.invokeFactory checks allowType() before calling constructInstance. - You no longer can call CopyContainer._verifyObjectPaste from PortalFolderBase._verifyObjectPaste without performing redundant permission checks. - Actions have a similar distinction between 'available' and 'allowed'. The new 'add' actions in CMF 2.2 map allowType() and isConstructionAllowed() to 'available' and 'allowed' keys. allowType() and isConstructionAllowed() are both the wrong place for checking additional restrictions. But allowType() could become part of a more general precondition that could be checked by checkObject and a new checkPortalType (=CMF specific checkFactory) function. How do you see this working? If it's simple enough I might have enough time to work on it this week. In CMF we would add a __setattr__.precondition to IFolderish, Plone folders would use a modified interface with a different precondition. The preconditions would implement an interface like this one: class IFolderishPrecondition(Interface): def __call__(container, name, object): Test whether container setitem arguments are valid. Raise zope.interface.Invalid if the object is invalid. def portaltype(container, name, portaltype): Test whether objects provided by the portal type are acceptable Return a boolean value. _verifyObjectPaste would use checkObject, other places where currently allowType() is used would use a new checkPortalType function. Does that make sense to you? Cheers, Yuppie ___ Zope-CMF maillist - Zope-CMF@lists.zope.org http://mail.zope.org/mailman/listinfo/zope-cmf See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
Re: [Zope-CMF] Extending FTI.isConstructionAllowed
Am 02.06.2009 um 14:47 schrieb Wichert Akkerman: That assumes the object has already been constructed and you're only testing constraints for adding the instance to the container. Our use case is different: we are testing at a point where construction has not happened yet. I think there are two things to consider here: 1) can I create the object at all? 2) can I add it to a particular container? so yuppies' reference to checkFactory and checkObject are definitely the places to start. Your use case refers not to object creation but containment depths so you're not checking the factory as whether you can create the object but the container as to whether it may contain the object. Surely extending the sample class to count your containment depth will solve the problem: raise the error and rollback the transaction? Charlie PS. written without reference to the scary code but not convinced that extending TypeTool in anyway is a solution to anything. -- Charlie Clark Helmholtzstr. 20 Düsseldorf D- 40215 Tel: +49-211-938-5360 GSM: +49-178-782-6226 ___ Zope-CMF maillist - Zope-CMF@lists.zope.org http://mail.zope.org/mailman/listinfo/zope-cmf See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
Re: [Zope-CMF] Extending FTI.isConstructionAllowed
Am 03.06.2009 um 10:12 schrieb Wichert Akkerman: There are two reasons I want to block creation of the object: performance, and user interface. The last one is more critical: the add-item menus used in Plone (and I suspect CMF as well) look for all object types that can be constructed in the current location. Obviously you do not want to do that by creating all possible objects and testing if you can add them to the container, so we need a way to test if creating an object and adding it to the current container is allowed without creating objects. I agree with you on the UI aspect! In which case, surely you should be looking to set a precondition on checkFactory? class IItemTypePrecondition(zope.interface.Interface): def __call__(container, name, object): Test whether container setitem arguments are valid. Raise zope.interface.Invalid if the object is invalid. def factory(container, name, factory): Test whether objects provided by the factory are acceptable Return a boolean value. It looks to me like the factory method is the place to do the checking. But I guess the problem is integrating this with the TypeInfo.allowed_content_types ? Charlie -- Charlie Clark Helmholtzstr. 20 Düsseldorf D- 40215 Tel: +49-211-938-5360 GSM: +49-178-782-6226 ___ Zope-CMF maillist - Zope-CMF@lists.zope.org http://mail.zope.org/mailman/listinfo/zope-cmf See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
Re: [Zope-CMF] Extending FTI.isConstructionAllowed
Hi Wichert! Wichert Akkerman wrote: I have a use case where I need to put additional restrictions on object creation, in particular I need to restrict the maximum depth of items inside of a container of a specific type. The ideal place to put such a restriction seems to be the isConstructionAllowed method on the FTI. Currently this method is not very extensible, which leads to complicated code in various FTI types. I am considering to add an extension point here, something like this: class ITypeConstructionFilter(Interface): def __init__(fti, container): Adapt on the FTI of the object being created and the target container def allowed(): Check if construction is allowed. current checks such as the workflow check that was added in CMF 2.2, or the type constraint logic Plone has in ATContentTypes could be moved to such an adapter. The standard isConstructionAllowed method could then query all registered adapters to check if construction should be possible. Does this sound sensible? After (re)reading all the comments and having a closer look at the code I came to these conclusions: 1.) CMF 2.1 checks two different restrictions: allowType() and isConstructionAllowed(). PortalFolderBase._verifyObjectPaste just checks allowType() because in CMF 2.1 isConstructionAllowed() does basically the same permission check as CopyContainer._verifyObjectPaste. Changing isConstructionAllowed() without changing PortalFolderBase._verifyObjectPaste creates inconsistent behavior. The _checkWorkflowAllowed change and your branch are both broken. 2.) The distinction between allowType() and isConstructionAllowed() was clear in CMF 2.1: allowType() checked a cheap, not permission related CMF specific restriction. isConstructionAllowed() checked generic permission related restictions. The new restrictions _checkWorkflowAllowed and ITypeConstructionFilter don't fit in one of these two categories. 3.) I was wrong about comparing isConstructionAllowed with checkFactory and checkObject. These are used for checking general container constraints, not for checking user specific permissions. checkFactory doesn't work for CMF because it doesn't take the portal type as argument. My conclusion: allowType() and isConstructionAllowed() are both the wrong place for checking additional restrictions. But allowType() could become part of a more general precondition that could be checked by checkObject and a new checkPortalType (=CMF specific checkFactory) function. Plone could use its own precondition that checks registered ITypeConstructionFilter adapters. Cheers, Yuppie ___ Zope-CMF maillist - Zope-CMF@lists.zope.org http://mail.zope.org/mailman/listinfo/zope-cmf See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
Re: [Zope-CMF] Extending FTI.isConstructionAllowed
Previously Tres Seaver wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Wichert Akkerman wrote: I have a use case where I need to put additional restrictions on object creation, in particular I need to restrict the maximum depth of items inside of a container of a specific type. The ideal place to put such a restriction seems to be the isConstructionAllowed method on the FTI. Currently this method is not very extensible, which leads to complicated code in various FTI types. I am considering to add an extension point here, something like this: class ITypeConstructionFilter(Interface): def __init__(fti, container): Adapt on the FTI of the object being created and the target container def allowed(): Check if construction is allowed. current checks such as the workflow check that was added in CMF 2.2, or the type constraint logic Plone has in ATContentTypes could be moved to such an adapter. The standard isConstructionAllowed method could then query all registered adapters to check if construction should be possible. Does this sound sensible? I'm not sure about querying all adapters: I think it would be clearer to query the one adapter whose name corresponds to the type name of the FTI (the query all case leads to tricky / emergent behavior). Querying a single adapter makes it very hard to use this as an extension point. Being able to have multiple independent validation-hooks is the whole point of my suggestion, and being able to only use a single adapter would make that impossible. Wichert. -- Wichert Akkerman wich...@wiggy.netIt is simple to make things. http://www.wiggy.net/ It is hard to make things simple. ___ Zope-CMF maillist - Zope-CMF@lists.zope.org http://mail.zope.org/mailman/listinfo/zope-cmf See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
Re: [Zope-CMF] Extending FTI.isConstructionAllowed
Previously yuppie wrote: It's no hook for adding restrictions, but it's a hook for using different implementations. and does not support portal types. Do you mean does not support per portal type hooks or do you mean does not support filtering based on portal type name? The latter. zope.* has no concept of FTIs or portal types. A CMF specific precondition would look up type restrictions in the fti of the container. checkFactory and checkObject are quite similar to isConstructionAllowed. I think we should reimplement this based on zope.container before we start adding new features. I looked at the code in zope.container and frankly it scared me. I found the documentation and code hard to follow, and the usage of sys._getframe() made me drop the idea of using it. Wichert. -- Wichert Akkerman wich...@wiggy.netIt is simple to make things. http://www.wiggy.net/ It is hard to make things simple. ___ Zope-CMF maillist - Zope-CMF@lists.zope.org http://mail.zope.org/mailman/listinfo/zope-cmf See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
Re: [Zope-CMF] Extending FTI.isConstructionAllowed
Previously Wichert Akkerman wrote: I have a use case where I need to put additional restrictions on object creation, in particular I need to restrict the maximum depth of items inside of a container of a specific type. The ideal place to put such a restriction seems to be the isConstructionAllowed method on the FTI. Currently this method is not very extensible, which leads to complicated code in various FTI types. I am considering to add an extension point here, something like this: class ITypeConstructionFilter(Interface): def __init__(fti, container): Adapt on the FTI of the object being created and the target container def allowed(): Check if construction is allowed. current checks such as the workflow check that was added in CMF 2.2, or the type constraint logic Plone has in ATContentTypes could be moved to such an adapter. The standard isConstructionAllowed method could then query all registered adapters to check if construction should be possible. I have implemented this on the wichert-constructionfilter branch of Products.CMFCore. I intend to merge this to trunk end of this week. Wichert. -- Wichert Akkerman wich...@wiggy.netIt is simple to make things. http://www.wiggy.net/ It is hard to make things simple. ___ Zope-CMF maillist - Zope-CMF@lists.zope.org http://mail.zope.org/mailman/listinfo/zope-cmf See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
Re: [Zope-CMF] Extending FTI.isConstructionAllowed
Wichert Akkerman wrote: Previously yuppie wrote: A CMF specific precondition would look up type restrictions in the fti of the container. checkFactory and checkObject are quite similar to isConstructionAllowed. I think we should reimplement this based on zope.container before we start adding new features. I looked at the code in zope.container and frankly it scared me. I found the documentation and code hard to follow, and the usage of sys._getframe() made me drop the idea of using it. That scary code is used for supporting 'contains' declarations in the interface. I don't propose to write something similar for CMF. AFAICS it is sufficient to set __setattr__.precondition directly for supporting checkObject. A precondition that just checks allowType would look like this: class PortalTypePrecondition: def __call__(self, container, name, obj): ti = container.getTypeInfo() if ti is None: return if not ti.allowType(obj.getPortalTypeName()): raise ValueError(u'Disallowed subobject type: %s' % type_name) Cheers, Yuppie ___ Zope-CMF maillist - Zope-CMF@lists.zope.org http://mail.zope.org/mailman/listinfo/zope-cmf See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
Re: [Zope-CMF] Extending FTI.isConstructionAllowed
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Wichert Akkerman wrote: Previously Tres Seaver wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Wichert Akkerman wrote: I have a use case where I need to put additional restrictions on object creation, in particular I need to restrict the maximum depth of items inside of a container of a specific type. The ideal place to put such a restriction seems to be the isConstructionAllowed method on the FTI. Currently this method is not very extensible, which leads to complicated code in various FTI types. I am considering to add an extension point here, something like this: class ITypeConstructionFilter(Interface): def __init__(fti, container): Adapt on the FTI of the object being created and the target container def allowed(): Check if construction is allowed. current checks such as the workflow check that was added in CMF 2.2, or the type constraint logic Plone has in ATContentTypes could be moved to such an adapter. The standard isConstructionAllowed method could then query all registered adapters to check if construction should be possible. Does this sound sensible? I'm not sure about querying all adapters: I think it would be clearer to query the one adapter whose name corresponds to the type name of the FTI (the query all case leads to tricky / emergent behavior). Querying a single adapter makes it very hard to use this as an extension point. Being able to have multiple independent validation-hooks is the whole point of my suggestion, and being able to only use a single adapter would make that impossible. I don't *want* multiple third-party products to register this adapter: I think it belongs to the integrator to set the policy for the site. Reusable policy is an oxymoron. Tres. - -- === Tres Seaver +1 540-429-0999 tsea...@palladion.com Palladion Software Excellence by Designhttp://palladion.com -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.6 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFKJVTi+gerLs4ltQ4RAnT7AJ0aNlB5Vr1MHdSnBMcxrcfb70dIDQCgmAv0 VhcD0BrbHpW1c60aZlCvai0= =haLI -END PGP SIGNATURE- ___ Zope-CMF maillist - Zope-CMF@lists.zope.org http://mail.zope.org/mailman/listinfo/zope-cmf See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
[Zope-CMF] Extending FTI.isConstructionAllowed
I have a use case where I need to put additional restrictions on object creation, in particular I need to restrict the maximum depth of items inside of a container of a specific type. The ideal place to put such a restriction seems to be the isConstructionAllowed method on the FTI. Currently this method is not very extensible, which leads to complicated code in various FTI types. I am considering to add an extension point here, something like this: class ITypeConstructionFilter(Interface): def __init__(fti, container): Adapt on the FTI of the object being created and the target container def allowed(): Check if construction is allowed. current checks such as the workflow check that was added in CMF 2.2, or the type constraint logic Plone has in ATContentTypes could be moved to such an adapter. The standard isConstructionAllowed method could then query all registered adapters to check if construction should be possible. Does this sound sensible? Wichert. ___ Zope-CMF maillist - Zope-CMF@lists.zope.org http://mail.zope.org/mailman/listinfo/zope-cmf See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
Re: [Zope-CMF] Extending FTI.isConstructionAllowed
Wichert Akkerman wrote: I have a use case where I need to put additional restrictions on object creation, in particular I need to restrict the maximum depth of items inside of a container of a specific type. The ideal place to put such a restriction seems to be the isConstructionAllowed method on the FTI. Currently this method is not very extensible, which leads to complicated code in various FTI types. I am considering to add an extension point here, something like this: class ITypeConstructionFilter(Interface): def __init__(fti, container): Adapt on the FTI of the object being created and the target container def allowed(): Check if construction is allowed. current checks such as the workflow check that was added in CMF 2.2, or the type constraint logic Plone has in ATContentTypes could be moved to such an adapter. The standard isConstructionAllowed method could then query all registered adapters to check if construction should be possible. Does this sound sensible? Question: zope.container.constraints handles this in a different way, using a precondition defined in the interface. Did you have a look at that code? If we switch to that pattern, we could use different preconditions for containers with different interfaces. Would that be sufficient for your use case? Cheers, Yuppie ___ Zope-CMF maillist - Zope-CMF@lists.zope.org http://mail.zope.org/mailman/listinfo/zope-cmf See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
Re: [Zope-CMF] Extending FTI.isConstructionAllowed
Previously yuppie wrote: Wichert Akkerman wrote: I have a use case where I need to put additional restrictions on object creation, in particular I need to restrict the maximum depth of items inside of a container of a specific type. The ideal place to put such a restriction seems to be the isConstructionAllowed method on the FTI. Currently this method is not very extensible, which leads to complicated code in various FTI types. I am considering to add an extension point here, something like this: class ITypeConstructionFilter(Interface): def __init__(fti, container): Adapt on the FTI of the object being created and the target container def allowed(): Check if construction is allowed. current checks such as the workflow check that was added in CMF 2.2, or the type constraint logic Plone has in ATContentTypes could be moved to such an adapter. The standard isConstructionAllowed method could then query all registered adapters to check if construction should be possible. Does this sound sensible? Question: zope.container.constraints handles this in a different way, using a precondition defined in the interface. Did you have a look at that code? If we switch to that pattern, we could use different preconditions for containers with different interfaces. Would that be sufficient for your use case? I don't think that is sufficient since it does not provide an extension point you can hook into and does not support portal types. Wichert. -- Wichert Akkerman wich...@wiggy.netIt is simple to make things. http://www.wiggy.net/ It is hard to make things simple. ___ Zope-CMF maillist - Zope-CMF@lists.zope.org http://mail.zope.org/mailman/listinfo/zope-cmf See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
Re: [Zope-CMF] Extending FTI.isConstructionAllowed
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Wichert Akkerman wrote: I have a use case where I need to put additional restrictions on object creation, in particular I need to restrict the maximum depth of items inside of a container of a specific type. The ideal place to put such a restriction seems to be the isConstructionAllowed method on the FTI. Currently this method is not very extensible, which leads to complicated code in various FTI types. I am considering to add an extension point here, something like this: class ITypeConstructionFilter(Interface): def __init__(fti, container): Adapt on the FTI of the object being created and the target container def allowed(): Check if construction is allowed. current checks such as the workflow check that was added in CMF 2.2, or the type constraint logic Plone has in ATContentTypes could be moved to such an adapter. The standard isConstructionAllowed method could then query all registered adapters to check if construction should be possible. Does this sound sensible? I'm not sure about querying all adapters: I think it would be clearer to query the one adapter whose name corresponds to the type name of the FTI (the query all case leads to tricky / emergent behavior). Tres. - -- === Tres Seaver +1 540-429-0999 tsea...@palladion.com Palladion Software Excellence by Designhttp://palladion.com -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.6 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFKH+mA+gerLs4ltQ4RAhG7AKDZsLKNRVUHBfLoq/tbGsqU50TVJgCgr3Np 06Ck6T4Xyvru7WKgm8vUjbs= =2I0k -END PGP SIGNATURE- ___ Zope-CMF maillist - Zope-CMF@lists.zope.org http://mail.zope.org/mailman/listinfo/zope-cmf See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
Re: [Zope-CMF] Extending FTI.isConstructionAllowed
Wichert Akkerman wrote: Previously yuppie wrote: Wichert Akkerman wrote: I have a use case where I need to put additional restrictions on object creation, in particular I need to restrict the maximum depth of items inside of a container of a specific type. The ideal place to put such a restriction seems to be the isConstructionAllowed method on the FTI. Currently this method is not very extensible, which leads to complicated code in various FTI types. I am considering to add an extension point here, something like this: class ITypeConstructionFilter(Interface): def __init__(fti, container): Adapt on the FTI of the object being created and the target container def allowed(): Check if construction is allowed. current checks such as the workflow check that was added in CMF 2.2, or the type constraint logic Plone has in ATContentTypes could be moved to such an adapter. The standard isConstructionAllowed method could then query all registered adapters to check if construction should be possible. Does this sound sensible? Question: zope.container.constraints handles this in a different way, using a precondition defined in the interface. Did you have a look at that code? If we switch to that pattern, we could use different preconditions for containers with different interfaces. Would that be sufficient for your use case? I don't think that is sufficient since it does not provide an extension point you can hook into It's no hook for adding restrictions, but it's a hook for using different implementations. and does not support portal types. Do you mean does not support per portal type hooks or do you mean does not support filtering based on portal type name? A CMF specific precondition would look up type restrictions in the fti of the container. checkFactory and checkObject are quite similar to isConstructionAllowed. I think we should reimplement this based on zope.container before we start adding new features. Cheers, Yuppie ___ Zope-CMF maillist - Zope-CMF@lists.zope.org http://mail.zope.org/mailman/listinfo/zope-cmf See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
