fix: refactor limit enforcement and fetching next page (#148)

Author: childish-sambino

client/page_util.go

@@ -2,11 +2,7 @@ package client
 
 import (
 	"encoding/json"
-	"errors"
 	"fmt"
-	"log"
-	"reflect"
-	"regexp"
 	"strings"
 )
 
@@ -29,91 +25,46 @@ func ReadLimits(pageSize *int, limit *int) int {
 	}
 }
 
-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)
+func GetNext(baseUrl string, response interface{}, getNextPage func(nextPageUri string) (interface{}, error)) (interface{}, error) {
+	nextPageUrl, err := getNextPageUrl(baseUrl, response)
 	if err != nil {
 		return nil, err
 	}
 
 	return getNextPage(nextPageUrl)
 }
 
-func GetPayload(baseUrl string, response interface{}) ([]interface{}, string, error) {
-	payload := toMap(response)
-	var data [][]interface{}
-	for _, v := range payload {
-		if v != nil {
-			kind := reflect.TypeOf(v).Kind()
-			switch kind {
-			//look for non metadata info
-			case reflect.Slice:
-				if len(data) > 0 {
-					//we expect this to be exactly 1
-					return nil, "", errors.New("payload contains more than 1 record of type array")
-				}
-				data = append(data, v.([]interface{}))
-			}
-		}
-	}
-
-	if len(data) == 1 {
-		return data[0], getNextPageUrl(baseUrl, payload), nil
-	}
-	return nil, "", errors.New("could not retrieve payload from response")
-}
-
-func toMap(s interface{}) map[string]interface{} {
+func toMap(s interface{}) (map[string]interface{}, error) {
 	var payload map[string]interface{}
-	test, errMarshal := json.Marshal(s)
-	if errMarshal != nil {
-		return nil
+	data, err := json.Marshal(s)
+	if err != nil {
+		return nil, err
 	}
 
-	errUnmarshal := json.Unmarshal(test, &payload)
-	if errUnmarshal != nil {
-		log.Print("Map creation error: ", errUnmarshal)
-		return nil
+	err = json.Unmarshal(data, &payload)
+	if err != nil {
+		return nil, err
 	}
-	return payload
+
+	return payload, err
 }
 
-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)
+func getNextPageUrl(baseUrl string, response interface{}) (string, error) {
+	payload, err := toMap(response)
 	if err != nil {
 		return "", err
 	}
 
-	*curRecord += len(payload)
-
-	if limit != nil {
-		//we have reached the desired limit
-		if *limit <= *curRecord {
-			return "", nil
-		}
-
-		remaining := *limit - *curRecord
-		if remaining > 0 {
-			pageSize := min(len(payload), remaining)
-			re := regexp.MustCompile(`PageSize=\d+`)
-			nextPageUrl = re.ReplaceAllString(nextPageUrl, fmt.Sprintf("PageSize=%d", pageSize))
-		}
-	}
-
-	return nextPageUrl, err
-}
-
-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)
+		return payload["meta"].(map[string]interface{})["next_page_url"].(string), nil
 	}
 
 	if payload != nil && payload["next_page_uri"] != nil {
 		// remove any leading and trailing '/'
-		return fmt.Sprintf("%s/%s", strings.Trim(baseUrl, "/"), strings.Trim(payload["next_page_uri"].(string), "/"))
+		return fmt.Sprintf("%s/%s", strings.Trim(baseUrl, "/"), strings.Trim(payload["next_page_uri"].(string), "/")), nil
 	}
 
-	return ""
+	return "", nil
 }
 
 func min(a int, b int) int {

client/page_util_test.go

@@ -35,16 +35,19 @@ func TestPageUtil_GetNextPageUri(t *testing.T) {
 		"page_size":     50,
 	}
 	baseUrl := "https://api.twilio.com/"
-	nextPageUrl := getNextPageUrl(baseUrl, payload)
+	nextPageUrl, err := getNextPageUrl(baseUrl, payload)
+	assert.Nil(t, err)
 	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)
+	nextPageUrl, err = getNextPageUrl(baseUrl, payload)
+	assert.Nil(t, err)
 	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)
+	nextPageUrl, err = getNextPageUrl(baseUrl, payload)
+	assert.Nil(t, err)
 	assert.Equal(t, "", nextPageUrl)
 }
 
@@ -56,7 +59,8 @@ func TestPageUtil_GetNextPageUrl(t *testing.T) {
 		},
 	}
 
-	nextPageUrl := getNextPageUrl("https://apitest.twilio.com", payload)
+	nextPageUrl, err := getNextPageUrl("https://apitest.twilio.com", payload)
+	assert.Nil(t, err)
 	assert.Equal(t, "https://api.twilio.com/2010-04-01/Accounts/ACXX/IncomingPhoneNumbers.json?PageSize=50&Page=1", nextPageUrl)
 }
 
@@ -147,27 +151,18 @@ func TestPageUtil_GetNext(t *testing.T) {
 	ps := &testResponse{}
 	_ = json.NewDecoder(response.Body).Decode(ps)
 
-	curRecord := 0
-	limit := 10
-
-	nextPageUrl, err := GetNext(baseUrl, ps, &curRecord, &limit, getSomething)
+	nextPageUrl, err := GetNext(baseUrl, ps, 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)
+	nextPageUrl, err = GetNext(baseUrl, nil, 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")
+	testMap, err := toMap("invalid")
+	assert.NotNil(t, err)
 	assert.Nil(t, testMap)
 
 	valid := testResponse{
@@ -181,42 +176,7 @@ func TestPageUtil_ToMap(t *testing.T) {
 		Start:           0,
 		Uri:             "uri",
 	}
-	testMap = toMap(valid)
+	testMap, err = toMap(valid)
+	assert.Nil(t, err)
 	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())
-}