Item13897: Bug fixes.

- Also took care of a discrepancy between Error and Try::Tiny where the latter doesn't rethrow unhandled exception. Added a class method rethrow() to Foswiki::Exception which tries to guess what kind of exception has been received and converts it into a Foswiki::Exception ancestor.
foswiki · Jan 25, 2016 · 2897df2 · 2897df2
1 parent d4974a3
commit 2897df2
Show file tree

Hide file tree

Showing 9 changed files with 153 additions and 62 deletions.
diff --git a/UnitTestContrib/lib/Unit/TestCase.pm b/UnitTestContrib/lib/Unit/TestCase.pm
@@ -682,6 +682,9 @@ sub captureSTD {
 
         $result = &$proc(@params);
     }
+    catch {
+        Foswiki::Exception->rethrow($_);
+    }
     finally {
         my $f;
         open( $f, '<', $stdoutfile )

diff --git a/UnitTestContrib/lib/Unit/TestRunner.pm b/UnitTestContrib/lib/Unit/TestRunner.pm
@@ -175,7 +175,7 @@ sub start {
         print "Running $suite\n";
 
 # vrurg: To much burden to maintain compatibility with pre-Moo positional argument passing.
-        my $tester = $suite->new( suite => $suite );
+        my $tester = $suite->new( testSuite => $suite );
         if ( $tester->isa('Unit::TestSuite') ) {
 
             # Get a list of included tests

diff --git a/UnitTestContrib/lib/Unit/TestSuite.pm b/UnitTestContrib/lib/Unit/TestSuite.pm
@@ -16,7 +16,7 @@ Foswiki.
 
 use Moo;
 use namespace::clean;
-extends 'Unit::TestCase';
+extends qw( Unit::TestCase );
 
 # SMELL This property is a placeholder for test suite name parameter passed on
 # on object creation. Need to be reconsidered.

diff --git a/core/lib/Foswiki/Access/TopicACLAccess.pm b/core/lib/Foswiki/Access/TopicACLAccess.pm
@@ -96,7 +96,7 @@ sub haveAccess {
     $mode = uc($mode);
 
     my ( $allow, $deny );
-    if ( $meta->{_topic} ) {
+    if ( $meta->has_topic ) {
 
         $allow = $this->_getACL( $meta, 'ALLOWTOPIC' . $mode );
         $deny  = $this->_getACL( $meta, 'DENYTOPIC' . $mode );
@@ -136,7 +136,7 @@ sub haveAccess {
         $meta = $meta->getContainer();    # Web
     }
 
-    if ( $meta->{_web} ) {
+    if ( $meta->has_web ) {
 
         $deny = $this->_getACL( $meta, 'DENYWEB' . $mode );
         if ( defined($deny)
@@ -203,7 +203,7 @@ sub haveAccess {
 sub _getACL {
     my ( $this, $meta, $mode ) = @_;
 
-    if ( defined $meta->topic && !defined $meta->getLoadedRev ) {
+    if ( $meta->has_topic && !defined $meta->getLoadedRev ) {
 
         # Lazy load the latest version.
         $meta->loadVersion();

diff --git a/core/lib/Foswiki/Exception.pm b/core/lib/Foswiki/Exception.pm
@@ -60,20 +60,31 @@ BEGIN {
     }
 }
 
-has line       => ( is => 'rwp', );
-has file       => ( is => 'rwp', );
-has text       => ( is => 'ro', );
-has object     => ( is => 'ro', );
-has stacktrace => ( is => 'rwp', );
+has line => (
+    is        => 'rwp',
+    predicate => 1,
+);
+has file => (
+    is        => 'rwp',
+    predicate => 1,
+);
+has text   => ( is => 'ro', );
+has object => ( is => 'ro', );
+has stacktrace => (
+    is        => 'rwp',
+    predicate => 1,
+);
 
 sub BUILD {
     my $this = shift;
 
-    my $trace = Carp::longmess('');
-    $this->_set_stacktrace($trace);
+    unless ( $this->has_stacktrace ) {
+        my $trace = Carp::longmess('');
+        $this->_set_stacktrace($trace);
+    }
     my ( undef, $file, $line ) = caller;
-    $this->_set_file($file);
-    $this->_set_line($line);
+    $this->_set_file($file) unless $this->has_file;
+    $this->_set_line($line) unless $this->has_line;
 }
 
 sub stringify {
@@ -87,6 +98,53 @@ sub stringify {
       );
 }
 
+=begin TML
+
+---++ ClassMethod rethrow($error)
+
+Receives any exception and rethrows it as Foswiki::Exception.
+
+=cut
+
+sub rethrow {
+    my $class = shift;
+    my ($e) = @_;
+
+    if ( ref($e) ) {
+        if ( $e->isa('Foswiki::Exception') ) {
+            $e->throw;
+        }
+        elsif ( $e->isa('Error') ) {
+            $class->throw(
+                text       => $e->text,
+                line       => $e->line,
+                file       => $e->file,
+                stacktrace => $e->stacktrace,
+                object     => $e->object,
+            );
+        }
+
+        # Wild cases where we've got non-exception objects. Generally it's a
+        # serious bug but we better try to provide as much information on what's
+        # happened as possible.
+        elsif ( $e->can('stringify') ) {
+            $class->throw( text => $e->stringify );
+        }
+        elsif ( $e->can('as_text') ) {
+            $class->throw( text => $e->as_text );
+        }
+        else {
+            # Finally we're no idea what kind of a object has been thrown to us.
+            $class->throw(
+                text => "Unknown kind of exception received: " . ref($e) );
+        }
+    }
+    else {
+        $class->throw( text => $e );
+    }
+
+}
+
 package Foswiki::Exception::ASSERT;
 use Moo;
 extends qw(Foswiki::Exception);

diff --git a/core/lib/Foswiki/Form.pm b/core/lib/Foswiki/Form.pm
@@ -29,14 +29,10 @@ of possible values.
 # intelligence is in the individual field types.
 
 package Foswiki::Form;
-use strict;
-use warnings;
-
-use Foswiki::Meta ();
-our @ISA = ('Foswiki::Meta');
+use v5.14;
 
 use Assert;
-use Error qw( :try );
+use Try::Tiny;
 
 use Foswiki::Sandbox                   ();
 use Foswiki::Form::FieldDefinition     ();
@@ -45,6 +41,10 @@ use Foswiki::AccessControlException    ();
 use Foswiki::OopsException             ();
 use Foswiki::Func                      ();
 
+use Moo;
+use namespace::clean;
+extends qw( Foswiki::Meta );
+
 BEGIN {
     if ( $Foswiki::cfg{UseLocale} ) {
         require locale;
@@ -62,6 +62,8 @@ my %reservedFieldNames = map { $_ => 1 }
 my @default_columns = qw/name type size value description attributes default/;
 my %valid_columns = map { $_ => 1 } @default_columns;
 
+our @_newParameters = qw(session web form def);
+
 =begin TML
 
 ---++ ClassMethod new ( $session, $web, $topic, \@def )