Merge pull request #1169

dird: bconsole: add support for comma separated jobstatus values in list jobs command

CHANGELOG.md

@@ -78,6 +78,7 @@ and since Bareos version 20 this project adheres to [Semantic Versioning](https:
 - jstreegrid: remove handling of IE < 8 using navigator interface to avoid warnings in chrome [PR #1140]
 - `bvfs_update` now uses `unordered_map` instead of `htable` for the pathid cache [PR #1138]
 - cats: filtered zero file jobs list is now sorted [PR #1172]
+- dird: console: changed list jobs jobstatus argument to accept comma separated value [PR #1169]
 
 ### Deprecated
 

core/src/cats/cats.h

@@ -789,7 +789,7 @@ class BareosDb : public BareosDbQueryEnum {
                       JobDbRecord* jr,
                       const char* range,
                       const char* clientname,
-                      int jobstatus,
+                      std::vector<char> jobstatusarray,
                       int joblevel,
                       std::vector<char> jobtypes,
                       const char* volumename,

core/src/cats/sql_list.cc

@@ -507,7 +507,7 @@ void BareosDb::ListJobRecords(JobControlRecord* jcr,
                               JobDbRecord* jr,
                               const char* range,
                               const char* clientname,
-                              int jobstatus,
+                              std::vector<char> jobstatuslist,
                               int joblevel,
                               std::vector<char> jobtypes,
                               const char* volumename,
@@ -539,8 +539,10 @@ void BareosDb::ListJobRecords(JobControlRecord* jcr,
     PmStrcat(selection, temp.c_str());
   }
 
-  if (jobstatus) {
-    temp.bsprintf("AND Job.JobStatus = '%c' ", jobstatus);
+  if (!jobstatuslist.empty()) {
+    std::string jobStatuses
+        = CreateDelimitedStringForSqlQueries(jobstatuslist, ',');
+    temp.bsprintf("AND Job.JobStatus in (%s) ", jobStatuses.c_str());
     PmStrcat(selection, temp.c_str());
   }
 

core/src/dird/ua_output.cc

@@ -676,9 +676,9 @@ static bool DoListCmd(UaContext* ua, const char* cmd, e_list_type llist)
   }
 
 
-  // jobstatus=X
-  int jobstatus = 0;
-  if (!GetUserJobStatusSelection(ua, &jobstatus)) {
+  // jobstatus=X,Y,Z....
+  std::vector<char> jobstatuslist;
+  if (!GetUserJobStatusSelection(ua, jobstatuslist)) {
     ua->ErrorMsg(_("invalid jobstatus parameter\n"));
     return false;
   }
@@ -756,9 +756,9 @@ static bool DoListCmd(UaContext* ua, const char* cmd, e_list_type llist)
     SetQueryRange(query_range, ua, &jr);
 
     ua->db->ListJobRecords(ua->jcr, &jr, query_range.c_str(), clientname,
-                           jobstatus, joblevel, jobtypes, volumename, poolname,
-                           schedtime, optionslist.last, optionslist.count,
-                           ua->send, llist);
+                           jobstatuslist, joblevel, jobtypes, volumename,
+                           poolname, schedtime, optionslist.last,
+                           optionslist.count, ua->send, llist);
   } else if (Bstrcasecmp(ua->argk[1], NT_("jobtotals"))) {
     // List JOBTOTALS
     ua->db->ListJobTotals(ua->jcr, &jr, ua->send);
@@ -807,7 +807,7 @@ static bool DoListCmd(UaContext* ua, const char* cmd, e_list_type llist)
         SetQueryRange(query_range, ua, &jr);
 
         ua->db->ListJobRecords(ua->jcr, &jr, query_range.c_str(), clientname,
-                               jobstatus, joblevel, jobtypes, volumename,
+                               jobstatuslist, joblevel, jobtypes, volumename,
                                poolname, schedtime, optionslist.last,
                                optionslist.count, ua->send, llist);
       }

core/src/dird/ua_select.cc

@@ -994,8 +994,9 @@ PoolResource* get_pool_resource(UaContext* ua)
 // List all jobs and ask user to select one
 int SelectJobDbr(UaContext* ua, JobDbRecord* jr)
 {
-  ua->db->ListJobRecords(ua->jcr, jr, "", NULL, 0, 0, std::vector<char>{}, NULL,
-                         NULL, 0, 0, 0, ua->send, HORZ_LIST);
+  ua->db->ListJobRecords(ua->jcr, jr, "", NULL, std::vector<char>{}, 0,
+                         std::vector<char>{}, NULL, NULL, 0, 0, 0, ua->send,
+                         HORZ_LIST);
   if (!GetPint(ua, _("Enter the JobId to select: "))) { return 0; }
 
   jr->JobId = ua->int64_val;
@@ -1952,28 +1953,37 @@ bool GetUserJobTypeListSelection(UaContext* ua,
   return true;
 }
 
-bool GetUserJobStatusSelection(UaContext* ua, int* jobstatus)
+bool GetUserJobStatusSelection(UaContext* ua, std::vector<char>& jobstatuslist)
 {
   int i;
 
   if ((i = FindArgWithValue(ua, NT_("jobstatus"))) >= 0) {
-    if (strlen(ua->argv[i]) == 1 && ua->argv[i][0] >= 'A'
-        && ua->argv[i][0] <= 'z') {
-      *jobstatus = ua->argv[i][0];
-    } else if (Bstrcasecmp(ua->argv[i], "terminated")) {
-      *jobstatus = JS_Terminated;
-    } else if (Bstrcasecmp(ua->argv[i], "warnings")) {
-      *jobstatus = JS_Warnings;
-    } else if (Bstrcasecmp(ua->argv[i], "canceled")) {
-      *jobstatus = JS_Canceled;
-    } else if (Bstrcasecmp(ua->argv[i], "running")) {
-      *jobstatus = JS_Running;
-    } else if (Bstrcasecmp(ua->argv[i], "error")) {
-      *jobstatus = JS_ErrorTerminated;
-    } else if (Bstrcasecmp(ua->argv[i], "fatal")) {
-      *jobstatus = JS_FatalError;
+    if (strlen(ua->argv[i]) > 0) {
+      std::vector<std::string> jobstatusinput_list;
+      jobstatusinput_list = split_string(ua->argv[i], ',');
+
+      for (auto& jobstatus : jobstatusinput_list) {
+        if (strlen(jobstatus.c_str()) == 1 && jobstatus.c_str()[0] >= 'A'
+            && jobstatus.c_str()[0] <= 'z') {
+          jobstatuslist.push_back(jobstatus[0]);
+        } else if (Bstrcasecmp(jobstatus.c_str(), "terminated")) {
+          jobstatuslist.push_back(JS_Terminated);
+        } else if (Bstrcasecmp(jobstatus.c_str(), "warnings")) {
+          jobstatuslist.push_back(JS_Warnings);
+        } else if (Bstrcasecmp(jobstatus.c_str(), "canceled")) {
+          jobstatuslist.push_back(JS_Canceled);
+        } else if (Bstrcasecmp(jobstatus.c_str(), "running")) {
+          jobstatuslist.push_back(JS_Running);
+        } else if (Bstrcasecmp(jobstatus.c_str(), "error")) {
+          jobstatuslist.push_back(JS_ErrorTerminated);
+        } else if (Bstrcasecmp(jobstatus.c_str(), "fatal")) {
+          jobstatuslist.push_back(JS_FatalError);
+        } else {
+          /* invalid jobstatus */
+          return false;
+        }
+      }
     } else {
-      /* invalid jobstatus */
       return false;
     }
   }

core/src/dird/ua_select.h

@@ -92,7 +92,7 @@ bool GetUserSlotList(UaContext* ua,
 bool GetUserJobTypeListSelection(UaContext* ua,
                                  std::vector<char>& passed_jobtypes,
                                  bool ask_user);
-bool GetUserJobStatusSelection(UaContext* ua, int* jobstatus);
+bool GetUserJobStatusSelection(UaContext* ua, std::vector<char>& jobstatus);
 bool GetUserJobLevelSelection(UaContext* ua, int* joblevel);
 
 int FindArgKeyword(UaContext* ua, const char** list);

core/src/tests/select_functions.cc

@@ -27,6 +27,8 @@
 #  include "include/bareos.h"
 #endif
 
+#include <unordered_map>
+
 #include "dird/ua_select.h"
 #include "dird/ua.h"
 #include "include/jcr.h"
@@ -190,3 +192,115 @@ TEST_F(JobTypeSelection, NonPermittedJobtypesAreNotParsed)
   EXPECT_FALSE(GetUserJobTypeListSelection(ua, jobtypelist, false));
   EXPECT_TRUE(jobtypelist.empty());
 }
+
+
+// Unit Tests for Job status argument
+
+
+class JobStatusSelection : public testing::Test {
+ protected:
+  void SetUp() override { ua = directordaemon::new_ua_context(&jcr); }
+
+  void TearDown() override { FreeUaContext(ua); }
+  void FakeListCommand(directordaemon::UaContext* ua, std::string arguments)
+  {
+    FakeCmd(ua, "list jobs " + arguments);
+  }
+  void FakeListJobStatusCommand(std::string argument_value)
+  {
+    FakeCmd(ua, "list jobs jobstatus=" + argument_value);
+  }
+
+  JobControlRecord jcr{};
+  directordaemon::UaContext* ua{nullptr};
+  std::unordered_map<char, std::string> allowed_jobstatuses{
+      {JS_Terminated, "terminated"}, {JS_Warnings, "warnings"},
+      {JS_Canceled, "canceled"},     {JS_Running, "running"},
+      {JS_ErrorTerminated, "error"}, {JS_FatalError, "fatal"}};
+};
+
+
+TEST_F(JobStatusSelection, NothingHappensWhenJobstatusNotSpecified)
+{
+  std::vector<char> jobstatuslist{};
+  FakeListCommand(ua, "");
+  EXPECT_TRUE(GetUserJobStatusSelection(ua, jobstatuslist));
+  EXPECT_TRUE(jobstatuslist.empty());
+}
+
+TEST_F(JobStatusSelection, ErrorWhenJobtatusArgumentSpecifiedButNoneGiven)
+{
+  std::vector<char> jobstatuslist{};
+  FakeListJobStatusCommand("");
+  EXPECT_FALSE(GetUserJobStatusSelection(ua, jobstatuslist));
+  EXPECT_TRUE(jobstatuslist.empty());
+}
+
+TEST_F(JobStatusSelection, ReturnOnlyOneJobStatusIfOnlyOneIsEntered)
+{
+  for (const auto& jobstatus : allowed_jobstatuses) {
+    std::vector<char> jobstatuslist{};
+    std::string argument{jobstatus.first};
+    FakeListJobStatusCommand(argument);
+    EXPECT_TRUE(GetUserJobStatusSelection(ua, jobstatuslist));
+    EXPECT_EQ(jobstatuslist[0], jobstatus.first);
+  }
+}
+
+TEST_F(JobStatusSelection,
+       ReturnOnlyOneShortJobStatusIfOnlyOneLongJobStatusIsEntered)
+{
+  for (const auto& jobstatus : allowed_jobstatuses) {
+    std::vector<char> jobstatuslist{};
+    std::string argument{jobstatus.second};
+    FakeListJobStatusCommand(argument);
+    EXPECT_TRUE(GetUserJobStatusSelection(ua, jobstatuslist));
+    EXPECT_EQ(jobstatuslist[0], jobstatus.first);
+  }
+}
+
+TEST_F(JobStatusSelection,
+       ReturnMultipleShortJobStatusIfMultipleLongJobStatusesEntered)
+{
+  std::vector<char> jobstatuslist{};
+  std::vector<char> expectedJobStatusList{};
+  std::string argumentForMultipleLongJobstatus;
+  for (const auto& jobstatus : allowed_jobstatuses) {
+    argumentForMultipleLongJobstatus += jobstatus.second;
+    argumentForMultipleLongJobstatus += ',';
+
+    expectedJobStatusList.push_back(jobstatus.first);
+  }
+  argumentForMultipleLongJobstatus.pop_back();
+
+  FakeListJobStatusCommand(argumentForMultipleLongJobstatus);
+  EXPECT_TRUE(GetUserJobStatusSelection(ua, jobstatuslist));
+  EXPECT_EQ(jobstatuslist, expectedJobStatusList);
+}
+
+TEST_F(JobStatusSelection,
+       ReturnMultipleShortJobStatusIfMultipleLongAndShortJobstatusesEntered)
+{
+  std::vector<char> jobstatuslist{};
+  std::vector<char> expectedJobStatusList{};
+  std::string argumentForMultipleLongAndShortJobstatus;
+  int i = 0;
+  for (const auto& jobstatus : allowed_jobstatuses) {
+    if (i % 2 == 0) {
+      argumentForMultipleLongAndShortJobstatus
+          += jobstatus.first;  // short jobstatus
+    } else {
+      argumentForMultipleLongAndShortJobstatus
+          += jobstatus.second;  // long jobstatus
+    }
+    argumentForMultipleLongAndShortJobstatus += ',';
+
+    expectedJobStatusList.push_back(jobstatus.first);
+    i++;
+  }
+  argumentForMultipleLongAndShortJobstatus.pop_back();
+
+  FakeListJobStatusCommand(argumentForMultipleLongAndShortJobstatus);
+  EXPECT_TRUE(GetUserJobStatusSelection(ua, jobstatuslist));
+  EXPECT_EQ(jobstatuslist, expectedJobStatusList);
+}

systemtests/tests/python-bareos/test_list_command.py

@@ -239,6 +239,15 @@ def test_list_jobs(self):
         for job in result["jobs"]:
             self.assertTrue(job["jobstatus"], "T")
 
+        # run RestoreFiles with a non existant jobId so it fails
+        director.call("run job=RestoreFiles jobid=999999 yes")
+
+        # list jobs jobstatus=X,Y,z
+        result = director.call("list jobs jobstatus=T,f")
+        self.assertTrue(result["jobs"])
+        for job in result["jobs"]:
+            self.assertTrue(job["jobstatus"] == "T" or job["jobstatus"] == "f")
+
         result = director.call("list jobs jobstatus=R")
         self.assertFalse(result["jobs"])