launchpad-dev team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

On Monday 26 July 2010 16:24:33 Aaron Bentley wrote:
> On 07/26/2010 11:10 AM, Julian Edwards wrote:
> > On Monday 26 July 2010 16:01:31 Jonathan Lange wrote:
> >>> I guess I'm struggling to see how @classmethod is useful then.  In
> >>> fact, it's dangerous.
> >> 
> >> Why do you think it's dangerous?
> > 
> > Because it allows someone to get hold of a new object that is not
> > security wrapped.
> 
> So does FooSet.new.  What's the difference?  

None - that's exactly what I was referring to.

> Either one can be
> registered as a utility, and calling it as a utility looks exactly the
> same.
> 
> >  I don't know why you'd ever want to do that.
> 
> It's very convenient in test code.

Why?  Why do you need to ignore security?

If it is *really* needed, I would *much* rather see an explicit 
removeSecurityProxy() with a comment explaining why you need to remove the 
wrapper.  It should be a conscious exception, not a trap you can fall into.

> >>   * This extra work does not buy any meaningful separation of
> >> 
> >> concerns. It's very normal to have code that creates an object
> >> together with the code that defines the behaviour of the object.
> > 
> > That I agree with :)
> 
> Another point is that having an IFooSet encourages people to hang a
> deleteFoo method from it, and this is an antipattern, because it
> prevents normal zope security checks from being available.  (You can
> only restrict permission on self, not other parameters to methods.)
> 
> def Foo.destroySelf(self) <- We can check whether the user has
> permission to delete this object at the zope level, because self is the
> object
> 
> def FooSet.destroyFoo(self, foo) <- We can't check whether the user has
> permission to delete this object at the zope level, because it's foo,
> not self.

I agree with all of this, except that I can't say I've ever been encouraged to 
have a delete method on a utility :)

But still, we can do store.remove() anywhere and that's not checked.

J