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 2021/06/30 17:44:30 UTC

[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #5986: Remove Long Desc 1 and Long Desc 2 fields from API 4.0

rawlinp commented on a change in pull request #5986:
URL: https://github.com/apache/trafficcontrol/pull/5986#discussion_r661680370



##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -352,67 +354,132 @@ func createV40(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV40 t
 	if errCode, userErr, sysErr := dbhelpers.CheckTopology(inf.Tx, ds); userErr != nil || sysErr != nil {
 		return nil, errCode, userErr, sysErr
 	}
-	resultRows, err := tx.Query(insertQuery(),
-		&ds.Active,
-		&ds.AnonymousBlockingEnabled,
-		&ds.CCRDNSTTL,
-		&ds.CDNID,
-		&ds.CheckPath,
-		&ds.ConsistentHashRegex,
-		&deepCachingType,
-		&ds.DisplayName,
-		&ds.DNSBypassCNAME,
-		&ds.DNSBypassIP,
-		&ds.DNSBypassIP6,
-		&ds.DNSBypassTTL,
-		&ds.DSCP,
-		&ds.EdgeHeaderRewrite,
-		&ds.GeoLimitRedirectURL,
-		&ds.GeoLimit,
-		&ds.GeoLimitCountries,
-		&ds.GeoProvider,
-		&ds.GlobalMaxMBPS,
-		&ds.GlobalMaxTPS,
-		&ds.FQPacingRate,
-		&ds.HTTPBypassFQDN,
-		&ds.InfoURL,
-		&ds.InitialDispersion,
-		&ds.IPV6RoutingEnabled,
-		&ds.LogsEnabled,
-		&ds.LongDesc,
-		&ds.LongDesc1,
-		&ds.LongDesc2,
-		&ds.MaxDNSAnswers,
-		&ds.MaxOriginConnections,
-		&ds.MidHeaderRewrite,
-		&ds.MissLat,
-		&ds.MissLong,
-		&ds.MultiSiteOrigin,
-		&ds.OriginShield,
-		&ds.ProfileID,
-		&ds.Protocol,
-		&ds.QStringIgnore,
-		&ds.RangeRequestHandling,
-		&ds.RegexRemap,
-		&ds.RegionalGeoBlocking,
-		&ds.RemapText,
-		&ds.RoutingName,
-		&ds.SigningAlgorithm,
-		&ds.SSLKeyVersion,
-		&ds.TenantID,
-		&ds.Topology,
-		&ds.TRRequestHeaders,
-		&ds.TRResponseHeaders,
-		&ds.TypeID,
-		&ds.XMLID,
-		&ds.EcsEnabled,
-		&ds.RangeSliceBlockSize,
-		&ds.FirstHeaderRewrite,
-		&ds.InnerHeaderRewrite,
-		&ds.LastHeaderRewrite,
-		&ds.ServiceCategory,
-		&ds.MaxRequestHeaderBytes,
-	)
+	if omitExtraLongDescFields {
+		if ds.LongDesc1 != nil || ds.LongDesc2 != nil {
+			return nil, http.StatusBadRequest, errors.New("the Long Description 1 and Long Description 2 fields are no longer supported in API 4.0 onwards"), nil

Review comment:
       this error message should probably use the json field names, i.e. `longDesc1` and `longDesc2` just to be clear

##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go
##########
@@ -716,6 +740,18 @@ func putV40(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (result ds
 		dsr.Status,
 		inf.IntParams["id"],
 	}
+	if dsr.Original != nil {
+		if dsr.Original.LongDesc1 != nil || dsr.Original.LongDesc2 != nil {
+			api.HandleErr(w, r, tx, http.StatusBadRequest, errors.New("the Long Description 1 and Long Description 2 fields are no longer supported in API 4.0 onwards"), nil)

Review comment:
       this error message should probably use the json field names, i.e. `longDesc1` and `longDesc2` just to be clear

##########
File path: 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:

Review comment:
       the fields should also be removed in the v4 API docs (`grep` for longDesc1 and longDesc2)

##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/request/assign.go
##########
@@ -223,6 +223,16 @@ func PutAssignment(w http.ResponseWriter, r *http.Request) {
 	}
 	var resp interface{}
 	if inf.Version.Major >= 4 {
+		if dsr.Requested.LongDesc1 != nil || dsr.Requested.LongDesc2 != nil {
+			api.HandleErr(w, r, tx, http.StatusBadRequest, errors.New("the Long Description 1 and Long Description 2 fields are no longer supported in API 4.0 onwards"), nil)

Review comment:
       this error message should probably use the json field names, i.e. `longDesc1` and `longDesc2` just to be clear

##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go
##########
@@ -449,7 +458,18 @@ func createV4(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (result
 		api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, nil)
 		return
 	}
-
+	if dsr.Original != nil {
+		if dsr.Original.LongDesc1 != nil || dsr.Original.LongDesc2 != nil {
+			api.HandleErr(w, r, tx, http.StatusBadRequest, errors.New("the Long Description 1 and Long Description 2 fields are no longer supported in API 4.0 onwards"), nil)

Review comment:
       this error message should probably use the json field names, i.e. `longDesc1` and `longDesc2` just to be clear

##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/safe.go
##########
@@ -65,11 +85,20 @@ func UpdateSafe(w http.ResponseWriter, r *http.Request) {
 		api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("decoding: %s", err), nil)
 		return
 	}
-	if ok, err := updateDSSafe(tx, dsID, dsr); err != nil {
+	if version.Major > 3 && version.Minor >= 0 {
+		if dsr.LongDesc1 != nil {
+			api.HandleErr(w, r, tx, http.StatusBadRequest, errors.New("the Long Description 1 field are no longer supported in API 4.0 onwards"), nil)

Review comment:
       this error message should probably use the json field name, i.e. `longDesc1` just to be clear. Also `are` -> `is`

##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go
##########
@@ -449,7 +458,18 @@ func createV4(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (result
 		api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, nil)
 		return
 	}
-
+	if dsr.Original != nil {
+		if dsr.Original.LongDesc1 != nil || dsr.Original.LongDesc2 != nil {
+			api.HandleErr(w, r, tx, http.StatusBadRequest, errors.New("the Long Description 1 and Long Description 2 fields are no longer supported in API 4.0 onwards"), nil)
+			return
+		}
+	}
+	if dsr.Requested != nil {
+		if dsr.Requested.LongDesc1 != nil || dsr.Requested.LongDesc2 != nil {
+			api.HandleErr(w, r, tx, http.StatusBadRequest, errors.New("the Long Description 1 and Long Description 2 fields are no longer supported in API 4.0 onwards"), nil)

Review comment:
       this error message should probably use the json field names, i.e. `longDesc1` and `longDesc2` just to be clear

##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -1006,67 +1075,132 @@ func updateV40(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV40 *
 		}
 	}
 
-	resultRows, err := tx.Query(updateDSQuery(),
-		&ds.Active,
-		&ds.CCRDNSTTL,
-		&ds.CDNID,
-		&ds.CheckPath,
-		&deepCachingType,
-		&ds.DisplayName,
-		&ds.DNSBypassCNAME,
-		&ds.DNSBypassIP,
-		&ds.DNSBypassIP6,
-		&ds.DNSBypassTTL,
-		&ds.DSCP,
-		&ds.EdgeHeaderRewrite,
-		&ds.GeoLimitRedirectURL,
-		&ds.GeoLimit,
-		&ds.GeoLimitCountries,
-		&ds.GeoProvider,
-		&ds.GlobalMaxMBPS,
-		&ds.GlobalMaxTPS,
-		&ds.FQPacingRate,
-		&ds.HTTPBypassFQDN,
-		&ds.InfoURL,
-		&ds.InitialDispersion,
-		&ds.IPV6RoutingEnabled,
-		&ds.LogsEnabled,
-		&ds.LongDesc,
-		&ds.LongDesc1,
-		&ds.LongDesc2,
-		&ds.MaxDNSAnswers,
-		&ds.MidHeaderRewrite,
-		&ds.MissLat,
-		&ds.MissLong,
-		&ds.MultiSiteOrigin,
-		&ds.OriginShield,
-		&ds.ProfileID,
-		&ds.Protocol,
-		&ds.QStringIgnore,
-		&ds.RangeRequestHandling,
-		&ds.RegexRemap,
-		&ds.RegionalGeoBlocking,
-		&ds.RemapText,
-		&ds.RoutingName,
-		&ds.SigningAlgorithm,
-		&ds.SSLKeyVersion,
-		&ds.TenantID,
-		&ds.TRRequestHeaders,
-		&ds.TRResponseHeaders,
-		&ds.TypeID,
-		&ds.XMLID,
-		&ds.AnonymousBlockingEnabled,
-		&ds.ConsistentHashRegex,
-		&ds.MaxOriginConnections,
-		&ds.EcsEnabled,
-		&ds.RangeSliceBlockSize,
-		&ds.Topology,
-		&ds.FirstHeaderRewrite,
-		&ds.InnerHeaderRewrite,
-		&ds.LastHeaderRewrite,
-		&ds.ServiceCategory,
-		&ds.MaxRequestHeaderBytes,
-		&ds.ID)
+	if omitExtraLongDescFields {
+		if ds.LongDesc1 != nil || ds.LongDesc2 != nil {
+			return nil, http.StatusBadRequest, errors.New("the Long Description 1 and Long Description 2 fields are no longer supported in API 4.0 onwards"), nil

Review comment:
       this error message should probably use the json field names, i.e. `longDesc1` and `longDesc2` just to be clear

##########
File path: 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` from the UI, and changed the backend so that routes corresponding API 4.0 and above no longer accept or return these fields. 

Review comment:
       the changelog message should specify that those are delivery service fields

##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go
##########
@@ -716,6 +740,18 @@ func putV40(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (result ds
 		dsr.Status,
 		inf.IntParams["id"],
 	}
+	if dsr.Original != nil {
+		if dsr.Original.LongDesc1 != nil || dsr.Original.LongDesc2 != nil {
+			api.HandleErr(w, r, tx, http.StatusBadRequest, errors.New("the Long Description 1 and Long Description 2 fields are no longer supported in API 4.0 onwards"), nil)
+			return
+		}
+	}
+	if dsr.Requested != nil {
+		if dsr.Requested.LongDesc1 != nil || dsr.Requested.LongDesc2 != nil {
+			api.HandleErr(w, r, tx, http.StatusBadRequest, errors.New("the Long Description 1 and Long Description 2 fields are no longer supported in API 4.0 onwards"), nil)

Review comment:
       this error message should probably use the json field names, i.e. `longDesc1` and `longDesc2` just to be clear




-- 
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