rawlinp commented on a change in pull request #4700:
URL: https://github.com/apache/trafficcontrol/pull/4700#discussion_r430701229
##########
File path: traffic_ops/client/server.go
##########
@@ -33,158 +33,144 @@ const (
API_SERVER_ASSIGN_DELIVERY_SERVICES = API_SERVER_DELIVERY_SERVICES +
"?replace=%t"
)
+func needAndCanFetch(id *int, name *string) bool {
+ return (id == nil || *id == 0) && name != nil && *name != ""
+}
+
// CreateServer creates a Server.
-func (to *Session) CreateServer(server tc.Server) (tc.Alerts, ReqInf, error) {
+func (to *Session) CreateServer(server tc.ServerNullable) (tc.Alerts, ReqInf,
error) {
+ var alerts tc.Alerts
var remoteAddr net.Addr
reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr:
remoteAddr}
- if server.CachegroupID == 0 && server.Cachegroup != "" {
- cg, _, err := to.GetCacheGroupNullableByName(server.Cachegroup)
+ if needAndCanFetch(server.CachegroupID, server.Cachegroup) {
+ cg, _, err := to.GetCacheGroupNullableByName(*server.Cachegroup)
if err != nil {
- return tc.Alerts{}, ReqInf{}, errors.New("no cachegroup
named " + server.Cachegroup + ":" + err.Error())
+ return alerts, reqInf, fmt.Errorf("no cachegroup named
%s: %v", *server.Cachegroup, err)
}
if len(cg) == 0 {
- return tc.Alerts{}, ReqInf{}, errors.New("no cachegroup
named " + server.Cachegroup)
+ return alerts, reqInf, fmt.Errorf("no cachegroup named
%s", *server.Cachegroup)
}
if cg[0].ID == nil {
- return tc.Alerts{}, ReqInf{}, errors.New("Cachegroup
named " + server.Cachegroup + " has a nil ID")
+ return alerts, reqInf, fmt.Errorf("Cachegroup named %s
has a nil ID", *server.Cachegroup)
}
- server.CachegroupID = *cg[0].ID
+ server.CachegroupID = cg[0].ID
}
- if server.CDNID == 0 && server.CDNName != "" {
- c, _, err := to.GetCDNByName(server.CDNName)
+ if needAndCanFetch(server.CDNID, server.CDNName) {
+ c, _, err := to.GetCDNByName(*server.CDNName)
if err != nil {
- return tc.Alerts{}, ReqInf{}, errors.New("no CDN named
" + server.CDNName + ":" + err.Error())
+ return alerts, reqInf, fmt.Errorf("no CDN named %s:
%v", *server.CDNName, err)
}
if len(c) == 0 {
- return tc.Alerts{}, ReqInf{}, errors.New("no CDN named
" + server.CDNName)
+ return alerts, reqInf, fmt.Errorf("no CDN named %s",
*server.CDNName)
}
- server.CDNID = c[0].ID
+ server.CDNID = &c[0].ID
}
- if server.PhysLocationID == 0 && server.PhysLocation != "" {
- ph, _, err := to.GetPhysLocationByName(server.PhysLocation)
+ if needAndCanFetch(server.PhysLocationID, server.PhysLocation) {
+ ph, _, err := to.GetPhysLocationByName(*server.PhysLocation)
if err != nil {
- return tc.Alerts{}, ReqInf{}, errors.New("no
physlocation named " + server.PhysLocation + ":" + err.Error())
+ return alerts, reqInf, fmt.Errorf("no physlocation
named %s: %v", *server.PhysLocation, err)
}
if len(ph) == 0 {
- return tc.Alerts{}, ReqInf{}, errors.New("no
physlocation named " + server.PhysLocation)
+ return alerts, reqInf, fmt.Errorf("no physlocation
named %s", *server.PhysLocation)
}
- server.PhysLocationID = ph[0].ID
+ server.PhysLocationID = &ph[0].ID
}
- if server.ProfileID == 0 && server.Profile != "" {
- pr, _, err := to.GetProfileByName(server.Profile)
+ if needAndCanFetch(server.ProfileID, server.Profile) {
+ pr, _, err := to.GetProfileByName(*server.Profile)
if err != nil {
- return tc.Alerts{}, ReqInf{}, errors.New("no profile
named " + server.Profile + ":" + err.Error())
+ return alerts, reqInf, fmt.Errorf("no profile named %s:
%v", *server.Profile, err)
}
if len(pr) == 0 {
- return tc.Alerts{}, ReqInf{}, errors.New("no profile
named " + server.Profile)
+ return alerts, reqInf, fmt.Errorf("no profile named
%s", *server.Profile)
}
- server.ProfileID = pr[0].ID
+ server.ProfileID = &pr[0].ID
}
- if server.StatusID == 0 && server.Status != "" {
- st, _, err := to.GetStatusByName(server.Status)
+ if needAndCanFetch(server.StatusID, server.Status) {
+ st, _, err := to.GetStatusByName(*server.Status)
if err != nil {
- return tc.Alerts{}, ReqInf{}, errors.New("no status
named " + server.Status + ":" + err.Error())
+ return alerts, reqInf, fmt.Errorf("no status named %s:
%v", *server.Status, err)
}
if len(st) == 0 {
- return tc.Alerts{}, ReqInf{}, errors.New("no status
named " + server.Status)
+ return alerts, reqInf, fmt.Errorf("no status named %s",
*server.Status)
}
- server.StatusID = st[0].ID
+ server.StatusID = &st[0].ID
}
- if server.TypeID == 0 && server.Type != "" {
+ if (server.TypeID == nil || *server.TypeID == 0) && server.Type != "" {
ty, _, err := to.GetTypeByName(server.Type)
if err != nil {
- return tc.Alerts{}, ReqInf{}, errors.New("no type named
" + server.Type + ":" + err.Error())
+ return alerts, reqInf, fmt.Errorf("no type named %s:
%v", server.Type, err)
}
if len(ty) == 0 {
- return tc.Alerts{}, ReqInf{}, errors.New("no type named
" + server.Type)
+ return alerts, reqInf, fmt.Errorf("no type named %s",
server.Type)
}
- server.TypeID = ty[0].ID
+ server.TypeID = &ty[0].ID
}
+
reqBody, err := json.Marshal(server)
if err != nil {
- return tc.Alerts{}, reqInf, err
+ return alerts, reqInf, err
}
resp, remoteAddr, err := to.request(http.MethodPost, API_SERVERS,
reqBody)
+ reqInf.RemoteAddr = remoteAddr
if err != nil {
- return tc.Alerts{}, reqInf, err
+ return alerts, reqInf, err
}
defer resp.Body.Close()
- var alerts tc.Alerts
+
err = json.NewDecoder(resp.Body).Decode(&alerts)
- return alerts, reqInf, nil
+ return alerts, reqInf, err
}
// UpdateServerByID updates a Server by ID.
-func (to *Session) UpdateServerByID(id int, server tc.Server) (tc.Alerts,
ReqInf, error) {
-
+func (to *Session) UpdateServerByID(id int, server tc.ServerNullable)
(tc.Alerts, ReqInf, error) {
+ var alerts tc.Alerts
var remoteAddr net.Addr
- reqBody, err := json.Marshal(server)
reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr:
remoteAddr}
+
+ reqBody, err := json.Marshal(server)
if err != nil {
- return tc.Alerts{}, reqInf, err
+ return alerts, reqInf, err
}
+
route := fmt.Sprintf("%s/%d", API_SERVERS, id)
resp, remoteAddr, err := to.request(http.MethodPut, route, reqBody)
+ reqInf.RemoteAddr = remoteAddr
if err != nil {
- return tc.Alerts{}, reqInf, err
+ return alerts, reqInf, err
}
defer resp.Body.Close()
- var alerts tc.Alerts
+
err = json.NewDecoder(resp.Body).Decode(&alerts)
- return alerts, reqInf, nil
+ return alerts, reqInf, err
}
// GetServers returns a list of Servers.
-func (to *Session) GetServers() ([]tc.Server, ReqInf, error) {
- resp, remoteAddr, err := to.request(http.MethodGet, API_SERVERS, nil)
- reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr:
remoteAddr}
- if err != nil {
- return nil, reqInf, err
+// The 'params' parameter can be used to optionally pass URL "query string
+// parameters" in the request.
+// It returns, in order, the requested servers, any Alerts the API returned,
+// the returned summary.count value, a request info object, and any error that
+// occurred.
+func (to *Session) GetServers(params *url.Values) ([]tc.ServerNullable,
tc.Alerts, uint64, ReqInf, error) {
Review comment:
We're starting to get pretty heavy in terms of the number of things
these client methods return. Should we just return `ServersV3Response` instead
of `[]tc.ServerNullable, tc.Alerts, uint64`, and allow clients to get whatever
fields they're interested in from that?
----------------------------------------------------------------
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:
[email protected]
