This is an automated email from the ASF dual-hosted git repository.
klesh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/devlake.git
The following commit(s) were added to refs/heads/main by this push:
new 4672ecbc7 fix(project): add transactional plugin deletion lifecycle
hook (#9081) (#9091)
4672ecbc7 is described below
commit 4672ecbc7d24671cc35bf1f7bf954d5caef98cd2
Author: Muhammad Muzamil kaleem <[email protected]>
AuthorDate: Thu Sep 17 08:06:52 2026 +0500
fix(project): add transactional plugin deletion lifecycle hook (#9081)
(#9091)
Project deletion deletes core DevLake records directly without exposing
a lifecycle hook for plugins that maintain project-scoped state.
While project renames provide a lifecycle extension through
ProjectService.RenameProject, there was no equivalent hook for project
deletion.
This commit:
- Introduces optional plugin.ProjectDeleteHook in core/plugin
- Traverses registered plugins via plugin.TraversalPlugin during
DeleteProject
- Executes hooks, blueprint cleanup, and project deletion inside a single
transaction
- Provides BlueprintManager.DeleteBlueprintInTransaction to participate in
caller transaction
- Adds comprehensive unit tests covering hook dispatch, veto rollback, and
atomic deletion
Closes #9081
Signed-off-by: Muzamil Kaleem <[email protected]>
---
backend/core/plugin/plugin_blueprint.go | 18 ++
.../pluginhelper/services/blueprint_helper.go | 25 ++-
.../pluginhelper/services/blueprint_helper_test.go | 54 ++++++
backend/server/services/project.go | 46 +++--
backend/server/services/project_test.go | 208 +++++++++++++++++++++
5 files changed, 334 insertions(+), 17 deletions(-)
diff --git a/backend/core/plugin/plugin_blueprint.go
b/backend/core/plugin/plugin_blueprint.go
index 68ea49386..2fa43e4f2 100644
--- a/backend/core/plugin/plugin_blueprint.go
+++ b/backend/core/plugin/plugin_blueprint.go
@@ -20,6 +20,7 @@ package plugin
import (
"encoding/json"
+ "github.com/apache/devlake/core/dal"
"github.com/apache/devlake/core/errors"
"github.com/apache/devlake/core/models"
)
@@ -92,6 +93,23 @@ type ProjectMapper interface {
MapProject(projectName string, scopes []Scope) (models.PipelinePlan,
errors.Error)
}
+// ProjectDeleteHook is implemented by plugins that persist state whose
+// lifecycle is tied to a DevLake project.
+//
+// BeforeDeleteProject is called inside the core deletion transaction before
+// the project's core records are deleted. Returning an error aborts deletion.
+//
+// Contract:
+// - Database mutations must use only the supplied transaction.
+// - The plugin must not commit or roll back the transaction.
+// - External side effects cannot be rolled back by the database transaction.
+// - Plugins must not depend on hook execution order across plugins.
+// Plugins that do not maintain project-scoped state do not need to implement
+// this interface.
+type ProjectDeleteHook interface {
+ BeforeDeleteProject(tx dal.Transaction, projectName string) errors.Error
+}
+
// CompositeDataSourcePluginBlueprintV200 is for unit test
type CompositeDataSourcePluginBlueprintV200 interface {
PluginMeta
diff --git a/backend/helpers/pluginhelper/services/blueprint_helper.go
b/backend/helpers/pluginhelper/services/blueprint_helper.go
index b25769179..7aa280151 100644
--- a/backend/helpers/pluginhelper/services/blueprint_helper.go
+++ b/backend/helpers/pluginhelper/services/blueprint_helper.go
@@ -262,7 +262,18 @@ func (b *BlueprintManager) DeleteBlueprint(id uint64)
errors.Error {
}
}
}()
- err = tx.Delete(&models.BlueprintLabel{}, dal.Where("blueprint_id = ?",
id))
+ err = b.DeleteBlueprintInTransaction(tx, id)
+ if err != nil {
+ return err
+ }
+ err = tx.Commit()
+ return err
+}
+
+// DeleteBlueprintInTransaction removes a blueprint and its dependent records
using
+// the caller's transaction. The caller owns commit and rollback.
+func (b *BlueprintManager) DeleteBlueprintInTransaction(tx dal.Transaction, id
uint64) errors.Error {
+ err := tx.Delete(&models.BlueprintLabel{}, dal.Where("blueprint_id =
?", id))
if err != nil {
return err
}
@@ -274,9 +285,15 @@ func (b *BlueprintManager) DeleteBlueprint(id uint64)
errors.Error {
if err != nil {
return err
}
- errors.Must(tx.Delete(&models.BlueprintConnection{},
dal.Where("blueprint_id = ?", id)))
- errors.Must(tx.Delete(&models.BlueprintScope{}, dal.Where("blueprint_id
= ?", id)))
- return tx.Commit()
+ err = tx.Delete(&models.BlueprintConnection{}, dal.Where("blueprint_id
= ?", id))
+ if err != nil {
+ return err
+ }
+ err = tx.Delete(&models.BlueprintScope{}, dal.Where("blueprint_id = ?",
id))
+ if err != nil {
+ return err
+ }
+ return nil
}
func (b *BlueprintManager) fillBlueprintDetail(blueprint *models.Blueprint)
errors.Error {
diff --git a/backend/helpers/pluginhelper/services/blueprint_helper_test.go
b/backend/helpers/pluginhelper/services/blueprint_helper_test.go
new file mode 100644
index 000000000..2c7be7651
--- /dev/null
+++ b/backend/helpers/pluginhelper/services/blueprint_helper_test.go
@@ -0,0 +1,54 @@
+/*
+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 services
+
+import (
+ "testing"
+
+ "github.com/apache/devlake/core/errors"
+ mockdal "github.com/apache/devlake/mocks/core/dal"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/mock"
+)
+
+func TestDeleteBlueprintInTransaction(t *testing.T) {
+ t.Run("uses the caller transaction for every delete", func(t
*testing.T) {
+ tx := mockdal.NewTransaction(t)
+ tx.On("Delete", mock.Anything, mock.Anything).Return(nil)
+ tx.On("Delete", mock.Anything).Return(nil)
+
+ manager := &BlueprintManager{}
+ err := manager.DeleteBlueprintInTransaction(tx, 42)
+
+ assert.NoError(t, err)
+ assert.Len(t, tx.Calls, 4)
+ })
+
+ t.Run("returns dependent deletion failures", func(t *testing.T) {
+ expected := errors.Default.New("unable to delete blueprint")
+ tx := mockdal.NewTransaction(t)
+ tx.On("Delete",
mock.AnythingOfType("*models.Blueprint")).Return(expected).Once()
+ tx.On("Delete", mock.Anything, mock.Anything).Return(nil)
+
+ manager := &BlueprintManager{}
+ err := manager.DeleteBlueprintInTransaction(tx, 42)
+
+ assert.ErrorIs(t, err, expected)
+ assert.Len(t, tx.Calls, 2)
+ })
+}
diff --git a/backend/server/services/project.go
b/backend/server/services/project.go
index 3fb8d4f05..acf4abe57 100644
--- a/backend/server/services/project.go
+++ b/backend/server/services/project.go
@@ -28,6 +28,7 @@ import (
"github.com/apache/devlake/core/errors"
"github.com/apache/devlake/core/models"
"github.com/apache/devlake/core/models/domainlayer/crossdomain"
+ "github.com/apache/devlake/core/plugin"
helper "github.com/apache/devlake/helpers/pluginhelper/api"
)
@@ -327,6 +328,9 @@ func thereAreUnfinishedPipelinesUnderProject(projectName
string) (bool, errors.E
if err != nil {
return false, err
}
+ if blueprint == nil {
+ return false, nil
+ }
return thereAreUnfinishedPipelinesUnderBlueprint(blueprint.ID)
}
@@ -366,10 +370,6 @@ func DeleteProject(name string) errors.Error {
if pipelinesAreUnfinished {
return errors.Default.New("There are unfinished pipelines in
the current project. It cannot be deleted at this time.")
}
- err = deleteProjectBlueprint(name)
- if err != nil {
- return err
- }
tx := db.Begin()
defer func() {
if r := recover(); r != nil || err != nil {
@@ -379,6 +379,13 @@ func DeleteProject(name string) errors.Error {
}
}
}()
+ if err = runProjectDeleteHooks(tx, name); err != nil {
+ return err
+ }
+ err = deleteProjectBlueprint(tx, name)
+ if err != nil {
+ return err
+ }
err = tx.Delete(&models.Project{}, dal.Where("name = ?", name))
if err != nil {
return errors.Default.Wrap(err, "error deleting project")
@@ -399,20 +406,33 @@ func DeleteProject(name string) errors.Error {
if err != nil {
return errors.Default.Wrap(err, "error deleting project Issue
metric")
}
- return tx.Commit()
+ err = tx.Commit()
+ return err
+}
+
+func runProjectDeleteHooks(tx dal.Transaction, projectName string)
errors.Error {
+ return plugin.TraversalPlugin(func(name string, pluginInst
plugin.PluginMeta) errors.Error {
+ if hook, ok := pluginInst.(plugin.ProjectDeleteHook); ok {
+ if err := hook.BeforeDeleteProject(tx, projectName);
err != nil {
+ return errors.Default.Wrap(err,
fmt.Sprintf("error executing delete hook for plugin %s", name))
+ }
+ }
+ return nil
+ })
}
-func deleteProjectBlueprint(projectName string) errors.Error {
- bp, err := bpManager.GetDbBlueprintByProjectName(projectName)
+func deleteProjectBlueprint(tx dal.Transaction, projectName string)
errors.Error {
+ bp := &models.Blueprint{}
+ err := tx.First(bp, dal.Where("project_name = ?", projectName))
if err != nil {
- if !db.IsErrorNotFound(err) {
+ if !tx.IsErrorNotFound(err) {
return errors.Default.Wrap(err, fmt.Sprintf("error
finding blueprint associated with project %s", projectName))
}
- } else {
- err = bpManager.DeleteBlueprint(bp.ID)
- if err != nil {
- return errors.Default.Wrap(err, fmt.Sprintf("error
deleting blueprint associated with project %s", projectName))
- }
+ return nil
+ }
+ err = bpManager.DeleteBlueprintInTransaction(tx, bp.ID)
+ if err != nil {
+ return errors.Default.Wrap(err, fmt.Sprintf("error deleting
blueprint associated with project %s", projectName))
}
return nil
}
diff --git a/backend/server/services/project_test.go
b/backend/server/services/project_test.go
new file mode 100644
index 000000000..ae2e83e3c
--- /dev/null
+++ b/backend/server/services/project_test.go
@@ -0,0 +1,208 @@
+/*
+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 services
+
+import (
+ "fmt"
+ "testing"
+
+ "github.com/apache/devlake/core/dal"
+ "github.com/apache/devlake/core/errors"
+ "github.com/apache/devlake/core/models"
+ "github.com/apache/devlake/core/plugin"
+ blueprintservices
"github.com/apache/devlake/helpers/pluginhelper/services"
+ mockdal "github.com/apache/devlake/mocks/core/dal"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/mock"
+)
+
+type testOrdinaryPlugin struct {
+ name string
+}
+
+func (p *testOrdinaryPlugin) Description() string { return "test ordinary
plugin" }
+func (p *testOrdinaryPlugin) RootPkgPath() string { return
"plugins/test_ordinary" }
+func (p *testOrdinaryPlugin) Name() string { return p.name }
+
+type testHookPlugin struct {
+ name string
+ deleteCalls []struct {
+ tx dal.Transaction
+ projectName string
+ }
+ deleteErr errors.Error
+}
+
+func (p *testHookPlugin) Description() string { return "test hook plugin" }
+func (p *testHookPlugin) RootPkgPath() string { return "plugins/test_hook" }
+func (p *testHookPlugin) Name() string { return p.name }
+func (p *testHookPlugin) BeforeDeleteProject(tx dal.Transaction, projectName
string) errors.Error {
+ p.deleteCalls = append(p.deleteCalls, struct {
+ tx dal.Transaction
+ projectName string
+ }{tx: tx, projectName: projectName})
+ return p.deleteErr
+}
+
+func registerProjectDeleteTestPlugin(t *testing.T, testPlugin
plugin.PluginMeta) {
+ t.Helper()
+ assert.NoError(t, plugin.RegisterPlugin(testPlugin.Name(), testPlugin))
+ t.Cleanup(func() {
+ delete(plugin.AllPlugins(), testPlugin.Name())
+ })
+}
+
+func withProjectTestDatabase(t *testing.T, testDB dal.Dal) {
+ t.Helper()
+ previousDB, previousManager := db, bpManager
+ db = testDB
+ bpManager = blueprintservices.NewBlueprintManager(testDB)
+ t.Cleanup(func() {
+ db = previousDB
+ bpManager = previousManager
+ })
+}
+
+func TestRunProjectDeleteHooks(t *testing.T) {
+ t.Run("skips ordinary plugins without ProjectDeleteHook", func(t
*testing.T) {
+ ordinary := &testOrdinaryPlugin{name: "test-ordinary-skip"}
+ registerProjectDeleteTestPlugin(t, ordinary)
+
+ tx := mockdal.NewTransaction(t)
+ assert.NoError(t, runProjectDeleteHooks(tx, "test-project"))
+ })
+
+ t.Run("invokes implementing plugins with exact transaction and project
name", func(t *testing.T) {
+ hook := &testHookPlugin{name: "test-hook-invoke"}
+ registerProjectDeleteTestPlugin(t, hook)
+
+ tx := mockdal.NewTransaction(t)
+ assert.NoError(t, runProjectDeleteHooks(tx, "test-project"))
+
+ assert.Equal(t, 1, len(hook.deleteCalls))
+ assert.Equal(t, tx, hook.deleteCalls[0].tx)
+ assert.Equal(t, "test-project", hook.deleteCalls[0].projectName)
+ })
+
+ t.Run("returns wrapped error on hook veto", func(t *testing.T) {
+ expectedErr := errors.Default.New("project delete vetoed by
plugin")
+ hook := &testHookPlugin{
+ name: "test-hook-veto",
+ deleteErr: expectedErr,
+ }
+ registerProjectDeleteTestPlugin(t, hook)
+
+ tx := mockdal.NewTransaction(t)
+ err := runProjectDeleteHooks(tx, "test-project")
+
+ assert.Error(t, err)
+ assert.ErrorIs(t, err, expectedErr)
+ assert.Contains(t, err.Error(), fmt.Sprintf("error executing
delete hook for plugin %s", hook.Name()))
+ })
+}
+
+func TestDeleteProject_RollsBackOnDeleteHookVeto(t *testing.T) {
+ hookErr := errors.Default.New("hook rejection")
+ hook := &testHookPlugin{
+ name: "test-hook-rollback-veto",
+ deleteErr: hookErr,
+ }
+ registerProjectDeleteTestPlugin(t, hook)
+
+ tx := mockdal.NewTransaction(t)
+ notFound := errors.NotFound.New("blueprint not found")
+ mockDB := mockdal.NewDal(t)
+ mockDB.On("First", mock.Anything, mock.Anything).Return(nil).Once()
+ mockDB.On("First", mock.Anything, mock.Anything).Return(notFound).Once()
+ mockDB.On("IsErrorNotFound", mock.Anything).Return(true).Twice()
+ mockDB.On("Begin").Return(tx).Once()
+ tx.On("Rollback").Return(nil).Once()
+ withProjectTestDatabase(t, mockDB)
+
+ err := DeleteProject("project-veto")
+
+ assert.Error(t, err)
+ assert.ErrorIs(t, err, hookErr)
+ assert.Contains(t, err.Error(), fmt.Sprintf("error executing delete
hook for plugin %s", hook.Name()))
+ assert.Equal(t, 1, len(hook.deleteCalls))
+ assert.Equal(t, "project-veto", hook.deleteCalls[0].projectName)
+ tx.AssertNotCalled(t, "Commit")
+}
+
+func TestDeleteProject_RollsBackOnBlueprintDeletionFailure(t *testing.T) {
+ expectedErr := errors.Default.New("unable to delete blueprint labels")
+ tx := mockdal.NewTransaction(t)
+ tx.On("First", mock.Anything, mock.Anything).Run(func(args
mock.Arguments) {
+ args.Get(0).(*models.Blueprint).ID = 42
+ }).Return(nil).Once()
+ tx.On("Delete", mock.Anything, mock.Anything).Return(expectedErr).Once()
+ tx.On("Rollback").Return(nil).Once()
+
+ mockDB := mockdal.NewDal(t)
+ mockDB.On("First", mock.Anything, mock.Anything).Run(func(args
mock.Arguments) {
+ if blueprint, ok := args.Get(0).(*models.Blueprint); ok {
+ blueprint.ID = 42
+ }
+ }).Return(nil).Twice()
+ mockDB.On("Pluck", mock.Anything, mock.Anything,
mock.Anything).Return(nil).Once()
+ mockDB.On("All", mock.Anything, mock.Anything).Return(nil)
+ mockDB.On("Count", mock.Anything).Return(int64(0), nil).Once()
+ mockDB.On("Begin").Return(tx).Once()
+ withProjectTestDatabase(t, mockDB)
+
+ err := DeleteProject("project-blueprint-failure")
+
+ assert.ErrorIs(t, err, expectedErr)
+ tx.AssertNotCalled(t, "Commit")
+}
+
+func TestDeleteProject_SuccessfulDeletionInSingleTransaction(t *testing.T) {
+ hook := &testHookPlugin{
+ name: "test-hook-success",
+ }
+ registerProjectDeleteTestPlugin(t, hook)
+
+ tx := mockdal.NewTransaction(t)
+ tx.On("First", mock.Anything, mock.Anything).Run(func(args
mock.Arguments) {
+ args.Get(0).(*models.Blueprint).ID = 42
+ }).Return(nil).Once()
+ tx.On("Delete", mock.Anything, mock.Anything).Return(nil)
+ tx.On("Delete", mock.Anything).Return(nil)
+ tx.On("Commit").Return(nil).Once()
+
+ mockDB := mockdal.NewDal(t)
+ mockDB.On("First", mock.Anything, mock.Anything).Run(func(args
mock.Arguments) {
+ if blueprint, ok := args.Get(0).(*models.Blueprint); ok {
+ blueprint.ID = 42
+ }
+ }).Return(nil).Twice()
+ mockDB.On("Pluck", mock.Anything, mock.Anything,
mock.Anything).Return(nil).Once()
+ mockDB.On("All", mock.Anything, mock.Anything).Return(nil)
+ mockDB.On("Count", mock.Anything).Return(int64(0), nil).Once()
+ mockDB.On("Begin").Return(tx).Once()
+ withProjectTestDatabase(t, mockDB)
+
+ err := DeleteProject("project-success")
+
+ assert.NoError(t, err)
+ assert.Equal(t, 1, len(hook.deleteCalls))
+ assert.Equal(t, tx, hook.deleteCalls[0].tx)
+ assert.Equal(t, "project-success", hook.deleteCalls[0].projectName)
+ assert.Len(t, tx.Calls, 11)
+ tx.AssertNotCalled(t, "Rollback")
+}
