This is an automated email from the ASF dual-hosted git repository.

rawlin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git


The following commit(s) were added to refs/heads/master by this push:
     new 56d09e6  Add and satisfy OptionsDelete API test for DELETE 
/regions?name={{name}} (#4707)
56d09e6 is described below

commit 56d09e6d08912dbbb90a6fa26c3095b69750fb28
Author: Zach Hoffman <z...@zrhoffman.net>
AuthorDate: Fri May 22 18:39:47 2020 -0600

    Add and satisfy OptionsDelete API test for DELETE /regions?name={{name}} 
(#4707)
    
    * 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
---
 traffic_ops/testing/api/v3/regions_test.go         | 33 ++++++++
 .../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 ++-
 4 files changed, 112 insertions(+), 27 deletions(-)

diff --git a/traffic_ops/testing/api/v3/regions_test.go 
b/traffic_ops/testing/api/v3/regions_test.go
index 6fc346e..cb6d954 100644
--- a/traffic_ops/testing/api/v3/regions_test.go
+++ b/traffic_ops/testing/api/v3/regions_test.go
@@ -16,6 +16,7 @@ package v3
 */
 
 import (
+       "strings"
        "testing"
 
        "github.com/apache/trafficcontrol/lib/go-tc"
@@ -25,6 +26,7 @@ func TestRegions(t *testing.T) {
        WithObjs(t, []TCObj{Parameters, Divisions, Regions}, func() {
                UpdateTestRegions(t)
                GetTestRegions(t)
+               DeleteTestRegionsByName(t)
        })
 }
 
@@ -84,6 +86,37 @@ func GetTestRegions(t *testing.T) {
        }
 }
 
+func DeleteTestRegionsByName(t *testing.T) {
+       for _, region := range testData.Regions {
+               delResp, _, err := TOSession.DeleteRegion(nil, &region.Name)
+               if err != nil {
+                       t.Errorf("cannot DELETE Region by name: %v - %v", err, 
delResp)
+               }
+
+               deleteLog, _, err := TOSession.GetLogsByLimit(1)
+               if err != nil {
+                       t.Fatalf("unable to get latest audit log entry")
+               }
+               if len(deleteLog) != 1 {
+                       t.Fatalf("log entry length - expected: 1, actual: %d", 
len(deleteLog))
+               }
+               if !strings.Contains(*deleteLog[0].Message, region.Name) {
+                       t.Errorf("region deletion audit log entry - expected: 
message containing region name '%s', actual: %s", region.Name, 
*deleteLog[0].Message)
+               }
+
+               // Retrieve the Region to see if it got deleted
+               regionResp, _, err := TOSession.GetRegionByName(region.Name)
+               if err != nil {
+                       t.Errorf("error deleting Region region: %s", 
err.Error())
+               }
+               if len(regionResp) > 0 {
+                       t.Errorf("expected Region : %s to be deleted", 
region.Name)
+               }
+       }
+
+       CreateTestRegions(t)
+}
+
 func DeleteTestRegions(t *testing.T) {
 
        for _, region := range testData.Regions {
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 {

Reply via email to