[tools v2] testbot/web: Explicitly call exit on return from Redirect().

[Francois Gouget]

Having Redirect() call exit() itself is confusing and requires
documenting that it does not return in each call point and/or adding
a defensive exit() call anyway.
Using the exit(Redirect(...)) construct is more explicit, concise and
robust.

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com>
---
v2: Remove a chunk that was patching a debug web page that's specific to 
    my test site.
---
 testbot/lib/ObjectModel/CGI/CollectionBlock.pm | 18 +++++++-----------
 testbot/lib/ObjectModel/CGI/ItemPage.pm        | 10 ++++------
 testbot/lib/ObjectModel/CGI/Page.pm            |  2 +-
 testbot/lib/WineTestBot/CGI/PageBase.pm        |  6 +++---
 testbot/web/JobDetails.pl                      |  8 +++-----
 testbot/web/Login.pl                           |  3 +--
 testbot/web/Submit.pl                          | 13 +++----------
 testbot/web/admin/Log.pl                       |  4 ++--
 testbot/web/admin/SpecialJobs.pl               |  3 +--
 testbot/web/admin/UserDetails.pl               |  9 +++------
 10 files changed, 28 insertions(+), 48 deletions(-)

diff --git a/testbot/lib/ObjectModel/CGI/CollectionBlock.pm b/testbot/lib/ObjectModel/CGI/CollectionBlock.pm
index a5e8187009..9027b87758 100644
--- a/testbot/lib/ObjectModel/CGI/CollectionBlock.pm
+++ b/testbot/lib/ObjectModel/CGI/CollectionBlock.pm
@@ -539,20 +539,16 @@ sub OnAction($$)
                    "=" . uri_escape($MasterColValues->[$ColIndex]);
       }
     }
-    $self->{EnclosingPage}->Redirect($Target);
-    return 1;
+    exit($self->{EnclosingPage}->Redirect($Target));
   }
-  else
+
+  my $Ok = 1;
+  foreach my $Key (@{$self->{Collection}->GetKeys()})
   {
-    my $Ok = 1;
-    foreach my $Key (@{$self->{Collection}->GetKeys()})
+    if (defined $self->{EnclosingPage}->GetParam($self->SelName($Key)) && $Ok)
     {
-      if (defined($self->{EnclosingPage}->GetParam($self->SelName($Key))) &&
-          $Ok)
-      {
-        my $Item = $self->{Collection}->GetItem($Key);
-        $Ok = $self->CallOnItemAction($Item, $Action);
-      }
+      my $Item = $self->{Collection}->GetItem($Key);
+      $Ok = $self->CallOnItemAction($Item, $Action);
     }
   }
 }
diff --git a/testbot/lib/ObjectModel/CGI/ItemPage.pm b/testbot/lib/ObjectModel/CGI/ItemPage.pm
index 6d9449173d..940115475f 100644
--- a/testbot/lib/ObjectModel/CGI/ItemPage.pm
+++ b/testbot/lib/ObjectModel/CGI/ItemPage.pm
@@ -151,13 +151,11 @@ sub OnAction($$)
   if ($Action eq "OK")
   {
     return !1 if (!$self->Save());
-    $self->RedirectToList();
-    exit;
+    exit($self->RedirectToList());
   }
-  elsif ($Action eq "Cancel")
+  if ($Action eq "Cancel")
   {
-    $self->RedirectToList();
-    exit;
+    exit($self->RedirectToList());
   }
 
   return $self->SUPER::OnAction($Action);
@@ -177,7 +175,7 @@ sub RedirectToList($)
                  "=" . url_escape($MasterColValues->[$ColIndex]);
     }
   }
-  $self->Redirect($Target); # does not return
+  return $self->Redirect($Target);
 }
 
 1;
diff --git a/testbot/lib/ObjectModel/CGI/Page.pm b/testbot/lib/ObjectModel/CGI/Page.pm
index 8abaaaa9d2..b0450d0e78 100644
--- a/testbot/lib/ObjectModel/CGI/Page.pm
+++ b/testbot/lib/ObjectModel/CGI/Page.pm
@@ -246,7 +246,7 @@ sub Redirect($$)
 {
   my ($self, $Location) = @_;
 
-  $self->{PageBase}->Redirect($self, $Location);
+  return $self->{PageBase}->Redirect($self, $Location);
 }
 
 sub GetCurrentSession($)
diff --git a/testbot/lib/WineTestBot/CGI/PageBase.pm b/testbot/lib/WineTestBot/CGI/PageBase.pm
index d38e8d96b6..b7edd8fcaa 100644
--- a/testbot/lib/WineTestBot/CGI/PageBase.pm
+++ b/testbot/lib/WineTestBot/CGI/PageBase.pm
@@ -56,7 +56,7 @@ sub new($$$$@)
         ! $Session->User->HasRole($RequiredRole))
     {
       my $LoginURL = "/Login.pl?Target=" . uri_escape($ENV{"REQUEST_URI"});
-      $self->Redirect($Page, MakeSecureURL($LoginURL));
+      exit($self->Redirect($Page, MakeSecureURL($LoginURL)));
     }
   }
 
@@ -75,7 +75,7 @@ sub CheckSecurePage($$)
 
   if ($UseSSL && ! SecureConnection())
   {
-    $self->Redirect($Page, MakeSecureURL($ENV{"REQUEST_URI"}));
+    exit($self->Redirect($Page, MakeSecureURL($ENV{"REQUEST_URI"})));
   }
 }
 
@@ -415,7 +415,7 @@ sub Redirect($$$)
   }
   $self->{Request}->headers_out->set("Location", $Location);
   $self->{Request}->status(Apache2::Const::REDIRECT);
-  exit;
+  return 0; # a suitable exit code
 }
 
 sub GetCurrentSession($)
diff --git a/testbot/web/JobDetails.pl b/testbot/web/JobDetails.pl
index 919c7a5e3b..8d93957ce6 100644
--- a/testbot/web/JobDetails.pl
+++ b/testbot/web/JobDetails.pl
@@ -52,7 +52,7 @@ sub _initialize($$$)
   $self->{Job} = CreateJobs()->GetItem($JobId);
   if (!defined $self->{Job})
   {
-    $self->Redirect("/"); # does not return
+    exit($self->Redirect("/"));
   }
   $self->{JobId} = $JobId;
 
@@ -193,8 +193,7 @@ sub OnCancel($)
     return !1;
   }
 
-  $self->Redirect("/JobDetails.pl?Key=" . $self->{JobId}); # does not return
-  exit;
+  exit($self->Redirect("/JobDetails.pl?Key=" . $self->{JobId}));
 }
 
 sub OnRestart($)
@@ -215,8 +214,7 @@ sub OnRestart($)
     return !1;
   }
 
-  $self->Redirect("/JobDetails.pl?Key=" . $self->{JobId}); # does not return
-  exit;
+  exit($self->Redirect("/JobDetails.pl?Key=" . $self->{JobId}));
 }
 
 sub OnAction($$$)
diff --git a/testbot/web/Login.pl b/testbot/web/Login.pl
index d9757edb92..ac64fc28ea 100644
--- a/testbot/web/Login.pl
+++ b/testbot/web/Login.pl
@@ -135,8 +135,7 @@ sub OnLogIn($)
   {
     $Target = "/";
   }
-  $self->Redirect(MakeSecureURL($Target)); # does not return
-  exit;
+  exit($self->Redirect(MakeSecureURL($Target)));
 }
 
 sub OnAction($$)
diff --git a/testbot/web/Submit.pl b/testbot/web/Submit.pl
index 98d13381e0..79d1c8772d 100644
--- a/testbot/web/Submit.pl
+++ b/testbot/web/Submit.pl
@@ -1265,8 +1265,7 @@ sub _SubmitJob($$)
     return undef;
   }
 
-  $self->Redirect("/JobDetails.pl?Key=". $NewJob->GetKey()); # does not return
-  exit;
+  exit($self->Redirect("/JobDetails.pl?Key=". $NewJob->GetKey()));
 }
 
 sub OnSubmit($)
@@ -1323,14 +1322,8 @@ sub OnOK($)
 {
   my ($self) = @_;
 
-  if (defined $self->{JobKey})
-  {
-    $self->Redirect("/JobDetails.pl?Key=$self->{JobKey}"); # does not return
-  }
-  else
-  {
-    $self->Redirect("/"); # does not return
-  }
+  my $Target = defined $self->{JobKey} ? "/JobDetails.pl?Key=$self->{JobKey}" : "/";
+  exit($self->Redirect($Target));
 }
 
 sub OnAction($$)
diff --git a/testbot/web/admin/Log.pl b/testbot/web/admin/Log.pl
index cb253bd03e..456e73f55d 100644
--- a/testbot/web/admin/Log.pl
+++ b/testbot/web/admin/Log.pl
@@ -71,8 +71,8 @@ sub GetActions($)
 sub OnDownload($)
 {
   my ($self) = @_;
-  $self->Redirect("/admin/SendLog.pl?Hours=". $self->GetParam("Hours")); # does not return
-  exit;
+
+  exit($self->Redirect("/admin/SendLog.pl?Hours=". $self->GetParam("Hours")));
 }
 
 sub OnAction($$)
diff --git a/testbot/web/admin/SpecialJobs.pl b/testbot/web/admin/SpecialJobs.pl
index 534336c483..02d9f8ea9d 100644
--- a/testbot/web/admin/SpecialJobs.pl
+++ b/testbot/web/admin/SpecialJobs.pl
@@ -237,8 +237,7 @@ sub OnSubmit($)
     return undef;
   }
 
-  $self->Redirect("/"); # does not return
-  exit;
+  exit($self->Redirect("/"));
 }
 
 sub OnAction($$)
diff --git a/testbot/web/admin/UserDetails.pl b/testbot/web/admin/UserDetails.pl
index 2cf0f597fb..8c26930e8e 100644
--- a/testbot/web/admin/UserDetails.pl
+++ b/testbot/web/admin/UserDetails.pl
@@ -74,8 +74,7 @@ sub OnApprove($)
   return !1 if (!$self->Save());
   $self->{ErrMessage} = $self->{Item}->Approve();
   return !1 if (defined $self->{ErrMessage});
-  $self->RedirectToList(); # does not return
-  exit;
+  exit($self->RedirectToList());
 }
 
 sub OnReject($)
@@ -87,8 +86,7 @@ sub OnReject($)
   return !1 if (defined $self->{ErrMessage});
   # Forcefully log out that user by deleting his web sessions
   DeleteSessions($self->{Item});
-  $self->RedirectToList(); # does not return
-  exit;
+  exit($self->RedirectToList());
 }
 
 sub OnOK($)
@@ -101,8 +99,7 @@ sub OnOK($)
     # Forcefully log out that user by deleting his web sessions
     DeleteSessions($self->{Item});
   }
-  $self->RedirectToList(); # does not return
-  exit;
+  exit($self->RedirectToList());
 }
 
 sub OnAction($$)
-- 
2.30.2