From 025d901e244a5ab4cfc551ac9fb4d1a87e5e0a70 Mon Sep 17 00:00:00 2001 From: Blake Gentry Date: Wed, 28 Aug 2024 20:22:49 -0500 Subject: [PATCH] refactor server/handler related APIs for devex (#133) * Rename `DBTXWithBegin` interface type to `DB`, and avoid defining it via an internal package type in favor of an inline definition. * Rename `HandlerOpts` to `ServerOpts` since it's used by the `NewServer` constructor. * Implement `ServeHTTP()` / `http.Handler` directly on the `Server` type so that users don't need to jump through a `.Handler()` method when mounting it into an HTTP mux. Fixes #130. Fixes #131. Fixes #132. --- CHANGELOG.md | 3 +++ cmd/riverui/main.go | 6 +++--- handler.go | 45 +++++++++++++++++++++++++---------------- handler_api_endpoint.go | 2 +- handler_test.go | 6 +++--- 5 files changed, 38 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ba8248..186046b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - The module name was changed from `github.com/riverqueue/riverui` to `riverqueue.com/riverui`. This change was made to facilitate bundling of module releases that include vendored frontend assets, which will enable the embedded `Handler` type to be usable by anybody who `go get` installs the module without requiring a complex build setup. +- Rename `HandlerOpts` to `ServerOpts` for consistency. The `Handler` type was renamed to `Server` in [PR #108](https://github.com/riverqueue/riverui/pull/108) but the opts type was not renamed until now. [PR #133](https://github.com/riverqueue/riverui/pull/133). +- Implement `http.Handler` on `Server` type via a `ServeHTTP` method so that it can be used directly without needing to call `.Handler()` on it. [PR #133](https://github.com/riverqueue/riverui/pull/133). +- Directly specify `DB` interface type and rename it. Avoids relying on embedding a type from an internal package. [PR #133](https://github.com/riverqueue/riverui/pull/133). ### Removed diff --git a/cmd/riverui/main.go b/cmd/riverui/main.go index aa7e069..992e586 100644 --- a/cmd/riverui/main.go +++ b/cmd/riverui/main.go @@ -81,9 +81,9 @@ func initAndServe(ctx context.Context) int { return 1 } - handlerOpts := &riverui.HandlerOpts{ + handlerOpts := &riverui.ServerOpts{ Client: client, - DBPool: dbPool, + DB: dbPool, DevMode: devMode, LiveFS: liveFS, Logger: logger, @@ -101,7 +101,7 @@ func initAndServe(ctx context.Context) int { return 1 } - logHandler := sloghttp.Recovery(server.Handler()) + logHandler := sloghttp.Recovery(server) config := sloghttp.Config{ WithSpanID: otelEnabled, WithTraceID: otelEnabled, diff --git a/handler.go b/handler.go index 79ecd28..8b73ef6 100644 --- a/handler.go +++ b/handler.go @@ -15,6 +15,7 @@ import ( "strings" "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgconn" "github.com/riverqueue/river" "github.com/riverqueue/river/rivershared/baseservice" @@ -24,20 +25,22 @@ import ( "riverqueue.com/riverui/internal/apiendpoint" "riverqueue.com/riverui/internal/apimiddleware" - "riverqueue.com/riverui/internal/dbsqlc" ) -type DBTXWithBegin interface { +// DB is the interface for a pgx database connection. +type DB interface { Begin(ctx context.Context) (pgx.Tx, error) - dbsqlc.DBTX + Exec(ctx context.Context, query string, args ...interface{}) (pgconn.CommandTag, error) + Query(ctx context.Context, query string, args ...interface{}) (pgx.Rows, error) + QueryRow(ctx context.Context, query string, args ...interface{}) pgx.Row } -// HandlerOpts are the options for creating a new Handler. -type HandlerOpts struct { +// ServerOpts are the options for creating a new Server. +type ServerOpts struct { // Client is the River client to use for API requests. Client *river.Client[pgx.Tx] - // DBPool is the database connection pool to use for API requests. - DBPool DBTXWithBegin + // DB is the database to use for API requests. + DB DB // DevMode is whether the server is running in development mode. DevMode bool // LiveFS is whether to use the live filesystem for the frontend. @@ -48,12 +51,12 @@ type HandlerOpts struct { Prefix string } -func (opts *HandlerOpts) validate() error { +func (opts *ServerOpts) validate() error { if opts.Client == nil { return errors.New("client is required") } - if opts.DBPool == nil { - return errors.New("db pool is required") + if opts.DB == nil { + return errors.New("db is required") } if opts.Logger == nil { return errors.New("logger is required") @@ -73,14 +76,18 @@ func NormalizePathPrefix(prefix string) string { return prefix } +// Server is an HTTP server that serves the River UI and API. It must be +// started with Start to initialize caching and background query functionality +// prior to serving requests. Server implements http.Handler, so it can be +// directly mounted in an http.ServeMux. type Server struct { baseStartStop startstop.BaseStartStop handler http.Handler services []startstop.Service } -// NewServer creates a new http.Handler that serves the River UI and API. -func NewServer(opts *HandlerOpts) (*Server, error) { +// NewServer creates a new Server that serves the River UI and API. +func NewServer(opts *ServerOpts) (*Server, error) { if opts == nil { return nil, errors.New("opts is required") } @@ -130,7 +137,7 @@ func NewServer(opts *HandlerOpts) (*Server, error) { Time: &baseservice.UnStubbableTimeGenerator{}, }, client: opts.Client, - dbPool: opts.DBPool, + dbPool: opts.DB, logger: opts.Logger, } @@ -186,12 +193,16 @@ func NewServer(opts *HandlerOpts) (*Server, error) { return server, nil } -// Handler returns an http.Handler that can be mounted to serve HTTP requests. -func (s *Server) Handler() http.Handler { return s.handler } +// ServeHTTP returns an http.ServeHTTP that can be mounted to serve HTTP +// requests. +func (s *Server) ServeHTTP(rw http.ResponseWriter, req *http.Request) { + s.handler.ServeHTTP(rw, req) +} // Start starts the server's background services. Notably, this does _not_ cause -// the server to start listening for HTTP in any way. To do that, call Handler -// and mount or run it using Go's built in `net/http`. +// the server to start listening for HTTP in any way. To serve HTTP requests, +// the Server implements `http.Handler` via a `ServeHTTP` method and can be +// mounted in an existing `http.ServeMux`. func (s *Server) Start(ctx context.Context) error { ctx, shouldStart, started, stopped := s.baseStartStop.StartInit(ctx) if !shouldStart { diff --git a/handler_api_endpoint.go b/handler_api_endpoint.go index 3b5adc2..313e351 100644 --- a/handler_api_endpoint.go +++ b/handler_api_endpoint.go @@ -31,7 +31,7 @@ import ( type apiBundle struct { archetype *baseservice.Archetype client *river.Client[pgx.Tx] - dbPool DBTXWithBegin + dbPool DB logger *slog.Logger } diff --git a/handler_test.go b/handler_test.go index b905b3e..fcb187b 100644 --- a/handler_test.go +++ b/handler_test.go @@ -46,9 +46,9 @@ func TestNewHandlerIntegration(t *testing.T) { require.NoError(t, err) t.Cleanup(func() { tx.Rollback(ctx) }) - server, err := NewServer(&HandlerOpts{ + server, err := NewServer(&ServerOpts{ Client: client, - DBPool: tx, + DB: tx, DevMode: true, LiveFS: true, Logger: logger, @@ -65,7 +65,7 @@ func TestNewHandlerIntegration(t *testing.T) { t.Logf("--> %s %s", method, path) - server.Handler().ServeHTTP(recorder, req) + server.ServeHTTP(recorder, req) status := recorder.Result().StatusCode //nolint:bodyclose t.Logf("Response status: %d", status)