[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5896: Add TO Client API for Cachegroup Parameters Automation

[GitBox <gi...@apache.org>]

ocket8888 commented on a change in pull request #5896:
URL: https://github.com/apache/trafficcontrol/pull/5896#discussion_r643513866



##########
File path: traffic_ops/testing/api/v4/cachegroups_parameters_test.go
##########
@@ -79,6 +90,114 @@ func CreateTestCacheGroupParameters(t *testing.T) {
 	testData.CacheGroupParameterRequests = append(testData.CacheGroupParameterRequests, resp.Response...)
 }
 
+func CreateTestCacheGroupParametersAlreadyExist(t *testing.T) {
+
+	resp, _, err := TOSession.GetAllCacheGroupParameters(client.RequestOptions{})
+	cachegroupParameters := resp.Response
+	getAllcgparameters := cachegroupParameters.CacheGroupParameters
+	parameterID := *getAllcgparameters[0].Parameter

Review comment:
       There are two possible segfaults on this line, `*` and `[0]`

##########
File path: traffic_ops/testing/api/v4/cachegroups_parameters_test.go
##########
@@ -79,6 +90,114 @@ func CreateTestCacheGroupParameters(t *testing.T) {
 	testData.CacheGroupParameterRequests = append(testData.CacheGroupParameterRequests, resp.Response...)
 }
 
+func CreateTestCacheGroupParametersAlreadyExist(t *testing.T) {
+
+	resp, _, err := TOSession.GetAllCacheGroupParameters(client.RequestOptions{})
+	cachegroupParameters := resp.Response
+	getAllcgparameters := cachegroupParameters.CacheGroupParameters
+	parameterID := *getAllcgparameters[0].Parameter
+
+	opts := client.NewRequestOptions()
+	opts.QueryParameters.Set("name", *getAllcgparameters[0].CacheGroup)
+
+	cgResponse, reqInf, err := TOSession.GetCacheGroups(opts)
+	cg := cgResponse.Response[0]

Review comment:
       this will segfault if Traffic Ops returns an empty response

##########
File path: traffic_ops/testing/api/v4/cachegroups_parameters_test.go
##########
@@ -155,3 +274,201 @@ func DeleteTestCacheGroupParameter(t *testing.T, cgp tc.CacheGroupParameterReque
 		t.Error("expected error when deleting cache group parameter with non existing parameter")
 	}
 }
+
+func GetTestPaginationSupportCgParameters(t *testing.T) {
+	opts := client.NewRequestOptions()
+	opts.QueryParameters.Set("orderby", "id")
+	resp, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters := resp.Response
+	if len(cachegroupParameters.CacheGroupParameters) < 3 {
+		t.Fatalf("Need at least 3 cachegroup parameters in Traffic Ops to test pagination support, found: %d", len(cachegroupParameters.CacheGroupParameters))
+	}
+
+	opts.QueryParameters.Set("orderby", "id")
+	opts.QueryParameters.Set("limit", "1")
+	respWithLimit, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters with limits: %v - alerts: %+v", err, respWithLimit.Alerts)
+	}
+	cachegroupParametersWithLimit := respWithLimit.Response
+	if !reflect.DeepEqual(cachegroupParameters.CacheGroupParameters[:1], cachegroupParametersWithLimit.CacheGroupParameters) {
+		t.Error("expected GET cachegroup parameters with limit = 1 to return first result")
+	}
+
+	opts.QueryParameters.Set("orderby", "id")
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("offset", "1")
+	respWithOffset, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters with offset: %v - alerts: %+v", err, respWithOffset.Alerts)
+	}
+	cachegroupParametersWithOffset := respWithOffset.Response
+	if !reflect.DeepEqual(cachegroupParameters.CacheGroupParameters[1:2], cachegroupParametersWithOffset.CacheGroupParameters) {
+		t.Error("expected GET cachegroup parameters with limit = 1, offset = 1 to return second result")
+	}
+
+	opts.QueryParameters.Set("orderby", "id")
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("page", "2")
+	respWithPage, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters with page: %v - alerts: %+v", err, respWithPage.Alerts)
+	}
+	cachegroupParametersWithPage := respWithPage.Response
+	if !reflect.DeepEqual(cachegroupParameters.CacheGroupParameters[1:2], cachegroupParametersWithPage.CacheGroupParameters) {
+		t.Error("expected GET cachegroup parameters with limit = 1, page = 2 to return second result")
+	}
+
+	opts.QueryParameters = url.Values{}
+	opts.QueryParameters.Set("limit", "-2")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if !alertsHaveError(resp.Alerts.Alerts, "limit parameter must be bigger than -1") {
+		t.Errorf("expected GET cachegroup parameters to return an error for limit is not bigger than -1, actual error: %v - alerts: %+v", err, resp.Alerts)
+	}
+
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("offset", "0")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if !alertsHaveError(resp.Alerts.Alerts, "offset parameter must be a positive integer") {
+		t.Errorf("expected GET cachegroup parameters to return an error for offset is not a positive integer, actual error: %v - alerts: %+v", err, resp.Alerts)
+	}
+
+	opts.QueryParameters = url.Values{}
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("page", "0")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if !alertsHaveError(resp.Alerts.Alerts, "page parameter must be a positive integer") {
+		t.Errorf("expected GET cachegroup parameters to return an error for page is not a positive integer, actual error: %v - alerts: %+v", err, resp.Alerts)
+	}
+}
+
+func SortTestCacheGroupParameters(t *testing.T) {
+	resp, _, err := TOSession.GetAllCacheGroupParameters(client.RequestOptions{})
+	if err != nil {
+		t.Fatalf("Expected no error, but got: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters := resp.Response
+	sortedList := make([]string, 0, len(cachegroupParameters.CacheGroupParameters))
+	for _, cgparameters := range cachegroupParameters.CacheGroupParameters {
+		sortedList = append(sortedList, strconv.Itoa(*cgparameters.Parameter))
+	}
+
+	res := sort.SliceIsSorted(sortedList, func(p, q int) bool {
+		return sortedList[p] < sortedList[q]
+	})
+	if !res {
+		t.Errorf("list is not sorted by their ID: %v", sortedList)
+	}
+}
+
+func SortTestCacheGroupParametersDesc(t *testing.T) {
+	resp, _, err := TOSession.GetAllCacheGroupParameters(client.RequestOptions{})
+	if err != nil {
+		t.Errorf("Expected no error, but got error in CacheGroup Parameters with default ordering: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters := resp.Response
+	respAsc := cachegroupParameters.CacheGroupParameters
+	if len(respAsc) < 1 {
+		t.Fatal("Need at least one CacheGroup Parameters in Traffic Ops to test CacheGroup Parameters sort ordering")
+	}
+
+	opts := client.NewRequestOptions()
+	opts.QueryParameters.Set("sortOrder", "desc")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Errorf("Expected no error, but got error in CacheGroup Parameters with Descending ordering: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters = resp.Response
+	respDesc := cachegroupParameters.CacheGroupParameters
+	if len(respDesc) < 1 {
+		t.Fatal("Need at least one CacheGroup Parameters in Traffic Ops to test CacheGroup Parameters sort ordering")
+	}
+
+	if len(respAsc) != len(respDesc) {
+		t.Fatalf("Traffic Ops returned %d CacheGroup Parameters using default sort order, but %d CacheGroup Parameters when sort order was explicitly set to descending", len(respAsc), len(respDesc))
+	}
+
+	// reverse the descending-sorted response and compare it to the ascending-sorted one
+	// TODO ensure at least two in each slice? A list of length one is
+	// trivially sorted both ascending and descending.
+	for start, end := 0, len(respDesc)-1; start < end; start, end = start+1, end-1 {
+		respDesc[start], respDesc[end] = respDesc[end], respDesc[start]
+	}
+	if *respDesc[0].Parameter != *respAsc[0].Parameter {
+		t.Errorf("CacheGroup Parameters responses are not equal after reversal: Asc: %v - Desc: %v", *respDesc[0].Parameter, *respAsc[0].Parameter)
+	}
+}
+
+func CreateTestCacheGroupParametersWithInvalidData(t *testing.T) {
+	resp, _, err := TOSession.GetParameters(client.RequestOptions{})

Review comment:
       The value of this error should be checked if it's being stored (really I think they should just always check it)

##########
File path: traffic_ops/testing/api/v4/cachegroups_parameters_test.go
##########
@@ -155,3 +274,201 @@ func DeleteTestCacheGroupParameter(t *testing.T, cgp tc.CacheGroupParameterReque
 		t.Error("expected error when deleting cache group parameter with non existing parameter")
 	}
 }
+
+func GetTestPaginationSupportCgParameters(t *testing.T) {
+	opts := client.NewRequestOptions()
+	opts.QueryParameters.Set("orderby", "id")
+	resp, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters := resp.Response
+	if len(cachegroupParameters.CacheGroupParameters) < 3 {
+		t.Fatalf("Need at least 3 cachegroup parameters in Traffic Ops to test pagination support, found: %d", len(cachegroupParameters.CacheGroupParameters))
+	}
+
+	opts.QueryParameters.Set("orderby", "id")
+	opts.QueryParameters.Set("limit", "1")
+	respWithLimit, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters with limits: %v - alerts: %+v", err, respWithLimit.Alerts)
+	}
+	cachegroupParametersWithLimit := respWithLimit.Response
+	if !reflect.DeepEqual(cachegroupParameters.CacheGroupParameters[:1], cachegroupParametersWithLimit.CacheGroupParameters) {
+		t.Error("expected GET cachegroup parameters with limit = 1 to return first result")
+	}
+
+	opts.QueryParameters.Set("orderby", "id")
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("offset", "1")
+	respWithOffset, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters with offset: %v - alerts: %+v", err, respWithOffset.Alerts)
+	}
+	cachegroupParametersWithOffset := respWithOffset.Response
+	if !reflect.DeepEqual(cachegroupParameters.CacheGroupParameters[1:2], cachegroupParametersWithOffset.CacheGroupParameters) {
+		t.Error("expected GET cachegroup parameters with limit = 1, offset = 1 to return second result")
+	}
+
+	opts.QueryParameters.Set("orderby", "id")
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("page", "2")
+	respWithPage, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters with page: %v - alerts: %+v", err, respWithPage.Alerts)
+	}
+	cachegroupParametersWithPage := respWithPage.Response
+	if !reflect.DeepEqual(cachegroupParameters.CacheGroupParameters[1:2], cachegroupParametersWithPage.CacheGroupParameters) {
+		t.Error("expected GET cachegroup parameters with limit = 1, page = 2 to return second result")
+	}
+
+	opts.QueryParameters = url.Values{}
+	opts.QueryParameters.Set("limit", "-2")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if !alertsHaveError(resp.Alerts.Alerts, "limit parameter must be bigger than -1") {
+		t.Errorf("expected GET cachegroup parameters to return an error for limit is not bigger than -1, actual error: %v - alerts: %+v", err, resp.Alerts)
+	}
+
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("offset", "0")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if !alertsHaveError(resp.Alerts.Alerts, "offset parameter must be a positive integer") {
+		t.Errorf("expected GET cachegroup parameters to return an error for offset is not a positive integer, actual error: %v - alerts: %+v", err, resp.Alerts)
+	}
+
+	opts.QueryParameters = url.Values{}
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("page", "0")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if !alertsHaveError(resp.Alerts.Alerts, "page parameter must be a positive integer") {
+		t.Errorf("expected GET cachegroup parameters to return an error for page is not a positive integer, actual error: %v - alerts: %+v", err, resp.Alerts)
+	}
+}
+
+func SortTestCacheGroupParameters(t *testing.T) {
+	resp, _, err := TOSession.GetAllCacheGroupParameters(client.RequestOptions{})
+	if err != nil {
+		t.Fatalf("Expected no error, but got: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters := resp.Response
+	sortedList := make([]string, 0, len(cachegroupParameters.CacheGroupParameters))
+	for _, cgparameters := range cachegroupParameters.CacheGroupParameters {
+		sortedList = append(sortedList, strconv.Itoa(*cgparameters.Parameter))
+	}
+
+	res := sort.SliceIsSorted(sortedList, func(p, q int) bool {
+		return sortedList[p] < sortedList[q]
+	})
+	if !res {
+		t.Errorf("list is not sorted by their ID: %v", sortedList)
+	}
+}
+
+func SortTestCacheGroupParametersDesc(t *testing.T) {
+	resp, _, err := TOSession.GetAllCacheGroupParameters(client.RequestOptions{})
+	if err != nil {
+		t.Errorf("Expected no error, but got error in CacheGroup Parameters with default ordering: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters := resp.Response
+	respAsc := cachegroupParameters.CacheGroupParameters
+	if len(respAsc) < 1 {
+		t.Fatal("Need at least one CacheGroup Parameters in Traffic Ops to test CacheGroup Parameters sort ordering")
+	}
+
+	opts := client.NewRequestOptions()
+	opts.QueryParameters.Set("sortOrder", "desc")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Errorf("Expected no error, but got error in CacheGroup Parameters with Descending ordering: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters = resp.Response
+	respDesc := cachegroupParameters.CacheGroupParameters
+	if len(respDesc) < 1 {
+		t.Fatal("Need at least one CacheGroup Parameters in Traffic Ops to test CacheGroup Parameters sort ordering")
+	}
+
+	if len(respAsc) != len(respDesc) {
+		t.Fatalf("Traffic Ops returned %d CacheGroup Parameters using default sort order, but %d CacheGroup Parameters when sort order was explicitly set to descending", len(respAsc), len(respDesc))
+	}
+
+	// reverse the descending-sorted response and compare it to the ascending-sorted one
+	// TODO ensure at least two in each slice? A list of length one is
+	// trivially sorted both ascending and descending.
+	for start, end := 0, len(respDesc)-1; start < end; start, end = start+1, end-1 {
+		respDesc[start], respDesc[end] = respDesc[end], respDesc[start]
+	}
+	if *respDesc[0].Parameter != *respAsc[0].Parameter {
+		t.Errorf("CacheGroup Parameters responses are not equal after reversal: Asc: %v - Desc: %v", *respDesc[0].Parameter, *respAsc[0].Parameter)
+	}
+}
+
+func CreateTestCacheGroupParametersWithInvalidData(t *testing.T) {
+	resp, _, err := TOSession.GetParameters(client.RequestOptions{})
+	parametersResp := resp.Response[0]

Review comment:
       this will segfault if the response was null, undefined, or empty

##########
File path: traffic_ops/v4-client/cachegroup_parameters.go
##########
@@ -58,3 +58,10 @@ func (to *Session) CreateCacheGroupParameter(cacheGroupID, parameterID int, opts
 	reqInf, err := to.post(apiCachegroupParameters, opts, cacheGroupParameterReq, &data)
 	return data, reqInf, err
 }
+
+// CreateCacheGroupParameter associates a Parameter with a Cache Group.

Review comment:
       I believe this actually associates multiple Cache Groups with multiple Parameters

##########
File path: traffic_ops/testing/api/v4/cachegroups_parameters_test.go
##########
@@ -79,6 +90,114 @@ func CreateTestCacheGroupParameters(t *testing.T) {
 	testData.CacheGroupParameterRequests = append(testData.CacheGroupParameterRequests, resp.Response...)
 }
 
+func CreateTestCacheGroupParametersAlreadyExist(t *testing.T) {
+
+	resp, _, err := TOSession.GetAllCacheGroupParameters(client.RequestOptions{})
+	cachegroupParameters := resp.Response
+	getAllcgparameters := cachegroupParameters.CacheGroupParameters
+	parameterID := *getAllcgparameters[0].Parameter
+
+	opts := client.NewRequestOptions()
+	opts.QueryParameters.Set("name", *getAllcgparameters[0].CacheGroup)
+
+	cgResponse, reqInf, err := TOSession.GetCacheGroups(opts)
+	cg := cgResponse.Response[0]
+	cacheGroupID := cg.ID
+	cgparameters, reqInf, err := TOSession.CreateCacheGroupParameter(*cacheGroupID, parameterID, client.RequestOptions{})
+	if reqInf.StatusCode != http.StatusBadRequest {
+		t.Errorf("Expected 400 status code, got %v", reqInf.StatusCode)
+	}
+	if err == nil {
+		t.Errorf("Expected Parameter already associated with cachegroup %v - Alerts %v", cgparameters, cgparameters.Alerts)
+	}
+}
+
+func CreateTestCacheGroupParametersMulAssignments(t *testing.T) {
+	if len(testData.CacheGroups) < 3 || len(testData.Parameters) < 3 {
+		t.Fatal("Need at least three Cache Group and three Parameter to test associating Parameters to Cache Groups")
+	}
+	firstCacheGroup := testData.CacheGroups[1]
+	secondCacheGroup := testData.CacheGroups[2]
+	if firstCacheGroup.Name == nil {
+		t.Fatal("Found Cache Group1 with null or undefined name in test data")
+	}
+	if secondCacheGroup.Name == nil {
+		t.Fatal("Found Cache Group2 with null or undefined name in test data")
+	}
+
+	opts := client.NewRequestOptions()
+	opts.QueryParameters.Set("name", *firstCacheGroup.Name)
+	cacheGroupResp1, _, err := TOSession.GetCacheGroups(opts)
+	if err != nil {
+		t.Fatalf("cannot get Cache Group '%s': %v - alerts: %+v", *firstCacheGroup.Name, err, cacheGroupResp1.Alerts)
+	}
+	if len(cacheGroupResp1.Response) != 1 {
+		t.Fatalf("Expected exactly one Cache Group named '%s' to exist, but found %d", *firstCacheGroup.Name, len(cacheGroupResp1.Response))
+	}
+	opts.QueryParameters.Set("name", *secondCacheGroup.Name)
+	cacheGroupResp2, _, err := TOSession.GetCacheGroups(opts)
+	if err != nil {
+		t.Fatalf("cannot get Cache Group '%s': %v - alerts: %+v", *secondCacheGroup.Name, err, cacheGroupResp2.Alerts)
+	}
+	if len(cacheGroupResp2.Response) != 1 {
+		t.Fatalf("Expected exactly one Cache Group named '%s' to exist, but found %d", *secondCacheGroup.Name, len(cacheGroupResp2.Response))
+	}
+
+	// Get Parameter to assign to Cache Group
+	firstParameter := testData.Parameters[1]
+	secondParameter := testData.Parameters[2]
+	opts.QueryParameters.Set("name", firstParameter.Name)
+	paramResp1, _, err := TOSession.GetParameters(opts)
+	if err != nil {
+		t.Errorf("cannot get Parameter '%s': %v - alerts: %+v", firstParameter.Name, err, paramResp1.Alerts)
+	}
+	if len(paramResp1.Response) != 1 {
+		t.Fatalf("Expected exactly one Parameter named '%s' to exist, but found %d", firstParameter.Name, len(paramResp1.Response))
+	}
+	opts.QueryParameters.Set("name", secondParameter.Name)
+	paramResp2, _, err := TOSession.GetParameters(opts)
+	if err != nil {
+		t.Errorf("cannot get Parameter '%s': %v - alerts: %+v", secondParameter.Name, err, paramResp2.Alerts)
+	}
+	if len(paramResp2.Response) < 1 {
+		t.Fatalf("Expected exactly one Parameter named '%s' to exist, but found %d", secondParameter.Name, len(paramResp2.Response))
+	}
+
+	// Assign Parameter to Cache Group
+	cacheGroupID1 := cacheGroupResp1.Response[0].ID
+	cacheGroupID2 := cacheGroupResp2.Response[0].ID

Review comment:
       these are later dereferenced but are never checked for `nil`

##########
File path: traffic_ops/testing/api/v4/cachegroups_parameters_test.go
##########
@@ -155,3 +274,201 @@ func DeleteTestCacheGroupParameter(t *testing.T, cgp tc.CacheGroupParameterReque
 		t.Error("expected error when deleting cache group parameter with non existing parameter")
 	}
 }
+
+func GetTestPaginationSupportCgParameters(t *testing.T) {
+	opts := client.NewRequestOptions()
+	opts.QueryParameters.Set("orderby", "id")
+	resp, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters := resp.Response
+	if len(cachegroupParameters.CacheGroupParameters) < 3 {
+		t.Fatalf("Need at least 3 cachegroup parameters in Traffic Ops to test pagination support, found: %d", len(cachegroupParameters.CacheGroupParameters))
+	}
+
+	opts.QueryParameters.Set("orderby", "id")
+	opts.QueryParameters.Set("limit", "1")
+	respWithLimit, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters with limits: %v - alerts: %+v", err, respWithLimit.Alerts)
+	}
+	cachegroupParametersWithLimit := respWithLimit.Response
+	if !reflect.DeepEqual(cachegroupParameters.CacheGroupParameters[:1], cachegroupParametersWithLimit.CacheGroupParameters) {
+		t.Error("expected GET cachegroup parameters with limit = 1 to return first result")
+	}
+
+	opts.QueryParameters.Set("orderby", "id")
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("offset", "1")
+	respWithOffset, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters with offset: %v - alerts: %+v", err, respWithOffset.Alerts)
+	}
+	cachegroupParametersWithOffset := respWithOffset.Response
+	if !reflect.DeepEqual(cachegroupParameters.CacheGroupParameters[1:2], cachegroupParametersWithOffset.CacheGroupParameters) {
+		t.Error("expected GET cachegroup parameters with limit = 1, offset = 1 to return second result")
+	}
+
+	opts.QueryParameters.Set("orderby", "id")
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("page", "2")
+	respWithPage, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters with page: %v - alerts: %+v", err, respWithPage.Alerts)
+	}
+	cachegroupParametersWithPage := respWithPage.Response
+	if !reflect.DeepEqual(cachegroupParameters.CacheGroupParameters[1:2], cachegroupParametersWithPage.CacheGroupParameters) {
+		t.Error("expected GET cachegroup parameters with limit = 1, page = 2 to return second result")
+	}
+
+	opts.QueryParameters = url.Values{}
+	opts.QueryParameters.Set("limit", "-2")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if !alertsHaveError(resp.Alerts.Alerts, "limit parameter must be bigger than -1") {
+		t.Errorf("expected GET cachegroup parameters to return an error for limit is not bigger than -1, actual error: %v - alerts: %+v", err, resp.Alerts)
+	}
+
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("offset", "0")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if !alertsHaveError(resp.Alerts.Alerts, "offset parameter must be a positive integer") {
+		t.Errorf("expected GET cachegroup parameters to return an error for offset is not a positive integer, actual error: %v - alerts: %+v", err, resp.Alerts)
+	}
+
+	opts.QueryParameters = url.Values{}
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("page", "0")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if !alertsHaveError(resp.Alerts.Alerts, "page parameter must be a positive integer") {
+		t.Errorf("expected GET cachegroup parameters to return an error for page is not a positive integer, actual error: %v - alerts: %+v", err, resp.Alerts)
+	}
+}
+
+func SortTestCacheGroupParameters(t *testing.T) {
+	resp, _, err := TOSession.GetAllCacheGroupParameters(client.RequestOptions{})
+	if err != nil {
+		t.Fatalf("Expected no error, but got: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters := resp.Response
+	sortedList := make([]string, 0, len(cachegroupParameters.CacheGroupParameters))
+	for _, cgparameters := range cachegroupParameters.CacheGroupParameters {
+		sortedList = append(sortedList, strconv.Itoa(*cgparameters.Parameter))
+	}
+
+	res := sort.SliceIsSorted(sortedList, func(p, q int) bool {
+		return sortedList[p] < sortedList[q]
+	})
+	if !res {
+		t.Errorf("list is not sorted by their ID: %v", sortedList)
+	}
+}
+
+func SortTestCacheGroupParametersDesc(t *testing.T) {
+	resp, _, err := TOSession.GetAllCacheGroupParameters(client.RequestOptions{})
+	if err != nil {
+		t.Errorf("Expected no error, but got error in CacheGroup Parameters with default ordering: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters := resp.Response
+	respAsc := cachegroupParameters.CacheGroupParameters
+	if len(respAsc) < 1 {
+		t.Fatal("Need at least one CacheGroup Parameters in Traffic Ops to test CacheGroup Parameters sort ordering")
+	}
+
+	opts := client.NewRequestOptions()
+	opts.QueryParameters.Set("sortOrder", "desc")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Errorf("Expected no error, but got error in CacheGroup Parameters with Descending ordering: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters = resp.Response
+	respDesc := cachegroupParameters.CacheGroupParameters
+	if len(respDesc) < 1 {
+		t.Fatal("Need at least one CacheGroup Parameters in Traffic Ops to test CacheGroup Parameters sort ordering")
+	}
+
+	if len(respAsc) != len(respDesc) {
+		t.Fatalf("Traffic Ops returned %d CacheGroup Parameters using default sort order, but %d CacheGroup Parameters when sort order was explicitly set to descending", len(respAsc), len(respDesc))
+	}
+
+	// reverse the descending-sorted response and compare it to the ascending-sorted one
+	// TODO ensure at least two in each slice? A list of length one is
+	// trivially sorted both ascending and descending.
+	for start, end := 0, len(respDesc)-1; start < end; start, end = start+1, end-1 {
+		respDesc[start], respDesc[end] = respDesc[end], respDesc[start]
+	}
+	if *respDesc[0].Parameter != *respAsc[0].Parameter {
+		t.Errorf("CacheGroup Parameters responses are not equal after reversal: Asc: %v - Desc: %v", *respDesc[0].Parameter, *respAsc[0].Parameter)
+	}
+}
+
+func CreateTestCacheGroupParametersWithInvalidData(t *testing.T) {
+	resp, _, err := TOSession.GetParameters(client.RequestOptions{})
+	parametersResp := resp.Response[0]
+	parameterID := parametersResp.ID
+	//Create CacheGroupParameters with Invalid Cachegroup ID
+	cgparameters, reqInf, err := TOSession.CreateCacheGroupParameter(0, parameterID, client.RequestOptions{})
+	if reqInf.StatusCode != http.StatusNotFound {
+		t.Errorf("Expected 404 status code, got %v", reqInf.StatusCode)
+	}
+	if err == nil {
+		t.Errorf("Expected cachegroup not found, but found Alerts %v", cgparameters.Alerts)
+	}
+
+	cgresp, _, err := TOSession.GetCacheGroups(client.RequestOptions{})
+	cachegroupResp := cgresp.Response[0]
+	cachegroupID := cachegroupResp.ID
+
+	//Create CacheGroupParameters with Invalid Parameter ID
+	cgparametersresp, reqInf, err := TOSession.CreateCacheGroupParameter(*cachegroupID, 0, client.RequestOptions{})
+	if reqInf.StatusCode != http.StatusNotFound {
+		t.Errorf("Expected 404 status code, got %v", reqInf.StatusCode)
+	}
+	if err == nil {
+		t.Errorf("Expected parameter not found, but found Alerts %v", cgparametersresp.Alerts)
+	}
+}
+
+func GetTestCacheGroupParametersByInvalidData(t *testing.T) {
+
+	//Get Cachegroup parameter by Cachegroup id
+	opts := client.NewRequestOptions()
+	opts.QueryParameters.Set("cachegroup", "0")
+	resp, _, _ := TOSession.GetAllCacheGroupParameters(opts)
+	cachegroupparameters := resp.Response
+	if len(cachegroupparameters.CacheGroupParameters) > 0 {
+		t.Errorf("Expected No response for the invalid cachegroup id, but found %d responses", len(cachegroupparameters.CacheGroupParameters))
+	}
+	//Get Cachegroup parameter by Parameter id
+	opts = client.NewRequestOptions()
+	opts.QueryParameters.Set("parameter", "0")
+	resp, _, _ = TOSession.GetAllCacheGroupParameters(opts)
+	cachegroupparameters = resp.Response
+	if len(cachegroupparameters.CacheGroupParameters) > 0 {
+		t.Errorf("Expected No response for the invalid parameter id, but found %d responses", len(cachegroupparameters.CacheGroupParameters))
+	}
+}
+
+func GetTestCacheGroupParameter(t *testing.T) {
+	firstData := testData.CacheGroupParameterRequests[0]

Review comment:
       This will segfault if the test data is in a bad state

##########
File path: traffic_ops/testing/api/v4/cachegroups_parameters_test.go
##########
@@ -155,3 +274,201 @@ func DeleteTestCacheGroupParameter(t *testing.T, cgp tc.CacheGroupParameterReque
 		t.Error("expected error when deleting cache group parameter with non existing parameter")
 	}
 }
+
+func GetTestPaginationSupportCgParameters(t *testing.T) {
+	opts := client.NewRequestOptions()
+	opts.QueryParameters.Set("orderby", "id")
+	resp, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters := resp.Response
+	if len(cachegroupParameters.CacheGroupParameters) < 3 {
+		t.Fatalf("Need at least 3 cachegroup parameters in Traffic Ops to test pagination support, found: %d", len(cachegroupParameters.CacheGroupParameters))
+	}
+
+	opts.QueryParameters.Set("orderby", "id")
+	opts.QueryParameters.Set("limit", "1")
+	respWithLimit, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters with limits: %v - alerts: %+v", err, respWithLimit.Alerts)
+	}
+	cachegroupParametersWithLimit := respWithLimit.Response
+	if !reflect.DeepEqual(cachegroupParameters.CacheGroupParameters[:1], cachegroupParametersWithLimit.CacheGroupParameters) {
+		t.Error("expected GET cachegroup parameters with limit = 1 to return first result")
+	}
+
+	opts.QueryParameters.Set("orderby", "id")
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("offset", "1")
+	respWithOffset, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters with offset: %v - alerts: %+v", err, respWithOffset.Alerts)
+	}
+	cachegroupParametersWithOffset := respWithOffset.Response
+	if !reflect.DeepEqual(cachegroupParameters.CacheGroupParameters[1:2], cachegroupParametersWithOffset.CacheGroupParameters) {
+		t.Error("expected GET cachegroup parameters with limit = 1, offset = 1 to return second result")
+	}
+
+	opts.QueryParameters.Set("orderby", "id")
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("page", "2")
+	respWithPage, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters with page: %v - alerts: %+v", err, respWithPage.Alerts)
+	}
+	cachegroupParametersWithPage := respWithPage.Response
+	if !reflect.DeepEqual(cachegroupParameters.CacheGroupParameters[1:2], cachegroupParametersWithPage.CacheGroupParameters) {
+		t.Error("expected GET cachegroup parameters with limit = 1, page = 2 to return second result")
+	}
+
+	opts.QueryParameters = url.Values{}
+	opts.QueryParameters.Set("limit", "-2")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if !alertsHaveError(resp.Alerts.Alerts, "limit parameter must be bigger than -1") {
+		t.Errorf("expected GET cachegroup parameters to return an error for limit is not bigger than -1, actual error: %v - alerts: %+v", err, resp.Alerts)
+	}
+
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("offset", "0")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if !alertsHaveError(resp.Alerts.Alerts, "offset parameter must be a positive integer") {
+		t.Errorf("expected GET cachegroup parameters to return an error for offset is not a positive integer, actual error: %v - alerts: %+v", err, resp.Alerts)
+	}
+
+	opts.QueryParameters = url.Values{}
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("page", "0")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if !alertsHaveError(resp.Alerts.Alerts, "page parameter must be a positive integer") {
+		t.Errorf("expected GET cachegroup parameters to return an error for page is not a positive integer, actual error: %v - alerts: %+v", err, resp.Alerts)
+	}
+}
+
+func SortTestCacheGroupParameters(t *testing.T) {
+	resp, _, err := TOSession.GetAllCacheGroupParameters(client.RequestOptions{})
+	if err != nil {
+		t.Fatalf("Expected no error, but got: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters := resp.Response
+	sortedList := make([]string, 0, len(cachegroupParameters.CacheGroupParameters))
+	for _, cgparameters := range cachegroupParameters.CacheGroupParameters {
+		sortedList = append(sortedList, strconv.Itoa(*cgparameters.Parameter))
+	}
+
+	res := sort.SliceIsSorted(sortedList, func(p, q int) bool {
+		return sortedList[p] < sortedList[q]
+	})
+	if !res {
+		t.Errorf("list is not sorted by their ID: %v", sortedList)
+	}
+}
+
+func SortTestCacheGroupParametersDesc(t *testing.T) {
+	resp, _, err := TOSession.GetAllCacheGroupParameters(client.RequestOptions{})
+	if err != nil {
+		t.Errorf("Expected no error, but got error in CacheGroup Parameters with default ordering: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters := resp.Response
+	respAsc := cachegroupParameters.CacheGroupParameters
+	if len(respAsc) < 1 {
+		t.Fatal("Need at least one CacheGroup Parameters in Traffic Ops to test CacheGroup Parameters sort ordering")
+	}
+
+	opts := client.NewRequestOptions()
+	opts.QueryParameters.Set("sortOrder", "desc")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Errorf("Expected no error, but got error in CacheGroup Parameters with Descending ordering: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters = resp.Response
+	respDesc := cachegroupParameters.CacheGroupParameters
+	if len(respDesc) < 1 {
+		t.Fatal("Need at least one CacheGroup Parameters in Traffic Ops to test CacheGroup Parameters sort ordering")
+	}
+
+	if len(respAsc) != len(respDesc) {
+		t.Fatalf("Traffic Ops returned %d CacheGroup Parameters using default sort order, but %d CacheGroup Parameters when sort order was explicitly set to descending", len(respAsc), len(respDesc))
+	}
+
+	// reverse the descending-sorted response and compare it to the ascending-sorted one
+	// TODO ensure at least two in each slice? A list of length one is
+	// trivially sorted both ascending and descending.
+	for start, end := 0, len(respDesc)-1; start < end; start, end = start+1, end-1 {
+		respDesc[start], respDesc[end] = respDesc[end], respDesc[start]
+	}
+	if *respDesc[0].Parameter != *respAsc[0].Parameter {

Review comment:
       This will segfault if either Parameter ID was null or undefined in the response

##########
File path: traffic_ops/testing/api/v4/cachegroups_parameters_test.go
##########
@@ -79,6 +90,114 @@ func CreateTestCacheGroupParameters(t *testing.T) {
 	testData.CacheGroupParameterRequests = append(testData.CacheGroupParameterRequests, resp.Response...)
 }
 
+func CreateTestCacheGroupParametersAlreadyExist(t *testing.T) {
+
+	resp, _, err := TOSession.GetAllCacheGroupParameters(client.RequestOptions{})
+	cachegroupParameters := resp.Response
+	getAllcgparameters := cachegroupParameters.CacheGroupParameters
+	parameterID := *getAllcgparameters[0].Parameter
+
+	opts := client.NewRequestOptions()
+	opts.QueryParameters.Set("name", *getAllcgparameters[0].CacheGroup)
+
+	cgResponse, reqInf, err := TOSession.GetCacheGroups(opts)
+	cg := cgResponse.Response[0]
+	cacheGroupID := cg.ID
+	cgparameters, reqInf, err := TOSession.CreateCacheGroupParameter(*cacheGroupID, parameterID, client.RequestOptions{})
+	if reqInf.StatusCode != http.StatusBadRequest {
+		t.Errorf("Expected 400 status code, got %v", reqInf.StatusCode)
+	}
+	if err == nil {
+		t.Errorf("Expected Parameter already associated with cachegroup %v - Alerts %v", cgparameters, cgparameters.Alerts)

Review comment:
       I think you meant to print the Cache Group ID here instead of `cgparameters`

##########
File path: traffic_ops/testing/api/v4/cachegroups_parameters_test.go
##########
@@ -155,3 +274,201 @@ func DeleteTestCacheGroupParameter(t *testing.T, cgp tc.CacheGroupParameterReque
 		t.Error("expected error when deleting cache group parameter with non existing parameter")
 	}
 }
+
+func GetTestPaginationSupportCgParameters(t *testing.T) {
+	opts := client.NewRequestOptions()
+	opts.QueryParameters.Set("orderby", "id")
+	resp, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters := resp.Response
+	if len(cachegroupParameters.CacheGroupParameters) < 3 {
+		t.Fatalf("Need at least 3 cachegroup parameters in Traffic Ops to test pagination support, found: %d", len(cachegroupParameters.CacheGroupParameters))
+	}
+
+	opts.QueryParameters.Set("orderby", "id")
+	opts.QueryParameters.Set("limit", "1")
+	respWithLimit, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters with limits: %v - alerts: %+v", err, respWithLimit.Alerts)
+	}
+	cachegroupParametersWithLimit := respWithLimit.Response
+	if !reflect.DeepEqual(cachegroupParameters.CacheGroupParameters[:1], cachegroupParametersWithLimit.CacheGroupParameters) {
+		t.Error("expected GET cachegroup parameters with limit = 1 to return first result")
+	}
+
+	opts.QueryParameters.Set("orderby", "id")
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("offset", "1")
+	respWithOffset, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters with offset: %v - alerts: %+v", err, respWithOffset.Alerts)
+	}
+	cachegroupParametersWithOffset := respWithOffset.Response
+	if !reflect.DeepEqual(cachegroupParameters.CacheGroupParameters[1:2], cachegroupParametersWithOffset.CacheGroupParameters) {
+		t.Error("expected GET cachegroup parameters with limit = 1, offset = 1 to return second result")
+	}
+
+	opts.QueryParameters.Set("orderby", "id")
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("page", "2")
+	respWithPage, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters with page: %v - alerts: %+v", err, respWithPage.Alerts)
+	}
+	cachegroupParametersWithPage := respWithPage.Response
+	if !reflect.DeepEqual(cachegroupParameters.CacheGroupParameters[1:2], cachegroupParametersWithPage.CacheGroupParameters) {
+		t.Error("expected GET cachegroup parameters with limit = 1, page = 2 to return second result")
+	}
+
+	opts.QueryParameters = url.Values{}
+	opts.QueryParameters.Set("limit", "-2")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if !alertsHaveError(resp.Alerts.Alerts, "limit parameter must be bigger than -1") {
+		t.Errorf("expected GET cachegroup parameters to return an error for limit is not bigger than -1, actual error: %v - alerts: %+v", err, resp.Alerts)
+	}
+
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("offset", "0")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if !alertsHaveError(resp.Alerts.Alerts, "offset parameter must be a positive integer") {
+		t.Errorf("expected GET cachegroup parameters to return an error for offset is not a positive integer, actual error: %v - alerts: %+v", err, resp.Alerts)
+	}
+
+	opts.QueryParameters = url.Values{}
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("page", "0")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if !alertsHaveError(resp.Alerts.Alerts, "page parameter must be a positive integer") {
+		t.Errorf("expected GET cachegroup parameters to return an error for page is not a positive integer, actual error: %v - alerts: %+v", err, resp.Alerts)
+	}
+}
+
+func SortTestCacheGroupParameters(t *testing.T) {
+	resp, _, err := TOSession.GetAllCacheGroupParameters(client.RequestOptions{})
+	if err != nil {
+		t.Fatalf("Expected no error, but got: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters := resp.Response
+	sortedList := make([]string, 0, len(cachegroupParameters.CacheGroupParameters))
+	for _, cgparameters := range cachegroupParameters.CacheGroupParameters {
+		sortedList = append(sortedList, strconv.Itoa(*cgparameters.Parameter))
+	}
+
+	res := sort.SliceIsSorted(sortedList, func(p, q int) bool {
+		return sortedList[p] < sortedList[q]
+	})
+	if !res {
+		t.Errorf("list is not sorted by their ID: %v", sortedList)
+	}
+}
+
+func SortTestCacheGroupParametersDesc(t *testing.T) {
+	resp, _, err := TOSession.GetAllCacheGroupParameters(client.RequestOptions{})
+	if err != nil {
+		t.Errorf("Expected no error, but got error in CacheGroup Parameters with default ordering: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters := resp.Response
+	respAsc := cachegroupParameters.CacheGroupParameters
+	if len(respAsc) < 1 {
+		t.Fatal("Need at least one CacheGroup Parameters in Traffic Ops to test CacheGroup Parameters sort ordering")
+	}
+
+	opts := client.NewRequestOptions()
+	opts.QueryParameters.Set("sortOrder", "desc")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Errorf("Expected no error, but got error in CacheGroup Parameters with Descending ordering: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters = resp.Response
+	respDesc := cachegroupParameters.CacheGroupParameters
+	if len(respDesc) < 1 {
+		t.Fatal("Need at least one CacheGroup Parameters in Traffic Ops to test CacheGroup Parameters sort ordering")
+	}
+
+	if len(respAsc) != len(respDesc) {
+		t.Fatalf("Traffic Ops returned %d CacheGroup Parameters using default sort order, but %d CacheGroup Parameters when sort order was explicitly set to descending", len(respAsc), len(respDesc))
+	}
+
+	// reverse the descending-sorted response and compare it to the ascending-sorted one
+	// TODO ensure at least two in each slice? A list of length one is
+	// trivially sorted both ascending and descending.
+	for start, end := 0, len(respDesc)-1; start < end; start, end = start+1, end-1 {
+		respDesc[start], respDesc[end] = respDesc[end], respDesc[start]
+	}
+	if *respDesc[0].Parameter != *respAsc[0].Parameter {
+		t.Errorf("CacheGroup Parameters responses are not equal after reversal: Asc: %v - Desc: %v", *respDesc[0].Parameter, *respAsc[0].Parameter)
+	}
+}
+
+func CreateTestCacheGroupParametersWithInvalidData(t *testing.T) {
+	resp, _, err := TOSession.GetParameters(client.RequestOptions{})
+	parametersResp := resp.Response[0]
+	parameterID := parametersResp.ID
+	//Create CacheGroupParameters with Invalid Cachegroup ID
+	cgparameters, reqInf, err := TOSession.CreateCacheGroupParameter(0, parameterID, client.RequestOptions{})
+	if reqInf.StatusCode != http.StatusNotFound {
+		t.Errorf("Expected 404 status code, got %v", reqInf.StatusCode)
+	}
+	if err == nil {
+		t.Errorf("Expected cachegroup not found, but found Alerts %v", cgparameters.Alerts)
+	}
+
+	cgresp, _, err := TOSession.GetCacheGroups(client.RequestOptions{})
+	cachegroupResp := cgresp.Response[0]

Review comment:
       This will segfault if the response was null, undefined, or empty

##########
File path: traffic_ops/testing/api/v4/cachegroups_parameters_test.go
##########
@@ -155,3 +274,201 @@ func DeleteTestCacheGroupParameter(t *testing.T, cgp tc.CacheGroupParameterReque
 		t.Error("expected error when deleting cache group parameter with non existing parameter")
 	}
 }
+
+func GetTestPaginationSupportCgParameters(t *testing.T) {
+	opts := client.NewRequestOptions()
+	opts.QueryParameters.Set("orderby", "id")
+	resp, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters := resp.Response
+	if len(cachegroupParameters.CacheGroupParameters) < 3 {
+		t.Fatalf("Need at least 3 cachegroup parameters in Traffic Ops to test pagination support, found: %d", len(cachegroupParameters.CacheGroupParameters))
+	}
+
+	opts.QueryParameters.Set("orderby", "id")
+	opts.QueryParameters.Set("limit", "1")
+	respWithLimit, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters with limits: %v - alerts: %+v", err, respWithLimit.Alerts)
+	}
+	cachegroupParametersWithLimit := respWithLimit.Response
+	if !reflect.DeepEqual(cachegroupParameters.CacheGroupParameters[:1], cachegroupParametersWithLimit.CacheGroupParameters) {
+		t.Error("expected GET cachegroup parameters with limit = 1 to return first result")
+	}
+
+	opts.QueryParameters.Set("orderby", "id")
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("offset", "1")
+	respWithOffset, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters with offset: %v - alerts: %+v", err, respWithOffset.Alerts)
+	}
+	cachegroupParametersWithOffset := respWithOffset.Response
+	if !reflect.DeepEqual(cachegroupParameters.CacheGroupParameters[1:2], cachegroupParametersWithOffset.CacheGroupParameters) {
+		t.Error("expected GET cachegroup parameters with limit = 1, offset = 1 to return second result")
+	}
+
+	opts.QueryParameters.Set("orderby", "id")
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("page", "2")
+	respWithPage, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters with page: %v - alerts: %+v", err, respWithPage.Alerts)
+	}
+	cachegroupParametersWithPage := respWithPage.Response
+	if !reflect.DeepEqual(cachegroupParameters.CacheGroupParameters[1:2], cachegroupParametersWithPage.CacheGroupParameters) {
+		t.Error("expected GET cachegroup parameters with limit = 1, page = 2 to return second result")
+	}
+
+	opts.QueryParameters = url.Values{}
+	opts.QueryParameters.Set("limit", "-2")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if !alertsHaveError(resp.Alerts.Alerts, "limit parameter must be bigger than -1") {
+		t.Errorf("expected GET cachegroup parameters to return an error for limit is not bigger than -1, actual error: %v - alerts: %+v", err, resp.Alerts)
+	}
+
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("offset", "0")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if !alertsHaveError(resp.Alerts.Alerts, "offset parameter must be a positive integer") {
+		t.Errorf("expected GET cachegroup parameters to return an error for offset is not a positive integer, actual error: %v - alerts: %+v", err, resp.Alerts)
+	}
+
+	opts.QueryParameters = url.Values{}
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("page", "0")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if !alertsHaveError(resp.Alerts.Alerts, "page parameter must be a positive integer") {
+		t.Errorf("expected GET cachegroup parameters to return an error for page is not a positive integer, actual error: %v - alerts: %+v", err, resp.Alerts)
+	}
+}
+
+func SortTestCacheGroupParameters(t *testing.T) {
+	resp, _, err := TOSession.GetAllCacheGroupParameters(client.RequestOptions{})
+	if err != nil {
+		t.Fatalf("Expected no error, but got: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters := resp.Response
+	sortedList := make([]string, 0, len(cachegroupParameters.CacheGroupParameters))
+	for _, cgparameters := range cachegroupParameters.CacheGroupParameters {
+		sortedList = append(sortedList, strconv.Itoa(*cgparameters.Parameter))

Review comment:
       this will segfault if the parameter was null or undefined in the response

##########
File path: traffic_ops/testing/api/v4/cachegroups_parameters_test.go
##########
@@ -155,3 +274,201 @@ func DeleteTestCacheGroupParameter(t *testing.T, cgp tc.CacheGroupParameterReque
 		t.Error("expected error when deleting cache group parameter with non existing parameter")
 	}
 }
+
+func GetTestPaginationSupportCgParameters(t *testing.T) {
+	opts := client.NewRequestOptions()
+	opts.QueryParameters.Set("orderby", "id")
+	resp, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters := resp.Response
+	if len(cachegroupParameters.CacheGroupParameters) < 3 {
+		t.Fatalf("Need at least 3 cachegroup parameters in Traffic Ops to test pagination support, found: %d", len(cachegroupParameters.CacheGroupParameters))
+	}
+
+	opts.QueryParameters.Set("orderby", "id")
+	opts.QueryParameters.Set("limit", "1")
+	respWithLimit, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters with limits: %v - alerts: %+v", err, respWithLimit.Alerts)
+	}
+	cachegroupParametersWithLimit := respWithLimit.Response
+	if !reflect.DeepEqual(cachegroupParameters.CacheGroupParameters[:1], cachegroupParametersWithLimit.CacheGroupParameters) {
+		t.Error("expected GET cachegroup parameters with limit = 1 to return first result")
+	}
+
+	opts.QueryParameters.Set("orderby", "id")
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("offset", "1")
+	respWithOffset, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters with offset: %v - alerts: %+v", err, respWithOffset.Alerts)
+	}
+	cachegroupParametersWithOffset := respWithOffset.Response
+	if !reflect.DeepEqual(cachegroupParameters.CacheGroupParameters[1:2], cachegroupParametersWithOffset.CacheGroupParameters) {
+		t.Error("expected GET cachegroup parameters with limit = 1, offset = 1 to return second result")
+	}
+
+	opts.QueryParameters.Set("orderby", "id")
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("page", "2")
+	respWithPage, _, err := TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Fatalf("cannot get cachegroup parameters with page: %v - alerts: %+v", err, respWithPage.Alerts)
+	}
+	cachegroupParametersWithPage := respWithPage.Response
+	if !reflect.DeepEqual(cachegroupParameters.CacheGroupParameters[1:2], cachegroupParametersWithPage.CacheGroupParameters) {
+		t.Error("expected GET cachegroup parameters with limit = 1, page = 2 to return second result")
+	}
+
+	opts.QueryParameters = url.Values{}
+	opts.QueryParameters.Set("limit", "-2")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if !alertsHaveError(resp.Alerts.Alerts, "limit parameter must be bigger than -1") {
+		t.Errorf("expected GET cachegroup parameters to return an error for limit is not bigger than -1, actual error: %v - alerts: %+v", err, resp.Alerts)
+	}
+
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("offset", "0")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if !alertsHaveError(resp.Alerts.Alerts, "offset parameter must be a positive integer") {
+		t.Errorf("expected GET cachegroup parameters to return an error for offset is not a positive integer, actual error: %v - alerts: %+v", err, resp.Alerts)
+	}
+
+	opts.QueryParameters = url.Values{}
+	opts.QueryParameters.Set("limit", "1")
+	opts.QueryParameters.Set("page", "0")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if !alertsHaveError(resp.Alerts.Alerts, "page parameter must be a positive integer") {
+		t.Errorf("expected GET cachegroup parameters to return an error for page is not a positive integer, actual error: %v - alerts: %+v", err, resp.Alerts)
+	}
+}
+
+func SortTestCacheGroupParameters(t *testing.T) {
+	resp, _, err := TOSession.GetAllCacheGroupParameters(client.RequestOptions{})
+	if err != nil {
+		t.Fatalf("Expected no error, but got: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters := resp.Response
+	sortedList := make([]string, 0, len(cachegroupParameters.CacheGroupParameters))
+	for _, cgparameters := range cachegroupParameters.CacheGroupParameters {
+		sortedList = append(sortedList, strconv.Itoa(*cgparameters.Parameter))
+	}
+
+	res := sort.SliceIsSorted(sortedList, func(p, q int) bool {
+		return sortedList[p] < sortedList[q]
+	})
+	if !res {
+		t.Errorf("list is not sorted by their ID: %v", sortedList)
+	}
+}
+
+func SortTestCacheGroupParametersDesc(t *testing.T) {
+	resp, _, err := TOSession.GetAllCacheGroupParameters(client.RequestOptions{})
+	if err != nil {
+		t.Errorf("Expected no error, but got error in CacheGroup Parameters with default ordering: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters := resp.Response
+	respAsc := cachegroupParameters.CacheGroupParameters
+	if len(respAsc) < 1 {
+		t.Fatal("Need at least one CacheGroup Parameters in Traffic Ops to test CacheGroup Parameters sort ordering")
+	}
+
+	opts := client.NewRequestOptions()
+	opts.QueryParameters.Set("sortOrder", "desc")
+	resp, _, err = TOSession.GetAllCacheGroupParameters(opts)
+	if err != nil {
+		t.Errorf("Expected no error, but got error in CacheGroup Parameters with Descending ordering: %v - alerts: %+v", err, resp.Alerts)
+	}
+	cachegroupParameters = resp.Response
+	respDesc := cachegroupParameters.CacheGroupParameters
+	if len(respDesc) < 1 {
+		t.Fatal("Need at least one CacheGroup Parameters in Traffic Ops to test CacheGroup Parameters sort ordering")
+	}
+
+	if len(respAsc) != len(respDesc) {
+		t.Fatalf("Traffic Ops returned %d CacheGroup Parameters using default sort order, but %d CacheGroup Parameters when sort order was explicitly set to descending", len(respAsc), len(respDesc))
+	}
+
+	// reverse the descending-sorted response and compare it to the ascending-sorted one
+	// TODO ensure at least two in each slice? A list of length one is
+	// trivially sorted both ascending and descending.
+	for start, end := 0, len(respDesc)-1; start < end; start, end = start+1, end-1 {
+		respDesc[start], respDesc[end] = respDesc[end], respDesc[start]
+	}
+	if *respDesc[0].Parameter != *respAsc[0].Parameter {
+		t.Errorf("CacheGroup Parameters responses are not equal after reversal: Asc: %v - Desc: %v", *respDesc[0].Parameter, *respAsc[0].Parameter)
+	}
+}
+
+func CreateTestCacheGroupParametersWithInvalidData(t *testing.T) {
+	resp, _, err := TOSession.GetParameters(client.RequestOptions{})
+	parametersResp := resp.Response[0]
+	parameterID := parametersResp.ID
+	//Create CacheGroupParameters with Invalid Cachegroup ID
+	cgparameters, reqInf, err := TOSession.CreateCacheGroupParameter(0, parameterID, client.RequestOptions{})
+	if reqInf.StatusCode != http.StatusNotFound {
+		t.Errorf("Expected 404 status code, got %v", reqInf.StatusCode)
+	}
+	if err == nil {
+		t.Errorf("Expected cachegroup not found, but found Alerts %v", cgparameters.Alerts)
+	}
+
+	cgresp, _, err := TOSession.GetCacheGroups(client.RequestOptions{})
+	cachegroupResp := cgresp.Response[0]
+	cachegroupID := cachegroupResp.ID
+
+	//Create CacheGroupParameters with Invalid Parameter ID
+	cgparametersresp, reqInf, err := TOSession.CreateCacheGroupParameter(*cachegroupID, 0, client.RequestOptions{})

Review comment:
       This will segfault if the Cache Group ID was null or undefined in the response




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org