Skip to content

Commit

Permalink
chore(allsrv): add high-level implementation notes
Browse files Browse the repository at this point in the history
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.

<details><summary>Spoiler</summary>

<h4>Praises</h4>

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

<h4>Concerns</h4>

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

</details>

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.
  • Loading branch information
jsteenb2 committed Jul 5, 2024
1 parent 643f58a commit 733884c
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 22 deletions.
84 changes: 63 additions & 21 deletions allsrv/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -26,109 +61,116 @@ 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))
http.Handle("DELETE /foo", http.HandlerFunc(s.delFoo))
}

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)
}
}

Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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)
}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module mess
module github.com/jsteenb2/mess

go 1.22

Expand Down

0 comments on commit 733884c

Please sign in to comment.