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 <zach@zrhoffman.net>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 @@
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 @@
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 @@
}
}
- if deleteKeyOptionExists {
+ if isOptionsDeleter {
obj := reflect.New(objectType).Interface().(OptionsDeleter)
obj.SetInfo(inf)
userErr, sysErr, errCode = obj.OptionsDelete()
@@ -390,14 +416,38 @@
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 @@
}
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 @@
}
}
- 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 @@
OptionsDelete() (error, error, int)
APIInfoer
Identifier
+ DeleteKeyOptions() map[string]dbhelpers.WhereColumnInfo
}
type Validator interface {
@@ -102,10 +103,6 @@
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) 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 {
