From a41d61dac8e8fc1689f2083f667b51a47c28e57b Mon Sep 17 00:00:00 2001 From: "Ali R. Vahdati" Date: Mon, 13 May 2024 12:54:38 +0200 Subject: [PATCH 1/2] Add a unit test for `GetProposal` --- datasetUtils/getProposal.go | 21 +++++++++++++++++++-- datasetUtils/getProposal_test.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 datasetUtils/getProposal_test.go diff --git a/datasetUtils/getProposal.go b/datasetUtils/getProposal.go index 5a220b8..50eacad 100644 --- a/datasetUtils/getProposal.go +++ b/datasetUtils/getProposal.go @@ -2,11 +2,28 @@ package datasetUtils import ( "encoding/json" - "io/ioutil" + "io" "log" "net/http" ) +/* +GetProposal retrieves a proposal from a given API server. + +Parameters: +- client: An *http.Client object used to send the request. +- APIServer: A string representing the base URL of the API server. +- ownerGroup: A string representing the owner group of the proposal. +- user: A map containing user information, including an access token. +- accessGroups: A slice of strings representing the access groups of the user. + +The function constructs a filter based on the ownerGroup, then sends a GET request to the API server with the filter and user's access token. The response is then parsed into a map and returned. + +If the request or JSON unmarshalling fails, the function will log the error and terminate the program. + +Returns: +- A map representing the proposal. If no proposal is found, an empty map is returned. +*/ func GetProposal(client *http.Client, APIServer string, ownerGroup string, user map[string]string, accessGroups []string) (proposal map[string]interface{}) { // Check if ownerGroup is in accessGroups list. No longer needed, done on server side and @@ -40,7 +57,7 @@ func GetProposal(client *http.Client, APIServer string, ownerGroup string, user log.Fatal(err) } defer resp.Body.Close() - body, _ := ioutil.ReadAll(resp.Body) + body, _ := io.ReadAll(resp.Body) // fmt.Printf("response Object:\n%v\n", string(body)) diff --git a/datasetUtils/getProposal_test.go b/datasetUtils/getProposal_test.go new file mode 100644 index 0000000..57945e5 --- /dev/null +++ b/datasetUtils/getProposal_test.go @@ -0,0 +1,32 @@ +package datasetUtils + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +func TestGetProposal(t *testing.T) { + // Create a mock server + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + // Send response to be tested + rw.Write([]byte(`[{"proposal": "test proposal"}]`)) + })) + // Close the server when test finishes + defer server.Close() + + // Create a client + client := &http.Client{} + + // Create a user + user := make(map[string]string) + user["accessToken"] = "testToken" + + // Call GetProposal + proposal := GetProposal(client, server.URL, "testOwnerGroup", user, []string{"testAccessGroup"}) + + // Check the proposal + if proposal["proposal"] != "test proposal" { + t.Errorf("Expected proposal 'test proposal', got '%s'", proposal["proposal"]) + } +} From 34e37159516daef20faef10717948cf787aeb1cd Mon Sep 17 00:00:00 2001 From: "Ali R. Vahdati" Date: Mon, 13 May 2024 13:02:08 +0200 Subject: [PATCH 2/2] Refactor GetProposal Changes: 1. Instead of using log.Fatal which will terminate the program, the function now returns an error if something goes wrong. q. Used fmt.Sprintf for string concatenation as this is more readable and efficient when concatenating multiple strings. --- cmd/datasetGetProposal/main.go | 5 ++- datasetIngestor/checkMetadata.go | 5 ++- datasetUtils/getProposal.go | 54 +++++++++++++------------------- datasetUtils/getProposal_test.go | 2 +- 4 files changed, 30 insertions(+), 36 deletions(-) diff --git a/cmd/datasetGetProposal/main.go b/cmd/datasetGetProposal/main.go index 1991ea8..2077ad7 100644 --- a/cmd/datasetGetProposal/main.go +++ b/cmd/datasetGetProposal/main.go @@ -86,7 +86,10 @@ func main() { } user, accessGroups := datasetUtils.Authenticate(client, APIServer, token, userpass) - proposal := datasetUtils.GetProposal(client, APIServer, ownerGroup, user, accessGroups) + proposal, err := datasetUtils.GetProposal(client, APIServer, ownerGroup, user, accessGroups) + if err != nil { + log.Fatalf("Error: %v\n", err) + } // proposal is of type map[string]interface{} if len(proposal) > 0 { diff --git a/datasetIngestor/checkMetadata.go b/datasetIngestor/checkMetadata.go index d0d740e..cf57a0d 100644 --- a/datasetIngestor/checkMetadata.go +++ b/datasetIngestor/checkMetadata.go @@ -238,7 +238,10 @@ func augmentMissingMetadata(user map[string]string, metaDataMap map[string]inter if !ok { return fmt.Errorf("ownerGroup is not a string") } - proposal := datasetUtils.GetProposal(client, APIServer, ownerGroup, user, accessGroups) + proposal, err := datasetUtils.GetProposal(client, APIServer, ownerGroup, user, accessGroups) + if err != nil { + log.Fatalf("Error: %v\n", err) + } if val, ok := proposal["pi_email"]; ok { piEmail, ok := val.(string) if !ok { diff --git a/datasetUtils/getProposal.go b/datasetUtils/getProposal.go index 50eacad..e8fe72c 100644 --- a/datasetUtils/getProposal.go +++ b/datasetUtils/getProposal.go @@ -3,7 +3,7 @@ package datasetUtils import ( "encoding/json" "io" - "log" + "fmt" "net/http" ) @@ -25,51 +25,39 @@ Returns: - A map representing the proposal. If no proposal is found, an empty map is returned. */ func GetProposal(client *http.Client, APIServer string, ownerGroup string, user map[string]string, - accessGroups []string) (proposal map[string]interface{}) { - // Check if ownerGroup is in accessGroups list. No longer needed, done on server side and - // takes also accessGroup for beamline accounts into account - - // if user["displayName"] != "ingestor" { - // validOwner := false - // for _, b := range accessGroups { - // if b == ownerGroup { - // validOwner = true - // break - // } - // } - // if validOwner { - // log.Printf("OwnerGroup information %s verified successfully.\n", ownerGroup) - // } else { - // log.Fatalf("You are not member of the ownerGroup %s which is needed to access this data", ownerGroup) - // } - // } - - filter := `{"where":{"ownerGroup":"` + ownerGroup + `"}}` - url := APIServer + "/Proposals?access_token=" + user["accessToken"] - - // fmt.Printf("=== resulting filter:%s\n", filter) + accessGroups []string) (map[string]interface{}, error) { + + filter := fmt.Sprintf(`{"where":{"ownerGroup":"%s"}}`, ownerGroup) + url := fmt.Sprintf("%s/Proposals?access_token=%s", APIServer, user["accessToken"]) + req, err := http.NewRequest("GET", url, nil) + if err != nil { + return nil, err + } req.Header.Set("Content-Type", "application/json") req.Header.Set("filter", filter) - + resp, err := client.Do(req) if err != nil { - log.Fatal(err) + return nil, err } defer resp.Body.Close() - body, _ := io.ReadAll(resp.Body) - - // fmt.Printf("response Object:\n%v\n", string(body)) - + + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, err + } + var respObj []map[string]interface{} err = json.Unmarshal(body, &respObj) if err != nil { - log.Fatal(err) + return nil, err } - // the first element contains the actual map + respMap := make(map[string]interface{}) if len(respObj) > 0 { respMap = respObj[0] } - return respMap + + return respMap, nil } diff --git a/datasetUtils/getProposal_test.go b/datasetUtils/getProposal_test.go index 57945e5..e19f5c7 100644 --- a/datasetUtils/getProposal_test.go +++ b/datasetUtils/getProposal_test.go @@ -23,7 +23,7 @@ func TestGetProposal(t *testing.T) { user["accessToken"] = "testToken" // Call GetProposal - proposal := GetProposal(client, server.URL, "testOwnerGroup", user, []string{"testAccessGroup"}) + proposal, _ := GetProposal(client, server.URL, "testOwnerGroup", user, []string{"testAccessGroup"}) // Check the proposal if proposal["proposal"] != "test proposal" {