[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5747: Client consistency

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

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



##########
File path: traffic_ops/v4-client/servercapability.go
##########
@@ -13,69 +15,49 @@
    limitations under the License.
 */
 
-package client
-
 import (
-	"fmt"
-	"net/http"
 	"net/url"
 
 	"github.com/apache/trafficcontrol/lib/go-tc"
 	"github.com/apache/trafficcontrol/traffic_ops/toclientlib"
 )
 
-const (
-	// APIServerCapabilities is the full path to the /server_capabilities API
-	// endpoint.
-	APIServerCapabilities = "/server_capabilities"
-)
+// apiServerCapabilities is the full path to the /server_capabilities API
+// endpoint.
+const apiServerCapabilities = "/server_capabilities"
 
 // CreateServerCapability creates the given Server Capability.
-func (to *Session) CreateServerCapability(sc tc.ServerCapability) (*tc.ServerCapabilityDetailResponse, toclientlib.ReqInf, error) {
+func (to *Session) CreateServerCapability(sc tc.ServerCapability, opts RequestOptions) (tc.ServerCapabilityDetailResponse, toclientlib.ReqInf, error) {
 	var scResp tc.ServerCapabilityDetailResponse
-	reqInf, err := to.post(APIServerCapabilities, sc, nil, &scResp)
-	if err != nil {
-		return nil, reqInf, err
-	}
-	return &scResp, reqInf, nil
+	reqInf, err := to.post(apiServerCapabilities, opts, sc, &scResp)
+	return scResp, reqInf, err
 }
 
 // GetServerCapabilities returns all the Server Capabilities in Traffic Ops.
-func (to *Session) GetServerCapabilities(header http.Header) ([]tc.ServerCapability, toclientlib.ReqInf, error) {
+func (to *Session) GetServerCapabilities(opts RequestOptions) (tc.ServerCapabilitiesResponse, toclientlib.ReqInf, error) {
 	var data tc.ServerCapabilitiesResponse
-	reqInf, err := to.get(APIServerCapabilities, header, &data)
-	return data.Response, reqInf, err
-}
-
-// GetServerCapability retrieves the Server Capability with the given Name.
-func (to *Session) GetServerCapability(name string, header http.Header) (*tc.ServerCapability, toclientlib.ReqInf, error) {
-	reqURL := fmt.Sprintf("%s?name=%s", APIServerCapabilities, url.QueryEscape(name))
-	var data tc.ServerCapabilitiesResponse
-	reqInf, err := to.get(reqURL, header, &data)
-	if err != nil {
-		return nil, reqInf, err
-	}
-	if len(data.Response) == 1 {
-		return &data.Response[0], reqInf, nil
-	}
-	return nil, reqInf, fmt.Errorf("expected one server capability in response, instead got: %+v", data.Response)
+	reqInf, err := to.get(apiServerCapabilities, opts, &data)
+	return data, reqInf, err
 }
 
 // UpdateServerCapability updates a Server Capability by name.
-func (to *Session) UpdateServerCapability(name string, sc *tc.ServerCapability, header http.Header) (*tc.ServerCapability, toclientlib.ReqInf, error) {
-	route := fmt.Sprintf("%s?name=%s", APIServerCapabilities, url.QueryEscape(name))
-	var data tc.ServerCapability
-	reqInf, err := to.put(route, sc, header, &data)
-	if err != nil {
-		return nil, reqInf, err
+func (to *Session) UpdateServerCapability(name string, sc tc.ServerCapability, opts RequestOptions) (tc.ServerCapabilityDetailResponse, toclientlib.ReqInf, error) {
+	if opts.QueryParameters == nil {
+		opts.QueryParameters = url.Values{}
 	}
-	return &data, reqInf, nil
+	opts.QueryParameters.Set("name", name)
+	var data tc.ServerCapabilityDetailResponse
+	reqInf, err := to.put(apiServerCapabilities, opts, sc, &data)
+	return data, reqInf, err
 }
 
 // DeleteServerCapability deletes the given server capability by name.
-func (to *Session) DeleteServerCapability(name string) (tc.Alerts, toclientlib.ReqInf, error) {
-	reqURL := fmt.Sprintf("%s?name=%s", APIServerCapabilities, url.QueryEscape(name))
+func (to *Session) DeleteServerCapability(name string, opts RequestOptions) (tc.Alerts, toclientlib.ReqInf, error) {
+	if opts.QueryParameters == nil {
+		opts.QueryParameters = url.Values{}
+	}
+	opts.QueryParameters.Set("name", name)

Review comment:
       I went back and forth on whether or not to add that sort of thing several times during this PR




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