[Zope-CMF] IndexableObjectWrapper

[Miles]

Hi,

Thanks for the feedback!  I think we're pretty much there with this.

>> As there are no adapters in CMFCore at the moment, there's no precedent 
>> to follow.  So:
>>
>>   - The multi-adapter is registered in a new file, implements.zcml
> 
> Why did you choose the name implements.zcml? I can't see how 
> 'implements' describes the contained registration. I would have used the 
> content.zcml file or added a catalog.zcml file.

To be honest, I was just guessing at a pattern to follow, that wouldn't 
lead to creating a million ZCML files.  I'll add it into content.zcml.

> And I would prefer a new marker interface 'IIndexableObject'. I guess 
> there are use cases that don't require restricted searchResults based on 
> allowedRolesAndUsers and usually we want to catalog more attributes than 
> just this one. So we need an interface that marks objects that provide 
> all the attributes we want to catalog, not an interface that specifies 
> the available attributes.

It's taken several readings for me to understand this, but I think I 
agree.  I've adjusted the registrations so that they are based on a 
marker interface, IIndexableObject.  If the object already provides this 
index, no wrapping is carried out.  I added some tests for this.

I also adjusted the registration to use ICatalogAware rather than 
IContentish, as that seemed to make more sense.

> 
>>   - The TraversingEventZCMLLayer now also loads implements.zcml, as the 
>> correct behaviour of both the catalog and the adapter class is required 
>> for the folder tests to pass.
>>
>>   - The CatalogTool tests set up the adapter at the moment, as a lot of 
>> the catalog tests require the adapter to work properly.  This is done in 
>> the _makeContent method as it applied to most tests that used the dummy 
>> content.  However, I think it belongs somewhere else, but I wasn't sure 
>> whether that place was a layer, a setup method or somewhere else.  Any 
>> suggestions?
> 
> I agree it belongs somewhere else. Maybe a registerWrapper method. But 
> can't we make the adapter lookup in catalog_object optional and wouldn't 
> that make test setups simpler?

Ok, I bit the bullet and did some work on the tests.

The catalog tests are adjusted to provide their a DummyContent class 
implementing IIndexableObject, so the adapter is not used.  Instead of 
setting the View permission, the tests set an allowedRolesAndUsers 
attribute as required.

The Folder tests are adjusted to use a DummyCatalog test fixture rather 
than the real catalog, removing the need for the adapter registration 
and making the tests a bit simpler, IMO.

> 
>> Otherwise, the branch is here and all the tests pass:
>> /Products.CMFCore/branches/miwa-catalog-adapter/Products/CMFCore
> 
> Please try to follow PEP 8 and wrap long lines.

I think this is done now; I don't think any of the files I touched have 
lines longer than 79 characters now - but if you do spot one then let me 
know!

Miles