[ic] [timed-build] not working for logged in users

[Kevin Walsh]

Peter <peter at pajamian.dhs.org> wrote:
> On 07/11/2006 05:54 PM, Jon Jensen wrote:
> > On Tue, 11 Jul 2006, Peter wrote:
> > 
> >>> If you consider timed builds a necessity, including for logged-in 
> >>> users (as I do for many sites) then you need to generate links on the 
> >>> fly so that they can contain session IDs if necessary. We do this on 
> >>> a few sites by having a wrapper around timed-build:
> >>
> >>
> >> This is unnecessary overhead for people who have a session cookie, 
> >> though, overhead that [timed-build] is designed to minimise.  It would 
> >> be better to only do this for people who don't pass a session cookie.
> >>
> >> [timed-build] as it is currently is designed to abort (not use the 
> >> cache) for people without a session cookie if login=1 is passed (or if 
> >> it isn't passed, but we're discussing using login=1 or auto=1 instead 
> >> of force=1).  Since a very small number of users don't have cookies it 
> >> is not a big deal (imo) to have the extra overhead of aborting 
> >> [timed-build] for those users, also since this is the documented and 
> >> coded functionality it's best (imo) to just leave it and fix the bugs 
> >> in it rather than to try to add more functionality to make up for it.
> > 
> > Your use case is not the same as mine, apparently. The cost to do even a 
> > single database query is vastly greater than the cost to interpolate 
> > some [area] tags. The cost to do a complex database query can be much, 
> > much higher, so saving that for even the relatively few logged-in users 
> > without cookies can be well worth the tiny cost.
> 
> Fair enough.  So how about doing the best of both worlds?  If there is a 
> session cookie there's no reason to go through and interpolate the 
> [area] tags.  If there isn't then we can come up with some logic to do 
> that transparently, behind the scenes in [timed-build]?
> 
> >> The bug in this case (I haven't fully tested it so I can't say for 
> >> sure) is that $Vend::Cookie is not being set (or is being unset) for 
> >> logged in users somewhere, so the test for $Vend::Cookie is failing 
> >> and the [timed-build] is aborting when login=1 is passed.
> > 
> > UserDB clears $Vend::Cookie at login time. I don't know why.
> 
> Then either that behavior should be fixed, or [timed-login] needs to be 
> changed so that it checks something more reliable than $Vend::Cookie.
> 
> The next step would probably be to determine if there is any other code 
> which relies on this strange behavior of $Vend::Cookie.
> 
The following patch works for me.  You can try it, but you should be
aware that it could possibly cause unintended side-effects.  I have been
running with the patch in since 21 March 2006 and haven't found any yet.

----------------------------------------------------------------------
--- Dispatch.pm 27 Jun 2006 14:24:42 -0000      1.69
+++ Dispatch.pm 14 Jul 2006 01:45:48 -0000
@@ -1394,8 +1394,6 @@

        $Vend::Session->{'user'} = $CGI::user;

-       undef $Vend::Cookie if  $Vend::Session->{logged_in};
-
        $CGI::pragma = 'no-cache'
                if delete $::Scratch->{mv_no_cache};
 #show_times("end session get") if $Global::ShowTimes;
----------------------------------------------------------------------

The above patch appears to clear a problem in the following from
Vend::Interpolate::timed_build():

----------------------------------------------------------------------
        return Vend::Interpolate::interpolate_html($_[0])
                if $abort
                or ( ! $opt->{force}
                                and
                                (   ! $Vend::Cookie
                                        or ! $opt->{login} && $Vend::Session->{logged_in}
                                )
                        );
----------------------------------------------------------------------

As you can probably see, the $opt->{login} test is irrelevant if the
flag is always cleared for logged-in users.

Using the "force" parameter causes session IDs to be stored in the
timed-build cache file.

With the patch above patch, the following happens:

    * force=1 forces the cache to be built (if out of date) regardless
      of whether or not that will cause session IDs to be included.
      Only use that parameter if there are no internal links.

    * Users without a cookie-maintained session never get (or build)
      the cache.

    * Non-logged-in users (with cookies) get (or build) the cache.

    * Logged-in users (with cookies and with login=1) get (or build)
      the cache.

    * Logged-in users (with cookies and without login=1) never get
      (or build) the cache.

That [timed-build] snippet has far too many negative tests for me to get
my head around sometimes. :-)

I recommend the above patch, but haven't committed it to the core because
concerns, within the icdevgroup, were raised about possible unknown
side-effects.  As I said, it has been working for me.  It would be helpful
if others could try it, in a non-critical environment, and report any
problems they find.

-- 
   _/   _/  _/_/_/_/  _/    _/  _/_/_/  _/    _/
  _/_/_/   _/_/      _/    _/    _/    _/_/  _/   K e v i n   W a l s h
 _/ _/    _/          _/ _/     _/    _/  _/_/    kevin at cursor.biz
_/   _/  _/_/_/_/      _/    _/_/_/  _/    _/