Item8848: the TOPICINFO fields in the cache cannot be trusted; a new …

…rev might be committed without an update to the topicinfo, or the cache topic might be written to without a checkin. In iether case, the topicinfo is wrong. So, consider the head of the RCS history to be the master source of revision info, but only load it when it's actually asked for e.g. for rendering or comparisons. We still might have a cache topic that is ahead of the rev history, but that's unavoidable and will self-repeait the next time that topic is properly committed.

git-svn-id: http://svn.foswiki.org/trunk@7213 0b4bb1d4-4e5a-0410-9cc4-b2b747904278

UnitTestContrib/test/unit/Fn_IF.pm

@@ -1254,6 +1254,7 @@ PONG
 
     $meta =
       Foswiki::Meta->load( $this->{session}, $this->{test_web}, $topicName );
+    $meta->getRevisionInfo();
     my $ti = $meta->get('TOPICINFO');
 
     $this->assert_str_equals( $ti->{version}, '1.1' );

UnitTestContrib/test/unit/Fn_REVINFO.pm

@@ -173,16 +173,13 @@ sub test_compatibility1 {
     {
         return;
     }
-    $this->assert(
-        open(
-            F, '>', "$Foswiki::cfg{DataDir}/$this->{test_web}/CrikeyMoses.txt"
-        )
-    );
-    print F <<'HERE';
+    my $topicObject =
+      Foswiki::Meta->new(
+          $this->{session}, $this->{test_web}, 'CrikeyMoses', <<'HERE');
 %META:TOPICINFO{author="ScumBag" date="1120846368" format="1.1" version="$Rev$"}%
 HERE
-    close(F);
-    my $topicObject =
+    $topicObject->save();
+    $topicObject =
       Foswiki::Meta->load( $this->{session}, $this->{test_web}, 'CrikeyMoses' );
     my $ui =
       $topicObject->expandMacros('%REVINFO{format="$username $wikiname"}%');
@@ -201,16 +198,13 @@ sub test_compatibility2 {
     {
         return;
     }
-    $this->assert(
-        open(
-            F, '>', "$Foswiki::cfg{DataDir}/$this->{test_web}/CrikeyMoses.txt"
-        )
-    );
-    print F <<'HERE';
+    my $topicObject =
+      Foswiki::Meta->new(
+          $this->{session}, $this->{test_web}, 'CrikeyMoses', <<'HERE');
 %META:TOPICINFO{author="scum" date="1120846368" format="1.1" version="$Rev$"}%
 HERE
-    close(F);
-    my $topicObject =
+    $topicObject->save();
+    $topicObject =
       Foswiki::Meta->load( $this->{session}, $this->{test_web}, 'CrikeyMoses' );
     my $ui =
       $topicObject->expandMacros('%REVINFO{format="$username $wikiname"}%');

UnitTestContrib/test/unit/StoreSmokeTests.pm

@@ -272,7 +272,7 @@ sub verify_releaselocksonsave {
 
     # get the date
     my $m = Foswiki::Meta->load( $this->{session}, $this->{test_web}, $topic );
-    my $t1 = $m->get('TOPICINFO')->{date};
+    my $t1 = $m->getRevisionInfo()->{date};
 
     # create rev 2 as TestUser1
     $query = new Unit::Request(

UnitTestContrib/test/unit/StoreTests.pm

@@ -298,6 +298,7 @@ sub test_beforeSaveHandlerChangeText {
 
     # remove topicinfo, useless for test
     $readMeta->remove('TOPICINFO');
+    $meta->remove('TOPICINFO');
 
     # set expected meta
     $meta->putKeyed( 'FIELD', { name => 'fieldname', value => 'text' } );
@@ -330,7 +331,6 @@ sub test_beforeSaveHandlerChangeMeta {
     $meta->putKeyed( "FIELD", $args );
     $meta->save( user => $this->{test_user_login} );
     $this->assert( $this->{session}->topicExists( $web, $topic ) );
-
     my $readMeta = Foswiki::Meta->load( $this->{session}, $web, $topic );
     my $readText = $readMeta->text;
 
@@ -341,8 +341,10 @@ sub test_beforeSaveHandlerChangeMeta {
 
     # set expected meta
     $meta->putKeyed( 'FIELD', { name => 'fieldname', value => 'meta' } );
-    delete $meta->get( 'TOPICINFO')->{rev};
-    delete $readMeta->get( 'TOPICINFO')->{rev};
+    foreach my $fld qw(rev version date) {
+        delete $meta->get( 'TOPICINFO')->{$fld};
+        delete $readMeta->get( 'TOPICINFO')->{$fld};
+    }
     $this->assert_str_equals( $meta->stringify(), $readMeta->stringify() );
     my $webObject = Foswiki::Meta->new( $this->{session}, $web );
     $webObject->removeFromStore();
@@ -383,8 +385,10 @@ sub test_beforeSaveHandlerChangeBoth {
 
     # set expected meta
     $meta->putKeyed( 'FIELD', { name => 'fieldname', value => 'meta' } );
-    delete $meta->get( 'TOPICINFO' )->{rev};
-    delete $readMeta->get( 'TOPICINFO')->{rev};
+    foreach my $fld qw(rev version date) {
+        delete $meta->get( 'TOPICINFO')->{$fld};
+        delete $readMeta->get( 'TOPICINFO')->{$fld};
+    }
     $this->assert_str_equals( $meta->stringify(), $readMeta->stringify() );
     my $webObject = Foswiki::Meta->new( $this->{session}, $web );
     $webObject->removeFromStore();

core/data/System/MetaData.txt

@@ -58,6 +58,8 @@ This macro caches some of the information that would normally be derived from th
 | format  | Format of this topic, will be used for automatic format conversion |
 | reprev  | Set when a revision is overwritten by the same author within the {ReplaceIfEditedAgainWithin} window (set in [[%SCRIPTURL{configure}%][ =configure= ]]). If =reprev= is the same as =version=, it prevents Foswiki from attempting to do a 3-way merge when merging overlapping edits by two different users. |
 
+Note that the *version and date fields are advisory only* and cannot be trusted. This is because processes outside of Foswiki's control may write topic files without maintaining these fields.
+
 ---+++ META:TOPICMOVED
 
 This only exists if the topic has been moved. If a topic is moved more than once, only the most recent META:TOPICMOVED meta datum exists in the topic. Older ones can to be found in the topic history.

core/lib/Foswiki/Meta.pm

@@ -1062,25 +1062,27 @@ Set TOPICINFO information on the object, as specified by the parameters.
 sub setRevisionInfo {
     my ( $this, $data ) = @_;
 
+    my $ti = $this->get('TOPICINFO') || {};
+
+    foreach my $k (keys %$data) {
+        $ti->{$k} = $data->{$k};
+    }
+
     # compatibility; older versions of the code use
     # RCS rev numbers. Save with them so old code can
     # read these topics
     my %args = %$data;
-    $args{version} = 1 if $args{version} < 1;
-    $args{version} = '1.' . $args{version};
-    $args{format}  = $EMBEDDING_FORMAT_VERSION;
+    $ti->{version} = 1 if $ti->{version} < 1;
+    $ti->{rev} = $ti->{version};
+    $ti->{version} = '1.' . $ti->{version};
+    $ti->{format}  = $EMBEDDING_FORMAT_VERSION;
 
-    $this->put( 'TOPICINFO', \%args );
+    $this->put( 'TOPICINFO', $ti );
 }
 
 =begin TML
 
----++ ObjectMethod getRevisionInfo($fromrev) -> \%info
-
-   * =$fromrev= revision number. If 0, undef, or out-of-range, will get info about the most recent revision.
-
-Try and get revision info from the meta information, or, if it is not
-present, kick down to the Store module for the same information.
+---++ ObjectMethod getRevisionInfo() -> \%info
 
 Return %info with at least:
 | date | in epochSec |
@@ -1094,8 +1096,18 @@ sub getRevisionInfo {
     my $this = shift;
 
     my $info;
+
+    # This used to try and get revision info from the meta
+    # information and only kick down to the Store module for the
+    # same information if it was not present. However there have
+    # been several cases where the meta information in the cache
+    # is badly out of step with the store, and the conclusion is
+    # that it can't be trusted. For this reason, when meta is read
+    # TOPICINFO version field is automatically undefined, which
+    # forces this function to re-get it from the store.
     my $topicinfo = $this->get('TOPICINFO');
-    if ($topicinfo) {
+
+    if ($topicinfo && defined $topicinfo->{version}) {
         $info = {
             date    => $topicinfo->{date},
             author  => $topicinfo->{author},
@@ -1119,12 +1131,13 @@ sub getRevisionInfo {
     return $info;
 }
 
-# Determins, and caches, the topic revision info of the base version,
-#warning: this is a horrid little legacy of the InfoCache object, and should be done away with.
+# Determines, and caches, the topic revision info of the base version,
+# SMELL: this is a horrid little legacy of the InfoCache object, and
+# should be done away with.
 sub getRev1Info {
     my ( $this, $attr ) = @_;
 
-#my ( $web, $topic ) = Foswiki::Func::normalizeWebTopicName( $this->{_defaultWeb}, $webtopic );
+    #my ( $web, $topic ) = Foswiki::Func::normalizeWebTopicName( $this->{_defaultWeb}, $webtopic );
     my $web   = $this->web;
     my $topic = $this->topic;
 
@@ -1536,6 +1549,10 @@ sub save {
 
     my $plugins = $this->{_session}->{plugins};
 
+    # make sure version and date in TOPICINFO are up-to-date
+    # (side effect of getRevisionInfo)
+    $this->getRevisionInfo();
+
     # Semantics inherited from Cairo. See
     # Foswiki:Codev.BugBeforeSaveHandlerBroken
     if ( $plugins->haveHandlerFor('beforeSaveHandler') ) {
@@ -2958,18 +2975,15 @@ sub setEmbeddedStoreForm {
         # Make sure we update the topic format for when we save
         $ti->{format} = $EMBEDDING_FORMAT_VERSION;
 
-        # add the rev derived from version=''
-        if ( $ti->{version} ) {
-            if ( $ti->{version} =~ /(\d+\.)?(\d*)/ ) {
-                $ti->{rev} = $2;
-            }
-            else {
-                $ti->{rev} = 1;
-            }
-        }
-        else {
-            $ti->{version} = $ti->{rev} = 0;
-        }
+        # The version, date and rev cached in TOPICINFO *cannot be trusted*
+        # See Tasks.Item8848
+        # These fields will be populated from the store when
+        # getRevisionInfo() is called. Important: always do this
+        # before calling get('TOPICINFO') if you want to use the
+        # rev info!
+        delete $ti->{version};
+        delete $ti->{date};
+        delete $ti->{rev};
     }
 
     # Other meta-data

core/lib/Foswiki/Search/InfoCache.pm

@@ -396,6 +396,7 @@ sub _getListOfWebs {
                     }
                     else {
                         $web = Foswiki::Sandbox::untaint( $web,\&Foswiki::Sandbox::validateWebName );
+ASSERT($web);
                         push( @tmpWebs, $web );
                         $webObject = Foswiki::Meta->new( $session, $web );
                     }
@@ -406,6 +407,7 @@ sub _getListOfWebs {
                           unless $Foswiki::WebFilter::user_allowed->ok(
                             $session, $w );
                         $w = Foswiki::Sandbox::untaint( $w,\&Foswiki::Sandbox::validateWebName );
+ASSERT($web);
                         push( @tmpWebs, $w );
                     }
                 }
@@ -438,6 +440,7 @@ sub _getListOfWebs {
 
     my @webs;
     foreach my $web (@tmpWebs) {
+        next unless defined $web;
         push( @webs, $web ) unless $excludeWeb{$web};
         $excludeWeb{$web} = 1;    # eliminate duplicates
     }

core/lib/Foswiki/Store/QueryAlgorithms/BruteForce.pm

@@ -164,6 +164,10 @@ sub getField {
         if ( $Foswiki::Query::Node::aliases{$field} ) {
             $realField = $Foswiki::Query::Node::aliases{$field};
         }
+        if ($realField eq 'META:TOPICINFO') {
+            # Ensure the revision info is populated from the store
+            $data->getRevisionInfo();
+        }
         if ( $realField =~ s/^META:// ) {
             if ( $Foswiki::Query::Node::isArrayType{$realField} ) {
 

core/lib/Foswiki/UI/Save.pm

@@ -262,6 +262,7 @@ sub buildNewTopic {
 
             require Foswiki::Merge;
 
+            $topicObject->getRevisionInfo();
             my $pti = $topicObject->get('TOPICINFO');
             if (   $pti->{reprev}
                 && $pti->{version}