Item9264: %QUERY might operate on outdated topic revision

%QUERY must always evaluate the expression on the latest revision. Might happen it gets to see one which cached an older revision. This problem is hard to reproduce as it is triggered by sideeffects that other plugins have on the Meta cache. It is fixed by force-reloading the latest revision, as %FORMFIELD does.

--- lib/Foswiki/Macros/QUERY.pm (revision 8036)
+++ lib/Foswiki/Macros/QUERY.pm (working copy)
@@ -13,6 +13,10 @@
     $expr = '' unless defined $expr;
     my $style = lc( $params->{style} || '' );
 
+        
+    # force-reload latest revision
+    $topicObject->reload(0);
+
     # Recursion block.
     $this->{evaluatingEval} ||= {};

Me wondering if there's a performance problem here, when every now and then code force-reloads revisions. Wouldn't it be better to detect the latest revision is loaded, or the cached revision is not the top rev?

-- MichaelDaum - 06 Jul 2010

Kinda. As it says in Meta.pm:

Unloaded objects return undef from getLoadedRev, or the loaded revision otherwise. reload allows you to load different revisions of the same topic.

So in this case $topicObject->reload(0) unless defined $topicObject->getLoadedRev() would be better. Arguably any topic load should "remember" the top rev, so it can know if the loaded rev is the latest. However that would require loading the entire history whenever a .txt is younger than the corresponding ,v, and I elected not to do that.

-- CrawfordCurrie - 06 Jul 2010

$topicObject->getLoadedRev() doesn't work as it happily returns 1 when rev 1 was loaded. So it isn't reloading rev 5 which is latest.

-- MichaelDaum - 06 Jul 2010

Foswiki::Meta has a latestIsLoaded() method.

$topicObject->reload(0) unless $topicObject->latestIsLoaded();

should do the trick. And before you ask, no, it doesn't load the entire revision history.

-- CrawfordCurrie - 07 Jul 2010

adding the latestIsLoaded() fixes QUERY. Still each call to latestIsLoaded() results in a fork of rlog. Not so FORMFIELD: several of them only cost one fork per topic.

-- MichaelDaum - 07 Jul 2010

As we discussed on IRC, the cache that FORMFIELD uses is in the wrong place, and is unreliable. The right place to cache to avoid repeated rlog calls is in the store impl, because it's there that is the only place that knows a cache is useful. A good store impl will not impose that burden on its callers.

I would have implemented this optimisation, were it not for all the other things I'm doing. It's not going to happen for 1.1 now - but of course, it could be released as a contrib.

-- CrawfordCurrie - 07 Jul 2010

Closing this bug report as its main concern has been fixed. Anything else discussed here needs a new effort and task.

-- MichaelDaum - 07 Jul 2010