rawlinp commented on a change in pull request #2862: Add Traffic Ops Golang federations endpoints URL: https://github.com/apache/trafficcontrol/pull/2862#discussion_r222118780
########## File path: traffic_ops/traffic_ops_golang/federations/allfederations.go ########## @@ -0,0 +1,131 @@ +package federations + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import ( + "database/sql" + "errors" + "net/http" + + "github.com/apache/trafficcontrol/lib/go-tc" + "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api" +) + +func GetAll(w http.ResponseWriter, r *http.Request) { + inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil) + if userErr != nil || sysErr != nil { + api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr) + return + } + + feds := []FedInfo{} + err := error(nil) + allFederations := []tc.IAllFederation{} + + if cdnParam, ok := inf.Params["cdnName"]; ok { + cdnName := tc.CDNName(cdnParam) + feds, err = getAllFederationsForCDN(inf.Tx.Tx, cdnName) + if err != nil { + api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("federations.GetAll getting all federations: "+err.Error())) + return + } + allFederations = append(allFederations, tc.AllFederationCDN{CDNName: &cdnName}) + } else { + feds, err = getAllFederations(inf.Tx.Tx) + if err != nil { + api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("federations.GetAll getting all federations by CDN: "+err.Error())) + return + } + } + + fedsResolvers, err := getFederationResolvers(inf.Tx.Tx, fedInfoIDs(feds)) + if err != nil { + api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("federations.Get getting federations resolvers: "+err.Error())) + return + } + allFederations = addResolvers(allFederations, feds, fedsResolvers) + + api.WriteResp(w, r, allFederations) +} + +func getAllFederations(tx *sql.Tx) ([]FedInfo, error) { + qry := ` +SELECT + fds.federation, + fd.ttl, + fd.cname, + ds.xml_id +FROM + federation_deliveryservice fds + JOIN deliveryservice ds ON ds.id = fds.deliveryservice + JOIN federation fd ON fd.id = fds.federation +ORDER BY + ds.xml_id +` + rows, err := tx.Query(qry) + if err != nil { + return nil, errors.New("all federations querying: " + err.Error()) + } + defer rows.Close() + + feds := []FedInfo{} + for rows.Next() { + f := FedInfo{} + if err := rows.Scan(&f.ID, &f.TTL, &f.CName, &f.DS); err != nil { + return nil, errors.New("all federations scanning: " + err.Error()) + } + feds = append(feds, f) + } + return feds, nil +} + +func getAllFederationsForCDN(tx *sql.Tx, cdn tc.CDNName) ([]FedInfo, error) { + qry := ` +SELECT + fds.federation, + fd.ttl, + fd.cname, + ds.xml_id +FROM + federation_deliveryservice fds + JOIN deliveryservice ds ON ds.id = fds.deliveryservice + JOIN federation fd ON fd.id = fds.federation + JOIN cdn on cdn.id = ds.cdn_id +WHERE + cdn.name = $1 +ORDER BY + ds.xml_id +` + rows, err := tx.Query(qry, cdn) Review comment: Really I'd like to add some helpers that handle the whole "run query, defer Rows.Close(), read rows into slice of structs" process. I've got some example code here: https://github.com/rawlinp/trafficcontrol/compare/master...rawlinp:add-get-rows-helpers#diff-885017a84a9274d9fe8d218c5489c9f9R105 Right now we're kind of all over the place in terms of sql vs sqlx, Query vs Queryx vs NamedQuery, Scan vs StructScan, etc. The general process is the same in a lot of places; it's just the chosen methods for running the query and scanning that seem the most customized. Having a standardized set of helper functions for querying DB rows into slices of structs would allow us to enforce things better that are easy to forget about, like `defer rows.Close()` and checking `rows.Err()` after the `for rows.Next() {...}` block. The latter we basically aren't doing anywhere, but the API reference states it's necessary because `Next()` can return false with an error (which we aren't checking). I'm not necessarily saying that should be done in this PR, but I'm curious what you think of the idea. Maybe a lot of existing queries are too customized to really benefit from helper functions like that, but maybe reworking some of that would be a good thing. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
