From 733884c017ce35aca773df96c63cebc4cde316cb Mon Sep 17 00:00:00 2001 From: Johnny Steenbergen Date: Sat, 13 Jan 2024 16:34:01 -0600 Subject: [PATCH] chore(allsrv): add high-level implementation notes These are some of my thoughts around this starter project. In this workshop, we'll look at a couple different examples of codebases. This `allsrv` pkg represents one extreme: the wildly understructured variety. Later we'll explore the other extreme, the intensely overstructured kind.
Spoiler

Praises

1. minimal public API 2. simple to read 3. minimal indirection/obvious code 4. is trivial in scope

Concerns

1. the server depends on a hard type, coupling to the exact inmem db * what happens if we want a different db? 2. auth is copy-pasted in each handler * what happens if we forget that copy pasta? 3. auth is hardcoded to basic auth * what happens if we want to adapt some other means of auth? 4. router being used is the GLOBAL http.DefaultServeMux * should avoid globals * what happens if you have multiple servers in this go module who reference default serve mux? 5. no tests * how do we ensure things work? * how do we know what is intended by the current implementation? 6. http/db are coupled to the same type * what happens when the concerns diverge? aka http wants a shape the db does not? (note: it happens A LOT) 7. Server only works with HTTP * what happens when we want to support grpc? thrift? other protocol? * this setup often leads to copy pasta/weak abstractions that tend to leak 8. Errors are opaque and limited 9. API is very bare bones * there is nothing actionable, so how does the consumer know to handle the error? * if the APIs evolve, how does the consumer distinguish between old and new? 10. Observability.... 11. hard coding UUID generation into db 12. possible race conditions in inmem store
I want to make sure you don't get the wrong impression. We're not here to learn how to redesign *all the things*. Rather, we're here to increase the number of tools in our toolbelt to deal with a legacy system. Replacing a legacy system is only one of many possible courses of action. In the event the legacy system is unsalvageable, a rewrite may be the only course of action. The key is understanding the problem space in its entirety. Our key metric in navigating this mess, is understanding how much non-value added work is being added because of the limitations of the legacy system. Often times there is a wishlist of asks from product managers and customers alike that are *"impossible"* because of some warts on the legacy design. As we work through this repo, try to envision different scenarios you've found yourself in. We'll cover a broad set of topics and pair this with the means to remedy the situation. Here's what we're going to cover: 1. Understanding the existing system * What do our tests tell us? * What gaps do we have to fill in our tests to inform us of different aspects of the legacy design? * What's the on call experience like? * What's the onboarding experience like? * How large are typical PRs? * How comfortable do engineers feel contributing changes? * How much of the team contributes changes? 2. Understand the external constraints imposed on the system that are out of your control * Verify the constraints are truly required * Most common bad assumption I see is automagik replication... we say this like its a must, but rarely do we understand the full picture. Its often unnecessary and undesirable from a user perspective. * The key is understanding the way in which the parts of the system interact 3. Understanding the cost of *abstraction* * Clean, Hexagonal, Domain Driven Design, etc... these all provide value, but are they worth the cost? * *DRY* up that code? * Microservice all the things `:table_flip:` * API should mimic the entire database `:double_table_flip:` * Follow what Johnny shared, because workshops dont' lie... dogma wins you nothing 4. Understand the customer/user's experience 5. Codify tribal knowledge * CLIs are your friend. They can be used to codify a ton of tribal knowledge. Done right they can be extremely user friendly as well. Shell completions ftw! 6. Much more `:yaaaaaaas:` My goal is each person taking this workshop will walk away with a few nuggets of wisdom that will improve their day to day. --- allsrv/server.go | 84 ++++++++++++++++++++++++++++++++++++------------ go.mod | 2 +- 2 files changed, 64 insertions(+), 22 deletions(-) diff --git a/allsrv/server.go b/allsrv/server.go index 11f7a9c..d889d2b 100644 --- a/allsrv/server.go +++ b/allsrv/server.go @@ -9,10 +9,45 @@ import ( "github.com/gofrs/uuid" ) +/* + + Concerns: + 1) the server depends on a hard type, coupling to the exact inmem db + a) what happens if we want a different db? + 2) auth is copy-pasted in each handler + a) what happens if we forget that copy pasta? + 3) auth is hardcoded to basic auth + a) what happens if we want to adapt some other means of auth? + 4) router being used is the GLOBAL http.DefaultServeMux + a) should avoid globals + b) what happens if you have multiple servers in this go module who reference default serve mux? + 5) no tests + a) how do we ensure things work? + b) how do we know what is intended by the current implementation? + 6) http/db are coupled to the same type + a) what happens when the concerns diverge? aka http wants a shape the db does not? (note: it happens A LOT) + 7) Server only works with HTTP + a) what happens when we want to support grpc? thrift? other protocol? + b) this setup often leads to copy pasta/weak abstractions that tend to leak + 8) Errors are opaque and limited + 9) API is very bare bones + 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 + 12) possible race conditions in inmem store + + Praises: + 1) minimal public API + 2) simple to read + 3) minimal indirection/obvious code + 4) is trivial in scope +*/ + type Server struct { - db *inmemDB + db *inmemDB // 1) - user, pass string + user, pass string // 3) } func NewServer(db *inmemDB, user, pass string) *Server { @@ -26,6 +61,7 @@ func NewServer(db *inmemDB, user, pass string) *Server { } func (s *Server) routes() { + // 4) 7) 9) 10) http.Handle("POST /foo", http.HandlerFunc(s.createFoo)) http.Handle("GET /foo", http.HandlerFunc(s.readFoo)) http.Handle("PUT /foo", http.HandlerFunc(s.updateFoo)) @@ -33,102 +69,108 @@ func (s *Server) routes() { } func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { + // 4) http.DefaultServeMux.ServeHTTP(w, r) } type Foo struct { + // 6) ID string `json:"id" gorm:"id"` Name string `json:"name" gorm:"name"` Note string `json:"note" gorm:"note"` } func (s *Server) createFoo(w http.ResponseWriter, r *http.Request) { + // 2) if user, pass, ok := r.BasicAuth(); !(ok && user == s.user && pass == s.pass) { - w.WriteHeader(http.StatusUnauthorized) + w.WriteHeader(http.StatusUnauthorized) // 9) return } var f Foo if err := json.NewDecoder(r.Body).Decode(&f); err != nil { - w.WriteHeader(http.StatusForbidden) + w.WriteHeader(http.StatusForbidden) // 9) return } newFooID, err := s.db.createFoo(f) if err != nil { - w.WriteHeader(http.StatusInternalServerError) + w.WriteHeader(http.StatusInternalServerError) // 9) return } f, err = s.db.readFoo(newFooID) if err != nil { - w.WriteHeader(http.StatusNotFound) + w.WriteHeader(http.StatusNotFound) // 9) return } w.WriteHeader(http.StatusCreated) if err := json.NewEncoder(w).Encode(f); err != nil { - log.Printf("unexpected error writing json value to response body: " + err.Error()) + log.Printf("unexpected error writing json value to response body: " + err.Error()) // 8) 10) } } func (s *Server) readFoo(w http.ResponseWriter, r *http.Request) { + // 2) if user, pass, ok := r.BasicAuth(); !(ok && user == s.user && pass == s.pass) { - w.WriteHeader(http.StatusUnauthorized) + w.WriteHeader(http.StatusUnauthorized) // 9) return } f, err := s.db.readFoo(r.URL.Query().Get("id")) if err != nil { - w.WriteHeader(http.StatusNotFound) + w.WriteHeader(http.StatusNotFound) // 9) return } if err := json.NewEncoder(w).Encode(f); err != nil { - log.Printf("unexpected error writing json value to response body: " + err.Error()) + log.Printf("unexpected error writing json value to response body: " + err.Error()) // 8) 10) } } func (s *Server) updateFoo(w http.ResponseWriter, r *http.Request) { + // 2) if user, pass, ok := r.BasicAuth(); !(ok && user == s.user && pass == s.pass) { - w.WriteHeader(http.StatusUnauthorized) + w.WriteHeader(http.StatusUnauthorized) // 9) return } var f Foo if err := json.NewDecoder(r.Body).Decode(&f); err != nil { - w.WriteHeader(http.StatusForbidden) + w.WriteHeader(http.StatusForbidden) // 9) return } if err := s.db.updateFoo(f); err != nil { - w.WriteHeader(http.StatusInternalServerError) + w.WriteHeader(http.StatusInternalServerError) // 9) return } } func (s *Server) delFoo(w http.ResponseWriter, r *http.Request) { + // 2) if user, pass, ok := r.BasicAuth(); !(ok && user == s.user && pass == s.pass) { - w.WriteHeader(http.StatusUnauthorized) + w.WriteHeader(http.StatusUnauthorized) // 9) return } if err := s.db.delFoo(r.URL.Query().Get("id")); err != nil { - w.WriteHeader(http.StatusNotFound) + w.WriteHeader(http.StatusNotFound) // 9) return } } type inmemDB struct { - m []Foo + m []Foo // 12) } func (db *inmemDB) createFoo(f Foo) (string, error) { - f.ID = uuid.Must(uuid.NewV4()).String() + f.ID = uuid.Must(uuid.NewV4()).String() // 11) for _, existing := range db.m { if f.Name == existing.Name { - return "", errors.New("foo " + f.Name + " exists") + return "", errors.New("foo " + f.Name + " exists") // 8) } } @@ -143,7 +185,7 @@ func (db *inmemDB) readFoo(id string) (Foo, error) { return f, nil } } - return Foo{}, errors.New("foo not found for id: " + id) + return Foo{}, errors.New("foo not found for id: " + id) // 8) } func (db *inmemDB) updateFoo(f Foo) error { @@ -153,7 +195,7 @@ func (db *inmemDB) updateFoo(f Foo) error { return nil } } - return errors.New("foo not found for id: " + f.ID) + return errors.New("foo not found for id: " + f.ID) // 8) } func (db *inmemDB) delFoo(id string) error { @@ -162,5 +204,5 @@ func (db *inmemDB) delFoo(id string) error { db.m = append(db.m[:i], db.m[i+1:]...) } } - return errors.New("foo not found for id: " + id) + return errors.New("foo not found for id: " + id) // 8) } diff --git a/go.mod b/go.mod index 7c472b1..bd4bf8f 100644 --- a/go.mod +++ b/go.mod @@ -1,4 +1,4 @@ -module mess +module github.com/jsteenb2/mess go 1.22