Repository: apache/trafficcontrol
Pull request #6593 (2022-03-01 01:13:12)

Layered Server Feature for `GET` API endpoint `servers/details`
<!--
Thank you for contributing! Please be sure to read our contribution guidelines: https://github.com/apache/trafficcontrol/blob/master/CONTRIBUTING.md
If this closes or relates to an existing issue, please reference it using one of the following:

Closes: #ISSUE
Related: #ISSUE

If this PR fixes a security vulnerability, DO NOT submit! Instead, contact
the Apache Traffic Control Security Team at security@trafficcontrol.apache.org and follow the
guidelines at https://apache.org/security regarding vulnerability disclosure.
-->

This PR (Layered Server Feature for `GET` API endpoint `servers/details`) is not related to any Issue but is a development for feature https://github.com/apache/trafficcontrol/pull/6095

<!-- **^ Add meaningful description above** --><hr/>

## Which Traffic Control components are affected by this PR?
<!-- Please delete all components from this list that are NOT affected by this PR.
Feel free to add the name of a tool or script that is affected but not on the list.
-->
- Documentation
- Traffic Ops

## What is the best way to verify this PR?
<!-- Please include here ALL the steps necessary to test your PR.
If your PR has tests (and most should), provide the steps needed to run the tests.
If not, please provide step-by-step instructions to test the PR manually and explain why your PR does not need tests. -->

Using CURL for 
API v4
```
curl -X GET -k -H "Accept: application/json" --cookie "mojolicious=<cookie>" https://localhost:8443/api/4.0/servers/details?hostName=abc
```
Result as follows: Note: Field profileNames exists and profileDesc does not exist
```
{"limit":1000,"orderby":"hostName","response":[{"cachegroup":"mb","cdnName":"ALL","domainName":"abc.com","guid":null,"hardwareInfo":null,"hostName":"abc","httpsPort":443,"id":9,"iloIpAddress":null,"iloIpGateway":null,"iloIpNetmask":null,"iloPassword":null,"iloUsername":null,"mgmtIpAddress":null,"mgmtIpGateway":null,"mgmtIpNetmask":null,"offlineReason":null,"physLocation":"boulder","**profileNames":["INFLUXDB","RIAK_ALL"]**,"rack":null,"status":"OFFLINE","tcpPort":80,"type":"RIAK","xmppId":"b9e71a88-48a5-4d3c-bf80-4bc58f9b5d75","xmppPasswd":null,"interfaces":[{"ipAddresses":[{"address":"8.8.8.8","gateway":"8.0.0.1","serviceAddress":true}],"maxBandwidth":null,"monitor":true,"mtu":null,"name":"int1","routerHostName":"","routerPortName":""}]}],"size":1}
```


API v3/v2
```
curl -X GET -k -H "Accept: application/json" --cookie "mojolicious=<cookie>" https://localhost:8443/api/3.0/servers/details?hostName=abc
```
Result as follows: Note: Field profile and profileDesc exists
```
{"limit":1000,"orderby":"hostName","response":[{"cachegroup":"mb","cdnName":"ALL","domainName":"abc.com","guid":null,"hardwareInfo":null,"hostName":"abc","httpsPort":443,"id":9,"iloIpAddress":null,"iloIpGateway":null,"iloIpNetmask":null,"iloPassword":null,"iloUsername":null,"mgmtIpAddress":null,"mgmtIpGateway":null,"mgmtIpNetmask":null,"offlineReason":null,"physLocation":"boulder","profile":"INFLUXDB","profileDesc":"InfluxDb profile","rack":null,"status":"OFFLINE","tcpPort":80,"type":"","xmppId":"b9e71a88-48a5-4d3c-bf80-4bc58f9b5d75","xmppPasswd":null,"interfaces":[{"ipAddresses":[{"address":"8.8.8.8","gateway":"8.0.0.1","serviceAddress":true}],"maxBandwidth":null,"monitor":true,"mtu":null,"name":"int1"}],"routerHostName":"","routerPortName":""}],"size":1}
```

## If this is a bugfix, which Traffic Control versions contained the bug?
<!-- Delete this section if the PR is not a bugfix, or if the bug is only in the master branch.
Examples:
- 5.1.2
- 5.1.3 (RC1)
 -->


## PR submission checklist
- [x] This PR has tests <!-- If not, please delete this text and explain why this PR does not need tests. -->
- [x] This PR has documentation <!-- If not, please delete this text and explain why this PR does not need documentation. -->
- [x] This PR has a CHANGELOG.md entry <!-- A fix for a bug from an ATC release, an improvement, or a new feature should have a changelog entry. -->
- [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://apache.org/security) for details)

<!--
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.
-->


ocket8888: This has a lot of overlap with #6544  - I'll review it, but I want to finish that one first.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3aaee21ca..043163cbf 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - [Traffic Ops | Traffic Go Clients | T3C] Add additional timestamp fields to server for queuing and dequeueing config and revalidate updates.
 - Added layered profile feature to 4.0 for `GET` /servers/, `POST` /servers/, `PUT` /servers/{id} and `DELETE` /servers/{id}.
 - Added a Traffic Ops endpoint and Traffic Portal page to view all CDNi configuration update requests and approve or deny.
+- Added layered profile feature to 4.0 for `GET` /servers/details.
 - Added layered profile feature to 4.0 for `GET` /deliveryservices/{id}/servers/ and /deliveryservices/{id}/servers/eligible.
 
 ### Fixed
diff --git a/docs/source/api/v4/servers_details.rst b/docs/source/api/v4/servers_details.rst
index afc95ed6a..dc0a917b5 100644
--- a/docs/source/api/v4/servers_details.rst
+++ b/docs/source/api/v4/servers_details.rst
@@ -98,8 +98,7 @@ Response Structure
 	:mgmtIpNetmask:  The IPv4 subnet mask of the server's management port
 	:offlineReason:         A user-entered reason why the server is in ADMIN_DOWN or OFFLINE status
 	:physLocation:          The name of the physical location where the server resides
-	:profile:               The :ref:`profile-name` of the :term:`Profile` used by this server
-	:profileDesc:           A :ref:`profile-description` of the :term:`Profile` used by this server
+	:profileNames:          List of :ref:`profile-name` of the :term:`Profiles` used by this server
 	:rack:  A string indicating "server rack" location
 	:status:                The status of the server
 
@@ -157,8 +156,7 @@ Response Structure
 				"mgmtIpNetmask": "",
 				"offlineReason": "",
 				"physLocation": "Apachecon North America 2018",
-				"profile": "ATS_EDGE_TIER_CACHE",
-				"profileDesc": "Edge Cache - Apache Traffic Server",
+				"profileNames": ["ATS_EDGE_TIER_CACHE"],
 				"rack": "",
 				"status": "REPORTED",
 				"tcpPort": 80,
diff --git a/lib/go-tc/servers.go b/lib/go-tc/servers.go
index 23971316b..2f435cdb3 100644
--- a/lib/go-tc/servers.go
+++ b/lib/go-tc/servers.go
@@ -87,8 +87,33 @@ type ServerDetailV30 struct {
 
 // ServerDetailV40 is the details for a server for API v4.
 type ServerDetailV40 struct {
-	ServerDetail
-	ServerInterfaces []ServerInterfaceInfoV40 `json:"interfaces"`
+	CacheGroup         *string                  `json:"cachegroup" db:"cachegroup"`
+	CDNName            *string                  `json:"cdnName" db:"cdn_name"`
+	DeliveryServiceIDs []int64                  `json:"deliveryservices,omitempty"`
+	DomainName         *string                  `json:"domainName" db:"domain_name"`
+	GUID               *string                  `json:"guid" db:"guid"`
+	HardwareInfo       map[string]string        `json:"hardwareInfo"`
+	HostName           *string                  `json:"hostName" db:"host_name"`
+	HTTPSPort          *int                     `json:"httpsPort" db:"https_port"`
+	ID                 *int                     `json:"id" db:"id"`
+	ILOIPAddress       *string                  `json:"iloIpAddress" db:"ilo_ip_address"`
+	ILOIPGateway       *string                  `json:"iloIpGateway" db:"ilo_ip_gateway"`
+	ILOIPNetmask       *string                  `json:"iloIpNetmask" db:"ilo_ip_netmask"`
+	ILOPassword        *string                  `json:"iloPassword" db:"ilo_password"`
+	ILOUsername        *string                  `json:"iloUsername" db:"ilo_username"`
+	MgmtIPAddress      *string                  `json:"mgmtIpAddress" db:"mgmt_ip_address"`
+	MgmtIPGateway      *string                  `json:"mgmtIpGateway" db:"mgmt_ip_gateway"`
+	MgmtIPNetmask      *string                  `json:"mgmtIpNetmask" db:"mgmt_ip_netmask"`
+	OfflineReason      *string                  `json:"offlineReason" db:"offline_reason"`
+	PhysLocation       *string                  `json:"physLocation" db:"phys_location"`
+	ProfileNames       []string                 `json:"profileNames" db:"profile_name"`
+	Rack               *string                  `json:"rack" db:"rack"`
+	Status             *string                  `json:"status" db:"status"`
+	TCPPort            *int                     `json:"tcpPort" db:"tcp_port"`
+	Type               string                   `json:"type" db:"server_type"`
+	XMPPID             *string                  `json:"xmppId" db:"xmpp_id"`
+	XMPPPasswd         *string                  `json:"xmppPasswd" db:"xmpp_passwd"`
+	ServerInterfaces   []ServerInterfaceInfoV40 `json:"interfaces"`
 }
 
 // ServersV1DetailResponse is the JSON object returned for a single server for v1.
diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
index e024126d0..dc029086b 100644
--- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
+++ b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
@@ -2091,6 +2091,43 @@ func UpdateServerProfileTableForV2V3(id *int, newProfile *string, origProfile st
 	return profileName, nil
 }
 
+// GetServerDetailFromV4 function converts server details from V4 to V3/V2
+func GetServerDetailFromV4(sd tc.ServerDetailV40, tx *sql.Tx) (tc.ServerDetail, error) {
+	var profileDesc *string
+	if err := tx.QueryRow(`SELECT p.description FROM profile p WHERE p.name=$1`, sd.ProfileNames[0]).Scan(&profileDesc); err != nil {
+		return tc.ServerDetail{}, fmt.Errorf("querying profile description by profile name: %w", err)
+	}
+	return tc.ServerDetail{
+		CacheGroup:         sd.CacheGroup,
+		CDNName:            sd.CDNName,
+		DeliveryServiceIDs: sd.DeliveryServiceIDs,
+		DomainName:         sd.DomainName,
+		GUID:               sd.GUID,
+		HardwareInfo:       sd.HardwareInfo,
+		HostName:           sd.HostName,
+		HTTPSPort:          sd.HTTPSPort,
+		ID:                 sd.ID,
+		ILOIPAddress:       sd.ILOIPAddress,
+		ILOIPGateway:       sd.ILOIPGateway,
+		ILOIPNetmask:       sd.ILOIPNetmask,
+		ILOPassword:        sd.ILOPassword,
+		ILOUsername:        sd.ILOUsername,
+		MgmtIPAddress:      sd.MgmtIPAddress,
+		MgmtIPGateway:      sd.MgmtIPGateway,
+		MgmtIPNetmask:      sd.MgmtIPNetmask,
+		OfflineReason:      sd.OfflineReason,
+		PhysLocation:       sd.PhysLocation,
+		Profile:            &sd.ProfileNames[0],
+		ProfileDesc:        profileDesc,
+		Rack:               sd.Rack,
+		Status:             sd.Status,
+		TCPPort:            sd.TCPPort,
+		Type:               sd.Type,
+		XMPPID:             sd.XMPPID,
+		XMPPPasswd:         sd.XMPPPasswd,
+	}, nil
+}
+
 // GetProfileIDDesc gets profile ID and desc for V3 servers
 func GetProfileIDDesc(tx *sql.Tx, name string) (id int, desc string) {
 	err := tx.QueryRow(`SELECT id, description from "profile" p WHERE p.name=$1`, name).Scan(&id, &desc)
diff --git a/traffic_ops/traffic_ops_golang/server/detail.go b/traffic_ops/traffic_ops_golang/server/detail.go
index 7f2eb9b1f..aa45dd955 100644
--- a/traffic_ops/traffic_ops_golang/server/detail.go
+++ b/traffic_ops/traffic_ops_golang/server/detail.go
@@ -91,7 +91,11 @@ func GetDetailParamHandler(w http.ResponseWriter, r *http.Request) {
 				routerPortName = interfaces[0].RouterPortName
 			}
 			v11server := tc.ServerDetailV11{}
-			v11server.ServerDetail = server.ServerDetail
+			v11server.ServerDetail, err = dbhelpers.GetServerDetailFromV4(server, inf.Tx.Tx)
+			if err != nil {
+				api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("failed to GetServerDetailFromV4: %w", err))
+				return
+			}
 			v11server.RouterHostName = &routerHostName
 			v11server.RouterPortName = &routerPortName
 			legacyInterface, err := tc.V4InterfaceInfoToLegacyInterfaces(interfaces)
@@ -117,7 +121,11 @@ func GetDetailParamHandler(w http.ResponseWriter, r *http.Request) {
 				routerHostName = interfaces[0].RouterHostName
 				routerPortName = interfaces[0].RouterPortName
 			}
-			v3Server.ServerDetail = server.ServerDetail
+			v3Server.ServerDetail, err = dbhelpers.GetServerDetailFromV4(server, inf.Tx.Tx)
+			if err != nil {
+				api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("failed to GetServerDetailFromV4: %w", err))
+				return
+			}
 			v3Server.RouterHostName = &routerHostName
 			v3Server.RouterPortName = &routerPortName
 			v3Interfaces, err := tc.V4InterfaceInfoToV3Interfaces(interfaces)
@@ -209,8 +217,7 @@ server.mgmt_ip_gateway,
 server.mgmt_ip_netmask,
 server.offline_reason,
 pl.name as phys_location,
-p.name as profile,
-p.description as profile_desc,
+(SELECT ARRAY_AGG(profile_name) FROM server_profile WHERE server_profile.server=server.id) AS profile_name,
 server.rack,
 st.name as status,
 server.tcp_port,
@@ -275,7 +282,38 @@ JOIN type t ON server.type = t.id
 
 	for rows.Next() {
 		s := tc.ServerDetailV40{}
-		if err := rows.Scan(&s.ID, &s.CacheGroup, &s.CDNName, pq.Array(&s.DeliveryServiceIDs), &s.DomainName, &s.GUID, &s.HostName, &s.HTTPSPort, &s.ILOIPAddress, &s.ILOIPGateway, &s.ILOIPNetmask, &s.ILOPassword, &s.ILOUsername, &serviceAddress, &service6Address, &serviceGateway, &service6Gateway, &serviceNetmask, &serviceInterface, &serviceMtu, &s.MgmtIPAddress, &s.MgmtIPGateway, &s.MgmtIPNetmask, &s.OfflineReason, &s.PhysLocation, &s.Profile, &s.ProfileDesc, &s.Rack, &s.Status, &s.TCPPort, &s.Type, &s.XMPPID, &s.XMPPPasswd); err != nil {
+		if err := rows.Scan(&s.ID,
+			&s.CacheGroup,
+			&s.CDNName,
+			pq.Array(&s.DeliveryServiceIDs),
+			&s.DomainName,
+			&s.GUID,
+			&s.HostName,
+			&s.HTTPSPort,
+			&s.ILOIPAddress,
+			&s.ILOIPGateway,
+			&s.ILOIPNetmask,
+			&s.ILOPassword,
+			&s.ILOUsername,
+			&serviceAddress,
+			&service6Address,
+			&serviceGateway,
+			&service6Gateway,
+			&serviceNetmask,
+			&serviceInterface,
+			&serviceMtu,
+			&s.MgmtIPAddress,
+			&s.MgmtIPGateway,
+			&s.MgmtIPNetmask,
+			&s.OfflineReason,
+			&s.PhysLocation,
+			pq.Array(&s.ProfileNames),
+			&s.Rack,
+			&s.Status,
+			&s.TCPPort,
+			&s.Type,
+			&s.XMPPID,
+			&s.XMPPPasswd); err != nil {
 			return nil, errors.New("Error scanning detail server: " + err.Error())
 		}
 		s.ServerInterfaces = []tc.ServerInterfaceInfoV40{}


Test patch:
diff --git a/traffic_ops/testing/api/v3/deliveryservice_request_comments_test.go b/traffic_ops/testing/api/v3/deliveryservice_request_comments_test.go
index f3aab587d..51dae401d 100644
--- a/traffic_ops/testing/api/v3/deliveryservice_request_comments_test.go
+++ b/traffic_ops/testing/api/v3/deliveryservice_request_comments_test.go
@@ -16,6 +16,7 @@ package v3
 */
 
 import (
+	"encoding/json"
 	"net/http"
 	"sort"
 	"testing"
@@ -23,182 +24,176 @@ import (
 
 	"github.com/apache/trafficcontrol/lib/go-rfc"
 	"github.com/apache/trafficcontrol/lib/go-tc"
+	"github.com/apache/trafficcontrol/traffic_ops/testing/api/assert"
+	"github.com/apache/trafficcontrol/traffic_ops/testing/api/utils"
+	"github.com/apache/trafficcontrol/traffic_ops/toclientlib"
 )
 
 func TestDeliveryServiceRequestComments(t *testing.T) {
 	WithObjs(t, []TCObj{CDNs, Types, Parameters, Tenants, DeliveryServiceRequests, DeliveryServiceRequestComments}, func() {
-		GetTestDeliveryServiceRequestCommentsIMS(t)
-		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.IfUnmodifiedSince, time)
-		header.Set(rfc.IfModifiedSince, time)
-		SortTestDeliveryServiceRequestComments(t)
-		UpdateTestDeliveryServiceRequestComments(t)
-		UpdateTestDeliveryServiceRequestCommentsWithHeaders(t, header)
-		header = make(map[string][]string)
-		etag := rfc.ETag(currentTime)
-		header.Set(rfc.IfMatch, etag)
-		UpdateTestDeliveryServiceRequestCommentsWithHeaders(t, header)
-		GetTestDeliveryServiceRequestComments(t)
-		GetTestDeliveryServiceRequestCommentsIMSAfterChange(t, header)
-	})
-}
-
-func UpdateTestDeliveryServiceRequestCommentsWithHeaders(t *testing.T, header http.Header) {
-	comments, _, _ := TOSession.GetDeliveryServiceRequestCommentsWithHdr(header)
 
-	if len(comments) > 0 {
-		firstComment := comments[0]
-		newFirstCommentValue := "new comment value"
-		firstComment.Value = newFirstCommentValue
-
-		_, reqInf, err := TOSession.UpdateDeliveryServiceRequestCommentByIDWithHdr(firstComment.ID, firstComment, header)
-		if err == nil {
-			t.Errorf("expected precondition failed error, but got none")
-		}
-		if reqInf.StatusCode != http.StatusPreconditionFailed {
-			t.Errorf("Expected status code 412, got %v", reqInf.StatusCode)
+		currentTime := time.Now().UTC().Add(-15 * time.Second)
+		currentTimeRFC := currentTime.Format(time.RFC1123)
+		tomorrow := currentTime.AddDate(0, 0, 1).Format(time.RFC1123)
+
+		methodTests := utils.V3TestCase{
+			"GET": {
+				"NOT MODIFIED when NO CHANGES made": {
+					ClientSession:  TOSession,
+					RequestHeaders: http.Header{rfc.IfModifiedSince: {tomorrow}},
+					Expectations:   utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusNotModified)),
+				},
+				"OK when VALID request": {
+					ClientSession: TOSession,
+					Expectations:  utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
+				},
+				"OK when VALID ID parameter": {
+					EndpointId:    GetDSRequestCommentId(t),
+					ClientSession: TOSession,
+					Expectations:  utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), utils.ResponseHasLength(1)),
+				},
+				"VALIDATE SORT when DEFAULT is ASC ORDER": {
+					ClientSession: TOSession,
+					Expectations:  utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), validateSortedDSRequestComments()),
+				},
+			},
+			"PUT": {
+				"OK when VALID request": {
+					EndpointId:    GetDSRequestCommentId(t),
+					ClientSession: TOSession,
+					RequestBody: map[string]interface{}{
+						"deliveryServiceRequestId": GetDSRequestId(t, "test-ds1")(),
+						"value":                    "updated comment",
+					},
+					Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
+				},
+				"PRECONDITION FAILED when updating with IF-UNMODIFIED-SINCE Header": {
+					EndpointId:     GetDSRequestCommentId(t),
+					ClientSession:  TOSession,
+					RequestHeaders: http.Header{rfc.IfUnmodifiedSince: {currentTimeRFC}},
+					RequestBody:    map[string]interface{}{},
+					Expectations:   utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusPreconditionFailed)),
+				},
+				"PRECONDITION FAILED when updating with IFMATCH ETAG Header": {
+					EndpointId:     GetDSRequestCommentId(t),
+					ClientSession:  TOSession,
+					RequestBody:    map[string]interface{}{},
+					RequestHeaders: http.Header{rfc.IfMatch: {rfc.ETag(currentTime)}},
+					Expectations:   utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusPreconditionFailed)),
+				},
+			},
+			"GET AFTER CHANGES": {
+				"OK when CHANGES made": {
+					ClientSession:  TOSession,
+					RequestHeaders: http.Header{rfc.IfModifiedSince: {currentTimeRFC}},
+					Expectations:   utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
+				},
+			},
 		}
-	}
-}
 
-func GetTestDeliveryServiceRequestCommentsIMSAfterChange(t *testing.T, header http.Header) {
-	_, reqInf, err := TOSession.GetDeliveryServiceRequestCommentsWithHdr(header)
-	if err != nil {
-		t.Fatalf("could not GET delivery service request comments: %v", err)
-	}
-	if reqInf.StatusCode != http.StatusOK {
-		t.Fatalf("Expected 200 status code, got %v", reqInf.StatusCode)
-	}
-	header = make(map[string][]string)
-	futureTime := time.Now().AddDate(0, 0, 1)
-	time := futureTime.Format(time.RFC1123)
-	header.Set(rfc.IfModifiedSince, time)
-	_, reqInf, err = TOSession.GetDeliveryServiceRequestCommentsWithHdr(header)
-	if err != nil {
-		t.Fatalf("could not GET delivery service request comments: %v", err)
-	}
-	if reqInf.StatusCode != http.StatusNotModified {
-		t.Fatalf("Expected 304 status code, got %v", reqInf.StatusCode)
-	}
-}
-
-func CreateTestDeliveryServiceRequestComments(t *testing.T) {
-
-	// Retrieve a delivery service request by xmlId so we can get the ID needed to create a dsr comment
-	dsr := testData.DeliveryServiceRequests[0].DeliveryService
-
-	resp, _, err := TOSession.GetDeliveryServiceRequestByXMLID(dsr.XMLID)
-	if err != nil {
-		t.Errorf("cannot GET delivery service request by xml id: %v - %v", dsr.XMLID, err)
-	}
-	if len(resp) != 1 {
-		t.Errorf("found %d delivery service request by xml id, expected %d: %s", len(resp), 1, dsr.XMLID)
-	} else {
-		respDSR := resp[0]
-
-		for _, comment := range testData.DeliveryServiceRequestComments {
-			comment.DeliveryServiceRequestID = respDSR.ID
-			resp, _, err := TOSession.CreateDeliveryServiceRequestComment(comment)
-			if err != nil {
-				t.Errorf("could not CREATE delivery service request comment: %v - %v", err, resp)
-			}
+		for method, testCases := range methodTests {
+			t.Run(method, func(t *testing.T) {
+				for name, testCase := range testCases {
+					comment := tc.DeliveryServiceRequestComment{}
+
+					if testCase.RequestBody != nil {
+						dat, err := json.Marshal(testCase.RequestBody)
+						assert.NoError(t, err, "Error occurred when marshalling request body: %v", err)
+						err = json.Unmarshal(dat, &comment)
+						assert.NoError(t, err, "Error occurred when unmarshalling request body: %v", err)
+					}
+
+					switch method {
+					case "GET", "GET AFTER CHANGES":
+						t.Run(name, func(t *testing.T) {
+							if name == "OK when VALID ID parameter" {
+								resp, reqInf, err := testCase.ClientSession.GetDeliveryServiceRequestCommentByIDWithHdr(testCase.EndpointId(), testCase.RequestHeaders)
+								for _, check := range testCase.Expectations {
+									check(t, reqInf, resp, tc.Alerts{}, err)
+								}
+							} else {
+								resp, reqInf, err := testCase.ClientSession.GetDeliveryServiceRequestCommentsWithHdr(testCase.RequestHeaders)
+								for _, check := range testCase.Expectations {
+									check(t, reqInf, resp, tc.Alerts{}, err)
+								}
+							}
+						})
+					case "PUT":
+						t.Run(name, func(t *testing.T) {
+							alerts, reqInf, err := testCase.ClientSession.UpdateDeliveryServiceRequestCommentByIDWithHdr(testCase.EndpointId(), comment, testCase.RequestHeaders)
+							for _, check := range testCase.Expectations {
+								check(t, reqInf, nil, alerts, err)
+							}
+						})
+					}
+				}
+			})
 		}
-	}
-
+	})
 }
 
-func SortTestDeliveryServiceRequestComments(t *testing.T) {
-	var header http.Header
-	var sortedList []string
-	resp, _, err := TOSession.GetDeliveryServiceRequestCommentsWithHdr(header)
-	if err != nil {
-		t.Fatalf("Expected no error, but got %v", err.Error())
-	}
-	for i, _ := range resp {
-		sortedList = append(sortedList, resp[i].XMLID)
-	}
+func GetDSRequestCommentId(t *testing.T) func() int {
+	return func() int {
+		resp, _, err := TOSession.GetDeliveryServiceRequestCommentsWithHdr(http.Header{})
+		assert.RequireNoError(t, err, "Get Delivery Service Request Comments failed with error: %v", err)
+		assert.RequireGreaterOrEqual(t, len(resp), 1, "Expected delivery service request comments response object length of atleast 1, but got %d", len(resp))
+		assert.RequireNotNil(t, resp[0].ID, "Expected id to not be nil")
 
-	res := sort.SliceIsSorted(sortedList, func(p, q int) bool {
-		return sortedList[p] < sortedList[q]
-	})
-	if res != true {
-		t.Errorf("list is not sorted by their names: %v", sortedList)
+		return resp[0].ID
 	}
 }
 
-func UpdateTestDeliveryServiceRequestComments(t *testing.T) {
-
-	comments, _, err := TOSession.GetDeliveryServiceRequestComments()
+func GetDSRequestId(t *testing.T, xmlId string) func() int {
+	return func() int {
+		resp, _, err := TOSession.GetDeliveryServiceRequestByXMLIDWithHdr(xmlId, http.Header{})
+		assert.RequireNoError(t, err, "Get Delivery Service Requests failed with error: %v", err)
+		assert.RequireGreaterOrEqual(t, len(resp), 1, "Expected delivery service requests response object length of atleast 1, but got %d", len(resp))
+		assert.RequireNotNil(t, resp[0].ID, "Expected id to not be nil")
 
-	firstComment := comments[0]
-	newFirstCommentValue := "new comment value"
-	firstComment.Value = newFirstCommentValue
-
-	var alert tc.Alerts
-	alert, _, err = TOSession.UpdateDeliveryServiceRequestCommentByID(firstComment.ID, firstComment)
-	if err != nil {
-		t.Errorf("cannot UPDATE delivery service request comment by id: %v - %v", err, alert)
+		return resp[0].ID
 	}
+}
 
-	// Retrieve the delivery service request comment to check that the value got updated
-	resp, _, err := TOSession.GetDeliveryServiceRequestCommentByID(firstComment.ID)
-	if err != nil {
-		t.Errorf("cannot GET delivery service request comment by id: '$%d', %v", firstComment.ID, err)
-	}
-	respDSRC := resp[0]
-	if respDSRC.Value != newFirstCommentValue {
-		t.Errorf("results do not match actual: %s, expected: %s", respDSRC.Value, newFirstCommentValue)
-	}
+func validateSortedDSRequestComments() utils.CkReqFunc {
+	return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, err error) {
+		var sortedList []string
+		dsReqComments := resp.([]tc.DeliveryServiceRequestComment)
 
-}
+		for _, comment := range dsReqComments {
+			sortedList = append(sortedList, comment.XMLID)
+		}
 
-func GetTestDeliveryServiceRequestCommentsIMS(t *testing.T) {
-	var header http.Header
-	header = make(map[string][]string)
-	futureTime := time.Now().AddDate(0, 0, 1)
-	time := futureTime.Format(time.RFC1123)
-	header.Set(rfc.IfModifiedSince, time)
-	_, reqInf, err := TOSession.GetDeliveryServiceRequestCommentsWithHdr(header)
-	if err != nil {
-		t.Fatalf("could not GET delivery service request comments: %v", err)
-	}
-	if reqInf.StatusCode != http.StatusNotModified {
-		t.Fatalf("Expected 304 status code, got %v", reqInf.StatusCode)
+		res := sort.SliceIsSorted(sortedList, func(p, q int) bool {
+			return sortedList[p] < sortedList[q]
+		})
+		assert.Equal(t, res, true, "List is not sorted by their names: %v", sortedList)
 	}
 }
 
-func GetTestDeliveryServiceRequestComments(t *testing.T) {
-
-	comments, _, _ := TOSession.GetDeliveryServiceRequestComments()
+func CreateTestDeliveryServiceRequestComments(t *testing.T) {
+	for _, comment := range testData.DeliveryServiceRequestComments {
+		resp, _, err := TOSession.GetDeliveryServiceRequestByXMLIDWithHdr(comment.XMLID, http.Header{})
+		assert.NoError(t, err, "Cannot get Delivery Service Request by XMLID '%s': %v", comment.XMLID, err)
+		assert.Equal(t, len(resp), 1, "Found %d Delivery Service request by XMLID '%s, expected exactly one", len(resp), comment.XMLID)
+		assert.NotNil(t, resp[0].ID, "Got Delivery Service Request with xml_id '%s' that had a null ID", comment.XMLID)
 
-	for _, comment := range comments {
-		resp, _, err := TOSession.GetDeliveryServiceRequestCommentByID(comment.ID)
-		if err != nil {
-			t.Errorf("cannot GET delivery service request comment by id: %v - %v", err, resp)
-		}
+		comment.DeliveryServiceRequestID = resp[0].ID
+		alerts, _, err := TOSession.CreateDeliveryServiceRequestComment(comment)
+		assert.NoError(t, err, "Could not create Delivery Service Request Comment: %v - alerts: %+v", err, alerts.Alerts)
 	}
 }
 
 func DeleteTestDeliveryServiceRequestComments(t *testing.T) {
+	resp, _, err := TOSession.GetDeliveryServiceRequestCommentsWithHdr(http.Header{})
+	assert.NoError(t, err, "Unexpected error getting Delivery Service Request Comments: %v", err)
 
-	comments, _, _ := TOSession.GetDeliveryServiceRequestComments()
-
-	for _, comment := range comments {
-		_, _, err := TOSession.DeleteDeliveryServiceRequestCommentByID(comment.ID)
-		if err != nil {
-			t.Errorf("cannot DELETE delivery service request comment by id: '%d' %v", comment.ID, err)
-		}
+	for _, comment := range resp {
+		alerts, _, err := TOSession.DeleteDeliveryServiceRequestCommentByID(comment.ID)
+		assert.NoError(t, err, "Cannot delete Delivery Service Request Comment #%d: %v - alerts: %+v", comment.ID, err, alerts.Alerts)
 
 		// Retrieve the delivery service request comment to see if it got deleted
-		comments, _, err := TOSession.GetDeliveryServiceRequestCommentByID(comment.ID)
-		if err != nil {
-			t.Errorf("error deleting delivery service request comment: %s", err.Error())
-		}
-		if len(comments) > 0 {
-			t.Errorf("expected delivery service request comment: %d to be deleted", comment.ID)
-		}
+		resp, _, err := TOSession.GetDeliveryServiceRequestCommentByIDWithHdr(comment.ID, http.Header{})
+		assert.NoError(t, err, "Unexpected error fetching Delivery Service Request Comment %d after deletion: %v", comment.ID, err)
+		assert.Equal(t, len(resp), 0, "Expected Delivery Service Request Comment #%d to be deleted, but it was found in Traffic Ops", comment.ID)
 	}
 }
diff --git a/traffic_ops/testing/api/v3/tc-fixtures.json b/traffic_ops/testing/api/v3/tc-fixtures.json
index ab8616ae4..7c2f2994f 100644
--- a/traffic_ops/testing/api/v3/tc-fixtures.json
+++ b/traffic_ops/testing/api/v3/tc-fixtures.json
@@ -281,16 +281,20 @@
     ],
     "deliveryServiceRequestComments": [
         {
-            "value": "this is comment one"
+            "value": "this is comment one",
+            "xmlId": "test-ds1"
         },
         {
-            "value": "this is comment two"
+            "value": "this is comment two",
+            "xmlId": "test-ds1"
         },
         {
-            "value": "this is comment three"
+            "value": "this is comment three",
+            "xmlId": "test-ds1"
         },
         {
-            "value": "this is comment four"
+            "value": "this is comment four",
+            "xmlId": "test-ds1"
         }
     ],
     "deliveryServiceRequests": [
diff --git a/traffic_ops/testing/api/v4/deliveryservice_request_comments_test.go b/traffic_ops/testing/api/v4/deliveryservice_request_comments_test.go
index dd5c41080..512289661 100644
--- a/traffic_ops/testing/api/v4/deliveryservice_request_comments_test.go
+++ b/traffic_ops/testing/api/v4/deliveryservice_request_comments_test.go
@@ -16,7 +16,9 @@ package v4
 */
 
 import (
+	"encoding/json"
 	"net/http"
+	"net/url"
 	"sort"
 	"strconv"
 	"testing"
@@ -24,220 +26,180 @@ import (
 
 	"github.com/apache/trafficcontrol/lib/go-rfc"
 	"github.com/apache/trafficcontrol/lib/go-tc"
+	"github.com/apache/trafficcontrol/traffic_ops/testing/api/assert"
+	"github.com/apache/trafficcontrol/traffic_ops/testing/api/utils"
+	"github.com/apache/trafficcontrol/traffic_ops/toclientlib"
 	client "github.com/apache/trafficcontrol/traffic_ops/v4-client"
 )
 
 func TestDeliveryServiceRequestComments(t *testing.T) {
 	WithObjs(t, []TCObj{CDNs, Types, Parameters, Tenants, DeliveryServiceRequests, DeliveryServiceRequestComments}, func() {
-		GetTestDeliveryServiceRequestCommentsIMS(t)
-		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.IfUnmodifiedSince, time)
-		header.Set(rfc.IfModifiedSince, time)
-		SortTestDeliveryServiceRequestComments(t)
-		UpdateTestDeliveryServiceRequestComments(t)
-		UpdateTestDeliveryServiceRequestCommentsWithHeaders(t, header)
-		header = make(map[string][]string)
-		etag := rfc.ETag(currentTime)
-		header.Set(rfc.IfMatch, etag)
-		UpdateTestDeliveryServiceRequestCommentsWithHeaders(t, header)
-		GetTestDeliveryServiceRequestComments(t)
-		GetTestDeliveryServiceRequestCommentsIMSAfterChange(t, header)
-	})
-}
-
-func UpdateTestDeliveryServiceRequestCommentsWithHeaders(t *testing.T, header http.Header) {
-	opts := client.NewRequestOptions()
-	opts.Header = header
-	comments, _, _ := TOSession.GetDeliveryServiceRequestComments(opts)
-
-	if len(comments.Response) > 0 {
-		firstComment := comments.Response[0]
-		newFirstCommentValue := "new comment value"
-		firstComment.Value = newFirstCommentValue
 
-		_, reqInf, err := TOSession.UpdateDeliveryServiceRequestComment(firstComment.ID, firstComment, opts)
-		if err == nil {
-			t.Errorf("expected precondition failed error, but got none")
+		currentTime := time.Now().UTC().Add(-15 * time.Second)
+		currentTimeRFC := currentTime.Format(time.RFC1123)
+		tomorrow := currentTime.AddDate(0, 0, 1).Format(time.RFC1123)
+
+		methodTests := utils.V4TestCase{
+			"GET": {
+				"NOT MODIFIED when NO CHANGES made": {
+					ClientSession: TOSession,
+					RequestOpts:   client.RequestOptions{Header: http.Header{rfc.IfModifiedSince: {tomorrow}}},
+					Expectations:  utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusNotModified)),
+				},
+				"OK when VALID request": {
+					ClientSession: TOSession,
+					Expectations:  utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
+				},
+				"OK when VALID ID parameter": {
+					ClientSession: TOSession,
+					RequestOpts:   client.RequestOptions{QueryParameters: url.Values{"id": {strconv.Itoa(GetDSRequestCommentId(t, "admin")())}}},
+					Expectations:  utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), utils.ResponseHasLength(1)),
+				},
+				"VALIDATE SORT when DEFAULT is ASC ORDER": {
+					ClientSession: TOSession,
+					Expectations:  utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), validateSortedDSRequestComments()),
+				},
+			},
+			"PUT": {
+				"OK when VALID request": {
+					EndpointId:    GetDSRequestCommentId(t, "admin"),
+					ClientSession: TOSession,
+					RequestBody: map[string]interface{}{
+						"deliveryServiceRequestId": GetDSRequestId(t, "test-ds1")(),
+						"value":                    "updated comment",
+					},
+					Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
+				},
+				"PRECONDITION FAILED when updating with IF-UNMODIFIED-SINCE Header": {
+					EndpointId:    GetDSRequestCommentId(t, "admin"),
+					ClientSession: TOSession,
+					RequestOpts:   client.RequestOptions{Header: http.Header{rfc.IfUnmodifiedSince: {currentTimeRFC}}},
+					RequestBody:   map[string]interface{}{},
+					Expectations:  utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusPreconditionFailed)),
+				},
+				"PRECONDITION FAILED when updating with IFMATCH ETAG Header": {
+					EndpointId:    GetDSRequestCommentId(t, "admin"),
+					ClientSession: TOSession,
+					RequestBody:   map[string]interface{}{},
+					RequestOpts:   client.RequestOptions{Header: http.Header{rfc.IfMatch: {rfc.ETag(currentTime)}}},
+					Expectations:  utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusPreconditionFailed)),
+				},
+			},
+			"GET AFTER CHANGES": {
+				"OK when CHANGES made": {
+					ClientSession: TOSession,
+					RequestOpts:   client.RequestOptions{Header: http.Header{rfc.IfModifiedSince: {currentTimeRFC}}},
+					Expectations:  utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
+				},
+			},
 		}
-		if reqInf.StatusCode != http.StatusPreconditionFailed {
-			t.Errorf("Expected status code 412, got %v", reqInf.StatusCode)
-		}
-	}
-}
 
-func GetTestDeliveryServiceRequestCommentsIMSAfterChange(t *testing.T, header http.Header) {
-	opts := client.NewRequestOptions()
-	opts.Header = header
-	resp, reqInf, err := TOSession.GetDeliveryServiceRequestComments(opts)
-	if err != nil {
-		t.Fatalf("could not get Delivery Service Request Comments: %v - alerts: %+v", err, resp.Alerts)
-	}
-	if reqInf.StatusCode != http.StatusOK {
-		t.Fatalf("Expected 200 status code, got %v", reqInf.StatusCode)
-	}
-	opts.Header = make(map[string][]string)
-	futureTime := time.Now().AddDate(0, 0, 1)
-	time := futureTime.Format(time.RFC1123)
-	opts.Header.Set(rfc.IfModifiedSince, time)
-	resp, reqInf, err = TOSession.GetDeliveryServiceRequestComments(opts)
-	if err != nil {
-		t.Fatalf("could not get Delivery Service Request Comments: %v - alerts: %+v", err, resp.Alerts)
-	}
-	if reqInf.StatusCode != http.StatusNotModified {
-		t.Fatalf("Expected 304 status code, got %v", reqInf.StatusCode)
-	}
-}
-
-func CreateTestDeliveryServiceRequestComments(t *testing.T) {
-	if len(testData.DeliveryServiceRequests) < 1 {
-		t.Fatal("Need at least one Delivery Service Request to test creating Delivery Service Request Comments")
-	}
-
-	// Retrieve a delivery service request by xmlId so we can get the ID needed to create a dsr comment
-	dsr := testData.DeliveryServiceRequests[0]
-	var ds *tc.DeliveryServiceV4
-	if dsr.ChangeType == tc.DSRChangeTypeDelete {
-		ds = dsr.Original
-	} else {
-		ds = dsr.Requested
-	}
-	resetDS(ds)
-	if ds == nil || ds.XMLID == nil {
-		t.Fatal("first DSR in the test data had a nil Delivery Service, or one with no XMLID")
-	}
-
-	opts := client.NewRequestOptions()
-	opts.QueryParameters.Set("xmlId", *ds.XMLID)
-	resp, _, err := TOSession.GetDeliveryServiceRequests(opts)
-	if err != nil {
-		t.Fatalf("cannot get Delivery Service Request by XMLID '%s': %v - alerts: %+v", *ds.XMLID, err, resp.Alerts)
-	}
-	if len(resp.Response) != 1 {
-		t.Fatalf("found %d Delivery Service request by XMLID '%s, expected exactly one", len(resp.Response), *ds.XMLID)
-	}
-	respDSR := resp.Response[0]
-	if respDSR.ID == nil {
-		t.Fatalf("got Delivery Service Request with xml_id '%s' that had a null ID", *ds.XMLID)
-	}
-
-	for _, comment := range testData.DeliveryServiceRequestComments {
-		comment.DeliveryServiceRequestID = *respDSR.ID
-		resp, _, err := TOSession.CreateDeliveryServiceRequestComment(comment, client.RequestOptions{})
-		if err != nil {
-			t.Errorf("could not create Delivery Service Request Comment: %v - alerts: %+v", err, resp.Alerts)
+		for method, testCases := range methodTests {
+			t.Run(method, func(t *testing.T) {
+				for name, testCase := range testCases {
+					comment := tc.DeliveryServiceRequestComment{}
+
+					if testCase.RequestBody != nil {
+						dat, err := json.Marshal(testCase.RequestBody)
+						assert.NoError(t, err, "Error occurred when marshalling request body: %v", err)
+						err = json.Unmarshal(dat, &comment)
+						assert.NoError(t, err, "Error occurred when unmarshalling request body: %v", err)
+					}
+
+					switch method {
+					case "GET", "GET AFTER CHANGES":
+						t.Run(name, func(t *testing.T) {
+							resp, reqInf, err := testCase.ClientSession.GetDeliveryServiceRequestComments(testCase.RequestOpts)
+							for _, check := range testCase.Expectations {
+								check(t, reqInf, resp.Response, resp.Alerts, err)
+							}
+						})
+					case "PUT":
+						t.Run(name, func(t *testing.T) {
+							alerts, reqInf, err := testCase.ClientSession.UpdateDeliveryServiceRequestComment(testCase.EndpointId(), comment, testCase.RequestOpts)
+							for _, check := range testCase.Expectations {
+								check(t, reqInf, nil, alerts, err)
+							}
+						})
+					}
+				}
+			})
 		}
-	}
+	})
 }
 
-func SortTestDeliveryServiceRequestComments(t *testing.T) {
-	var sortedList []string
-	resp, _, err := TOSession.GetDeliveryServiceRequestComments(client.RequestOptions{})
-	if err != nil {
-		t.Fatalf("Expected no error, but got: %v - alerts: %+v", err, resp.Alerts)
-	}
-	for _, dsrc := range resp.Response {
-		sortedList = append(sortedList, dsrc.XMLID)
-	}
+func GetDSRequestCommentId(t *testing.T, author string) func() int {
+	return func() int {
+		opts := client.NewRequestOptions()
+		opts.QueryParameters.Set("author", author)
 
-	res := sort.SliceIsSorted(sortedList, func(p, q int) bool {
-		return sortedList[p] < sortedList[q]
-	})
-	if res != true {
-		t.Errorf("list is not sorted by their names: %v", sortedList)
+		resp, _, err := TOSession.GetDeliveryServiceRequestComments(opts)
+		assert.RequireNoError(t, err, "Get Delivery Service Request Comments failed with error: %v", err)
+		assert.RequireGreaterOrEqual(t, len(resp.Response), 1, "Expected delivery service request comments response object length of atleast 1, but got %d", len(resp.Response))
+		assert.RequireNotNil(t, resp.Response[0].ID, "Expected id to not be nil")
+
+		return resp.Response[0].ID
 	}
 }
 
-func UpdateTestDeliveryServiceRequestComments(t *testing.T) {
+func GetDSRequestId(t *testing.T, xmlId string) func() int {
+	return func() int {
+		opts := client.NewRequestOptions()
+		opts.QueryParameters.Set("xmlId", xmlId)
 
-	comments, _, err := TOSession.GetDeliveryServiceRequestComments(client.RequestOptions{})
-	if err != nil {
-		t.Errorf("Unexpected error getting Delivery Service Request Comments: %v - alerts: %+v", err, comments.Alerts)
-	}
-	if len(comments.Response) < 1 {
-		t.Fatal("Expected at least one Delivery Service Request Comment to exist in Traffic Ops - none did")
-	}
-	firstComment := comments.Response[0]
-	newFirstCommentValue := "new comment value"
-	firstComment.Value = newFirstCommentValue
-
-	var alert tc.Alerts
-	alert, _, err = TOSession.UpdateDeliveryServiceRequestComment(firstComment.ID, firstComment, client.RequestOptions{})
-	if err != nil {
-		t.Errorf("cannot update Delivery Service Request Comment #%d: %v - alerts: %+v", firstComment.ID, err, alert.Alerts)
-	}
+		resp, _, err := TOSession.GetDeliveryServiceRequests(opts)
+		assert.RequireNoError(t, err, "Get Delivery Service Requests failed with error: %v", err)
+		assert.RequireGreaterOrEqual(t, len(resp.Response), 1, "Expected delivery service requests response object length of atleast 1, but got %d", len(resp.Response))
+		assert.RequireNotNil(t, resp.Response[0].ID, "Expected id to not be nil")
 
-	// Retrieve the delivery service request comment to check that the value got updated
-	opts := client.NewRequestOptions()
-	opts.QueryParameters.Set("id", strconv.Itoa(firstComment.ID))
-	resp, _, err := TOSession.GetDeliveryServiceRequestComments(opts)
-	if err != nil {
-		t.Errorf("cannot get Delivery Service Request Comment #%d: %v - alerts: %+v", firstComment.ID, err, resp.Alerts)
-	}
-	if len(resp.Response) != 1 {
-		t.Fatalf("Expected exactly one Delivery Service Request Comment to exist with ID %d, found: %d", firstComment.ID, len(resp.Response))
-	}
-	respDSRC := resp.Response[0]
-	if respDSRC.Value != newFirstCommentValue {
-		t.Errorf("results do not match actual: %s, expected: %s", respDSRC.Value, newFirstCommentValue)
+		return *resp.Response[0].ID
 	}
-
 }
 
-func GetTestDeliveryServiceRequestCommentsIMS(t *testing.T) {
-	futureTime := time.Now().AddDate(0, 0, 1)
-	time := futureTime.Format(time.RFC1123)
+func validateSortedDSRequestComments() utils.CkReqFunc {
+	return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, err error) {
+		var sortedList []string
+		dsReqComments := resp.([]tc.DeliveryServiceRequestComment)
 
-	opts := client.NewRequestOptions()
-	opts.Header.Set(rfc.IfModifiedSince, time)
-	resp, reqInf, err := TOSession.GetDeliveryServiceRequestComments(opts)
-	if err != nil {
-		t.Fatalf("could not get Delivery Service Request Comments: %v - alerts: %+v", err, resp.Alerts)
-	}
-	if reqInf.StatusCode != http.StatusNotModified {
-		t.Fatalf("Expected 304 status code, got %v", reqInf.StatusCode)
+		for _, comment := range dsReqComments {
+			sortedList = append(sortedList, comment.XMLID)
+		}
+
+		res := sort.SliceIsSorted(sortedList, func(p, q int) bool {
+			return sortedList[p] < sortedList[q]
+		})
+		assert.Equal(t, res, true, "List is not sorted by their names: %v", sortedList)
 	}
 }
 
-func GetTestDeliveryServiceRequestComments(t *testing.T) {
-	comments, _, err := TOSession.GetDeliveryServiceRequestComments(client.RequestOptions{})
-	if err != nil {
-		t.Errorf("Unexpected error getting Delivery Service Request Comments: %v - alerts: %+v", err, comments.Alerts)
-	}
+func CreateTestDeliveryServiceRequestComments(t *testing.T) {
+	for _, comment := range testData.DeliveryServiceRequestComments {
+		opts := client.NewRequestOptions()
+		opts.QueryParameters.Set("xmlId", comment.XMLID)
+		resp, _, err := TOSession.GetDeliveryServiceRequests(opts)
+		assert.NoError(t, err, "Cannot get Delivery Service Request by XMLID '%s': %v - alerts: %+v", comment.XMLID, err, resp.Alerts)
+		assert.Equal(t, len(resp.Response), 1, "Found %d Delivery Service request by XMLID '%s, expected exactly one", len(resp.Response), comment.XMLID)
+		assert.NotNil(t, resp.Response[0].ID, "Got Delivery Service Request with xml_id '%s' that had a null ID", comment.XMLID)
 
-	opts := client.NewRequestOptions()
-	for _, comment := range comments.Response {
-		opts.QueryParameters.Set("id", strconv.Itoa(comment.ID))
-		resp, _, err := TOSession.GetDeliveryServiceRequestComments(opts)
-		if err != nil {
-			t.Errorf("cannot get Delivery Service Request Comment by id %d: %v - alerts: %+v", comment.ID, err, resp.Alerts)
-		}
+		comment.DeliveryServiceRequestID = *resp.Response[0].ID
+		alerts, _, err := TOSession.CreateDeliveryServiceRequestComment(comment, client.RequestOptions{})
+		assert.NoError(t, err, "Could not create Delivery Service Request Comment: %v - alerts: %+v", err, alerts.Alerts)
 	}
 }
 
 func DeleteTestDeliveryServiceRequestComments(t *testing.T) {
 	comments, _, err := TOSession.GetDeliveryServiceRequestComments(client.RequestOptions{})
-	if err != nil {
-		t.Errorf("Unexpected error getting Delivery Service Request Comments: %v - alerts: %+v", err, comments.Alerts)
-	}
+	assert.NoError(t, err, "Unexpected error getting Delivery Service Request Comments: %v - alerts: %+v", err, comments.Alerts)
 
-	opts := client.NewRequestOptions()
 	for _, comment := range comments.Response {
 		resp, _, err := TOSession.DeleteDeliveryServiceRequestComment(comment.ID, client.RequestOptions{})
-		if err != nil {
-			t.Errorf("cannot delete Delivery Service Request Comment #%d: %v - alerts: %+v", comment.ID, err, resp.Alerts)
-		}
+		assert.NoError(t, err, "Cannot delete Delivery Service Request Comment #%d: %v - alerts: %+v", comment.ID, err, resp.Alerts)
 
 		// Retrieve the delivery service request comment to see if it got deleted
+		opts := client.NewRequestOptions()
 		opts.QueryParameters.Set("id", strconv.Itoa(comment.ID))
 		comments, _, err := TOSession.GetDeliveryServiceRequestComments(opts)
-		if err != nil {
-			t.Errorf("Unexpected error fetching Delivery Service Request Comment %d after deletion: %v - alerts: %+v", comment.ID, err, comments.Alerts)
-		}
-		if len(comments.Response) > 0 {
-			t.Errorf("expected Delivery Service Request Comment #%d to be deleted, but it was found in Traffic Ops", comment.ID)
-		}
+		assert.NoError(t, err, "Unexpected error fetching Delivery Service Request Comment %d after deletion: %v - alerts: %+v", comment.ID, err, comments.Alerts)
+		assert.Equal(t, len(comments.Response), 0, "Expected Delivery Service Request Comment #%d to be deleted, but it was found in Traffic Ops", comment.ID)
 	}
 }
diff --git a/traffic_ops/testing/api/v4/tc-fixtures.json b/traffic_ops/testing/api/v4/tc-fixtures.json
index 7af7a485c..ebf1fab2a 100644
--- a/traffic_ops/testing/api/v4/tc-fixtures.json
+++ b/traffic_ops/testing/api/v4/tc-fixtures.json
@@ -310,16 +310,20 @@
     ],
     "deliveryServiceRequestComments": [
         {
-            "value": "this is comment one"
+            "value": "this is comment one",
+            "xmlId": "test-ds1"
         },
         {
-            "value": "this is comment two"
+            "value": "this is comment two",
+            "xmlId": "test-ds1"
         },
         {
-            "value": "this is comment three"
+            "value": "this is comment three",
+            "xmlId": "test-ds1"
         },
         {
-            "value": "this is comment four"
+            "value": "this is comment four",
+            "xmlId": "test-ds1"
         }
     ],
     "deliveryServiceRequests": [
@@ -5005,8 +5009,8 @@
         {
             "cachegroup": "cachegroup1",
             "cdnName": "cdn1",
-			"configUpdateTime": "2022-01-01T17:00:00-07:00",
-			"configApplyTime": "1969-12-31T17:00:00-07:00",
+            "configUpdateTime": "2022-01-01T17:00:00-07:00",
+            "configApplyTime": "1969-12-31T17:00:00-07:00",
             "domainName": "ga.atlanta.kabletown.net",
             "guid": null,
             "hostName": "config-update-time",
@@ -5056,8 +5060,8 @@
         {
             "cachegroup": "cachegroup1",
             "cdnName": "cdn1",
-			"configUpdateTime": "2022-01-01T17:00:00-07:00",
-			"configApplyTime": "1969-12-31T17:00:00-07:00",
+            "configUpdateTime": "2022-01-01T17:00:00-07:00",
+            "configApplyTime": "1969-12-31T17:00:00-07:00",
             "domainName": "ga.atlanta.kabletown.net",
             "guid": null,
             "hostName": "config-update-time-no-updpend",
@@ -5145,8 +5149,8 @@
             "profileNames": ["EDGE1"],
             "rack": "RR 119.02",
             "revalPending": false,
-			"revalUpdateTime": "2022-01-01T17:00:00-07:00",
-			"revalApplyTime": "1969-12-31T17:00:00-07:00",
+            "revalUpdateTime": "2022-01-01T17:00:00-07:00",
+            "revalApplyTime": "1969-12-31T17:00:00-07:00",
             "status": "REPORTED",
             "tcpPort": 80,
             "type": "EDGE",
@@ -5195,8 +5199,8 @@
             "physLocation": "Denver",
             "profileNames": ["EDGE1"],
             "rack": "RR 119.02",
-			"revalUpdateTime": "2022-01-01T17:00:00-07:00",
-			"revalApplyTime": "1969-12-31T17:00:00-07:00",
+            "revalUpdateTime": "2022-01-01T17:00:00-07:00",
+            "revalApplyTime": "1969-12-31T17:00:00-07:00",
             "status": "REPORTED",
             "tcpPort": 80,
             "type": "EDGE",
@@ -5207,8 +5211,8 @@
         {
             "cachegroup": "cachegroup1",
             "cdnName": "cdn1",
-			"configUpdateTime": "2022-01-01T17:00:00-07:00",
-			"configApplyTime": "1969-12-31T17:00:00-07:00",
+            "configUpdateTime": "2022-01-01T17:00:00-07:00",
+            "configApplyTime": "1969-12-31T17:00:00-07:00",
             "domainName": "ga.atlanta.kabletown.net",
             "guid": null,
             "hostName": "config-reval-update-time",
@@ -5248,8 +5252,8 @@
             "profileNames": ["EDGE1"],
             "rack": "RR 119.02",
             "revalPending": false,
-			"revalUpdateTime": "2022-01-01T17:00:00-07:00",
-			"revalApplyTime": "1969-12-31T17:00:00-07:00",
+            "revalUpdateTime": "2022-01-01T17:00:00-07:00",
+            "revalApplyTime": "1969-12-31T17:00:00-07:00",
             "status": "REPORTED",
             "tcpPort": 80,
             "type": "EDGE",
@@ -5260,8 +5264,8 @@
         {
             "cachegroup": "cachegroup1",
             "cdnName": "cdn1",
-			"configUpdateTime": "2022-01-01T17:00:00-07:00",
-			"configApplyTime": "1969-12-31T17:00:00-07:00",
+            "configUpdateTime": "2022-01-01T17:00:00-07:00",
+            "configApplyTime": "1969-12-31T17:00:00-07:00",
             "domainName": "ga.atlanta.kabletown.net",
             "guid": null,
             "hostName": "config-reval-update-time-only",
@@ -5300,8 +5304,8 @@
             "physLocation": "Denver",
             "profileNames": ["EDGE1"],
             "rack": "RR 119.02",
-			"revalUpdateTime": "2022-01-01T17:00:00-07:00",
-			"revalApplyTime": "1969-12-31T17:00:00-07:00",
+            "revalUpdateTime": "2022-01-01T17:00:00-07:00",
+            "revalApplyTime": "1969-12-31T17:00:00-07:00",
             "status": "REPORTED",
             "tcpPort": 80,
             "type": "EDGE",
diff --git a/traffic_ops/traffic_ops_golang/server/detail_test.go b/traffic_ops/traffic_ops_golang/server/detail_test.go
index 9ad95ac56..bbcfa5378 100644
--- a/traffic_ops/traffic_ops_golang/server/detail_test.go
+++ b/traffic_ops/traffic_ops_golang/server/detail_test.go
@@ -13,7 +13,9 @@ package server
 */
 
 import (
+	"fmt"
 	"strconv"
+	"strings"
 	"testing"
 
 	"github.com/apache/trafficcontrol/lib/go-tc"
@@ -75,8 +77,7 @@ func TestGetDetailServers(t *testing.T) {
 		"mgmt_ip_netmask",
 		"offline_reason",
 		"phys_location",
-		"profile",
-		"profile_desc",
+		"profile_name",
 		"rack",
 		"status",
 		"tcp_port",
@@ -93,8 +94,6 @@ func TestGetDetailServers(t *testing.T) {
 	serviceNetmask := util.StrPtr("")
 	serviceInterface := util.StrPtr("")
 	serviceMtu := util.StrPtr("")
-	//routerHostName := util.StrPtr("")
-	//routerPort := util.StrPtr("")
 
 	for _, sd := range testServerDetails {
 		detailRows = detailRows.AddRow(
@@ -123,8 +122,7 @@ func TestGetDetailServers(t *testing.T) {
 			sd.MgmtIPNetmask,
 			sd.OfflineReason,
 			sd.PhysLocation,
-			sd.Profile,
-			sd.ProfileDesc,
+			fmt.Sprintf("{%s}", strings.Join(sd.ProfileNames, ",")),
 			sd.Rack,
 			sd.Status,
 			sd.TCPPort,
@@ -143,7 +141,7 @@ func TestGetDetailServers(t *testing.T) {
 	mock.ExpectQuery("SELECT serverid").WillReturnRows(hwInfoRows)
 	mock.ExpectCommit()
 
-	actualSrvs, err := getDetailServers(db.MustBegin().Tx, &auth.CurrentUser{PrivLevel: 30}, "test", 1, "id", 10, api.Version{Major: 3})
+	actualSrvs, err := getDetailServers(db.MustBegin().Tx, &auth.CurrentUser{PrivLevel: 30}, "test", 1, "id", 10, api.Version{Major: 4})
 	if err != nil {
 		t.Fatalf("an error '%s' occurred during read", err)
 	}
@@ -170,9 +168,7 @@ func TestGetDetailServers(t *testing.T) {
 
 func getMockServerDetails() []tc.ServerDetailV40 {
 	srvData := tc.ServerDetailV40{
-		ServerDetail: tc.ServerDetail{
-			ID: util.IntPtr(1),
-		},
+		ID:               util.IntPtr(1),
 		ServerInterfaces: []tc.ServerInterfaceInfoV40{}, // left empty because it must be written as json above since sqlmock does not support nested arrays
 	}
 	return []tc.ServerDetailV40{srvData}