Skip to content

Commit 8fc47be

Browse files
committed
Get rid of lookup shunt
1 parent 06d612a commit 8fc47be

4 files changed

Lines changed: 70 additions & 166 deletions

File tree

internal/jobexecutor/job_executor_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -907,7 +907,7 @@ func TestJobExecutor_Execute(t *testing.T) {
907907
executor, bundle := setup(t)
908908

909909
// Add a middleware so we can verify it's in the trace too:
910-
executor.PluginLookupGlobal = pluginlookup.NewPluginLookup([]rivertype.Plugin{
910+
executor.PluginLookupGlobal = pluginlookup.NewPluginLookup([]any{
911911
&testMiddleware{
912912
work: func(ctx context.Context, job *rivertype.JobRow, next func(context.Context) error) error {
913913
return next(ctx)
@@ -1078,7 +1078,7 @@ func TestJobExecutor_Execute(t *testing.T) {
10781078
workBeginCalled bool
10791079
workEndCalled bool
10801080
)
1081-
executor.PluginLookupGlobal = pluginlookup.NewPluginLookup([]rivertype.Plugin{
1081+
executor.PluginLookupGlobal = pluginlookup.NewPluginLookup([]any{
10821082
HookWorkBeginFunc(func(ctx context.Context, job *rivertype.JobRow) error {
10831083
workBeginCalled = true
10841084
return nil
@@ -1108,7 +1108,7 @@ func TestJobExecutor_Execute(t *testing.T) {
11081108
workEnd1Called bool
11091109
workEnd2Called bool
11101110
)
1111-
executor.PluginLookupGlobal = pluginlookup.NewPluginLookup([]rivertype.Plugin{
1111+
executor.PluginLookupGlobal = pluginlookup.NewPluginLookup([]any{
11121112
HookWorkEndFunc(func(ctx context.Context, job *rivertype.JobRow, err error) error {
11131113
workEnd1Called = true
11141114
require.EqualError(t, err, "job error")
@@ -1142,7 +1142,7 @@ func TestJobExecutor_Execute(t *testing.T) {
11421142
workEnd1Called bool
11431143
workEnd2Called bool
11441144
)
1145-
executor.PluginLookupGlobal = pluginlookup.NewPluginLookup([]rivertype.Plugin{
1145+
executor.PluginLookupGlobal = pluginlookup.NewPluginLookup([]any{
11461146
HookWorkEndFunc(func(ctx context.Context, job *rivertype.JobRow, err error) error {
11471147
workEnd1Called = true
11481148
require.EqualError(t, err, "job error")

internal/maintenance/periodic_job_enqueuer_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1055,7 +1055,7 @@ func TestPeriodicJobEnqueuer(t *testing.T) {
10551055
require.NoError(t, err)
10561056

10571057
var periodicJobsStartHookCalled bool
1058-
svc.Config.PluginLookupGlobal = pluginlookup.NewPluginLookup([]rivertype.Plugin{
1058+
svc.Config.PluginLookupGlobal = pluginlookup.NewPluginLookup([]any{
10591059
HookPeriodicJobsStartFunc(func(ctx context.Context, params *rivertype.HookPeriodicJobsStartParams) error {
10601060
periodicJobsStartHookCalled = true
10611061

@@ -1117,7 +1117,7 @@ func TestPeriodicJobEnqueuer(t *testing.T) {
11171117
}
11181118

11191119
var periodicJobsStartHookCalled bool
1120-
svc.Config.PluginLookupGlobal = pluginlookup.NewPluginLookup([]rivertype.Plugin{
1120+
svc.Config.PluginLookupGlobal = pluginlookup.NewPluginLookup([]any{
11211121
HookPeriodicJobsStartFunc(func(ctx context.Context, params *rivertype.HookPeriodicJobsStartParams) error {
11221122
periodicJobsStartHookCalled = true
11231123

@@ -1182,7 +1182,7 @@ func TestPeriodicJobEnqueuer(t *testing.T) {
11821182

11831183
svc, _ := setup(t)
11841184

1185-
svc.Config.PluginLookupGlobal = pluginlookup.NewPluginLookup([]rivertype.Plugin{
1185+
svc.Config.PluginLookupGlobal = pluginlookup.NewPluginLookup([]any{
11861186
HookPeriodicJobsStartFunc(func(ctx context.Context, params *rivertype.HookPeriodicJobsStartParams) error {
11871187
return errors.New("hook start error")
11881188
}),

internal/pluginlookup/plugin_lookup.go

Lines changed: 42 additions & 138 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package pluginlookup
22

33
import (
4-
"context"
54
"reflect"
65
"sync"
76

@@ -24,80 +23,67 @@ const (
2423
PluginKindMiddlewareWorker PluginKind = "middleware_worker"
2524
)
2625

27-
// InitBaseServices initializes base services embedded in plugins, including
28-
// those hidden behind wrappers for legacy hooks and middleware.
29-
func InitBaseServices(archetype *baseservice.Archetype, plugins []rivertype.Plugin) {
26+
// InitBaseServices initializes base services embedded in normalized plugins,
27+
// hooks, and middleware.
28+
func InitBaseServices(archetype *baseservice.Archetype, plugins []any) {
3029
for _, plugin := range plugins {
31-
if legacyPlugin, ok := plugin.(*legacyPlugin); ok {
32-
legacyPlugin.initBaseService(archetype)
33-
continue
34-
}
35-
3630
if withBaseService, ok := plugin.(baseservice.WithBaseService); ok {
3731
baseservice.Init(archetype, withBaseService)
3832
}
3933
}
4034
}
4135

42-
// NormalizePlugins converts hook, middleware, and plugin registrations into a
43-
// single plugin slice while preserving legacy hook and middleware registrations
44-
// that don't yet opt into Plugin.
36+
// NormalizePlugins combines configured hooks, middleware, and plugins into a
37+
// single plugin slice while preserving legacy hooks and middleware that don't
38+
// yet opt into Plugin.
4539
//
4640
// Plugins passed explicitly are ordered before hooks and middleware, so
4741
// Config.Plugins entries take precedence over plugins bridged from Config.Hooks
4842
// or Config.Middleware. Relative order is preserved within each input slice.
49-
// Repeated registrations of the same non-zero-sized pointer are collapsed so a
50-
// legacy hybrid registered as both a hook and middleware runs only once for
51-
// each operation. Registrations repeated within one input slice are preserved.
52-
func NormalizePlugins(hooks []rivertype.Hook, middlewares []rivertype.Middleware, plugins []rivertype.Plugin) []rivertype.Plugin {
43+
// Repeated occurrences of the same non-zero-sized pointer are collapsed so a
44+
// legacy hybrid configured as both a hook and middleware runs only once for
45+
// each operation. Repeated values within one input slice are preserved.
46+
func NormalizePlugins(hooks []rivertype.Hook, middlewares []rivertype.Middleware, plugins []rivertype.Plugin) []any {
5347
var (
54-
normalizedPlugins = make([]rivertype.Plugin, 0, len(hooks)+len(middlewares)+len(plugins))
48+
normalizedPlugins = make([]any, 0, len(hooks)+len(middlewares)+len(plugins))
5549
seenPointers = make(map[pluginPointerIdentity]struct{})
5650
)
5751

58-
appendGroup := func(group []pluginRegistration) {
52+
appendGroup := func(group []any) {
5953
groupPointers := make(map[pluginPointerIdentity]struct{})
60-
for _, registration := range group {
61-
if identity, ok := pluginPointerIdentityFor(registration.original); ok {
54+
for _, plugin := range group {
55+
if identity, ok := pluginPointerIdentityFor(plugin); ok {
6256
if _, ok := seenPointers[identity]; ok {
6357
continue
6458
}
6559
groupPointers[identity] = struct{}{}
6660
}
6761

68-
normalizedPlugins = append(normalizedPlugins, registration.plugin)
62+
normalizedPlugins = append(normalizedPlugins, plugin)
6963
}
7064

7165
for identity := range groupPointers {
7266
seenPointers[identity] = struct{}{}
7367
}
7468
}
7569

76-
pluginRegistrations := make([]pluginRegistration, 0, len(plugins))
70+
pluginGroup := make([]any, 0, len(plugins))
7771
for _, plugin := range plugins {
78-
pluginRegistrations = append(pluginRegistrations, pluginRegistration{original: plugin, plugin: plugin})
72+
pluginGroup = append(pluginGroup, plugin)
7973
}
80-
appendGroup(pluginRegistrations)
74+
appendGroup(pluginGroup)
8175

82-
hookRegistrations := make([]pluginRegistration, 0, len(hooks))
76+
hookGroup := make([]any, 0, len(hooks))
8377
for _, hook := range hooks {
84-
if plugin, ok := hook.(rivertype.Plugin); ok {
85-
hookRegistrations = append(hookRegistrations, pluginRegistration{original: hook, plugin: plugin})
86-
} else {
87-
hookRegistrations = append(hookRegistrations, pluginRegistration{original: hook, plugin: newLegacyPlugin(hook)})
88-
}
78+
hookGroup = append(hookGroup, hook)
8979
}
90-
appendGroup(hookRegistrations)
80+
appendGroup(hookGroup)
9181

92-
middlewareRegistrations := make([]pluginRegistration, 0, len(middlewares))
82+
middlewareGroup := make([]any, 0, len(middlewares))
9383
for _, middleware := range middlewares {
94-
if plugin, ok := middleware.(rivertype.Plugin); ok {
95-
middlewareRegistrations = append(middlewareRegistrations, pluginRegistration{original: middleware, plugin: plugin})
96-
} else {
97-
middlewareRegistrations = append(middlewareRegistrations, pluginRegistration{original: middleware, plugin: newLegacyPlugin(middleware)})
98-
}
84+
middlewareGroup = append(middlewareGroup, middleware)
9985
}
100-
appendGroup(middlewareRegistrations)
86+
appendGroup(middlewareGroup)
10187

10288
return normalizedPlugins
10389
}
@@ -110,46 +96,47 @@ func NormalizePlugins(hooks []rivertype.Hook, middlewares []rivertype.Middleware
11096
// PluginLookup, but may also be EmptyPluginLookup as a memory allocation
11197
// optimization for bundles where no plugins are present.
11298
type PluginLookupInterface interface {
113-
ByKind(kind PluginKind) []rivertype.Plugin
99+
ByKind(kind PluginKind) []any
114100
}
115101

116102
// NewPluginLookup returns a new plugin lookup interface based on the given
117103
// plugins that satisfies PluginLookupInterface. This is often pluginLookup,
118104
// but may be emptyPluginLookup as an optimization for the common case of an
119105
// empty plugin bundle.
120-
func NewPluginLookup(plugins []rivertype.Plugin) PluginLookupInterface {
106+
//
107+
// The plugins parameter is []any rather than []rivertype.Plugin because the
108+
// normalized plugin list may contain legacy hooks and middleware that don't
109+
// implement rivertype.Plugin. Keeping their original concrete values avoids
110+
// compatibility wrappers that would need to forward every operation-specific
111+
// interface, along with auxiliary interfaces like baseservice.WithBaseService.
112+
func NewPluginLookup(plugins []any) PluginLookupInterface {
121113
if len(plugins) < 1 {
122114
return &emptyPluginLookup{}
123115
}
124116

125-
pluginsByKind := make(map[PluginKind][]rivertype.Plugin)
117+
pluginsByKind := make(map[PluginKind][]any)
126118

127119
for _, plugin := range plugins {
128120
if plugin == nil {
129121
continue
130122
}
131123

132-
extension := any(plugin)
133-
if legacyPlugin, ok := plugin.(*legacyPlugin); ok {
134-
extension = legacyPlugin.extension
135-
}
136-
137-
if _, ok := extension.(rivertype.HookInsertBegin); ok {
124+
if _, ok := plugin.(rivertype.HookInsertBegin); ok {
138125
pluginsByKind[PluginKindHookInsertBegin] = append(pluginsByKind[PluginKindHookInsertBegin], plugin)
139126
}
140-
if _, ok := extension.(rivertype.HookPeriodicJobsStart); ok {
127+
if _, ok := plugin.(rivertype.HookPeriodicJobsStart); ok {
141128
pluginsByKind[PluginKindHookPeriodicJobsStart] = append(pluginsByKind[PluginKindHookPeriodicJobsStart], plugin)
142129
}
143-
if _, ok := extension.(rivertype.HookWorkBegin); ok {
130+
if _, ok := plugin.(rivertype.HookWorkBegin); ok {
144131
pluginsByKind[PluginKindHookWorkBegin] = append(pluginsByKind[PluginKindHookWorkBegin], plugin)
145132
}
146-
if _, ok := extension.(rivertype.HookWorkEnd); ok {
133+
if _, ok := plugin.(rivertype.HookWorkEnd); ok {
147134
pluginsByKind[PluginKindHookWorkEnd] = append(pluginsByKind[PluginKindHookWorkEnd], plugin)
148135
}
149-
if _, ok := extension.(rivertype.JobInsertMiddleware); ok {
136+
if _, ok := plugin.(rivertype.JobInsertMiddleware); ok {
150137
pluginsByKind[PluginKindMiddlewareJobInsert] = append(pluginsByKind[PluginKindMiddlewareJobInsert], plugin)
151138
}
152-
if _, ok := extension.(rivertype.WorkerMiddleware); ok {
139+
if _, ok := plugin.(rivertype.WorkerMiddleware); ok {
153140
pluginsByKind[PluginKindMiddlewareWorker] = append(pluginsByKind[PluginKindMiddlewareWorker], plugin)
154141
}
155142
}
@@ -167,10 +154,10 @@ func NewPluginLookup(plugins []rivertype.Plugin) PluginLookupInterface {
167154
// for globally installed plugins or plugins for specific job kinds through the
168155
// use of JobPluginLookup.
169156
type pluginLookup struct {
170-
pluginsByKind map[PluginKind][]rivertype.Plugin
157+
pluginsByKind map[PluginKind][]any
171158
}
172159

173-
func (c *pluginLookup) ByKind(kind PluginKind) []rivertype.Plugin {
160+
func (c *pluginLookup) ByKind(kind PluginKind) []any {
174161
return c.pluginsByKind[kind]
175162
}
176163

@@ -184,84 +171,10 @@ func (c *pluginLookup) ByKind(kind PluginKind) []rivertype.Plugin {
184171
// that go unused.
185172
type emptyPluginLookup struct{}
186173

187-
func (c *emptyPluginLookup) ByKind(kind PluginKind) []rivertype.Plugin {
174+
func (c *emptyPluginLookup) ByKind(kind PluginKind) []any {
188175
return nil
189176
}
190177

191-
//
192-
// legacyPlugin
193-
//
194-
195-
// legacyPlugin wraps a legacy hook or middleware so it can participate in
196-
// plugin lookup. It forwards both hook and middleware capabilities so a legacy
197-
// extension that implements both still works when registered through either
198-
// legacy config field.
199-
type legacyPlugin struct {
200-
extension any
201-
}
202-
203-
func newLegacyPlugin(extension any) rivertype.Plugin {
204-
return &legacyPlugin{extension: extension}
205-
}
206-
207-
func (p *legacyPlugin) initBaseService(archetype *baseservice.Archetype) {
208-
if withBaseService, ok := p.extension.(baseservice.WithBaseService); ok {
209-
baseservice.Init(archetype, withBaseService)
210-
}
211-
}
212-
213-
func (p *legacyPlugin) IsHook() bool { return true }
214-
func (p *legacyPlugin) IsMiddleware() bool { return true }
215-
func (p *legacyPlugin) IsPlugin() bool { return true }
216-
217-
func (p *legacyPlugin) InsertBegin(ctx context.Context, params *rivertype.JobInsertParams) error {
218-
hook, ok := p.extension.(rivertype.HookInsertBegin)
219-
if !ok {
220-
return nil
221-
}
222-
return hook.InsertBegin(ctx, params)
223-
}
224-
225-
func (p *legacyPlugin) InsertMany(ctx context.Context, manyParams []*rivertype.JobInsertParams, doInner func(context.Context) ([]*rivertype.JobInsertResult, error)) ([]*rivertype.JobInsertResult, error) {
226-
middleware, ok := p.extension.(rivertype.JobInsertMiddleware)
227-
if !ok {
228-
return doInner(ctx)
229-
}
230-
return middleware.InsertMany(ctx, manyParams, doInner)
231-
}
232-
233-
func (p *legacyPlugin) Start(ctx context.Context, params *rivertype.HookPeriodicJobsStartParams) error {
234-
hook, ok := p.extension.(rivertype.HookPeriodicJobsStart)
235-
if !ok {
236-
return nil
237-
}
238-
return hook.Start(ctx, params)
239-
}
240-
241-
func (p *legacyPlugin) Work(ctx context.Context, job *rivertype.JobRow, doInner func(context.Context) error) error {
242-
middleware, ok := p.extension.(rivertype.WorkerMiddleware)
243-
if !ok {
244-
return doInner(ctx)
245-
}
246-
return middleware.Work(ctx, job, doInner)
247-
}
248-
249-
func (p *legacyPlugin) WorkBegin(ctx context.Context, job *rivertype.JobRow) error {
250-
hook, ok := p.extension.(rivertype.HookWorkBegin)
251-
if !ok {
252-
return nil
253-
}
254-
return hook.WorkBegin(ctx, job)
255-
}
256-
257-
func (p *legacyPlugin) WorkEnd(ctx context.Context, job *rivertype.JobRow, err error) error {
258-
hook, ok := p.extension.(rivertype.HookWorkEnd)
259-
if !ok {
260-
return err
261-
}
262-
return hook.WorkEnd(ctx, job, err)
263-
}
264-
265178
//
266179
// pluginPointerIdentity
267180
//
@@ -284,15 +197,6 @@ func pluginPointerIdentityFor(plugin any) (pluginPointerIdentity, bool) {
284197
return pluginPointerIdentity{pointer: value.Pointer(), typeOf: value.Type()}, true
285198
}
286199

287-
//
288-
// pluginRegistration
289-
//
290-
291-
type pluginRegistration struct {
292-
original any
293-
plugin rivertype.Plugin
294-
}
295-
296200
//
297201
// JobPluginLookup
298202
//

0 commit comments

Comments
 (0)