1
\$\begingroup\$

I'm new to Go from another language and I'm most interested to know if this implementation has anything that's not idiomatic Go, but any feedback is welcome and you're not going to hurt my feelings by being direct.

The API that I wrote the client for has a pagination header whose key is "Link" and whose value is one long string like

<https://domain.com/api/v1/things?page=7&per_page=10>; rel="current",<https://domain.com/api/v1/things?page=8&per_page=10>; rel="next",<https://domain.com/api/v1/things?page=6&per_page=10>; rel="prev",<https://domain.com/api/v1/things?page=1&per_page=10>; rel="first",<https://domain.com/api/v1/things?page=8&per_page=10>; rel="last"

The "next" URL needs to be extracted from that header value string, as that provides the URL for the next page of data.

The client needs to make an initial request to ?page=1&per_page=10 (which is the default if no query is provided), then follow the headers until all the data has been fetched.

To get just one page of data and the "next" URL, I have the following:

type ResponseData struct {
    body    []byte
    nextUrl string
}

func DoReq(url, authToken string) (*ResponseData, error) {
    httpRequest, err := http.NewRequest(http.MethodGet, url, nil)
    if err != nil {
        return nil, err
    }

    httpRequest.Header.Add("Authorization", "Bearer "+authToken)

    httpResponse, err := http.DefaultClient.Do(httpRequest)
    if err != nil {
        return nil, err
    }
    defer httpResponse.Body.Close()

    body, err := io.ReadAll(httpResponse.Body)
    if err != nil {
        return nil, err
    }

    if !status2xx(httpResponse.StatusCode) {
        return nil, fmt.Errorf("Status %d %s %s", httpResponse.StatusCode, httpRequest.URL, body)
    }

    resp := &ResponseData{body: body}

    if linkHeaderValue := httpResponse.Header.Get("Link"); "" != linkHeaderValue {
        resp.nextUrl = getNextUrl(linkHeaderValue)
    }

    return resp, nil
}


func status2xx(statusCode int) bool {
    return statusCode >= 200 && statusCode < 300
}

func getNextUrl(linkHeaderVal string) string {
    links := strings.Split(linkHeaderVal, ",")

    for i := range links {
        if strings.HasSuffix(links[i], `rel="next"`) {
            return strings.Trim(strings.Split(links[i], "; ")[0], "<>")
        }
    }

    return ""
}

To get the data for all pages I have:

func GetAllResponseData(initialUrl, authToken string) ([][]byte, error) {
    var allData [][]byte

    url := initialUrl

    for "" != url {
         // DoReq returns a ResponseData{body []byte, nextUrl string}
        resp, err := DoReq(url, authToken)
        if err != nil {
            return nil, err
        }

        allData = append(allData, resp.body)

        url = resp.nextUrl
    }

    return allData, nil
}
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

I don't see anything that is obviously outside the bounds of "idiomatic", but there might be a few things to improve depending on what you're optimizing for.

If you're looking to boost performance, then I'd definitely look at increasing the page size if possible. Network requests can (relatively) take a lot of time, so increasing the page size should result in fewer requests and better throughput.

Also, it looks like you're storing all response bodies in one big slice of []byte, which could create a lot of memory pressure. This might be due to requirements in other areas of your code, but it might also be a good place to explore concurrent solutions. A channel may be a good idea to process responses as they're received, and it also allows consuming logic to apply back-pressure on the producer in case processing those responses can take a lot of time. Something like this might be helpful.

// ConsumerData is used to carry either a []byte response payload,
// or an error that occurred during processing.
type ConsumerData struct {
    Data []byte
    Err error
}

// GetAllResponseData can be run in a separate goroutine to introduce concurrent processing.
// Using a context for cancellation would be really nice too.
func GetAllResponseData(initialUrl, authToken string, responses chan <-[]*ConsumerData) {
    // Should always close channels on the sending side.
    defer close(responses)
    url := initialUrl

    for "" != url {
        // DoReq returns a ResponseData{body []byte, nextUrl string}
        resp, err := DoReq(url, authToken)
        if err != nil {
            responses <- &ConsumerData{Err: err}
            return
        }

        responses <- &ConsumerData{Data: resp.body}

        url = resp.nextUrl
    }
}

Another implementation that uses a context.

func GetAllResponseDataCtx(ctx context.Context, initialUrl, authToken string, responses chan <-[]*ConsumerData) {
    // Should always close channels on the sending side.
    defer close(responses)
    url := initialUrl

    for "" != url {
        select {
            case <-ctx.Done():
                responses <- &ConsumerData{Err: ctx.Err()}
                return
            default:
                // DoReq returns a ResponseData{body []byte, nextUrl string}
                resp, err := DoReq(url, authToken)
                if err != nil {
                    responses <- &ConsumerData{Err: err}
                    return
                }

                responses <- &ConsumerData{Data: resp.body}

                url = resp.nextUrl
        }
    }
}
```
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.