Mailing List Archive: Zope: CMF

IndexableObjectWrapper

1 2 3

View All

Index | Next | Previous | View Threaded

andrew at sawdog

Mar 10, 2009, 6:20 AM

Post #26 of 60 (1105 views)
Permalink

Re: IndexableObjectWrapper [In reply to]

On 3/10/09 1:17 AM, "Charlie Clark" <charlie [at] begeistert> wrote:

>
> Am 10.03.2009 um 10:01 schrieb Wichert Akkerman:
>
>> The debate is currently focusing on GPL versus BSD license. Any
>> opinions
>> on a choice between those two would be very welcome.
>
>
> ...: BSD.
>
+2
> Charlie
> --
> Charlie Clark
> Helmholtzstr. 20
> Düsseldorf
> D- 40215
> Tel: +49-211-938-5360
> GSM: +49-178-782-6226
>
>
>
> _______________________________________________
> Zope-CMF maillist - Zope-CMF [at] lists
> http://mail.zope.org/mailman/listinfo/zope-cmf
>
> See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

miles at jamkit

Mar 10, 2009, 6:20 AM

Post #27 of 60 (1112 views)
Permalink

Re: IndexableObjectWrapper [In reply to]

>> class IndexableObjectWrapper(object):
>>
>> implements(IIndexableObjectWrapper)
>> __providedBy__ = IndexableObjectSpecification()
>>
>> What does this code actually achieve (I get the implements bit, obviously)?!
>
> This makes the wrapper transparent, allowing the index to look up
> adapters for the interfaces of the object. TextIndexNG3 works that way.

Brilliant! Now it all makes sense. Any objections to me adding some
comments into IndexableObjectSpecification to this effect?

Mile

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

optilude+lists at gmail

Mar 10, 2009, 6:21 AM

Post #28 of 60 (1109 views)
Permalink

Re: IndexableObjectWrapper [In reply to]

Jens Vagelpohl wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
>
> On Mar 10, 2009, at 11:08 , Martin Aspeli wrote:
>
>> I think we all agree on this. In retrospect, it would've been a better
>> idea to push for plone.indexer to be a part of CMF. However, I
>> implemented it driven by Plone's release cycle and feature proposal
>> process, which is why it ended up as it did. *I* want this feature for
>> Plone, but it'd be even better if others could benefit.
>
> IMHO the release cycle argument doesn't wash. We've always had CMF
> releases in preparation for important Plone releases, and I'm happy to
> continue that.

I'm not arguing. I'm saying that it didn't really cross my mind, because
I was improving something that was already Plone specific (the
ExtensibleIndexableObjectWrapper) in response to a Plone demand and
working towards a Plone deadline. Hindsight is a good thing, and maybe
it would've been better to try to all of it down into the stack. In this
case, I didn't.

Except that I kind of did... I created a package with no other Plone
dependencies, in the hope that it *could* be useful. I didn't take the
time to discuss it on this list, which I should have. However, the
desire for it to be re-usable by other CMF consumers is clearly in evidence.

>> Of course, it's not too late for that. If the license issue can be
>> overcome (and I'm pretty sure that it will by April/May), then CMF can
>> depend on plone.indexer if it so wants, and I'm willing to help make
>> that possible if it means changing plone.indexer or helping with the
>> CMF
>> level implementation.
>
> Thanks, I appreciate that.
>
> Generally, I think now that the ZF has cleared up the remaining issues
> about code ownership etc. we finally have two entities, the ZF and the
> Plone Foundation, that are the perfect platform for "official" issues
> like code donations, or for coordinating other cooperation issues. I
> can't judge how the Plone Foundation acts within the Plone community,
> but as far as the Zope Foundation goes, Martijn has been doing a lot
> of work to make it more relevant and an important player in the actual
> software development process.

Indeed!

Bear in mind that the Plone Foundation has an explicit goal *not* to
interfere with software development. However, it does deal with issues
of IP and so I agree the two foundations are the right forum for this
type of thing.

>> In the future, it may be that we can meet in the middle on this. When
>> the PLIP process kicks off, it'd be good if the CMF developers had a
>> look in as well. We should probably be better at announcing the
>> various
>> deadlines and proposals on this list, but if you guys see something
>> that
>> you feel would be a good fit further down, it doesn't hurt to raise
>> that, lest the developer hasn't thought about it.
>
> Is there any kind of low-traffic announcement list for things like
> PLIPs? I'm not subscribed to any Plone list because of (for me at
> least) signal to noise ratio fears.

There's plone-announce, but I don't think this was announced there. I
actually think the Plone release manager should cross-post a few
important announcements to this list, though.

The actual feature discussion happens on the medium-traffic
framework-team list, which you can join. In fact, it'd be great if you
did, as we'd appreciate your input, but I realise it may not be
something you want to spend a lot of time on.

Martin

--
Author of `Professional Plone Development`, a book for developers who
want to work with Plone. See http://martinaspeli.net/plone-book

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

optilude+lists at gmail

Mar 10, 2009, 6:23 AM

Post #29 of 60 (1109 views)
Permalink

Re: IndexableObjectWrapper [In reply to]

Hi Miles,

>> This should work with plain CMF and be simple to hook into the catalogue
>> tool.
>
> (For me) this is the root of the problem - it can only be hooked into
> the catalog by subclassing at the moment, there is no other mechanism to
> use different implementations. If there was, then plone.indexer (or
> whatever) could be used directly.
>
> Therefore, can I make a proposal (which I am happy to carry out), as I
> think this is a desirable change?
>
> PROPOSAL: Use CA to look up the indexable object wrapper

+1 - Plone already has this, and it's pretty easy to do. If CMF got
this, then Plone could drop its own custom adapter and use the CMF one
to hook in plone.indexer, as could anyone else wanting to use this package.

> PROBLEM: It is not possible to provide alternative implementations of
> the IndexableObjectWrapper class at the moment - this prevents
> customising the indexing process.
>
> SOLUTION: Look up the IndexableObjectWrapper using the Component
> Architecture. The catalog tool will look up a named utility that
> creates a wrapped object suitable for indexing.

Actually, I'd suggest you look it up as an adapter on the object being
indexed. Also, it probably shouldn't be named, as there's nothing to
"switch" the name on.

> The default implementation of the utility will return the object
> wrapped using the current IndexableObjectWrapper.
>
> If required, Local Site configuration can be used to provide different
> implementations as needed.
>
> BACKWARDS COMPATIBILITY: Any catalog implementation that used a
> different wrapper class would have to subclass the main catalog and
> override the relevant function, so will be unaffected by the change.
>
> Any thoughts?

+1 in general

Martin

--
Author of `Professional Plone Development`, a book for developers who
want to work with Plone. See http://martinaspeli.net/plone-book

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

jens at dataflake

Mar 10, 2009, 6:37 AM

Post #30 of 60 (1113 views)
Permalink

Re: IndexableObjectWrapper [In reply to]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Mar 10, 2009, at 14:21 , Martin Aspeli wrote:

> The actual feature discussion happens on the medium-traffic
> framework-team list, which you can join. In fact, it'd be great if you
> did, as we'd appreciate your input, but I realise it may not be
> something you want to spend a lot of time on.

I'll join, sure. Joining does not imply activity beyond reading ;-)

If there was a narrow-scoped announce-type list that contains
announcements like PLIPs or important development decisions that may
be of interest to the CMF developers as well then it would be useful
to pipe it into this list. That's one reason I asked.

jens

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.8 (Darwin)

iEYEARECAAYFAkm2bQkACgkQRAx5nvEhZLKP4wCeK5mW5+r4Ie0s1sghrz20zqWG
gDgAnjQelFAI2qdpRiZMCog+JpCIGW0s
=JEV8
-----END PGP SIGNATURE-----
_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

hannosch at hannosch

Mar 10, 2009, 6:59 AM

Post #31 of 60 (1108 views)
Permalink

Re: IndexableObjectWrapper [In reply to]

Martin Aspeli wrote:
> Jens Vagelpohl wrote:
>> Is there any kind of low-traffic announcement list for things like
>> PLIPs? I'm not subscribed to any Plone list because of (for me at
>> least) signal to noise ratio fears.
>
> There's plone-announce, but I don't think this was announced there.

plone-announce is targeted at consumers of Plone, so has information
like "New Symposium coming up", "Plone 3.2 released".

> I actually think the Plone release manager should cross-post a few
> important announcements to this list, though.

We could probably do that. There is a lot of information which isn't
relevant at all to the CMF level nor the Zope level in the same ways.

> The actual feature discussion happens on the medium-traffic
> framework-team list, which you can join. In fact, it'd be great if you
> did, as we'd appreciate your input, but I realise it may not be
> something you want to spend a lot of time on.

One way to get quite a filtered list is to look at the PLIP's themselves
from time to time. For Plone 3.3 this is:
http://plone.org/products/plone/releases/3.3 We can make sure to post
the important dates for our release process to this list and provide at
least this kind of URL.

For Plone 4.0 we haven't probably started the process yet (my fault) but
have some things available in the listing at:
http://dev.plone.org/plone/milestone/4.0

The items on the 4.0 list here are basically all of my personal pet
peeves and there's a lot of more things we are going to do, like
completely revamp the admin UI and probably use Dexterity for the
default types, while retaining backwards compatible support with Archetypes.

Hanno

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

optilude+lists at gmail

Mar 10, 2009, 7:31 AM

Post #32 of 60 (1109 views)
Permalink

Re: IndexableObjectWrapper [In reply to]

Charlie Clark wrote:
> Am 10.03.2009 um 10:49 schrieb Wichert Akkerman:
>
>> Perhaps in the future the Plone Foundation would be willing to donate
>> code to the Zope Foundation. At this moment that is a bridge too far,
>> and I fear that as soon as I suggest that at this point in time the
>> entire relicensing movement will die in a never-ending debate. Lets
>> save that one for a later day.
>
>
> I suspect you're right on that and that we on this list can all say
> that, with hindsight, the licence debate should never have happened.

I don't think that's what Wichert said. There's a very constructive
licensing discussion going on within the Plone Foundation membership,
which is most likely going to lead to a more permissive licensing regime
for certain pages, precisely to make those packages easier to adopt in
other project.

> Let's hope it can be resolved in the future. I hope this will become
> easier as development moves to a more library-based approach -- all
> hail TurPlango!

I think it will.

> What we cannot (dual-)license for the CMF but think we need for the
> CMF we will have to reimplement.

These two statements are somewhat contradictory. You're saying that CMF
can't re-use library code written in other contexts?

Let's say CMF decided that the GS handlers would be better written with
elementtree than minidom. Should CMF demand that elementree be ZPL
licensed and move to svn.zope.org? Or should the CMF developers
re-implement elementtree?

The Zope and Python worlds are rapidly becoming comprised of smaller,
more re-usable packages. I don't think it's in anyone's interests to be
monolithic, nor to shun re-use of packages not built specifically under
the umbrella of this one project. I have a feeling the CMF developers
(of which, in a still-regrettable-too-limited capacity, I would consider
myself one) don't think so either.

> I have to agree with Jens that saying
> something was added to Plone rather than the CMF because of the Plone
> release cycle is disingenuous. As long as I have been following the
> list the CMF releases have been timed for Plone. It would be good to
> see this change from now on

I sincerely hope that Jens and Yuppie will be willing to continue the
much appreciated tradition of being willing to make releases in response
to the demands of the main consumers of the CMF, of which Plone is
arguably the biggest.

> as I suspect the number of core developers
> on both projects is limited and both projects would benefit from
> effectively pooled resources and clear ideas of what goes where.

I don't understand how having release cycles out of sync would make
resource allocation more efficient or give any clearer idea of what
packages would go where.

> I also have to agree with Jens that the Plone mailing lists are way
> too noisy.

Let me try to summarise a bit more succinctly:

- I think it'd be a good idea if the Plone release manager/framework
team announced their review cycles to this list and asked for input from
the CMF developers.

- I explained the rationale behind the evolution of one particular
package, plone.indexer.

- At no point did I suggest that the CMF release cycle was a hindrance
to pushing packages down there.

Martin

--
Author of `Professional Plone Development`, a book for developers who
want to work with Plone. See http://martinaspeli.net/plone-book

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

optilude+lists at gmail

Mar 10, 2009, 7:32 AM

Post #33 of 60 (1109 views)
Permalink

Re: IndexableObjectWrapper [In reply to]

Jens Vagelpohl wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
>
> On Mar 10, 2009, at 14:21 , Martin Aspeli wrote:
>
>> The actual feature discussion happens on the medium-traffic
>> framework-team list, which you can join. In fact, it'd be great if you
>> did, as we'd appreciate your input, but I realise it may not be
>> something you want to spend a lot of time on.
>
> I'll join, sure. Joining does not imply activity beyond reading ;-)
>
> If there was a narrow-scoped announce-type list that contains
> announcements like PLIPs or important development decisions that may
> be of interest to the CMF developers as well then it would be useful
> to pipe it into this list. That's one reason I asked.

I think there ought to be. I'll speak to the release teams.

Martin

--
Author of `Professional Plone Development`, a book for developers who
want to work with Plone. See http://martinaspeli.net/plone-book

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

optilude+lists at gmail

Mar 10, 2009, 7:35 AM

Post #34 of 60 (1116 views)
Permalink

Re: IndexableObjectWrapper [In reply to]

yuppie wrote:

> AFAICS wrapping the object before looking up adapters is unnecessary.
> The catalog should do the lookup directly and the existing features
> provided by IndexableObjectWrapper should be reimplemented as adapters.

Bear in mind that there is a difference between getting the wrapper
itself, and getting the value to catalogue for a particular *attribute*
of the wrapper (e.g. allowedRolesAndUsers).

The CatalogTool subclass in Plone solves the former. plone.indexer uses
the hook it puts in place to solve the latter.

I think at a minimum, CMF should just make the whole wrapper swappable
by looking it up as an adapter. Then people could choose to use
plone.indexer if they so needed (we'd obviously need to change
plone.indexer to provide an appropriate adapter based on a CMF interface
rather than a Plone one, and then deprecate the Plone version). For
smaller applications, it may just be unnecessary complexity.

Martin

--
Author of `Professional Plone Development`, a book for developers who
want to work with Plone. See http://martinaspeli.net/plone-book

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

y.2009 at wcm-solutions

Mar 10, 2009, 8:49 AM

Post #35 of 60 (1109 views)
Permalink

Re: IndexableObjectWrapper [In reply to]

Hi Martin!

Martin Aspeli wrote:
> yuppie wrote:
>> For the CMF project it is essential to have full control over its own
>> layer of the stack and to participate in the development of the Zope
>> layer. Using packages from the Plone repository means either using them
>> as a black box or joining the Plone Foundation to participate in their
>> development. IMO both options are not acceptable.
>
> You don't need to join the Plone Foundation to develop packages in
> svn.plone.org. You *do* need to sign a contributor agreement granting
> some IP rights over that code to the Plone Foundation, in return for a
> promise that it will always be available under an open source license.
> There are no limits on who can sign that agreement or for what purpose
> they choose to work with the code in that repository.
>
> Of course, the same goes for svn.zope.org: you need to sign a document
> and grant certain rights over your work to the Zope Foundation.
>
> If CMF really cannot use packages not in svn.zope.org, then that's a bit
> sad and will invariably lead to a lot of re-invention rather than
> re-use. I don't really understand why it is "essential" to have this
> level of control over every line of code you use. This is entirely a
> self-imposed restriction.

Just to make it clear: I'm talking about the code in the CMF and the
Zope layer. Not about lower level layers. I'm absolutely fine with
having Python and some other libraries in different repositories. And
I'm not talking about CMF users. I'm sure there are good reasons to use
Plone libraries together with the CMF.

I'm talking about closely related code. Maintaining it in different
repositories with different code ownership, license and policies creates
extra costs. Either everybody has to work in both repositories or you
have to ask people to make changes are releases. Refactoring code across
repository boundaries becomes almost impossible.

>> For technical reasons this collaboration is asymmetric. Plone is built
>> on top of Zope and CMF, not the other way round. If you want to work
>> really close with these communities, you have to be part of them and use
>> their repository.
>
> I wrote a package that, I hope, is useful. I am pretty sure it'll work
> with "plain CMF" and therefore with other consumers of the CMF, and if
> doesn't, I'd consider that a bug and fix it. I'm willing to work to make
> that package a part of the CMF platform, whether optional or fully
> integrated.
>
> To a certain extent, it already is: someone suing the CMF can choose to
> use plone.indexer in their own project, though they'll need to install
> it themselves. I don't quite see how this is asymmetric. Rather, it is
> an outcome of the evolution of this particular package.
>
> It's up to the CMF maintainers to decide whether it is desirable to
> actually adopt this package as part of a standard release.
>
> It's not always going to be appropriate (or even if it is, it's not
> always going to be the case) for every new line of code that *could* be
> useful to all/most CMF consumers to be built as part of the CMF itself
> from the outset. If the CMF project has no model for incorporating code
> from outside its (rather small) community, then I think that is more to
> the detriment of CMF than to those who created those packages and chose
> to put the effort in to reduce their dependencies and make them more
> generally useful.

I don't think it's the role of the CMF project to integrate all the code
that could be useful for people building applications. Others like Plone
can do that much better.

The CMF has to become simpler, not more complex. Third party products
always add complexity. Incorporating new code means replacing old code
in a backwards compatible way, not just adding another hook.

Code evolution is useful, but it can't replace discussions and
consolidation.

Cheers,

Yuppie

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

y.2009 at wcm-solutions

Mar 10, 2009, 8:53 AM

Post #36 of 60 (1114 views)
Permalink

Re: IndexableObjectWrapper [In reply to]

Martin Aspeli wrote:
> yuppie wrote:
>
>> AFAICS wrapping the object before looking up adapters is unnecessary.
>> The catalog should do the lookup directly and the existing features
>> provided by IndexableObjectWrapper should be reimplemented as adapters.
>
> Bear in mind that there is a difference between getting the wrapper
> itself, and getting the value to catalogue for a particular *attribute*
> of the wrapper (e.g. allowedRolesAndUsers).

Why do we need the wrapper? Why can't we look up directly the adapter
for the particular attribute?

Cheers, Yuppie

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

optilude+lists at gmail

Mar 10, 2009, 12:18 PM

Post #37 of 60 (1103 views)
Permalink

Re: IndexableObjectWrapper [In reply to]

yuppie wrote:
> Martin Aspeli wrote:
>> yuppie wrote:
>>
>>> AFAICS wrapping the object before looking up adapters is unnecessary.
>>> The catalog should do the lookup directly and the existing features
>>> provided by IndexableObjectWrapper should be reimplemented as adapters.
>> Bear in mind that there is a difference between getting the wrapper
>> itself, and getting the value to catalogue for a particular *attribute*
>> of the wrapper (e.g. allowedRolesAndUsers).
>
> Why do we need the wrapper? Why can't we look up directly the adapter
> for the particular attribute?

This could work as well. It does make it harder to change the policy en
masse for a particular type (you'd need to override all the adapters,
say). One reason to do that may be if you have a very simple object that
you know exactly how to catalogue, and you don't want the overhead of
looking up various adapters.

Still, the important use case, imho, is to make custom "indexers" for
your custom types. I quite like the pattern in plone.indexer where we
use an annotation to make a function into an indexer adapter:

http://pypi.python.org/pypi/plone.indexer

Martin

--
Author of `Professional Plone Development`, a book for developers who
want to work with Plone. See http://martinaspeli.net/plone-book

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

y.2009 at wcm-solutions

Mar 11, 2009, 5:18 AM

Post #38 of 60 (1105 views)
Permalink

Re: IndexableObjectWrapper [In reply to]

Hi Martin!

Martin Aspeli wrote:
> yuppie wrote:
>> Martin Aspeli wrote:
>>> yuppie wrote:
>>>
>>>> AFAICS wrapping the object before looking up adapters is unnecessary.
>>>> The catalog should do the lookup directly and the existing features
>>>> provided by IndexableObjectWrapper should be reimplemented as adapters.
>>> Bear in mind that there is a difference between getting the wrapper
>>> itself, and getting the value to catalogue for a particular *attribute*
>>> of the wrapper (e.g. allowedRolesAndUsers).
>> Why do we need the wrapper? Why can't we look up directly the adapter
>> for the particular attribute?
>
> This could work as well. It does make it harder to change the policy en
> masse for a particular type (you'd need to override all the adapters,
> say). One reason to do that may be if you have a very simple object that
> you know exactly how to catalogue, and you don't want the overhead of
> looking up various adapters.

I did have a closer look at this: The right place for looking up
attribute specific adapters directly is deep in the ZCatalog code. And
it might indeed be overkill to use separate adapters for several
attributes of several content types.

So now I basically agree with the solution Miles and you did propose.
But I still have some questions:

Why is IIndexableObjectWrapper in Plone a multi-adapter and not a simple
adapter for object?

Could we push this further down the stack to the ZCatalog, making it
unnecessary to override catalog_object in CMF? AFAICS all CMF-specific
stuff could be done inside the wrapper.

> Still, the important use case, imho, is to make custom "indexers" for
> your custom types. I quite like the pattern in plone.indexer where we
> use an annotation to make a function into an indexer adapter:
>
> http://pypi.python.org/pypi/plone.indexer

I agree that's an important use case, but looking up
IIndexableObjectWrapper based on the object provides already a solution
for it. So we have a basic solution inside the framework and hook for
plugging in alternative solutions like plone.indexer.

Cheers,

Yuppie

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

optilude+lists at gmail

Mar 11, 2009, 5:47 AM

Post #39 of 60 (1094 views)
Permalink

Re: IndexableObjectWrapper [In reply to]

yuppie wrote:

> Why is IIndexableObjectWrapper in Plone a multi-adapter and not a simple
> adapter for object?

Some of the indexers need access to the site root. I think the idea was
that you could have different adapters in different types of sites by
marking the site root, but a single adapter is probably enough.

> Could we push this further down the stack to the ZCatalog, making it
> unnecessary to override catalog_object in CMF? AFAICS all CMF-specific
> stuff could be done inside the wrapper.

Maybe. Touching ZCatalog is kinda scary, though.

>> Still, the important use case, imho, is to make custom "indexers" for
>> your custom types. I quite like the pattern in plone.indexer where we
>> use an annotation to make a function into an indexer adapter:
>>
>> http://pypi.python.org/pypi/plone.indexer
>
> I agree that's an important use case, but looking up
> IIndexableObjectWrapper based on the object provides already a solution
> for it. So we have a basic solution inside the framework and hook for
> plugging in alternative solutions like plone.indexer.

Yep, I think that's a good idea.

Martin

--
Author of `Professional Plone Development`, a book for developers who
want to work with Plone. See http://martinaspeli.net/plone-book

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

miles at jamkit

Mar 11, 2009, 12:20 PM

Post #40 of 60 (1081 views)
Permalink

Re: IndexableObjectWrapper [In reply to]

Hi,

<snip>

>> Could we push this further down the stack to the ZCatalog, making it
>> unnecessary to override catalog_object in CMF? AFAICS all CMF-specific
>> stuff could be done inside the wrapper.
>
> Maybe. Touching ZCatalog is kinda scary, though.

I think trying to remove catalog_object completely might require
significantly more thought. There are lots more ways in which the basic
ZCatalog can be used that we need to consider, in contrast to
CatalogTool. What happens if you have two catalogs - should they always
both apply the same wrappers? What if you want an unwrapped object to
be cataloged in one of them? It's making my head hurt ...

However, one thing that I'd like an opinion on is whether it's useful to
move the handling of workflow variables out of catalog_object and into
the IndexableObjectWrapper class? To my mind it seems cleaner to move
it, but I'm not sure on the BBB impact.

Miles

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

dieter at handshake

Mar 11, 2009, 12:52 PM

Post #41 of 60 (1075 views)
Permalink

Re: IndexableObjectWrapper [In reply to]

Wichert Akkerman wrote at 2009-3-10 10:01 +0100:
> ...
>The debate is currently focusing on GPL versus BSD license. Any opinions
>on a choice between those two would be very welcome.

I like BSD more, but could live with LGPL as well.

--
Dieter
_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

dieter at handshake

Mar 11, 2009, 12:52 PM

Post #42 of 60 (1081 views)
Permalink

Re: IndexableObjectWrapper [In reply to]

y.2009 at wcm-solutions

Mar 11, 2009, 1:06 PM

Post #43 of 60 (1075 views)
Permalink

Re: IndexableObjectWrapper [In reply to]

Hi!

Miles wrote:
> Hi,
>
> <snip>
>
>>> Could we push this further down the stack to the ZCatalog, making it
>>> unnecessary to override catalog_object in CMF? AFAICS all CMF-specific
>>> stuff could be done inside the wrapper.
>> Maybe. Touching ZCatalog is kinda scary, though.
>
> I think trying to remove catalog_object completely might require
> significantly more thought. There are lots more ways in which the basic
> ZCatalog can be used that we need to consider, in contrast to
> CatalogTool. What happens if you have two catalogs - should they always
> both apply the same wrappers? What if you want an unwrapped object to
> be cataloged in one of them? It's making my head hurt ...

It would be an additional hook. If no wrapper is registered the behavior
of ZCatalog would be exactly the same as now. But so far we don't have
decided to make Zope 2.12 required for CMF 2.2, so we have start with an
implementation in CMF. For now I just want to make sure we don't add
CMF-specific stuff if we don't need it.

> However, one thing that I'd like an opinion on is whether it's useful to
> move the handling of workflow variables out of catalog_object and into
> the IndexableObjectWrapper class? To my mind it seems cleaner to move
> it, but I'm not sure on the BBB impact.

+1

I can't see any additional BBB issues. Who ever uses a customized
IndexableObjectWrapper uses also a customized catalog_object method.

I'm still not sure if we should just adapt the object or also the portal
or the catalog:

Registering different adapters for different portals doesn't make much
sense to me. If you need portal specific behavior you can register local
adapters. Registering different adapters for different kinds of catalogs
might be more useful and while 'portal' is a CMF specific concept the
catalog itself exists always.

The other reason for adapting portal or catalog is that we want to use
them inside the adapter. We need some kind of context for looking up
stuff like workflow variables. But do we need the portal, the context of
the catalog or the context of the object? If the context of the object
is sufficient, we don't need a multi-adapter. If we just need the
catalog and its context, we still have a generic solution for Zope 2. If
we need the portal, we have a CMF-specific solution.

Cheers,

Yuppie

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

miles at jamkit

Mar 13, 2009, 4:20 AM

Post #44 of 60 (1061 views)
Permalink

Re: IndexableObjectWrapper [In reply to]

Hi,

Good point about the Zope version.

>> However, one thing that I'd like an opinion on is whether it's useful to
>> move the handling of workflow variables out of catalog_object and into
>> the IndexableObjectWrapper class? To my mind it seems cleaner to move
>> it, but I'm not sure on the BBB impact.
>
> +1
>
> I can't see any additional BBB issues. Who ever uses a customized
> IndexableObjectWrapper uses also a customized catalog_object method.
>
>
> I'm still not sure if we should just adapt the object or also the portal
> or the catalog:
>
> Registering different adapters for different portals doesn't make much
> sense to me. If you need portal specific behavior you can register local
> adapters. Registering different adapters for different kinds of catalogs
> might be more useful and while 'portal' is a CMF specific concept the
> catalog itself exists always.
>
> The other reason for adapting portal or catalog is that we want to use
> them inside the adapter. We need some kind of context for looking up
> stuff like workflow variables. But do we need the portal, the context of
> the catalog or the context of the object? If the context of the object
> is sufficient, we don't need a multi-adapter. If we just need the
> catalog and its context, we still have a generic solution for Zope 2. If
> we need the portal, we have a CMF-specific solution.

My thinking was that, from a given object, we can always get the portal
(or indeed whatever object the wrapper needs). Local adapters is the
normal route for portal-specific behaviour, so we should stick to that
to keep everything in one place.

Using a multi-adapter, and adapting the catalog would appear to be
sensible - and as you say, could be generic for zope 2 in future. In
some sites, I've derived from catalog so as to use the
allowedRolesAndUsers and date filtering behaviours in other catalogs
within the portal. So I can definitely see a use-case for it.

So +1 from me to a multi-adapter on (object, catalog).

Miles

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

miles at jamkit

Mar 16, 2009, 9:49 AM

Post #45 of 60 (1043 views)
Permalink

IndexableObjectWrapper [In reply to]

Hi,

This change is now done, and checked in on a branch.

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

- 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?

Otherwise, the branch is here and all the tests pass:
/Products.CMFCore/branches/miwa-catalog-adapter/Products/CMFCore

Thanks,

Miles

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

y.2009 at wcm-solutions

Mar 17, 2009, 4:32 AM

Post #46 of 60 (1032 views)
Permalink

Re: IndexableObjectWrapper [In reply to]

Hi Miles!

Miles wrote:
> This change is now done, and checked in on a branch.

Great!

> 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.

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.

> - 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?

> 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.

Cheers,

Yuppie

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

miles at jamkit

Mar 17, 2009, 8:20 AM

Post #47 of 60 (1034 views)
Permalink

Re: IndexableObjectWrapper [In reply to]

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

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

tseaver at palladion

Mar 17, 2009, 10:59 AM

Post #48 of 60 (1036 views)
Permalink

Re: IndexableObjectWrapper [In reply to]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

yuppie wrote:

>> - 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?

Agreed. I had expected that the catalog would do a queryAdapter, and
default to the existing wrapper class if not found.

Tres.
- --
===================================================================
Tres Seaver +1 540-429-0999 tseaver [at] palladion
Palladion Software "Excellence by Design" http://palladion.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFJv+To+gerLs4ltQ4RAvt/AJ9D75eOsZh7FINca9MJsnTU26998gCghu8C
TaiqRzwLe2WDoHIqaD7qGhs=
=kd1W
-----END PGP SIGNATURE-----

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

y.2009 at wcm-solutions

Mar 18, 2009, 3:58 AM

Post #49 of 60 (1032 views)
Permalink

Re: IndexableObjectWrapper [In reply to]

Hi!

Tres Seaver wrote:
> yuppie wrote:
>> But
>> can't we make the adapter lookup in catalog_object optional and wouldn't
>> that make test setups simpler?
>
> Agreed. I had expected that the catalog would do a queryAdapter, and
> default to the existing wrapper class if not found.

Well. I had expected it would default to the unwrapped object.
Restricted catalog queries rely on allowedRolesAndUsers, but AFAICS it
is not essential for CMFCore that the object is wrapped before cataloged.

The current code on the branch requires the new IIndexableObject
interface. I'm not sure if we should make it required. It makes things
more explicit, but creates an additional burden for simple use cases and
unit tests. It should be sufficient to assume that objects without
registered adapter are indexable unwrapped.

Cheers,

Yuppie

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

y.2009 at wcm-solutions

Mar 18, 2009, 3:59 AM

Post #50 of 60 (1029 views)
Permalink

Re: IndexableObjectWrapper [In reply to]

Hi!

Miles wrote:
>> 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.

Sorry. But it looks like you did get my point anyway.

> If the object already provides this
> index, no wrapping is carried out. I added some tests for this.

I'm afraid there are still different opinions about useful fallbacks.
Please see my reply to Tres.

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

In the long run I'd like to get rid of ICatalogAware. Improving the
usage of object events should make it obsolete.

The IndexableObjectWrapper doesn't use any features of ICatalogAware, so
I can't see a need to use it here.

>> 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.

Great!

Cheers,

Yuppie

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

1 2 3

View All

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads