Remove Long Desc 1 and Long Desc 2 fields from API 4.0 (#5986)

* wip

* Adding TP changes and tests

* code review

CHANGELOG.md

@@ -101,6 +101,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - The `traffic_ops_ort.pl` tool has been deprecated in favor of `t3c`, and will be removed in the next major version.
 
 ### Removed
+- Removed the `Long Description 2` and `Long Description 3` fields of `DeliveryService` from the UI, and changed the backend so that routes corresponding API 4.0 and above no longer accept or return these fields. 
 - The Perl implementation of Traffic Ops has been stripped out, along with the Go implementation's "fall-back to Perl" behavior.
 - Traffic Ops no longer includes an `app/public` directory, as the static webserver has been removed along with the Perl Traffic Ops implementation. Traffic Ops also no longer attempts to download MaxMind GeoIP City databases when running the Traffic Ops Postinstall script.
 - The `compare` tool stack has been removed, as it no longer serves a purpose.

docs/source/api/v4/deliveryservice_requests.rst

@@ -138,8 +138,6 @@ The response is an array of representations of :term:`Delivery Service Requests`
 			"lastUpdated": "0001-01-01 00:00:00+00",
 			"logsEnabled": true,
 			"longDesc": "Apachecon North America 2018",
-			"longDesc1": null,
-			"longDesc2": null,
 			"matchList": [
 				{
 					"type": "HOST_REGEXP",
@@ -254,8 +252,6 @@ The request must be a well-formed representation of a :term:`Delivery Service Re
 			"lastUpdated": "2020-02-13 16:43:54+00",
 			"logsEnabled": true,
 			"longDesc": "Apachecon North America 2018",
-			"longDesc1": null,
-			"longDesc2": null,
 			"matchList": [
 				{
 					"type": "HOST_REGEXP",
@@ -379,8 +375,6 @@ The response will be a representation of the created :term:`Delivery Service Req
 				"lastUpdated": "0001-01-01 00:00:00+00",
 				"logsEnabled": true,
 				"longDesc": "Apachecon North America 2018",
-				"longDesc1": null,
-				"longDesc2": null,
 				"matchList": [
 					{
 						"type": "HOST_REGEXP",
@@ -464,8 +458,6 @@ The response will be a representation of the created :term:`Delivery Service Req
 				"lastUpdated": "2020-02-13 16:43:54+00",
 				"logsEnabled": true,
 				"longDesc": "Apachecon North America 2018",
-				"longDesc1": null,
-				"longDesc2": null,
 				"matchList": [
 					{
 						"type": "HOST_REGEXP",
@@ -639,8 +631,6 @@ The response is a full representation of the edited :term:`Delivery Service Requ
 			"lastUpdated": "2020-09-25 02:09:54+00",
 			"logsEnabled": true,
 			"longDesc": "Apachecon North America 2018",
-			"longDesc1": null,
-			"longDesc2": null,
 			"matchList": [
 				{
 					"type": "HOST_REGEXP",
@@ -726,8 +716,6 @@ The response is a full representation of the edited :term:`Delivery Service Requ
 			"lastUpdated": null,
 			"logsEnabled": false,
 			"longDesc": "long desc",
-			"longDesc1": null,
-			"longDesc2": null,
 			"matchList": null,
 			"maxDnsAnswers": null,
 			"midHeaderRewrite": null,

docs/source/api/v4/deliveryservice_requests_id_assign.rst

@@ -172,8 +172,6 @@ The response contains a full representation of the newly assigned :term:`Deliver
 			"lastUpdated": "2020-09-25 02:09:54+00",
 			"logsEnabled": true,
 			"longDesc": "Apachecon North America 2018",
-			"longDesc1": null,
-			"longDesc2": null,
 			"matchList": [
 				{
 					"type": "HOST_REGEXP",
@@ -259,8 +257,6 @@ The response contains a full representation of the newly assigned :term:`Deliver
 			"lastUpdated": null,
 			"logsEnabled": false,
 			"longDesc": "long desc",
-			"longDesc1": null,
-			"longDesc2": null,
 			"matchList": null,
 			"maxDnsAnswers": null,
 			"midHeaderRewrite": null,

docs/source/api/v4/deliveryservice_requests_id_status.rst

@@ -169,8 +169,6 @@ The response is a full representation of the modified :term:`DSR`.
 			"lastUpdated": "2020-09-25 02:09:54+00",
 			"logsEnabled": true,
 			"longDesc": "Apachecon North America 2018",
-			"longDesc1": null,
-			"longDesc2": null,
 			"matchList": [
 				{
 					"type": "HOST_REGEXP",
@@ -256,8 +254,6 @@ The response is a full representation of the modified :term:`DSR`.
 			"lastUpdated": null,
 			"logsEnabled": false,
 			"longDesc": "long desc",
-			"longDesc1": null,
-			"longDesc2": null,
 			"matchList": null,
 			"maxDnsAnswers": null,
 			"midHeaderRewrite": null,

docs/source/api/v4/deliveryservices.rst

@@ -107,8 +107,6 @@ Response Structure
 :lastUpdated:               The date and time at which this :term:`Delivery Service` was last updated, in :ref:`non-rfc-datetime`
 :logsEnabled:               A boolean that defines the :ref:`ds-logs-enabled` setting on this :term:`Delivery Service`
 :longDesc:                  The :ref:`ds-longdesc` of this :term:`Delivery Service`
-:longDesc1:                 The :ref:`ds-longdesc2` of this :term:`Delivery Service`
-:longDesc2:                 The :ref:`ds-longdesc3` of this :term:`Delivery Service`
 :matchList:                 The :term:`Delivery Service`'s :ref:`ds-matchlist`
 
 	:pattern:   A regular expression - the use of this pattern is dependent on the ``type`` field (backslashes are escaped)
@@ -193,8 +191,6 @@ Response Structure
 		"lastUpdated": "2019-05-15 14:32:05+00",
 		"logsEnabled": true,
 		"longDesc": "Apachecon North America 2018",
-		"longDesc1": null,
-		"longDesc2": null,
 		"matchList": [
 			{
 				"type": "HOST_REGEXP",
@@ -293,8 +289,6 @@ Request Structure
 :lastHeaderRewrite:         A set of :ref:`ds-last-header-rw-rules`
 :logsEnabled:               A boolean that defines the :ref:`ds-logs-enabled` setting on this :term:`Delivery Service`
 :longDesc:                  The :ref:`ds-longdesc` of this :term:`Delivery Service`
-:longDesc1:                 An optional field containing the :ref:`ds-longdesc2` of this :term:`Delivery Service`
-:longDesc2:                 An optional field containing the :ref:`ds-longdesc3` of this :term:`Delivery Service`
 :maxDnsAnswers:             The :ref:`ds-max-dns-answers` allowed for this :term:`Delivery Service`
 :maxOriginConnections:      The :ref:`ds-max-origin-connections`
 :maxRequestHeaderBytes:     The :ref:`ds-max-request-header-bytes`
@@ -407,8 +401,6 @@ Response Structure
 :lastUpdated:               The date and time at which this :term:`Delivery Service` was last updated, in :ref:`non-rfc-datetime`
 :logsEnabled:               A boolean that defines the :ref:`ds-logs-enabled` setting on this :term:`Delivery Service`
 :longDesc:                  The :ref:`ds-longdesc` of this :term:`Delivery Service`
-:longDesc1:                 The :ref:`ds-longdesc2` of this :term:`Delivery Service`
-:longDesc2:                 The :ref:`ds-longdesc3` of this :term:`Delivery Service`
 :matchList:                 The :term:`Delivery Service`'s :ref:`ds-matchlist`
 
 	:pattern:   A regular expression - the use of this pattern is dependent on the ``type`` field (backslashes are escaped)
@@ -500,8 +492,6 @@ Response Structure
 			"lastUpdated": "2018-11-19 19:45:49+00",
 			"logsEnabled": true,
 			"longDesc": "A Delivery Service created expressly for API documentation examples",
-			"longDesc1": null,
-			"longDesc2": null,
 			"matchList": [
 				{
 					"type": "HOST_REGEXP",

docs/source/api/v4/deliveryservices_id.rst

@@ -64,8 +64,6 @@ Request Structure
 :lastHeaderRewrite:         A set of :ref:`ds-last-header-rw-rules`
 :logsEnabled:               A boolean that defines the :ref:`ds-logs-enabled` setting on this :term:`Delivery Service`
 :longDesc:                  The :ref:`ds-longdesc` of this :term:`Delivery Service`
-:longDesc1:                 An optional field containing the :ref:`ds-longdesc2` of this :term:`Delivery Service`
-:longDesc2:                 An optional field containing the :ref:`ds-longdesc3` of this :term:`Delivery Service`
 :maxDnsAnswers:             The :ref:`ds-max-dns-answers` allowed for this :term:`Delivery Service`
 :maxOriginConnections:      The :ref:`ds-max-origin-connections`
 :midHeaderRewrite:          A set of :ref:`ds-mid-header-rw-rules`

docs/source/api/v4/deliveryservices_id_safe.rst

@@ -40,7 +40,6 @@ Request Structure
 :displayName: A string that is the :ref:`ds-display-name`
 :infoUrl:     An optional\ [#optional]_ string containing the :ref:`ds-info-url`
 :longDesc:    An optional\ [#optional]_ string containing the :ref:`ds-longdesc` of this :term:`Delivery Service`
-:longDesc1:   An optional\ [#optional]_ string containing the :ref:`ds-longdesc2` of this :term:`Delivery Service`
 
 .. code-block:: http
 	:caption: Request Example
@@ -56,7 +55,7 @@ Request Structure
 	{
 		"displayName": "test",
 		"infoUrl": "this is not even a real URL",
-		"longDesc": "longDesc1 is implicitly set to null in this example
+		"longDesc": "this is a description of the delivery service"
 	}
 
 Response Structure
@@ -97,8 +96,6 @@ Response Structure
 :lastUpdated:               The date and time at which this :term:`Delivery Service` was last updated, in :ref:`non-rfc-datetime`
 :logsEnabled:               A boolean that defines the :ref:`ds-logs-enabled` setting on this :term:`Delivery Service`
 :longDesc:                  The :ref:`ds-longdesc` of this :term:`Delivery Service`
-:longDesc1:                 The :ref:`ds-longdesc2` of this :term:`Delivery Service`
-:longDesc2:                 The :ref:`ds-longdesc3` of this :term:`Delivery Service`
 :matchList:                 The :term:`Delivery Service`'s :ref:`ds-matchlist`
 
 	:pattern:   A regular expression - the use of this pattern is dependent on the ``type`` field (backslashes are escaped)
@@ -183,9 +180,7 @@ Response Structure
 			"lastHeaderRewrite": null,
 			"lastUpdated": "2020-02-10 15:33:03+00",
 			"logsEnabled": true,
-			"longDesc": "longDesc1 is implicitly set to null in this example",
-			"longDesc1": null,
-			"longDesc2": null,
+			"longDesc": "this is a description of the delivery service",
 			"matchList": [
 				{
 					"type": "HOST_REGEXP",

docs/source/api/v4/servers_id_deliveryservices.rst

@@ -103,8 +103,6 @@ Response Structure
 :lastUpdated:               The date and time at which this :term:`Delivery Service` was last updated, in :ref:`non-rfc-datetime`
 :logsEnabled:               A boolean that defines the :ref:`ds-logs-enabled` setting on this :term:`Delivery Service`
 :longDesc:                  The :ref:`ds-longdesc` of this :term:`Delivery Service`
-:longDesc1:                 The :ref:`ds-longdesc2` of this :term:`Delivery Service`
-:longDesc2:                 The :ref:`ds-longdesc3` of this :term:`Delivery Service`
 :matchList:                 The :term:`Delivery Service`'s :ref:`ds-matchlist`
 
 	:pattern:   A regular expression - the use of this pattern is dependent on the ``type`` field (backslashes are escaped)
@@ -187,8 +185,6 @@ Response Structure
 		"lastUpdated": "2019-06-10 15:14:29+00",
 		"logsEnabled": true,
 		"longDesc": "Apachecon North America 2018",
-		"longDesc1": null,
-		"longDesc2": null,
 		"matchList": [
 			{
 				"type": "HOST_REGEXP",

docs/source/overview/delivery_services.rst

@@ -460,34 +460,6 @@ Free text field that has no strictly defined purpose, but it is suggested that i
 	| longDesc | Traffic Control source code and :ref:`to-api` responses | unchanged (``string``, ``String`` etc.) |
 	+----------+---------------------------------------------------------+-----------------------------------------+
 
-.. _ds-longdesc2:
-
-Long Description 2
-------------------
-Free text field that has no strictly defined purpose.
-
-.. table::
-
-	+----------------------------+---------------------------------------------------------+-----------------------------------------+
-	| Name                       | Use(s)                                                  | Type(s)                                 |
-	+============================+=========================================================+=========================================+
-	| longDesc1\ [#cardinality]_ | Traffic Control source code and :ref:`to-api` responses | unchanged (``string``, ``String`` etc.) |
-	+----------------------------+---------------------------------------------------------+-----------------------------------------+
-
-.. _ds-longdesc3:
-
-Long Description 3
-------------------
-Free text field that has no strictly defined purpose.
-
-.. table::
-
-	+----------------------------+---------------------------------------------------------+-----------------------------------------+
-	| Name                       | Use(s)                                                  | Type(s)                                 |
-	+============================+=========================================================+=========================================+
-	| longDesc2\ [#cardinality]_ | Traffic Control source code and :ref:`to-api` responses | unchanged (``string``, ``String`` etc.) |
-	+----------------------------+---------------------------------------------------------+-----------------------------------------+
-
 .. _ds-matchlist:
 
 Match List

lib/go-tc/deliveryservices.go

@@ -304,8 +304,8 @@ type DeliveryServiceNullableFieldsV11 struct {
 	LastUpdated              *TimeNoMod              `json:"lastUpdated" db:"last_updated"`
 	LogsEnabled              *bool                   `json:"logsEnabled" db:"logs_enabled"`
 	LongDesc                 *string                 `json:"longDesc" db:"long_desc"`
-	LongDesc1                *string                 `json:"longDesc1" db:"long_desc_1"`
-	LongDesc2                *string                 `json:"longDesc2" db:"long_desc_2"`
+	LongDesc1                *string                 `json:"longDesc1,omitempty" db:"long_desc_1"`
+	LongDesc2                *string                 `json:"longDesc2,omitempty" db:"long_desc_2"`
 	MatchList                *[]DeliveryServiceMatch `json:"matchList"`
 	MaxDNSAnswers            *int                    `json:"maxDnsAnswers" db:"max_dns_answers"`
 	MidHeaderRewrite         *string                 `json:"midHeaderRewrite" db:"mid_header_rewrite"`
@@ -339,6 +339,13 @@ type DeliveryServiceRemovedFieldsV11 struct {
 	CacheURL *string `json:"cacheurl" db:"cacheurl"`
 }
 
+// RemoveLD1AndLD2 removes the Long Description 1 and Long Description 2 fields from a V 4.x DS, and returns the resulting struct.
+func (ds *DeliveryServiceV4) RemoveLD1AndLD2() DeliveryServiceV4 {
+	ds.LongDesc1 = nil
+	ds.LongDesc2 = nil
+	return *ds
+}
+
 // DowngradeToV3 converts the 4.x DS to a 3.x DS
 func (ds *DeliveryServiceV4) DowngradeToV3() DeliveryServiceNullableV30 {
 	return DeliveryServiceNullableV30{
@@ -591,7 +598,7 @@ type DeliveryServiceSafeUpdateRequest struct {
 	DisplayName *string `json:"displayName"`
 	InfoURL     *string `json:"infoUrl"`
 	LongDesc    *string `json:"longDesc"`
-	LongDesc1   *string `json:"longDesc1"`
+	LongDesc1   *string `json:"longDesc1,omitempty"`
 }
 
 // Validate implements the github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api.ParseValidator

traffic_ops/testing/api/v4/deliveryservice_requests_test.go

@@ -24,6 +24,7 @@ import (
 
 	"github.com/apache/trafficcontrol/lib/go-rfc"
 	tc "github.com/apache/trafficcontrol/lib/go-tc"
+	"github.com/apache/trafficcontrol/lib/go-util"
 	client "github.com/apache/trafficcontrol/traffic_ops/v4-client"
 )
 
@@ -65,6 +66,7 @@ func TestDeliveryServiceRequests(t *testing.T) {
 		header = make(map[string][]string)
 		header.Set(rfc.IfModifiedSince, time)
 		header.Set(rfc.IfUnmodifiedSince, time)
+		UpdateTestDeliveryServiceRequestsWithLongDescFields(t)
 		UpdateTestDeliveryServiceRequests(t)
 		UpdateTestDeliveryServiceRequestsWithHeaders(t, header)
 		GetTestDeliveryServiceRequestsIMSAfterChange(t, header)
@@ -75,6 +77,61 @@ func TestDeliveryServiceRequests(t *testing.T) {
 	})
 }
 
+func UpdateTestDeliveryServiceRequestsWithLongDescFields(t *testing.T) {
+	if len(testData.DeliveryServiceRequests) < dsrGood+1 {
+		t.Fatalf("Need at least %d Delivery Service Requests to test updating them", dsrGood+1)
+	}
+
+	// Retrieve the DeliveryServiceRequest by name so we can get the id for the Update
+	dsr := testData.DeliveryServiceRequests[dsrGood]
+	var ds *tc.DeliveryServiceV4
+	if dsr.ChangeType == tc.DSRChangeTypeDelete {
+		dsr.Original.LongDesc1 = util.StrPtr("long desc 1")
+		dsr.Original.LongDesc2 = util.StrPtr("long desc 2")
+		ds = dsr.Original
+	} else {
+		dsr.Requested.LongDesc1 = util.StrPtr("long desc 1")
+		dsr.Requested.LongDesc2 = util.StrPtr("long desc 2")
+		ds = dsr.Requested
+	}
+
+	if ds == nil || ds.XMLID == nil {
+		t.Fatalf("the %dth DSR in the test data had no DeliveryService - or that DeliveryService had no XMLID", dsrGood)
+	}
+
+	opts := client.NewRequestOptions()
+	opts.QueryParameters.Set("xmlId", *ds.XMLID)
+	resp, _, err := TOSession.GetDeliveryServiceRequests(opts)
+	if err != nil {
+		t.Errorf("cannot get Delivery Service Request with XMLID '%s': %v - alerts: %+v", *ds.XMLID, err, resp.Alerts)
+	}
+	if len(resp.Response) == 0 {
+		t.Fatalf("Expected at least one Deliver Service Request to exist with XMLID '%s', but none were found in Traffic Ops", *ds.XMLID)
+	}
+	respDSR := resp.Response[0]
+	if respDSR.ID == nil {
+		t.Fatalf("got a DSR by XMLID '%s' with a null or undefined ID", *ds.XMLID)
+	}
+	var respDS *tc.DeliveryServiceV4
+	if dsr.ChangeType == tc.DSRChangeTypeDelete {
+		respDS = dsr.Original
+		respDSR.Original = respDS
+	} else {
+		respDS = dsr.Requested
+		respDSR.Requested = respDS
+	}
+	expDisplayName := "new display name"
+	respDS.DisplayName = &expDisplayName
+	id := *respDSR.ID
+	_, reqInf, err := TOSession.UpdateDeliveryServiceRequest(id, respDSR, client.RequestOptions{})
+	if err == nil {
+		t.Errorf("expected an error stating that Long Desc 1 and Long Desc 2 fields are not supported in api version 4.0 onwards, but got nothing")
+	}
+	if reqInf.StatusCode != http.StatusBadRequest {
+		t.Errorf("expected a 400 status code, but got %d", reqInf.StatusCode)
+	}
+}
+
 // Note that this test is suceptible to breaking if the structure of the test
 // data's DSRs is altered at all.
 func UpdateTestDeliveryServiceRequestsWithHeaders(t *testing.T, header http.Header) {