search

zopefoundation / zope.publisher

Map requests from HTTP/WebDAV clients, web browsers, XML-RPC and FTP clients onto Python objects
Other
3 stars 13 forks source link

`getMultiAdapter` can't be used for looking up `IDefaultViewName` #60

Open tseaver opened 9 years ago

tseaver commented 9 years ago

In https://bugs.launchpad.net/zope.component/+bug/260379, @icemac reported:

See summary.

The names are unicode strings that are registered as factories and get called when getMultiAdapter is used.

@srichter followed up:

This is not a bug. It is simply a fact of the system. It was never intended that the default view names are to be looked up via a getMultiAdapter() call. This is also the reason, we provide a custom lookup function for default view names:

zope.app.publisher.browser.getDefaultViewName zope.app.publisher.browser.queryDefaultViewName

@ctheune replied:

IMHO this breaks component architecture usability in a similar way as views which are adaptations to Interface.

It took me quite a while to find those getDefaultViewName functions because I looked at the registrations, find a multi-adapter registration and I immediately think 'sure, so lets adapt context and request to IDefaultViewName' and then a 10 minute long search for IDefaultViewName to figure out where the publisher itself uses this.

When someone sees an adapter being registered, I think its valid that he assumes he can retrieve it using get(Multi)Adapter.

@srichter:

But default views are not registered with a regular adapter directive. There is a special directive for a reason. The fact that the CA is reused for this case is an implementation detail.

@philikon:

Which is?

I think Theuni's assumptions where somewhat correct. While I agree that he tried to use IDefaultViewName incorrectly (after all, it suggests that it describes a view name rather than a view itself), I do wonder why couldn't there simply be an IDefaultBrowserPage interface that allows you to find a view w/o having to specify a name?

So instead of having to do this dance:

   default_view_name = getDefaultViewName(obj, request)
   default_view = getMultiAdapter((obj, request), name=default_view_name)

you could simply do

   default_view = getMultiAdapter((obj, request), IDefaultBrowserPage)

I think that's much more intuitive.

@ctheune:

Yes. I had that idea recently too.

jamadden commented 3 years ago

Moving this to zope.publisher, which is where IDefaultViewName and the ZCML directives are defined. This doesn't seem to be a problem with zope.component, merely the way zope.publisher is using it.