DeepCachingType -- "NEVER" is now default

[dangogh]

When creating a new deliveryservice, the new DeepCachingType entry should have a reasonable default that's not invalid.

@rawlinp this is what I was suggesting...

[rob05c]

The default value of the enum should match the default value of the type, i.e. "". So per

case "":
+		// default when omitted
+		return DeepCachingTypeNever

below, DeepCachingTypeNever = DeepCachingType("NEVER") should be
DeepCachingTypeNever = DeepCachingType("")

This will prevent errors if someone default-initializes a DeepCachingType, e.g. t := DeepCachingType{} or t := DeepCachingType("") expecting it to be the default for the enum.

[dangogh]

good point.. I'll change that..

[asfgit]

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/976/
Test PASSed.

[rob05c]

Is this ever serialised to JSON, e.g. for the Client/Server? Might need
func (t DeepCachingType) MarshalJSON() ([]byte, error) { return json.Marshal(t.String()) } too.

That will make everything but string(t) work (which should never be done). There's no perfect solution, but this is probably the safest, and the only way to make DeliveryService{} default to Never.

[asfgit]

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/977/
Test PASSed.

[dangogh]

adding tests to verify JSON conversions

[dangogh]

@rob05c helpful suggestions.. thanks.. anything more?

[dangogh]

fyi -- would love to see this get into 2.2 so no one starts relying on current behavior

[rob05c]

Looks good! Sorry about the delay, I didn't think about JSON at first.

Again, there is no perfect solution, but IMO this is the best option, it gives us a correct default that serializes and stringifies correctly, and works if someone default-constructs either the enum or things containing it, DeepCachingType{} or DeliveryService{}.

A type DeepCachingType struct{s string}; const DeepCachingTypeAlways = DeepCachingType{s: "ALWAYS"} would be a perfect solution that prevented wrong string(t) calls, but Go doesn't allow const structs :(

As the RM, I'll make sure this gets in 2.2.

[rob05c]

Haven't tested running TO yet, but the code looks good.

[asfgit]

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/979/
Test PASSed.

[rawlinp]

looks good to me 👍

[dewrich]

@rob05c Gonna leave this one up to you to merge for 2.2