You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by mi...@apache.org on 2018/06/21 17:08:51 UTC

[trafficcontrol] 03/03: Remove TO ds encoding/json imports

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

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

commit f0395ce4fe1ce4f7a38fc22578738cddaaa4ce73
Author: Robert Butts <ro...@apache.org>
AuthorDate: Sat Jun 2 14:13:16 2018 -0600

    Remove TO ds encoding/json imports
---
 LICENSE                                            |  8 ++--
 lib/go-tc/deliveryservices.go                      | 10 ++--
 .../api/v13/deliveryservice_requests_test.go       | 29 ++----------
 .../deliveryservice/deliveryservicesv12.go         | 41 ++++++----------
 .../deliveryservice/deliveryservicesv13.go         | 23 +++------
 .../deliveryservice/request/validate.go            |  7 +--
 .../deliveryservice/servers/servers.go             | 54 +++++++---------------
 7 files changed, 54 insertions(+), 118 deletions(-)

diff --git a/LICENSE b/LICENSE
index cf36251..8f264c1 100644
--- a/LICENSE
+++ b/LICENSE
@@ -379,12 +379,12 @@ The envconfig component is used under the MIT license:
 ./traffic_ops/vendor/github.com/kelseyhightower/envconfig/LICENSE
 
 The govalidator component is used under the MIT license:
-@traffic_ops/vendor/github.com/asaskevich/govalidator/*
-./traffic_ops/vendor/github.com/asaskevich/govalidator/LICENSE
+@vendor/github.com/asaskevich/govalidator/*
+./vendor/github.com/asaskevich/govalidator/LICENSE
 
 The ozzo-validation component is used under the MIT license:
-@traffic_ops/vendor/github.com/go-ozzo/ozzo-validation/*
-./traffic_ops/vendor/github.com/go-ozzo/ozzo-validation/LICENSE
+@vendor/github.com/go-ozzo/ozzo-validation/*
+./vendor/github.com/go-ozzo/ozzo-validation/LICENSE
 
 The spinner-small.gif is used under the WTFPL license:
 @traffic_ops/app/public/images/spinner-small.gif
diff --git a/lib/go-tc/deliveryservices.go b/lib/go-tc/deliveryservices.go
index 484a0b3..354b76d 100644
--- a/lib/go-tc/deliveryservices.go
+++ b/lib/go-tc/deliveryservices.go
@@ -9,8 +9,8 @@ import (
 	"regexp"
 	"strings"
 
-	"github.com/apache/incubator-trafficcontrol/lib/go-tc/tovalidate"
-	"github.com/apache/incubator-trafficcontrol/lib/go-util"
+	"github.com/apache/trafficcontrol/lib/go-tc/tovalidate"
+	"github.com/apache/trafficcontrol/lib/go-util"
 
 	"github.com/asaskevich/govalidator"
 	"github.com/go-ozzo/ozzo-validation"
@@ -382,12 +382,12 @@ func (ds *DeliveryServiceNullableV12) Validate(tx *sql.Tx) error {
 		"xmlId":               validation.Validate(ds.XMLID, noSpaces, noPeriods, validation.Length(1, 48)),
 	}
 	toErrs := tovalidate.ToErrors(errs)
+	if err := ds.validateTypeFields(tx); err != nil {
+		toErrs = append(toErrs, errors.New("type fields: "+err.Error()))
+	}
 	if len(toErrs) > 0 {
 		return errors.New(util.JoinErrsStr(toErrs))
 	}
-	if err := ds.validateTypeFields(tx); err != nil {
-		return errors.New("type fields: " + err.Error())
-	}
 	return nil
 }
 
diff --git a/traffic_ops/testing/api/v13/deliveryservice_requests_test.go b/traffic_ops/testing/api/v13/deliveryservice_requests_test.go
index 1505ae2..6f83fb1 100644
--- a/traffic_ops/testing/api/v13/deliveryservice_requests_test.go
+++ b/traffic_ops/testing/api/v13/deliveryservice_requests_test.go
@@ -75,7 +75,6 @@ func CreateTestDeliveryServiceRequests(t *testing.T) {
 }
 
 func TestDeliveryServiceRequestRequired(t *testing.T) {
-
 	CreateTestCDNs(t)
 	CreateTestTypes(t)
 	dsr := testData.DeliveryServiceRequests[dsrRequired]
@@ -84,26 +83,11 @@ func TestDeliveryServiceRequestRequired(t *testing.T) {
 		t.Errorf("Error occurred %v", err)
 	}
 
-	expected := []string{
-		"'active' cannot be blank",
-		"'cdnId' cannot be blank",
-		"'dscp' cannot be blank",
-		"'geoLimit' cannot be blank",
-		"'geoProvider' cannot be blank",
-		"'infoUrl' must be a valid URL",
-		"'initialDispersion' must be greater than zero",
-		"'logsEnabled' cannot be blank",
-		"'orgServerFqdn' must be a valid URL",
-		"'regionalGeoBlocking' cannot be blank",
-		"'routingName' must be a valid hostname",
-		"'typeId' cannot be blank",
-		"'xmlId' cannot contain spaces",
+	if len(alerts.Alerts) == 0 {
+		t.Errorf("Expected: validation error alerts, actual: %+v", alerts)
 	}
-
-	utils.Compare(t, expected, alerts.ToStrings())
 	DeleteTestTypes(t)
 	DeleteTestCDNs(t)
-
 }
 
 func TestDeliveryServiceRequestRules(t *testing.T) {
@@ -142,14 +126,9 @@ func TestDeliveryServiceRequestRules(t *testing.T) {
 	if err != nil {
 		t.Errorf("Error occurred %v", err)
 	}
-
-	expected := []string{
-		"'displayName' the length must be between 1 and 48",
-		"'routingName' cannot contain periods",
-		"'xmlId' cannot contain spaces",
+	if len(alerts.Alerts) == 0 {
+		t.Errorf("Expected: validation error alerts, actual: %+v", alerts)
 	}
-
-	utils.Compare(t, expected, alerts.ToStrings())
 	DeleteTestTypes(t)
 	DeleteTestCDNs(t)
 
diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv12.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv12.go
index cb737f3..596df13 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv12.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv12.go
@@ -21,7 +21,6 @@ package deliveryservice
 
 import (
 	"database/sql"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"net/http"
@@ -37,22 +36,10 @@ import (
 	"github.com/jmoiron/sqlx"
 )
 
-type TODeliveryServiceV12 struct {
-	tc.DeliveryServiceNullableV12
-	Cfg config.Config
-	DB  *sqlx.DB
-}
-
-func (ds TODeliveryServiceV12) MarshalJSON() ([]byte, error) {
-	return json.Marshal(ds.DeliveryServiceNullableV12)
-}
-
-func (ds *TODeliveryServiceV12) UnmarshalJSON(data []byte) error {
-	return json.Unmarshal(data, ds.DeliveryServiceNullableV12)
-}
+type TODeliveryServiceV12 tc.DeliveryServiceNullableV12
 
 func GetRefTypeV12(cfg config.Config, db *sqlx.DB) *TODeliveryServiceV12 {
-	return &TODeliveryServiceV12{Cfg: cfg, DB: db}
+	return &TODeliveryServiceV12{}
 }
 
 func (ds TODeliveryServiceV12) GetKeyFieldsInfo() []api.KeyFieldInfo {
@@ -113,12 +100,17 @@ func (ds *TODeliveryServiceV12) GetXMLID(tx *sql.Tx) (string, bool, error) {
 	if ds.ID == nil {
 		return "", false, errors.New("missing ID")
 	}
+	return GetXMLID(tx, *ds.ID)
+}
+
+// GetXMLID loads the DeliveryService's xml_id from the database, from the ID. Returns whether the delivery service was found, and any error.
+func GetXMLID(tx *sql.Tx, id int) (string, bool, error) {
 	xmlID := ""
-	if err := tx.QueryRow(`SELECT xml_id FROM deliveryservice where id = $1`, ds.ID).Scan(&xmlID); err != nil {
+	if err := tx.QueryRow(`SELECT xml_id FROM deliveryservice where id = $1`, id).Scan(&xmlID); err != nil {
 		if err == sql.ErrNoRows {
 			return "", false, nil
 		}
-		return "", false, fmt.Errorf("querying xml_id for delivery service ID '%v': %v", *ds.ID, err)
+		return "", false, fmt.Errorf("querying xml_id for delivery service ID '%v': %v", id, err)
 	}
 	return xmlID, true, nil
 }
@@ -130,7 +122,7 @@ func (ds *TODeliveryServiceV12) IsTenantAuthorized(user *auth.CurrentUser, db *s
 		return false, errors.New("beginning transaction: " + err.Error())
 	}
 	defer dbhelpers.FinishTx(tx, util.BoolPtr(true))
-	return isTenantAuthorized(user, tx, &ds.DeliveryServiceNullableV12)
+	return isTenantAuthorized(user, tx, (*tc.DeliveryServiceNullableV12)(ds))
 }
 
 // getTenantID returns the tenant Id of the given delivery service. Note it may return a nil id and nil error, if the tenant ID in the database is nil.
@@ -181,7 +173,7 @@ func (ds *TODeliveryServiceV12) Validate(db *sqlx.DB) []error {
 		return []error{errors.New("beginning transaction: " + err.Error())}
 	}
 	defer dbhelpers.FinishTx(tx, util.BoolPtr(true))
-	return []error{ds.DeliveryServiceNullableV12.Validate(tx)}
+	return []error{(*tc.DeliveryServiceNullableV12)(ds).Validate(tx)}
 }
 
 func CreateV12(w http.ResponseWriter, r *http.Request) {
@@ -232,15 +224,9 @@ func (ds *TODeliveryServiceV12) Read(db *sqlx.DB, params map[string]string, user
 }
 
 func (ds *TODeliveryServiceV12) Delete(db *sqlx.DB, user auth.CurrentUser) (error, tc.ApiErrorType) {
-	v13 := &TODeliveryServiceV13{
-		Cfg: ds.Cfg,
-		DB:  ds.DB,
-		DeliveryServiceNullableV13: tc.DeliveryServiceNullableV13{
-			DeliveryServiceNullableV12: ds.DeliveryServiceNullableV12,
-		},
-	}
+	v13 := (*TODeliveryServiceV13)(&tc.DeliveryServiceNullableV13{DeliveryServiceNullableV12: *(*tc.DeliveryServiceNullableV12)(ds)})
 	err, errType := v13.Delete(db, user)
-	ds.DeliveryServiceNullableV12 = v13.DeliveryServiceNullableV12 // TODO avoid copy
+	*ds = (TODeliveryServiceV12)(v13.DeliveryServiceNullableV12) // TODO avoid copy
 	return err, errType
 }
 
@@ -271,5 +257,6 @@ func UpdateV12(w http.ResponseWriter, r *http.Request) {
 		api.HandleErr(w, r, errCode, userErr, sysErr)
 		return
 	}
+	*inf.CommitTx = true
 	api.WriteResp(w, r, []tc.DeliveryServiceNullableV12{dsv13.DeliveryServiceNullableV12})
 }
diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv13.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv13.go
index d332c3e..65b608a 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv13.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv13.go
@@ -21,7 +21,6 @@ package deliveryservice
 
 import (
 	"database/sql"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"net/http"
@@ -43,25 +42,16 @@ import (
 )
 
 //we need a type alias to define functions on
-type TODeliveryServiceV13 struct {
-	tc.DeliveryServiceNullableV13
-	Cfg config.Config
-	DB  *sqlx.DB
-}
+type TODeliveryServiceV13 tc.DeliveryServiceNullableV13
 
 func (ds *TODeliveryServiceV13) V12() *TODeliveryServiceV12 {
-	return &TODeliveryServiceV12{DeliveryServiceNullableV12: ds.DeliveryServiceNullableV12, DB: ds.DB, Cfg: ds.Cfg}
-}
-
-func (ds TODeliveryServiceV13) MarshalJSON() ([]byte, error) {
-	return json.Marshal(ds.DeliveryServiceNullableV13)
-}
-func (ds *TODeliveryServiceV13) UnmarshalJSON(data []byte) error {
-	return json.Unmarshal(data, ds.DeliveryServiceNullableV13)
+	v13 := (*tc.DeliveryServiceNullableV13)(ds)
+	v12 := &v13.DeliveryServiceNullableV12
+	return (*TODeliveryServiceV12)(v12)
 }
 
 func GetRefTypeV13(cfg config.Config, db *sqlx.DB) *TODeliveryServiceV13 {
-	return &TODeliveryServiceV13{Cfg: cfg, DB: db}
+	return &TODeliveryServiceV13{}
 }
 
 func (ds TODeliveryServiceV13) GetKeyFieldsInfo() []api.KeyFieldInfo {
@@ -92,7 +82,7 @@ func (ds *TODeliveryServiceV13) Validate(db *sqlx.DB) []error {
 		return []error{errors.New("beginning transaction: " + err.Error())}
 	}
 	defer dbhelpers.FinishTx(tx, util.BoolPtr(true))
-	return []error{ds.DeliveryServiceNullableV13.Validate(tx)}
+	return []error{(*tc.DeliveryServiceNullableV13)(ds).Validate(tx)}
 }
 
 // CreateV13 implements the http.HandlerFunc type, and handles API 1.3 POST requests.
@@ -388,6 +378,7 @@ func UpdateV13(w http.ResponseWriter, r *http.Request) {
 		api.HandleErr(w, r, errCode, userErr, sysErr)
 		return
 	}
+	*inf.CommitTx = true
 	api.WriteResp(w, r, []tc.DeliveryServiceNullableV13{ds})
 }
 
diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/request/validate.go b/traffic_ops/traffic_ops_golang/deliveryservice/request/validate.go
index f50cb02..c9cd298 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/request/validate.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/request/validate.go
@@ -39,7 +39,7 @@ func (req *TODeliveryServiceRequest) Validate(db *sqlx.DB) []error {
 		return []error{errors.New("beginning transaction: " + err.Error())}
 	}
 	commitTx := false
-	dbhelpers.FinishTx(tx, &commitTx)
+	defer dbhelpers.FinishTx(tx, &commitTx)
 
 	fromStatus := tc.RequestStatusDraft
 	if req.ID != nil && *req.ID > 0 {
@@ -65,10 +65,11 @@ func (req *TODeliveryServiceRequest) Validate(db *sqlx.DB) []error {
 		"deliveryservice": validation.Validate(req.DeliveryService, validation.Required),
 		"status":          validation.Validate(req.Status, validation.Required, validation.By(validTransition)),
 	}
-
 	errs := tovalidate.ToErrors(errMap)
 	// ensure the deliveryservice requested is valid
-	errs = append(errs, req.DeliveryService.Validate(tx))
+	if err := req.DeliveryService.Validate(tx); err != nil {
+		errs = append(errs, err)
+	}
 	commitTx = true
 	return errs
 }
diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go b/traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go
index ddb76e4..f6bb1bd 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go
@@ -32,6 +32,7 @@ import (
 	"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"
 
 	"github.com/go-ozzo/ozzo-validation"
@@ -350,24 +351,18 @@ func GetReplaceHandler(db *sqlx.DB) http.HandlerFunc {
 			}
 		}()
 
-		// if the object has tenancy enabled, check that user is able to access the tenant
-		// check user tenancy access to this resource.
-		row := db.QueryRow("SELECT xml_id FROM deliveryservice WHERE id = $1", *dsId)
-		var xmlId string
-		row.Scan(&xmlId)
-		hasAccess, err, apiStatus := tenant.HasTenant(user, xmlId, tx.Tx)
-		if !hasAccess {
-			switch apiStatus {
-			case tc.SystemError:
-				handleErrs(http.StatusInternalServerError, err)
-				return
-			case tc.DataMissingError:
-				handleErrs(http.StatusBadRequest, err)
-				return
-			case tc.ForbiddenError:
-				handleErrs(http.StatusForbidden, err)
-				return
-			}
+		xmlID, ok, err := deliveryservice.GetXMLID(tx.Tx, *dsId)
+		if err != nil {
+			api.HandleErr(w, r, http.StatusInternalServerError, nil, errors.New("deliveryserviceserver getting XMLID: "+err.Error()))
+			return
+		}
+		if !ok {
+			api.HandleErr(w, r, http.StatusBadRequest, errors.New("no delivery service with that ID exists"), nil)
+			return
+		}
+		if userErr, sysErr, errCode := tenant.Check(user, xmlID, tx.Tx); userErr != nil || sysErr != nil {
+			api.HandleErr(w, r, errCode, userErr, sysErr)
+			return
 		}
 
 		if *payload.Replace {
@@ -466,7 +461,6 @@ func GetCreateHandler(db *sqlx.DB) http.HandlerFunc {
 			return
 		}
 
-		// perform the insert transaction
 		rollbackTransaction := true
 		tx, err := db.Beginx()
 		if err != nil {
@@ -484,25 +478,9 @@ func GetCreateHandler(db *sqlx.DB) http.HandlerFunc {
 			}
 		}()
 
-		// if the object has tenancy enabled, check that user is able to access the tenant
-		// check user tenancy access to this resource.
-		hasAccess, err, apiStatus := tenant.HasTenant(user, xmlId, tx.Tx)
-		if !hasAccess {
-			switch apiStatus {
-			case tc.SystemError:
-				handleErrs(http.StatusInternalServerError, err)
-				return
-			case tc.DataMissingError:
-				handleErrs(http.StatusBadRequest, err)
-				return
-			case tc.ForbiddenError:
-				handleErrs(http.StatusForbidden, err)
-				return
-			default:
-				e := http.StatusInternalServerError
-				handleErrs(e, err)
-				return
-			}
+		if userErr, sysErr, errCode := tenant.Check(user, xmlId, tx.Tx); userErr != nil || sysErr != nil {
+			api.HandleErr(w, r, errCode, userErr, sysErr)
+			return
 		}
 
 		row := db.QueryRow(selectDeliveryService(), xmlId)