From 8b476e772273c0a769182e03dc74faedd951c59a Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Fri, 26 Nov 2021 09:50:56 +0100 Subject: [PATCH] Simplify web router code (#541) --- cmd/server/main.go | 3 +- cmd/server/server.go | 6 +- cmd/server/setup.go | 14 ---- .../router/middleware/{logger => }/logger.go | 2 +- server/router/router.go | 14 +--- server/web/config.go | 2 +- server/web/opts.go | 42 ---------- server/web/opts_test.go | 36 -------- server/web/web.go | 82 ++++++------------- 9 files changed, 34 insertions(+), 167 deletions(-) rename server/router/middleware/{logger => }/logger.go (98%) delete mode 100644 server/web/opts.go delete mode 100644 server/web/opts_test.go diff --git a/cmd/server/main.go b/cmd/server/main.go index 58107a0d1..168918b72 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -35,10 +35,9 @@ func main() { app.Usage = "woodpecker server" app.Action = run app.Flags = flags - app.Before = before if err := app.Run(os.Args); err != nil { - fmt.Fprintln(os.Stderr, err) + _, _ = fmt.Fprintln(os.Stderr, err) os.Exit(1) } } diff --git a/cmd/server/server.go b/cmd/server/server.go index 5f7ae79df..e1efece7b 100644 --- a/cmd/server/server.go +++ b/cmd/server/server.go @@ -45,8 +45,8 @@ import ( "github.com/woodpecker-ci/woodpecker/server/remote" "github.com/woodpecker-ci/woodpecker/server/router" "github.com/woodpecker-ci/woodpecker/server/router/middleware" - "github.com/woodpecker-ci/woodpecker/server/router/middleware/logger" "github.com/woodpecker-ci/woodpecker/server/store" + "github.com/woodpecker-ci/woodpecker/server/web" ) func run(c *cli.Context) error { @@ -120,7 +120,7 @@ func run(c *cli.Context) error { var webUIServe func(w http.ResponseWriter, r *http.Request) if proxyWebUI == "" { - webUIServe = setupTree(c).ServeHTTP + webUIServe = web.New().ServeHTTP } else { origin, _ := url.Parse(proxyWebUI) @@ -138,7 +138,7 @@ func run(c *cli.Context) error { // setup the server and start the listener handler := router.Load( webUIServe, - logger.Logger(time.RFC3339, true), + middleware.Logger(time.RFC3339, true), middleware.Version, middleware.Config(c), middleware.Store(c, store_), diff --git a/cmd/server/setup.go b/cmd/server/setup.go index d78957cb0..55e263845 100644 --- a/cmd/server/setup.go +++ b/cmd/server/setup.go @@ -20,7 +20,6 @@ import ( "os" "time" - "github.com/gin-gonic/gin" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" "github.com/rs/zerolog/log" @@ -43,7 +42,6 @@ import ( "github.com/woodpecker-ci/woodpecker/server/remote/gogs" "github.com/woodpecker-ci/woodpecker/server/store" "github.com/woodpecker-ci/woodpecker/server/store/datastore" - "github.com/woodpecker-ci/woodpecker/server/web" ) func setupStore(c *cli.Context) (store.Store, error) { @@ -303,18 +301,6 @@ func setupCoding(c *cli.Context) (remote.Remote, error) { return coding.New(opts) } -func setupTree(c *cli.Context) *gin.Engine { - tree := gin.New() - tree.UseRawPath = true - web.New( - web.WithSync(time.Hour*72), - web.WithDocs(c.String("docs")), - ).Register(tree) - return tree -} - -func before(c *cli.Context) error { return nil } - func setupMetrics(g *errgroup.Group, store_ store.Store) { pendingJobs := promauto.NewGauge(prometheus.GaugeOpts{ Namespace: "woodpecker", diff --git a/server/router/middleware/logger/logger.go b/server/router/middleware/logger.go similarity index 98% rename from server/router/middleware/logger/logger.go rename to server/router/middleware/logger.go index 13503f966..76d495a61 100644 --- a/server/router/middleware/logger/logger.go +++ b/server/router/middleware/logger.go @@ -1,4 +1,4 @@ -package logger +package middleware import ( "time" diff --git a/server/router/router.go b/server/router/router.go index 2e18f532d..62b2ec48f 100644 --- a/server/router/router.go +++ b/server/router/router.go @@ -29,7 +29,7 @@ import ( ) // Load loads the router -func Load(serveHTTP func(w http.ResponseWriter, r *http.Request), middleware ...gin.HandlerFunc) http.Handler { +func Load(noRouteHandler http.HandlerFunc, middleware ...gin.HandlerFunc) http.Handler { e := gin.New() e.UseRawPath = true e.Use(gin.Recovery()) @@ -46,17 +46,9 @@ func Load(serveHTTP func(w http.ResponseWriter, r *http.Request), middleware ... e.Use(session.SetUser()) e.Use(token.Refresh) - e.NoRoute(func(c *gin.Context) { - req := c.Request.WithContext( - web.WithUser( - c.Request.Context(), - session.User(c), - ), - ) - serveHTTP(c.Writer, req) - }) + e.NoRoute(gin.WrapF(noRouteHandler)) - e.GET("/web-config.js", web.WebConfig) + e.GET("/web-config.js", web.Config) e.GET("/logout", api.GetLogout) e.GET("/login", api.HandleLogin) diff --git a/server/web/config.go b/server/web/config.go index f3a5ae277..b861f32b3 100644 --- a/server/web/config.go +++ b/server/web/config.go @@ -29,7 +29,7 @@ import ( "github.com/woodpecker-ci/woodpecker/version" ) -func WebConfig(c *gin.Context) { +func Config(c *gin.Context) { user := session.User(c) var csrf string diff --git a/server/web/opts.go b/server/web/opts.go deleted file mode 100644 index aa976fa65..000000000 --- a/server/web/opts.go +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright 2018 Drone.IO Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package web - -import "time" - -// Options defines website handler options. -type Options struct { - sync time.Duration - docs string -} - -// Option configures the website handler. -type Option func(*Options) - -// WithSync configures the website handler with the duration value -// used to determine if the user account requires synchronization. -func WithSync(d time.Duration) Option { - return func(o *Options) { - o.sync = d - } -} - -// WithDocs configures the website handler with the documentation -// website address, which should be included in the user interface. -func WithDocs(s string) Option { - return func(o *Options) { - o.docs = s - } -} diff --git a/server/web/opts_test.go b/server/web/opts_test.go deleted file mode 100644 index 0e676f91e..000000000 --- a/server/web/opts_test.go +++ /dev/null @@ -1,36 +0,0 @@ -// Copyright 2018 Drone.IO Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package web - -import ( - "testing" - "time" -) - -func TestWithSync(t *testing.T) { - opts := new(Options) - WithSync(time.Minute)(opts) - if got, want := opts.sync, time.Minute; got != want { - t.Errorf("Want sync duration %v, got %v", want, got) - } -} - -func TestWithDocs(t *testing.T) { - opts := new(Options) - WithDocs("http://docs.drone.io")(opts) - if got, want := opts.docs, "http://docs.drone.io"; got != want { - t.Errorf("Want documentation url %q, got %q", want, got) - } -} diff --git a/server/web/web.go b/server/web/web.go index 6a153f7f4..58b730791 100644 --- a/server/web/web.go +++ b/server/web/web.go @@ -15,7 +15,6 @@ package web import ( - "context" "crypto/md5" "fmt" "net/http" @@ -24,71 +23,40 @@ import ( "github.com/gin-gonic/gin" "github.com/rs/zerolog/log" - "github.com/woodpecker-ci/woodpecker/server/model" "github.com/woodpecker-ci/woodpecker/web" ) -// Endpoint provides the website endpoints. -type Endpoint interface { - // Register registers the provider endpoints. - Register(*gin.Engine) +// etag is an identifier for a resource version +// it lets caches determine if resource is still the same and not send it again +var etag = fmt.Sprintf("%x", md5.Sum([]byte(time.Now().String()))) + +// New returns a gin engine to serve the web frontend. +func New() *gin.Engine { + e := gin.New() + + e.Use(setupCache) + + h := http.FileServer(web.HttpFS()) + e.GET("/favicon.svg", gin.WrapH(h)) + e.GET("/assets/*filepath", gin.WrapH(h)) + + e.NoRoute(handleIndex) + + return e } -// New returns the default website endpoint. -func New(opt ...Option) Endpoint { - opts := new(Options) - for _, f := range opt { - f(opts) - } - - return &website{ - fs: web.HttpFS(), - opts: opts, - data: web.MustLookup("index.html"), - } -} - -type website struct { - opts *Options - fs http.FileSystem - data []byte -} - -func (w *website) Register(mux *gin.Engine) { - h := http.FileServer(w.fs) - h = setupCache(h) - mux.GET("/favicon.svg", gin.WrapH(h)) - mux.GET("/assets/*filepath", gin.WrapH(h)) - mux.NoRoute(gin.WrapF(w.handleIndex)) -} - -func (w *website) handleIndex(rw http.ResponseWriter, r *http.Request) { +func handleIndex(c *gin.Context) { + rw := c.Writer + data := web.MustLookup("index.html") rw.Header().Set("Content-Type", "text/html; charset=UTF-8") rw.WriteHeader(200) - if _, err := rw.Write(w.data); err != nil { + if _, err := rw.Write(data); err != nil { log.Error().Err(err).Msg("can not write index.html") } } -func setupCache(h http.Handler) http.Handler { - data := []byte(time.Now().String()) - etag := fmt.Sprintf("%x", md5.Sum(data)) - - return http.HandlerFunc( - func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Cache-Control", "public, max-age=31536000") - w.Header().Del("Expires") - w.Header().Set("ETag", etag) - h.ServeHTTP(w, r) - }, - ) +func setupCache(c *gin.Context) { + c.Writer.Header().Set("Cache-Control", "public, max-age=31536000") + c.Writer.Header().Del("Expires") + c.Writer.Header().Set("ETag", etag) } - -// WithUser returns a context with the current authenticated user. -func WithUser(c context.Context, user *model.User) context.Context { - return context.WithValue(c, userKey, user) -} - -type key int - -const userKey key = 0