rawlinp commented on a change in pull request #3081: Fixes missing 
deliveryservice data fields from the servers/deliveryservices endpoint.
URL: https://github.com/apache/trafficcontrol/pull/3081#discussion_r239585862
 
 

 ##########
 File path: traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go
 ##########
 @@ -472,106 +473,72 @@ func dssSelectQuery() string {
 
 type TODSSDeliveryService struct {
        ReqInfo *api.APIInfo `json:"-"`
-       tc.DSSDeliveryService
+       tc.DeliveryServiceNullable
 }
 
 func (dss *TODSSDeliveryService) APIInfo() *api.APIInfo {
        return dss.ReqInfo
 }
 
 func TypeSingleton(reqInfo *api.APIInfo) api.Reader {
-       return &TODSSDeliveryService{reqInfo, tc.DSSDeliveryService{}}
+       return &TODSSDeliveryService{reqInfo, tc.DeliveryServiceNullable{}}
 }
 
 // Read shows all of the delivery services associated with the specified 
server.
 func (dss *TODSSDeliveryService) Read() ([]interface{}, error, error, int) {
-       orderby := dss.APIInfo().Params["orderby"]
-       if orderby == "" {
-               orderby = "deliveryService"
+       returnable := []interface{}{}
+       params := dss.APIInfo().Params
+       tx := dss.APIInfo().Tx.Tx
+       user := dss.APIInfo().User
+
+       if _, ok := params["orderby"]; !ok {
+               params["orderby"] = "xml_id"
+       }
+
+       // Query Parameters to Database Query column mappings
+       // see the fields mapped in the SQL query
+       queryParamsToSQLCols := map[string]dbhelpers.WhereColumnInfo{
+               "xml_id": dbhelpers.WhereColumnInfo{"ds.xml_id", nil},
+               "xmlId":  dbhelpers.WhereColumnInfo{"ds.xml_id", nil},
+       }
+       where, orderBy, queryValues, errs := 
dbhelpers.BuildWhereAndOrderBy(params, queryParamsToSQLCols)
+       if len(errs) > 0 {
+               for _, err := range errs {
 
 Review comment:
   So I don't think this for loop actually catches the error since "id" isn't 
in the `queryParamsToSQLCols`. But, I don't think we can really check it there 
since we don't have a real `WhereColumnInfo` since you're adding your own 
`WHERE` clause below.
   
   I think we should just remove this for loop and add the following explicit 
check up to line 493 above:
   ```
        if err := api.IsInt(params["id"]); err != nil {
                return nil, errors.New("Resource not found."), nil, 
http.StatusNotFound //matches perl response
        }
   ```
   That should make this emit the proper error when passing something like 
"foo" as the id in the path.

----------------------------------------------------------------
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

Reply via email to