Skip to content

Commit

Permalink
feat(api): silently remove duplicate hooks (#6116)
Browse files Browse the repository at this point in the history
  • Loading branch information
fsamin authored Mar 24, 2022
1 parent 7213842 commit 5a33878
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 4 deletions.
19 changes: 16 additions & 3 deletions engine/api/workflow/dao.go
Original file line number Diff line number Diff line change
Expand Up @@ -927,7 +927,7 @@ func CompleteWorkflow(ctx context.Context, db gorp.SqlExecutor, w *sdk.Workflow,
if err := checkProjectIntegration(proj, w, n); err != nil {
return err
}
if err := checkHooks(db, w, n); err != nil {
if err := checkHooks(ctx, db, w, n); err != nil {
return err
}
if err := checkOutGoingHook(db, w, n); err != nil {
Expand Down Expand Up @@ -1026,7 +1026,8 @@ func checkOutGoingHook(db gorp.SqlExecutor, w *sdk.Workflow, n *sdk.Node) error
return nil
}

func checkHooks(db gorp.SqlExecutor, w *sdk.Workflow, n *sdk.Node) error {
func checkHooks(ctx context.Context, db gorp.SqlExecutor, w *sdk.Workflow, n *sdk.Node) error {
var duplicateHookIdx []int64
for i := range n.Hooks {
h := &n.Hooks[i]
if h.HookModelID != 0 {
Expand Down Expand Up @@ -1083,11 +1084,23 @@ func checkHooks(db gorp.SqlExecutor, w *sdk.Workflow, n *sdk.Node) error {
for j := range n.Hooks {
h2 := n.Hooks[j]
if i != j && h.Ref() == h2.Ref() {
return sdk.NewErrorFrom(sdk.ErrWrongRequest, "invalid workflow: duplicate hook %s", model.Name)
log.ErrorWithStackTrace(ctx, sdk.NewErrorFrom(sdk.ErrWrongRequest, "invalid workflow: duplicate hook %s", model.Name))
if !sdk.IsInInt64Array(int64(i), duplicateHookIdx) && !sdk.IsInInt64Array(int64(i), duplicateHookIdx) {
duplicateHookIdx = append(duplicateHookIdx, int64(j))
}
}
}
}

// Remove duplicate hooks
var newHooks = make([]sdk.NodeHook, 0, len(n.Hooks))
for i := range n.Hooks {
if !sdk.IsInInt64Array(int64(i), duplicateHookIdx) {
newHooks = append(newHooks, n.Hooks[i])
}
}
n.Hooks = newHooks

return nil
}

Expand Down
11 changes: 10 additions & 1 deletion engine/api/workflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"encoding/json"
"fmt"
"io"
"io/ioutil"
"net/http"
"net/http/httptest"
"reflect"
Expand Down Expand Up @@ -1752,7 +1753,15 @@ func Test_putWorkflowWithDuplicateHooksShouldRaiseAnError(t *testing.T) {
req = assets.NewAuthentifiedRequest(t, u, pass, "PUT", uri, &wf)
w = httptest.NewRecorder()
router.Mux.ServeHTTP(w, req)
require.Equal(t, 400, w.Code)
require.Equal(t, 200, w.Code)

btes, err := ioutil.ReadAll(w.Body)
require.NoError(t, err)

var resultWorkflow sdk.Workflow
require.NoError(t, sdk.JSONUnmarshal(btes, &resultWorkflow))

require.Len(t, resultWorkflow.WorkflowData.Node.Hooks, 1)
}

func Test_getWorkflowsHandler_FilterByRepo(t *testing.T) {
Expand Down

0 comments on commit 5a33878

Please sign in to comment.