Skip to content

Commit 4672ecb

Browse files
authored
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 <113013457+muzamilkm@users.noreply.github.com>
1 parent 74487a7 commit 4672ecb

5 files changed

Lines changed: 334 additions & 17 deletions

File tree

‎backend/core/plugin/plugin_blueprint.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ package plugin
2020
import (
2121
"encoding/json"
2222

23+
"github.com/apache/devlake/core/dal"
2324
"github.com/apache/devlake/core/errors"
2425
"github.com/apache/devlake/core/models"
2526
)
@@ -92,6 +93,23 @@ type ProjectMapper interface {
9293
MapProject(projectName string, scopes []Scope) (models.PipelinePlan, errors.Error)
9394
}
9495

96+
// ProjectDeleteHook is implemented by plugins that persist state whose
97+
// lifecycle is tied to a DevLake project.
98+
//
99+
// BeforeDeleteProject is called inside the core deletion transaction before
100+
// the project's core records are deleted. Returning an error aborts deletion.
101+
//
102+
// Contract:
103+
// - Database mutations must use only the supplied transaction.
104+
// - The plugin must not commit or roll back the transaction.
105+
// - External side effects cannot be rolled back by the database transaction.
106+
// - Plugins must not depend on hook execution order across plugins.
107+
// Plugins that do not maintain project-scoped state do not need to implement
108+
// this interface.
109+
type ProjectDeleteHook interface {
110+
BeforeDeleteProject(tx dal.Transaction, projectName string) errors.Error
111+
}
112+
95113
// CompositeDataSourcePluginBlueprintV200 is for unit test
96114
type CompositeDataSourcePluginBlueprintV200 interface {
97115
PluginMeta

‎backend/helpers/pluginhelper/services/blueprint_helper.go‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,18 @@ func (b *BlueprintManager) DeleteBlueprint(id uint64) errors.Error {
262262
}
263263
}
264264
}()
265-
err = tx.Delete(&models.BlueprintLabel{}, dal.Where("blueprint_id = ?", id))
265+
err = b.DeleteBlueprintInTransaction(tx, id)
266+
if err != nil {
267+
return err
268+
}
269+
err = tx.Commit()
270+
return err
271+
}
272+
273+
// DeleteBlueprintInTransaction removes a blueprint and its dependent records using
274+
// the caller's transaction. The caller owns commit and rollback.
275+
func (b *BlueprintManager) DeleteBlueprintInTransaction(tx dal.Transaction, id uint64) errors.Error {
276+
err := tx.Delete(&models.BlueprintLabel{}, dal.Where("blueprint_id = ?", id))
266277
if err != nil {
267278
return err
268279
}
@@ -274,9 +285,15 @@ func (b *BlueprintManager) DeleteBlueprint(id uint64) errors.Error {
274285
if err != nil {
275286
return err
276287
}
277-
errors.Must(tx.Delete(&models.BlueprintConnection{}, dal.Where("blueprint_id = ?", id)))
278-
errors.Must(tx.Delete(&models.BlueprintScope{}, dal.Where("blueprint_id = ?", id)))
279-
return tx.Commit()
288+
err = tx.Delete(&models.BlueprintConnection{}, dal.Where("blueprint_id = ?", id))
289+
if err != nil {
290+
return err
291+
}
292+
err = tx.Delete(&models.BlueprintScope{}, dal.Where("blueprint_id = ?", id))
293+
if err != nil {
294+
return err
295+
}
296+
return nil
280297
}
281298

282299
func (b *BlueprintManager) fillBlueprintDetail(blueprint *models.Blueprint) errors.Error {
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
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 services
19+
20+
import (
21+
"testing"
22+
23+
"github.com/apache/devlake/core/errors"
24+
mockdal "github.com/apache/devlake/mocks/core/dal"
25+
"github.com/stretchr/testify/assert"
26+
"github.com/stretchr/testify/mock"
27+
)
28+
29+
func TestDeleteBlueprintInTransaction(t *testing.T) {
30+
t.Run("uses the caller transaction for every delete", func(t *testing.T) {
31+
tx := mockdal.NewTransaction(t)
32+
tx.On("Delete", mock.Anything, mock.Anything).Return(nil)
33+
tx.On("Delete", mock.Anything).Return(nil)
34+
35+
manager := &BlueprintManager{}
36+
err := manager.DeleteBlueprintInTransaction(tx, 42)
37+
38+
assert.NoError(t, err)
39+
assert.Len(t, tx.Calls, 4)
40+
})
41+
42+
t.Run("returns dependent deletion failures", func(t *testing.T) {
43+
expected := errors.Default.New("unable to delete blueprint")
44+
tx := mockdal.NewTransaction(t)
45+
tx.On("Delete", mock.AnythingOfType("*models.Blueprint")).Return(expected).Once()
46+
tx.On("Delete", mock.Anything, mock.Anything).Return(nil)
47+
48+
manager := &BlueprintManager{}
49+
err := manager.DeleteBlueprintInTransaction(tx, 42)
50+
51+
assert.ErrorIs(t, err, expected)
52+
assert.Len(t, tx.Calls, 2)
53+
})
54+
}

‎backend/server/services/project.go‎

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/apache/devlake/core/errors"
2929
"github.com/apache/devlake/core/models"
3030
"github.com/apache/devlake/core/models/domainlayer/crossdomain"
31+
"github.com/apache/devlake/core/plugin"
3132
helper "github.com/apache/devlake/helpers/pluginhelper/api"
3233
)
3334

@@ -327,6 +328,9 @@ func thereAreUnfinishedPipelinesUnderProject(projectName string) (bool, errors.E
327328
if err != nil {
328329
return false, err
329330
}
331+
if blueprint == nil {
332+
return false, nil
333+
}
330334
return thereAreUnfinishedPipelinesUnderBlueprint(blueprint.ID)
331335
}
332336

@@ -366,10 +370,6 @@ func DeleteProject(name string) errors.Error {
366370
if pipelinesAreUnfinished {
367371
return errors.Default.New("There are unfinished pipelines in the current project. It cannot be deleted at this time.")
368372
}
369-
err = deleteProjectBlueprint(name)
370-
if err != nil {
371-
return err
372-
}
373373
tx := db.Begin()
374374
defer func() {
375375
if r := recover(); r != nil || err != nil {
@@ -379,6 +379,13 @@ func DeleteProject(name string) errors.Error {
379379
}
380380
}
381381
}()
382+
if err = runProjectDeleteHooks(tx, name); err != nil {
383+
return err
384+
}
385+
err = deleteProjectBlueprint(tx, name)
386+
if err != nil {
387+
return err
388+
}
382389
err = tx.Delete(&models.Project{}, dal.Where("name = ?", name))
383390
if err != nil {
384391
return errors.Default.Wrap(err, "error deleting project")
@@ -399,20 +406,33 @@ func DeleteProject(name string) errors.Error {
399406
if err != nil {
400407
return errors.Default.Wrap(err, "error deleting project Issue metric")
401408
}
402-
return tx.Commit()
409+
err = tx.Commit()
410+
return err
411+
}
412+
413+
func runProjectDeleteHooks(tx dal.Transaction, projectName string) errors.Error {
414+
return plugin.TraversalPlugin(func(name string, pluginInst plugin.PluginMeta) errors.Error {
415+
if hook, ok := pluginInst.(plugin.ProjectDeleteHook); ok {
416+
if err := hook.BeforeDeleteProject(tx, projectName); err != nil {
417+
return errors.Default.Wrap(err, fmt.Sprintf("error executing delete hook for plugin %s", name))
418+
}
419+
}
420+
return nil
421+
})
403422
}
404423

405-
func deleteProjectBlueprint(projectName string) errors.Error {
406-
bp, err := bpManager.GetDbBlueprintByProjectName(projectName)
424+
func deleteProjectBlueprint(tx dal.Transaction, projectName string) errors.Error {
425+
bp := &models.Blueprint{}
426+
err := tx.First(bp, dal.Where("project_name = ?", projectName))
407427
if err != nil {
408-
if !db.IsErrorNotFound(err) {
428+
if !tx.IsErrorNotFound(err) {
409429
return errors.Default.Wrap(err, fmt.Sprintf("error finding blueprint associated with project %s", projectName))
410430
}
411-
} else {
412-
err = bpManager.DeleteBlueprint(bp.ID)
413-
if err != nil {
414-
return errors.Default.Wrap(err, fmt.Sprintf("error deleting blueprint associated with project %s", projectName))
415-
}
431+
return nil
432+
}
433+
err = bpManager.DeleteBlueprintInTransaction(tx, bp.ID)
434+
if err != nil {
435+
return errors.Default.Wrap(err, fmt.Sprintf("error deleting blueprint associated with project %s", projectName))
416436
}
417437
return nil
418438
}

0 commit comments

Comments
 (0)