Changeset 13781

Timestamp:

01/22/12 19:31:26 (4 weeks ago)

Author:

CrawfordCurrie

Message:

Item11461: no need to fake the loaded rev, as we know the new content being saved is correct for the current rev (even if it was generated internally)

Location:

trunk

Files:

• UnitTestContrib/test/unit/PluginHandlerTests.pm (modified) (6 diffs)
• core/lib/Foswiki/Meta.pm (modified) (2 diffs)

trunk/UnitTestContrib/test/unit/PluginHandlerTests.pm

@@ -5 +5 @@
 # Make sure that all the right plugin handlers are called in the
 # right places with the right parameters.
-#
-# Here are the handlers we need to test, and the current status:
-#
-# | *Handler*                    | *Tested by* |
-# | afterAttachmentSaveHandler   | *untested* |
-# | afterUploadHandler           | *untested* |
-# | afterCommonTagsHandler       | test_commonTagsHandlers |
-# | afterEditHandler             | *untested* |
-# | afterRenameHandler           | *untested* |
-# | afterSaveHandler             | test_saveHandlers |
-# | beforeUploadHandler          | *untested* |
-# | beforeAttachmentSaveHandler  | *untested* |
-# | beforeCommonTagsHandler      | test_commonTagsHandlers |
-# | beforeEditHandler            | *untested* |
-# | beforeSaveHandler            | test_saveHandlers |
-# | commonTagsHandler            | test_commonTagsHandlers |
-# | earlyInitPlugin              | test_earlyInit |
-# | endRenderingHandler          | test_renderingHandlers |
-# | initPlugin                   | all tests |
-# | initializeUserHandler        | test_earlyInit |
-# | insidePREHandler             | test_renderingHandlers |
-# | modifyHeaderHandler          | *untested* |
-# | mergeHandler                 | *untested* |
-# | outsidePREHandler            | test_renderingHandlers |
-# | postRenderingHandler         | test_renderingHandlers |
-# | preRenderingHandler          | test_renderingHandlers |
-# | redirectrequestHandler       | *untested* |
-# | registrationHandler          | *untested* |
-# | renderFormFieldForEditHandler| *untested* |
-# | renderWikiWordHandler        | *untested* |
-# | startRenderingHandler        | test_renderingHandlers |
-# | writeHeaderHandler           | *untested* |
 #
 # We do this by actually writing a valid plugin implementation to the
@@ -194 +162 @@
 sub beforeSaveHandler {
     #my( $text, $topic, $theWeb, $meta ) = @_;
-    $tester->assert_str_equals('Tropic', $_[1], "TWO $_[1]");
+    # ensure we have a loaded rev
+    $tester->assert($_[3]->getLoadedRev());
+     $tester->assert_str_equals('Tropic', $_[1], "TWO $_[1]");
     $tester->assert_str_equals($tester->{test_web}, $_[2], "THREE $_[2]");
     $tester->assert($_[3]->isa('Foswiki::Meta'), "FOUR $_[3]");
@@ -212 +182 @@
     $tester->assert_null($_[3]);
     $tester->assert($_[4]->isa('Foswiki::Meta'), "OUCH $_[4]");
+    # ensure we have a loaded rev
+    $tester->assert($_[4]->getLoadedRev());
     $tester->assert_str_equals('Wibble', $_[4]->get('WIBBLE')->{wibble});
     $tester->assert_matches( qr/B4SAVE/, $_[0]);
@@ -227 +199 @@
 HERE
 
-# Test to ensure that the before and after save handlers are both called,
-# and that modifications made to the text are actaully written to the topic file
-    my $meta =
-      Foswiki::Meta->load( $this->{session}, $this->{test_web}, "Tropic" );
+    # Test to ensure that the before and after save handlers are both called,
+    # and that modifications made to the text are actaully written to the topic file
+    my $meta = Foswiki::Meta->new( $this->{session}, $this->{test_web}, "Tropic", $text );
     $meta->put( 'WIBBLE', { wibble => 'Wibble' } );
     $meta->save();
@@ -553 +524 @@
 sub afterUploadHandler {
     my ($attachmentAttrHash, $meta) = @_;
+    # ensure we have a loaded rev
+    $tester->assert($meta->getLoadedRev());
     $called->{afterUploadHandler}++;
 }
@@ -594 +567 @@
 sub beforeUploadHandler {
     my( $attrHashRef, $meta ) = @_;
+    # ensure we have a loaded rev
+    $tester->assert($meta->getLoadedRev());
     $called->{beforeUploadHandler}++;
 }

trunk/core/lib/Foswiki/Meta.pm

@@ -1877 +1877 @@
         my $pretext = $text;               # text before the handler modifies it
         my $premeta = $this->stringify();  # just the meta, no text
-        # The meta obj may not have a loaded rev yet. If anything in the
-        # beforeSaveHandlers tries to do an access check with an unloadedRev,
-        # it will attempt to load the latest which we really don't want it to do.
-        # So we mark it as NEW.
-        $this->{_loadedRev} = 'NEW' unless defined $this->{_loadedRev};
+        unless ( $this->{_loadedRev} ) {
+            # The meta obj doesn't have a loaded rev yet, and we have to block the
+            # beforeSaveHandlers from loading the topic from store. We are saving,
+            # and anything we have in $this is going to get written anyway, so we
+            # can simply mark it as "the latest".
+            # SMELL: this may not work if the beforeSaveHandler tries to use the
+            # meta obj for access control checks, so that is not recommended.
+            $this->{_loadedRev} = $this->getLatestRev();
+        }
+
         $plugins->dispatch( 'beforeSaveHandler', $text, $this->{_topic},
             $this->{_web}, $this );
-        undef $this->{_loadedRev} if $this->{_loadedRev} eq 'NEW';
 
         # If the text has changed; it may be a text or meta change, or both
@@ -1912 +1916 @@
         $signal = shift;
     };
+
+    ASSERT($newRev, $this->{loadedRev}) if DEBUG;
 
     # Semantics inherited from TWiki. See