[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #6441: Geo limit countries can now be parsed as an array and a string

[GitBox <gi...@apache.org>]

ocket8888 commented on a change in pull request #6441:
URL: https://github.com/apache/trafficcontrol/pull/6441#discussion_r779039404



##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -643,7 +643,7 @@ type DeliveryServiceNullableFieldsV11 struct {
 	// countries within which the Delivery Service's content ought to be made
 	// available. This has no effect if GeoLimit is not a pointer to exactly the
 	// value 2.
-	GeoLimitCountries *string `json:"geoLimitCountries" db:"geo_limit_countries"`
+	GeoLimitCountries *interface{} `json:"geoLimitCountries" db:"geo_limit_countries"`

Review comment:
       When `geoLimitCountries` is passed into the API as a string, it's still really an array, just one that's represented as a string. Because of that, I think if we're going to make a type change here it makes the most sense to use a string slice. To get it to parse independent of input type, there are a few ways but what I'd recommend is a type alias for `[]string` that implements [`encoding/json.Unmarshaler`](https://pkg.go.dev/encoding/json#Unmarshaler) to handle the two situations and parse country code array strings out into actual arrays immediately, which is currently being enforced as validation (and so in that case could/would need to be removed from the validation). 
   
   Also, the types of fields in legacy API version structures cannot be changed. While the change is non-breaking for the API, it is breaking the call signature of legacy clients using our Go client library. If anyone is using the `GeoLimitCountries` field of that struct - or any that nest it - then their programs will no longer compile after this change: https://go.dev/play/p/Dk7Eu8XbKt4




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