From 9d8df8d05dd6baac85a404e1d01c74600bc023af Mon Sep 17 00:00:00 2001 From: Gani Georgiev Date: Mon, 29 May 2023 14:50:38 +0300 Subject: [PATCH] added option to remove single registered hook handler --- CHANGELOG.md | 4 +++ tools/hook/hook.go | 74 +++++++++++++++++++++++++++++++++-------- tools/hook/hook_test.go | 44 +++++++++++++++++++++--- 3 files changed, 103 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01c69ac6..8e765d3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ - Added VK OAuth2 provider ([#2533](https://github.com/pocketbase/pocketbase/pull/2533); thanks @imperatrona). +- Renamed `Hook.Reset()` -> `Hook.RemoveAll()`. + +- `Hook.Add()` and `Hook.PreAdd` now returns a unique string identifier that could be used to remove the registered hook handler via `Hook.Remove(handlerId)`. + ## v0.16.4-WIP diff --git a/tools/hook/hook.go b/tools/hook/hook.go index b9393423..eb6780ee 100644 --- a/tools/hook/hook.go +++ b/tools/hook/hook.go @@ -2,7 +2,10 @@ package hook import ( "errors" + "fmt" "sync" + + "github.com/pocketbase/pocketbase/tools/security" ) var StopPropagation = errors.New("Event hook propagation stopped") @@ -10,36 +13,65 @@ var StopPropagation = errors.New("Event hook propagation stopped") // Handler defines a hook handler function. type Handler[T any] func(e T) error +// handlerPair defines a pair of string id and Handler. +type handlerPair[T any] struct { + id string + handler Handler[T] +} + // Hook defines a concurrent safe structure for handling event hooks // (aka. callbacks propagation). type Hook[T any] struct { mux sync.RWMutex - handlers []Handler[T] + handlers []*handlerPair[T] } // PreAdd registers a new handler to the hook by prepending it to the existing queue. -func (h *Hook[T]) PreAdd(fn Handler[T]) { +// +// Returns an autogenerated hook id that could be used later to remove the hook with Hook.Remove(id). +func (h *Hook[T]) PreAdd(fn Handler[T]) string { h.mux.Lock() defer h.mux.Unlock() + id := generateHookId() + // minimize allocations by shifting the slice h.handlers = append(h.handlers, nil) copy(h.handlers[1:], h.handlers) - h.handlers[0] = fn + h.handlers[0] = &handlerPair[T]{id, fn} + + return id } // Add registers a new handler to the hook by appending it to the existing queue. -func (h *Hook[T]) Add(fn Handler[T]) { +// +// Returns an autogenerated hook id that could be used later to remove the hook with Hook.Remove(id). +func (h *Hook[T]) Add(fn Handler[T]) string { h.mux.Lock() defer h.mux.Unlock() - h.handlers = append(h.handlers, fn) + id := generateHookId() + + h.handlers = append(h.handlers, &handlerPair[T]{id, fn}) + + return id } -// Reset removes all registered handlers. -// -// @todo for consistency with other Go methods consider renaming it to Clear. -func (h *Hook[T]) Reset() { +// Remove removes a single hook handler by its id. +func (h *Hook[T]) Remove(id string) { + h.mux.Lock() + defer h.mux.Unlock() + + for i := len(h.handlers) - 1; i >= 0; i-- { + if h.handlers[i].id == id { + h.handlers = append(h.handlers[:i], h.handlers[i+1:]...) + return + } + } +} + +// RemoveAll removes all registered handlers. +func (h *Hook[T]) RemoveAll() { h.mux.Lock() defer h.mux.Unlock() @@ -57,14 +89,24 @@ func (h *Hook[T]) Reset() { // - any non-nil error is returned in one of the handlers func (h *Hook[T]) Trigger(data T, oneOffHandlers ...Handler[T]) error { h.mux.RLock() - handlers := make([]Handler[T], 0, len(h.handlers)+len(oneOffHandlers)) + + handlers := make([]*handlerPair[T], 0, len(h.handlers)+len(oneOffHandlers)) handlers = append(handlers, h.handlers...) - handlers = append(handlers, oneOffHandlers...) - // unlock is not deferred to avoid deadlocks when Trigger is called recursive by the handlers + + // append the one off handlers + for i, oneOff := range oneOffHandlers { + handlers = append(handlers, &handlerPair[T]{ + id: fmt.Sprintf("@%d", i), + handler: oneOff, + }) + } + + // unlock is not deferred to avoid deadlocks in case Trigger + // is called recursively by the handlers h.mux.RUnlock() - for _, fn := range handlers { - err := fn(data) + for _, item := range handlers { + err := item.handler(data) if err == nil { continue } @@ -78,3 +120,7 @@ func (h *Hook[T]) Trigger(data T, oneOffHandlers ...Handler[T]) error { return nil } + +func generateHookId() string { + return security.PseudorandomString(8) +} diff --git a/tools/hook/hook_test.go b/tools/hook/hook_test.go index cdba3b5d..a3ab688c 100644 --- a/tools/hook/hook_test.go +++ b/tools/hook/hook_test.go @@ -36,22 +36,56 @@ func TestHookAddAndPreAdd(t *testing.T) { } } -func TestHookReset(t *testing.T) { +func TestHookRemove(t *testing.T) { h := Hook[int]{} - h.Reset() // should do nothing and not panic + h1Called := false + h2Called := false + + id1 := h.Add(func(data int) error { h1Called = true; return nil }) + h.Add(func(data int) error { h2Called = true; return nil }) + + h.Remove("missing") // should do nothing and not panic + + if total := len(h.handlers); total != 2 { + t.Fatalf("Expected %d handlers, got %d", 2, total) + } + + h.Remove(id1) + + if total := len(h.handlers); total != 1 { + t.Fatalf("Expected %d handlers, got %d", 1, total) + } + + if err := h.Trigger(1); err != nil { + t.Fatal(err) + } + + if h1Called { + t.Fatalf("Expected hook 1 to be removed and not called") + } + + if !h2Called { + t.Fatalf("Expected hook 2 to be called") + } +} + +func TestHookRemoveAll(t *testing.T) { + h := Hook[int]{} + + h.RemoveAll() // should do nothing and not panic h.Add(func(data int) error { return nil }) h.Add(func(data int) error { return nil }) if total := len(h.handlers); total != 2 { - t.Fatalf("Expected 2 handlers before Reset, found %d", total) + t.Fatalf("Expected 2 handlers before RemoveAll, found %d", total) } - h.Reset() + h.RemoveAll() if total := len(h.handlers); total != 0 { - t.Fatalf("Expected no handlers after Reset, found %d", total) + t.Fatalf("Expected no handlers after RemoveAll, found %d", total) } }