Fix Traffic Ops /deliveryserviceserver IMS requests

CHANGELOG.md

@@ -72,6 +72,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - [#5732](https://github.com/apache/trafficcontrol/issues/5732) - TO API POST /cdns/dnsseckeys/generate times out with large numbers of delivery services
 - Fixed server creation through legacy API versions to default `monitor` to `true`.
 - Fixed t3c to generate topology parents correctly for parents with the Type MID+ or EDGE+ versus just the literal. Naming cache types to not be exactly 'EDGE' or 'MID' is still discouraged and not guaranteed to work, but it's unfortunately somewhat common, so this fixes it in one particular case.
+- [#5965](https://github.com/apache/trafficcontrol/issues/5965) - Fixed Traffic Ops /deliveryserviceservers If-Modified-Since requests.
 - Fixed t3c to create config files and directories as ats.ats
 - Fixed t3c-apply service restart and ats config reload logic.
 - Reduced TR dns.max-threads ansible default from 10000 to 100. 

traffic_ops/testing/api/v3/deliveryserviceservers_test.go

@@ -22,7 +22,9 @@ import (
 	"strconv"
 	"strings"
 	"testing"
+	"time"
 
+	"github.com/apache/trafficcontrol/lib/go-rfc"
 	"github.com/apache/trafficcontrol/lib/go-tc"
 	"github.com/apache/trafficcontrol/lib/go-util"
 )
@@ -34,6 +36,7 @@ func TestDeliveryServiceServers(t *testing.T) {
 		AssignOriginsToTopologyBasedDeliveryServices(t)
 		TryToRemoveLastServerInDeliveryService(t)
 		AssignServersToNonTopologyBasedDeliveryServiceThatUsesMidTier(t)
+		GetTestDSSIMS(t)
 	})
 }
 
@@ -377,6 +380,26 @@ func AssignServersToNonTopologyBasedDeliveryServiceThatUsesMidTier(t *testing.T)
 	}
 }
 
+func GetTestDSSIMS(t *testing.T) {
+	const noLimit = 999999
+	_, reqInf, err := TOSession.GetDeliveryServiceServersWithLimitsWithHdr(noLimit, nil, nil, nil)
+	if err != nil {
+		t.Errorf("deliveryserviceservers expected: no error, actual: %v", err)
+	} else if reqInf.StatusCode != http.StatusOK {
+		t.Errorf("expected deliveryserviceservers response code %v, actual %v", http.StatusOK, reqInf.StatusCode)
+	}
+
+	reqHdr := http.Header{}
+	reqHdr.Set(rfc.IfModifiedSince, time.Now().UTC().Add(2*time.Second).Format(time.RFC1123))
+
+	_, reqInf, err = TOSession.GetDeliveryServiceServersWithLimitsWithHdr(noLimit, nil, nil, reqHdr)
+	if err != nil {
+		t.Errorf("deliveryserviceservers IMS request expected: no error, actual: %v", err)
+	} else if reqInf.StatusCode != http.StatusNotModified {
+		t.Errorf("expected deliveryserviceservers IMS response code %v, actual %v", http.StatusNotModified, reqInf.StatusCode)
+	}
+}
+
 func CreateTestDeliveryServiceServersWithRequiredCapabilities(t *testing.T) {
 	sscs := testData.ServerServerCapabilities
 

traffic_ops/testing/api/v4/deliveryserviceservers_test.go

@@ -22,7 +22,9 @@ import (
 	"strconv"
 	"strings"
 	"testing"
+	"time"
 
+	"github.com/apache/trafficcontrol/lib/go-rfc"
 	"github.com/apache/trafficcontrol/lib/go-tc"
 	"github.com/apache/trafficcontrol/lib/go-util"
 	client "github.com/apache/trafficcontrol/traffic_ops/v4-client"
@@ -35,6 +37,7 @@ func TestDeliveryServiceServers(t *testing.T) {
 		AssignOriginsToTopologyBasedDeliveryServices(t)
 		TryToRemoveLastServerInDeliveryService(t)
 		AssignServersToNonTopologyBasedDeliveryServiceThatUsesMidTier(t)
+		GetTestDSSIMS(t)
 	})
 }
 
@@ -193,6 +196,28 @@ func TryToRemoveLastServerInDeliveryService(t *testing.T) {
 	}
 }
 
+func GetTestDSSIMS(t *testing.T) {
+	const noLimit = 999999
+
+	limit := noLimit
+	opts := client.NewRequestOptions()
+	opts.QueryParameters.Set("limit", strconv.Itoa(limit))
+	_, reqInf, err := TOSession.GetDeliveryServiceServers(opts)
+	if err != nil {
+		t.Errorf("deliveryserviceservers expected: no error, actual: %v", err)
+	} else if reqInf.StatusCode != http.StatusOK {
+		t.Errorf("expected deliveryserviceservers response code %v, actual %v", http.StatusOK, reqInf.StatusCode)
+	}
+
+	opts.Header.Set(rfc.IfModifiedSince, time.Now().UTC().Add(2*time.Second).Format(time.RFC1123))
+	_, reqInf, err = TOSession.GetDeliveryServiceServers(opts)
+	if err != nil {
+		t.Errorf("deliveryserviceservers IMS request expected: no error, actual: %v", err)
+	} else if reqInf.StatusCode != http.StatusNotModified {
+		t.Errorf("expected deliveryserviceservers IMS response code %v, actual %v", http.StatusNotModified, reqInf.StatusCode)
+	}
+}
+
 func AssignServersToTopologyBasedDeliveryService(t *testing.T) {
 	opts := client.NewRequestOptions()
 	opts.QueryParameters.Set("xmlId", "ds-top")

traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go

@@ -245,7 +245,10 @@ func (dss *TODeliveryServiceServer) readDSS(h http.Header, tx *sqlx.Tx, user *au
 		log.Warnf("Error getting the max last updated query %v", err)
 	}
 	if useIMS {
-		runSecond, maxTime = ims.TryIfModifiedSinceQuery(tx, h, map[string]interface{}{}, query1)
+		queryValues := map[string]interface{}{
+			"accessibleTenants": pq.Array(tenantIDs),
+		}
+		runSecond, maxTime = ims.TryIfModifiedSinceQuery(tx, h, queryValues, query1)
 		if !runSecond {
 			log.Debugln("IMS HIT")
 			return nil, nil, &maxTime
@@ -284,7 +287,7 @@ func selectQuery(orderBy string, limit string, offset string, dsIDs []int64, ser
 	FROM deliveryservice_server s`
 
 	if getMaxQuery {
-		selectStmt = `SELECT max(t) from (
+		selectStmt = `SELECT max(t) from ( (
 		SELECT max(s.last_updated) as t FROM deliveryservice_server s`
 	}
 	allowedOrderByCols := map[string]string{
@@ -317,14 +320,19 @@ AND s.server = ANY(:serverids)
 `
 	}
 
+	if getMaxQuery {
+		selectStmt += ` GROUP BY s.deliveryservice`
+	}
+
 	if orderBy != "" {
 		selectStmt += ` ORDER BY ` + orderBy
 	}
 
-	selectStmt += ` LIMIT ` + limit + ` OFFSET ` + offset + ` ROWS`
+	selectStmt += ` LIMIT ` + limit + ` OFFSET ` + offset + ` ROWS `
 	if getMaxQuery {
-		return selectStmt + `UNION ALL
-		select max(last_updated) as t from last_deleted l where l.table_name='deliveryservice_server') as res`, nil
+		return selectStmt + ` )
+UNION ALL
+select max(last_updated) as t from last_deleted l where l.table_name='deliveryservice_server') as res`, nil
 	}
 	return selectStmt, nil
 }