[tools] testbot/web: Use AddError() and ResetErrors() on the submit job page.

[Francois Gouget]

This replaces a lot of direct field accesses.
Also clearly identify unused variables.

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com>
---
 testbot/web/Submit.pl | 100 ++++++++++++++++--------------------------
 1 file changed, 37 insertions(+), 63 deletions(-)

diff --git a/testbot/web/Submit.pl b/testbot/web/Submit.pl
index c5423c7951..4368aeeb30 100644
--- a/testbot/web/Submit.pl
+++ b/testbot/web/Submit.pl
@@ -77,16 +77,14 @@ sub _Upload($)
     {
       umask($OldUMask);
       delete $self->{FileName};
-      $self->{ErrField} = "File";
-      $self->{ErrMessage} = "Unable to save the uploaded file";
+      $self->AddError("Unable to save the uploaded file", "File");
       return undef;
     }
   }
   else
   {
     delete $self->{FileName};
-    $self->{ErrField} = "File";
-    $self->{ErrMessage} = "Unable to upload the file";
+    $self->AddError("Unable to upload the file", "File");
     return undef;
   }
   return 1;
@@ -102,8 +100,7 @@ sub _GetFileType($)
   my $Fh;
   if (!sysopen($Fh, $self->_GetStagingFilePath(), O_RDONLY))
   {
-    $self->{ErrField} = "FileName";
-    $self->{ErrMessage} = "Unable to open '$self->{FileName}'";
+    $self->AddError("Unable to open '$self->{FileName}'", "FileName");
     return undef;
   }
 
@@ -153,8 +150,7 @@ sub _GetFileType($)
 
   if ($self->{FileType} !~ /^(?:exe32|exe64|patch)$/)
   {
-    $self->{ErrField} = "FileName";
-    $self->{ErrMessage} = "Unsupported file type";
+    $self->SerError("FileName", "Unsupported file type");
     return undef;
   }
 
@@ -169,14 +165,12 @@ sub _AnalyzePatch($)
 
   if (!$self->{Impacts}->{TestBuild})
   {
-    $self->{ErrField} = "FileName";
-    $self->{ErrMessage} = "'$self->{FileName}' is not a valid patch";
+    $self->AddError("'$self->{FileName}' is not a valid patch", "FileName");
     return undef;
   }
   if ($self->{Impacts}->{PatchedModuleOrTests} == 0)
   {
-    $self->{ErrField} = "FileName";
-    $self->{ErrMessage} = "The patch does not directly impact the Wine dlls and programs";
+    $self->AddError("The patch does not directly impact the Wine dlls and programs", "FileName");
     return undef;
   }
   return 1;
@@ -194,23 +188,20 @@ sub _ValidateFileName($)
   if (!defined $self->{FileName})
   {
     delete $self->{FileName};
-    $self->{ErrField} = "FileName";
-    $self->{ErrMessage} = "You must provide a file to test";
+    $self->AddError("You must provide a file to test", "FileName");
     return undef;
   }
   if (!IsValidFileName($self->{FileName}))
   {
     delete $self->{FileName};
-    $self->{ErrField} = "FileName";
-    $self->{ErrMessage} = "The filename contains invalid characters";
+    $self->AddError("The filename contains invalid characters", "FileName");
     return undef;
   }
   my $PropertyDescriptor = CreateSteps()->GetPropertyDescriptorByName("FileName");
   if ($PropertyDescriptor->GetMaxLength() - 32 - 1 < length($self->{FileName}))
   {
     delete $self->{FileName};
-    $self->{ErrField} = "FileName";
-    $self->{ErrMessage} = "The filename is too long";
+    $self->AddError("The filename is too long", "FileName");
     return undef;
   }
 
@@ -290,7 +281,7 @@ sub _ValidateVMSelection($;$)
 
   if (@Deselected)
   {
-    $self->{ErrMessage} = "The following VMs are incompatible and have been deselected: @Deselected";
+    $self->AddError("The following VMs are incompatible and have been deselected: @Deselected");
     return undef;
   }
   return 1;
@@ -337,8 +328,7 @@ sub _ValidateTestExecutable($;$)
 
   if (!defined $self->{TestExecutable})
   {
-    $self->{ErrField} = "TestExecutable";
-    $self->{ErrMessage} = "You must specify the test executable filename";
+    $self->AddError("You must specify the test executable filename", "TestExecutable");
     return undef;
   }
   elsif (!$self->{TestExecutables}->{$self->{TestExecutable}} or
@@ -346,8 +336,7 @@ sub _ValidateTestExecutable($;$)
          # to defend against malicious patches.
          !IsValidFileName($self->{TestExecutable}))
   {
-    $self->{ErrField} = "TestExecutable";
-    $self->{ErrMessage} = "Invalid test executable filename";
+    $self->AddError("Invalid test executable filename", "TestExecutable");
     return undef;
   }
 
@@ -393,14 +382,12 @@ sub Validate($)
     {
       # Note that Page 1 should have provided a default
       # so there is no reason for it to still be undefined.
-      $self->{ErrField} = "Branch";
-      $self->{ErrMessage} = "You must specify the branch to test";
+      $self->AddError("You must specify the branch to test", "Branch");
       return undef;
     }
     if (!CreateBranches()->GetItem($self->{Branch}))
     {
-      $self->{ErrField} = "Branch";
-      $self->{ErrMessage} = "The '$self->{Branch}' branch does not exist";
+      $self->AddError("The '$self->{Branch}' branch does not exist", "Branch");
       return undef;
     }
   }
@@ -410,7 +397,7 @@ sub Validate($)
     return undef if (!$self->_ValidateVMSelection());
     if (!$self->{CheckedVMCount})
     {
-      $self->{ErrMessage} = "Select at least one VM";
+      $self->AddError("Select at least one VM");
       return undef;
     }
   }
@@ -424,18 +411,17 @@ sub Validate($)
         !$self->{Run64} and $self->{ShowRun64})
     {
       # Don't set ErrField since Run32 and Run64 may be hidden
-      $self->{ErrMessage} = "You must at least pick one of the 32 or 64 bit tests to run on the selected Windows VMs.";
+      $self->AddError("You must at least pick one of the 32 or 64 bit tests to run on the selected Windows VMs.");
       return undef;
     }
 
     if ($self->{CmdLineArg} eq "" and !$self->{NoCmdLineArgWarn})
     {
-      $self->{ErrMessage} = "You did not specify a command line argument. ".
-                            "This is most likely not correct, so please ".
-                            "fix this. If you are sure that you really don't ".
-                            'want a command line argument, press "Submit" '.
-                            "again.";
-      $self->{ErrField} = "CmdLineArg";
+      $self->AddError("You did not specify a command line argument. ".
+                      "This is most likely not correct, so please ".
+                      "fix this. If you are sure that you really don't ".
+                      'want a command line argument, press "Submit" again.',
+                      "CmdLineArg");
       $self->{NoCmdLineArgWarn} = 1;
       return undef;
     }
@@ -969,8 +955,7 @@ sub OnUnset($)
   if (!$self->_ValidateFileName())
   {
     # Ignore the error. What counts is not using a suspicious FileName.
-    delete $self->{ErrField};
-    delete $self->{ErrMessage};
+    $self->ResetErrors();
   }
   elsif (defined $self->{FileName})
   {
@@ -1012,12 +997,8 @@ sub OnSinglePage($)
   my ($self) = @_;
 
   # The checks and defaults are the same as for Page1Next()
-  if (!$self->OnPage1Next())
-  {
-    # But ignore all errors. The user can change everything anyway.
-    delete $self->{ErrField};
-    delete $self->{ErrMessage};
-  }
+  # But ignore all errors. The user can change everything anyway.
+  $self->ResetErrors() if (!$self->OnPage1Next());
 
   $self->_SetPage(4);
   return 1;
@@ -1151,7 +1132,7 @@ sub _SubmitJob($$)
       }
       if (defined $ErrMessage)
       {
-        $self->{ErrMessage} = $ErrMessage;
+        $self->AddError($ErrMessage);
         return !1;
       }
     }
@@ -1210,7 +1191,7 @@ sub _SubmitJob($$)
       my ($ErrMessage, $Missions) = ParseMissionStatement($WineMissions{$VM});
       if (defined $ErrMessage)
       {
-        $self->{ErrMessage} = $ErrMessage;
+        $self->AddError($ErrMessage);
         return !1;
       }
       my $Missions = $Missions->[0];
@@ -1229,34 +1210,32 @@ sub _SubmitJob($$)
   }
 
   # Now save it all (or whatever's left to save)
-  my ($ErrKey, $ErrProperty, $ErrMessage) = $Jobs->Save();
+  my ($_ErrKey, $_ErrProperty, $ErrMessage) = $Jobs->Save();
   if (defined $ErrMessage)
   {
-    $self->{ErrMessage} = $ErrMessage;
+    $self->AddError($ErrMessage);
     return undef;
   }
 
   # Stage the test patch/executable so the job can pick it up
   if (!rename($Staging, "$DataDir/staging/job". $NewJob->Id ."_$self->{FileName}"))
   {
-    $self->{ErrMessage} = "Could not stage '$self->{FileName}': $!\n";
+    $self->AddError("Could not stage '$self->{FileName}': $!");
     return undef;
   }
 
   # Switch Status to staging to indicate we are done setting up the job
   $NewJob->Status("staging");
-  ($ErrKey, $ErrProperty, $ErrMessage) = $Jobs->Save();
+  ($_ErrKey, $_ErrProperty, $ErrMessage) = $Jobs->Save();
   if (defined $ErrMessage)
   {
-    $self->{ErrMessage} = $ErrMessage;
+    $self->AddError($ErrMessage);
     return undef;
   }
 
   # Notify engine
-  my $ErrMessage = RescheduleJobs();
-  if (defined $ErrMessage)
+  if ($self->AddError(RescheduleJobs()))
   {
-    $self->{ErrMessage} = $ErrMessage;
     $self->_SetPage(0);
     $self->{JobKey} = $NewJob->GetKey();
     return undef;
@@ -1277,15 +1256,15 @@ sub OnSubmit($)
   my $Staging = CreateNewLink($OldStaging, "$DataDir/staging", "-websubmit2_$self->{FileName}");
   if (!defined $Staging)
   {
-    $self->{ErrMessage} = "Could not rename '$self->{FileName}': $!";
+    $self->AddError("Could not rename '$self->{FileName}': $!");
     return undef;
   }
   if (!unlink $OldStaging)
   {
     unlink $Staging;
-    $self->{ErrMessage} = $!{ENOENT} ?
+    $self->AddError($!{ENOENT} ?
         "$self->{FileName} has already been submitted or has expired" :
-        "Could not remove the staging '$self->{FileName}' file: $!";
+        "Could not remove the staging '$self->{FileName}' file: $!");
     return undef;
   }
 
@@ -1303,13 +1282,8 @@ sub OnSetShowAllVMs($$)
   my ($self, $Value) = @_;
 
   # Call _ValidateVMSelection() to identify incompatible VMs so they are not
-  # marked as Checked
-  if (!$self->_ValidateVMSelection("deselect"))
-  {
-    # Ignore errors
-    delete $self->{ErrField};
-    delete $self->{ErrMessage};
-  }
+  # marked as Checked but ignore errors.
+  $self->ResetErrors() if (!$self->_ValidateVMSelection("deselect"));
   $self->{ShowAll} = $Value;
 
   return undef;
-- 
2.30.2