Some minor updates, and get rid of OnLoad/OnUnload

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

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()

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
-		},
 	})
 }
 

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])
 		}
 	}

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, "<empty>")
 			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, "<empty>")
 			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
 			}

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, ",")
 		}
 	}
 

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