Skip to content

Commit 2d4834c

Browse files
authored
fix(org): resolve API handlers at request time to survive late plugin init (#9022)
Route registration can evaluate ApiResources() before InitPlugins() has run (InitPlugins is only invoked from pipelineServiceInit, which is delayed e.g. by a pending DB migration at boot). The org plugin returned bound method values on p.handlers, capturing a nil receiver permanently and making every org endpoint panic with a nil pointer dereference. Resolve p.handlers at request time via method expressions and return a clean error while the plugin is not initialized yet. Closes #9021
1 parent 2cfe630 commit 2d4834c

2 files changed

Lines changed: 68 additions & 9 deletions

File tree

‎backend/plugins/org/impl/impl.go‎

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -102,24 +102,41 @@ func (p Org) RootPkgPath() string {
102102
return "github.com/apache/incubator-devlake/plugins/org"
103103
}
104104

105-
func (p Org) ApiResources() map[string]map[string]plugin.ApiResourceHandler {
105+
// wrapHandler defers the resolution of p.handlers to request time.
106+
// ApiResources() may be evaluated during route registration, which can happen
107+
// before InitPlugins() has called Init() (InitPlugins runs inside
108+
// pipelineServiceInit); a bound method value like p.handlers.GetTeam would
109+
// capture a nil receiver permanently, making every org endpoint panic with a
110+
// nil pointer dereference. See https://github.com/apache/devlake/issues/9021.
111+
func (p *Org) wrapHandler(
112+
method func(*api.Handlers, *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error),
113+
) plugin.ApiResourceHandler {
114+
return func(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
115+
if p.handlers == nil {
116+
return nil, errors.Internal.New("org plugin is not initialized yet, please retry later")
117+
}
118+
return method(p.handlers, input)
119+
}
120+
}
121+
122+
func (p *Org) ApiResources() map[string]map[string]plugin.ApiResourceHandler {
106123
return map[string]map[string]plugin.ApiResourceHandler{
107124
"teams.csv": {
108-
"GET": p.handlers.GetTeam,
109-
"PUT": p.handlers.CreateTeam,
125+
"GET": p.wrapHandler((*api.Handlers).GetTeam),
126+
"PUT": p.wrapHandler((*api.Handlers).CreateTeam),
110127
},
111128
"users.csv": {
112-
"GET": p.handlers.GetUser,
113-
"PUT": p.handlers.CreateUser,
129+
"GET": p.wrapHandler((*api.Handlers).GetUser),
130+
"PUT": p.wrapHandler((*api.Handlers).CreateUser),
114131
},
115132

116133
"user_account_mapping.csv": {
117-
"GET": p.handlers.GetUserAccountMapping,
118-
"PUT": p.handlers.CreateUserAccountMapping,
134+
"GET": p.wrapHandler((*api.Handlers).GetUserAccountMapping),
135+
"PUT": p.wrapHandler((*api.Handlers).CreateUserAccountMapping),
119136
},
120137
"project_mapping.csv": {
121-
"GET": p.handlers.GetProjectMapping,
122-
"PUT": p.handlers.CreateProjectMapping,
138+
"GET": p.wrapHandler((*api.Handlers).GetProjectMapping),
139+
"PUT": p.wrapHandler((*api.Handlers).CreateProjectMapping),
123140
},
124141
}
125142
}
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*
2+
Licensed to the Apache Software Foundation (ASF) under one or more
3+
contributor license agreements. See the NOTICE file distributed with
4+
this work for additional information regarding copyright ownership.
5+
The ASF licenses this file to You under the Apache License, Version 2.0
6+
(the "License"); you may not use this file except in compliance with
7+
the License. You may obtain a copy of the License at
8+
9+
http://www.apache.org/licenses/LICENSE-2.0
10+
11+
Unless required by applicable law or agreed to in writing, software
12+
distributed under the License is distributed on an "AS IS" BASIS,
13+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
See the License for the specific language governing permissions and
15+
limitations under the License.
16+
*/
17+
18+
package impl
19+
20+
import (
21+
"testing"
22+
23+
"github.com/apache/incubator-devlake/core/plugin"
24+
"github.com/stretchr/testify/assert"
25+
)
26+
27+
// Route registration may evaluate ApiResources() before InitPlugins() has run
28+
// (see https://github.com/apache/devlake/issues/9021). Handlers obtained from
29+
// an uninitialized plugin must fail gracefully instead of panicking with a
30+
// nil pointer dereference, and keep working once Init() runs later.
31+
func TestApiResourcesBeforeInitFailsGracefully(t *testing.T) {
32+
p := &Org{}
33+
for resource, methods := range p.ApiResources() {
34+
for method, handler := range methods {
35+
assert.NotPanics(t, func() {
36+
out, err := handler(&plugin.ApiResourceInput{})
37+
assert.Nilf(t, out, "%s %s should return no output before Init", method, resource)
38+
assert.NotNilf(t, err, "%s %s should return an error before Init", method, resource)
39+
}, "%s %s must not panic before Init", method, resource)
40+
}
41+
}
42+
}

0 commit comments

Comments
 (0)