[Zope-CMF] listFolderContents

[David (Hamish) Harvey]

--On Saturday, June 29, 2002 17:44:12 +0000 Florent Guillaume 
<fg@nuxeo.com> wrote:

> David (Hamish) Harvey <david.harvey@bristol.ac.uk> wrote:
>> <aside>
>> The spec parameter passed to listFolderContents is currently discarded:
>> this *is* a bug, and I've submitted it as issue 520. I thought I'd done
>> so  before but couldn't see it. I suspect that the issues under
>> discussion have  been somewhat disguised by this bug.
>> </aside>
>
> Yeah, I saw that yesterday too and fixed it in CVS.

I must check the CVS before submitting bug reports.
I must check the CVS before submitting bug reports.
I must check the CVS before submitting bug reports.
I must check the CVS before submitting bug reports.

Excellent!

>> > Note that this filtering relies on the "Meta Type" (content_meta_type)
>> > parameter of a Type Information in the Types Tool to be correctly set,
>> > namely to be equal to the meta_type of the underlying object.
>>
>> If to be "correctly set", content_meta_type==<meta_type of underlying
>> object> then content_meta_type and all the machinery that works with it
>> is  redundant and shouldn't be there. This can't be true --
>> content_meta_type  is there for a reason and therefore, logically, one
>> must be allowed to set  it to have a different value from the underlying
>> meta_type.
>
> This stumped me too. The thing is, the Types Tool is a factory. With
> only the Type Information, it has no way to get to an object to ask it
> its meta_type, because that would mean constructing one first.  So the
> reason content_meta_type is there, is that the Types Tool has no other
> way to know a priori what the meta_type of constructed objects would
> be.

So we're saying: content_meta_type just holds the value that the created 
objects will have as meta_type (this feels crude, and is evidently not ever 
so effective, given the amount of confusion it's causing :-). In that case 
the equality of content_meta_type on the FTI and meta_type on the created 
object should be checked at creation time.

> (As an aside, this means that Scriptable Types are not allowed to
> create different kinds of objects with different meta_types depending
> on the circumstances.)
>
>
>> > _morphSpec only deals with meta_types.
>>
>> Not true. _morphSpec accepts a
>> list of values, and assigns each of these in turn to a variable which it
>> confusingly calls meta_type. However, the values it retrieves from
>> 	types_tool.listContentTypes( by_metatype=1 )
>> are content_meta_types. The definition of listContentTypes uses
>> 	name = t.Metatype()
>> where t is a TypeInformation. TypeInformation.Metatype() returns
>> content_meta_type.
>
> I also thought it was "confusingly" called that, and that, really,
> being in a CMF portal, we wanted to deal only with portal_types, and
> that contentIds & co would take a list of portal_types, but I was
> wrong. The code is actually very coherent once you see what I saw :-)
> which is, that this choice of type of parameters is very poor, and
> that it really should have used portal_types.

OK. If content_meta_type does have to be == object's meta_type then this 
makes sense. Duplicated information, though. Ugh. It raises the possibility 
of the two values being different... :-)

>> The real killer, and the reason that at present content_meta_type
>> must == underlying meta_type, is that _morphSpec deals with
>> content_meta_type, while objectIds which is ultimately responsible
>> for handling the spec parameter expects it to contain meta_types. It
>> would be preferable to the current situation, probably, if
>> _morphSpec was modified to deal with meta_types, but at the same
>> time this would be limiting and far from perfect.
>
> _morphSpec's docstring is actually very clear:
>         spec is a sequence of meta_types, a string containing one meta
> type,         or None.  If spec is empty or None, returns all contentish
>         meta_types.  Otherwise ensures all of the given meta types are
> contentish.
> Besides its result is called on objectIds() which points in the same
> direction. So _morphSpec *does* deal with meta_types.
>
> In turn, reading the code, this means that
> listContentTypes(by_metatype=1) must return a list of meta_types. So
> the content_meta_type attribute of the Type Information must be a real
> meta_type.

Indeed, so the upshot of this is I have a bug in *my* code, which "fixes" 
objectIds to deal with portal_types. There is still a potential problem 
though - more on this below.

> I've been confused about what the "Meta Type" field in the Types Tool
> was for such a long time that I probably led others to be confused
> too.

No, I think the code is fundamentally confusing! Two modifications would 
help enourmously - a BIG notice on content_meta_type to explain the reason 
for its existence, and an equality check at object creation time.

>> > One could call for instance:
>> >   listFolderContents(contentFilter={'Type': ['MyDocumentType',
>> > 'MyOtherType']})
>>
>> This is orthogonal to spec, although similar effects can be achieved. I
>> would use this if in a *particular* context in a site I wanted to list
>> only  objects with a particular portal_type.
>
> Ok. This is in fact a pretty common requirement (although using the
> catalog on portal_type would probably be as useable).

This is discussed below.

>> However if a number of FactoryTypeInformations, each with the same
>> underlying type and thus the same underlying meta_type, I want to be
>> able to use the View security setting on those FTIs to control the
>> visibility of instances created from these FTIs across the *whole*
>> site. I thus need functioning spec machinery based on
>> content_meta_type/portal_type.
>
> Ah yes, I think see your problem, and were the bug is.
>
> I think it can be solved by refining _filteredItems to also check that
> 'Type' is restricted by the list of allowed types returned by
> listContentTypes(by_metatype=0). We also would need to NOT
> short-circuit the case were filter is empty in contentIds & co.
>
> You'd then filter using the approach I showed above, and not spec
> (which, really, contains meta_types :-).
>
> Does that look ok ? I'll make a patch if you approve that line.

Scenario:

I have two FTIs which both construct Documents. The final portal_type 
attribute for objects created by each is different (just checked: it's set 
to the id of the FTI). I want to allow Members to view and create one, only 
Managers the other[1], say. With the current setup, and if I have set 
content_meta_type "correctly" in the FTIs, then as Member I get back from 
listContentTypes a list which *includes* "Document" because *a* type with 
Document as it's base type is visible. So if any Document is visible given 
my roles, all Documents are?

Construction is, I think, correctly controlled by the security settings on 
the FTI, because the allowedContentTypes method on PortalFolder examines 
the TIs individually.

The point here is that as time goes by and the requirements of my site 
change slowly, I want to be able to turn on and off visibility and 
creatability of objects represented by each FTI independently, whereas at 
present I would need to modify all of the calls to listFolderContents to 
update the contentFilter. I can't help but feel that as it stands, and 
given your proposal for using contentFilter in CMF, the spec machinary 
amounts to the vestigial remains of a Zope mechanism with little purpose in 
CMF?

[1] Actually I want members to be able to view but not create the second - 
the fact that I implemented this and am therefore able to really use the 
security settings of the FTI realistically might be why I feel this issue 
quite acutely.

> (Note that all that doesn't solve the problem that the code is using
> ti.Type() all over the place, which is a human-readable string, where
> it should use ti.getId(), which is what is set in the portal_type
> attribute of created objects. But this is a separate concern, and
> changing that would cause hairy backward-compatibility problems.)

In fact it makes it worse! ;-) It might be possible to help rather than 
hinder here. It wouldn't disturb anything to add a "ContentType" or some 
such parameter (named carefully :-) to the ContentFilter which checks 
against the ids of the TIs? listContentTypes would also need to be able to 
return ids (rather than the "title, or id if that isn't available) at the 
moment). Then we could filter on id rather than the human-readable name.

A couple of other thoughts

* The fact that TIs have a unique id, of course, means that 
content_meta_type would be pretty redundant if it had the semantics I 
thought it did (by which I mean to reinforce that I have been swayed by 
your arguments).

* Your proposed scheme for using contentFilter relies on _filteredItems 
being called even with a None filter, so you'd need to remove the early 
returns from contentIds, contentValues, and make sure that a filter of None 
means something to _filteredItems.

* For what is the DynamicType machinery? Why would I want to change the 
portal_type of an object at arbitrary times? And would I then be expected 
to make sure that the corresponding TI was renamed to match (or at least 
there existed a TI with id equal to my newly set portal_type)?

* There would be a case (backward compatibility permitting :-) for having 
two separate listContentTypes type methods - one which returns either 
content_meta_type or id according to a parameter, the other that returns 
human-readable names for use by skins.

* Blue sky: If the i18n machinery, couldn't the human readable title be 
disposed of completely, and the id used as an i18n key?

Hmm, I hope this makes some sense. Sadly I don't have time at the moment to 
make it more concise.

Cheers,
Hamish