From 33784cbb9408f405ae2ba9dfcb71d6a56d2015bd Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Fri, 16 Feb 2024 16:53:50 -0800 Subject: [PATCH] chore: more small lint/perf fixes (#493) Signed-off-by: James Hillyerd --- pkg/message/manager.go | 6 ++--- pkg/message/manager_test.go | 36 ++++++++++++++-------------- pkg/msghub/hub.go | 2 +- pkg/policy/address.go | 7 +++--- pkg/rest/client/apiv1_client_opts.go | 2 +- pkg/rest/client/rest.go | 2 +- pkg/rest/client/rest_test.go | 11 +++++---- pkg/server/web/handlers.go | 5 ++-- pkg/server/web/helpers.go | 2 +- pkg/server/web/server.go | 2 +- pkg/test/integration_test.go | 3 +-- 11 files changed, 40 insertions(+), 38 deletions(-) diff --git a/pkg/message/manager.go b/pkg/message/manager.go index eec8199..a41765e 100644 --- a/pkg/message/manager.go +++ b/pkg/message/manager.go @@ -79,9 +79,9 @@ func (s *StoreManager) Deliver( tstamp := now.UTC().Format(recvdTimeFmt) // Process inbound message through extensions. - mailboxes := make([]string, len(recipients)) - for i, recip := range recipients { - mailboxes[i] = recip.Mailbox + mailboxes := make([]string, 0, len(recipients)) + for _, recip := range recipients { + mailboxes = append(mailboxes, recip.Mailbox) } // Construct InboundMessage event and process through extensions. diff --git a/pkg/message/manager_test.go b/pkg/message/manager_test.go index 07f9b29..74ab905 100644 --- a/pkg/message/manager_test.go +++ b/pkg/message/manager_test.go @@ -368,10 +368,10 @@ func TestGetMessage(t *testing.T) { // Add a test message. subject := "getMessage1" - id := addTestMessage(sm, "box1", subject) + id := addTestMessage(sm, "get-box", subject) // Verify retrieval of the test message. - msg, err := sm.GetMessage("box1", id) + msg, err := sm.GetMessage("get-box", id) require.NoError(t, err, "GetMessage must succeed") require.NotNil(t, msg, "GetMessage must return a result") assert.Equal(t, subject, msg.Subject) @@ -383,19 +383,19 @@ func TestMarkSeen(t *testing.T) { // Add a test message. subject := "getMessage1" - id := addTestMessage(sm, "box1", subject) + id := addTestMessage(sm, "seen-box", subject) // Verify test message unseen. - msg, err := sm.GetMessage("box1", id) + msg, err := sm.GetMessage("seen-box", id) require.NoError(t, err, "GetMessage must succeed") require.NotNil(t, msg, "GetMessage must return a result") assert.False(t, msg.Seen, "msg should be unseen") - err = sm.MarkSeen("box1", id) + err = sm.MarkSeen("seen-box", id) require.NoError(t, err, "MarkSeen should succeed") // Verify test message seen. - msg, err = sm.GetMessage("box1", id) + msg, err = sm.GetMessage("seen-box", id) require.NoError(t, err, "GetMessage must succeed") require.NotNil(t, msg, "GetMessage must return a result") assert.True(t, msg.Seen, "msg should have been seen") @@ -405,17 +405,17 @@ func TestRemoveMessage(t *testing.T) { sm, _ := testStoreManager() // Add test messages. - id1 := addTestMessage(sm, "box1", "subject 1") - id2 := addTestMessage(sm, "box1", "subject 2") - id3 := addTestMessage(sm, "box1", "subject 3") - got, err := sm.GetMetadata("box1") + id1 := addTestMessage(sm, "rm-box", "subject 1") + id2 := addTestMessage(sm, "rm-box", "subject 2") + id3 := addTestMessage(sm, "rm-box", "subject 3") + got, err := sm.GetMetadata("rm-box") require.NoError(t, err) require.Len(t, got, 3) // Delete message 2 and verify. - err = sm.RemoveMessage("box1", id2) + err = sm.RemoveMessage("rm-box", id2) require.NoError(t, err) - got, err = sm.GetMetadata("box1") + got, err = sm.GetMetadata("rm-box") require.NoError(t, err) require.Len(t, got, 2, "Should be 2 messages remaining") @@ -431,17 +431,17 @@ func TestPurgeMessages(t *testing.T) { sm, _ := testStoreManager() // Add test messages. - _ = addTestMessage(sm, "box1", "subject 1") - _ = addTestMessage(sm, "box1", "subject 2") - _ = addTestMessage(sm, "box1", "subject 3") - got, err := sm.GetMetadata("box1") + _ = addTestMessage(sm, "purge-box", "subject 1") + _ = addTestMessage(sm, "purge-box", "subject 2") + _ = addTestMessage(sm, "purge-box", "subject 3") + got, err := sm.GetMetadata("purge-box") require.NoError(t, err) require.Len(t, got, 3) // Purge and verify. - err = sm.PurgeMessages("box1") + err = sm.PurgeMessages("purge-box") require.NoError(t, err) - got, err = sm.GetMetadata("box1") + got, err = sm.GetMetadata("purge-box") require.NoError(t, err) assert.Empty(t, got, "Purge should remove all mailbox messages") } diff --git a/pkg/msghub/hub.go b/pkg/msghub/hub.go index 14727f7..16749a3 100644 --- a/pkg/msghub/hub.go +++ b/pkg/msghub/hub.go @@ -140,7 +140,7 @@ func (hub *Hub) RemoveListener(l Listener) { // for unit tests. func (hub *Hub) Sync() { done := make(chan struct{}) - hub.opChan <- func(h *Hub) { + hub.opChan <- func(_ *Hub) { close(done) } <-done diff --git a/pkg/policy/address.go b/pkg/policy/address.go index f9d7ea4..cfafda4 100644 --- a/pkg/policy/address.go +++ b/pkg/policy/address.go @@ -306,15 +306,16 @@ LOOP: case c == '\\': inCharQuote = true case c == '"': - if inCharQuote { + switch { + case inCharQuote: err = buf.WriteByte(c) if err != nil { return } inCharQuote = false - } else if inStringQuote { + case inStringQuote: inStringQuote = false - } else { + default: if i == 0 { inStringQuote = true } else { diff --git a/pkg/rest/client/apiv1_client_opts.go b/pkg/rest/client/apiv1_client_opts.go index 5b5b313..283d801 100644 --- a/pkg/rest/client/apiv1_client_opts.go +++ b/pkg/rest/client/apiv1_client_opts.go @@ -13,7 +13,7 @@ type options struct { // Option can apply itself to the private options type. type Option interface { - apply(*options) + apply(opts *options) } func getDefaultOptions() *options { diff --git a/pkg/rest/client/rest.go b/pkg/rest/client/rest.go index 623f089..6fd1402 100644 --- a/pkg/rest/client/rest.go +++ b/pkg/rest/client/rest.go @@ -11,7 +11,7 @@ import ( // httpClient allows http.Client to be mocked for tests type httpClient interface { - Do(*http.Request) (*http.Response, error) + Do(req *http.Request) (*http.Response, error) } // Generic REST restClient diff --git a/pkg/rest/client/rest_test.go b/pkg/rest/client/rest_test.go index f8b6260..fc58161 100644 --- a/pkg/rest/client/rest_test.go +++ b/pkg/rest/client/rest_test.go @@ -7,6 +7,8 @@ import ( "net/http" "net/url" "testing" + + "github.com/stretchr/testify/require" ) const baseURLStr = "http://test.local:8080" @@ -78,10 +80,11 @@ func TestDoTable(t *testing.T) { t.Run(testname, func(t *testing.T) { mth := &mockHTTPClient{} c := &restClient{mth, test.base} - _, err := c.do(test.method, test.uri, test.wantBody) - if err != nil { - t.Fatal(err) - } + resp, err := c.do(test.method, test.uri, test.wantBody) + require.NoError(t, err) + err = resp.Body.Close() + require.NoError(t, err) + if mth.req.Method != test.wantMethod { t.Errorf("req.Method == %q, want %q", mth.req.Method, test.wantMethod) } diff --git a/pkg/server/web/handlers.go b/pkg/server/web/handlers.go index d19d232..d06a694 100644 --- a/pkg/server/web/handlers.go +++ b/pkg/server/web/handlers.go @@ -5,7 +5,6 @@ import ( "net/http" "os" - "github.com/inbucket/inbucket/v3/pkg/config" "github.com/rs/zerolog/log" ) @@ -86,13 +85,13 @@ func requestLoggingWrapper(next http.Handler) http.Handler { } // spaTemplateHandler creates a handler to serve the index.html template for our SPA. -func spaTemplateHandler(tmpl *template.Template, basePath string, - webConfig config.Web) http.Handler { +func spaTemplateHandler(tmpl *template.Template, basePath string) http.Handler { tmplData := struct { BasePath string }{ BasePath: basePath, } + return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { // ensure we do now allow click jacking w.Header().Set("X-Frame-Options", "SameOrigin") diff --git a/pkg/server/web/helpers.go b/pkg/server/web/helpers.go index 7ab2890..2285b41 100644 --- a/pkg/server/web/helpers.go +++ b/pkg/server/web/helpers.go @@ -21,6 +21,6 @@ func TextToHTML(text string) string { // WrapURL wraps a tag around the provided URL func WrapURL(url string) string { - unescaped := strings.Replace(url, "&", "&", -1) + unescaped := strings.ReplaceAll(url, "&", "&") return fmt.Sprintf("%s", unescaped, url) } diff --git a/pkg/server/web/server.go b/pkg/server/web/server.go index 1c8f04b..bf52289 100644 --- a/pkg/server/web/server.go +++ b/pkg/server/web/server.go @@ -109,7 +109,7 @@ func NewServer(conf *config.Root, mm message.Manager, mh *msghub.Hub) *Server { // SPA managed paths. spaHandler := cookieHandler(appConfigCookie(conf.Web), - spaTemplateHandler(indexTmpl, prefix("/"), conf.Web)) + spaTemplateHandler(indexTmpl, prefix("/"))) Router.Path(prefix("/")).Handler(spaHandler) Router.Path(prefix("/monitor")).Handler(spaHandler) Router.Path(prefix("/status")).Handler(spaHandler) diff --git a/pkg/test/integration_test.go b/pkg/test/integration_test.go index de56deb..9660886 100644 --- a/pkg/test/integration_test.go +++ b/pkg/test/integration_test.go @@ -282,8 +282,7 @@ func clearEnv() { } // Backup ciritcal env variables. - switch runtime.GOOS { - case "windows": + if runtime.GOOS == "windows" { backup("SYSTEMROOT") }