diff --git a/backend/plugins/org/impl/impl.go b/backend/plugins/org/impl/impl.go index e68257eec31..39d020fac5d 100644 --- a/backend/plugins/org/impl/impl.go +++ b/backend/plugins/org/impl/impl.go @@ -102,24 +102,41 @@ func (p Org) RootPkgPath() string { return "github.com/apache/incubator-devlake/plugins/org" } -func (p Org) ApiResources() map[string]map[string]plugin.ApiResourceHandler { +// wrapHandler defers the resolution of p.handlers to request time. +// ApiResources() may be evaluated during route registration, which can happen +// before InitPlugins() has called Init() (InitPlugins runs inside +// pipelineServiceInit); a bound method value like p.handlers.GetTeam would +// capture a nil receiver permanently, making every org endpoint panic with a +// nil pointer dereference. See https://github.com/apache/devlake/issues/9021. +func (p *Org) wrapHandler( + method func(*api.Handlers, *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error), +) plugin.ApiResourceHandler { + return func(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) { + if p.handlers == nil { + return nil, errors.Internal.New("org plugin is not initialized yet, please retry later") + } + return method(p.handlers, input) + } +} + +func (p *Org) ApiResources() map[string]map[string]plugin.ApiResourceHandler { return map[string]map[string]plugin.ApiResourceHandler{ "teams.csv": { - "GET": p.handlers.GetTeam, - "PUT": p.handlers.CreateTeam, + "GET": p.wrapHandler((*api.Handlers).GetTeam), + "PUT": p.wrapHandler((*api.Handlers).CreateTeam), }, "users.csv": { - "GET": p.handlers.GetUser, - "PUT": p.handlers.CreateUser, + "GET": p.wrapHandler((*api.Handlers).GetUser), + "PUT": p.wrapHandler((*api.Handlers).CreateUser), }, "user_account_mapping.csv": { - "GET": p.handlers.GetUserAccountMapping, - "PUT": p.handlers.CreateUserAccountMapping, + "GET": p.wrapHandler((*api.Handlers).GetUserAccountMapping), + "PUT": p.wrapHandler((*api.Handlers).CreateUserAccountMapping), }, "project_mapping.csv": { - "GET": p.handlers.GetProjectMapping, - "PUT": p.handlers.CreateProjectMapping, + "GET": p.wrapHandler((*api.Handlers).GetProjectMapping), + "PUT": p.wrapHandler((*api.Handlers).CreateProjectMapping), }, } } diff --git a/backend/plugins/org/impl/impl_test.go b/backend/plugins/org/impl/impl_test.go new file mode 100644 index 00000000000..4b6313ee33e --- /dev/null +++ b/backend/plugins/org/impl/impl_test.go @@ -0,0 +1,42 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package impl + +import ( + "testing" + + "github.com/apache/incubator-devlake/core/plugin" + "github.com/stretchr/testify/assert" +) + +// Route registration may evaluate ApiResources() before InitPlugins() has run +// (see https://github.com/apache/devlake/issues/9021). Handlers obtained from +// an uninitialized plugin must fail gracefully instead of panicking with a +// nil pointer dereference, and keep working once Init() runs later. +func TestApiResourcesBeforeInitFailsGracefully(t *testing.T) { + p := &Org{} + for resource, methods := range p.ApiResources() { + for method, handler := range methods { + assert.NotPanics(t, func() { + out, err := handler(&plugin.ApiResourceInput{}) + assert.Nilf(t, out, "%s %s should return no output before Init", method, resource) + assert.NotNilf(t, err, "%s %s should return an error before Init", method, resource) + }, "%s %s must not panic before Init", method, resource) + } + } +}