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 <dylan_v...@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 r...@apache.org.