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/07/06 14:44:25 UTC

[trafficcontrol] 03/03: Updated according to comments on PR

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 fb4d2490acffbf906174ffcde7486261627d0fbb
Author: moltzaum <ma...@moltzau.net>
AuthorDate: Thu Jul 5 14:45:33 2018 -0600

    Updated according to comments on PR
    
    Most significant issue was efficiency in using two separate queries instead of a join. Minor changes in coding style was added. Removed some hard-coded values in favor of variables.
---
 lib/go-tc/v13/domains.go                        | 25 ------------
 traffic_ops/client/v13/cdn_domains.go           |  2 +-
 traffic_ops/testing/api/v13/cdn_domains_test.go |  2 +-
 traffic_ops/traffic_ops_golang/cdn/domains.go   | 51 +++++++------------------
 4 files changed, 16 insertions(+), 64 deletions(-)

diff --git a/lib/go-tc/v13/domains.go b/lib/go-tc/v13/domains.go
index 4c9cb46..7908c4f 100644
--- a/lib/go-tc/v13/domains.go
+++ b/lib/go-tc/v13/domains.go
@@ -19,46 +19,21 @@ package v13
  * under the License.
  */
 
-/*
-TODO: Adapt the comments to be better used by swagger. I am writing comments
-that look similar to others in `go-tc/cdns.go`, but I'm unsure how it will
-end up looking. Note: I also made a few assumptions with the "required" field
-*/
-
-// A List of Domains Response
-// swagger:response DomainsResponse
-// in: body
 type DomainsResponse struct {
-	// in: body
 	Response []Domain `json:"response"`
 }
 
-// Domain ...
 type Domain struct {
 
-	// Profile ID
-	//
-	// required: true
 	ProfileID int `json:"profileId" db:"profile_id"`
 
-	// Parameter ID
-	//
-	// required: false
 	ParameterID int `json:"parameterId" db:"parameter_id"`
 
-	// Profile Name
-	//
-	// required: true
 	ProfileName string `json:"profileName" db:"profile_name"`
 
-	// Profile Description
-	//
-	// required: true
 	ProfileDescription string `json:"profileDescription" db:"profile_description"`
 
 	// DomainName of the CDN
-	//
-	// required: true
 	DomainName string `json:"domainName" db:"domain_name"`
 }
 
diff --git a/traffic_ops/client/v13/cdn_domains.go b/traffic_ops/client/v13/cdn_domains.go
index 42e748a..7da297f 100644
--- a/traffic_ops/client/v13/cdn_domains.go
+++ b/traffic_ops/client/v13/cdn_domains.go
@@ -21,7 +21,7 @@ import (
 
 func (to *Session) GetDomains() ([]v13.Domain, ReqInf, error) {
 	var data v13.DomainsResponse
-	inf, err := get(to, "/api/1.3/cdns/domains", &data)
+	inf, err := get(to, apiBase + "/cdns/domains", &data)
 	if err != nil {
 		return nil, inf, err
 	}
diff --git a/traffic_ops/testing/api/v13/cdn_domains_test.go b/traffic_ops/testing/api/v13/cdn_domains_test.go
index fd99e23..ea2523d 100644
--- a/traffic_ops/testing/api/v13/cdn_domains_test.go
+++ b/traffic_ops/testing/api/v13/cdn_domains_test.go
@@ -18,7 +18,7 @@ package v13
 import (
 	"testing"
 
-	log "github.com/apache/trafficcontrol/lib/go-log"
+	"github.com/apache/trafficcontrol/lib/go-log"
 )
 
 func GetTestDomains(t *testing.T) {
diff --git a/traffic_ops/traffic_ops_golang/cdn/domains.go b/traffic_ops/traffic_ops_golang/cdn/domains.go
index f85fddf..c3b6f27 100644
--- a/traffic_ops/traffic_ops_golang/cdn/domains.go
+++ b/traffic_ops/traffic_ops_golang/cdn/domains.go
@@ -22,51 +22,37 @@ package cdn
 import (
 	"fmt"
 	"net/http"
-
 	"github.com/apache/trafficcontrol/lib/go-tc/v13"
 	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
 	"github.com/jmoiron/sqlx"
 )
 
-// GetDomainsList gathers a list of domains, except for the domain name.
-// There seems to be an issue nesting queries (performing a query while
-// rows.Next still needs to iterate). https://github.com/lib/pq/issues/81
-func getDomainsList(tx *sqlx.Tx) ([]v13.Domain, []int, error) {
+const RouterProfilePrefix = "CCR"
 
-	var (
-		cdn  int
-		id   int
-		name string
-		desc string
-	)
+func getDomainsList(tx *sqlx.Tx) ([]v13.Domain, error) {
 
 	domains := []v13.Domain{}
-	cdn_ids := []int{}
 
-	q := `SELECT cdn, id, name, description FROM Profile WHERE name LIKE 'CCR%'`
+	q := `SELECT p.id, p.name, p.description, domain_name FROM profile AS p
+	JOIN cdn ON p.cdn = cdn.id WHERE p.name LIKE '` + RouterProfilePrefix + `%'`;
+
 	rows, err := tx.Query(q)
 	if err != nil {
-		return nil, nil, fmt.Errorf("querying for profile: %s", err)
+		return nil, fmt.Errorf("querying for profile: %s", err)
 	}
 	defer rows.Close()
 
 	for rows.Next() {
-		if err := rows.Scan(&cdn, &id, &name, &desc); err != nil {
-			return nil, nil, fmt.Errorf("getting profile: %s", err)
-		}
 
-		elem := v13.Domain{
-			ProfileID:          id,
-			ParameterID:        -1,
-			ProfileName:        name,
-			ProfileDescription: desc,
+		d := v13.Domain{ParameterID: -1}
+		err := rows.Scan(&d.ProfileID, &d.ProfileName, &d.ProfileDescription, &d.DomainName)
+		if err != nil {
+			return nil, fmt.Errorf("getting profile: %s", err)
 		}
-
-		cdn_ids = append(cdn_ids, cdn)
-		domains = append(domains, elem)
+		domains = append(domains, d)
 	}
 
-	return domains, cdn_ids, nil
+	return domains, nil
 }
 
 func DomainsHandler(w http.ResponseWriter, r *http.Request) {
@@ -78,20 +64,11 @@ func DomainsHandler(w http.ResponseWriter, r *http.Request) {
 	}
 	defer inf.Close()
 
-	domains, cdn_ids, err := getDomainsList(inf.Tx)
+	domains, err := getDomainsList(inf.Tx)
 	if err != nil {
-		api.HandleErr(w, r, http.StatusInternalServerError, nil, err)
+		api.HandleErr(w, r, http.StatusInternalServerError, err, err)
 		return
 	}
 
-	for i, cdn := range cdn_ids {
-		row := inf.Tx.QueryRow(`SELECT DOMAIN_NAME FROM CDN WHERE id = $1`, cdn)
-		err := row.Scan(&domains[i].DomainName)
-		if err != nil {
-			api.HandleErr(w, r, http.StatusInternalServerError, nil, fmt.Errorf("getting domain name of cdn: %s", err))
-			return
-		}
-	}
-
 	api.WriteResp(w, r, domains)
 }