Item13078: problem existed not in PFS but in the core Store, which wa…

…s calling an undocumented method in the RCS store. Took the opportunity to clean up the API further and rename the ever-confusing VC subdirectory.

RCSStoreContrib/lib/Foswiki/Store/RcsLite.pm

@@ -6,12 +6,12 @@
 
 Implementation of =Foswiki::Store= for stores that use the RCS version
 control system to manage disk files. This class inherits most of its
-functionality from =Foswiki::Store::VC::Store=, which it shares with
+functionality from =Foswiki::Store::Rcs::Store=, which it shares with
 =Foswiki::Store::RcsWrap=.
 
 For readers who are familiar with Foswiki version 1.0, this class
 has no equivalent in Foswiki 1.0. The equivalent of the old
-=Foswiki::Store::RcsLite= is the new =Foswiki::Store::VC::RcsLiteHandler=.
+=Foswiki::Store::RcsLite= is the new =Foswiki::Store::Rcs::RcsLiteHandler=.
 
 =cut
 
@@ -20,14 +20,14 @@ package Foswiki::Store::RcsLite;
 use strict;
 use warnings;
 
-use Foswiki::Store::VC::Store ();
-our @ISA = ('Foswiki::Store::VC::Store');
+use Foswiki::Store::Rcs::Store ();
+our @ISA = ('Foswiki::Store::Rcs::Store');
 
-use Foswiki::Store::VC::RcsLiteHandler ();
+use Foswiki::Store::Rcs::RcsLiteHandler ();
 
 sub getHandler {
     my $this = shift;
-    return new Foswiki::Store::VC::RcsLiteHandler( $this, @_ );
+    return new Foswiki::Store::Rcs::RcsLiteHandler( $this, @_ );
 }
 
 1;

RCSStoreContrib/lib/Foswiki/Store/RcsWrap.pm

@@ -6,12 +6,12 @@
 
 Implementation of =Foswiki::Store= for stores that use the RCS version
 control system to manage disk files. This class inherits most of its
-functionality from =Foswiki::Store::VC::Store=, which it shares with
+functionality from =Foswiki::Store::Rcs::Store=, which it shares with
 =Foswiki::Store::RcsLite=.
 
 For readers who are familiar with Foswiki version 1.0, this class
 has no equivalent in Foswiki 1.0. The equivalent of the old
-=Foswiki::Store::RcsWrap= is the new =Foswiki::Store::VC::RcsWrapHandler=.
+=Foswiki::Store::RcsWrap= is the new =Foswiki::Store::Rcs::RcsWrapHandler=.
 
 =cut
 
@@ -20,14 +20,14 @@ package Foswiki::Store::RcsWrap;
 use strict;
 use warnings;
 
-use Foswiki::Store::VC::Store ();
-our @ISA = ('Foswiki::Store::VC::Store');
+use Foswiki::Store::Rcs::Store ();
+our @ISA = ('Foswiki::Store::Rcs::Store');
 
-use Foswiki::Store::VC::RcsWrapHandler ();
+use Foswiki::Store::Rcs::RcsWrapHandler ();
 
 sub getHandler {
     my $this = shift;
-    return new Foswiki::Store::VC::RcsWrapHandler( $this, @_ );
+    return new Foswiki::Store::Rcs::RcsWrapHandler( $this, @_ );
 }
 
 1;

RCSStoreContrib/test/unit/RCSStoreContrib/RCSHandlerTests.pm

@@ -1,6 +1,6 @@
-# Tests for low-level RCS handler code. Store::VC::Store creates a
+# Tests for low-level RCS handler code. Store::Rcs::Store creates a
 # transitory handler object for each store item. The handler
-# behaviour is only exposed to Store::VC::Store, which is in turn tested
+# behaviour is only exposed to Store::Rcs::Store, which is in turn tested
 # in VCStoreTests.
 
 package RCSHandlerTests;
@@ -17,8 +17,8 @@ sub new {
 
 use Foswiki;
 use Foswiki::Store;
-use Foswiki::Store::VC::RcsLiteHandler;
-use Foswiki::Store::VC::RcsWrapHandler;
+use Foswiki::Store::Rcs::RcsLiteHandler;
+use Foswiki::Store::Rcs::RcsWrapHandler;
 use File::Path;
 use FoswikiStoreTestCase ();
 
@@ -46,13 +46,13 @@ my @rcsTypes = qw/Lite Wrap/;    # SMELL: can't skip if no RCS installed
 sub RcsLite {
     my $this = shift;
     $Foswiki::cfg{Store}{Implementation} = 'Foswiki::Store::RcsLite';
-    $class = 'Foswiki::Store::VC::RcsLiteHandler';
+    $class = 'Foswiki::Store::Rcs::RcsLiteHandler';
 }
 
 sub RcsWrap {
     my $this = shift;
     $Foswiki::cfg{Store}{Implementation} = 'Foswiki::Store::RcsWrap';
-    $class = 'Foswiki::Store::VC::RcsWrapHandler';
+    $class = 'Foswiki::Store::Rcs::RcsWrapHandler';
 }
 
 sub RcsWrap_coMustCopy {
@@ -129,7 +129,7 @@ sub test_mktmp {
 
     # this is only used on WINDOWS so needs a special test
     my $this    = shift;
-    my $tmpfile = Foswiki::Store::VC::Handler::mkTmpFilename();
+    my $tmpfile = Foswiki::Store::Rcs::Handler::mkTmpFilename();
     $this->assert( !-e $tmpfile );
 }
 
@@ -267,7 +267,7 @@ sub verify_RcsWrapOnly_ciLocked {
     my $topic = "CiTestLockedTempDeleteMeItsOk";
 
     # create the fixture
-    my $rcs = Foswiki::Store::VC::RcsWrapHandler->new( new StoreStub,
+    my $rcs = Foswiki::Store::Rcs::RcsWrapHandler->new( new StoreStub,
         $testWeb, $topic, "" );
     $rcs->addRevisionFromText( "Shooby Dooby", "original", "BungditDin" );
 
@@ -546,7 +546,7 @@ sub checkDifferences {
     my $diff = $rcs->revisionDiff( 1, 2 );
 
     # apply the differences to the text of topic 1
-    my $data = Foswiki::Store::VC::RcsLiteHandler::_split($from);
+    my $data = Foswiki::Store::Rcs::RcsLiteHandler::_split($from);
     my $l    = 0;
 
     #print STDERR "\nStart: ",join('\n',@$data),"\n";
@@ -963,7 +963,7 @@ sub test_Item945 {
     my $testTopic = "TestItem945";
     for my $depth ( 0 .. $#historyItem945 ) {
         my ( $rcsType, @params ) = @{ $historyItem945[$depth] };
-        my $class = "Foswiki::Store::VC::Rcs${rcsType}Handler";
+        my $class = "Foswiki::Store::Rcs::Rcs${rcsType}Handler";
         my $rcs = $class->new( new StoreStub, $testWeb, $testTopic );
         $rcs->addRevisionFromText(@params);
         $rcs->finish();
@@ -975,9 +975,10 @@ sub item945_checkHistory {
     my ( $this, $depth, $testWeb, $testTopic ) = @_;
     for my $rcsType (@rcsTypes) {
         my $rcs =
-          "Foswiki::Store::VC::Rcs${rcsType}Handler"->new( new StoreStub,
+          "Foswiki::Store::Rcs::Rcs${rcsType}Handler"->new( new StoreStub,
             $testWeb, $testTopic );
-	#print STDERR "Check ".`cat $Foswiki::cfg{DataDir}/$testWeb/$testTopic.txt,v`."\n";
+
+#print STDERR "Check ".`cat $Foswiki::cfg{DataDir}/$testWeb/$testTopic.txt,v`."\n";
         $this->item945_checkHistoryRcs( $rcs, $depth );
         $rcs->finish();
     }
@@ -1024,7 +1025,7 @@ sub test_Item945_diff {
     my $testTopic = "TestItem945";
     for my $rcsType (@rcsTypes) {
         my $rcs =
-          "Foswiki::Store::VC::Rcs${rcsType}Handler"->new( new StoreStub,
+          "Foswiki::Store::Rcs::Rcs${rcsType}Handler"->new( new StoreStub,
             $testWeb, $testTopic . "Rcs$rcsType" );
         $this->item945_fillTopic( $rcs, $time, $testWeb,
             $testTopic . "Rcs$rcsType" );
@@ -1044,34 +1045,35 @@ sub test_Item945_diff {
 }
 
 sub deverify_Item11476_worst_case_performance {
+
     # Create lots of revs. We're not so worried about the time taken
     # to create a new rev as we are about the time taken to read basic
     # info, such as number of revisions.
     my $rcs;
     my $topic = "RcsTimeTest";
     local $| = 1;
     $rcs = $class->new( new StoreStub, $testWeb, $topic, "" );
-    for (my $i = 1; $i < 1000; $i++) {
-	my $string1 = <<HERE;
+    for ( my $i = 1 ; $i < 1000 ; $i++ ) {
+        my $string1 = <<HERE;
 %META:TOPICINFO{author="Author$i" date="$time" format="1.1" version="$i"}%
 $i men went to mow, went to mow a meadow,
 $i men, $i - 1 men, $i - 2 men... 1 man and his dog,
 went to mow a meadow
 HERE
-	$rcs->addRevisionFromText( $string1, "$i", "Author$i" );
-	print ".";
+        $rcs->addRevisionFromText( $string1, "$i", "Author$i" );
+        print ".";
     }
     print STDERR "Generated\n";
-    $Foswiki::Store::VC::RcsLiteHandler::trace = 1;
+    $Foswiki::Store::Rcs::RcsLiteHandler::trace = 1;
     my $t0 = Benchmark->new();
-    for (my $i = 1; $i < 100; $i++) {
-	$rcs = $class->new( new StoreStub, $testWeb, $topic, "" );
-	$rcs->getInfo();
-	print ".";
+    for ( my $i = 1 ; $i < 100 ; $i++ ) {
+        $rcs = $class->new( new StoreStub, $testWeb, $topic, "" );
+        $rcs->getInfo();
+        print ".";
     }
     my $t1 = Benchmark->new();
     print STDERR "Timed\n";
-    my $td = timediff($t1, $t0);
-    print "the code took:",timestr($td),"\n";   
+    my $td = timediff( $t1, $t0 );
+    print "the code took:", timestr($td), "\n";
 }
 1;

UnitTestContrib/test/unit/StoreImplementationTests.pm

@@ -359,6 +359,7 @@ sub verify_moveTopic {
 sub verify_saveTopic {
     my $this = shift;
 
+    $this->_makeWeb();
     my $meta =
       Foswiki::Meta->new( $this->{session}, $this->{t_web}, $this->{t_topic} );
     $meta->text("1 2 3");
@@ -807,6 +808,7 @@ sub verify_moveWeb {
 sub verify_setLease_getLease {
     my $this = shift;
 
+    $this->_makeWeb();
     my $meta =
       Foswiki::Meta->new( $this->{session}, $this->{t_web}, $this->{t_topic} );
     $this->assert_null( $this->{sut}->getLease($meta) );
@@ -881,6 +883,7 @@ sub verify_delRev {
 
 sub verify_atomicLocks {
     my $this = shift;
+    $this->_makeWeb();
     my $meta =
       Foswiki::Meta->new( $this->{session}, $this->{t_web}, "AtomicLock" );
     $meta->text("Kaboom");

core/lib/Foswiki/Attach.pm

@@ -165,8 +165,7 @@ sub formatVersions {
 
     while ( $revIt->hasNext() ) {
         my $rev = $revIt->next();
-        my $info =
-          $topicObject->getAttachmentRevisionInfo( $attrs{name}, $rev );
+        my $info = $topicObject->getRevisionInfo( $attrs{name}, $rev );
         $info->{name} = $attrs{name};
         $info->{attr} = $attrs{attr};
         $info->{size} = $attrs{size};

core/lib/Foswiki/Func.pm

@@ -1683,7 +1683,7 @@ sub getRevisionInfo {
     if ($attachment) {
         $topicObject =
           Foswiki::Meta->load( $Foswiki::Plugins::SESSION, $web, $topic );
-        $info = $topicObject->getAttachmentRevisionInfo( $attachment, $rev );
+        $info = $topicObject->getRevisionInfo( $attachment, $rev );
     }
     else {
         $topicObject =

core/lib/Foswiki/Meta.pm

@@ -454,7 +454,6 @@ sub load {
 #SVEN: sadly, Item10805 shows that the metacache is not yet multi-user safe, and as the Groups code in TopicUserMapping changes to user=admin, we can't use it here
 #which makes it clear I need to write a full cache validation set of tests for MetaCache
 #TODO: need to extract the metacache from search, and extract the additional derived info from it too
-#TODO: this mess is because the Listeners cannot assign a cached meta object to an already existing unloaded meta
 #NEW: the metacache has to return a _copy_ of the cached item, otherwise code that ->finish() es its copy will also ->finish() the cached version and any other refs.
 #       which in Sven's opinion means we need to invert things better. (I get ~10% (.2S on 2S req's) speedup on simpler SEARCH topics doing reuse)
 #        my $m =
@@ -1496,9 +1495,13 @@ sub setRevisionInfo {
 
 =begin TML
 
----++ ObjectMethod getRevisionInfo() -> \%info
+---++ ObjectMethod getRevisionInfo([$attachment [,$rev]]) -> \%info
 
-Return revision info for the loaded revision in %info with at least:
+   * =$attachment= - (optional) attachment name to get info about
+   * =$rev= - (optional) revision of attachment for which to get info
+
+Return revision info for the loaded revision of a topic or
+attachment with at least:
    * ={date}= in epochSec
    * ={author}= canonical user ID
    * ={version}= the revision number
@@ -1511,9 +1514,15 @@ The comment is *always* blank
 =cut
 
 sub getRevisionInfo {
-    my $this = shift;
+    my ( $this, $attachment, $rev ) = @_;
+
     _assertIsTopic($this) if DEBUG;
 
+    if ($attachment) {
+        return $this->{_session}->{store}
+          ->getVersionInfo( $this, $rev, $attachment );
+    }
+
     my $info;
     if (    not defined( $this->{_loadedRev} )
         and not Foswiki::Func::topicExists( $this->{_web}, $this->{_topic} ) )
@@ -2675,21 +2684,23 @@ sub onTick {
 
 =begin TML
 
----++ ObjectMethod getAttachmentRevisionInfo($attachment, $rev) -> \%info
+---++ *Deprecated* ObjectMethod getAttachmentRevisionInfo($attachment, $rev) -> \%info
    * =$attachment= - attachment name
    * =$rev= - optional integer attachment revision number
 Get revision info for an attachment. Only valid on topics.
 
 $info will contain at least: date, author, version, comment
 
+*Deprecated* 2014-11-03 use getRevisionInfo instead.
+
 =cut
 
 sub getAttachmentRevisionInfo {
     my ( $this, $attachment, $fromrev ) = @_;
     _assertIsTopic($this) if DEBUG;
 
     return $this->{_session}->{store}
-      ->getAttachmentVersionInfo( $this, $fromrev, $attachment );
+      ->getVersionInfo( $this, $fromrev, $attachment );
 }
 
 =begin TML
@@ -3570,7 +3581,7 @@ sub summariseChanges {
 
 =begin TML
 
----++ ObjectMethod getEmbeddedStoreForm() -> $text
+---++ *Deprecated* ObjectMethod getEmbeddedStoreForm() -> $text
 
 Generate the embedded store form of the topic. The embedded store
 form has meta-data values embedded using %META: lines. The text
@@ -3592,7 +3603,7 @@ sub getEmbeddedStoreForm {
 
 =begin TML
 
----++ ObjectMethod setEmbeddedStoreForm( $text )
+---++ *Deprecated* ObjectMethod setEmbeddedStoreForm( $text )
 
 Populate this object with embedded meta-data from $text. This method
 is a utility provided for use with stores that store data embedded in

core/lib/Foswiki/Store.pm

@@ -480,7 +480,7 @@ sub getRevisionDiff {
 
 ---++ ObjectMethod getVersionInfo($topicObject, $rev, $attachment) -> \%info
 
-Get revision info of a topic or attachment.
+Get revision info for a topic or attachment.
    * =$topicObject= Topic object, required
    * =$rev= revision number. If 0, undef, or out-of-range, will get info
      about the most recent revision.
@@ -489,7 +489,10 @@ Return %info with at least:
 | date | in epochSec |
 | user | user *object* |
 | version | the revision number |
-| comment | comment in the VC system, may or may not be the same as the comment in embedded meta-data |
+| comment | comment in the store system, may or may not be the same as the comment in embedded meta-data |
+
+If =$attachment= and =$rev= are both given, then =$rev= applies to the
+attachment, not the topic.
 
 =cut