From 3ccda9c48aa0aef53caa33c999fba5668992e0f1 Mon Sep 17 00:00:00 2001 From: Ali Vahdati <3798865+kavir1698@users.noreply.github.com> Date: Wed, 22 May 2024 09:48:49 +0200 Subject: [PATCH] Test and refactor `GetProposal` (#68) * Add a unit test for `GetProposal` * 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 | 6 ++- datasetIngestor/checkMetadata.go | 5 ++- datasetUtils/getProposal.go | 69 +++++++++++++++++--------------- datasetUtils/getProposal_test.go | 32 +++++++++++++++ 4 files changed, 78 insertions(+), 34 deletions(-) create mode 100644 datasetUtils/getProposal_test.go diff --git a/cmd/datasetGetProposal/main.go b/cmd/datasetGetProposal/main.go index 98df8a9..4f82a0d 100644 --- a/cmd/datasetGetProposal/main.go +++ b/cmd/datasetGetProposal/main.go @@ -87,7 +87,11 @@ func main() { auth := &datasetUtils.RealAuthenticator{} user, accessGroups := datasetUtils.Authenticate(auth, 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 709b91f..47b8ccd 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 5a220b8..e8fe72c 100644 --- a/datasetUtils/getProposal.go +++ b/datasetUtils/getProposal.go @@ -2,57 +2,62 @@ package datasetUtils import ( "encoding/json" - "io/ioutil" - "log" + "io" + "fmt" "net/http" ) -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 +/* +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. - // 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) - // } - // } +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. - filter := `{"where":{"ownerGroup":"` + ownerGroup + `"}}` - url := APIServer + "/Proposals?access_token=" + user["accessToken"] +If the request or JSON unmarshalling fails, the function will log the error and terminate the program. - // fmt.Printf("=== resulting filter:%s\n", filter) +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) (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, _ := ioutil.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 new file mode 100644 index 0000000..e19f5c7 --- /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"]) + } +}