refactor: dedup admin entity handlers and server construction

1 files changed, 59 insertions, 38 deletions

diff --git a/internal/admin/server.go b/internal/admin/server.go
index f35be84..4cceadd 100644
--- a/internal/admin/server.go
+++ b/internal/admin/server.go
@@ -91,8 +91,29 @@ type AdminServer struct {
 	loginLimiter     *loginLimiter
 }
 
-func NewServer(cfg *config.Config, useTLS bool, accountStore *player.AccountStore, w *world.World, is *item.ItemStore, os *object.ObjectStore, ms *world.MobStore, dataDir string, reloadCourses func(), rewriteRoomIDs func(map[int]int), evictRoom func(roomID int), rebuildCraftIndex func(), configPath string, runOnOwner func(func()) bool) (*AdminServer, error) {
-	if runOnOwner == nil {
+// ServerDeps are the long-lived stores and wired callbacks the admin server
+// uses. It replaces a 15-argument positional constructor in which every
+// callback had the same func() shape, so a mis-ordered call compiled silently
+// and only failed at runtime.
+type ServerDeps struct {
+	Config            *config.Config
+	UseTLS            bool
+	Accounts          *player.AccountStore
+	World             *world.World
+	Items             *item.ItemStore
+	Objects           *object.ObjectStore
+	Mobs              *world.MobStore
+	DataDir           string
+	ReloadCourses     func()
+	RewriteRoomIDs    func(map[int]int)
+	EvictRoom         func(roomID int)
+	RebuildCraftIndex func()
+	ConfigPath        string
+	RunOnOwner        func(func()) bool
+}
+
+func NewServer(deps ServerDeps) (*AdminServer, error) {
+	if deps.RunOnOwner == nil {
 		return nil, fmt.Errorf("admin server requires an owner dispatcher")
 	}
 	secret := make([]byte, 32)
@@ -111,42 +132,42 @@ func NewServer(cfg *config.Config, useTLS bool, accountStore *player.AccountStor
 	}
 
 	var port int
-	if useTLS {
-		port = cfg.AdminHTTPS.Port
+	if deps.UseTLS {
+		port = deps.Config.AdminHTTPS.Port
 	} else {
-		port = cfg.AdminHTTP.Port
+		port = deps.Config.AdminHTTP.Port
 	}
 
 	var cert tls.Certificate
-	if useTLS {
-		cert, err = loadOrGenerateCert(cfg)
+	if deps.UseTLS {
+		cert, err = loadOrGenerateCert(deps.Config)
 		if err != nil {
 			return nil, fmt.Errorf("admin cert: %w", err)
 		}
 	}
 
 	s := &AdminServer{
-		cfg:               cfg,
-		useTLS:            useTLS,
-		accountStore:      accountStore,
-		world:             w,
-		itemStore:         is,
-		objectStore:       os,
-		mobStore:          ms,
-		dataDir:           dataDir,
-		undoStack:         NewUndoStack(dataDir),
+		cfg:               deps.Config,
+		useTLS:            deps.UseTLS,
+		accountStore:      deps.Accounts,
+		world:             deps.World,
+		itemStore:         deps.Items,
+		objectStore:       deps.Objects,
+		mobStore:          deps.Mobs,
+		dataDir:           deps.DataDir,
+		undoStack:         NewUndoStack(deps.DataDir),
 		tmpl:              tmpl,
 		cookieSecret:      secret,
-		reloadCourses:     reloadCourses,
-		rewriteRoomIDs:    rewriteRoomIDs,
-		evictRoom:         evictRoom,
-		rebuildCraftIndex: rebuildCraftIndex,
-		configPath:        configPath,
-		runOnOwner:        runOnOwner,
+		reloadCourses:     deps.ReloadCourses,
+		rewriteRoomIDs:    deps.RewriteRoomIDs,
+		evictRoom:         deps.EvictRoom,
+		rebuildCraftIndex: deps.RebuildCraftIndex,
+		configPath:        deps.ConfigPath,
+		runOnOwner:        deps.RunOnOwner,
 	}
 	// Undo/redo of room-ID changes applies the (inverse) runtime migration
 	// through the same wired callback the rename uses.
-	s.undoStack.SetMigrateRoomIDs(rewriteRoomIDs)
+	s.undoStack.SetMigrateRoomIDs(deps.RewriteRoomIDs)
 
 	mux := http.NewServeMux()
 
@@ -194,38 +215,38 @@ func NewServer(cfg *config.Config, useTLS bool, accountStore *player.AccountStor
 	apiMux.HandleFunc("/api/objects/rename", s.handleEntityRename("objects"))
 	apiMux.HandleFunc("/api/objects/move", s.handleEntityMove("objects"))
 	apiMux.HandleFunc("/api/objects/dirs", s.handleEntityDirs("objects"))
-	apiMux.HandleFunc("/api/objects", s.handleObjects)
-	apiMux.HandleFunc("/api/objects/", s.handleObjectByID)
+	apiMux.HandleFunc("/api/objects", s.handleEntityList(objectSpec))
+	apiMux.HandleFunc("/api/objects/", s.handleEntityByID(objectSpec))
 	apiMux.HandleFunc("/api/items/rename", s.handleEntityRename("items"))
 	apiMux.HandleFunc("/api/items/move", s.handleEntityMove("items"))
 	apiMux.HandleFunc("/api/items/dirs", s.handleEntityDirs("items"))
-	apiMux.HandleFunc("/api/items", s.handleItems)
-	apiMux.HandleFunc("/api/items/", s.handleItemByID)
+	apiMux.HandleFunc("/api/items", s.handleEntityList(itemSpec))
+	apiMux.HandleFunc("/api/items/", s.handleEntityByID(itemSpec))
 	apiMux.HandleFunc("/api/mobs/rename", s.handleEntityRename("mobs"))
 	apiMux.HandleFunc("/api/mobs/move", s.handleEntityMove("mobs"))
 	apiMux.HandleFunc("/api/mobs/dirs", s.handleEntityDirs("mobs"))
-	apiMux.HandleFunc("/api/mobs", s.handleMobs)
-	apiMux.HandleFunc("/api/mobs/", s.handleMobByID)
+	apiMux.HandleFunc("/api/mobs", s.handleEntityList(mobSpec))
+	apiMux.HandleFunc("/api/mobs/", s.handleEntityByID(mobSpec))
 	apiMux.HandleFunc("/api/drops/rename", s.handleEntityRename("drops"))
 	apiMux.HandleFunc("/api/drops/move", s.handleEntityMove("drops"))
 	apiMux.HandleFunc("/api/drops/dirs", s.handleEntityDirs("drops"))
-	apiMux.HandleFunc("/api/drops", s.handleDrops)
-	apiMux.HandleFunc("/api/drops/", s.handleDropByID)
+	apiMux.HandleFunc("/api/drops", s.handleEntityList(dropSpec))
+	apiMux.HandleFunc("/api/drops/", s.handleEntityByID(dropSpec))
 	apiMux.HandleFunc("/api/hazards/rename", s.handleEntityRename("hazards"))
 	apiMux.HandleFunc("/api/hazards/move", s.handleEntityMove("hazards"))
 	apiMux.HandleFunc("/api/hazards/dirs", s.handleEntityDirs("hazards"))
-	apiMux.HandleFunc("/api/hazards", s.handleHazards)
-	apiMux.HandleFunc("/api/hazards/", s.handleHazardByID)
+	apiMux.HandleFunc("/api/hazards", s.handleEntityList(hazardSpec))
+	apiMux.HandleFunc("/api/hazards/", s.handleEntityByID(hazardSpec))
 	apiMux.HandleFunc("/api/techs/rename", s.handleEntityRename("techs"))
 	apiMux.HandleFunc("/api/techs/move", s.handleEntityMove("techs"))
 	apiMux.HandleFunc("/api/techs/dirs", s.handleEntityDirs("techs"))
-	apiMux.HandleFunc("/api/techs", s.handleTechs)
-	apiMux.HandleFunc("/api/techs/", s.handleTechByID)
+	apiMux.HandleFunc("/api/techs", s.handleEntityList(techSpec))
+	apiMux.HandleFunc("/api/techs/", s.handleEntityByID(techSpec))
 	apiMux.HandleFunc("/api/modules/rename", s.handleEntityRename("modules"))
 	apiMux.HandleFunc("/api/modules/move", s.handleEntityMove("modules"))
 	apiMux.HandleFunc("/api/modules/dirs", s.handleEntityDirs("modules"))
-	apiMux.HandleFunc("/api/modules", s.handleModules)
-	apiMux.HandleFunc("/api/modules/", s.handleModuleByID)
+	apiMux.HandleFunc("/api/modules", s.handleEntityList(moduleSpec))
+	apiMux.HandleFunc("/api/modules/", s.handleEntityByID(moduleSpec))
 	apiMux.HandleFunc("/api/courses", s.handleCourses)
 	apiMux.HandleFunc("/api/courses/", s.handleCourseByID)
 	apiMux.HandleFunc("/api/players", s.handlePlayers)
@@ -313,7 +334,7 @@ func NewServer(cfg *config.Config, useTLS bool, accountStore *player.AccountStor
 		WriteTimeout:      60 * time.Second,
 		IdleTimeout:       60 * time.Second,
 	}
-	if useTLS {
+	if deps.UseTLS {
 		s.httpServer.TLSConfig = &tls.Config{
 			Certificates: []tls.Certificate{cert},
 			MinVersion:   tls.VersionTLS12,