You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by da...@apache.org on 2018/07/04 02:39:38 UTC
[trafficcontrol] 13/15: documentation cleanup and code
simplifications
This is an automated email from the ASF dual-hosted git repository.
dangogh pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
commit 2c495a9a05f20aa99604f3394f13e777233db64d
Author: Dylan Volz <Dy...@comcast.com>
AuthorDate: Mon Jul 2 07:31:42 2018 -0600
documentation cleanup and code simplifications
---
.../traffic_ops_golang/api/shared_handlers.go | 29 +++-------
traffic_ops/traffic_ops_golang/asn/asns.go | 66 +++++++++++-----------
traffic_ops/traffic_ops_golang/asn/asns_test.go | 8 +--
.../deliveryservice/deliveryservicesv12.go | 6 +-
.../deliveryservice/deliveryservicesv13.go | 18 ++----
.../deliveryservice/request/requests.go | 3 +-
traffic_ops/traffic_ops_golang/routes.go | 2 +-
7 files changed, 54 insertions(+), 78 deletions(-)
diff --git a/traffic_ops/traffic_ops_golang/api/shared_handlers.go b/traffic_ops/traffic_ops_golang/api/shared_handlers.go
index b2b6465..9beee6e 100644
--- a/traffic_ops/traffic_ops_golang/api/shared_handlers.go
+++ b/traffic_ops/traffic_ops_golang/api/shared_handlers.go
@@ -102,10 +102,7 @@ func GetCombinedParams(r *http.Request) (map[string]string, error) {
return combinedParams, nil
}
-//decodes and validates a pointer to a struct implementing the Validator interface
-// we lose the ability to unmarshal the struct if a struct implementing the interface is passed in,
-// because when when it is de-referenced it is a pointer to an interface. A new copy is created so that
-// there are no issues with concurrent goroutines
+// decodeAndValidateRequestBody decodes and validates a pointer to a struct implementing the Validator interface
func decodeAndValidateRequestBody(r *http.Request, v Validator) error {
defer r.Body.Close()
@@ -115,7 +112,7 @@ func decodeAndValidateRequestBody(r *http.Request, v Validator) error {
return v.Validate()
}
-//this creates a handler function from the pointer to a struct implementing the Reader interface
+// ReadHandler creates a handler function from the pointer to a struct implementing the Reader interface
// this handler retrieves the user from the context
// combines the path and query parameters
// produces the proper status code based on the error code returned
@@ -162,7 +159,7 @@ func ReadHandler(typeFactory CRUDFactory) http.HandlerFunc {
}
}
-//this creates a handler function from the pointer to a struct implementing the Reader interface
+// ReadOnlyHandler creates a handler function from the pointer to a struct implementing the Reader interface
// this handler retrieves the user from the context
// combines the path and query parameters
// produces the proper status code based on the error code returned
@@ -208,8 +205,7 @@ func ReadOnlyHandler(typeFactory func(reqInfo *APIInfo) Reader) http.HandlerFunc
}
}
-//this creates a handler function from the pointer to a struct implementing the Updater interface
-//it must be immediately assigned to a local variable
+// UpdateHandler creates a handler function from the pointer to a struct implementing the Updater interface
// this generic handler encapsulates the logic for handling:
// *fetching the id from the path parameter
// *current user
@@ -245,8 +241,6 @@ func UpdateHandler(typeFactory CRUDFactory) http.HandlerFunc {
return
}
- //create local instance of the shared typeRef pointer
- //no operations should be made on the typeRef
//decode the body and validate the request struct
err = decodeAndValidateRequestBody(r, u)
if err != nil {
@@ -302,8 +296,7 @@ func UpdateHandler(typeFactory CRUDFactory) http.HandlerFunc {
return
}
//auditing here
- err = CreateChangeLog(ApiChange, Updated, u, inf.User, inf.Tx)
- if err != nil {
+ if err := CreateChangeLog(ApiChange, Updated, u, inf.User, inf.Tx); err != nil {
HandleErr(w, r, http.StatusInternalServerError, tc.DBError, errors.New("inserting changelog: "+err.Error()))
return
}
@@ -325,8 +318,7 @@ func UpdateHandler(typeFactory CRUDFactory) http.HandlerFunc {
}
}
-//this creates a handler function from the pointer to a struct implementing the Deleter interface
-//it must be immediately assigned to a local variable
+// DeleteHandler creates a handler function from the pointer to a struct implementing the Deleter interface
// this generic handler encapsulates the logic for handling:
// *fetching the id from the path parameter
// *current user
@@ -392,8 +384,7 @@ func DeleteHandler(typeFactory CRUDFactory) http.HandlerFunc {
}
//audit here
log.Debugf("changelog for delete on object")
- err = CreateChangeLog(ApiChange, Deleted, d, inf.User, inf.Tx)
- if err != nil {
+ if err = CreateChangeLog(ApiChange, Deleted, d, inf.User, inf.Tx); err != nil {
HandleErr(w, r, http.StatusInternalServerError, tc.DBError, errors.New("inserting changelog: "+err.Error()))
return
}
@@ -414,8 +405,7 @@ func DeleteHandler(typeFactory CRUDFactory) http.HandlerFunc {
}
}
-//this creates a handler function from the pointer to a struct implementing the Creator interface
-//it must be immediately assigned to a local variable
+// CreateHandler creates a handler function from the pointer to a struct implementing the Creator interface
// this generic handler encapsulates the logic for handling:
// *fetching the id from the path parameter
// *current user
@@ -464,8 +454,7 @@ func CreateHandler(typeConstructor CRUDFactory) http.HandlerFunc {
return
}
- err = CreateChangeLog(ApiChange, Created, i, inf.User, inf.Tx)
- if err != nil {
+ if err = CreateChangeLog(ApiChange, Created, i, inf.User, inf.Tx); err != nil {
HandleErr(w, r, http.StatusInternalServerError, tc.DBError, errors.New("inserting changelog: "+err.Error()))
return
}
diff --git a/traffic_ops/traffic_ops_golang/asn/asns.go b/traffic_ops/traffic_ops_golang/asn/asns.go
index 3e790a7..f2a9657 100644
--- a/traffic_ops/traffic_ops_golang/asn/asns.go
+++ b/traffic_ops/traffic_ops_golang/asn/asns.go
@@ -159,45 +159,43 @@ func (asn *TOASNV11) Read(parameters map[string]string) ([]interface{}, []error,
}
// V11ReadAll implements the asns 1.1 route, which is different from the 1.1 route for a single ASN and from 1.2+ routes, in that it wraps the content in an additional "asns" object.
-func V11ReadAll() http.HandlerFunc {
- return func(w http.ResponseWriter, r *http.Request) {
- handleErrs := tc.GetHandleErrorsFunc(w, r)
-
- inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
- if userErr != nil || sysErr != nil {
- api.HandleErr(w, r, errCode, userErr, sysErr)
- return
- }
- defer inf.Close()
+func V11ReadAll(w http.ResponseWriter, r *http.Request) {
+ handleErrs := tc.GetHandleErrorsFunc(w, r)
- params, err := api.GetCombinedParams(r)
- if err != nil {
- handleErrs(http.StatusInternalServerError, err)
- return
- }
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
- asns, errs, errType := read(inf.Tx, params)
- if len(errs) > 0 {
- tc.HandleErrorsWithType(errs, errType, handleErrs)
- return
- }
- *inf.CommitTx = true
- resp := struct {
- Response struct {
- ASNs []tc.ASNNullable `json:"asns"`
- } `json:"response"`
- }{Response: struct {
+ params, err := api.GetCombinedParams(r)
+ if err != nil {
+ handleErrs(http.StatusInternalServerError, err)
+ return
+ }
+
+ asns, errs, errType := read(inf.Tx, params)
+ if len(errs) > 0 {
+ tc.HandleErrorsWithType(errs, errType, handleErrs)
+ return
+ }
+ *inf.CommitTx = true
+ resp := struct {
+ Response struct {
ASNs []tc.ASNNullable `json:"asns"`
- }{ASNs: asns}}
+ } `json:"response"`
+ }{Response: struct {
+ ASNs []tc.ASNNullable `json:"asns"`
+ }{ASNs: asns}}
- respBts, err := json.Marshal(resp)
- if err != nil {
- handleErrs(http.StatusInternalServerError, err)
- return
- }
- w.Header().Set("Content-Type", "application/json")
- fmt.Fprintf(w, "%s", respBts)
+ respBts, err := json.Marshal(resp)
+ if err != nil {
+ handleErrs(http.StatusInternalServerError, err)
+ return
}
+ w.Header().Set("Content-Type", "application/json")
+ fmt.Fprintf(w, "%s", respBts)
}
func read(tx *sqlx.Tx, parameters map[string]string) ([]tc.ASNNullable, []error, tc.ApiErrorType) {
diff --git a/traffic_ops/traffic_ops_golang/asn/asns_test.go b/traffic_ops/traffic_ops_golang/asn/asns_test.go
index 3c1c159..00963f0 100644
--- a/traffic_ops/traffic_ops_golang/asn/asns_test.go
+++ b/traffic_ops/traffic_ops_golang/asn/asns_test.go
@@ -35,7 +35,7 @@ import (
)
func getTestASNs() []tc.ASNNullable {
- ASNs := []tc.ASNNullable{}
+ asns := []tc.ASNNullable{}
i := 1
c := "Yukon"
testCase := tc.ASNNullable{
@@ -45,13 +45,13 @@ func getTestASNs() []tc.ASNNullable {
ID: &i,
LastUpdated: &tc.TimeNoMod{Time: time.Now()},
}
- ASNs = append(ASNs, testCase)
+ asns = append(asns, testCase)
testCase2 := testCase
*testCase2.ASN = 2
- ASNs = append(ASNs, testCase2)
+ asns = append(asns, testCase2)
- return ASNs
+ return asns
}
func TestGetASNs(t *testing.T) {
diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv12.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv12.go
index e1876bb..a84a14f 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv12.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv12.go
@@ -174,7 +174,7 @@ func (ds *TODeliveryServiceV12) Validate() error {
return ds.DeliveryServiceNullableV12.Validate(ds.ReqInfo.Tx.Tx)
}
-// unimplemented, needed to satisfy CRUDer, since the framework doesn't allow a create to return an array of one
+// Create is unimplemented, needed to satisfy CRUDer, since the framework doesn't allow a create to return an array of one
func (ds *TODeliveryServiceV12) Create() (error, tc.ApiErrorType) {
return errors.New("The Create method is not implemented"), http.StatusNotImplemented
}
@@ -228,7 +228,7 @@ func (ds *TODeliveryServiceV12) Read(params map[string]string) ([]interface{}, [
return returnable, nil, tc.NoError
}
-//The DeliveryService implementation of the Deleter interface
+//Delete is the DeliveryService implementation of the Deleter interface
//all implementations of Deleter should use transactions and return the proper errorType
func (ds *TODeliveryServiceV12) Delete() (error, tc.ApiErrorType) {
log.Debugln("TODeliveryServiceV12.Delete calling id '%v' xmlid '%v'\n", ds.ID, ds.XMLID)
@@ -291,7 +291,7 @@ func (ds *TODeliveryServiceV12) Delete() (error, tc.ApiErrorType) {
return nil, tc.NoError
}
-// unimplemented, needed to satisfy CRUDer, since the framework doesn't allow an update to return an array of one
+// Update is unimplemented, needed to satisfy CRUDer, since the framework doesn't allow an update to return an array of one
func (ds *TODeliveryServiceV12) Update() (error, tc.ApiErrorType) {
return errors.New("The Update method is not implemented"), http.StatusNotImplemented
}
diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv13.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv13.go
index d7b0f7a..efc385d 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv13.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv13.go
@@ -93,20 +93,11 @@ func (ds *TODeliveryServiceV13) Validate() error {
return ds.DeliveryServiceNullableV13.Validate(ds.ReqInfo.Tx.Tx)
}
-// unimplemented, needed to satisfy CRUDer, since the framework doesn't allow a create to return an array of one
+// Create is unimplemented, needed to satisfy CRUDer, since the framework doesn't allow a create to return an array of one
func (ds *TODeliveryServiceV13) Create() (error, tc.ApiErrorType) {
return errors.New("The Create method is not implemented"), http.StatusNotImplemented
}
-// Create implements the Creator interface.
-//all implementations of Creator should use transactions and return the proper errorType
-//ParsePQUniqueConstraintError is used to determine if a ds with conflicting values exists
-//if so, it will return an errorType of DataConflict and the type should be appended to the
-//generic error message returned
-//The insert sql returns the id and lastUpdated values of the newly inserted ds and have
-//to be added to the struct
-// func (ds *TODeliveryServiceV13) Create(db *sqlx.Tx, user auth.CurrentUser) (error, tc.ApiErrorType) { //
-//
// TODO allow users to post names (type, cdn, etc) and get the IDs from the names. This isn't trivial to do in a single query, without dynamically building the entire insert query, and ideally inserting would be one query. But it'd be much more convenient for users. Alternatively, remove IDs from the database entirely and use real candidate keys.
func CreateV13() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
@@ -148,8 +139,7 @@ func CreateV13() http.HandlerFunc {
}
}
-// create creates the given ds in the database, and returns the DS with its id and other fields created on insert set. On error, the HTTP status cdoe, user error, and system error are returned. The status code SHOULD NOT be used, if both errors are nil.
-
+// create creates the given ds in the database, and returns the DS with its id and other fields created on insert set. On error, the HTTP status code, user error, and system error are returned. The status code SHOULD NOT be used, if both errors are nil.
func create(tx *sql.Tx, cfg config.Config, user *auth.CurrentUser, ds tc.DeliveryServiceNullableV13) (tc.DeliveryServiceNullableV13, int, error, error) {
// TODO change DeepCachingType to implement sql.Valuer and sql.Scanner, so sqlx struct scan can be used.
deepCachingType := tc.DeepCachingType("").String()
@@ -377,7 +367,7 @@ func getTypeFromID(id int, tx *sql.Tx) (tc.DSType, error) {
return tc.DSTypeFromString(name), nil
}
-// unimplemented, needed to satisfy CRUDer, since the framework doesn't allow an update to return an array of one
+// Update is unimplemented, needed to satisfy CRUDer, since the framework doesn't allow an update to return an array of one
func (ds *TODeliveryServiceV13) Update() (error, tc.ApiErrorType) {
return errors.New("The Update method is not implemented"), http.StatusNotImplemented
}
@@ -567,7 +557,7 @@ func update(tx *sql.Tx, cfg config.Config, user *auth.CurrentUser, ds *tc.Delive
return *ds, http.StatusOK, nil, nil
}
-//The DeliveryService implementation of the Deleter interface
+// Delete is the DeliveryService implementation of the Deleter interface
//all implementations of Deleter should use transactions and return the proper errorType
func (ds *TODeliveryServiceV13) Delete() (error, tc.ApiErrorType) {
return ds.V12().Delete()
diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go b/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go
index 9dccd08..f367480 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go
@@ -35,7 +35,7 @@ import (
"github.com/lib/pq"
)
-//we need a type alias to define functions on
+// TODeliveryServiceRequest is the type alias to define functions on
type TODeliveryServiceRequest struct {
ReqInfo *api.APIInfo `json:"-"`
tc.DeliveryServiceRequestNullable
@@ -52,7 +52,6 @@ func (req TODeliveryServiceRequest) GetKeyFieldsInfo() []api.KeyFieldInfo {
return []api.KeyFieldInfo{{"id", api.GetIntKey}}
}
-//Implementation of the Identifier, Validator interface functions
func (req TODeliveryServiceRequest) GetKeys() (map[string]interface{}, bool) {
if req.ID == nil {
return map[string]interface{}{"id": 0}, false
diff --git a/traffic_ops/traffic_ops_golang/routes.go b/traffic_ops/traffic_ops_golang/routes.go
index d26503d..9b1d696 100644
--- a/traffic_ops/traffic_ops_golang/routes.go
+++ b/traffic_ops/traffic_ops_golang/routes.go
@@ -85,7 +85,7 @@ func Routes(d ServerData) ([]Route, []RawRoute, http.Handler, error) {
//ASN: CRUD
{1.2, http.MethodGet, `asns/?(\.json)?$`, api.ReadHandler(asn.GetTypeSingleton()), auth.PrivLevelReadOnly, Authenticated, nil},
- {1.1, http.MethodGet, `asns/?(\.json)?$`, asn.V11ReadAll(), auth.PrivLevelReadOnly, Authenticated, nil},
+ {1.1, http.MethodGet, `asns/?(\.json)?$`, asn.V11ReadAll, auth.PrivLevelReadOnly, Authenticated, nil},
{1.1, http.MethodGet, `asns/{id}$`, api.ReadHandler(asn.GetTypeSingleton()), auth.PrivLevelReadOnly, Authenticated, nil},
{1.1, http.MethodPut, `asns/{id}$`, api.UpdateHandler(asn.GetTypeSingleton()), auth.PrivLevelOperations, Authenticated, nil},
{1.1, http.MethodPost, `asns/?$`, api.CreateHandler(asn.GetTypeSingleton()), auth.PrivLevelOperations, Authenticated, nil},