Skip to content

Commit

Permalink
chore(allsrv): inject id generation fn
Browse files Browse the repository at this point in the history
Motivation for this change:

  1. we want determinism in tests, this allows for that
  2. we don't want the db owning the business logic of what an ID looks like
    a) if we switch out dbs, each db has to make sure its ID gen aligns... youch

Our tests are now fairly easy to follow. At this time, our `Server` is really
the owner of the id gen business logic. This is ok for now, but for more
complex scenarios, this can pose a serious problem.
  • Loading branch information
jsteenb2 committed Jul 5, 2024
1 parent e9374a0 commit 1a50c7e
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 20 deletions.
40 changes: 25 additions & 15 deletions allsrv/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import (
a) there is nothing actionable, so how does the consumer know to handle the error?
b) if the APIs evolve, how does the consumer distinguish between old and new?
10) Observability....
11) hard coding UUID generation into db
11) hard coding UUID generation into db
12) possible race conditions in inmem store
Praises:
Expand All @@ -48,14 +48,31 @@ type Server struct {
db *InmemDB // 1)

user, pass string // 3)

idFn func() string // 11)
}

// WithIDFn sets the id generation fn for the server.
func WithIDFn(fn func() string) func(*Server) {
return func(s *Server) {
s.idFn = fn
}
}

func NewServer(db *InmemDB, user, pass string) *Server {
func NewServer(db *InmemDB, user, pass string, opts ...func(*Server)) *Server {
s := Server{
db: db,
user: user,
pass: pass,
idFn: func() string {
// defaults to using a uuid
return uuid.Must(uuid.NewV4()).String()
},
}
for _, o := range opts {
o(&s)
}

s.routes()
return &s
}
Expand Down Expand Up @@ -93,15 +110,10 @@ func (s *Server) createFoo(w http.ResponseWriter, r *http.Request) {
return
}

newFooID, err := s.db.createFoo(f)
if err != nil {
w.WriteHeader(http.StatusInternalServerError) // 9)
return
}
f.ID = s.idFn() // 11)

f, err = s.db.readFoo(newFooID)
if err != nil {
w.WriteHeader(http.StatusNotFound) // 9)
if err := s.db.createFoo(f); err != nil {
w.WriteHeader(http.StatusInternalServerError) // 9)
return
}

Expand Down Expand Up @@ -166,18 +178,16 @@ type InmemDB struct {
m []Foo // 12)
}

func (db *InmemDB) createFoo(f Foo) (string, error) {
f.ID = uuid.Must(uuid.NewV4()).String() // 11)

func (db *InmemDB) createFoo(f Foo) error {
for _, existing := range db.m {
if f.Name == existing.Name {
return "", errors.New("foo " + f.Name + " exists") // 8)
return errors.New("foo " + f.Name + " exists") // 8)
}
}

db.m = append(db.m, f)

return f.ID, nil
return nil
}

func (db *InmemDB) readFoo(id string) (Foo, error) {
Expand Down
9 changes: 4 additions & 5 deletions allsrv/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ func TestServer(t *testing.T) {
t.Run("foo create", func(t *testing.T) {
t.Run("when provided a valid foo should pass", func(t *testing.T) {
db := new(allsrv.InmemDB)
svr := allsrv.NewServer(db, "dodgers@stink.com", "PaSsWoRd")
svr := allsrv.NewServer(db, "dodgers@stink.com", "PaSsWoRd", allsrv.WithIDFn(func() string {
return "id1"
}))

req := httptest.NewRequest("POST", "/foo", newJSONBody(t, allsrv.Foo{
Name: "first-foo",
Expand All @@ -31,11 +33,8 @@ func TestServer(t *testing.T) {

assert.Equal(t, http.StatusCreated, rec.Code)
expectJSONBody(t, rec.Body, func(t *testing.T, got allsrv.Foo) {
assert.NotZero(t, got.ID)
got.ID = "" // this hurts :-(

want := allsrv.Foo{
ID: "", // ruh ohhh
ID: "id1",
Name: "first-foo",
Note: "some note",
}
Expand Down

0 comments on commit 1a50c7e

Please sign in to comment.