You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2022/04/22 20:20:13 UTC

[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #6758: Refactor Delivery Service Requests

rimashah25 commented on code in PR #6758:
URL: https://github.com/apache/trafficcontrol/pull/6758#discussion_r856543542


##########
traffic_ops/testing/api/v3/deliveryservice_requests_test.go:
##########
@@ -16,366 +16,317 @@ package v3
 */
 
 import (
+	"encoding/json"
 	"net/http"
+	"net/url"
 	"strings"
 	"testing"
 	"time"
 
 	"github.com/apache/trafficcontrol/lib/go-rfc"
-	tc "github.com/apache/trafficcontrol/lib/go-tc"
-)
-
-const (
-	dsrGood      = 0
-	dsrBadTenant = 1
-	dsrRequired  = 2
-	dsrDraft     = 3
+	"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 TestDeliveryServiceRequests(t *testing.T) {
 	WithObjs(t, []TCObj{CDNs, Types, Parameters, Tenants, DeliveryServiceRequests}, func() {
-		GetTestDeliveryServiceRequestsIMS(t)
-		GetTestDeliveryServiceRequests(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.IfModifiedSince, time)
-		header.Set(rfc.IfUnmodifiedSince, time)
-		UpdateTestDeliveryServiceRequests(t)
-		UpdateTestDeliveryServiceRequestsWithHeaders(t, header)
-		GetTestDeliveryServiceRequestsIMSAfterChange(t, header)
-		header = make(map[string][]string)
-		etag := rfc.ETag(currentTime)
-		header.Set(rfc.IfMatch, etag)
-		UpdateTestDeliveryServiceRequestsWithHeaders(t, header)
-	})
-}
-
-func UpdateTestDeliveryServiceRequestsWithHeaders(t *testing.T, header http.Header) {
-	// Retrieve the DeliveryServiceRequest by name so we can get the id for the Update
-	dsr := testData.DeliveryServiceRequests[dsrGood]
-	resp, _, err := TOSession.GetDeliveryServiceRequestByXMLIDWithHdr(dsr.DeliveryService.XMLID, header)
-	if err != nil {
-		t.Errorf("cannot GET DeliveryServiceRequest by name: %v - %v", dsr.DeliveryService.XMLID, err)
-	}
-	if len(resp) == 0 {
-		t.Fatal("Length of GET DeliveryServiceRequest is 0")
-	}
-	respDSR := resp[0]
-	respDSR.DeliveryService.DisplayName = "new display name"
 
-	_, reqInf, err := TOSession.UpdateDeliveryServiceRequestByIDWithHdr(respDSR.ID, respDSR, header)
-	if err == nil {
-		t.Errorf("Expected error about precondition failed, but got none")
-	}
-	if reqInf.StatusCode != http.StatusPreconditionFailed {
-		t.Errorf("Expected status code 412, got %v", reqInf.StatusCode)
-	}
-}
-
-func GetTestDeliveryServiceRequestsIMSAfterChange(t *testing.T, header http.Header) {
-	dsr := testData.DeliveryServiceRequests[dsrGood]
-	_, reqInf, err := TOSession.GetDeliveryServiceRequestByXMLIDWithHdr(dsr.DeliveryService.XMLID, header)
-	if err != nil {
-		t.Fatalf("Expected no error, but got %v", err.Error())
-	}
-	if reqInf.StatusCode != http.StatusOK {
-		t.Fatalf("Expected 200 status code, got %v", reqInf.StatusCode)
-	}
-	currentTime := time.Now().UTC()
-	currentTime = currentTime.Add(1 * time.Second)
-	timeStr := currentTime.Format(time.RFC1123)
-	header.Set(rfc.IfModifiedSince, timeStr)
-	_, reqInf, err = TOSession.GetDeliveryServiceRequestByXMLIDWithHdr(dsr.DeliveryService.XMLID, header)
-	if err != nil {
-		t.Fatalf("Expected no error, but got %v", err.Error())
-	}
-	if reqInf.StatusCode != http.StatusNotModified {
-		t.Fatalf("Expected 304 status code, got %v", reqInf.StatusCode)
-	}
-}
-
-func CreateTestDeliveryServiceRequests(t *testing.T) {
-	t.Log("CreateTestDeliveryServiceRequests")
-
-	dsr := testData.DeliveryServiceRequests[dsrGood]
-	respDSR, _, err := TOSession.CreateDeliveryServiceRequest(dsr)
-	t.Log("Response: ", respDSR)
-	if err != nil {
-		t.Errorf("could not CREATE DeliveryServiceRequests: %v", err)
-	}
-
-}
-
-func TestDeliveryServiceRequestRequired(t *testing.T) {
-	WithObjs(t, []TCObj{CDNs, Types, Parameters, Tenants}, func() {
-		dsr := testData.DeliveryServiceRequests[dsrRequired]
-		_, _, err := TOSession.CreateDeliveryServiceRequest(dsr)
-		if err == nil {
-			t.Error("expected: validation error, actual: nil")
+		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 XMLID parameter": {
+					ClientSession: TOSession, RequestParams: url.Values{"xmlId": {"test-ds1"}},
+					Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), utils.ResponseLengthGreaterOrEqual(1),
+						validateGetDSRequestFields(map[string]interface{}{"XMLID": "test-ds1"})),
+				},
+			},
+			"PUT": {
+				"OK when VALID request": {
+					EndpointId: GetDeliveryServiceRequestId(t, "test-ds1"), ClientSession: TOSession,
+					RequestBody: map[string]interface{}{
+						"changeType": "create",
+						"deliveryService": generateDeliveryService(t, map[string]interface{}{
+							"displayName": "NEW DISPLAY NAME",
+							"tenantId":    GetTenantId(t, "tenant1"),
+							"xmlId":       "test-ds1",
+						}),
+						"status": "draft",
+					},
+					Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
+				},
+				"OK when UPDATING STATUS FROM DRAFT TO SUBMITTED": {
+					EndpointId: GetDeliveryServiceRequestId(t, "test-ds1"), ClientSession: TOSession,
+					RequestBody: map[string]interface{}{
+						"changeType": "create",
+						"deliveryService": generateDeliveryService(t, map[string]interface{}{
+							"tenantId": GetTenantId(t, "tenant1"),
+							"xmlId":    "test-ds1",
+						}),
+						"status": "submitted",
+					},
+					Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
+				},
+				"PRECONDITION FAILED when updating with IF-UNMODIFIED-SINCE Header": {
+					EndpointId: GetDeliveryServiceRequestId(t, "test-ds1"), 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: GetDeliveryServiceRequestId(t, "test-ds1"), ClientSession: TOSession,
+					RequestBody:    map[string]interface{}{},
+					RequestHeaders: http.Header{rfc.IfMatch: {rfc.ETag(currentTime)}},
+					Expectations:   utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusPreconditionFailed)),
+				},
+			},
+			"POST": {
+				"OK when VALID request": {
+					ClientSession: TOSession, RequestBody: map[string]interface{}{
+						"changeType": "create",
+						"deliveryService": generateDeliveryService(t, map[string]interface{}{
+							"ccrDnsTtl":          30,
+							"deepCachingType":    "NEVER",
+							"initialDispersion":  3,
+							"ipv6RoutingEnabled": true,
+							"longDesc":           "long desc",
+							"orgServerFqdn":      "http://example.test",
+							"profileName":        nil,
+							"tenant":             "root",
+							"xmlId":              "test-ds2",
+						}),
+						"status": "draft",
+					},
+					Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
+				},
+				"BAD REQUEST when MISSING REQUIRED FIELDS": {
+					ClientSession: TOSession, RequestBody: map[string]interface{}{
+						"changeType": "create",
+						"deliveryService": map[string]interface{}{
+							"type":  "HTTP",
+							"xmlId": "test-ds-fields",
+						},
+						"status": "draft",
+					},
+					Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+				},
+				"BAD REQUEST when VALIDATION RULES ARE NOT FOLLOWED": {
+					ClientSession: TOSession, RequestBody: map[string]interface{}{
+						"changeType": "create",
+						"deliveryService": map[string]interface{}{
+							"ccrDnsTtl":            30,
+							"deepCachingType":      "NEVER",
+							"displayName":          strings.Repeat("X", 49),
+							"dscp":                 0,
+							"geoLimit":             0,
+							"geoProvider":          1,
+							"infoUrl":              "xxx",
+							"initialDispersion":    1,
+							"ipv6RoutingEnabled":   true,
+							"logsEnabled":          true,
+							"longDesc":             "long desc",
+							"missLat":              0.0,
+							"missLong":             0.0,
+							"multiSiteOrigin":      false,
+							"orgServerFqdn":        "http://example.test",
+							"protocol":             0,
+							"qstringIgnore":        0,
+							"rangeRequestHandling": 0,
+							"regionalGeoBlocking":  true,
+							"routingName":          strings.Repeat("X", 1) + "." + strings.Repeat("X", 48),
+							"tenant":               "tenant1",
+							"type":                 "HTTP",
+							"xmlId":                "X " + strings.Repeat("X", 46),
+						},
+						"status": "draft",
+					},
+					Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+				},
+				"BAD REQUEST when NON-DRAFT": {
+					ClientSession: TOSession, RequestBody: map[string]interface{}{
+						"changeType": "create",
+						"deliveryService": map[string]interface{}{
+							"active":               false,
+							"cdnName":              "cdn1",
+							"displayName":          "Testing transitions",
+							"dscp":                 3,
+							"geoLimit":             1,
+							"geoProvider":          1,
+							"initialDispersion":    1,
+							"ipv6RoutingEnabled":   true,
+							"logsEnabled":          true,
+							"missLat":              0.0,
+							"missLong":             0.0,
+							"multiSiteOrigin":      false,
+							"orgServerFqdn":        "http://example.test",
+							"protocol":             0,
+							"qstringIgnore":        0,
+							"rangeRequestHandling": 0,
+							"regionalGeoBlocking":  true,
+							"routingName":          "goodroute",
+							"tenant":               "tenant1",
+							"type":                 "HTTP",
+							"xmlId":                "test-transitions",
+						},
+						"status": "pending",
+					},
+					Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+				},
+				"BAD REQUEST when ALREADY EXISTS": {
+					ClientSession: TOSession, RequestBody: map[string]interface{}{
+						"changeType": "create",
+						"deliveryService": map[string]interface{}{
+							"active":               true,
+							"cdnName":              "cdn1",
+							"displayName":          "Good Kabletown CDN",
+							"dscp":                 1,
+							"geoLimit":             1,
+							"geoProvider":          1,
+							"initialDispersion":    1,
+							"ipv6RoutingEnabled":   true,
+							"logsEnabled":          true,
+							"missLat":              0.0,
+							"missLong":             0.0,
+							"multiSiteOrigin":      false,
+							"orgServerFqdn":        "http://example.test",
+							"protocol":             0,
+							"qstringIgnore":        0,
+							"rangeRequestHandling": 0,
+							"regionalGeoBlocking":  true,
+							"routingName":          "goodroute",
+							"tenant":               "tenant1",
+							"type":                 "HTTP",
+							"xmlId":                "test-ds1",
+						},
+						"status": "draft",
+					},
+					Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+				},
+			},
+			"DELETE": {
+				"OK when VALID request": {
+					EndpointId: GetDeliveryServiceRequestId(t, "test-deletion"), ClientSession: TOSession,
+					Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
+				},
+			},
+			"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 TestDeliveryServiceRequestRules(t *testing.T) {
-	WithObjs(t, []TCObj{CDNs, Types, Parameters, Tenants}, func() {
-		routingName := strings.Repeat("X", 1) + "." + strings.Repeat("X", 48)
-		// Test the xmlId length and form
-		XMLID := "X " + strings.Repeat("X", 46)
-		displayName := strings.Repeat("X", 49)
-
-		dsr := testData.DeliveryServiceRequests[dsrGood]
-		dsr.DeliveryService.DisplayName = displayName
-		dsr.DeliveryService.RoutingName = routingName
-		dsr.DeliveryService.XMLID = XMLID
-
-		_, _, err := TOSession.CreateDeliveryServiceRequest(dsr)
-		if err == nil {
-			t.Error("expected: validation error, actual: nil")
-		}
-	})
-}
-
-func TestDeliveryServiceRequestTypeFields(t *testing.T) {
-	WithObjs(t, []TCObj{CDNs, Types, Tenants, Parameters}, func() {
-		dsr := testData.DeliveryServiceRequests[dsrBadTenant]
-
-		alerts, _, err := TOSession.CreateDeliveryServiceRequest(dsr)
-		if err != nil {
-			t.Errorf("Error occurred %v", err)
-		}
-
-		found := false
-		for _, alert := range alerts.Alerts {
-			if alert.Level == tc.ErrorLevel.String() {
-				t.Errorf("Expected only succuss-level alerts creating a DSR, got error-level alert: %s", alert.Text)
-			} else if alert.Level == tc.SuccessLevel.String() {
-				t.Logf("Got expected alert creating a DSR: %s", alert.Text)
-				found = true
-			}
-		}
-		if !found {
-			t.Errorf("Expected a success-level alert creating a DSR, got none")
+		for method, testCases := range methodTests {
+			t.Run(method, func(t *testing.T) {
+				for name, testCase := range testCases {
+					dsReq := tc.DeliveryServiceRequest{}
+
+					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, &dsReq)
+						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 XMLID parameter" {
+								resp, reqInf, err := testCase.ClientSession.GetDeliveryServiceRequestByXMLIDWithHdr(testCase.RequestParams["xmlId"][0], testCase.RequestHeaders)
+								for _, check := range testCase.Expectations {
+									check(t, reqInf, resp, tc.Alerts{}, err)
+								}
+							} else {
+								resp, reqInf, err := testCase.ClientSession.GetDeliveryServiceRequestsWithHdr(testCase.RequestHeaders)
+								for _, check := range testCase.Expectations {
+									check(t, reqInf, resp, tc.Alerts{}, err)
+								}
+							}
+						})
+					case "POST":
+						t.Run(name, func(t *testing.T) {
+							alerts, reqInf, err := testCase.ClientSession.CreateDeliveryServiceRequest(dsReq)
+							for _, check := range testCase.Expectations {
+								check(t, reqInf, nil, alerts, err)
+							}
+						})
+					case "PUT":
+						t.Run(name, func(t *testing.T) {
+							alerts, reqInf, err := testCase.ClientSession.UpdateDeliveryServiceRequestByIDWithHdr(testCase.EndpointId(), dsReq, testCase.RequestHeaders)
+							for _, check := range testCase.Expectations {
+								check(t, reqInf, nil, alerts, err)
+							}
+						})
+					case "DELETE":

Review Comment:
   Why is there a case DELETE, when it is already covered in withobjs_test.go?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org