This is an automated email from the ASF dual-hosted git repository.
rawlin pushed a commit to branch 4.1.x
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
The following commit(s) were added to refs/heads/4.1.x by this push:
new 2964a3d Add and satisfy OptionsDelete API test for DELETE
/regions?name={{name}} (#4707) (#4732)
2964a3d is described below
commit 2964a3d2db6a060c9473dc86918a59a783c19833
Author: Rawlin Peters <[email protected]>
AuthorDate: Thu May 28 10:44:24 2020 -0600
Add and satisfy OptionsDelete API test for DELETE /regions?name={{name}}
(#4707) (#4732)
* Add an API test for DELETE /regions?name={{name}}
* No changelog entry
* Combine OptionsDeleter and HasDeleteKeyOptions interfaces
Rename variables:
- hasDeleteKeyOption() -> checkIfOptionsDeleter()
- deleteKeyOptionsExists isOptionsDeleter
* Do not bail if the type is an options deleter
* Add new DeleteHandler() logic to DeprecatedDeleteHandler()
* Remove unnecessary "else" clause
* Add "name" param to GetKeys() and SetKeys()
* Set object keys from DeleteKeyOptions() or GetKeyFieldsInfo() depending
on whether the object is an options deleter
* Add audit log assertions to the API test for deleting a region by name
* No need to return "name" param from GetKeys()
* DeprecatedDeleteHandler: Set object keys from DeleteKeyOptions() or
GetKeyFieldsInfo() depending on whether the object is an options deleter
* kf.Field -> key
(cherry picked from commit 56d09e6d08912dbbb90a6fa26c3095b69750fb28)
Co-authored-by: Zach Hoffman <[email protected]>
---
.../traffic_ops_golang/api/shared_handlers.go | 92 +++++++++++++++++-----
.../traffic_ops_golang/api/shared_interfaces.go | 5 +-
traffic_ops/traffic_ops_golang/region/regions.go | 9 ++-
3 files changed, 79 insertions(+), 27 deletions(-)
diff --git a/traffic_ops/traffic_ops_golang/api/shared_handlers.go
b/traffic_ops/traffic_ops_golang/api/shared_handlers.go
index 846822e..5847a81 100644
--- a/traffic_ops/traffic_ops_golang/api/shared_handlers.go
+++ b/traffic_ops/traffic_ops_golang/api/shared_handlers.go
@@ -114,8 +114,8 @@ func decodeAndValidateRequestBody(r *http.Request, v
Validator) error {
return v.Validate()
}
-func hasDeleteKeyOption(obj interface{}, params map[string]string) (bool,
error, error, int) {
- optionsDeleter, ok := obj.(HasDeleteKeyOptions)
+func checkIfOptionsDeleter(obj interface{}, params map[string]string) (bool,
error, error, int) {
+ optionsDeleter, ok := obj.(OptionsDeleter)
if !ok {
return false, nil, nil, http.StatusOK
}
@@ -310,26 +310,52 @@ func DeleteHandler(deleter Deleter) http.HandlerFunc {
obj := reflect.New(objectType).Interface().(Deleter)
obj.SetInfo(inf)
- deleteKeyOptionExists, userErr, sysErr, errCode :=
hasDeleteKeyOption(obj, inf.Params)
+ isOptionsDeleter, userErr, sysErr, errCode :=
checkIfOptionsDeleter(obj, inf.Params)
if userErr != nil || sysErr != nil {
HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
return
}
- keyFields := obj.GetKeyFieldsInfo() // expecting a slice of the
key fields info which is a struct with the field name and a function to convert
a string into a interface{} of the right type. in most that will be
[{Field:"id",Func: func(s string)(interface{},error){return strconv.Atoi(s)}}]
- keys := make(map[string]interface{})
- for _, kf := range keyFields {
- paramKey := inf.Params[kf.Field]
- if paramKey == "" {
- HandleErr(w, r, inf.Tx.Tx,
http.StatusBadRequest, errors.New("missing key: "+kf.Field), nil)
- return
+ var (
+ keys = make(map[string]interface{})
+ err error
+ )
+ if isOptionsDeleter {
+ for key, info := range
obj.(OptionsDeleter).DeleteKeyOptions() {
+ paramKey := inf.Params[key]
+ if paramKey == "" {
+ continue
+ }
+ switch reflect.ValueOf(info.Checker) {
+ case reflect.ValueOf(IsInt):
+ if keys[key], err =
GetIntKey(paramKey); err != nil {
+ HandleErr(w, r, inf.Tx.Tx,
http.StatusBadRequest, errors.New("failed to parse key: "+key), nil)
+ return
+ }
+ case reflect.ValueOf(IsBool):
+ if keys[key], err =
strconv.ParseBool(paramKey); err != nil {
+ HandleErr(w, r, inf.Tx.Tx,
http.StatusBadRequest, errors.New("failed to parse key: "+key), nil)
+ return
+ }
+ default:
+ keys[key] = paramKey
+ }
}
+ } else {
+ keyFields := obj.GetKeyFieldsInfo() // expecting a
slice of the key fields info which is a struct with the field name and a
function to convert a string into a interface{} of the right type. in most that
will be [{Field:"id",Func: func(s string)(interface{},error){return
strconv.Atoi(s)}}]
+ for _, kf := range keyFields {
+ paramKey := inf.Params[kf.Field]
+ if paramKey == "" {
+ HandleErr(w, r, inf.Tx.Tx,
http.StatusBadRequest, errors.New("missing key: "+kf.Field), nil)
+ return
+ }
- paramValue, err := kf.Func(paramKey)
- if err != nil {
- HandleErr(w, r, inf.Tx.Tx,
http.StatusBadRequest, errors.New("failed to parse key: "+kf.Field), nil)
- return
+ paramValue, err := kf.Func(paramKey)
+ if err != nil {
+ HandleErr(w, r, inf.Tx.Tx,
http.StatusBadRequest, errors.New("failed to parse key: "+kf.Field), nil)
+ return
+ }
+ keys[kf.Field] = paramValue
}
- keys[kf.Field] = paramValue
}
obj.SetKeys(keys) // if the type assertion of a key fails it
will be should be set to the zero value of the type and the delete should fail
(this means the code is not written properly no changes of user input should
cause this.)
@@ -345,7 +371,7 @@ func DeleteHandler(deleter Deleter) http.HandlerFunc {
}
}
- if deleteKeyOptionExists {
+ if isOptionsDeleter {
obj :=
reflect.New(objectType).Interface().(OptionsDeleter)
obj.SetInfo(inf)
userErr, sysErr, errCode = obj.OptionsDelete()
@@ -390,14 +416,38 @@ func DeprecatedDeleteHandler(deleter Deleter, alternative
*string) http.HandlerF
obj := reflect.New(objectType).Interface().(Deleter)
obj.SetInfo(inf)
- deleteKeyOptionExists, userErr, sysErr, errCode :=
hasDeleteKeyOption(obj, inf.Params)
+ isOptionsDeleter, userErr, sysErr, errCode :=
checkIfOptionsDeleter(obj, inf.Params)
if userErr != nil || sysErr != nil {
HandleDeprecatedErr(w, r, inf.Tx.Tx, errCode, userErr,
sysErr, alternative)
return
}
- if !deleteKeyOptionExists {
+ var (
+ keys = make(map[string]interface{})
+ err error
+ )
+ if isOptionsDeleter {
+ for key, info := range
obj.(OptionsDeleter).DeleteKeyOptions() {
+ paramKey := inf.Params[key]
+ if paramKey == "" {
+ continue
+ }
+ switch reflect.ValueOf(info.Checker) {
+ case reflect.ValueOf(IsInt):
+ if keys[key], err =
GetIntKey(paramKey); err != nil {
+ HandleDeprecatedErr(w, r,
inf.Tx.Tx, http.StatusBadRequest, errors.New("missing key: "+key), nil,
alternative)
+ return
+ }
+ case reflect.ValueOf(IsBool):
+ if keys[key], err =
strconv.ParseBool(paramKey); err != nil {
+ HandleDeprecatedErr(w, r,
inf.Tx.Tx, http.StatusBadRequest, errors.New("failed to parse key: "+key), nil,
alternative)
+ return
+ }
+ default:
+ keys[key] = paramKey
+ }
+ }
+ } else {
keyFields := obj.GetKeyFieldsInfo() // expecting a
slice of the key fields info which is a struct with the field name and a
function to convert a string into a interface{} of the right type. in most that
will be [{Field:"id",Func: func(s string)(interface{},error){return
strconv.Atoi(s)}}]
- keys := make(map[string]interface{})
for _, kf := range keyFields {
paramKey := inf.Params[kf.Field]
if paramKey == "" {
@@ -412,8 +462,8 @@ func DeprecatedDeleteHandler(deleter Deleter, alternative
*string) http.HandlerF
}
keys[kf.Field] = paramValue
}
- obj.SetKeys(keys) // if the type assertion of a key
fails it will be should be set to the zero value of the type and the delete
should fail (this means the code is not written properly no changes of user
input should cause this.)
}
+ obj.SetKeys(keys) // if the type assertion of a key fails it
will be should be set to the zero value of the type and the delete should fail
(this means the code is not written properly no changes of user input should
cause this.)
if t, ok := obj.(Tenantable); ok {
authorized, err := t.IsTenantAuthorized(inf.User)
@@ -427,7 +477,7 @@ func DeprecatedDeleteHandler(deleter Deleter, alternative
*string) http.HandlerF
}
}
- if deleteKeyOptionExists {
+ if isOptionsDeleter {
obj :=
reflect.New(objectType).Interface().(OptionsDeleter)
obj.SetInfo(inf)
userErr, sysErr, errCode = obj.OptionsDelete()
diff --git a/traffic_ops/traffic_ops_golang/api/shared_interfaces.go
b/traffic_ops/traffic_ops_golang/api/shared_interfaces.go
index 3e8aa7a..d02c1c7 100644
--- a/traffic_ops/traffic_ops_golang/api/shared_interfaces.go
+++ b/traffic_ops/traffic_ops_golang/api/shared_interfaces.go
@@ -92,6 +92,7 @@ type OptionsDeleter interface {
OptionsDelete() (error, error, int)
APIInfoer
Identifier
+ DeleteKeyOptions() map[string]dbhelpers.WhereColumnInfo
}
type Validator interface {
@@ -102,10 +103,6 @@ type Tenantable interface {
IsTenantAuthorized(user *auth.CurrentUser) (bool, error)
}
-type HasDeleteKeyOptions interface {
- DeleteKeyOptions() map[string]dbhelpers.WhereColumnInfo
-}
-
// APIInfoer is an interface that guarantees the existance of a variable
through its setters and getters.
// Every CRUD operation uses this login session context
type APIInfoer interface {
diff --git a/traffic_ops/traffic_ops_golang/region/regions.go
b/traffic_ops/traffic_ops_golang/region/regions.go
index 8a84b94..d2a38b0 100644
--- a/traffic_ops/traffic_ops_golang/region/regions.go
+++ b/traffic_ops/traffic_ops_golang/region/regions.go
@@ -70,8 +70,13 @@ func (region TORegion) DeleteKeyOptions()
map[string]dbhelpers.WhereColumnInfo {
}
func (region *TORegion) SetKeys(keys map[string]interface{}) {
- i, _ := keys["id"].(int) //this utilizes the non panicking type
assertion, if the thrown away ok variable is false i will be the zero of the
type, 0 here.
- region.ID = i
+ //this utilizes the non panicking type assertion, if the thrown away ok
variable is false i will be the zero of the type, 0 here.
+ if id, exists := keys["id"].(int); exists {
+ region.ID = id
+ }
+ if name, exists := keys["name"].(string); exists {
+ region.Name = name
+ }
}
func (region *TORegion) GetAuditName() string {