startupshutdown: is an alias for 'on' (#1880)

startupshutdown/startupshutdown.go

@@ -15,12 +15,12 @@
 package startupshutdown
 
 import (
-	"log"
+	"fmt"
-	"os"
-	"os/exec"
 	"strings"
 
+	"github.com/google/uuid"
 	"github.com/mholt/caddy"
+	"github.com/mholt/caddy/onevent/hook"
 )
 
 func init() {
@@ -28,60 +28,68 @@ func init() {
 	caddy.RegisterPlugin("shutdown", caddy.Plugin{Action: Shutdown})
 }
 
-// Startup registers a startup callback to execute during server start.
+// Startup (an alias for 'on startup') registers a startup callback to execute during server start.
 func Startup(c *caddy.Controller) error {
-	return registerCallback(c, c.OnFirstStartup)
+	config, err := onParse(c, caddy.InstanceStartupEvent)
-}
+	if err != nil {
-
+		return c.ArgErr()
-// Shutdown registers a shutdown callback to execute during server stop.
-func Shutdown(c *caddy.Controller) error {
-	return registerCallback(c, c.OnFinalShutdown)
-}
-
-// registerCallback registers a callback function to execute by
-// using c to parse the directive. It registers the callback
-// to be executed using registerFunc.
-func registerCallback(c *caddy.Controller, registerFunc func(func() error)) error {
-	var funcs []func() error
-
-	for c.Next() {
-		args := c.RemainingArgs()
-		if len(args) == 0 {
-			return c.ArgErr()
-		}
-
-		nonblock := false
-		if len(args) > 1 && args[len(args)-1] == "&" {
-			// Run command in background; non-blocking
-			nonblock = true
-			args = args[:len(args)-1]
-		}
-
-		command, args, err := caddy.SplitCommandAndArgs(strings.Join(args, " "))
-		if err != nil {
-			return c.Err(err.Error())
-		}
-
-		fn := func() error {
-			cmd := exec.Command(command, args...)
-			cmd.Stdin = os.Stdin
-			cmd.Stdout = os.Stdout
-			cmd.Stderr = os.Stderr
-			if nonblock {
-				log.Printf("[INFO] Nonblocking Command:\"%s %s\"", command, strings.Join(args, " "))
-				return cmd.Start()
-			}
-			log.Printf("[INFO] Blocking Command:\"%s %s\"", command, strings.Join(args, " "))
-			return cmd.Run()
-		}
-
-		funcs = append(funcs, fn)
 	}
 
-	return c.OncePerServerBlock(func() error {
+	// Register Event Hooks.
-		for _, fn := range funcs {
+	for _, cfg := range config {
-			registerFunc(fn)
+		caddy.RegisterEventHook("on-"+cfg.ID, cfg.Hook)
-		}
+	}
-		return nil
+
-	})
+	fmt.Println("NOTICE: Startup directive will be removed in a later version. Please migrate to 'on startup'")
+
+	return nil
+}
+
+// Shutdown (an alias for 'on shutdown') registers a shutdown callback to execute during server start.
+func Shutdown(c *caddy.Controller) error {
+	config, err := onParse(c, caddy.ShutdownEvent)
+	if err != nil {
+		return c.ArgErr()
+	}
+
+	// Register Event Hooks.
+	for _, cfg := range config {
+		caddy.RegisterEventHook("on-"+cfg.ID, cfg.Hook)
+	}
+
+	fmt.Println("NOTICE: Shutdown directive will be removed in a later version. Please migrate to 'on shutdown'")
+
+	return nil
+}
+
+func onParse(c *caddy.Controller, event caddy.EventName) ([]*hook.Config, error) {
+	var config []*hook.Config
+
+	for c.Next() {
+		cfg := new(hook.Config)
+
+		args := c.RemainingArgs()
+		if len(args) == 0 {
+			return config, c.ArgErr()
+		}
+
+		// Configure Event.
+		cfg.Event = event
+
+		// Assign an unique ID.
+		cfg.ID = uuid.New().String()
+
+		// Extract command and arguments.
+		command, args, err := caddy.SplitCommandAndArgs(strings.Join(args, " "))
+		if err != nil {
+			return config, c.Err(err.Error())
+		}
+
+		cfg.Command = command
+		cfg.Args = args
+
+		config = append(config, cfg)
+	}
+
+	return config, nil
 }

startupshutdown/startupshutdown_test.go

@@ -15,66 +15,55 @@
 package startupshutdown
 
 import (
-	"os"
-	"path/filepath"
-	"strconv"
 	"testing"
-	"time"
 
 	"github.com/mholt/caddy"
 )
 
-// The Startup function's tests are symmetrical to Shutdown tests,
-// because the Startup and Shutdown functions share virtually the
-// same functionality
 func TestStartup(t *testing.T) {
-	tempDirPath := os.TempDir()
-
-	testDir := filepath.Join(tempDirPath, "temp_dir_for_testing_startupshutdown")
-	defer func() {
-		// clean up after non-blocking startup function quits
-		time.Sleep(500 * time.Millisecond)
-		os.RemoveAll(testDir)
-	}()
-	osSenitiveTestDir := filepath.FromSlash(testDir)
-	os.RemoveAll(osSenitiveTestDir) // start with a clean slate
-
-	var registeredFunction func() error
-	fakeRegister := func(fn func() error) {
-		registeredFunction = fn
-	}
-
 	tests := []struct {
-		input              string
+		name      string
-		shouldExecutionErr bool
+		input     string
-		shouldRemoveErr    bool
+		shouldErr bool
 	}{
-		// test case #0 tests proper functionality blocking commands
+		{name: "noInput", input: "startup", shouldErr: true},
-		{"startup mkdir " + osSenitiveTestDir, false, false},
+		{name: "startup", input: "startup cmd arg", shouldErr: false},
-
-		// test case #1 tests proper functionality of non-blocking commands
-		{"startup mkdir " + osSenitiveTestDir + " &", false, true},
-
-		// test case #2 tests handling of non-existent commands
-		{"startup " + strconv.Itoa(int(time.Now().UnixNano())), true, true},
 	}
 
-	for i, test := range tests {
+	for _, test := range tests {
-		c := caddy.NewTestController("", test.input)
+		t.Run(test.name, func(t *testing.T) {
-		err := registerCallback(c, fakeRegister)
+			c := caddy.NewTestController("", test.input)
-		if err != nil {
+
-			t.Errorf("Expected no errors, got: %v", err)
+			err := Startup(c)
-		}
+			if err == nil && test.shouldErr {
-		if registeredFunction == nil {
+				t.Error("Test didn't error, but it should have")
-			t.Fatalf("Expected function to be registered, but it wasn't")
+			} else if err != nil && !test.shouldErr {
-		}
+				t.Errorf("Test errored, but it shouldn't have; got '%v'", err)
-		err = registeredFunction()
+			}
-		if err != nil && !test.shouldExecutionErr {
+		})
-			t.Errorf("Test %d received an error of:\n%v", i, err)
-		}
-		err = os.Remove(osSenitiveTestDir)
-		if err != nil && !test.shouldRemoveErr {
-			t.Errorf("Test %d received an error of:\n%v", i, err)
-		}
 	}
 }
+
+func TestShutdown(t *testing.T) {
+	tests := []struct {
+		name      string
+		input     string
+		shouldErr bool
+	}{
+		{name: "noInput", input: "shutdown", shouldErr: true},
+		{name: "shutdown", input: "shutdown cmd arg", shouldErr: false},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			c := caddy.NewTestController("", test.input)
+
+			err := Shutdown(c)
+			if err == nil && test.shouldErr {
+				t.Error("Test didn't error, but it should have")
+			} else if err != nil && !test.shouldErr {
+				t.Errorf("Test errored, but it shouldn't have; got '%v'", err)
+			}
+		})
+	}
+}