You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by ro...@apache.org on 2018/09/20 22:48:50 UTC
[trafficcontrol] 02/03: Changed to return usrErr, sysErr,
httpCode idiom
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 <ma...@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
}