Sidnei da Silva wrote:
On Mon, Oct 31, 2005 at 10:05:45AM -0500, Jim Fulton wrote:
| Sidnei da Silva wrote:
| >Found some lovely piece of code deep into the FTP parts of Zope 2 last
| >saturday, one of them is truely ugly. It's listing the contents of
| >the current and parent folders for no apparent reason (or at least, it
| >didn't make sense either to me or Chris McDonough).
| | There's a comment stating the intent.

Yes, but the original code does the check in a truely obscure way, at
least to me. I've thought of spelling:

        # check to see if we are acquiring our objectValues or not
        if not (len(REQUEST.PARENTS) > 1 and
                self.objectValues() == REQUEST.PARENTS[1].objectValues()):


        if self.objectValues.im_self is not self:

That might be better.

However I'm not sure that would be the be the correct
replacement. There doesn't seem to exist any test for this.

Of course, there's no test for this.  We weren't doing automated
testing back then.  Feel free to write one.

| >The code in question is in the ``manage_FTPstat`` method of
| >``OFS.ObjectManager``. Tracing back the source of this code, it seems
| >to have been (surprisingly) introduced by Jim Fulton, back in
| >1999. [1]
| >
| >Later on, Amos Latteier changed part of the code (namely
| >``manage_FTPlist``) to use a slightly more friendlier, yet equally
| >confusing ``is_acquired`` method. [2]
| >
| >I'm now sitting here, trying to make sense of this code and wondering
| >what was the original intention in the first place. Would anyone have
| >a clue?
| | I think the comments make this pretyu clear. The intent is to avoid
| providing listings of acquired objects.  In fact, the intent of both
| bits of code, I think is to prevent FTP access to acquired objects.

Wouldn't that be better implemented by making the same check as it's
done for WebDAV and set 'maybe_webdav_client' (abusing the name)
so that it sets the 'no_acquire_flag'?


| I suspect that there is a clearer way to implement the intent.

I have three suggestions right now:

 1. change manage_FTPstat to use the same thing as manage_FTPlist
    (using App.Common.is_acquired, and possibly rewriting is_acquired
    to a simpler check)

That's an improvement,


 2. Throw away the `is_acquired` check and replace it by a much
    simpler check

Sounds good, doing so consistently.


 3. Setting 'maybe_webdav_client' in the request object so the object
    is never acquired for FTP, the same way as it's done for WebDAV.

Possibly.  I don't have time to look at that.


Jim Fulton           mailto:[EMAIL PROTECTED]       Python Powered!
CTO                  (540) 361-1714  
Zope Corporation
Zope-Coders mailing list

Reply via email to