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 821f1f60759e3f9eab2c99cf768b68de55d450c4
Author: moltzaum <matt...@moltzau.net>
AuthorDate: Thu Sep 20 14:06:00 2018 -0600
Refactored a little bit
---
traffic_ops/traffic_ops_golang/api/api.go | 54 +++++++++++++++++++---
traffic_ops/traffic_ops_golang/api/api_test.go | 14 ++++++
traffic_ops/traffic_ops_golang/api/crud.go | 4 +-
.../traffic_ops_golang/cachegroup/cachegroups.go | 4 +-
.../traffic_ops_golang/dbhelpers/db_helpers.go | 44 ------------------
.../dbhelpers/db_helpers_test.go | 11 -----
.../deliveryservice/deliveryservicesv13.go | 4 +-
.../deliveryservice/request/comment/comments.go | 2 +-
.../deliveryservice/request/requests.go | 4 +-
.../deliveryservice/servers/servers.go | 26 ++---------
traffic_ops/traffic_ops_golang/origin/origins.go | 4 +-
.../profileparameter/profile_parameters.go | 2 +-
.../steeringtargets/steeringtargets.go | 4 +-
13 files changed, 79 insertions(+), 98 deletions(-)
diff --git a/traffic_ops/traffic_ops_golang/api/api.go
b/traffic_ops/traffic_ops_golang/api/api.go
index 10b5b80..6d3d5d0 100644
--- a/traffic_ops/traffic_ops_golang/api/api.go
+++ b/traffic_ops/traffic_ops_golang/api/api.go
@@ -27,6 +27,7 @@ import (
"fmt"
"io"
"net/http"
+ "regexp"
"strconv"
"strings"
"time"
@@ -36,7 +37,6 @@ import (
"github.com/apache/trafficcontrol/lib/go-util"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/auth"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/config"
-
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
"github.com/jmoiron/sqlx"
"github.com/lib/pq"
@@ -416,24 +416,64 @@ func TypeErrToAPIErr(err error, errType tc.ApiErrorType)
(error, error, int) {
}
}
-// ParseDBErr parses pq errors for uniqueness constraint violations, and
returns the (userErr, sysErr, httpCode) format expected by the API helpers.
-// The dataType is the name of the API object, e.g. 'coordinate' or 'delivery
service', used to construct the error string.
-func ParseDBErr(ierr error, dataType string) (error, error, int) {
+// small helper function to help with parsing below
+func toCamelCase(str string) string {
+ mutable := []byte(str)
+ for i := 0; i < len(str); i++ {
+ if mutable[i] == '_' && i+1 < len(str) {
+ mutable[i+1] = strings.ToUpper(string(str[i+1]))[0]
+ }
+ }
+ return strings.Replace(string(mutable[:]), "_", "", -1)
+}
+
+// parses pq errors for not null constraint
+func parseNotNullConstraintError(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, nil, http.StatusOK
+ }
+ return fmt.Errorf("%s is a required field", toCamelCase(match[1])),
nil, http.StatusBadRequest
+}
+
+// parses pq errors for violated foreign key constraints
+func parseNotPresentFKConstraintError(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, nil, http.StatusOK
+ }
+ return fmt.Errorf("%s not found", match[1]), nil, http.StatusNotFound
+}
+
+// parses pq errors for uniqueness constraint violations
+func parseUniqueConstraintError(err *pq.Error) (error, error, int) {
+ pattern := regexp.MustCompile(`Key \((.+)\)=\((.+)\) already exists`)
+ match := pattern.FindStringSubmatch(err.Detail)
+ if match == nil {
+ return nil, nil, http.StatusOK
+ }
+ return fmt.Errorf("%s %s already exists.", match[1], match[2]), nil,
http.StatusBadRequest
+}
+
+// ParseDBError parses pq errors for uniqueness constraint violations, and
returns the (userErr, sysErr, httpCode) format expected by the API helpers.
+func ParseDBError(ierr error) (error, error, int) {
err, ok := ierr.(*pq.Error)
if !ok {
return nil, errors.New("database returned non pq error: " +
err.Error()), http.StatusInternalServerError
}
- if usrErr, sysErr, errCode :=
dbhelpers.ParsePQNotNullConstraintError(err); errCode != http.StatusOK {
+ if usrErr, sysErr, errCode := parseNotNullConstraintError(err); errCode
!= http.StatusOK {
return usrErr, sysErr, errCode
}
- if usrErr, sysErr, errCode :=
dbhelpers.ParsePQPresentFKConstraintError(err); errCode != http.StatusOK {
+ if usrErr, sysErr, errCode := parseNotPresentFKConstraintError(err);
errCode != http.StatusOK {
return usrErr, sysErr, errCode
}
- if usrErr, sysErr, errCode :=
dbhelpers.ParsePQUniqueConstraintError(err); errCode != http.StatusOK {
+ if usrErr, sysErr, errCode := parseUniqueConstraintError(err); errCode
!= http.StatusOK {
return usrErr, sysErr, errCode
}
diff --git a/traffic_ops/traffic_ops_golang/api/api_test.go
b/traffic_ops/traffic_ops_golang/api/api_test.go
new file mode 100644
index 0000000..700d9c8
--- /dev/null
+++ b/traffic_ops/traffic_ops_golang/api/api_test.go
@@ -0,0 +1,14 @@
+package api
+
+import "testing"
+
+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/api/crud.go
b/traffic_ops/traffic_ops_golang/api/crud.go
index bb44c30..5c0dd35 100644
--- a/traffic_ops/traffic_ops_golang/api/crud.go
+++ b/traffic_ops/traffic_ops_golang/api/crud.go
@@ -62,7 +62,7 @@ type GenericDeleter interface {
func GenericCreate(val GenericCreator) (error, error, int) {
resultRows, err := val.APIInfo().Tx.NamedQuery(val.InsertQuery(), val)
if err != nil {
- return ParseDBErr(err, val.GetType())
+ return ParseDBError(err)
}
defer resultRows.Close()
@@ -113,7 +113,7 @@ func GenericRead(val GenericReader) ([]interface{}, error,
error, int) {
func GenericUpdate(val GenericUpdater) (error, error, int) {
rows, err := val.APIInfo().Tx.NamedQuery(val.UpdateQuery(), val)
if err != nil {
- return ParseDBErr(err, val.GetType())
+ return ParseDBError(err)
}
defer rows.Close()
diff --git a/traffic_ops/traffic_ops_golang/cachegroup/cachegroups.go
b/traffic_ops/traffic_ops_golang/cachegroup/cachegroups.go
index 5c838b6..9a833a0 100644
--- a/traffic_ops/traffic_ops_golang/cachegroup/cachegroups.go
+++ b/traffic_ops/traffic_ops_golang/cachegroup/cachegroups.go
@@ -202,7 +202,7 @@ func (cg *TOCacheGroup) Create() (error, error, int) {
cg.SecondaryParentCachegroupID,
)
if err != nil {
- return api.ParseDBErr(err, cg.GetType())
+ return api.ParseDBError(err)
}
defer resultRows.Close()
@@ -375,7 +375,7 @@ func (cg *TOCacheGroup) Update() (error, error, int) {
cg.ID,
)
if err != nil {
- return api.ParseDBErr(err, cg.GetType())
+ return api.ParseDBError(err)
}
defer resultRows.Close()
diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
index 059576b..866342c 100644
--- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
+++ b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
@@ -22,9 +22,6 @@ package dbhelpers
import (
"database/sql"
"errors"
- "fmt"
- "net/http"
- "regexp"
"strings"
"github.com/apache/trafficcontrol/lib/go-log"
@@ -104,47 +101,6 @@ func parseCriteriaAndQueryValues(queryParamsToSQLCols
map[string]WhereColumnInfo
return criteria, queryValues, errs
}
-// small helper function to help with parsing below
-func toCamelCase(str string) string {
- mutable := []byte(str)
- for i := 0; i < len(str); i++ {
- if mutable[i] == '_' && i+1 < len(str) {
- mutable[i+1] = strings.ToUpper(string(str[i+1]))[0]
- }
- }
- return strings.Replace(string(mutable[:]), "_", "", -1)
-}
-
-// parses pq errors for not null constraint
-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, nil, http.StatusOK
- }
- 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, error, int) {
- pattern := regexp.MustCompile(`Key \(.+\)=\(.+\) is not present in
table "(.+)"`)
- match := pattern.FindStringSubmatch(err.Detail)
- if match == nil {
- return nil, nil, http.StatusOK
- }
- return fmt.Errorf("%s not found", match[1]), nil, http.StatusNotFound
-}
-
-// parses pq errors for uniqueness constraint violations
-func ParsePQUniqueConstraintError(err *pq.Error) (error, error, int) {
- pattern := regexp.MustCompile(`Key \((.+)\)=\((.+)\) already exists`)
- match := pattern.FindStringSubmatch(err.Detail)
- if match == nil {
- return nil, nil, http.StatusOK
- }
- 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.
func FinishTx(tx *sql.Tx, commit *bool) {
if tx == nil {
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 74f4f07..5607dfc 100644
--- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers_test.go
+++ b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers_test.go
@@ -71,14 +71,3 @@ 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 865ebe1..649afb1 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv13.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv13.go
@@ -147,7 +147,7 @@ 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 {
- usrErr, sysErr, code := api.ParseDBErr(err, "delivery service")
+ usrErr, sysErr, code := api.ParseDBError(err)
return tc.DeliveryServiceNullable{}, code, usrErr, sysErr
}
defer resultRows.Close()
@@ -453,7 +453,7 @@ 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 {
- usrErr, sysErr, code := api.ParseDBErr(err, "delivery service")
+ usrErr, sysErr, code := api.ParseDBError(err)
return tc.DeliveryServiceNullable{}, code, usrErr, sysErr
}
defer resultRows.Close()
diff --git
a/traffic_ops/traffic_ops_golang/deliveryservice/request/comment/comments.go
b/traffic_ops/traffic_ops_golang/deliveryservice/request/comment/comments.go
index 5b41695..db1ded1 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/request/comment/comments.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/request/comment/comments.go
@@ -114,7 +114,7 @@ func (comment *TODeliveryServiceRequestComment) Update()
(error, error, int) {
current := TODeliveryServiceRequestComment{}
err := comment.ReqInfo.Tx.QueryRowx(selectQuery() + `WHERE dsrc.id=` +
strconv.Itoa(*comment.ID)).StructScan(¤t)
if err != nil {
- return api.ParseDBErr(err, comment.GetType())
+ return api.ParseDBError(err)
}
userID := tc.IDNoMod(comment.ReqInfo.User.ID)
diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go
b/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go
index 7751b04..613ef70 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go
@@ -389,7 +389,7 @@ func (req *deliveryServiceRequestAssignment) Update()
(error, error, int) {
}
if _, err = req.APIInfo().Tx.Tx.Exec(`UPDATE deliveryservice_request
SET assignee_id = $1 WHERE id = $2`, v, *req.ID); err != nil {
- return api.ParseDBErr(err, req.GetType())
+ return api.ParseDBError(err)
}
if err =
req.APIInfo().Tx.QueryRowx(selectDeliveryServiceRequestsQuery()+` WHERE r.id =
$1`, *req.ID).StructScan(req); err != nil {
@@ -455,7 +455,7 @@ func (req *deliveryServiceRequestStatus) Update() (error,
error, int) {
// LastEditedBy field should not change with status update
if _, err = req.APIInfo().Tx.Tx.Exec(`UPDATE deliveryservice_request
SET status = $1 WHERE id = $2`, *req.Status, *req.ID); err != nil {
- return api.ParseDBErr(err, req.GetType())
+ return api.ParseDBError(err)
}
if err =
req.APIInfo().Tx.QueryRowx(selectDeliveryServiceRequestsQuery()+` WHERE r.id =
$1`, *req.ID).StructScan(req); err != nil {
diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go
b/traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go
index 0e26c0f..e6a00ce 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go
@@ -32,7 +32,6 @@ import (
"github.com/apache/trafficcontrol/lib/go-util"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/auth"
-
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/deliveryservice"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/tenant"
@@ -267,17 +266,8 @@ func GetReplaceHandler(w http.ResponseWriter, r
*http.Request) {
for _, server := range servers {
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, _, code :=
dbhelpers.ParsePQUniqueConstraintError(pqErr)
- log.Errorln("could not begin transaction: %v",
err)
- 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
- }
- api.HandleErr(w, r, inf.Tx.Tx,
http.StatusInternalServerError, nil, errors.New("inserting for delivery service
servers replace: "+err.Error()))
- return
- }
- api.HandleErr(w, r, inf.Tx.Tx,
http.StatusInternalServerError, nil, errors.New("inserting for delivery service
servers replace: "+err.Error()))
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, inf.Tx.Tx, code, usrErr, sysErr)
return
}
respServers = append(respServers, server)
@@ -328,16 +318,8 @@ 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, _, 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
- }
- api.HandleErr(w, r, inf.Tx.Tx,
http.StatusInternalServerError, nil, errors.New("ds servers inserting for
create delivery service servers: "+err.Error()))
- return
- }
- api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError,
nil, errors.New("ds servers inserting for create delivery service servers
received non pq error: "+err.Error()))
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, inf.Tx.Tx, code, usrErr, sysErr)
return
}
diff --git a/traffic_ops/traffic_ops_golang/origin/origins.go
b/traffic_ops/traffic_ops_golang/origin/origins.go
index 0df598f..15d117b 100644
--- a/traffic_ops/traffic_ops_golang/origin/origins.go
+++ b/traffic_ops/traffic_ops_golang/origin/origins.go
@@ -341,7 +341,7 @@ func (origin *TOOrigin) Update() (error, error, int) {
log.Debugf("about to run exec query: %s with origin: %++v",
updateQuery(), origin)
resultRows, err := origin.ReqInfo.Tx.NamedQuery(updateQuery(), origin)
if err != nil {
- return api.ParseDBErr(err, origin.GetType())
+ return api.ParseDBError(err)
}
defer resultRows.Close()
@@ -397,7 +397,7 @@ func (origin *TOOrigin) Create() (error, error, int) {
resultRows, err := origin.ReqInfo.Tx.NamedQuery(insertQuery(), origin)
if err != nil {
- return api.ParseDBErr(err, origin.GetType())
+ return api.ParseDBError(err)
}
defer resultRows.Close()
diff --git
a/traffic_ops/traffic_ops_golang/profileparameter/profile_parameters.go
b/traffic_ops/traffic_ops_golang/profileparameter/profile_parameters.go
index 287b8db..872cf65 100644
--- a/traffic_ops/traffic_ops_golang/profileparameter/profile_parameters.go
+++ b/traffic_ops/traffic_ops_golang/profileparameter/profile_parameters.go
@@ -124,7 +124,7 @@ func (pp *TOProfileParameter) Validate() error {
func (pp *TOProfileParameter) Create() (error, error, int) {
resultRows, err := pp.APIInfo().Tx.NamedQuery(insertQuery(), pp)
if err != nil {
- return api.ParseDBErr(err, pp.GetType())
+ return api.ParseDBError(err)
}
defer resultRows.Close()
diff --git a/traffic_ops/traffic_ops_golang/steeringtargets/steeringtargets.go
b/traffic_ops/traffic_ops_golang/steeringtargets/steeringtargets.go
index 9642983..c205b08 100644
--- a/traffic_ops/traffic_ops_golang/steeringtargets/steeringtargets.go
+++ b/traffic_ops/traffic_ops_golang/steeringtargets/steeringtargets.go
@@ -182,7 +182,7 @@ func (st *TOSteeringTargetV11) Create() (error, error, int)
{
rows, err := st.ReqInfo.Tx.NamedQuery(insertQuery(), st)
if err != nil {
- return api.ParseDBErr(err, st.GetType())
+ return api.ParseDBError(err)
}
defer rows.Close()
@@ -223,7 +223,7 @@ func (st *TOSteeringTargetV11) Update() (error, error, int)
{
rows, err := st.ReqInfo.Tx.NamedQuery(updateQuery(), st)
if err != nil {
- return api.ParseDBErr(err, st.GetType())
+ return api.ParseDBError(err)
}
defer rows.Close()
