[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #4695: Cachegroups API updates for Topologies

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

rawlinp commented on a change in pull request #4695:
URL: https://github.com/apache/trafficcontrol/pull/4695#discussion_r426905155



##########
File path: traffic_ops/testing/api/v3/cachegroups_test.go
##########
@@ -70,16 +72,47 @@ func GetTestCacheGroupsByName(t *testing.T) {
 		if err != nil {
 			t.Errorf("cannot GET CacheGroup by name: %v - %v", err, resp)
 		}
+		if *resp[0].Name != *cg.Name {
+			t.Errorf("name expected: %s, actual: %s", *cg.Name, *resp[0].Name)
+		}
 	}
 }
 
 func GetTestCacheGroupsByShortName(t *testing.T) {
 	for _, cg := range testData.CacheGroups {
-		resp, _, err := TOSession.GetCacheGroupNullableByName(*cg.ShortName)
+		resp, _, err := TOSession.GetCacheGroupNullableByShortName(*cg.ShortName)
 		if err != nil {
 			t.Errorf("cannot GET CacheGroup by shortName: %v - %v", err, resp)
 		}
+		if *resp[0].ShortName != *cg.ShortName {
+			t.Errorf("short name expected: %s, actual: %s", *cg.ShortName, *resp[0].ShortName)
+		}
+	}
+}
+
+func GetTestCacheGroupsByTopology(t *testing.T) {
+	for _, top := range testData.Topologies {
+		qparams := url.Values{}
+		qparams.Set("topology", top.Name)

Review comment:
       Yeah, I think I like the original version better because `[]string{}` is kind of distracting IMO.

##########
File path: traffic_ops/traffic_ops_golang/cachegroup/cachegroups.go
##########
@@ -427,6 +432,7 @@ func (cg *TOCacheGroup) Read() ([]interface{}, error, error, int) {
 		"name":      dbhelpers.WhereColumnInfo{"cachegroup.name", nil},
 		"shortName": dbhelpers.WhereColumnInfo{"cachegroup.short_name", nil},
 		"type":      dbhelpers.WhereColumnInfo{"cachegroup.type", nil},
+		"topology":  dbhelpers.WhereColumnInfo{"topology_cachegroup.topology", nil},

Review comment:
       chose 1st option in 59d6fb1005576218c08246171707843ba05e3e6f

##########
File path: traffic_ops/client/cachegroup.go
##########
@@ -189,3 +189,24 @@ func (to *Session) DeleteCacheGroupByID(id int) (tc.Alerts, ReqInf, error) {
 	}
 	return alerts, reqInf, nil
 }
+
+func (to *Session) GetCacheGroupsByQueryParams(qparams url.Values) ([]tc.CacheGroupNullable, ReqInf, error) {

Review comment:
       added in 59d6fb1005576218c08246171707843ba05e3e6f 

##########
File path: traffic_ops/client/cachegroup.go
##########
@@ -189,3 +189,24 @@ func (to *Session) DeleteCacheGroupByID(id int) (tc.Alerts, ReqInf, error) {
 	}
 	return alerts, reqInf, nil
 }
+
+func (to *Session) GetCacheGroupsByQueryParams(qparams url.Values) ([]tc.CacheGroupNullable, ReqInf, error) {
+	route := API_CACHEGROUPS
+	if len(qparams) > 0 {
+		route += "?" + qparams.Encode()
+	}
+
+	resp, remoteAddr, err := to.request(http.MethodGet, route, nil)
+	reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr: remoteAddr}
+	if err != nil {
+		return nil, reqInf, err
+	}
+	defer resp.Body.Close()

Review comment:
       done in 59d6fb1005576218c08246171707843ba05e3e6f 




----------------------------------------------------------------
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