Item12952: better implementation of the horrible configureTest hack. …

…Cleanup of wizard return values.
foswiki · Sep 24, 2014 · bebdc1b · bebdc1b
1 parent b2b393d
commit bebdc1b
Show file tree

Hide file tree

Showing 11 changed files with 198 additions and 232 deletions.
diff --git a/ConfigurePlugin/lib/Foswiki/Plugins/ConfigurePlugin.pm b/ConfigurePlugin/lib/Foswiki/Plugins/ConfigurePlugin.pm
@@ -960,7 +960,9 @@ and time-consuming integrity checks.
    * =keys= - name of a checker to use if =wizard= is not given
    * =method= - name of the method in the wizard or checker to call
 
-The return result is a hash containing the following keys:
+If the wizard method returns an object, that will be passed back
+as the result of the call. If the wizard method returns undef, the
+return result is a hash containing the following keys:
     * =report= - Error/Warning etc messages, formatted as HTML. Each
       entry in this array is a hash with keys 'level' (e.g. error, warning)
       and 'message'.
@@ -974,38 +976,41 @@ sub wizard {
     my $target;
     my $root = _loadSpec();
     if ( defined $params->{wizard} ) {
-        die unless $params->{wizard} =~ /^(\w+)$/;    # untaint
+        die "Bad wizard" unless $params->{wizard} =~ /^(\w+)$/;    # untaint
         $target = Foswiki::Configure::Wizard::loadWizard( $1, $params );
     }
     else {
-        die unless $params->{keys};
+        die "No wizard and no keys" unless $params->{keys};
         my $vob = $root->getValueObject( $params->{keys} );
         $target = Foswiki::Configure::Checker::loadChecker($vob);
     }
     die unless $target;
     my $method = $params->{method};
     die unless $method =~ /^(\w+)$/;
-    $method = $1;                                     # untaint
+    $method = $1;                                                  # untaint
     my $reporter = Foswiki::Configure::Reporter->new();
 
     _getSetParams( $params, $root );
 
-    $target->$method($reporter);
+    my $response = $target->$method($reporter);
 
-    my @report;
-    foreach
-      my $level ( 'errors', 'warnings', 'confirmations', 'notes', 'changes' )
-    {
-        push(
-            @report,
-            {
-                level   => $level,
-                message => $reporter->text($level)
-            }
-        ) if $reporter->has($level);
+    unless ($response) {
+        my @report;
+        foreach my $level ( 'errors', 'warnings', 'confirmations', 'notes',
+            'changes' )
+        {
+            push(
+                @report,
+                {
+                    level   => $level,
+                    message => $reporter->text($level)
+                }
+            ) if $reporter->has($level);
+        }
+        $response = { changes => $reporter->{changes}, report => \@report };
     }
 
-    return { changes => $reporter->{changes}, report => \@report };
+    return $response;
 }
 
 =pod

diff --git a/ConfigurePlugin/pub/System/ConfigurePlugin/render_tml.uncompressed.js b/ConfigurePlugin/pub/System/ConfigurePlugin/render_tml.uncompressed.js
@@ -156,9 +156,9 @@ var TML = {
             }
 
             if (m = /^---(\++) (.*)$/.exec(line)) {
-                lines[i] = '<h' + length(m[1]) + '>'
+                lines[i] = '<h' + m[1].length + '>'
                     + m[2]
-                    + '</h' + length(m[1]) + '>';
+                    + '</h' + m[1].length + '>';
             }
         }
 

diff --git a/core/lib/Foswiki/Configure/Wizards/AutoConfigureEmail.pm b/core/lib/Foswiki/Configure/Wizards/AutoConfigureEmail.pm
@@ -147,6 +147,8 @@ NOCERT
         $Foswiki::cfg{EnableEmail} = 1;
         $reporter->CHANGED('{EnableEmail}');
     }
+
+    return undef;    # return the report
 }
 
 # Return 0 on failure

diff --git a/core/lib/Foswiki/Configure/Wizards/ExploreExtensions.pm b/core/lib/Foswiki/Configure/Wizards/ExploreExtensions.pm
@@ -207,13 +207,15 @@ sub get_installed_extensions {
     my ( $this, $reporter ) = @_;
 
     $this->_get_extensions( $reporter, 'installed' );
+    return undef;    # return the report
 }
 
 # Wizard - Constructs an HTML table of not-installed extensions
 sub get_other_extensions {
     my ( $this, $reporter ) = @_;
 
     $this->_get_extensions( $reporter, 'uninstalled' );
+    return undef;    # return the report
 }
 
 sub _get_extensions {

diff --git a/core/lib/Foswiki/Configure/Wizards/InstallExtensions.pm b/core/lib/Foswiki/Configure/Wizards/InstallExtensions.pm
@@ -35,11 +35,13 @@ our $installRoot;
 sub add {
     my ( $this, $reporter ) = @_;
     $this->_action( 'add', $reporter );
+    return undef;    # return the report
 }
 
 sub remove {
     my ( $this, $reporter ) = @_;
     $this->_action( 'remove', $reporter );
+    return undef;    # return the report
 }
 
 sub _action {

diff --git a/core/lib/Foswiki/Configure/Wizards/Save.pm b/core/lib/Foswiki/Configure/Wizards/Save.pm
@@ -237,6 +237,7 @@ sub save {
         unlink $backup if ($backup);
         $reporter->NOTE("No change made to $lsc");
     }
+    return undef;    # return the report
 }
 
 sub _compareConfigs {

diff --git a/core/lib/Foswiki/Configure/Wizards/ScriptHash.pm b/core/lib/Foswiki/Configure/Wizards/ScriptHash.pm
@@ -40,59 +40,43 @@ sub verify {
     my $cookie = CGI->cookie(@pars);
     my $net    = Foswiki::Net->new;
 
-    # Flags must be defined and false.  Avoid 'used once' warnings.
-
-    local $Foswiki::Net::LWPAvailable = 0 && $Foswiki::Net::LWPAvailable;
-    local $Foswiki::Net::noHTTPResponse = 1 || $Foswiki::Net::noHTTPResponse;
-
     unless ( defined $Foswiki::VERSION ) {
         ( my $fwi, $Foswiki::VERSION ) = Foswiki::Configure::Dependency::extractModuleVersion( 'Foswiki', 1 );
         $Foswiki::Version = '0.0' unless ($fwi);
     }
 
-    my $test   = '/Web/Topic/Env/Echo?configurationTest=yes';
-    my $target = eval "\$Foswiki::cfg$keys";
+    my $script;
 
-    my $script = 'view';
-    my ( $root, $view, $viewtarget );
-
-    if ( $keys =~ /^\{[^}]+\}\{([^}]+)\}$/ ) {
+    if ( $keys =~ /^\{ScriptUrlPaths\}\{([^}]+)\}$/ ) {
         $script = $1;
     }
     else {
-        $target ||= '';
-        $target .= "/$script" . ( $Foswiki::cfg{ScriptSuffix} || '' );
-        $root       = 1;
-        $view       = $Foswiki::cfg{ScriptUrlPaths}{view};
-        $viewtarget = $view;
-        $viewtarget = $Foswiki::cfg{ScriptUrlPath}
-          if ( !defined $viewtarget );
-        Foswiki::Configure::Load::expandValue($viewtarget);
-        $view = '$Foswiki::cfg{ScriptUrlPath}/view$Foswiki::cfg{ScriptSuffix}'
-          if ( !defined $view );
-        Foswiki::Configure::Load::expandValue($view);
+        $script = 'view';
     }
-    $target =
-      '$Foswiki::cfg{ScriptUrlPath}/' . $script . '$Foswiki::cfg{ScriptSuffix}'
-      unless ( defined $target );
-    Foswiki::Configure::Load::expandValue($target);
-    my $data;
 
-    my $url = $Foswiki::cfg{DefaultUrlHost} . $target . $test;
+    my $url =
+        $Foswiki::cfg{DefaultUrlHost}
+      . $Foswiki::cfg{ScriptUrlPath}
+      . "/$script"
+      . ( $Foswiki::cfg{ScriptSuffix} || '' );
+
     $reporter->NOTE("Tracing access to =$url=");
 
-    my ( $limit, $try ) = (10);
-    my @headers = ( Cookie => join( '=', $cookie->name, $cookie->value ), );
+    my $try     = 10;
+    my %headers = (
+        Cookie             => join( '=', $cookie->name, $cookie->value ),
+        'X-Foswiki-Tickle' => 1,
+        'X-Requested-With' => 'FoswikiReflectionRequest'
+    );
 
     if ($user) {
         require MIME::Base64;
         my $auth = MIME::Base64::encode_base64( "$user:$password", '' );
-        push @headers, Authorization => "Basic $auth";
+        $headers{Authorization} = "Basic $auth";
     }
-    push( @headers, 'X-Requested-With' => 'FoswikiReflectionRequest' );
 
-    for ( $try = 1 ; $try <= $limit ; $try++ ) {
-        my $response = $net->getExternalResource( $url, @headers );
+    while ( $try-- ) {
+        my $response = $net->getExternalResource( $url, headers => \%headers );
 
         if ( $response->is_error ) {
             my $content = $response->content || '';
@@ -116,76 +100,53 @@ sub verify {
                 "Redirected (" . $response->code . ") " . "to =$url=" );
             next;
         }
-        $data = $response->content;
-        unless ( $url =~ m,^(https?://([^:/]+)(:\d+)?)(/.*)?\Q$test\E$, ) {
+        unless ( $url =~ m,^(https?://([^:/]+)(:\d+)?)(/.*)?, ) {
             $reporter->ERROR("=$url= does not match request");
             last;
         }
-        my ( $host, $hname, $port, $path ) = ( $1, $2, $3, $4 );
-        if ( $host ne $Foswiki::cfg{DefaultUrlHost} ) {
-            $reporter->WARN(
-"=$host= does not match {DefaultUrlHost} (=$Foswiki::cfg{DefaultUrlHost}=)"
-            );
-        }
-        $path ||= '';
+
+        my $data = $response->content;
 
         my $info = JSON->new->decode($data);
 
-        unless ( ref($info) && defined $info->{ENV}->{SCRIPT_NAME} ) {
+        unless ( ref($info) && defined $info->{SCRIPT_NAME} ) {
             $reporter->ERROR(
                 "Server returned incorrect diagnostic data: =$data= ");
+            last;
         }
-        else {
-            if ( $info->{ENV}->{SCRIPT_NAME} eq $target ) {
-                $reporter->NOTE(
-                    "Server received the expected path (=$target=)");
-            }
-            elsif ($root) {
-                if ( $info->{ENV}->{SCRIPT_NAME} eq $view ) {
-                    $reporter->NOTE(
-"Server received =$view=, which is the value of {ScriptUrlPaths}{view}.  This indicates that short(er) URLs are active and functioning correctly."
-                    );
-                }
-                else {
-                    $reporter->ERROR(
-"Server received =$info->{ENV}->{SCRIPT_NAME}=, but the expected path is =$viewtarget=.
-Changing {ScriptUrlPaths}{view} to =$info->{ENV}->{SCRIPT_NAME}= will probably correct this error. (Server may be configured for Shorter URLs.)"
-                    );
-                }
-            }
-            else {
-                $reporter->ERROR(
-"Server received =$info->{ENV}->{SCRIPT_NAME}=, but the expected path is =$target=.
-The correct setting for $keys is probably =$info->{ENV}->{SCRIPT_NAME}=.  (Server may be configured for Shorter URLs.)"
-                );
-            }
-        }
-        if ( $path eq $target ) {
-            $reporter->NOTE("Path =$path= is correct");
+
+        my $target = $Foswiki::cfg{ScriptUrlPaths}{$script}
+          || "$Foswiki::cfg{ScriptUrlPath}/$script"
+          . ( $Foswiki::cfg{ScriptSuffix} || '' );
+
+        if ( $info->{SCRIPT_NAME} eq $target ) {
+            $reporter->NOTE("Server received the expected path (=$target=)");
         }
-        elsif ($root) {
-            if ( $path eq $view ) {
+        elsif ( $script eq 'view' ) {
+            if ( $info->{SCRIPT_NAME} eq $Foswiki::cfg{ScriptUrlPath} ) {
                 $reporter->NOTE(
-                    "Path =$path= is correct for =view= with short(er) URLs");
+"Server received =$info->{SCRIPT_NAME}=, which is the value of {ScriptUrlPaths}{$script}.  This indicates that short(er) URLs are active and functioning correctly."
+                );
             }
             else {
-                $reporter->ERROR( "Path used by "
-                      . ( $try > 1 ? "final " : '' )
-                      . "GET (=$path=) does not match {ScriptUrlPath} (=$viewtarget=)"
+                $reporter->ERROR(
+"Server received =$info->{SCRIPT_NAME}=, but the expected path is =$Foswiki::cfg{ScriptUrlPath}=.
+Changing {ScriptUrlPaths}{view} to =$info->{SCRIPT_NAME}= will probably correct this error. (Server may be configured for Shorter URLs.)"
                 );
             }
         }
         else {
-            $reporter->ERROR( "Path used by "
-                  . ( $try > 1 ? "final " : '' )
-                  . "GET (=$path=) does not match $keys (=$target=)" );
+            $reporter->ERROR(
+"Server received =$info->{SCRIPT_NAME}=, but the expected path is =$target=.
+The correct setting for $keys is probably =$info->{SCRIPT_NAME}=.  (Server may be configured for Shorter URLs.)"
+            );
         }
-
         last;
     }
-    if ( $try > $limit ) {
-        $reporter->ERROR("Excessive redirects (&gt;$limit) stopped trace.");
+    unless ($try) {
+        $reporter->ERROR("Excessive redirects stopped trace.");
     }
+    return undef;    # return the report
 }
 
 1;

diff --git a/core/lib/Foswiki/Configure/Wizards/SendTestEmail.pm b/core/lib/Foswiki/Configure/Wizards/SendTestEmail.pm
@@ -43,6 +43,7 @@ sub send {
 
     eval { _sendTestEmail( $Foswiki::cfg{WebMasterEmail}, $reporter ); };
     die $@ if $@;
+    return undef;    # return the report
 }
 
 # Send a test email to the address in the value
@@ -410,7 +411,7 @@ MAILTEST
         return if $neterrors;
     }
 
-    return $reporter->NOTE(<<ACCEPTED);
+    $reporter->NOTE(<<ACCEPTED);
 Mail was accepted for delivery to $addrs from $Foswiki::cfg{WebMasterEmail}. Be sure to check any SPAM and Bulk-email folders before assuming that delivery has failed.
 ACCEPTED
 }