Skip to content

Commit

Permalink
refactor: optimize list hooks for avoid unnecessary json marshal/unma…
Browse files Browse the repository at this point in the history
…rshal
  • Loading branch information
chengjoey committed Oct 28, 2024
1 parent 9aaa584 commit db4af33
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (

"github.com/sirupsen/logrus"

"github.com/erda-project/erda-proto-go/core/messenger/eventbox/pb"
"github.com/erda-project/erda/apistructs"
"github.com/erda-project/erda/internal/core/messenger/eventbox/constant"
"github.com/erda-project/erda/internal/core/messenger/eventbox/dispatcher/errors"
Expand Down Expand Up @@ -61,7 +62,7 @@ func (w *WebhookFilter) Filter(m *types.Message) *errors.DispatchError {
Org: "-1",
}, eventLabel.Event)

var hs []apistructs.Hook
var hs []*pb.Hook
if eventLabel.OrgID != "-1" {
hs = w.impl.SearchHooks(apistructs.HookLocation{
Org: eventLabel.OrgID,
Expand All @@ -78,10 +79,10 @@ func (w *WebhookFilter) Filter(m *types.Message) *errors.DispatchError {

urls := []string{}
for _, h := range hs {
urls = append(urls, h.URL)
urls = append(urls, h.Url)
}
for _, h := range internalHs {
urls = append(urls, h.URL)
urls = append(urls, h.Url)
}

if err := replaceLabel(m, urls); err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"bou.ke/monkey"
"github.com/stretchr/testify/assert"

"github.com/erda-project/erda-proto-go/core/messenger/eventbox/pb"
"github.com/erda-project/erda/apistructs"
"github.com/erda-project/erda/internal/core/messenger/eventbox/constant"
"github.com/erda-project/erda/internal/core/messenger/eventbox/dispatcher/errors"
Expand Down Expand Up @@ -115,28 +116,23 @@ func TestWebhookFilter(t *testing.T) {
whi = &webhook.WebHookImpl{}
)

hook := webhook.Hook{
ID: "1",
CreateHookRequest: apistructs.CreateHookRequest{
Name: "test-event",
Events: []string{"event1", "event2"},
URL: "https://localhost:8080/hook",
Active: true,
HookLocation: apistructs.HookLocation{
Org: "-1",
Project: "-1",
Application: "-1",
Env: make([]string, 0),
},
},
hook := &pb.Hook{
Id: "1",
Name: "test-event",
Events: []string{"event1", "event2"},
Url: "https://localhost:8080/hook",
Active: true,
OrgID: "-1",
ProjectID: "-1",
ApplicationID: "-1",
}

monkey.PatchInstanceMethod(reflect.TypeOf(whi), "SearchHooks", func(whi *webhook.WebHookImpl, location webhook.HookLocation, event string) []webhook.Hook {
return []webhook.Hook{hook}
monkey.PatchInstanceMethod(reflect.TypeOf(whi), "SearchHooks", func(whi *webhook.WebHookImpl, location webhook.HookLocation, event string) []*pb.Hook {
return []*pb.Hook{hook}
})

monkey.PatchInstanceMethod(reflect.TypeOf(whi), "ListHooks", func(whi *webhook.WebHookImpl, location webhook.HookLocation) (webhook.ListHooksResponse, error) {
return []webhook.Hook{hook}, nil
monkey.PatchInstanceMethod(reflect.TypeOf(whi), "ListHooks", func(whi *webhook.WebHookImpl, location webhook.HookLocation) ([]*pb.Hook, error) {
return []*pb.Hook{hook}, nil
})

defer monkey.UnpatchAll()
Expand Down
5 changes: 2 additions & 3 deletions internal/core/messenger/eventbox/webhook/builtin.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,8 @@ func createIfNotExist(impl *WebHookImpl, req *CreateHookRequest) error {
if err != nil {
return err
}
hooks := []apistructs.Hook(resp)
for i := range hooks {
if hooks[i].Name == req.Name {
for i := range resp {
if resp[i].Name == req.Name {
return nil
}
}
Expand Down
14 changes: 1 addition & 13 deletions internal/core/messenger/eventbox/webhook/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,20 +80,8 @@ func (w *WebHookHTTP) ListHooks(ctx context.Context, req *pb.ListHooksRequest, v
return nil, err
}

if err != nil {
return nil, err
}
data, err := json.Marshal(r)
if err != nil {
return nil, err
}
hooks := make([]*pb.Hook, 0)
err = json.Unmarshal(data, &hooks)
if err != nil {
return nil, err
}
return &pb.ListHooksResponse{
Data: hooks,
Data: r,
}, nil
}

Expand Down
19 changes: 10 additions & 9 deletions internal/core/messenger/eventbox/webhook/impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/pkg/errors"
"github.com/sirupsen/logrus"

"github.com/erda-project/erda-proto-go/core/messenger/eventbox/pb"
"github.com/erda-project/erda/apistructs"
"github.com/erda-project/erda/internal/core/messenger/eventbox/constant"
"github.com/erda-project/erda/pkg/crypto/uuid"
Expand Down Expand Up @@ -62,31 +63,31 @@ func hookCheckOrg(h Hook, orgID string) bool {
}

// ListHooks 列出符合 location 的 hook 列表。
func (w *WebHookImpl) ListHooks(location HookLocation) (ListHooksResponse, error) {
func (w *WebHookImpl) ListHooks(location HookLocation) ([]*pb.Hook, error) {
dir := mkLocationDir(location)

var r []*pb.Hook
ids := []string{}
keys, err := w.js.ListKeys(context.Background(), dir)
if err != nil {
return ListHooksResponse{}, errors.Wrap(InternalServerErr, fmt.Sprintf("list hooks fail: %v", err))
return r, errors.Wrap(InternalServerErr, fmt.Sprintf("list hooks fail: %v", err))
}
for _, k := range keys {
parts := strings.Split(k, "/")
if len(parts) == 0 {
return ListHooksResponse{},
return r,
errors.Wrap(InternalServerErr, fmt.Sprintf("list hooks fail: bad hook index format: %v", k))
}
ids = append(ids, parts[len(parts)-1])
}
r := ListHooksResponse{}
for _, id := range ids {
h := Hook{}
if err := w.js.Get(context.Background(), mkHookEtcdName(id), &h); err != nil {
h := &pb.Hook{}
if err := w.js.Get(context.Background(), mkHookEtcdName(id), h); err != nil {
if err == jsonstore.NotFoundErr {
// 如果有索引但没找到数据,则忽略这个webhook
continue
}
return ListHooksResponse{},
return r,
errors.Wrap(InternalServerErr, fmt.Sprintf("list hooks fail: get hook: %v", mkHookEtcdName(id)))
}
if envSatisfy(location.Env, h.Env) {
Expand Down Expand Up @@ -256,12 +257,12 @@ func (w *WebHookImpl) DeleteHook(realOrg, id string) error {
}

/* search hooks which include 'event' and is 'active' */
func (w *WebHookImpl) SearchHooks(location HookLocation, event string) []Hook {
func (w *WebHookImpl) SearchHooks(location HookLocation, event string) []*pb.Hook {
hs, err := w.ListHooks(location)
if err != nil {
return nil
}
r := []Hook{}
r := []*pb.Hook{}
for _, h := range hs {
for _, e := range h.Events {
if e == event && h.Active {
Expand Down

0 comments on commit db4af33

Please sign in to comment.