Refactor/create job

cmd/commands/datasetArchiver.go

@@ -126,7 +126,10 @@ For further help see "` + MANUAL + `"`,
 
 		log.Printf("You chose to archive the new datasets\n")
 		log.Printf("Submitting Archive Job for the ingested datasets.\n")
-		jobId := datasetUtils.CreateJob(client, APIServer, user, archivableDatasets, &tapecopies)
+		jobId, err := datasetUtils.CreateArchivalJob(client, APIServer, user, archivableDatasets, &tapecopies)
+		if err != nil {
+			log.Fatalf("Couldn't create a job: %s\n", err.Error())
+		}
 		fmt.Println(jobId)
 	},
 }

cmd/commands/datasetIngestor.go

@@ -372,7 +372,12 @@ For Windows you need instead to specify -user username:password on the command l
 			log.Printf("Submitting Archive Job for the ingested datasets.\n")
 			// TODO: change param type from pointer to regular as it is unnecessary
 			//   for it to be passed as pointer
-			datasetUtils.CreateJob(client, APIServer, user, datasetList, &tapecopies)
+			_, err := datasetUtils.CreateArchivalJob(client, APIServer, user, datasetList, &tapecopies)
+			if err != nil {
+				color.Set(color.FgRed)
+				log.Printf("Could not create the archival job for the ingested datasets: %s", err.Error())
+				color.Unset()
+			}
 		}
 
 		// print out results to STDOUT, one line per dataset

datasetUtils/createJob.go

@@ -3,7 +3,7 @@ package datasetUtils
 import (
 	"bytes"
 	"encoding/json"
-	"log"
+	"fmt"
 	"net/http"
 	"time"
 )
@@ -13,9 +13,9 @@ type Job struct {
 }
 
 /*
-`CreateJob` creates a new job on the server. It takes in an HTTP client, the API server URL, a user map, a list of datasets, and a pointer to an integer representing the number of tape copies. 
+`CreateArchivalJob` creates a new job on the server. It takes in an HTTP client, the API server URL, a user map, a list of datasets, and a pointer to an integer representing the number of tape copies.
 
-The function constructs a job map with various parameters, including the email of the job initiator, the type of job, the creation time, the job parameters, and the job status message. It also includes a list of datasets. 
+The function constructs a job map with various parameters, including the email of the job initiator, the type of job, the creation time, the job parameters, and the job status message. It also includes a list of datasets.
 
 The job map is then marshalled into JSON and sent as a POST request to the server. If the server responds with a status code of 200, the function decodes the job ID from the response and returns it. If the server responds with any other status code, the function returns an empty string.
 
@@ -29,7 +29,7 @@ Parameters:
 Returns:
 - jobId: A string representing the job ID if the job was successfully created, or an empty string otherwise
 */
-func CreateJob(client *http.Client, APIServer string, user map[string]string, datasetList []string, tapecopies *int) (jobId string) {
+func CreateArchivalJob(client *http.Client, APIServer string, user map[string]string, datasetList []string, tapecopies *int) (jobId string, err error) {
 	// important: define field with capital names and rename fields via 'json' constructs
 	// otherwise the marshaling will omit the fields !
 
@@ -75,23 +75,21 @@ func CreateJob(client *http.Client, APIServer string, user map[string]string, da
 
 	resp, err := client.Do(req)
 	if err != nil {
-		log.Fatal(err)
+		return "", err
 	}
 	defer resp.Body.Close()
 
 	if resp.StatusCode == 200 {
-		log.Println("Job response Status: okay")
-		log.Println("A confirmation email will be sent to", user["mail"])
+		// the request succeeded based on status code
+		// an email should be sent by SciCat to user["email"]
 		decoder := json.NewDecoder(resp.Body)
 		var j Job
 		err := decoder.Decode(&j)
 		if err != nil {
-			log.Println("Could not decode id from job:", err)
-			return ""
+			return "", fmt.Errorf("CreateJob - could not decode id from job: %v", err)
 		}
-		return j.Id
+		return j.Id, err
 	} else {
-		log.Println("Job response Status: there are problems:", resp.StatusCode)
-		return ""
+		return "", fmt.Errorf("CreateJob - request returned unexpected status code: %d", resp.StatusCode)
 	}
 }

datasetUtils/createJob_test.go

@@ -1,13 +1,13 @@
 package datasetUtils
 
 import (
-	"net/http"
-	"net/http/httptest"
-	"testing"
-	"io"
 	"bytes"
 	"encoding/json"
+	"io"
+	"net/http"
+	"net/http/httptest"
 	"reflect"
+	"testing"
 )
 
 type MockTransport struct {
@@ -25,10 +25,10 @@ func TestCreateJob(t *testing.T) {
 			rw.Write([]byte(`{"id": "123"}`))
 		}))
 		defer server.Close()
-		
+
 		// Create a client
 		client := server.Client()
-		
+
 		// Define the parameters
 		APIServer := server.URL
 		user := map[string]string{
@@ -39,26 +39,29 @@ func TestCreateJob(t *testing.T) {
 		datasetList := []string{"dataset1", "dataset2"}
 		tapecopies := new(int)
 		*tapecopies = 1
-		
+
 		// Call the function
-		jobId := CreateJob(client, APIServer, user, datasetList, tapecopies)
-
+		jobId, err := CreateArchivalJob(client, APIServer, user, datasetList, tapecopies)
+		if err != nil {
+			t.Errorf("Unexpected error received: %v", err)
+		}
+
 		// Check the result
 		if jobId != "123" {
 			t.Errorf("Expected jobId to be '123', got '%s'", jobId)
 		}
 	})
-	
+
 	t.Run("server returns non-200 status code", func(t *testing.T) {
 		// Create a mock server
 		server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
 			rw.WriteHeader(http.StatusInternalServerError)
 		}))
 		defer server.Close()
-		
+
 		// Create a client
 		client := server.Client()
-		
+
 		// Define the parameters
 		APIServer := server.URL
 		user := map[string]string{
@@ -69,26 +72,34 @@ func TestCreateJob(t *testing.T) {
 		datasetList := []string{"dataset1", "dataset2"}
 		tapecopies := new(int)
 		*tapecopies = 1
-		
+
 		// Call the function
-		jobId := CreateJob(client, APIServer, user, datasetList, tapecopies)
-
+		jobId, err := CreateArchivalJob(client, APIServer, user, datasetList, tapecopies)
+		if err == nil {
+			t.Errorf("Expected an error to be returned from CreateJob")
+		}
+
+		const expectedError = "CreateJob - request returned unexpected status code: 500"
+		if err.Error() != expectedError {
+			t.Errorf("Got incorrect error from CreateJob - expected: \"%s\", gotten: \"%s\"", expectedError, err.Error())
+		}
+
 		// Check the result
 		if jobId != "" {
 			t.Errorf("Expected jobId to be '', got '%s'", jobId)
 		}
 	})
-	
+
 	t.Run("server returns invalid JSON", func(t *testing.T) {
 		// Create a mock server
 		server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
 			rw.Write([]byte(`invalid json`))
 		}))
 		defer server.Close()
-		
+
 		// Create a client
 		client := server.Client()
-		
+
 		// Define the parameters
 		APIServer := server.URL
 		user := map[string]string{
@@ -99,64 +110,76 @@ func TestCreateJob(t *testing.T) {
 		datasetList := []string{"dataset1", "dataset2"}
 		tapecopies := new(int)
 		*tapecopies = 1
-		
+
 		// Call the function
-		jobId := CreateJob(client, APIServer, user, datasetList, tapecopies)
-
+		jobId, err := CreateArchivalJob(client, APIServer, user, datasetList, tapecopies)
+
+		if err == nil {
+			t.Error("Expected an error to be returned from CreateJob")
+		}
+
+		const expectedError = "CreateJob - could not decode id from job: invalid character 'i' looking for beginning of value"
+		if err.Error() != expectedError {
+			t.Errorf("Got incorrect error from CreateJob - expected: \"%s\", gotten: \"%s\"", expectedError, err.Error())
+		}
+
 		// Check the result
 		if jobId != "" {
 			t.Errorf("Expected jobId to be '', got '%s'", jobId)
 		}
 	})
-	
+
 	t.Run("client.Do called with expected payload", func(t *testing.T) {
 		// Create a mock server
 		server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
 			rw.Write([]byte(`{"id": "123"}`))
 		}))
 		defer server.Close()
-		
+
 		user := map[string]string{
 			"mail":        "[email protected]",
 			"username":    "testuser",
 			"accessToken": "testtoken",
 		}
-    datasetList := []string{"dataset1", "dataset2"}
+		datasetList := []string{"dataset1", "dataset2"}
 		tapecopies := new(int)
-    *tapecopies = 2
+		*tapecopies = 2
 
 		// Create a mock client
 		client := &http.Client{
 			Transport: &MockTransport{
 				RoundTripFunc: func(req *http.Request) (*http.Response, error) {
 					body, _ := io.ReadAll(req.Body)
-					
+
 					// Parse the actual and expected payloads
 					var actualPayload, expectedPayload map[string]interface{}
 					json.Unmarshal(body, &actualPayload)
 					json.Unmarshal([]byte(`{"creationTime":"2024-05-21T15:25:34+02:00","datasetList":[{"pid":"dataset1","files":[]},{"pid":"dataset2","files":[]}],"emailJobInitiator":"[email protected]","jobParams":{"tapeCopies":"two","username":"testuser"},"jobStatusMessage":"jobSubmitted","type":"archive"}`), &expectedPayload)
-					
+
 					// Ignore the creationTime field
 					delete(actualPayload, "creationTime")
 					delete(expectedPayload, "creationTime")
-					
+
 					// Check if the payloads match
 					if !reflect.DeepEqual(actualPayload, expectedPayload) {
 						t.Errorf("Expected payload to be '%v', got '%v'", expectedPayload, actualPayload)
 					}
-					
+
 					// We still need to return a response
 					return &http.Response{
 						StatusCode: 200,
 						Body:       io.NopCloser(bytes.NewBufferString(`{"id": "123"}`)),
-						}, nil
-					},
+					}, nil
 				},
-			}
-
+			},
+		}
+
 		// Call the function with the mock client
-		jobId := CreateJob(client, server.URL, user, datasetList, tapecopies)
-
+		jobId, err := CreateArchivalJob(client, server.URL, user, datasetList, tapecopies)
+		if err != nil {
+			t.Errorf("Got an error when creating a job: %s", err.Error())
+		}
+
 		// Check the result
 		if jobId != "123" {
 			t.Errorf("Expected jobId to be '123', got '%s'", jobId)