srijeet0406 commented on a change in pull request #5512:
URL: https://github.com/apache/trafficcontrol/pull/5512#discussion_r605003125
##########
File path: traffic_ops/v4-client/crconfig.go
##########
@@ -72,15 +77,10 @@ func (to *Session) GetCRConfigNew(cdn string) ([]byte,
toclientlib.ReqInf, error
return resp.Response, reqInf, nil
}
-// SnapshotCRConfig snapshots a CDN by name.
-// Deprecated: SnapshotCRConfig will be removed in 6.0. Use
SnapshotCRConfigWithHdr.
-func (to *Session) SnapshotCRConfig(cdn string) (toclientlib.ReqInf, error) {
- return to.SnapshotCRConfigWithHdr(cdn, nil)
-}
-
-// SnapshotCDNByID snapshots a CDN by ID.
+// SnapshotCRConfigByID creates a new Snapshot for the CDN with the given ID -
+// NOT just a new CRConfig!
func (to *Session) SnapshotCRConfigByID(id int) (tc.Alerts,
toclientlib.ReqInf, error) {
Review comment:
I think we should pass in the header param here as well.
##########
File path: traffic_ops/testing/api/v4/profiles_test.go
##########
@@ -76,20 +77,21 @@ func GetTestProfilesIMS(t *testing.T) {
time := futureTime.Format(time.RFC1123)
header.Set(rfc.IfModifiedSince, time)
for _, pr := range testData.Profiles {
- _, reqInf, err := TOSession.GetProfileByNameWithHdr(pr.Name,
header)
- if err != nil {
- t.Fatalf("Expected no error, but got %v", err.Error())
- }
- if reqInf.StatusCode != http.StatusNotModified {
- t.Fatalf("Expected 304 status code, got %v",
reqInf.StatusCode)
- }
- _, reqInf, err =
TOSession.GetProfileByParameterWithHdr(pr.Parameter, header)
+ _, reqInf, err := TOSession.GetProfileByName(pr.Name, header)
if err != nil {
t.Fatalf("Expected no error, but got %v", err.Error())
}
if reqInf.StatusCode != http.StatusNotModified {
t.Fatalf("Expected 304 status code, got %v",
reqInf.StatusCode)
}
+ // TODO: Figure out how this ever worked and what it's meant to
test.
Review comment:
Seems like you're testing this above. Can this be removed?
##########
File path: traffic_ops/v4-client/deliveryservice.go
##########
@@ -32,134 +32,90 @@ import (
const (
// API_DELIVERY_SERVICES is the API path on which Traffic Ops serves
Delivery Service
// information. More specific information is typically found on
sub-paths of this.
- // See Also:
https://traffic-control-cdn.readthedocs.io/en/latest/api/v3/deliveryservices.html
APIDeliveryServices = "/deliveryservices"
// APIDeliveryServiceId is the API path on which Traffic Ops serves
information about
// a specific Delivery Service identified by an integral, unique
identifier. It is
// intended to be used with fmt.Sprintf to insert its required path
parameter (namely the ID
// of the Delivery Service of interest).
- // See Also:
https://traffic-control-cdn.readthedocs.io/en/latest/api/v3/deliveryservices_id.html
- APIDeliveryServiceID = APIDeliveryServices + "/%v"
+ APIDeliveryServiceID = APIDeliveryServices + "/%d"
// APIDeliveryServiceHealth is the API path on which Traffic Ops serves
information about
// the 'health' of a specific Delivery Service identified by an
integral, unique identifier. It is
// intended to be used with fmt.Sprintf to insert its required path
parameter (namely the ID
// of the Delivery Service of interest).
- // See Also:
https://traffic-control-cdn.readthedocs.io/en/latest/api/v3/deliveryservices_id_health.html
APIDeliveryServiceHealth = APIDeliveryServiceID + "/health"
// APIDeliveryServiceCapacity is the API path on which Traffic Ops
serves information about
// the 'capacity' of a specific Delivery Service identified by an
integral, unique identifier. It is
// intended to be used with fmt.Sprintf to insert its required path
parameter (namely the ID
// of the Delivery Service of interest).
- // See Also:
https://traffic-control-cdn.readthedocs.io/en/latest/api/v3/deliveryservices_id_capacity.html
APIDeliveryServiceCapacity = APIDeliveryServiceID + "/capacity"
// APIDeliveryServiceEligibleServers is the API path on which Traffic
Ops serves information about
// the servers which are eligible to be assigned to a specific Delivery
Service identified by an integral,
// unique identifier. It is intended to be used with fmt.Sprintf to
insert its required path parameter
// (namely the ID of the Delivery Service of interest).
- // See Also:
https://traffic-control-cdn.readthedocs.io/en/latest/api/v3/deliveryservices_id_servers_eligible.html
APIDeliveryServiceEligibleServers = APIDeliveryServiceID +
"/servers/eligible"
// APIDeliveryServicesSafeUpdate is the API path on which Traffic Ops
provides the functionality to
// update the "safe" subset of properties of a Delivery Service
identified by an integral, unique
// identifer. It is intended to be used with fmt.Sprintf to insert its
required path parameter
// (namely the ID of the Delivery Service of interest).
- // See Also:
https://traffic-control-cdn.readthedocs.io/en/latest/api/v3/deliveryservices_id_safe.html
APIDeliveryServicesSafeUpdate = APIDeliveryServiceID + "/safe"
// APIDeliveryServiceXMLIDSSLKeys is the API path on which Traffic Ops
serves information about
// and functionality relating to the SSL keys used by a Delivery
Service identified by its XMLID. It is
// intended to be used with fmt.Sprintf to insert its required path
parameter (namely the XMLID
// of the Delivery Service of interest).
- // See Also:
https://traffic-control-cdn.readthedocs.io/en/latest/api/v3/deliveryservices_xmlid_xmlid_sslkeys.html
APIDeliveryServiceXMLIDSSLKeys = APIDeliveryServices +
"/xmlId/%s/sslkeys"
// APIDeliveryServiceGenerateSSLKeys is the API path on which Traffic
Ops will generate new SSL keys
- // See Also:
https://traffic-control-cdn.readthedocs.io/en/latest/api/v3/deliveryservices_sslkeys_generate.html
APIDeliveryServiceGenerateSSLKeys = APIDeliveryServices +
"/sslkeys/generate"
// APIDeliveryServiceURISigningKeys is the API path on which Traffic
Ops serves information
// about and functionality relating to the URI-signing keys used by a
Delivery Service identified
// by its XMLID. It is intended to be used with fmt.Sprintf to insert
its required path parameter
// (namely the XMLID of the Delivery Service of interest).
- // See Also:
https://traffic-control-cdn.readthedocs.io/en/latest/api/v3/deliveryservices_xmlid_urisignkeys.html
APIDeliveryServicesURISigningKeys = APIDeliveryServices +
"/%s/urisignkeys"
// APIDeliveryServicesURLSigKeys is the API path on which Traffic Ops
serves information
// about and functionality relating to the URL-signing keys used by a
Delivery Service identified
// by its XMLID. It is intended to be used with fmt.Sprintf to insert
its required path parameter
// (namely the XMLID of the Delivery Service of interest).
- // See Also:
https://traffic-control-cdn.readthedocs.io/en/latest/api/v3/deliveryservices_xmlid_xmlid_urlkeys.html
APIDeliveryServicesURLSigKeys = APIDeliveryServices +
"/xmlid/%s/urlkeys"
// APIDeliveryServicesRegexes is the API path on which Traffic Ops
serves Delivery Service
// 'regex' (Regular Expression) information.
- // See Also:
https://traffic-control-cdn.readthedocs.io/en/latest/api/v3/deliveryservices_regexes.html
APIDeliveryServicesRegexes = "/deliveryservices_regexes"
// APIServerDeliveryServices is the API path on which Traffic Ops
serves functionality
// related to the associations a specific server and its assigned
Delivery Services. It is
// intended to be used with fmt.Sprintf to insert its required path
parameter (namely the ID
// of the server of interest).
- // See Also:
https://traffic-control-cdn.readthedocs.io/en/latest/api/v3/servers_id_deliveryservices.html
APIServerDeliveryServices = "/servers/%d/deliveryservices"
// APIDeliveryServiceServer is the API path on which Traffic Ops serves
functionality related
// to the associations between Delivery Services and their assigned
Server(s).
- // See Also:
https://traffic-control-cdn.readthedocs.io/en/latest/api/v3/deliveryserviceserver.html
APIDeliveryServiceServer = "/deliveryserviceserver"
// APIDeliveryServicesServers is the API path on which Traffic Ops
serves functionality related
// to the associations between a Delivery Service and its assigned
Server(s).
- // See Also:
https://traffic-control-cdn.readthedocs.io/en/latest/api/v3/deliveryservices_xmlid_servers.html
APIDeliveryServicesServers = "/deliveryservices/%s/servers"
)
-func (to *Session) GetDeliveryServicesByServerV30WithHdr(id int, header
http.Header) ([]tc.DeliveryServiceNullableV30, toclientlib.ReqInf, error) {
- var data tc.DeliveryServicesResponseV30
- reqInf, err := to.get(fmt.Sprintf(APIServerDeliveryServices, id),
header, &data)
- return data.Response, reqInf, err
-}
-
-// GetDeliveryServicesByServer returns all of the (tenant-visible) Delivery
Services assigned to
-// the server identified by the integral, unique identifier 'id'.
-//
-// Warning: This method coerces its returned data into an APIv1.5 format.
-//
-// Deprecated: Please used versioned library imports in the future, and
-// versioned methods, specifically, for API v3.0 - in this case,
-// GetDeliveryServicesByServerV30WithHdr.
-func (to *Session) GetDeliveryServicesByServer(id int)
([]tc.DeliveryServiceNullable, toclientlib.ReqInf, error) {
- return to.GetDeliveryServicesByServerWithHdr(id, nil)
-}
-
-func (to *Session) GetDeliveryServicesByServerWithHdr(id int, header
http.Header) ([]tc.DeliveryServiceNullable, toclientlib.ReqInf, error) {
- var data tc.DeliveryServicesNullableResponse
-
+// GetDeliveryServicesByServer retrieves all Delivery Services assigned to the
+// server with the given ID.
+func (to *Session) GetDeliveryServicesByServer(id int, header http.Header)
([]tc.DeliveryServiceV4, toclientlib.ReqInf, error) {
+ var data tc.DeliveryServicesResponseV4
reqInf, err := to.get(fmt.Sprintf(APIServerDeliveryServices, id),
header, &data)
return data.Response, reqInf, err
}
-// GetDeliveryServicesV30WithHdr returns all (tenant-visible) Delivery
Services that
+// GetDeliveryServices returns all (tenant-visible) Delivery Services that
// satisfy the passed query string parameters. See the API documentation for
// information on the available parameters.
-func (to *Session) GetDeliveryServicesV30WithHdr(header http.Header, params
url.Values) ([]tc.DeliveryServiceNullableV30, toclientlib.ReqInf, error) {
- uri := APIDeliveryServices
- if params != nil {
- uri += "?" + params.Encode()
- }
- var data tc.DeliveryServicesResponseV30
- reqInf, err := to.get(uri, header, &data)
- return data.Response, reqInf, err
-}
-
-// GetDeliveryServicesV4 returns all (tenant-visible) Delivery Services that
-// satisfy the passed query string parameters. See the API documentation for
-// information on the available parameters.
-func (to *Session) GetDeliveryServicesV4(header http.Header, params
url.Values) ([]tc.DeliveryServiceV4, toclientlib.ReqInf, error) {
+func (to *Session) GetDeliveryServices(header http.Header, params url.Values)
([]tc.DeliveryServiceV4, toclientlib.ReqInf, error) {
Review comment:
I think I mentioned this before, but could you make `header` the last
argument?
##########
File path: traffic_ops/v4-client/user.go
##########
@@ -27,122 +27,94 @@ import (
"github.com/apache/trafficcontrol/traffic_ops/toclientlib"
)
-func (to *Session) GetUsersWithHdr(header http.Header) ([]tc.User,
toclientlib.ReqInf, error) {
+// GetUsers retrieves all (Tenant-accessible) Users stored in Traffic Ops.
+func (to *Session) GetUsers(header http.Header) ([]tc.User,
toclientlib.ReqInf, error) {
data := tc.UsersResponse{}
route := "/users"
inf, err := to.get(route, header, &data)
return data.Response, inf, err
}
-// GetUsers returns all users accessible from current user
-// Deprecated: GetUsers will be removed in 6.0. Use GetUsersWithHdr.
-func (to *Session) GetUsers() ([]tc.User, toclientlib.ReqInf, error) {
- return to.GetUsersWithHdr(nil)
-}
-
-func (to *Session) GetUsersByRoleWithHdr(roleName string, header http.Header)
([]tc.User, toclientlib.ReqInf, error) {
+// GetUsersByRole retrieves all (Tenant-accessible) with the Role that has the
+// given Name.
+func (to *Session) GetUsersByRole(roleName string, header http.Header)
([]tc.User, toclientlib.ReqInf, error) {
data := tc.UsersResponse{}
route := "/users?role=" + url.QueryEscape(roleName)
inf, err := to.get(route, header, &data)
return data.Response, inf, err
}
-// GetUsersByRole returns all users accessible from current user for a given
role
-// Deprecated: GetUsersByRole will be removed in 6.0. Use
GetUsersByRoleWithHdr.
-func (to *Session) GetUsersByRole(roleName string) ([]tc.User,
toclientlib.ReqInf, error) {
- return to.GetUsersByRoleWithHdr(roleName, nil)
-}
-
-func (to *Session) GetUserByIDWithHdr(id int, header http.Header) ([]tc.User,
toclientlib.ReqInf, error) {
+// GetUserByID retrieves the User with the given ID.
+func (to *Session) GetUserByID(id int, header http.Header) ([]tc.User,
toclientlib.ReqInf, error) {
data := tc.UsersResponse{}
route := fmt.Sprintf("/users/%d", id)
inf, err := to.get(route, header, &data)
return data.Response, inf, err
}
-// Deprecated: GetUserByID will be removed in 6.0. Use GetUserByIDWithHdr.
-func (to *Session) GetUserByID(id int) ([]tc.User, toclientlib.ReqInf, error) {
- return to.GetUserByIDWithHdr(id, nil)
-}
-
-func (to *Session) GetUserByUsernameWithHdr(username string, header
http.Header) ([]tc.User, toclientlib.ReqInf, error) {
+// GetUserByUsername retrieves the User with the given Username.
+func (to *Session) GetUserByUsername(username string, header http.Header)
([]tc.User, toclientlib.ReqInf, error) {
data := tc.UsersResponse{}
route := "/users?username=" + url.QueryEscape(username)
inf, err := to.get(route, header, &data)
return data.Response, inf, err
}
-// Deprecated: GetUserByUsername will be removed in 6.0. Use
GetUserByUsernameWithHdr.
-func (to *Session) GetUserByUsername(username string) ([]tc.User,
toclientlib.ReqInf, error) {
- return to.GetUserByUsernameWithHdr(username, nil)
-}
-
-func (to *Session) GetUserCurrentWithHdr(header http.Header) (*tc.UserCurrent,
toclientlib.ReqInf, error) {
+// GetUserCurrent retrieves the currently authenticated User.
+func (to *Session) GetUserCurrent(header http.Header) (tc.UserCurrent,
toclientlib.ReqInf, error) {
route := `/user/current`
resp := tc.UserCurrentResponse{}
reqInf, err := to.get(route, header, &resp)
- if err != nil {
- return nil, reqInf, err
- }
- return &resp.Response, reqInf, nil
-}
-
-// GetUserCurrent gets information about the current user
-// Deprecated: GetUserCurrent will be removed in 6.0. Use
GetUserCurrentWithHdr.
-func (to *Session) GetUserCurrent() (*tc.UserCurrent, toclientlib.ReqInf,
error) {
- return to.GetUserCurrentWithHdr(nil)
+ return resp.Response, reqInf, err
}
// UpdateCurrentUser replaces the current user data with the provided tc.User
structure.
-func (to *Session) UpdateCurrentUser(u tc.User) (*tc.UpdateUserResponse,
toclientlib.ReqInf, error) {
+func (to *Session) UpdateCurrentUser(u tc.User) (tc.UpdateUserResponse,
toclientlib.ReqInf, error) {
user := struct {
User tc.User `json:"user"`
}{u}
var clientResp tc.UpdateUserResponse
reqInf, err := to.put("/user/current", user, nil, &clientResp)
- return &clientResp, reqInf, err
+ return clientResp, reqInf, err
}
-// CreateUser creates a user
-func (to *Session) CreateUser(user *tc.User) (*tc.CreateUserResponse,
toclientlib.ReqInf, error) {
+// CreateUser creates the given user
+func (to *Session) CreateUser(user tc.User) (tc.CreateUserResponse,
toclientlib.ReqInf, error) {
if user.TenantID == nil && user.Tenant != nil {
- tenant, _, err := to.TenantByNameWithHdr(*user.Tenant, nil)
+ tenant, _, err := to.GetTenantByName(*user.Tenant, nil)
if err != nil {
- return nil, toclientlib.ReqInf{}, err
- }
- if tenant == nil {
- return nil, toclientlib.ReqInf{}, errors.New("no tenant
with name " + *user.Tenant)
+ return tc.CreateUserResponse{}, toclientlib.ReqInf{},
err
}
user.TenantID = &tenant.ID
}
if user.RoleName != nil && *user.RoleName != "" {
- roles, _, _, err := to.GetRoleByNameWithHdr(*user.RoleName, nil)
+ roles, _, _, err := to.GetRoleByName(*user.RoleName, nil)
if err != nil {
- return nil, toclientlib.ReqInf{}, err
+ return tc.CreateUserResponse{}, toclientlib.ReqInf{},
err
}
if len(roles) == 0 || roles[0].ID == nil {
- return nil, toclientlib.ReqInf{}, errors.New("no role
with name " + *user.RoleName)
+ return tc.CreateUserResponse{}, toclientlib.ReqInf{},
errors.New("no role with name " + *user.RoleName)
}
user.Role = roles[0].ID
}
route := "/users"
var clientResp tc.CreateUserResponse
reqInf, err := to.post(route, user, nil, &clientResp)
- return &clientResp, reqInf, err
+ return clientResp, reqInf, err
}
-// UpdateUserByID updates user with the given id
-func (to *Session) UpdateUserByID(id int, u *tc.User) (*tc.UpdateUserResponse,
toclientlib.ReqInf, error) {
+// UpdateUser replaces the User identified by 'id' with the one provided.
+func (to *Session) UpdateUser(id int, u tc.User) (tc.UpdateUserResponse,
toclientlib.ReqInf, error) {
Review comment:
Same comment here about passing in the header
##########
File path: traffic_ops/testing/api/v4/profiles_test.go
##########
@@ -396,26 +404,26 @@ func DeleteTestProfiles(t *testing.T) {
profileID := resp[0].ID
// query by name does not retrieve associated parameters. But
query by id does.
- resp, _, err = TOSession.GetProfileByID(profileID)
+ resp, _, err = TOSession.GetProfileByID(profileID, nil)
if err != nil {
t.Errorf("cannot GET Profile by id: %v - %v", err, resp)
}
// delete any profile_parameter associations first
// the parameter is what's being deleted, but the delete is
cascaded to profile_parameter
for _, param := range resp[0].Parameters {
Review comment:
Could you add a `len` check for `resp`?
##########
File path: traffic_ops/testing/api/v4/cdnfederations_test.go
##########
@@ -147,21 +147,21 @@ func SortTestCDNFederations(t *testing.T) {
func UpdateTestCDNFederations(t *testing.T) {
for _, id := range fedIDs {
- fed, _, err := TOSession.GetCDNFederationsByID("foo", id)
+ fed, _, err := TOSession.GetCDNFederationsByID("foo", id, nil)
if err != nil {
t.Errorf("cannot GET federation by id: %v", err)
}
expectedCName := "new.cname."
fed.Response[0].CName = &expectedCName
- resp, _, err :=
TOSession.UpdateCDNFederationsByID(fed.Response[0], "foo", id)
+ resp, _, err := TOSession.UpdateCDNFederation(fed.Response[0],
"foo", id, nil)
Review comment:
Could you add a nil check on `fed` and a len check on `fed.Response`?
##########
File path: traffic_ops/v4-client/server_update_status.go
##########
@@ -25,9 +25,10 @@ import (
"github.com/apache/trafficcontrol/traffic_ops/toclientlib"
)
-// UpdateServerStatus updates a server's status and returns the response.
+// UpdateServerStatus updates the Status of the server identified by
+// 'serverID'.
func (to *Session) UpdateServerStatus(serverID int, req tc.ServerPutStatus)
(*tc.Alerts, toclientlib.ReqInf, error) {
Review comment:
You could pass in the `header` here as well.
--
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]
