From ff5b4639d597203f8aec43e5eae8fe3774976d32 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Thu, 16 May 2019 11:46:17 -0600 Subject: [PATCH] Some minor updates, and get rid of OnLoad/OnUnload --- caddy.go | 54 +----------------------------- modules.go | 24 ++----------- modules/caddyhttp/caddylog/log.go | 18 ---------- modules/caddyhttp/matchers.go | 4 +-- modules/caddyhttp/matchers_test.go | 8 ++--- modules/caddyhttp/replacer.go | 17 +++++----- modules/caddyhttp/staticresp.go | 2 +- 7 files changed, 20 insertions(+), 107 deletions(-) diff --git a/caddy.go b/caddy.go index 2aa0c6a2..585b9df8 100644 --- a/caddy.go +++ b/caddy.go @@ -100,33 +100,6 @@ func Run(cfg *Config) error { } }() - // OnLoad - err = func() error { - for modName, instances := range moduleInstances { - mod, err := GetModule(modName) - if err != nil { - return err - } - if mod.OnLoad != nil { - var priorState interface{} - if oldCfg != nil { - priorState = oldCfg.moduleStates[modName] - } - modState, err := mod.OnLoad(instances, priorState) - if err != nil { - return fmt.Errorf("module OnLoad: %s: %v", modName, err) - } - if modState != nil { - cfg.moduleStates[modName] = modState - } - } - } - return nil - }() - if err != nil { - return err - } - // Start err = func() error { h := Handle{cfg} @@ -158,31 +131,6 @@ func Run(cfg *Config) error { } } - // OnUnload - err = func() error { - for modName := range oldModuleInstances { - mod, err := GetModule(modName) - if err != nil { - return err - } - if mod.OnUnload != nil { - var unloadingState interface{} - if oldCfg != nil { - unloadingState = oldCfg.moduleStates[modName] - } - err := mod.OnUnload(unloadingState) - if err != nil { - log.Printf("[ERROR] module OnUnload: %s: %v", modName, err) - continue - } - } - } - return nil - }() - if err != nil { - return err - } - // shut down listeners that are no longer being used err = func() error { listenersMu.Lock() @@ -240,7 +188,7 @@ type Handle struct { // App returns the configured app named name. If no app with // that name is currently configured, a new empty one will be -// instantiated. (The app module must still be plugged in.) +// instantiated. (The app module must still be registered.) func (h Handle) App(name string) (interface{}, error) { if app, ok := h.current.apps[name]; ok { return app, nil diff --git a/modules.go b/modules.go index 6afa6fb9..b2c9b96f 100644 --- a/modules.go +++ b/modules.go @@ -22,27 +22,6 @@ type Module struct { // It must return a pointer; if not, it // is converted into one. New func() (interface{}, error) - - // OnLoad is invoked after all module - // instances ave been loaded. It receives - // pointers to each instance of this - // module, and any state from a previous - // running configuration, which may be - // nil. - // - // If this module is to carry "global" - // state between all instances through - // reloads, you might find it helpful - // to return it. - // TODO: Is this really better/safer than a global variable? - OnLoad func(instances []interface{}, priorState interface{}) (newState interface{}, err error) - - // OnUnload is called after all module - // instances have been stopped, possibly - // in favor of a new configuration. It - // receives the state given by OnLoad (if - // any). - OnUnload func(state interface{}) error } func (m Module) String() string { return m.Name } @@ -53,6 +32,9 @@ func RegisterModule(mod Module) error { if mod.Name == "caddy" { return fmt.Errorf("modules cannot be named 'caddy'") } + if strings.HasPrefix(mod.Name, "caddy.") { + return fmt.Errorf("modules cannot be namespaced in 'caddy'") + } modulesMu.Lock() defer modulesMu.Unlock() diff --git a/modules/caddyhttp/caddylog/log.go b/modules/caddyhttp/caddylog/log.go index dfc9da58..d3110cc9 100644 --- a/modules/caddyhttp/caddylog/log.go +++ b/modules/caddyhttp/caddylog/log.go @@ -13,24 +13,6 @@ func init() { caddy2.RegisterModule(caddy2.Module{ Name: "http.middleware.log", New: func() (interface{}, error) { return new(Log), nil }, - // TODO: Examples of OnLoad and OnUnload. - OnLoad: func(instances []interface{}, priorState interface{}) (interface{}, error) { - var counter int - if priorState != nil { - counter = priorState.(int) - } - counter++ - for _, inst := range instances { - logInst := inst.(*Log) - logInst.counter = counter - } - log.Println("State is now:", counter) - return counter, nil - }, - OnUnload: func(state interface{}) error { - log.Println("Closing log files, state:", state) - return nil - }, }) } diff --git a/modules/caddyhttp/matchers.go b/modules/caddyhttp/matchers.go index 0179dd7b..7df4d8f7 100644 --- a/modules/caddyhttp/matchers.go +++ b/modules/caddyhttp/matchers.go @@ -233,14 +233,14 @@ func (mre *matchRegexp) match(input string, repl *Replacer, scope string) bool { // save all capture groups, first by index for i, match := range matches { - key := fmt.Sprintf("http.matchers.%s.%s.%d", scope, mre.Name, i) + key := fmt.Sprintf("matchers.%s.%s.%d", scope, mre.Name, i) repl.Map(key, match) } // then by name for i, name := range mre.compiled.SubexpNames() { if i != 0 && name != "" { - key := fmt.Sprintf("http.matchers.%s.%s.%s", scope, mre.Name, name) + key := fmt.Sprintf("matchers.%s.%s.%s", scope, mre.Name, name) repl.Map(key, matches[i]) } } diff --git a/modules/caddyhttp/matchers_test.go b/modules/caddyhttp/matchers_test.go index 7da09a60..f23efab0 100644 --- a/modules/caddyhttp/matchers_test.go +++ b/modules/caddyhttp/matchers_test.go @@ -217,10 +217,10 @@ func TestPathREMatcher(t *testing.T) { } for key, expectVal := range tc.expectRepl { - placeholder := fmt.Sprintf("{http.matchers.path_regexp.%s}", key) + placeholder := fmt.Sprintf("{matchers.path_regexp.%s}", key) actualVal := repl.Replace(placeholder, "") if actualVal != expectVal { - t.Errorf("Test %d [%v]: Expected placeholder {http.matchers.path_regexp.%s} to be '%s' but got '%s'", + t.Errorf("Test %d [%v]: Expected placeholder {matchers.path_regexp.%s} to be '%s' but got '%s'", i, tc.match.Pattern, key, expectVal, actualVal) continue } @@ -334,10 +334,10 @@ func TestHeaderREMatcher(t *testing.T) { } for key, expectVal := range tc.expectRepl { - placeholder := fmt.Sprintf("{http.matchers.header_regexp.%s}", key) + placeholder := fmt.Sprintf("{matchers.header_regexp.%s}", key) actualVal := repl.Replace(placeholder, "") if actualVal != expectVal { - t.Errorf("Test %d [%v]: Expected placeholder {http.matchers.header_regexp.%s} to be '%s' but got '%s'", + t.Errorf("Test %d [%v]: Expected placeholder {matchers.header_regexp.%s} to be '%s' but got '%s'", i, tc.match, key, expectVal, actualVal) continue } diff --git a/modules/caddyhttp/replacer.go b/modules/caddyhttp/replacer.go index 1fd34281..e7aa250e 100644 --- a/modules/caddyhttp/replacer.go +++ b/modules/caddyhttp/replacer.go @@ -93,19 +93,20 @@ func (r *Replacer) defaults() map[string]string { m["request.uri"] = r.req.URL.RequestURI() m["request.uri.path"] = r.req.URL.Path - // TODO: why should header fields, cookies, and query params get special treatment like this? - // maybe they should be scoped by words like "request.header." just like everything else. for field, vals := range r.req.Header { - m[">"+strings.ToLower(field)] = strings.Join(vals, ",") - } - for field, vals := range r.resp.Header() { - m["<"+strings.ToLower(field)] = strings.Join(vals, ",") + m["request.header."+strings.ToLower(field)] = strings.Join(vals, ",") } for _, cookie := range r.req.Cookies() { - m["~"+cookie.Name] = cookie.Value + m["request.cookie."+cookie.Name] = cookie.Value } for param, vals := range r.req.URL.Query() { - m["?"+param] = strings.Join(vals, ",") + m["request.uri.query."+param] = strings.Join(vals, ",") + } + } + + if r.resp != nil { + for field, vals := range r.resp.Header() { + m["response.header."+strings.ToLower(field)] = strings.Join(vals, ",") } } diff --git a/modules/caddyhttp/staticresp.go b/modules/caddyhttp/staticresp.go index 408de606..506689af 100644 --- a/modules/caddyhttp/staticresp.go +++ b/modules/caddyhttp/staticresp.go @@ -25,7 +25,7 @@ type Static struct { func (s Static) ServeHTTP(w http.ResponseWriter, r *http.Request) error { repl := r.Context().Value(ReplacerCtxKey).(*Replacer) - // close the connection + // close the connection after responding r.Close = s.Close // set all headers, with replacements