fix: pagination with next_page_url (#110)

* fix: pagination with next_page_urls

* add more unit tests

Author: shwetha-manvinkurke

client/mock_client.go

@@ -7,6 +7,7 @@ import (
 	"log"
 	"reflect"
 	"regexp"
+	"strings"
 )
 
 //Takes a limit on the max number of records to read and a max pageSize and calculates the max number of pages to read.
@@ -28,16 +29,16 @@ func ReadLimits(pageSize *int, limit *int) int {
 	}
 }
 
-func GetNext(response interface{}, curRecord *int, limit *int, getNextPage func(nextPageUri string) (interface{}, error)) (interface{}, error) {
-	nextPageUri, err := getNextPageUri(response, curRecord, limit)
+func GetNext(baseUrl string, response interface{}, curRecord *int, limit *int, getNextPage func(nextPageUri string) (interface{}, error)) (interface{}, error) {
+	nextPageUrl, err := getNextPageAddress(baseUrl, response, curRecord, limit)
 	if err != nil {
 		return nil, err
 	}
 
-	return getNextPage(nextPageUri)
+	return getNextPage(nextPageUrl)
 }
 
-func GetPayload(response interface{}) ([]interface{}, string, error) {
+func GetPayload(baseUrl string, response interface{}) ([]interface{}, string, error) {
 	payload := toMap(response)
 	var data [][]interface{}
 	for _, v := range payload {
@@ -56,25 +57,29 @@ func GetPayload(response interface{}) ([]interface{}, string, error) {
 	}
 
 	if len(data) == 1 {
-		return data[0], getNextPageUrl(payload), nil
+		return data[0], getNextPageUrl(baseUrl, payload), nil
 	}
 	return nil, "", errors.New("could not retrieve payload from response")
 }
 
 func toMap(s interface{}) map[string]interface{} {
 	var payload map[string]interface{}
-	test, err := json.Marshal(s)
-	if err != nil {
-		log.Print("Map creation error: ", err)
+	test, errMarshal := json.Marshal(s)
+	if errMarshal != nil {
+		return nil
+	}
+
+	errUnmarshal := json.Unmarshal(test, &payload)
+	if errUnmarshal != nil {
+		log.Print("Map creation error: ", errUnmarshal)
 		return nil
 	}
-	_ = json.Unmarshal(test, &payload)
 	return payload
 }
 
-func getNextPageUri(response interface{}, curRecord *int, limit *int) (string, error) {
-	//get just the non metadata info and the next page uri
-	payload, nextPageUri, err := GetPayload(response)
+func getNextPageAddress(baseUrl string, response interface{}, curRecord *int, limit *int) (string, error) {
+	//get just the non metadata info and the next page url
+	payload, nextPageUrl, err := GetPayload(baseUrl, response)
 	if err != nil {
 		return "", err
 	}
@@ -91,20 +96,21 @@ func getNextPageUri(response interface{}, curRecord *int, limit *int) (string, e
 		if remaining > 0 {
 			pageSize := min(len(payload), remaining)
 			re := regexp.MustCompile(`PageSize=\d+`)
-			nextPageUri = re.ReplaceAllString(nextPageUri, fmt.Sprintf("PageSize=%d", pageSize))
+			nextPageUrl = re.ReplaceAllString(nextPageUrl, fmt.Sprintf("PageSize=%d", pageSize))
 		}
 	}
 
-	return nextPageUri, err
+	return nextPageUrl, err
 }
 
-func getNextPageUrl(payload map[string]interface{}) string {
+func getNextPageUrl(baseUrl string, payload map[string]interface{}) string {
 	if payload != nil && payload["meta"] != nil && payload["meta"].(map[string]interface{})["next_page_url"] != nil {
 		return payload["meta"].(map[string]interface{})["next_page_url"].(string)
 	}
 
 	if payload != nil && payload["next_page_uri"] != nil {
-		return payload["next_page_uri"].(string)
+		// remove any leading and trailing '/'
+		return fmt.Sprintf("%s/%s", strings.Trim(baseUrl, "/"), strings.Trim(payload["next_page_uri"].(string), "/"))
 	}
 
 	return ""

client/page_util.go

@@ -1,12 +1,19 @@
 package client
 
 import (
+	"bytes"
+	"encoding/json"
+	"io/ioutil"
+	"net/http"
+	"net/url"
 	"testing"
 
+	"github.com/golang/mock/gomock"
+
 	"github.com/stretchr/testify/assert"
 )
 
-func TestReadLimits(t *testing.T) {
+func TestPageUtil_ReadLimits(t *testing.T) {
 	assert.Equal(t, 5, ReadLimits(nil, setLimit(5)))
 	assert.Equal(t, 5, ReadLimits(setPageSize(10), setLimit(5)))
 	assert.Equal(t, 1000, ReadLimits(nil, setLimit(5000)))
@@ -21,3 +28,195 @@ func setLimit(limit int) *int {
 func setPageSize(pageSize int) *int {
 	return &pageSize
 }
+
+func TestPageUtil_GetNextPageUri(t *testing.T) {
+	payload := map[string]interface{}{
+		"next_page_uri": "/2010-04-01/Accounts/ACXX/IncomingPhoneNumbers.json?PageSize=50&Page=1",
+		"page_size":     50,
+	}
+	baseUrl := "https://api.twilio.com/"
+	nextPageUrl := getNextPageUrl(baseUrl, payload)
+	assert.Equal(t, "https://api.twilio.com/2010-04-01/Accounts/ACXX/IncomingPhoneNumbers.json?PageSize=50&Page=1", nextPageUrl)
+
+	payload["next_page_uri"] = "2010-04-01/Accounts/ACXX/IncomingPhoneNumbers.json?PageSize=50&Page=1"
+	baseUrl = "https://api.twilio.com"
+	nextPageUrl = getNextPageUrl(baseUrl, payload)
+	assert.Equal(t, "https://api.twilio.com/2010-04-01/Accounts/ACXX/IncomingPhoneNumbers.json?PageSize=50&Page=1", nextPageUrl)
+
+	payload = map[string]interface{}{}
+	nextPageUrl = getNextPageUrl(baseUrl, payload)
+	assert.Equal(t, "", nextPageUrl)
+}
+
+func TestPageUtil_GetNextPageUrl(t *testing.T) {
+	payload := map[string]interface{}{
+		"meta": map[string]interface{}{
+			"next_page_url": "https://api.twilio.com/2010-04-01/Accounts/ACXX/IncomingPhoneNumbers.json?PageSize=50&Page=1",
+			"page_size":     50,
+		},
+	}
+
+	nextPageUrl := getNextPageUrl("https://apitest.twilio.com", payload)
+	assert.Equal(t, "https://api.twilio.com/2010-04-01/Accounts/ACXX/IncomingPhoneNumbers.json?PageSize=50&Page=1", nextPageUrl)
+}
+
+func getTestClient(t *testing.T) *MockBaseClient {
+	mockCtrl := gomock.NewController(t)
+	testClient := NewMockBaseClient(mockCtrl)
+	testClient.EXPECT().AccountSid().DoAndReturn(func() string {
+		return "AC222222222222222222222222222222"
+	}).AnyTimes()
+
+	testClient.EXPECT().SendRequest(
+		gomock.Any(),
+		gomock.Any(),
+		gomock.Any(),
+		gomock.Any()).
+		DoAndReturn(func(method string, rawURL string, data url.Values,
+			headers map[string]interface{}) (*http.Response, error) {
+			response := map[string]interface{}{
+				"end":            4,
+				"first_page_uri": "/2010-04-01/Accounts/ACXX/Messages.json?From=9999999999&PageNumber=&To=4444444444&PageSize=2&Page=0",
+				"messages": []map[string]interface{}{
+					{
+						"direction": "outbound-api",
+						"from":      "4444444444",
+						"to":        "9999999999",
+						"body":      "Message 0",
+						"status":    "delivered",
+					},
+					{
+						"direction": "outbound-api",
+						"from":      "4444444444",
+						"to":        "9999999999",
+						"body":      "Message 1",
+						"status":    "delivered",
+					},
+				},
+				"uri":           "/2010-04-01/Accounts/ACXX/Messages.json?From=9999999999&PageNumber=&To=4444444444&PageSize=2&Page=0&PageToken=dummy",
+				"page_size":     5,
+				"start":         0,
+				"next_page_uri": "/2010-04-01/Accounts/ACXX/Messages.json?From=9999999999&PageNumber=&To=4444444444&PageSize=2&Page=1&PageToken=PASMXX",
+				"page":          0,
+			}
+
+			resp, _ := json.Marshal(response)
+
+			return &http.Response{
+				Body: ioutil.NopCloser(bytes.NewReader(resp)),
+			}, nil
+		},
+		)
+
+	return testClient
+}
+
+type testResponse struct {
+	End             int           `json:"end,omitempty"`
+	FirstPageUri    string        `json:"first_page_uri,omitempty"`
+	Messages        []testMessage `json:"messages,omitempty"`
+	NextPageUri     string        `json:"next_page_uri,omitempty"`
+	Page            int           `json:"page,omitempty"`
+	PageSize        int           `json:"page_size,omitempty"`
+	PreviousPageUri string        `json:"previous_page_uri,omitempty"`
+	Start           int           `json:"start,omitempty"`
+	Uri             string        `json:"uri,omitempty"`
+}
+
+type testMessage struct {
+	// The message text
+	Body *string `json:"body,omitempty"`
+	// The direction of the message
+	Direction *string `json:"direction,omitempty"`
+	// The phone number that initiated the message
+	From *string `json:"from,omitempty"`
+	// The status of the message
+	Status *string `json:"status,omitempty"`
+	// The phone number that received the message
+	To *string `json:"to,omitempty"`
+}
+
+func getSomething(nextPageUrl string) (interface{}, error) {
+	return nextPageUrl, nil
+}
+
+func TestPageUtil_GetNext(t *testing.T) {
+	testClient := getTestClient(t)
+	baseUrl := "https://api.twilio.com"
+	response, _ := testClient.SendRequest("get", "", nil, nil) //nolint:bodyclose
+	ps := &testResponse{}
+	_ = json.NewDecoder(response.Body).Decode(ps)
+
+	curRecord := 0
+	limit := 10
+
+	nextPageUrl, err := GetNext(baseUrl, ps, &curRecord, &limit, getSomething)
+	assert.Equal(t, "https://api.twilio.com/2010-04-01/Accounts/ACXX/Messages.json?From=9999999999&PageNumber=&To=4444444444&PageSize=2&Page=1&PageToken=PASMXX", nextPageUrl)
+	assert.Nil(t, err)
+
+	curRecord = 15
+	nextPageUrl, err = GetNext(baseUrl, ps, &curRecord, &limit, getSomething)
+	assert.Empty(t, nextPageUrl)
+	assert.Nil(t, err)
+}
+
+func TestPageUtil_GetNextWithErr(t *testing.T) {
+	nextPageUrl, err := GetNext("baseUrl", nil, nil, nil, getSomething)
+	assert.Nil(t, nextPageUrl)
+	assert.NotNil(t, err)
+}
+
+func TestPageUtil_ToMap(t *testing.T) {
+	testMap := toMap("invalid")
+	assert.Nil(t, testMap)
+
+	valid := testResponse{
+		End:             0,
+		FirstPageUri:    "first",
+		Messages:        nil,
+		NextPageUri:     "next",
+		Page:            0,
+		PageSize:        0,
+		PreviousPageUri: "previous",
+		Start:           0,
+		Uri:             "uri",
+	}
+	testMap = toMap(valid)
+	assert.NotNil(t, testMap)
+}
+
+func TestPageUtil_GetNextPageAddress(t *testing.T) {
+	nextPageAddress, _ := getNextPageAddress("baseUrl", nil, nil, nil)
+	assert.Empty(t, nextPageAddress)
+}
+
+func TestPageUtil_GetPayload(t *testing.T) {
+	response := map[string]interface{}{
+		"end":            4,
+		"first_page_uri": "/2010-04-01/Accounts/ACXX/Messages.json?From=9999999999&PageNumber=&To=4444444444&PageSize=2&Page=0",
+		"messages": []map[string]interface{}{
+			{
+				"direction": "outbound-api",
+				"from":      "4444444444",
+				"to":        "9999999999",
+				"body":      "Message 0",
+				"status":    "delivered",
+			},
+		},
+		"messages2": []map[string]interface{}{
+			{
+				"direction": "outbound-api",
+				"from":      "4444444444",
+				"to":        "9999999999",
+				"body":      "Message 0",
+				"status":    "delivered",
+			},
+		},
+	}
+
+	payload, nextPageUrl, err := GetPayload("baseUrl", response)
+	assert.Nil(t, payload)
+	assert.Empty(t, nextPageUrl)
+	assert.NotNil(t, err)
+	assert.Equal(t, "payload contains more than 1 record of type array", err.Error())
+}