Some additional cleanup

Author: brandur

client.go

@@ -191,7 +191,7 @@ type Config struct {
 	// JobInsertMiddleware are optional functions that can be called around job
 	// insertion.
 	//
-	// Deprecated: Prefer the use of Middleware instead (which may contain
+	// Deprecated: Prefer the use of Plugins instead (which may contain
 	// instances of rivertype.JobInsertMiddleware).
 	JobInsertMiddleware []rivertype.JobInsertMiddleware
 
@@ -446,7 +446,7 @@ type Config struct {
 	// WorkerMiddleware are optional functions that can be called around
 	// all job executions.
 	//
-	// Deprecated: Prefer the use of Middleware instead (which may contain
+	// Deprecated: Prefer the use of Plugins instead (which may contain
 	// instances of rivertype.WorkerMiddleware).
 	WorkerMiddleware []rivertype.WorkerMiddleware
 
@@ -827,9 +827,9 @@ func NewClient[TTx any](driver riverdriver.Driver[TTx], config *Config) (*Client
 	}
 
 	var (
-		configuredMiddleware = pluginconfig.Middleware(config.Middleware, config.JobInsertMiddleware, config.WorkerMiddleware)
-		plugins              = append(riverplugin.DefaultPlugins(), config.Plugins...)
-		allPlugins           = pluginlookup.NormalizePlugins(config.Hooks, configuredMiddleware, plugins)
+		middleware = pluginconfig.CombinedMiddleware(config.Middleware, config.JobInsertMiddleware, config.WorkerMiddleware)
+		plugins    = append(riverplugin.DefaultPlugins(), config.Plugins...)
+		allPlugins = pluginlookup.NormalizePlugins(config.Hooks, middleware, plugins)
 	)
 	pluginlookup.InitBaseServices(archetype, allPlugins)
 

internal/pluginconfig/plugin_config.go

@@ -2,11 +2,18 @@ package pluginconfig
 
 import "github.com/riverqueue/river/rivertype"
 
-// Middleware combines middleware from the current and legacy configuration
-// fields into one list. Explicit middleware is preserved first, followed by
-// legacy job insert and worker middleware. A worker middleware already present
-// in the job insert list is included only once.
-func Middleware(middleware []rivertype.Middleware, jobInsertMiddleware []rivertype.JobInsertMiddleware, workerMiddleware []rivertype.WorkerMiddleware) []rivertype.Middleware {
+// CombinedMiddleware combines middleware from the current and legacy
+// configuration fields into one list. Explicit middleware is preserved first,
+// followed by legacy job insert and worker middleware.
+//
+// The reason this exists is that River originally had a seprate configuration
+// field for each type of middleware (JobInsertMiddleware, WorkerMiddleware)
+// before it was unified to a combined Middleware field so that a new one
+// wouldn't be needed every time a new type of middleware was added. Middleware
+// then became Plugins. The JobInsertMiddleware and WorkerMiddleware fields have
+// been deprecated for quite some time and we should consider just removing them
+// to simplify this set up.
+func CombinedMiddleware(middleware []rivertype.Middleware, jobInsertMiddleware []rivertype.JobInsertMiddleware, workerMiddleware []rivertype.WorkerMiddleware) []rivertype.Middleware {
 	allMiddleware := make([]rivertype.Middleware, 0,
 		len(middleware)+len(jobInsertMiddleware)+len(workerMiddleware))
 	allMiddleware = append(allMiddleware, middleware...)

internal/pluginconfig/plugin_config_test.go

@@ -8,7 +8,7 @@ import (
 	"github.com/riverqueue/river/rivertype"
 )
 
-func TestMiddleware(t *testing.T) {
+func TestCombinedMiddleware(t *testing.T) {
 	t.Parallel()
 
 	t.Run("CombinesMiddlewareInOrder", func(t *testing.T) {
@@ -24,7 +24,7 @@ func TestMiddleware(t *testing.T) {
 			middleware,
 			jobInsertMiddleware,
 			workerMiddleware,
-		}, Middleware(
+		}, CombinedMiddleware(
 			[]rivertype.Middleware{middleware},
 			[]rivertype.JobInsertMiddleware{jobInsertMiddleware},
 			[]rivertype.WorkerMiddleware{workerMiddleware},
@@ -36,7 +36,7 @@ func TestMiddleware(t *testing.T) {
 
 		middleware := &testJobInsertAndWorkerMiddleware{}
 
-		require.Equal(t, []rivertype.Middleware{middleware}, Middleware(
+		require.Equal(t, []rivertype.Middleware{middleware}, CombinedMiddleware(
 			nil,
 			[]rivertype.JobInsertMiddleware{middleware},
 			[]rivertype.WorkerMiddleware{middleware},

rivertest/worker.go

@@ -148,17 +148,9 @@ func (w *Worker[T, TTx]) workJob(ctx context.Context, tb testing.TB, tx TTx, job
 	completer := jobcompleter.NewInlineCompleter(archetype, w.config.Schema, exec, w.client.Pilot(), subscribeCh)
 
 	var (
-		configuredMiddleware = pluginconfig.Middleware(
-			w.config.Middleware,          //nolint:staticcheck // Bridge legacy middleware into the test worker's unified plugin path.
-			w.config.JobInsertMiddleware, //nolint:staticcheck // Bridge legacy middleware into the test worker's unified plugin path.
-			w.config.WorkerMiddleware,    //nolint:staticcheck // Bridge legacy middleware into the test worker's unified plugin path.
-		)
+		middleware = pluginconfig.CombinedMiddleware(w.config.Middleware, w.config.JobInsertMiddleware, w.config.WorkerMiddleware) //nolint:staticcheck
 		plugins    = append(riverplugin.DefaultPlugins(), w.config.Plugins...)
-		allPlugins = pluginlookup.NormalizePlugins(
-			w.config.Hooks, //nolint:staticcheck // Bridge legacy Hooks into the test worker's unified plugin path.
-			configuredMiddleware,
-			plugins,
-		)
+		allPlugins = pluginlookup.NormalizePlugins(w.config.Hooks, middleware, plugins) //nolint:staticcheck
 	)
 	pluginlookup.InitBaseServices(archetype, allPlugins)
 

rivertype/river_type.go

@@ -451,9 +451,7 @@ type Middleware interface {
 //
 // Plugin structs should embed river.PluginDefaults, or embed either
 // river.HookDefaults or river.MiddlewareDefaults, then implement any
-// operation-specific hook or middleware interfaces they need. If a type embeds
-// both defaults directly, it must also implement IsPlugin itself to avoid the
-// embedded method-name collision.
+// operation-specific hook or middleware interfaces they need.
 type Plugin interface {
 	IsPlugin() bool
 }