Opened 9 years ago

Last modified 2 months ago

#2927 enhancement reopened

proxyForInterface should really be proxyForInterface*s*

Reported by: Glyph Owned by:
Priority: normal Milestone:
Component: core Keywords:
Cc: Jean-Paul Calderone, Glyph Branch: branches/proxyForInterfaces-2927
branch-diff, diff-cov, branch-cov, buildbot
Author: glyph

Description (last modified by Glyph)

An object may implement more than one interface, and there's no use-case that I'm aware of to restrict this API to proxying for only one interface at a time.

The prototypical use-case that I do know of, xmantissa.sharing, which implements a similar proxy class, requires the ability to share (or not) multiple interfaces. It does this to limit access; for example, a user who can retrieve an IRead proxy for object X might not be able to retrieve the IWrite proxy.

Change History (12)

comment:1 Changed 9 years ago by Jean-Paul Calderone

Priority: highlow

Suggestion: things you "might" want shouldn't be high priority.

However, if it is actually the case that you want to do this soon, or if you do want it, of course feel free to undo my priority meddling.

comment:2 Changed 9 years ago by Glyph

Description: modified (diff)
Priority: lowhigh
Status: newassigned

It was really just a turn of a phrase. I've clarified.

comment:3 Changed 8 years ago by Jean-Paul Calderone

Priority: highnormal

The API has long since been released. I'm sending this back to low priority since the motivation for it seems to be notional, not practical (otherwise surely someone would have implemented it by now).

comment:4 Changed 8 years ago by Glyph

Description: modified (diff)

Removing justification of the priority from the description since it no longer has that priority.

comment:5 Changed 8 years ago by Glyph

Author: glyph
Branch: branches/proxyForInterfaces-2927

(In [26735]) Branching to 'proxyForInterfaces-2927'

comment:6 Changed 8 years ago by Glyph

Keywords: review added
Owner: Glyph deleted
Status: assignednew

comment:7 Changed 8 years ago by Jean-Paul Calderone

Cc: Jean-Paul Calderone added

Just a note that the implemented method goes beyond what's required for the indicated use-case. Allowing one object to act as a proxy for different interfaces for different objects isn't what Mantissa needs - Mantissa just wants to create a proxy for different interfaces for one object.

In general, I think it's probably not ideal to smash multiple objects together like this (at least, I can see the problems it will cause, but not the problems it will solve).

comment:8 Changed 8 years ago by Glyph

Keywords: review removed
Owner: set to Glyph

Hmm... thinking about what you're saying and looking at the implementation now, I realize that there's a lot of potential for confusing and incorrect behavior when attribute names conflict. As it stands it's pretty bad.

I'll think about it a bit and either fix it up to be better defined in the face of conflicts (and provide some justifying use cases) or simplify it so that you only get the one object.

comment:9 Changed 8 years ago by Glyph

Resolution: wontfix
Status: newclosed

Upon even further consideration, never mind.

Mantissa's use-case is very similar, but ultimately subtly different. I don't think we'd want to dynamically create type objects for every possible combination of proxy interfaces, and the sharing system doesn't have a conveniently fixed type hierarchy that it can keep around. Also, it doesn't customize its proxies at all, so it doesn't bother with that part.

Also, you can already use proxyForInterface to proxy to multiple interfaces without any additional code. Here's an example:

from zope.interface import implements, Interface
from twisted.python.components import proxyForInterface

class IFoo(Interface):
    def foo():
        "foo"
class IBar(Interface):
    def bar():
        "bar"
class FooBar(object):
    implements(IFoo, IBar)
    def foo(self):
        print 'foo!'
    def bar(self):
        print 'bar!'

class WrapFooBar(proxyForInterface(IFoo), proxyForInterface(IBar)):
    def __init__(self, original):
        self.original = original

    def foo(self):
        print 'fooing'
        super(WrapFooBar, self).foo()
        print 'fooed'

    def bar(self):
        print 'baring'
        super(WrapFooBar, self).bar()
        print 'bared'

x = WrapFooBar(FooBar())
x.foo()
x.bar()

comment:10 Changed 6 years ago by <automation>

Owner: Glyph deleted

comment:12 Changed 2 months ago by Glyph

Resolution: wontfix
Status: closedreopened

OK no, we should totally do this; the interface-combination thing is a hack, and a small tweak to proxyForInterface could fix it.

Also, the inheritance thing only works by accident as long as the originalAttribute matches in all cases, because proxyForInterface makes no attempt to make its constructor cooperative.

Note: See TracTickets for help on using tickets.