rawlinp commented on a change in pull request #4644:
URL: https://github.com/apache/trafficcontrol/pull/4644#discussion_r578796053
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -278,14 +300,42 @@ func createV30(w http.ResponseWriter, r *http.Request,
inf *api.APIInfo, dsV30 t
}
return nil, status, userErr, sysErr
}
+func createV31(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV31
tc.DeliveryServiceV31) (*tc.DeliveryServiceV31, int, error, error) {
+ tx := inf.Tx.Tx
+ dsNullable := tc.DeliveryServiceNullableV30(dsV31)
+ ds := dsNullable.UpgradeToV4()
+ res, status, userErr, sysErr := createV40(w, r, inf,
tc.DeliveryServiceV40(ds))
+ if res == nil {
+ return nil, status, userErr, sysErr
+ }
+
+ ds = tc.DeliveryServiceNullableV4(*res)
+ if dsV31.CacheURL != nil {
+ rows, err := tx.Query("UPDATE deliveryservice SET cacheurl = $1
WHERE ID = $2",
Review comment:
nit: updates that don't return anything should probably use `Exec`
instead. Then you don't have to do `rows.Close()`.
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -501,20 +548,22 @@ func (ds *TODeliveryService) Read(h http.Header, useIMS
bool) ([]interface{}, er
for _, ds := range dses {
switch {
// NOTE: it's required to handle minor version cases in a
descending >= manner
- case version.Major > 3 && version.Major >= 0:
+ case version.Major > 4 && version.Minor >= 0:
Review comment:
I think this switch statement is incorrect. The `case version.Major > 4
&& version.Minor >= 0:` case should be removed, and the `case version.Major > 3
&& version.Minor >= 0:` case should append `ds` not `ds.DowngradeToV3()` since
that is the case that actually handles 4.0 requests.
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -823,12 +886,39 @@ WHERE
}
return nil, status, userErr, sysErr
}
-
func updateV31(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV31
*tc.DeliveryServiceV31) (*tc.DeliveryServiceV31, int, error, error) {
+ dsNull := tc.DeliveryServiceNullableV30(*dsV31)
+ ds := dsNull.UpgradeToV4()
+ dsV40 := tc.DeliveryServiceV40(ds)
+ tx := inf.Tx.Tx
+ res, status, usrErr, sysErr := updateV40(w, r, inf, &dsV40)
+ if res == nil {
+ return nil, status, usrErr, sysErr
+ }
+ ds = tc.DeliveryServiceNullableV4(*res)
+ if dsV31.CacheURL != nil {
+ rows, err := tx.Query("UPDATE deliveryservice SET cacheurl = $1
WHERE ID = $2",
Review comment:
Ditto above comment about `Exec`
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -278,14 +300,42 @@ func createV30(w http.ResponseWriter, r *http.Request,
inf *api.APIInfo, dsV30 t
}
return nil, status, userErr, sysErr
}
+func createV31(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV31
tc.DeliveryServiceV31) (*tc.DeliveryServiceV31, int, error, error) {
+ tx := inf.Tx.Tx
+ dsNullable := tc.DeliveryServiceNullableV30(dsV31)
+ ds := dsNullable.UpgradeToV4()
+ res, status, userErr, sysErr := createV40(w, r, inf,
tc.DeliveryServiceV40(ds))
+ if res == nil {
Review comment:
Should we be checking `userErr != nil || sysErr != nil` instead?
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -173,43 +173,80 @@ type DeliveryServiceV11 struct {
XMLID string `json:"xmlId"`
}
+type DeliveryServiceV40 struct {
+ DeliveryServiceFieldsV31
+ DeliveryServiceFieldsV30
+ DeliveryServiceFieldsV15
+ DeliveryServiceFieldsV14
+ DeliveryServiceFieldsV13
+ DeliveryServiceNullableFieldsV11
+}
+
+type DeliveryServiceV31 struct {
+ DeliveryServiceV30
+ DeliveryServiceFieldsV31
+}
+
+// DeliveryServiceFieldsV31 contains additions to delivery services in api v3.1
+type DeliveryServiceFieldsV31 struct {
+ MaxRequestHeaderBytes *int `json:"maxRequestHeaderBytes"
db:"max_request_header_bytes"`
+}
+
type DeliveryServiceV30 struct {
DeliveryServiceNullableV15
+ DeliveryServiceFieldsV30
+}
+
+// DeliveryServiceFieldsV30 contains additions to delivery services in api v3.0
+type DeliveryServiceFieldsV30 struct {
Topology *string `json:"topology" db:"topology"`
FirstHeaderRewrite *string `json:"firstHeaderRewrite"
db:"first_header_rewrite"`
InnerHeaderRewrite *string `json:"innerHeaderRewrite"
db:"inner_header_rewrite"`
LastHeaderRewrite *string `json:"lastHeaderRewrite"
db:"last_header_rewrite"`
ServiceCategory *string `json:"serviceCategory"
db:"service_category"`
}
-type DeliveryServiceV31 struct {
- DeliveryServiceV30
- MaxRequestHeaderBytes *int `json:"maxRequestHeaderBytes"
db:"max_request_header_bytes"`
-}
-
// DeliveryServiceNullableV30 is the aliased structure that we should be using
for all api 3.x delivery structure operations
// This type should always alias the latest 3.x minor version struct. For ex,
if you wanted to create a DeliveryServiceV32 struct, you would do the following:
// type DeliveryServiceNullableV30 DeliveryServiceV32
// DeliveryServiceV32 = DeliveryServiceV31 + the new fields
type DeliveryServiceNullableV30 DeliveryServiceV31
+// DeliveryServiceNullableV4 is the aliased structure that we should be using
for all api 4.x delivery structure operations
+type DeliveryServiceNullableV4 DeliveryServiceV40
Review comment:
We can/should probably drop `Nullable` from the name for all new
structs/types (since structs should always be nullable going forward).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
