GET /api/3.0/servers?dsId=1 500s when requested with If-Modified-Since header (#4953)
* Fixing 500 errors with server endpoints
* code review fixes
* go fmt
diff --git a/traffic_ops/client/deliveryserviceserver.go b/traffic_ops/client/deliveryserviceserver.go
index 336145e..975eb9c 100644
--- a/traffic_ops/client/deliveryserviceserver.go
+++ b/traffic_ops/client/deliveryserviceserver.go
@@ -88,13 +88,13 @@
}
// GetDeliveryServiceServers gets all delivery service servers, with the default API limit.
-func (to *Session) GetDeliveryServiceServers() (tc.DeliveryServiceServerResponse, ReqInf, error) {
- return to.getDeliveryServiceServers(url.Values{})
+func (to *Session) GetDeliveryServiceServers(h http.Header) (tc.DeliveryServiceServerResponse, ReqInf, error) {
+ return to.getDeliveryServiceServers(url.Values{}, h)
}
// GetDeliveryServiceServersN gets all delivery service servers, with a limit of n.
func (to *Session) GetDeliveryServiceServersN(n int) (tc.DeliveryServiceServerResponse, ReqInf, error) {
- return to.getDeliveryServiceServers(url.Values{"limit": []string{strconv.Itoa(n)}})
+ return to.getDeliveryServiceServers(url.Values{"limit": []string{strconv.Itoa(n)}}, nil)
}
// GetDeliveryServiceServersWithLimits gets all delivery service servers, allowing specifying the limit of mappings to return, the delivery services to return, and the servers to return.
@@ -121,16 +121,19 @@
vals.Set("serverids", strings.Join(serverIDStrs, ","))
}
- return to.getDeliveryServiceServers(vals)
+ return to.getDeliveryServiceServers(vals, nil)
}
-func (to *Session) getDeliveryServiceServers(urlQuery url.Values) (tc.DeliveryServiceServerResponse, ReqInf, error) {
+func (to *Session) getDeliveryServiceServers(urlQuery url.Values, h http.Header) (tc.DeliveryServiceServerResponse, ReqInf, error) {
route := apiBase + `/deliveryserviceserver`
if qry := urlQuery.Encode(); qry != "" {
route += `?` + qry
}
- reqResp, remoteAddr, err := to.request(http.MethodGet, route, nil, nil)
+ reqResp, remoteAddr, err := to.request(http.MethodGet, route, nil, h)
reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr: remoteAddr}
+ if reqResp != nil {
+ reqInf.StatusCode = reqResp.StatusCode
+ }
if err != nil {
return tc.DeliveryServiceServerResponse{}, reqInf, errors.New("requesting from Traffic Ops: " + err.Error())
}
diff --git a/traffic_ops/client/server.go b/traffic_ops/client/server.go
index 2e147aa..ee1f612 100644
--- a/traffic_ops/client/server.go
+++ b/traffic_ops/client/server.go
@@ -326,6 +326,9 @@
resp, remoteAddr, err := to.request(http.MethodGet, endpoint, nil, header)
reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr: remoteAddr}
+ if resp != nil {
+ reqInf.StatusCode = resp.StatusCode
+ }
if err != nil {
return nil, reqInf, err
}
diff --git a/traffic_ops/testing/api/v3/cachegroupsdeliveryservices_test.go b/traffic_ops/testing/api/v3/cachegroupsdeliveryservices_test.go
index d4d9f00..2b3a9a6 100644
--- a/traffic_ops/testing/api/v3/cachegroupsdeliveryservices_test.go
+++ b/traffic_ops/testing/api/v3/cachegroupsdeliveryservices_test.go
@@ -32,7 +32,7 @@
const TestEdgeServerCacheGroupName = "cachegroup3"
func CreateTestCachegroupsDeliveryServices(t *testing.T) {
- dss, _, err := TOSession.GetDeliveryServiceServers()
+ dss, _, err := TOSession.GetDeliveryServiceServers(nil)
if err != nil {
t.Fatalf("cannot GET DeliveryServiceServers: %v", err)
}
@@ -151,7 +151,7 @@
}
}
- dss, _, err = TOSession.GetDeliveryServiceServers()
+ dss, _, err = TOSession.GetDeliveryServiceServers(nil)
if err != nil {
t.Errorf("cannot GET DeliveryServiceServers: %v", err)
}
diff --git a/traffic_ops/testing/api/v3/deliveryserviceservers_test.go b/traffic_ops/testing/api/v3/deliveryserviceservers_test.go
index 55f3d10..58c1ed6 100644
--- a/traffic_ops/testing/api/v3/deliveryserviceservers_test.go
+++ b/traffic_ops/testing/api/v3/deliveryserviceservers_test.go
@@ -252,7 +252,7 @@
t.Errorf("POST delivery service servers: %v", err)
}
- dsServers, _, err := TOSession.GetDeliveryServiceServers()
+ dsServers, _, err := TOSession.GetDeliveryServiceServers(nil)
if err != nil {
t.Errorf("GET delivery service servers: %v", err)
}
@@ -272,7 +272,7 @@
t.Errorf("DELETE delivery service server: %v", err)
}
- dsServers, _, err = TOSession.GetDeliveryServiceServers()
+ dsServers, _, err = TOSession.GetDeliveryServiceServers(nil)
if err != nil {
t.Errorf("GET delivery service servers: %v", err)
}
diff --git a/traffic_ops/testing/api/v3/servers_test.go b/traffic_ops/testing/api/v3/servers_test.go
index 7aae452..41291df 100644
--- a/traffic_ops/testing/api/v3/servers_test.go
+++ b/traffic_ops/testing/api/v3/servers_test.go
@@ -170,6 +170,17 @@
if err != nil {
t.Fatalf("Failed to get server by Delivery Service ID: %v", err)
}
+
+ currentTime := time.Now().UTC().Add(5 * time.Second)
+ time := currentTime.Format(time.RFC1123)
+ var header http.Header
+ header = make(map[string][]string)
+ header.Set(rfc.IfModifiedSince, time)
+ _, reqInf, _ := TOSession.GetServers(¶ms, header)
+ if reqInf.StatusCode != http.StatusNotModified {
+ t.Errorf("Expected a status code of 304, got %v", reqInf.StatusCode)
+ }
+
params.Del("dsId")
resp, _, err := TOSession.GetServers(nil, nil)
diff --git a/traffic_ops/testing/api/v3/servers_to_deliveryservice_assignment_test.go b/traffic_ops/testing/api/v3/servers_to_deliveryservice_assignment_test.go
index 65d6575..5e8510a 100644
--- a/traffic_ops/testing/api/v3/servers_to_deliveryservice_assignment_test.go
+++ b/traffic_ops/testing/api/v3/servers_to_deliveryservice_assignment_test.go
@@ -18,7 +18,9 @@
"net/http"
"net/url"
"testing"
+ "time"
+ "github.com/apache/trafficcontrol/lib/go-rfc"
"github.com/apache/trafficcontrol/lib/go-tc"
)
@@ -88,6 +90,16 @@
if !found {
t.Errorf(`Server/DS assignment not found after "successful" assignment!`)
}
+
+ currentTime := time.Now().UTC().Add(5 * time.Second)
+ time := currentTime.Format(time.RFC1123)
+ var header http.Header
+ header = make(map[string][]string)
+ header.Set(rfc.IfModifiedSince, time)
+ _, reqInf, _ := TOSession.GetServerIDDeliveryServices(*firstServer.ID, header)
+ if reqInf.StatusCode != http.StatusNotModified {
+ t.Errorf("Expected a status code of 304, got %v", reqInf.StatusCode)
+ }
}
func AssignIncorrectTestDeliveryService(t *testing.T) {
diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go b/traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go
index abc738c..a28eddd 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go
@@ -814,7 +814,7 @@
func selectMaxLastUpdatedQuery(where string) string {
return `SELECT max(t) from (
- SELECT max(dss.last_updated) as t from deliveryservice_server dss ` + where +
+ SELECT max(dss.last_updated) as t from deliveryservice_server dss JOIN deliveryservice ds ON ds.id = dss.deliveryservice ` + where +
` UNION ALL
select max(last_updated) as t from last_deleted l where l.table_name='deliveryservice_server') as res`
}
diff --git a/traffic_ops/traffic_ops_golang/server/servers.go b/traffic_ops/traffic_ops_golang/server/servers.go
index a2b10f5..b9772a4 100644
--- a/traffic_ops/traffic_ops_golang/server/servers.go
+++ b/traffic_ops/traffic_ops_golang/server/servers.go
@@ -640,14 +640,15 @@
return
}
-func selectMaxLastUpdatedQuery(where string) string {
+func selectMaxLastUpdatedQuery(queryAddition string, where string) string {
return `SELECT max(t) from (
SELECT max(s.last_updated) as t from server s JOIN cachegroup cg ON s.cachegroup = cg.id
JOIN cdn cdn ON s.cdn_id = cdn.id
JOIN phys_location pl ON s.phys_location = pl.id
JOIN profile p ON s.profile = p.id
JOIN status st ON s.status = st.id
-JOIN type t ON s.type = t.id ` + where +
+JOIN type t ON s.type = t.id ` +
+ queryAddition + where +
` UNION ALL
select max(last_updated) as t from last_deleted l where l.table_name='server') as res`
}
@@ -723,7 +724,7 @@
serversList := []tc.ServerNullable{}
if useIMS {
- runSecond, maxTime = ims.TryIfModifiedSinceQuery(tx, h, queryValues, selectMaxLastUpdatedQuery(where))
+ runSecond, maxTime = ims.TryIfModifiedSinceQuery(tx, h, queryValues, selectMaxLastUpdatedQuery(queryAddition, where))
if !runSecond {
log.Debugln("IMS HIT")
return serversList, 0, nil, nil, http.StatusNotModified, &maxTime
diff --git a/traffic_ops/traffic_ops_golang/util/ims/ims.go b/traffic_ops/traffic_ops_golang/util/ims/ims.go
index daa2702..b6c32e9 100644
--- a/traffic_ops/traffic_ops_golang/util/ims/ims.go
+++ b/traffic_ops/traffic_ops_golang/util/ims/ims.go
@@ -2,12 +2,13 @@
import (
"database/sql"
+ "net/http"
+ "time"
+
"github.com/apache/trafficcontrol/lib/go-log"
"github.com/apache/trafficcontrol/lib/go-rfc"
"github.com/apache/trafficcontrol/lib/go-tc"
"github.com/jmoiron/sqlx"
- "net/http"
- "time"
)
/*
@@ -60,7 +61,7 @@
defer rows.Close()
}
if err != nil {
- log.Warnf("Couldn't get the max last updated time: %v", err)
+ log.Errorf("Couldn't get the max last updated time: %v", err)
return runSecond, maxTime
}
if err == sql.ErrNoRows {
