Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow concurrent, re-creatable usage #44

Merged
merged 6 commits into from Apr 16, 2019
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
54 changes: 54 additions & 0 deletions uuid_source.go
@@ -0,0 +1,54 @@
package uuid

import (
"io"
"crypto/rand"
)

// A UuidSource holds a random number generator and generates UUIDs using it as its source.
//
// It is useful when a process need its own random number generator,
// e.g. when running some processes concurrently.
type UuidSource struct {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be UUIDSource, but if you are mimicking math/rand then just call it Source.

One issue with this setup it you still can only have a single rander function. Rather than add all of this complexity, it might be easier to simply create one new function, say NewRandomFromReader (I don't really like that name, it is a bit of a mouthful, but I don't expect it to be used often at all).

func NewRandomFromReadr(r io.Reader) (UUID, error)

Now an external package can create this type if they want.

type Source struct {
        r io.Reader
}

func NewUUIDSource(r io.Reader) Source {
        return Source{r: r}
}

func (s Source) NewRandom() (uuid.UUID, error) {
        return uuid.NewRandomFromReader(s.r)
}

That will keep the uuid package cleaner.

Copy link
Contributor Author

@trabetti trabetti Mar 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pborman , Thanks for looking into this PR!
Let me ask to clarify - are you suggesting here that instead of everything in the uuid_source file, we just add the func NewRandomFromReadr(r io.Reader) (UUID, error) function to version4.go and let the user build the structs around it if they need? (it is actually the func newRandom(r io.Reader) (UUID, error) that I added there). I am not clear, as you are also commenting on the content of the uuid_source files.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is exactly correct. And yes, the NewRandomFromReader is essentially the newRandom function you added. The uuid_source files would no longer be needed, although a simple test showing that NewRandomFromReader did in fact use the supplied reader would probably be a good addition. This could be very simple where you have a bytes.NewReader that has 16 bytes of data and the first NewRandomFromReader gives you the UUID you expect and the second NewRandomFromReader returns an error.

You might want to consider putting a package in GitHub that has the functionality of your uuid_source file so others can also use it if they need that sort of functionality.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, Got it.

You might want to consider putting a package in GitHub that has the functionality of your uuid_source file so others can also use it if they need that sort of functionality.

A package for using it seems too much for just few lines of code. Maybe I'll just add an example?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rander io.Reader
}

// Creates a new UuidSource which holds its own random number generator.
//
// Calling NewSource with nil sets the random number generator to a default
// generator.
func NewSource(r io.Reader) UuidSource {
var uuidSource UuidSource
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return Source{render: r}

No reason to special case nil. They can pass in rand.Reader if they want.

uuidSource.SetRand(r)
return uuidSource

}

// SetRand sets the random number generator of the UuidSource to r, which implements io.Reader.
// If r.Read returns an error when the package requests random data then
// a panic will be issued.
//
// Calling SetRand with nil sets the random number generator to a default
// generator.
func (uuidSource *UuidSource) SetRand(r io.Reader) {
if r == nil {
uuidSource.rander = rand.Reader
return
}
uuidSource.rander = r
}

// NewRandom returns a Random (Version 4) UUID based on the random number generator in the UuidSource.
//
// See more detailed explanation here: https://godoc.org/github.com/google/uuid#NewRandom
func (uuidSource UuidSource) NewRandom() (UUID, error) {
return newRandom(uuidSource.rander)
}

// New creates a new random UUID based on the random number generator in the UuidSource or panics. New is equivalent to
// the expression
//
// uuid.Must(uuid.NewRandom())
func (uuidSource UuidSource) New() UUID {
return Must(uuidSource.NewRandom())
}
62 changes: 62 additions & 0 deletions uuid_source_test.go
@@ -0,0 +1,62 @@
package uuid

import (
"testing"
"strings"

regen "github.com/zach-klippenstein/goregen"
)

func TestUuidSources(t *testing.T) {

myString, _ := regen.Generate("[a-zA-Z]{1000}")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than depend on another external package (creating another dependency) why not just make a very simple reader that returns a deterministic set based on some seed. For example:

type reader struct {
	n int
	c int
}

func (r *reader) Read(buf []byte) (int, error) {
	// Make an uninitialized reader just return sequential bytes.
	 if r.n == 0 {
		r.n = 1
	}
	c, n := r.c, r.n
	for i := range buf {
		c = (c + n) * n
		buf[i] = byte(c)
	}
	r.c = c
	return len(buf), nil
}


// Two identical sources, should give same sequence
uuidSourceA := NewSource(strings.NewReader(myString))
uuidSourceB := NewSource(strings.NewReader(myString))

for i := 0; i < 10; i++ {
uuid1 := uuidSourceA.New()
uuid2 := uuidSourceB.New()
if uuid1 != uuid2 {
t.Errorf("expected duplicates, got %q and %q", uuid1, uuid2)
}
}

// Set rander with nil, each source will be random
uuidSourceA.SetRand(nil)
uuidSourceB.SetRand(nil)

for i := 0; i < 10; i++ {
uuid1 := uuidSourceA.New()
uuid2 := uuidSourceB.New()
if uuid1 == uuid2 {
t.Errorf("unexpected duplicates, got %q", uuid1)
}
}

// Set rander to rand source with same seed, should give same sequence
uuidSourceA.SetRand(strings.NewReader(myString))
uuidSourceB.SetRand(strings.NewReader(myString))

for i := 0; i < 10; i++ {
uuid1 := uuidSourceA.New()
uuid2 := uuidSourceB.New()
if uuid1 != uuid2 {
t.Errorf("expected duplicates, got %q and %q", uuid1, uuid2)
}
}

// Set rander to rand source with different seeds, should not give same sequence
uuidSourceA.SetRand(strings.NewReader("456" + myString))
uuidSourceB.SetRand(strings.NewReader("myString" + myString))

for i := 0; i < 10; i++ {
uuid1 := uuidSourceA.New()
uuid2 := uuidSourceB.New()
if uuid1 == uuid2 {
t.Errorf("unexpected duplicates, got %q", uuid1)
}
}

}
21 changes: 21 additions & 0 deletions uuid_test.go
Expand Up @@ -479,6 +479,27 @@ func TestBadRand(t *testing.T) {
}
}

func TestSetRand(t *testing.T) {
myString := "805-9dd6-1a877cb526c678e71d38-7122-44c0-9b7c-04e7001cc78783ac3e82-47a3-4cc3-9951-13f3339d88088f5d685a-11f7-4078-ada9-de44ad2daeb7"

SetRand(strings.NewReader(myString))
uuid1 := New()
uuid2 := New()

SetRand(strings.NewReader(myString))
uuid3 := New()
uuid4 := New()

if uuid1 != uuid3 {
t.Errorf("expected duplicates, got %q and %q", uuid1, uuid3)
}
if uuid2 != uuid4 {
t.Errorf("expected duplicates, got %q and %q", uuid2, uuid4)
}


}

var asString = "f47ac10b-58cc-0372-8567-0e02b2c3d479"
var asBytes = []byte(asString)

Expand Down
7 changes: 6 additions & 1 deletion version4.go
Expand Up @@ -27,12 +27,17 @@ func New() UUID {
// equivalent to the odds of creating a few tens of trillions of UUIDs in a
// year and having one duplicate.
func NewRandom() (UUID, error) {
return newRandom(rander)
}

func newRandom(r io.Reader) (UUID, error) {
var uuid UUID
_, err := io.ReadFull(rander, uuid[:])
_, err := io.ReadFull(r, uuid[:])
if err != nil {
return Nil, err
}
uuid[6] = (uuid[6] & 0x0f) | 0x40 // Version 4
uuid[8] = (uuid[8] & 0x3f) | 0x80 // Variant is 10
return uuid, nil
}