This is an automated email from the ASF dual-hosted git repository. rob pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
commit fccaf76ceece2fbd8f46fe55eba3103bdae2bfc5 Author: moltzaum <matt...@moltzau.net> AuthorDate: Thu Sep 20 12:35:26 2018 -0600 Changed to return usrErr, sysErr, httpCode idiom --- traffic_ops/traffic_ops_golang/api/api.go | 6 +++--- .../cdnfederation/cdnfederations.go | 18 ------------------ .../traffic_ops_golang/dbhelpers/db_helpers.go | 19 ++++++++++--------- .../traffic_ops_golang/dbhelpers/db_helpers_test.go | 11 +++++++++++ .../deliveryservice/deliveryservicesv13.go | 17 ++++------------- .../deliveryservice/servers/servers.go | 9 +++++---- 6 files changed, 33 insertions(+), 47 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/api/api.go b/traffic_ops/traffic_ops_golang/api/api.go index ec152a0..10b5b80 100644 --- a/traffic_ops/traffic_ops_golang/api/api.go +++ b/traffic_ops/traffic_ops_golang/api/api.go @@ -425,15 +425,15 @@ func ParseDBErr(ierr error, dataType string) (error, error, int) { return nil, errors.New("database returned non pq error: " + err.Error()), http.StatusInternalServerError } - if usrErr, sysErr, errCode := TypeErrToAPIErr(dbhelpers.ParsePQNotNullConstraintError(err)); errCode != http.StatusOK { + if usrErr, sysErr, errCode := dbhelpers.ParsePQNotNullConstraintError(err); errCode != http.StatusOK { return usrErr, sysErr, errCode } - if usrErr, sysErr, errCode := TypeErrToAPIErr(dbhelpers.ParsePQPresentFKConstraintError(err)); errCode != http.StatusOK { + if usrErr, sysErr, errCode := dbhelpers.ParsePQPresentFKConstraintError(err); errCode != http.StatusOK { return usrErr, sysErr, errCode } - if usrErr, sysErr, errCode := TypeErrToAPIErr(dbhelpers.ParsePQUniqueConstraintError(err)); errCode != http.StatusOK { + if usrErr, sysErr, errCode := dbhelpers.ParsePQUniqueConstraintError(err); errCode != http.StatusOK { return usrErr, sysErr, errCode } diff --git a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go index 692abc8..28317cb 100644 --- a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go +++ b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go @@ -26,7 +26,6 @@ import ( "strconv" "strings" - "github.com/apache/trafficcontrol/lib/go-log" "github.com/apache/trafficcontrol/lib/go-tc" "github.com/apache/trafficcontrol/lib/go-tc/tovalidate" "github.com/apache/trafficcontrol/lib/go-util" @@ -35,7 +34,6 @@ import ( "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/tenant" "github.com/asaskevich/govalidator" "github.com/go-ozzo/ozzo-validation" - "github.com/lib/pq" ) // we need a type alias to define functions on @@ -126,22 +124,6 @@ func (fed *TOCDNFederation) Validate() error { return util.JoinErrs(tovalidate.ToErrors(validateErrs)) } -// This separates out errors depending on whether or not some constraint prevented -// the operation from occuring. -func parseQueryError(parseErr error, method string) (error, tc.ApiErrorType) { - if pqErr, ok := parseErr.(*pq.Error); ok { - err, eType := dbhelpers.ParsePQUniqueConstraintError(pqErr) - if eType == tc.DataConflictError { - log.Errorf("data conflict error: %v", err) - return errors.New("a federation with " + err.Error()), eType - } - return err, eType - } else { - log.Errorf("received error: %++v from %s execution", parseErr, method) - return tc.DBError, tc.SystemError - } -} - // fedAPIInfo.Params["name"] is not used on creation, rather the cdn name // is connected when the federations/:id/deliveryservice links a federation // Note: cdns and deliveryservies have a 1-1 relationship diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go index d3ca854..059576b 100644 --- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go +++ b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go @@ -23,6 +23,7 @@ import ( "database/sql" "errors" "fmt" + "net/http" "regexp" "strings" @@ -115,33 +116,33 @@ func toCamelCase(str string) string { } // parses pq errors for not null constraint -func ParsePQNotNullConstraintError(err *pq.Error) (error, tc.ApiErrorType) { +func ParsePQNotNullConstraintError(err *pq.Error) (error, error, int) { pattern := regexp.MustCompile(`null value in column "(.+)" violates not-null constraint`) match := pattern.FindStringSubmatch(err.Message) if match == nil { - return nil, tc.NoError + return nil, nil, http.StatusOK } - return fmt.Errorf("%s is a required field", toCamelCase(match[1])), tc.DataConflictError + return fmt.Errorf("%s is a required field", toCamelCase(match[1])), nil, http.StatusBadRequest } // parses pq errors for violated foreign key constraints -func ParsePQPresentFKConstraintError(err *pq.Error) (error, tc.ApiErrorType) { +func ParsePQPresentFKConstraintError(err *pq.Error) (error, error, int) { pattern := regexp.MustCompile(`Key \(.+\)=\(.+\) is not present in table "(.+)"`) match := pattern.FindStringSubmatch(err.Detail) if match == nil { - return nil, tc.NoError + return nil, nil, http.StatusOK } - return fmt.Errorf("%s not found", match[1]), tc.DataMissingError + return fmt.Errorf("%s not found", match[1]), nil, http.StatusNotFound } // parses pq errors for uniqueness constraint violations -func ParsePQUniqueConstraintError(err *pq.Error) (error, tc.ApiErrorType) { +func ParsePQUniqueConstraintError(err *pq.Error) (error, error, int) { pattern := regexp.MustCompile(`Key \((.+)\)=\((.+)\) already exists`) match := pattern.FindStringSubmatch(err.Detail) if match == nil { - return nil, tc.NoError + return nil, nil, http.StatusOK } - return fmt.Errorf("%s %s already exists.", match[1], match[2]), tc.DataConflictError + return fmt.Errorf("%s %s already exists.", match[1], match[2]), nil, http.StatusBadRequest } // FinishTx commits the transaction if commit is true when it's called, otherwise it rolls back the transaction. This is designed to be called in a defer. diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers_test.go b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers_test.go index 5607dfc..74f4f07 100644 --- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers_test.go +++ b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers_test.go @@ -71,3 +71,14 @@ FROM table t } } + +func TestCamelCase(t *testing.T) { + + testStrings := []string{"hello_world", "trailing_underscore_", "w_h_a_t____"} + expected := []string{"helloWorld", "trailingUnderscore", "wHAT"} + for i, str := range testStrings { + if toCamelCase(str) != expected[i] { + t.Errorf("expected: %v error, actual: %v", expected[i], toCamelCase(str)) + } + } +} diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv13.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv13.go index 6075e3f..865ebe1 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv13.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv13.go @@ -147,11 +147,8 @@ func create(tx *sql.Tx, cfg config.Config, user *auth.CurrentUser, ds tc.Deliver resultRows, err := tx.Query(insertQuery(), &ds.Active, &ds.AnonymousBlockingEnabled, &ds.CacheURL, &ds.CCRDNSTTL, &ds.CDNID, &ds.CheckPath, &deepCachingType, &ds.DisplayName, &ds.DNSBypassCNAME, &ds.DNSBypassIP, &ds.DNSBypassIP6, &ds.DNSBypassTTL, &ds.DSCP, &ds.EdgeHeaderRewrite, &ds.GeoLimitRedirectURL, &ds.GeoLimit, &ds.GeoLimitCountries, &ds.GeoProvider, &ds.GlobalMaxMBPS, &ds.GlobalMaxTPS, &ds.FQPacingRate, &ds.HTTPBypassFQDN, &ds.InfoURL, &ds.InitialDispersion, &ds.IPV6RoutingEnabl [...] if err != nil { - if pqerr, ok := err.(*pq.Error); ok { - err, _ := dbhelpers.ParsePQUniqueConstraintError(pqerr) - return tc.DeliveryServiceNullable{}, http.StatusBadRequest, errors.New("a delivery service with " + err.Error()), nil - } - return tc.DeliveryServiceNullable{}, http.StatusInternalServerError, nil, errors.New("inserting ds: " + err.Error()) + usrErr, sysErr, code := api.ParseDBErr(err, "delivery service") + return tc.DeliveryServiceNullable{}, code, usrErr, sysErr } defer resultRows.Close() @@ -456,14 +453,8 @@ func update(tx *sql.Tx, cfg config.Config, user *auth.CurrentUser, ds *tc.Delive resultRows, err := tx.Query(updateDSQuery(), &ds.Active, &ds.CacheURL, &ds.CCRDNSTTL, &ds.CDNID, &ds.CheckPath, &deepCachingType, &ds.DisplayName, &ds.DNSBypassCNAME, &ds.DNSBypassIP, &ds.DNSBypassIP6, &ds.DNSBypassTTL, &ds.DSCP, &ds.EdgeHeaderRewrite, &ds.GeoLimitRedirectURL, &ds.GeoLimit, &ds.GeoLimitCountries, &ds.GeoProvider, &ds.GlobalMaxMBPS, &ds.GlobalMaxTPS, &ds.FQPacingRate, &ds.HTTPBypassFQDN, &ds.InfoURL, &ds.InitialDispersion, &ds.IPV6RoutingEnabled, &ds.LogsEnabled, &ds.Lon [...] if err != nil { - if err, ok := err.(*pq.Error); ok { - err, eType := dbhelpers.ParsePQUniqueConstraintError(err) - if eType == tc.DataConflictError { - return tc.DeliveryServiceNullable{}, http.StatusBadRequest, errors.New("a delivery service with " + err.Error()), nil - } - return tc.DeliveryServiceNullable{}, http.StatusInternalServerError, nil, errors.New("query updating delivery service: pq: " + err.Error()) - } - return tc.DeliveryServiceNullable{}, http.StatusInternalServerError, nil, errors.New("query updating delivery service: " + err.Error()) + usrErr, sysErr, code := api.ParseDBErr(err, "delivery service") + return tc.DeliveryServiceNullable{}, code, usrErr, sysErr } defer resultRows.Close() if !resultRows.Next() { diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go b/traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go index a9888b0..0e26c0f 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go @@ -268,9 +268,9 @@ func GetReplaceHandler(w http.ResponseWriter, r *http.Request) { dtos := map[string]interface{}{"id": dsId, "server": server} if _, err := inf.Tx.NamedExec(insertIdsQuery(), dtos); err != nil { if pqErr, ok := err.(*pq.Error); ok { - err, eType := dbhelpers.ParsePQUniqueConstraintError(pqErr) + err, _, code := dbhelpers.ParsePQUniqueConstraintError(pqErr) log.Errorln("could not begin transaction: %v", err) - if eType == tc.DataConflictError { + if code == http.StatusBadRequest { api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("inserting for delivery service servers replace: "+err.Error())) return } @@ -327,9 +327,10 @@ func GetCreateHandler(w http.ResponseWriter, r *http.Request) { res, err := inf.Tx.Tx.Exec(`INSERT INTO deliveryservice_server (deliveryservice, server) SELECT $1, id FROM server WHERE host_name = ANY($2::text[])`, dsID, pq.Array(serverNames)) if err != nil { + if pqErr, ok := err.(*pq.Error); ok { - err, eType := dbhelpers.ParsePQUniqueConstraintError(pqErr) - if eType == tc.DataConflictError { + err, _, code := dbhelpers.ParsePQUniqueConstraintError(pqErr) + if code == http.StatusBadRequest { api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("a deliveryservice-server association with "+err.Error()), nil) return }