Item14152: Implemented callbacks for extensions.

This is pretty much all the features I wanted to implement for the new extensions model. These can be further extended or dropped but the current state more or less demonstrates my view. - Callbacks now support Foswiki::Exception::Ext::Flow. - Implemented restart flow feature for callbacks. Didn't have time to implement a test for this. - Fixed a problem with Foswiki::Class callbacks feature and missing BUILD method on the target class. - Removed Foswiki::Exception::CB
foswiki · Sep 17, 2016 · 698f70a · 698f70a
1 parent 9f66599
commit 698f70a
Show file tree

Hide file tree

Showing 9 changed files with 267 additions and 82 deletions.
diff --git a/UnitTestContrib/test/unit/ExtensionsTests.pm b/UnitTestContrib/test/unit/ExtensionsTests.pm
@@ -1,6 +1,7 @@
 # See bottom of file for license and copyright information
 
 package Foswiki::ExtensionsTests::SampleClass;
+use utf8;
 
 use Foswiki::Class qw(extensible);
 extends qw(Foswiki::Object);
@@ -87,6 +88,7 @@ sub _genExtModules {
         my $code    = shift @extCode // '';
         my $ret     = eval <<EXT;
 package $extName;
+use Assert;
 use Foswiki::Class qw(extension);
 extends qw(Foswiki::Extension);
 
@@ -428,6 +430,8 @@ CFG_EXT
 sub test_tag_handlers {
     my $this = shift;
 
+    $this->_disableAllCurrentExtensions;
+
     # Testing three macros registered by the same extension using three
     # different approaches:
     #   1. A macro to be handled by an extension method with the same name as
@@ -510,6 +514,8 @@ MPKG
 sub test_extName_method {
     my $this = shift;
 
+    $this->_disableAllCurrentExtensions;
+
     my ($ext) = $this->_genExtModules( 1, <<'SNEXT');
 our $NAME = "AutoGenExt";
 SNEXT
@@ -520,6 +526,144 @@ SNEXT
         $this->app->extensions->extName($ext) );
 }
 
+sub test_callbacks {
+    my $this = shift;
+
+    $this->_disableAllCurrentExtensions;
+
+    my $cbTestClassName = 'Foswiki::Extension::Test::CBTest';
+    my $cbTestClass     = <<CBMOD;
+package $cbTestClassName;
+
+use Foswiki::Class qw(callbacks);
+extends qw(Foswiki::Object);
+
+callback_names qw(LongEnoughNotToMakeAConflict);
+
+sub sampleSingleCallBack {
+    my \$this = shift;
+    
+    my \$cbParam = {
+        param1 => 'This is param1',
+        param2 => 3.14,
+        param3 => 'І ще трохи, на пробу',
+    };
+    
+    \$this->callback('LongEnoughNotToMakeAConflict', \$cbParam);
+    
+    return \$cbParam->{rcIExpect} // '*unexpected undef*';
+}
+
+sub sampleCBChain {
+    my \$this = shift;
+    
+    my \$cbParam = {
+        param1 => 'This is param1',
+        param2 => 3.14,
+        param3 => 'І ще трохи, на пробу',
+    };
+    
+    my \$cbReturn = \$this->callback('LongEnoughNotToMakeAConflict', \$cbParam);
+    
+    return {
+        rc => \$cbParam->{rcIExpect},
+        cbReturn => \$cbReturn,
+    };
+}
+
+1;
+CBMOD
+
+    if ( !eval($cbTestClass) || $@ ) {
+        Foswiki::Exception::Fatal->throw(
+            text => "Failed to compile callbacks test class: " . $@ );
+    }
+
+    my @ext = $this->_genExtModules( 1, <<EXT);
+callbackHandler LongEnoughNotToMakeAConflict => sub {
+    my \$this = shift;
+    my (\$obj, \$params) = \@_;
+    
+    ASSERT(defined \$obj, "Callback initiator is not defined");
+    ASSERT(\$obj->isa('$cbTestClassName' ),
+      "Callback initiator of a wrong class" );
+
+    \$params->{rcIExpect} =
+        \$params->{param3}
+      . ' щоб не був повний '
+      . \$params->{param2} . '-ць';
+};
+EXT
+
+    $this->reCreateFoswikiApp;
+
+    my $cbObj = $this->create($cbTestClassName);
+
+    my $rc = $cbObj->sampleSingleCallBack;
+
+    $this->assert_str_equals(
+'І ще трохи, на пробу щоб не був повний 3.14-ць',
+        $rc
+    );
+
+    $this->_disableAllCurrentExtensions;
+
+    @ext = $this->_genExtModules( 3, <<EXT1, <<EXT2, <<EXT3);
+    
+callbackHandler LongEnoughNotToMakeAConflict => sub {
+    my \$this = shift;
+    my (\$obj, \$params) = \@_;
+    
+    \$params->{rcIExpect} = 'This is from ext1';
+};
+EXT1
+
+has beTheHighlander => ( is => 'rw', default => 0, );
+
+callbackHandler LongEnoughNotToMakeAConflict => sub {
+    my \$this = shift;
+    Foswiki::Exception::Ext::Last->throw(
+        extension => \$this,
+        rc => 'This is from ext2',
+    ) if \$this->beTheHighlander;
+};
+EXT2
+
+callbackHandler LongEnoughNotToMakeAConflict => sub {
+    my \$this = shift;
+    my (\$obj, \$params) = \@_;
+
+    \$params->{rcIExpect} = 'А це вже з ext3';
+};
+EXT3
+
+    $this->_setExtDependencies(
+        $ext[1] => $ext[0],
+        $ext[2] => $ext[1],
+    );
+
+    $this->reCreateFoswikiApp;
+
+    $rc = $cbObj->sampleCBChain;
+
+    $this->assert( ref($rc) eq 'HASH',
+        "sampleCBChain did not return hashref?" );
+    $this->assert_str_equals( 'А це вже з ext3', $rc->{rc} );
+    $this->assert_null( $rc->{cbReturn},
+        "The callback call must have returned undef" );
+
+    $this->app->extensions->extObject( $ext[1] )->beTheHighlander(1);
+
+    $rc = $cbObj->sampleCBChain;
+
+    $this->assert( ref($rc) eq 'HASH',
+        "sampleCBChain did not return hashref?" );
+    $this->assert_str_equals( 'This is from ext1', $rc->{rc} );
+    $this->assert_not_null( $rc->{cbReturn},
+        "The callback call must have returned a value from exception" );
+    $this->assert_str_equals( $rc->{cbReturn}, 'This is from ext2' );
+}
+
 1;
 __END__
 Foswiki - The Free and Open Source Wiki, http://foswiki.org/

diff --git a/core/lib/Foswiki/App.pm b/core/lib/Foswiki/App.pm
@@ -327,7 +327,7 @@ sub BUILD {
 
     $Foswiki::app = $this;
 
-    ASSERT( defined $this->extensions, "Extensions failed to initialize" );
+    $this->extensions->initialize;
 
     unless ( $this->cfg->data->{isVALID} ) {
         $this->cfg->bootstrapSystemSettings;
@@ -402,7 +402,6 @@ sub BUILD {
     }
     else {
         my $plogin = $this->plugins->load;
-        $this->extensions->initialize;
         $this->engine->user($plogin) if $plogin;
     }
 

diff --git a/core/lib/Foswiki/Aux/Callbacks.pm b/core/lib/Foswiki/Aux/Callbacks.pm
@@ -108,9 +108,12 @@ sub _getApp {
     my $this = shift;
 
     return (
-          $this->isa('Foswiki::App')
-        ? $this
-        : ( $this->does('Foswiki::AppObject') ? $this->app : $Foswiki::app )
+        $this->isa('Foswiki::App') ? $this
+        : (
+            $this->does('Foswiki::AppObject') ? $this->app
+            : (      $this->does('Foswiki::Aux::_ExtensibleRole')
+                  && $this->_has__appObj ? $this->__appObj : $Foswiki::app )
+        )
     );
 }
 
@@ -197,34 +200,50 @@ sub callback {
 
     return unless $cbList;
 
-    foreach my $cbInfo (@$cbList) {
-        try {
-            $cbInfo->{code}
-              ->( $this, data => $cbInfo->{data}, params => $params, );
-        }
-        catch {
-            my $e = Foswiki::Exception::Fatal->transmute( $_, 0 );
-            if ( $e->isa('Foswiki::Exception::CB') ) {
-                if ( $e->isa('Foswiki::Exception::CB::Last') ) {
-                    $lastException = $e;
+    my $restart;
+    do {
+        $restart = 0;
+        my $lastIteration = 0;
+        my ( $cbIdx, $cbInfo );
+        values @$cbList;
+        while (!$lastIteration
+            && !$lastException
+            && ( ( $cbIdx, $cbInfo ) = each @$cbList ) )
+        {
+            try {
+                $cbInfo->{code}
+                  ->( $this, data => $cbInfo->{data}, params => $params, );
+            }
+            catch {
+                my $e = Foswiki::Exception::Fatal->transmute( $_, 0 );
+                if ( $e->isa('Foswiki::Exception::Ext::Flow') ) {
+                    if ( $e->isa('Foswiki::Exception::Ext::Last') ) {
+                        $lastException = $e;
+                    }
+                    elsif ( $e->isa('Foswiki::Exception::Ext::Restart') ) {
+                        $params->{execRestarted} = {
+                            code => $cbInfo->{code},
+                            data => $cbInfo->{data},
+                        };
+                        $lastIteration = $restart = 1;
+                    }
+                    else {
+                        Foswiki::Exception::Fatal->throw(
+                                text => "Unknown callback exception "
+                              . ref($e)
+                              . "; the exception data is following:\n"
+                              . $e->stringify, );
+                    }
                 }
                 else {
-                    Foswiki::Exception::Fatal->throw(
-                            text => "Unknown callback exception "
-                          . ref($e)
-                          . "; the exception data is following:\n"
-                          . $e->stringify, );
+                    $e->rethrow;
                 }
-            }
-            else {
-                $e->rethrow;
-            }
-        };
-        last if $lastException;
-    }
+            };
+        }
+    } while ($restart);
 
-    if ( $lastException && $lastException->has_returnValue ) {
-        return $lastException->returnValue;
+    if ( $lastException && $lastException->has_rc ) {
+        return $lastException->rc;
     }
 
     return;

diff --git a/core/lib/Foswiki/Class.pm b/core/lib/Foswiki/Class.pm
@@ -77,6 +77,7 @@ manually by the class using =with=.
 # _handler_someword - function which implements exported keyword `someword'
 
 use Carp;
+use Class::Method::Modifiers qw(install_modifier);
 
 require Foswiki;
 require Moo::Role;
@@ -144,6 +145,15 @@ sub import {
     }
 
     on_scope_end {
+        if ( $options{callbacks}{use} ) {
+            my $ns = Foswiki::getNS($target);
+
+            # Install BUILD method if callbacks feature requested.
+            # Otherwise Foswiki::Aux::Callbacks fails to apply cleanly.
+            unless ( defined $ns->{BUILD} && defined *{ $ns->{BUILD} }{CODE} ) {
+                install_modifier( $target, fresh => BUILD => sub { } );
+            }
+        }
         $class->_apply_roles;
     };
 
@@ -259,16 +269,23 @@ sub _handler_tagHandler ($;$) {
     }
 }
 
+sub _handler_callbackHandler ($&) {
+    my $target = caller;
+
+    Foswiki::Extensions::registerExtCallback( $target, @_ );
+}
+
 sub _install_extension {
     my ( $class, $target ) = @_;
 
-    _inject_code( $target, 'plugBefore', \&_handler_plugBefore );
-    _inject_code( $target, 'plugAround', \&_handler_plugAround );
-    _inject_code( $target, 'plugAfter',  \&_handler_plugAfter );
-    _inject_code( $target, 'extClass',   \&_handler_extClass );
-    _inject_code( $target, 'extAfter',   \&_handler_extAfter );
-    _inject_code( $target, 'extBefore',  \&_handler_extBefore );
-    _inject_code( $target, 'tagHandler', \&_handler_tagHandler );
+    _inject_code( $target, 'plugBefore',      \&_handler_plugBefore );
+    _inject_code( $target, 'plugAround',      \&_handler_plugAround );
+    _inject_code( $target, 'plugAfter',       \&_handler_plugAfter );
+    _inject_code( $target, 'extClass',        \&_handler_extClass );
+    _inject_code( $target, 'extAfter',        \&_handler_extAfter );
+    _inject_code( $target, 'extBefore',       \&_handler_extBefore );
+    _inject_code( $target, 'tagHandler',      \&_handler_tagHandler );
+    _inject_code( $target, 'callbackHandler', \&_handler_callbackHandler );
 }
 
 sub _handler_pluggable ($&) {