feat(uuid): Added support for NullUUID

[sejr]

Per the discussion in #54, this introduces a NullUUID type to the package that works like other nullable types in the database/sql package. Figured I'd just get the PR out while I'm playing around in this space.

[pborman]

Hi, thank you for the solution. This does look like it will resolve a problem, but I think it can be a standalone package rather than built into the base UUID package. What do you think?

[sejr]

I think that concern was brought up in the issue I mentioned. While this is a rather trivial addition, I think it is warranted in this base package.

It would obviously not be appropriate to add it to database/sql, where the other Null types are defined, because we would be adding a "non-standard" package dependency on google/uuid.

The other options are to (1) write your own package defining this extra type (which is what I have done), or (2) bring in a separate package, such as go.uuid. With the latter approach you wouldn't necessarily even need google/uuid, but knowing it is maintained by folks at Google gives me and others more confidence.

I think that's a decent enough case to warrant filling this small feature gap.

Additional anecdote. Honestly I think a battle-tested UUID implementation is worth adding to the standard library, but that's a separate discussion.

[pborman]

I think the name of the type would need to be changed. NullUUID sounds like it should be a constant that represents a Null UUID (which some might say is the zero UUID). I am not sure about a good name at the moment, but I do think we need a better name. If there can be a good descriptive name that is not too long then we can add it to the main package. Thanks (sorry for the long delay).

[mvrahden]

I think the name of the type would need to be changed. NullUUID sounds like it should be a constant that represents a Null UUID (which some might say is the zero UUID). I am not sure about a good name at the moment, but I do think we need a better name. If there can be a good descriptive name that is not too long then we can add it to the main package. Thanks (sorry for the long delay).

@pborman how about Nullable?

[sejr]

NullableUUID seems like a decent alternative, though that would make the UUID type separate from the other null types (NullString, NullInt64, etc).

Another option would be to keep it as NullUUID but also provide a ZeroUUID value that would more clearly represent that use case.

Edit: the zero UUID already exists as uuid.Nil. Not sure if/how we would want to change that.

[pborman]

The argument about NullString and NullInt64 are good ones when looking at "database/sql". I think adding a statement a statement that this is matching the basic types in "database/sql" would be good.

Also, as commented by @msal4, you should lose the type indent.

...
//     // NULL value
//  }
//
type NullUUID struct {
        UUID  UUID
        Valid bool // Valid is true if UUID is not NULL
}

Due to some recent changes I am hoping I will have more time going forward than I have had in the past for keeping up with this package.

[pborman]

Oh, would you add a the Marshal/Unmarshal{Text,Binary} methods on this type? This will be helpful for people using JSON.

[sejr]

@pborman based on your comment about usage with JSON, I assumed the Marshal functions would return successfully with a null literal rather than an error. LMK if you'd prefer otherwise.

[pborman]

So I tried the following program https://play.golang.org/p/Rmdq0ukjv73:

package main

import (
	"encoding/json"
	"fmt"

	"github.com/google/uuid"
)

type NullUUID struct {
	UUID uuid.UUID
	Valid  bool
}

var input = []byte(`"12345678-abcd-1234-abcd-0123456789ab"`)

func main() {
	var n NullUUID
	var u uuid.UUID
	err := json.Unmarshal(input, &n)
	fmt.Printf("Unmarshal NullUUID: %+v %v\n", n, err);
	err = json.Unmarshal(input, &u)
	fmt.Printf("Unmarshal UUID: %+v %v\n", u, err);

	data, err := json.Marshal(&n)
	fmt.Printf("Marshal Empty NullUUID %s %v\n", data, err)
	n.Valid = true
	n.UUID = u
	data, err = json.Marshal(&n)
	fmt.Printf("Marshal Filled NullUUID %s %v\n", data, err)

	data, err = json.Marshal(&u)
	fmt.Printf("Marshal UUID: %s %v\n", data, err)
}

And got the following output:

Unmarshal NullUUID: {UUID:00000000-0000-0000-0000-000000000000 Valid:false} json: cannot unmarshal string into Go value of type main.NullUUID
Unmarshal UUID: 12345678-abcd-1234-abcd-0123456789ab <nil>
Marshal Empty NullUUID {"UUID":"00000000-0000-0000-0000-000000000000","Valid":false} <nil>
Marshal Filled NullUUID {"UUID":"12345678-abcd-1234-abcd-0123456789ab","Valid":true} <nil>
Marshal UUID: "12345678-abcd-1234-abcd-0123456789ab" <nil>

When Marshalling I would expect:

Marshal Empty NullUUID "" <nil>
    or
Marshal Empty NullUUID null <nil>

Marshal Filled NullUUID "12345678-abcd-1234-abcd-0123456789ab" <nil>

[sejr]

@pborman that same code run against the version I just pushed yields:

Unmarshal NullUUID: {UUID:12345678-abcd-1234-abcd-0123456789ab Valid:true} <nil>
Unmarshal UUID: 12345678-abcd-1234-abcd-0123456789ab <nil>
Marshal Empty NullUUID null <nil>
Marshal Filled NullUUID "12345678-abcd-1234-abcd-0123456789ab" <nil>
Marshal UUID: "12345678-abcd-1234-abcd-0123456789ab" <nil>

[mvrahden]

What use is this Valid field for?
As far as I see, it's kind of redundant information for an error case, right?
In that case it should yield the Zero value (00000000-0000-0000-0000-000000000000) anyway and we are good to go.

[msal4]

@mvrahden this is how nullable types are implemented in the std lib so it's important as to not confuse users familiar with this pattern. also, it makes sense since you don't want to check if the value is equal to "00000000-0000-0000-0000-000000000000" every time you want to check for nullability

[mvrahden]

@msal4 I see. In that case, it makes sense to stay consistent. Thanks for the clarification 👍

[pborman]

Over all this looks good. I will merge it and then clean up a few things and then cut a new release with the most recent changes. Thank you.