safespring-community / cloutility-api-client

https://www.safespring.com/en/services/backup/
ISC License
0 stars 5 forks source link

Use URL library to properly parse URLs #18

Closed oquinena closed 1 year ago

oquinena commented 1 year ago

The current implementation concatinates strings to be used as urls which may lead to errors. The URL library should be used to parse urls instead.

Arghya721 commented 1 year ago

Hi @oquinena

I guess you are telling about these implementations.

endpoint := c.BaseURL + "/v1/bunits?bunitId=" + strconv.Itoa(bunitID)

oquinena commented 1 year ago

Yes, exactly. If, for instance, the baseurl contains a trailing /, this will cause the request to fail.

Let me know if you want to give it a go, and I'll assign you the issue🙂

Arghya721 commented 1 year ago

We can implement something like this.

path:= "/v1/bunits/" + strconv.Itoa(bUnitID) + "/defaultserver/clientoptionsets" baseURL, err := url.Parse(c.BaseURL) if err != nil { // handle error } baseURL.Path = path endpoint := baseURL.String()

I will try to open a pr with the necessary changes.

oquinena commented 1 year ago

Looks good, but I'd keep it even more simple and just pass the stringified url to the apiRequest method. Kinda like this:

        s := c.BaseURL + "/v1/me"
    endpoint, err := url.Parse(s)
    if err != nil {
        return nil, fmt.Errorf("error parsing url: %s", err)
    }
    body, err := c.apiRequest(endpoint.String(), http.MethodGet, nil)

Anyways, I'm assigning you the issue 👍🏽

Arghya721 commented 1 year ago

Hi @oquinena I have made the necessary changes. Please inform me if there is any issue with the implementation.