refactor: routes as pointer receivers for server (#38)

jdockerty · web-flow · commit 5e93b2eb5388 · 2024-06-02T18:38:39.000+01:00
* chore: remove routes package

* refactor: add routes as receiver funcs from server

Having these routes hang off from the `State` didn't really make sense.
They're for the HTTP server so should be one-level up, they still have
access to the `State` because this is part of the `PlaygroundServer`
already. Doing this removes the need of passing the `State` into every
function, as it can be accessed internally.

* fix: kubecfg version hack script

* chore: use logger for frontend

This was missed during the initial slog pass, tacking it on here for ease

* chore: remove log package
diff --git a/cmd/server/cmd.go b/cmd/server/cmd.go
@@ -57,7 +57,7 @@ func main() {
 	state := state.NewWithLogger(bindAddress, shareAddress, logger)
 	playground := server.New(state)
 
-	slog.Info("Listening on", "address", bindAddress)
+	logger.Info("Listening on", "address", bindAddress)
 	go func() {
 		if err := playground.Serve(); err != http.ErrServerClosed {
 			slog.Error("Unexpected shutdown", slog.Any("error", err))
@@ -66,14 +66,14 @@ func main() {
 
 	<-ctx.Done()
 	stop()
-	slog.Info("Shutting down, use Ctrl+C again to force")
+	logger.Info("Shutting down, use Ctrl+C again to force")
 
 	// Inform the server that it had 5 seconds to handle connections and shutdown
 	timeoutCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()
 	if err := playground.Server.Shutdown(timeoutCtx); err != nil {
-		slog.Error("Server forced shutdown", slog.Any("error", err))
+		logger.Error("Server forced shutdown", slog.Any("error", err))
 	}
 
-	slog.Info("Server shutdown")
+	logger.Info("Server shutdown")
 }
diff --git a/internal/server/backend_routes.go b/internal/server/backend_routes.go
@@ -1,4 +1,4 @@
-package routes
+package server
 
 import (
 	"encoding/hex"
@@ -7,7 +7,6 @@ import (
 	"regexp"
 
 	"github.com/google/go-jsonnet"
-	"github.com/jdockerty/jsonnet-playground/internal/server/state"
 )
 
 var (
@@ -23,33 +22,33 @@ var (
 )
 
 // Health indicates whether the server is running.
-func Health(state *state.State) http.HandlerFunc {
+func (srv *PlaygroundServer) Health() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
-		state.Logger.Debug("health")
+		srv.State.Logger.Debug("health")
 		_, _ = w.Write([]byte("OK"))
 	}
 }
 
 // HandleRun receives Jsonnet input via text and evaluates it.
-func HandleRun(state *state.State) http.HandlerFunc {
+func (srv *PlaygroundServer) HandleRun() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		if r.Method != http.MethodPost {
-			state.Logger.Error("incorrect method to run handler", "method", r.Method)
+			srv.State.Logger.Error("incorrect method to run handler", "method", r.Method)
 			http.Error(w, "must be POST", http.StatusBadRequest)
 			return
 		}
 
 		incomingJsonnet := r.FormValue("jsonnet-input")
-		state.Logger.Info("run triggered", "jsonnet", incomingJsonnet)
-		evaluated, err := state.EvaluateSnippet(incomingJsonnet)
+		srv.State.Logger.Info("run triggered", "jsonnet", incomingJsonnet)
+		evaluated, err := srv.State.EvaluateSnippet(incomingJsonnet)
 		if err != nil {
-			state.Logger.Error("invalid snippet", "jsonnet", incomingJsonnet)
+			srv.State.Logger.Error("invalid snippet", "jsonnet", incomingJsonnet)
 			// TODO: display an error for the bad req rather than using a 200
 			_, _ = w.Write([]byte(err.Error()))
 			return
 		}
 
-		state.Logger.Info("evaluated", "jsonnet", evaluated)
+		srv.State.Logger.Info("evaluated", "jsonnet", evaluated)
 		_, _ = w.Write([]byte(evaluated))
 	}
 }
@@ -59,89 +58,89 @@ func HandleRun(state *state.State) http.HandlerFunc {
 // store - this storage mechanism is ephemeral.
 //
 // At a later date, this will include a persistence layer.
-func HandleCreateShare(state *state.State) http.HandlerFunc {
+func (srv *PlaygroundServer) HandleCreateShare() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		if r.Method != http.MethodPost {
-			state.Logger.Error("incorrect method to create share handler", "method", r.Method)
+			srv.State.Logger.Error("incorrect method to create share handler", "method", r.Method)
 			http.Error(w, "must be POST", http.StatusBadRequest)
 			return
 		}
 
 		incomingJsonnet := r.FormValue("jsonnet-input")
-		_, err := state.EvaluateSnippet(incomingJsonnet)
+		_, err := srv.State.EvaluateSnippet(incomingJsonnet)
 		if err != nil {
-			state.Logger.Error("invalid share", "jsonnet", incomingJsonnet)
+			srv.State.Logger.Error("invalid share", "jsonnet", incomingJsonnet)
 			// TODO: display an error for the bad req rather than using a 200
 			_, _ = w.Write([]byte("Share is not available for invalid Jsonnet. Run your snippet to see the result."))
 			return
 		}
 
-		snippetHash := hex.EncodeToString(state.Hasher.Sum([]byte(incomingJsonnet)))[:15]
-		if _, ok := state.Store[snippetHash]; !ok {
-			state.Logger.Info("store creation", "hash", snippetHash)
-			state.Store[snippetHash] = incomingJsonnet
+		snippetHash := hex.EncodeToString(srv.State.Hasher.Sum([]byte(incomingJsonnet)))[:15]
+		if _, ok := srv.State.Store[snippetHash]; !ok {
+			srv.State.Logger.Info("store creation", "hash", snippetHash)
+			srv.State.Store[snippetHash] = incomingJsonnet
 		} else {
-			state.Logger.Info("store update", "hash", snippetHash)
-			state.Store[snippetHash] = incomingJsonnet
+			srv.State.Logger.Info("store update", "hash", snippetHash)
+			srv.State.Store[snippetHash] = incomingJsonnet
 		}
-		shareMsg := fmt.Sprintf("%s/share/%s", state.Config.ShareDomain, snippetHash)
-		state.Logger.Debug("created share link", "link", shareMsg)
+		shareMsg := fmt.Sprintf("%s/share/%s", srv.State.Config.ShareDomain, snippetHash)
+		srv.State.Logger.Debug("created share link", "link", shareMsg)
 		_, _ = w.Write([]byte("Link: " + shareMsg))
 	}
 }
 
 // HandleGetShare attempts to retrieve a shared snippet hash from the internal
 // store. If this does not exist, an error is displayed.
-func HandleGetShare(state *state.State) http.HandlerFunc {
+func (srv *PlaygroundServer) HandleGetShare() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		if r.Method != http.MethodGet {
-			state.Logger.Error("incorrect method to get share handler", "method", r.Method)
+			srv.State.Logger.Error("incorrect method to get share handler", "method", r.Method)
 			http.Error(w, "must be GET", http.StatusBadRequest)
 			return
 		}
 		shareHash := r.PathValue("shareHash")
-		state.Logger.Info("attempting to load shared snippet", "hash", shareHash)
+		srv.State.Logger.Info("attempting to load shared snippet", "hash", shareHash)
 
-		snippet, ok := state.Store[shareHash]
+		snippet, ok := srv.State.Store[shareHash]
 		if !ok {
-			state.Logger.Warn("no share snippet exists", "hash", shareHash)
+			srv.State.Logger.Warn("no share snippet exists", "hash", shareHash)
 			errMsg := fmt.Errorf("No share snippet exists for %s, it might have expired.\n", shareHash)
 			_, _ = w.Write([]byte(errMsg.Error()))
 			return
 		}
-		state.Logger.Info("loaded shared snippet", "hash", shareHash)
+		srv.State.Logger.Info("loaded shared snippet", "hash", shareHash)
 		_, _ = w.Write([]byte(snippet))
 	}
 }
 
 // Format the input Jsonnet according to the standard jsonnetfmt rules.
-func HandleFormat(state *state.State) http.HandlerFunc {
+func (srv *PlaygroundServer) HandleFormat() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		if r.Method != http.MethodPost {
-			state.Logger.Error("incorrect method to format handler", "method", r.Method)
+			srv.State.Logger.Error("incorrect method to format handler", "method", r.Method)
 			http.Error(w, "must be POST", http.StatusBadRequest)
 			return
 		}
 
 		incomingJsonnet := r.FormValue("jsonnet-input")
-		state.Logger.Info("attempting to format", "jsonnet", incomingJsonnet)
-		formattedJsonnet, err := state.FormatSnippet(incomingJsonnet)
+		srv.State.Logger.Info("attempting to format", "jsonnet", incomingJsonnet)
+		formattedJsonnet, err := srv.State.FormatSnippet(incomingJsonnet)
 		if err != nil {
-			state.Logger.Warn("cannot format invalid jsonnet")
+			srv.State.Logger.Warn("cannot format invalid jsonnet")
 			http.Error(w, "Format is not available for invalid Jsonnet. Run your snippet to see the result.", http.StatusBadRequest)
 			return
 		}
-		state.Logger.Info("formatted", "jsonnet", formattedJsonnet)
+		srv.State.Logger.Info("formatted", "jsonnet", formattedJsonnet)
 		_, _ = w.Write([]byte(formattedJsonnet))
 	}
 }
 
 // Retrieve the current version of Jsonnet/Kubecfg in use for the running application.
 // This is purely informational for the frontend.
-func HandleVersions(state *state.State) http.HandlerFunc {
+func (srv *PlaygroundServer) HandleVersions() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		if r.Method != http.MethodGet {
-			state.Logger.Error("incorrect method to versions handler", "method", r.Method)
+			srv.State.Logger.Error("incorrect method to versions handler", "method", r.Method)
 			http.Error(w, "must be POST", http.StatusBadRequest)
 			return
 		}
@@ -152,17 +151,17 @@ func HandleVersions(state *state.State) http.HandlerFunc {
 // Middleware to stop Jsonnet snippets which contain file:///, typically paired
 // with an import, being used and becoming shareable. These are rejected before
 // running through the Jsonnet VM and a generic error is displayed.
-func DisableFileImports(state *state.State, next http.Handler) http.HandlerFunc {
+func DisableFileImports(srv *PlaygroundServer, next http.Handler) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		err := r.ParseForm()
 		if err != nil {
-			state.Logger.Error("unable to parse form")
+			srv.State.Logger.Error("unable to parse form")
 			http.Error(w, "unable to parse form", http.StatusBadRequest)
 			return
 		}
 		incomingJsonnet := r.FormValue("jsonnet-input")
 		if ok := disallowFileImports.Match([]byte(incomingJsonnet)); ok {
-			state.Logger.Warn("attempt to import file", "jsonnet", incomingJsonnet)
+			srv.State.Logger.Warn("attempt to import file", "jsonnet", incomingJsonnet)
 			_, _ = w.Write([]byte("File imports are disabled."))
 			return
 		}
diff --git a/internal/server/backend_routes_test.go b/internal/server/backend_routes_test.go
@@ -1,4 +1,4 @@
-package routes_test
+package server_test
 
 import (
 	"crypto/sha512"
@@ -11,7 +11,7 @@ import (
 	"testing"
 
 	"github.com/google/go-jsonnet"
-	"github.com/jdockerty/jsonnet-playground/internal/server/routes"
+	"github.com/jdockerty/jsonnet-playground/internal/server"
 	"github.com/jdockerty/jsonnet-playground/internal/server/state"
 	"github.com/kubecfg/kubecfg/pkg/kubecfg"
 	"github.com/stretchr/testify/assert"
@@ -33,9 +33,9 @@ func TestHandleRun(t *testing.T) {
 		expected   string
 		shouldFail bool
 	}{
-		{name: "hello-world", input: "{hello: 'world'}", expected: "../../../testdata/hello-world.json", shouldFail: false},
-		{name: "blank", input: "{}", expected: "../../../testdata/blank.json", shouldFail: false},
-		{name: "kubecfg", input: "local kubecfg = import 'internal:///kubecfg.libsonnet';\n{myVeryNestedObj:: { foo: { bar: { baz: { qux: 'some-val' }}}}, hasValue: kubecfg.objectHasPathAll($.myVeryNestedObj, 'foo.bar.baz.qux')}", expected: "../../../testdata/kubecfg.json", shouldFail: false},
+		{name: "hello-world", input: "{hello: 'world'}", expected: "../../testdata/hello-world.json", shouldFail: false},
+		{name: "blank", input: "{}", expected: "../../testdata/blank.json", shouldFail: false},
+		{name: "kubecfg", input: "local kubecfg = import 'internal:///kubecfg.libsonnet';\n{myVeryNestedObj:: { foo: { bar: { baz: { qux: 'some-val' }}}}, hasValue: kubecfg.objectHasPathAll($.myVeryNestedObj, 'foo.bar.baz.qux')}", expected: "../../testdata/kubecfg.json", shouldFail: false},
 		{name: "invalid-jsonnet", input: "{", expected: "Invalid Jsonnet", shouldFail: true},
 		{name: "invalid-jsonnet-2", input: "{hello:}", expected: "Invalid Jsonnet", shouldFail: true},
 		{name: "file-import-jsonnet", input: "local f = import 'file:///proc/self/environ'; error 'test' + f", expected: "File imports are disabled", shouldFail: true},
@@ -49,7 +49,8 @@ func TestHandleRun(t *testing.T) {
 		req := httptest.NewRequest("POST", "/api/run", nil)
 		req.PostForm = data
 
-		handler := routes.HandleRun(state.New("127.0.0.1", "https://example.com"))
+		srv := server.New(state.New("127.0.0.1", "https://example.com"))
+		handler := srv.HandleRun()
 		handler.ServeHTTP(rec, req)
 
 		if tc.shouldFail {
@@ -87,7 +88,8 @@ func TestHandleCreateShare(t *testing.T) {
 		req := httptest.NewRequest("POST", "/api/share", nil)
 		req.PostForm = data
 
-		handler := routes.HandleCreateShare(state.New("127.0.0.1", "https://example.com"))
+		srv := server.New(state.New("127.0.0.1", "https://example.com"))
+		handler := srv.HandleCreateShare()
 		handler.ServeHTTP(rec, req)
 
 		if tc.shouldFail {
@@ -107,7 +109,8 @@ func TestHandleGetShare(t *testing.T) {
 	snippetHash := hex.EncodeToString(sha512.New().Sum([]byte(snippet)))[:15]
 
 	// Get non-existent snippet
-	handler := routes.HandleGetShare(s)
+	srv := server.New(s)
+	handler := srv.HandleGetShare()
 	path := fmt.Sprintf("/api/share/%s", snippetHash)
 	rec := httptest.NewRecorder()
 	req := httptest.NewRequest("GET", path, nil)
@@ -117,10 +120,10 @@ func TestHandleGetShare(t *testing.T) {
 
 	// Add snippet to store
 	evaluated, _ := vm.EvaluateAnonymousSnippet("", snippet)
-	s.Store[snippetHash] = evaluated
+	srv.State.Store[snippetHash] = evaluated
 
 	// Get snippet which has been added
-	handler = routes.HandleGetShare(s)
+	handler = srv.HandleGetShare()
 	rec = httptest.NewRecorder()
 	req = httptest.NewRequest("GET", fmt.Sprintf("/api/share/%s", snippetHash), nil)
 	req.SetPathValue("shareHash", snippetHash)
@@ -134,10 +137,11 @@ func TestHandleVersions(t *testing.T) {
 	assert := assert.New(t)
 	s := state.New("127.0.0.1", "https://example.com")
 
-	handler := routes.HandleVersions(s)
+	srv := server.New(s)
+	handler := srv.HandleVersions()
 	rec := httptest.NewRecorder()
 	req := httptest.NewRequest("GET", "/api/versions", nil)
 	handler.ServeHTTP(rec, req)
 
-	assert.Contains(rec.Body.String(), string(routes.VersionResponse))
+	assert.Contains(rec.Body.String(), string(server.VersionResponse))
 }
diff --git a/internal/server/frontend_routes.go b/internal/server/frontend_routes.go
@@ -1,12 +1,10 @@
-package routes
+package server
 
 import (
 	"context"
-	"log"
 	"net/http"
 
 	"github.com/jdockerty/jsonnet-playground/internal/components"
-	"github.com/jdockerty/jsonnet-playground/internal/server/state"
 )
 
 // HandleAssets wires up the static asset handling for the server.
@@ -15,17 +13,16 @@ func HandleAssets(pattern string, fsHandler http.Handler) http.Handler {
 }
 
 // HandleShare is the rendering of the shared snippet view.
-func HandleShare(state *state.State) http.HandlerFunc {
+func (srv *PlaygroundServer) HandleShare() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		shareHash := r.PathValue("shareHash")
-		log.Printf("Incoming share view for %+v\n", shareHash)
+		srv.State.Logger.Info("share view loading", "shareHash", shareHash)
 
 		if shareHash == "" {
-			log.Println("Browsed to share with no hash, rendering root page")
+			srv.State.Logger.Debug("browse to share with no hash, rendering root page")
 			_ = components.RootPage("").Render(context.Background(), w)
 			return
 		}
-		log.Println("Rendering share page")
 		sharePage := components.RootPage(shareHash)
 		_ = sharePage.Render(context.Background(), w)
 	}
diff --git a/internal/server/server.go b/internal/server/server.go
@@ -7,7 +7,6 @@ import (
 
 	"github.com/a-h/templ"
 	"github.com/jdockerty/jsonnet-playground/internal/components"
-	"github.com/jdockerty/jsonnet-playground/internal/server/routes"
 	"github.com/jdockerty/jsonnet-playground/internal/server/state"
 )
 
@@ -37,17 +36,17 @@ func (srv *PlaygroundServer) Routes() error {
 	// Frontend routes
 	rootPage := components.RootPage("")
 	fs := http.FileServer(http.Dir(path))
-	http.Handle("/assets/", routes.HandleAssets("/assets/", fs))
+	http.Handle("/assets/", HandleAssets("/assets/", fs))
 	http.Handle("/", templ.Handler(rootPage))
-	http.HandleFunc("/share/{shareHash}", routes.HandleShare(srv.State))
+	http.HandleFunc("/share/{shareHash}", srv.HandleShare())
 
 	// Backend/API routes
-	http.HandleFunc("/api/health", routes.Health(srv.State))
-	http.HandleFunc("/api/run", routes.DisableFileImports(srv.State, routes.HandleRun(srv.State)))
-	http.HandleFunc("/api/format", routes.DisableFileImports(srv.State, routes.HandleFormat(srv.State)))
-	http.HandleFunc("/api/share", routes.DisableFileImports(srv.State, routes.HandleCreateShare(srv.State)))
-	http.HandleFunc("/api/share/{shareHash}", routes.DisableFileImports(srv.State, routes.HandleGetShare(srv.State)))
-	http.HandleFunc("/api/versions", routes.HandleVersions(srv.State))
+	http.HandleFunc("/api/health", srv.Health())
+	http.HandleFunc("/api/run", DisableFileImports(srv, srv.HandleRun()))
+	http.HandleFunc("/api/format", DisableFileImports(srv, srv.HandleFormat()))
+	http.HandleFunc("/api/share", DisableFileImports(srv, srv.HandleCreateShare()))
+	http.HandleFunc("/api/share/{shareHash}", DisableFileImports(srv, srv.HandleGetShare()))
+	http.HandleFunc("/api/versions", srv.HandleVersions())
 	return nil
 }
 
diff --git a/scripts/kubecfg_hack.sh b/scripts/kubecfg_hack.sh
@@ -1,7 +1,7 @@
 #!/bin/sh
 
 ACTUAL_KUBECFG_VERSION=$(go mod edit -json | jq -r '.Require[] | select(.Path | contains("kubecfg/kubecfg")).Version' | tr -d " ")
-WRITTEN_KUBECFG_VERSION=$(grep "KubecfgVersion  " internal/server/routes/backend.go | cut -d "=" -f 2 | tr -d '" ')
+WRITTEN_KUBECFG_VERSION=$(grep "KubecfgVersion  " internal/server/backend_routes.go | cut -d "=" -f 2 | tr -d '" ')
 
 if [ "${ACTUAL_KUBECFG_VERSION}" != "${WRITTEN_KUBECFG_VERSION}" ]; then
     echo "Written kubecfg version (${WRITTEN_KUBECFG_VERSION}) does not match go.mod file version (${ACTUAL_KUBECFG_VERSION})"
