JSON Marshalling of User Booleans Not Working Correctly

[bflad]

I ran into an interesting bug/feature of json.Marshal that does not pass false values from the User struct to the Datadog API using this library because of omitempty behavior. Seems exactly related to this StackOverflow post: http://stackoverflow.com/questions/37756236/json-golang-boolean-omitempty

The User struct is currently defined as follows, which means that users cannot be re-enabled or have their admin rights revoked (which I have verified while discovering this issue):

type User struct {
        //...
	IsAdmin  bool   `json:"is_admin,omitempty"`
	Verified bool   `json:"verified,omitempty"`
	Disabled bool   `json:"disabled,omitempty"`
}

Here's example Go code to show the behavior using Boolean pointers versus not:

package main

import (
	"encoding/json"
	"fmt"
)

type SomeStruct struct {
	SomeValue bool `json:"some_value,omitempty"`
}

type SomeStructPointers struct {
	SomeValue *bool `json:"some_value,omitempty"`
}

func main() {
	tBool := true
	fBool := false

	bad1, _ := json.Marshal(SomeStruct{tBool})
	bad2, _ := json.Marshal(SomeStruct{fBool})

	fmt.Println(string(bad1))
	fmt.Println(string(bad2))

	tBoolPointer := new(bool)
	*tBoolPointer = true
	fBoolPointer := new(bool)
	*fBoolPointer = false

	good1, _ := json.Marshal(SomeStructPointers{tBoolPointer})
	good2, _ := json.Marshal(SomeStructPointers{fBoolPointer})

	fmt.Println(string(good1))
	fmt.Println(string(good2))
}

And its output:

# go run bool_pointers.go
{"some_value":true}
{}
{"some_value":true}
{"some_value":false}

I would imagine changing to Boolean pointers (*bool) would probably be a fairly breaking change. What do you think would be the best path towards resolution? I can submit a PR if you point me in the direction you would like to move forward.

[zorkian]

It would be a breaking change, but as of right now this is broken and so we do need to do something. In #56 we were having a discussion about using pointers for optional fields. In this case IsAdmin and Disabled are both optional in the protocol, but Verified isn't (but we can't write that field).

So, I think converting those two to bool pointers would be reasonable, but as you mention does break the API. Let's discuss over on #56 what we'll do and then how to proceed.

[ojongerius]

@bflad AFAIC this is solved in https://github.com/zorkian/go-datadog-api/tree/v2 . Could you confirm this?

Are you ok with switching to v2 or do you prefer to have this solved in master (soon to become tagged with v1) too? If you absolutely want it fixed in master feel free to raise a PR with that destination.

[bflad]

I can take a look tomorrow, but was thinking it would require the changes in v2 to work properly and didn't spend time on a workaround. Thanks for the hard work with the pointers!

[ojongerius]

👍

[bflad]

Thanks for the Terraform PR as well, that was exactly my real use case! 💯

[ojongerius]

🎉