You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by da...@apache.org on 2018/07/04 02:39:35 UTC

[trafficcontrol] 10/15: convert tenancy package to new interfaces

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

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

commit 840729ac74e150edba557151bb14b59680eaabec
Author: Dylan Volz <Dy...@comcast.com>
AuthorDate: Wed Jun 27 12:16:08 2018 -0600

    convert tenancy package to new interfaces
---
 lib/go-tc/parameters.go                          | 52 +++++++------
 traffic_ops/traffic_ops_golang/routes.go         | 19 +++--
 traffic_ops/traffic_ops_golang/tenant/tenancy.go | 96 ++++--------------------
 3 files changed, 51 insertions(+), 116 deletions(-)

diff --git a/lib/go-tc/parameters.go b/lib/go-tc/parameters.go
index 9bbf3b9..1151f30 100644
--- a/lib/go-tc/parameters.go
+++ b/lib/go-tc/parameters.go
@@ -79,7 +79,7 @@ type ProfileParameterByNamePost struct {
 	Value      *string `json:"value"`
 }
 
-func (p *ProfileParameterByNamePost) Validate(tx *sql.Tx) error {
+func (p *ProfileParameterByNamePost) Validate(tx *sql.Tx) []error {
 	errs := []string{}
 	if p.ConfigFile == nil || *p.ConfigFile == "" {
 		errs = append(errs, "configFile")
@@ -94,7 +94,7 @@ func (p *ProfileParameterByNamePost) Validate(tx *sql.Tx) error {
 		errs = append(errs, "value")
 	}
 	if len(errs) > 0 {
-		return errors.New("required fields missing: " + strings.Join(errs, ", "))
+		return []error{errors.New("required fields missing: " + strings.Join(errs, ", "))}
 	}
 	return nil
 }
@@ -121,16 +121,18 @@ func (pp *ProfileParametersByNamePost) UnmarshalJSON(bts []byte) error {
 	return nil
 }
 
-func (pp *ProfileParametersByNamePost) Validate(tx *sql.Tx) error {
-	errs := []string{}
+func (pp *ProfileParametersByNamePost) Validate(tx *sql.Tx) []error {
+	errs := []error{}
 	ppArr := ([]ProfileParameterByNamePost)(*pp)
 	for i, profileParam := range ppArr {
-		if err := profileParam.Validate(tx); err != nil {
-			errs = append(errs, "object "+strconv.Itoa(i)+": "+err.Error())
+		if ppErrs := profileParam.Validate(tx); len(ppErrs) > 0 {
+			for _, err := range ppErrs {
+				errs = append(errs, errors.New("object "+strconv.Itoa(i)+": "+err.Error()))
+			}
 		}
 	}
 	if len(errs) > 0 {
-		return errors.New("validate errors: " + strings.Join(errs, "; "))
+		return errs
 	}
 	return nil
 }
@@ -158,25 +160,25 @@ func (pp *PostProfileParam) Sanitize(tx *sql.Tx) {
 	}
 }
 
-func (pp *PostProfileParam) Validate(tx *sql.Tx) error {
+func (pp *PostProfileParam) Validate(tx *sql.Tx) []error {
 	pp.Sanitize(tx)
-	errs := []string{}
+	errs := []error{}
 	if pp.ProfileID == nil {
-		errs = append(errs, "profileId missing")
+		errs = append(errs, errors.New("profileId missing"))
 	} else if ok, err := ProfileExists(*pp.ProfileID, tx); err != nil {
-		errs = append(errs, "checking profile ID "+strconv.Itoa(int(*pp.ProfileID))+" existence: "+err.Error())
+		errs = append(errs, errors.New("checking profile ID "+strconv.Itoa(int(*pp.ProfileID))+" existence: "+err.Error()))
 	} else if !ok {
-		errs = append(errs, "no profile with ID "+strconv.Itoa(int(*pp.ProfileID))+" exists")
+		errs = append(errs, errors.New("no profile with ID "+strconv.Itoa(int(*pp.ProfileID))+" exists"))
 	}
 	if pp.ParamIDs == nil {
-		errs = append(errs, "paramIds missing")
+		errs = append(errs, errors.New("paramIds missing"))
 	} else if ok, err := ParamsExist(*pp.ParamIDs, tx); err != nil {
-		errs = append(errs, fmt.Sprintf("checking parameter IDs %v existence: "+err.Error(), *pp.ParamIDs))
+		errs = append(errs, errors.New(fmt.Sprintf("checking parameter IDs %v existence: "+err.Error(), *pp.ParamIDs)))
 	} else if !ok {
-		errs = append(errs, fmt.Sprintf("parameters with IDs %v don't all exist", *pp.ParamIDs))
+		errs = append(errs, errors.New(fmt.Sprintf("parameters with IDs %v don't all exist", *pp.ParamIDs)))
 	}
 	if len(errs) > 0 {
-		return errors.New("validate errors: " + strings.Join(errs, ", "))
+		return errs
 	}
 	return nil
 }
@@ -193,26 +195,26 @@ func (pp *PostParamProfile) Sanitize(tx *sql.Tx) {
 	}
 }
 
-func (pp *PostParamProfile) Validate(tx *sql.Tx) error {
+func (pp *PostParamProfile) Validate(tx *sql.Tx) []error {
 	pp.Sanitize(tx)
 
-	errs := []string{}
+	errs := []error{}
 	if pp.ParamID == nil {
-		errs = append(errs, "paramId missing")
+		errs = append(errs, errors.New("paramId missing"))
 	} else if ok, err := ParamExists(*pp.ParamID, tx); err != nil {
-		errs = append(errs, "checking param ID "+strconv.Itoa(int(*pp.ParamID))+" existence: "+err.Error())
+		errs = append(errs, errors.New("checking param ID "+strconv.Itoa(int(*pp.ParamID))+" existence: "+err.Error()))
 	} else if !ok {
-		errs = append(errs, "no parameter with ID "+strconv.Itoa(int(*pp.ParamID))+" exists")
+		errs = append(errs, errors.New("no parameter with ID "+strconv.Itoa(int(*pp.ParamID))+" exists"))
 	}
 	if pp.ProfileIDs == nil {
-		errs = append(errs, "profileIds missing")
+		errs = append(errs, errors.New("profileIds missing"))
 	} else if ok, err := ProfilesExist(*pp.ProfileIDs, tx); err != nil {
-		errs = append(errs, fmt.Sprintf("checking profiles IDs %v existence: "+err.Error(), *pp.ProfileIDs))
+		errs = append(errs, errors.New(fmt.Sprintf("checking profiles IDs %v existence: "+err.Error(), *pp.ProfileIDs)))
 	} else if !ok {
-		errs = append(errs, fmt.Sprintf("profiles with IDs %v don't all exist", *pp.ProfileIDs))
+		errs = append(errs, errors.New(fmt.Sprintf("profiles with IDs %v don't all exist", *pp.ProfileIDs)))
 	}
 	if len(errs) > 0 {
-		return errors.New("validate errors: " + strings.Join(errs, ", "))
+		return errs
 	}
 	return nil
 }
diff --git a/traffic_ops/traffic_ops_golang/routes.go b/traffic_ops/traffic_ops_golang/routes.go
index 6b3b215..d26503d 100644
--- a/traffic_ops/traffic_ops_golang/routes.go
+++ b/traffic_ops/traffic_ops_golang/routes.go
@@ -304,21 +304,20 @@ func Routes(d ServerData) ([]Route, []RawRoute, http.Handler, error) {
 		{1.1, http.MethodGet, `profiles/{id}/unassigned_parameters/?(\.json)?$`, profileparameter.GetUnassigned(d.DB.DB), auth.PrivLevelReadOnly, Authenticated, nil},
 		{1.1, http.MethodGet, `profiles/name/{name}/parameters/?(\.json)?$`, profileparameter.GetProfileName(d.DB.DB), auth.PrivLevelReadOnly, Authenticated, nil},
 		{1.1, http.MethodGet, `parameters/profile/{name}/?(\.json)?$`, profileparameter.GetProfileName(d.DB.DB), auth.PrivLevelReadOnly, Authenticated, nil},
-		{1.1, http.MethodPost, `profiles/name/{name}/parameters/?$`, profileparameter.PostProfileParamsByName(d.DB.DB), auth.PrivLevelOperations, Authenticated, nil},
-		{1.1, http.MethodPost, `profiles/{id}/parameters/?$`, profileparameter.PostProfileParamsByID(d.DB.DB), auth.PrivLevelOperations, Authenticated, nil},
+		{1.1, http.MethodPost, `profiles/name/{name}/parameters/?$`, profileparameter.PostProfileParamsByName, auth.PrivLevelOperations, Authenticated, nil},
+		{1.1, http.MethodPost, `profiles/{id}/parameters/?$`, profileparameter.PostProfileParamsByID, auth.PrivLevelOperations, Authenticated, nil},
 		{1.1, http.MethodGet, `profileparameters/?(\.json)?$`, api.ReadHandler(profileparameter.GetTypeSingleton()), auth.PrivLevelReadOnly, Authenticated, nil},
 		{1.1, http.MethodPost, `profileparameters/?$`, api.CreateHandler(profileparameter.GetTypeSingleton()), auth.PrivLevelOperations, Authenticated, nil},
-		{1.1, http.MethodPost, `profileparameter/?$`, profileparameter.PostProfileParam(d.DB.DB), auth.PrivLevelOperations, Authenticated, nil},
-		{1.1, http.MethodPost, `parameterprofile/?$`, profileparameter.PostParamProfile(d.DB.DB), auth.PrivLevelOperations, Authenticated, nil},
+		{1.1, http.MethodPost, `profileparameter/?$`, profileparameter.PostProfileParam, auth.PrivLevelOperations, Authenticated, nil},
+		{1.1, http.MethodPost, `parameterprofile/?$`, profileparameter.PostParamProfile, auth.PrivLevelOperations, Authenticated, nil},
 		{1.1, http.MethodDelete, `profileparameters/{profileId}/{parameterId}$`, api.DeleteHandler(profileparameter.GetTypeSingleton()), auth.PrivLevelOperations, Authenticated, nil},
 
-
 		//Tenants
-		{1.1, http.MethodGet, `tenants/?(\.json)?$`, api.ReadHandler(tenant.GetRefType(), d.DB), auth.PrivLevelReadOnly, Authenticated, nil},
-		{1.1, http.MethodGet, `tenants/{id}$`, api.ReadHandler(tenant.GetRefType(), d.DB), auth.PrivLevelReadOnly, Authenticated, nil},
-		{1.1, http.MethodPut, `tenants/{id}$`, api.UpdateHandler(tenant.GetRefType(), d.DB), auth.PrivLevelOperations, Authenticated, nil},
-		{1.1, http.MethodPost, `tenants/?$`, api.CreateHandler(tenant.GetRefType(), d.DB), auth.PrivLevelOperations, Authenticated, nil},
-		{1.1, http.MethodDelete, `tenants/{id}$`, api.DeleteHandler(tenant.GetRefType(), d.DB), auth.PrivLevelOperations, Authenticated, nil},
+		{1.1, http.MethodGet, `tenants/?(\.json)?$`, api.ReadHandler(tenant.GetTypeSingleton()), auth.PrivLevelReadOnly, Authenticated, nil},
+		{1.1, http.MethodGet, `tenants/{id}$`, api.ReadHandler(tenant.GetTypeSingleton()), auth.PrivLevelReadOnly, Authenticated, nil},
+		{1.1, http.MethodPut, `tenants/{id}$`, api.UpdateHandler(tenant.GetTypeSingleton()), auth.PrivLevelOperations, Authenticated, nil},
+		{1.1, http.MethodPost, `tenants/?$`, api.CreateHandler(tenant.GetTypeSingleton()), auth.PrivLevelOperations, Authenticated, nil},
+		{1.1, http.MethodDelete, `tenants/{id}$`, api.DeleteHandler(tenant.GetTypeSingleton()), auth.PrivLevelOperations, Authenticated, nil},
 
 		//CRConfig
 		{1.1, http.MethodGet, `cdns/{cdn}/snapshot/?$`, crconfig.SnapshotGetHandler(d.DB, d.Config), auth.PrivLevelReadOnly, Authenticated, nil},
diff --git a/traffic_ops/traffic_ops_golang/tenant/tenancy.go b/traffic_ops/traffic_ops_golang/tenant/tenancy.go
index cd8edd3..6b66141 100644
--- a/traffic_ops/traffic_ops_golang/tenant/tenancy.go
+++ b/traffic_ops/traffic_ops_golang/tenant/tenancy.go
@@ -117,7 +117,7 @@ func CheckID(tx *sql.Tx, user *auth.CurrentUser, dsID int) (error, error, int) {
 // NOTE: This method does not use the use_tenancy parameter and if this method is being used
 // to control tenancy the parameter must be checked. The method IsResourceAuthorizedToUser checks the use_tenancy parameter
 // and should be used for this purpose in most cases.
-func GetUserTenantList(user auth.CurrentUser, db *sqlx.DB) ([]TOTenant, error) {
+func GetUserTenantList(user auth.CurrentUser, db *sqlx.DB) ([]tc.TenantNullable, error) {
 	query := `WITH RECURSIVE q AS (SELECT id, name, active, parent_id, last_updated FROM tenant WHERE id = $1
 	UNION SELECT t.id, t.name, t.active, t.parent_id, t.last_updated  FROM tenant t JOIN q ON q.id = t.parent_id)
 	SELECT id, name, active, parent_id, last_updated FROM q;`
@@ -136,14 +136,14 @@ func GetUserTenantList(user auth.CurrentUser, db *sqlx.DB) ([]TOTenant, error) {
 	}
 	defer rows.Close()
 
-	tenants := []TOTenant{}
+	tenants := []tc.TenantNullable{}
 
 	for rows.Next() {
 		if err := rows.Scan(&tenantID, &name, &active, &parentID, &lastUpdated); err != nil {
 			return nil, err
 		}
 
-		tenants = append(tenants, TOTenant{ID: &tenantID, Name: &name, Active: &active, ParentID: &parentID})
+		tenants = append(tenants, tc.TenantNullable{ID: &tenantID, Name: &name, Active: &active, ParentID: &parentID})
 	}
 
 	return tenants, nil
@@ -292,7 +292,6 @@ func IsResourceAuthorizedToUserTx(resourceTenantID int, user *auth.CurrentUser,
 	}
 }
 
-
 // TOTenant provides a local type against which to define methods
 type TOTenant struct {
 	ReqInfo *api.APIInfo `json:"-"`
@@ -353,7 +352,7 @@ func (ten *TOTenant) SetKeys(keys map[string]interface{}) {
 }
 
 // Validate fulfills the api.Validator interface
-func (ten TOTenant) Validate(db *sqlx.DB) []error {
+func (ten TOTenant) Validate() []error {
 	errs := validation.Errors{
 		"name":     validation.Validate(ten.Name, validation.Required),
 		"active":   validation.Validate(ten.Active), // only validate it's boolean
@@ -369,24 +368,8 @@ func (ten TOTenant) Validate(db *sqlx.DB) []error {
 //generic error message returned
 //The insert sql returns the id and lastUpdated values of the newly inserted tenant and have
 //to be added to the struct
-func (ten *TOTenant) Create(db *sqlx.DB, user auth.CurrentUser) (error, tc.ApiErrorType) {
-	rollbackTransaction := true
-	tx, err := db.Beginx()
-	defer func() {
-		if tx == nil || !rollbackTransaction {
-			return
-		}
-		err := tx.Rollback()
-		if err != nil {
-			log.Errorln(errors.New("rolling back transaction: " + err.Error()))
-		}
-	}()
-
-	if err != nil {
-		log.Error.Printf("could not begin transaction: %v", err)
-		return tc.DBError, tc.SystemError
-	}
-	resultRows, err := tx.NamedQuery(insertQuery(), ten)
+func (ten *TOTenant) Create() (error, tc.ApiErrorType) {
+	resultRows, err := ten.ReqInfo.Tx.NamedQuery(insertQuery(), ten)
 	if err != nil {
 		if pqErr, ok := err.(*pq.Error); ok {
 			err, eType := dbhelpers.ParsePQUniqueConstraintError(pqErr)
@@ -421,17 +404,12 @@ func (ten *TOTenant) Create(db *sqlx.DB, user auth.CurrentUser) (error, tc.ApiEr
 	}
 	ten.SetKeys(map[string]interface{}{"id": id})
 	ten.LastUpdated = &lastUpdated
-	err = tx.Commit()
-	if err != nil {
-		log.Errorln("Could not commit transaction: ", err)
-		return tc.DBError, tc.SystemError
-	}
-	rollbackTransaction = false
+
 	return nil, tc.NoError
 }
 
 // Read implements the tc.Reader interface
-func (ten *TOTenant) Read(db *sqlx.DB, parameters map[string]string, user auth.CurrentUser) ([]interface{}, []error, tc.ApiErrorType) {
+func (ten *TOTenant) Read(parameters map[string]string) ([]interface{}, []error, tc.ApiErrorType) {
 	var rows *sqlx.Rows
 
 	// Query Parameters to Database Query column mappings
@@ -451,7 +429,7 @@ func (ten *TOTenant) Read(db *sqlx.DB, parameters map[string]string, user auth.C
 	query := selectQuery() + where + orderBy
 	log.Debugln("Query is ", query)
 
-	rows, err := db.NamedQuery(query, queryValues)
+	rows, err := ten.ReqInfo.Tx.NamedQuery(query, queryValues)
 	if err != nil {
 		log.Errorf("Error querying tenants: %v", err)
 		return nil, []error{tc.DBError}, tc.SystemError
@@ -476,25 +454,9 @@ func (ten *TOTenant) Read(db *sqlx.DB, parameters map[string]string, user auth.C
 //ParsePQUniqueConstraintError is used to determine if a tenant with conflicting values exists
 //if so, it will return an errorType of DataConflict and the type should be appended to the
 //generic error message returned
-func (ten *TOTenant) Update(db *sqlx.DB, user auth.CurrentUser) (error, tc.ApiErrorType) {
-	rollbackTransaction := true
-	tx, err := db.Beginx()
-	defer func() {
-		if tx == nil || !rollbackTransaction {
-			return
-		}
-		err := tx.Rollback()
-		if err != nil {
-			log.Errorln(errors.New("rolling back transaction: " + err.Error()))
-		}
-	}()
-
-	if err != nil {
-		log.Error.Printf("could not begin transaction: %v", err)
-		return tc.DBError, tc.SystemError
-	}
+func (ten *TOTenant) Update() (error, tc.ApiErrorType) {
 	log.Debugf("about to run exec query: %s with tenant: %++v", updateQuery(), ten)
-	resultRows, err := tx.NamedQuery(updateQuery(), ten)
+	resultRows, err := ten.ReqInfo.Tx.NamedQuery(updateQuery(), ten)
 	if err != nil {
 		if pqErr, ok := err.(*pq.Error); ok {
 			err, eType := dbhelpers.ParsePQUniqueConstraintError(pqErr)
@@ -525,41 +487,19 @@ func (ten *TOTenant) Update(db *sqlx.DB, user auth.CurrentUser) (error, tc.ApiEr
 		}
 		return fmt.Errorf("this update affected too many rows: %d", rowsAffected), tc.SystemError
 	}
-	err = tx.Commit()
-	if err != nil {
-		log.Errorln("Could not commit transaction: ", err)
-		return tc.DBError, tc.SystemError
-	}
-	rollbackTransaction = false
 	return nil, tc.NoError
 }
 
 //Delete implements the Deleter interface
 //all implementations of Deleter should use transactions and return the proper errorType
-func (ten *TOTenant) Delete(db *sqlx.DB, user auth.CurrentUser) (error, tc.ApiErrorType) {
+func (ten *TOTenant) Delete() (error, tc.ApiErrorType) {
 	if ten.ID == nil {
 		// should never happen...
 		return errors.New("invalid tenant: id is nil"), tc.SystemError
 	}
-	rollbackTransaction := true
-	tx, err := db.Beginx()
-	defer func() {
-		if tx == nil || !rollbackTransaction {
-			return
-		}
-		err := tx.Rollback()
-		if err != nil {
-			log.Errorln(errors.New("rolling back transaction: " + err.Error()))
-		}
-	}()
-
-	if err != nil {
-		log.Error.Printf("could not begin transaction: %v", err)
-		return tc.DBError, tc.SystemError
-	}
 
 	log.Debugf("about to run exec query: %s with tenant: %++v", deleteQuery(), ten)
-	result, err := tx.NamedExec(deleteQuery(), ten)
+	result, err := ten.ReqInfo.Tx.NamedExec(deleteQuery(), ten)
 	if err != nil {
 		if pqErr, ok := err.(*pq.Error); ok {
 			err = fmt.Errorf("pqErr is %++v\n", pqErr)
@@ -579,7 +519,7 @@ func (ten *TOTenant) Delete(db *sqlx.DB, user auth.CurrentUser) (error, tc.ApiEr
 
 			// another query to get tenant name for the error message
 			name := strconv.Itoa(*ten.ID)
-			if err := db.QueryRow(`SELECT name FROM tenant WHERE id = $1`, *ten.ID).Scan(&name); err != nil {
+			if err := ten.ReqInfo.Tx.QueryRow(`SELECT name FROM tenant WHERE id = $1`, *ten.ID).Scan(&name); err != nil {
 				// use ID as a backup for name the error -- this should never happen
 				log.Debugf("error getting tenant name: %++v", err)
 			}
@@ -600,12 +540,7 @@ func (ten *TOTenant) Delete(db *sqlx.DB, user auth.CurrentUser) (error, tc.ApiEr
 		}
 		return fmt.Errorf("this delete affected too many rows: %d", rowsAffected), tc.SystemError
 	}
-	err = tx.Commit()
-	if err != nil {
-		log.Errorln("Could not commit transaction: ", err)
-		return tc.DBError, tc.SystemError
-	}
-	rollbackTransaction = false
+
 	return nil, tc.NoError
 }
 
@@ -667,4 +602,3 @@ func getDSTenantIDByIDTx(tx *sql.Tx, id int) (*int, bool, error) {
 	}
 	return tenantID, true, nil
 }
-