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/06/01 16:07:02 UTC

[incubator-trafficcontrol] 02/05: fix n+1 tenancy query issue and []uint8 DeepCachingType mismatch

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/incubator-trafficcontrol.git

commit 74027fa3900bfe720ec210f1e4962e84561ca495
Author: Dylan Volz <Dy...@comcast.com>
AuthorDate: Thu May 31 20:03:19 2018 -0600

    fix n+1 tenancy query issue and []uint8 DeepCachingType mismatch
---
 .../traffic_ops_golang/dbhelpers/db_helpers.go     | 12 +++---
 .../deliveryservice/deliveryservicesv12.go         |  9 +----
 .../deliveryservice/deliveryservicesv13.go         | 45 +++++++++++++++++-----
 3 files changed, 43 insertions(+), 23 deletions(-)

diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
index 3349504..c396d74 100644
--- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
+++ b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
@@ -36,12 +36,12 @@ type WhereColumnInfo struct {
 	Checker func(string) error
 }
 
-const baseWhere = "\nWHERE"
-const baseOrderBy = "\nORDER BY"
+const BaseWhere = "\nWHERE"
+const BaseOrderBy = "\nORDER BY"
 
 func BuildWhereAndOrderBy(parameters map[string]string, queryParamsToSQLCols map[string]WhereColumnInfo) (string, string, map[string]interface{}, []error) {
-	whereClause := baseWhere
-	orderBy := baseOrderBy
+	whereClause := BaseWhere
+	orderBy := BaseOrderBy
 	var criteria string
 	var queryValues map[string]interface{}
 	var errs []error
@@ -63,10 +63,10 @@ func BuildWhereAndOrderBy(parameters map[string]string, queryParamsToSQLCols map
 			log.Debugln("Incorrect name for orderby: ", orderby)
 		}
 	}
-	if whereClause == baseWhere {
+	if whereClause == BaseWhere {
 		whereClause = ""
 	}
-	if orderBy == baseOrderBy {
+	if orderBy == BaseOrderBy {
 		orderBy = ""
 	}
 	log.Debugf("\n--\n Where: %s \n Order By: %s", whereClause, orderBy)
diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv12.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv12.go
index cd2d22a..9d9c2f0 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv12.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv12.go
@@ -28,7 +28,6 @@ import (
 	"regexp"
 	"strings"
 
-	"github.com/apache/incubator-trafficcontrol/lib/go-log"
 	"github.com/apache/incubator-trafficcontrol/lib/go-tc"
 	"github.com/apache/incubator-trafficcontrol/lib/go-util"
 	"github.com/apache/incubator-trafficcontrol/traffic_ops/traffic_ops_golang/api"
@@ -356,7 +355,7 @@ func CreateV12(db *sqlx.DB, cfg config.Config) http.HandlerFunc {
 
 func (ds *TODeliveryServiceV12) Read(db *sqlx.DB, params map[string]string, user auth.CurrentUser) ([]interface{}, []error, tc.ApiErrorType) {
 	returnable := []interface{}{}
-	dses, errs, errType := readGetDeliveryServices(params, db)
+	dses, errs, errType := readGetDeliveryServices(params, db, user)
 	if len(errs) > 0 {
 		for _, err := range errs {
 			if err.Error() == `id cannot parse to integer` {
@@ -366,12 +365,6 @@ func (ds *TODeliveryServiceV12) Read(db *sqlx.DB, params map[string]string, user
 		return nil, errs, errType
 	}
 
-	dses, err := filterAuthorized(dses, user, db)
-	if err != nil {
-		log.Errorln("Checking tenancy: " + err.Error())
-		return nil, []error{errors.New("Error checking tenancy.")}, tc.SystemError
-	}
-
 	for _, ds := range dses {
 		returnable = append(returnable, ds.DeliveryServiceNullableV12)
 	}
diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv13.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv13.go
index 4d45bad..e3957c3 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv13.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservicesv13.go
@@ -276,7 +276,7 @@ func create(db *sql.DB, cfg config.Config, user *auth.CurrentUser, ds tc.Deliver
 
 func (ds *TODeliveryServiceV13) Read(db *sqlx.DB, params map[string]string, user auth.CurrentUser) ([]interface{}, []error, tc.ApiErrorType) {
 	returnable := []interface{}{}
-	dses, errs, errType := readGetDeliveryServices(params, db)
+	dses, errs, errType := readGetDeliveryServices(params, db, user)
 	if len(errs) > 0 {
 		for _, err := range errs {
 			if err.Error() == `id cannot parse to integer` { // TODO create const for string
@@ -286,12 +286,6 @@ func (ds *TODeliveryServiceV13) Read(db *sqlx.DB, params map[string]string, user
 		return nil, errs, errType
 	}
 
-	dses, err := filterAuthorized(dses, user, db)
-	if err != nil {
-		log.Errorln("Checking tenancy: " + err.Error())
-		return nil, []error{errors.New("Error checking tenancy.")}, tc.SystemError
-	}
-
 	for _, ds := range dses {
 		returnable = append(returnable, ds)
 	}
@@ -616,7 +610,28 @@ func filterAuthorized(dses []tc.DeliveryServiceNullableV13, user auth.CurrentUse
 	return newDSes, nil
 }
 
-func readGetDeliveryServices(params map[string]string, db *sqlx.DB) ([]tc.DeliveryServiceNullableV13, []error, tc.ApiErrorType) {
+func addTenancyCheck(where string, queryValues map[string]interface{}, user auth.CurrentUser, db *sqlx.DB) (string, map[string]interface{}, error) {
+	if where == "" {
+		where = dbhelpers.BaseWhere + " ds.tenant_id = ANY((:accessibleTenants)::::bigint[])"
+	} else {
+		where += " AND ds.tenant_id = ANY((:accessibleTenants)::::bigint[])"
+	}
+
+	tenants, err := tenant.GetUserTenantList(user, db)
+	if err != nil {
+		return "", queryValues, err
+	}
+
+	tenantIDs := make([]int, len(tenants))
+	for i, tenant := range tenants {
+		tenantIDs[i] = tenant.ID
+	}
+	queryValues["accessibleTenants"] = pq.Array(tenantIDs)
+
+	return where, queryValues, nil
+}
+
+func readGetDeliveryServices(params map[string]string, db *sqlx.DB, user auth.CurrentUser) ([]tc.DeliveryServiceNullableV13, []error, tc.ApiErrorType) {
 	if strings.HasSuffix(params["id"], ".json") {
 		params["id"] = params["id"][:len(params["id"])-len(".json")]
 	}
@@ -632,8 +647,20 @@ func readGetDeliveryServices(params map[string]string, db *sqlx.DB) ([]tc.Delive
 		return nil, errs, tc.DataConflictError
 	}
 
+	if tenant.IsTenancyEnabled(db) {
+		log.Debugln("Tenancy is enabled")
+		var err error
+		where, queryValues, err = addTenancyCheck(where, queryValues, user, db)
+		if err != nil {
+			log.Errorln("received error querying for user's tenants: " + err.Error())
+			return nil, []error{tc.DBError}, tc.SystemError
+		}
+	}
 	query := selectQuery() + where + orderBy
 
+	log.Debugln("generated deliveryServices query: " + query)
+	log.Debugf("executing with values: %++v\n", queryValues)
+
 	tx, err := db.Beginx()
 	if err != nil {
 		log.Errorln("could not begin transaction: " + err.Error())
@@ -965,7 +992,7 @@ ds.ccr_dns_ttl,
 ds.cdn_id,
 cdn.name as cdnName,
 ds.check_path,
-ds.deep_caching_type,
+ds.deep_caching_type::::text as deep_caching_type,
 ds.display_name,
 ds.dns_bypass_cname,
 ds.dns_bypass_ip,

-- 
To stop receiving notification emails like this one, please contact
rob@apache.org.