From 53a1481a034a63e733ad4e63e9b4a2b04462f937 Mon Sep 17 00:00:00 2001
From: Paul Larsen <pnvlarsen@gmail.com>
Date: Mon, 2 May 2022 21:47:52 +0100
Subject: [PATCH 01/11] Add handler removals

---
 ext/dispatcher.go     | 47 +++++++++++++++++++++++++++++++++++++++++++
 ext/handler.go        |  4 +++-
 ext/handlers/named.go | 16 +++++++++++++++
 3 files changed, 66 insertions(+), 1 deletion(-)
 create mode 100644 ext/handlers/named.go

diff --git a/ext/dispatcher.go b/ext/dispatcher.go
index 8913f4c4..6fc0e54f 100644
--- a/ext/dispatcher.go
+++ b/ext/dispatcher.go
@@ -237,6 +237,53 @@ func (d *Dispatcher) AddHandlerToGroup(handler Handler, group int) {
 	d.handlers[group] = append(currHandlers, handler)
 }
 
+// RemoveHandlerFromGroup removes a handler by name from the specified group.
+// If multiple handlers have the same name, only the first one is removed.
+// Returns true if the handler was successfully removed.
+func (d *Dispatcher) RemoveHandlerFromGroup(handlerName string, group int) bool {
+	currHandlers, ok := d.handlers[group]
+	if !ok {
+		// group does not exist; removal failed.
+		return false
+	}
+
+	for i, handler := range currHandlers {
+		if handler.Name() == handlerName {
+			d.handlers[group] = append(currHandlers[:i], currHandlers[i+1:]...)
+
+			// Had one item, we removed one, so none left.
+			if len(currHandlers) == 1 {
+				// No more handlers, so group should also be removed.
+				d.RemoveGroup(group)
+			}
+			return true
+		}
+	}
+	// handler not found - removal failed.
+	return false
+}
+
+// RemoveGroup removes an entire group from the dispatcher's processing.
+// If group can't be found, this is a noop.
+func (d *Dispatcher) RemoveGroup(group int) {
+	if _, ok := d.handlers[group]; !ok {
+		// Group doesn't exist in map, so already removed.
+		return
+	}
+
+	groups := d.handlerGroups
+	for j, handlerGroup := range groups {
+		if handlerGroup == group {
+			d.handlerGroups = append(groups[:j], groups[j+1:]...)
+			delete(d.handlers, group)
+			// Group found, and deleted. Success!
+			return
+		}
+	}
+	// Group not found in list - so already removed.
+	return
+}
+
 // processRawUpdate takes a JSON update to be unmarshalled and processed by Dispatcher.ProcessUpdate.
 func (d *Dispatcher) processRawUpdate(b *gotgbot.Bot, r json.RawMessage) error {
 	var upd gotgbot.Update
diff --git a/ext/handler.go b/ext/handler.go
index 23096d96..85d96f9f 100644
--- a/ext/handler.go
+++ b/ext/handler.go
@@ -1,6 +1,8 @@
 package ext
 
-import "github.com/PaulSonOfLars/gotgbot/v2"
+import (
+	"github.com/PaulSonOfLars/gotgbot/v2"
+)
 
 type Handler interface {
 	// CheckUpdate checks whether the update should handled by this handler.
diff --git a/ext/handlers/named.go b/ext/handlers/named.go
new file mode 100644
index 00000000..c734a520
--- /dev/null
+++ b/ext/handlers/named.go
@@ -0,0 +1,16 @@
+package handlers
+
+import (
+	"github.com/PaulSonOfLars/gotgbot/v2/ext"
+)
+
+type Named struct {
+	// Custom name to identify handler by
+	CustomName string
+	// Inlined version of parent handler to inherit methods.
+	ext.Handler
+}
+
+func (n Named) Name() string {
+	return n.CustomName
+}

From d28a46af3bca7985dbd51a8b8a17d65cf5b71487 Mon Sep 17 00:00:00 2001
From: Paul Larsen <pnvlarsen@gmail.com>
Date: Mon, 2 May 2022 22:06:02 +0100
Subject: [PATCH 02/11] Add some dispatcher tests to confirm removals work fine

---
 ext/dispatcher_ext_test.go | 179 +++++++++++++++++++++++++++++++++++++
 ext/handlers/named.go      |   7 ++
 2 files changed, 186 insertions(+)
 create mode 100644 ext/dispatcher_ext_test.go

diff --git a/ext/dispatcher_ext_test.go b/ext/dispatcher_ext_test.go
new file mode 100644
index 00000000..efa5695a
--- /dev/null
+++ b/ext/dispatcher_ext_test.go
@@ -0,0 +1,179 @@
+package ext_test
+
+import (
+	"sort"
+	"testing"
+
+	"github.com/PaulSonOfLars/gotgbot/v2"
+	"github.com/PaulSonOfLars/gotgbot/v2/ext"
+	"github.com/PaulSonOfLars/gotgbot/v2/ext/handlers"
+	"github.com/PaulSonOfLars/gotgbot/v2/ext/handlers/filters/message"
+)
+
+func TestDispatcher(t *testing.T) {
+	type testHandler struct {
+		group     int
+		shouldRun bool
+		returnVal error
+	}
+
+	for name, testParams := range map[string]struct {
+		handlers   []testHandler
+		numMatches int
+	}{
+		"one group two handlers": {
+			handlers: []testHandler{
+				{
+					group:     0,
+					shouldRun: true,
+					returnVal: nil,
+				}, {
+					group:     0,
+					shouldRun: false, // same group, so doesnt run
+					returnVal: nil,
+				},
+			},
+			numMatches: 1,
+		},
+		"two handlers two groups": {
+			handlers: []testHandler{
+				{
+					group:     0,
+					shouldRun: true,
+					returnVal: nil,
+				}, {
+					group:     1,
+					shouldRun: true, // second group, so also runs
+					returnVal: nil,
+				},
+			},
+			numMatches: 2,
+		},
+		"end groups": {
+			handlers: []testHandler{
+				{
+					group:     0,
+					shouldRun: true,
+					returnVal: ext.EndGroups,
+				}, {
+					group:     1,
+					shouldRun: false, // ended, so second group doesnt run
+					returnVal: nil,
+				},
+			},
+			numMatches: 1,
+		},
+		"continue groups": {
+			handlers: []testHandler{
+				{
+					group:     0,
+					shouldRun: true,
+					returnVal: ext.ContinueGroups,
+				}, {
+					group:     0,
+					shouldRun: true, // continued, so second item in same group runs
+					returnVal: nil,
+				},
+			},
+			numMatches: 2,
+		},
+	} {
+		name, testParams := name, testParams
+
+		t.Run(name, func(t *testing.T) {
+			d := ext.NewDispatcher(nil)
+			var events []int
+			for idx, h := range testParams.handlers {
+				idx, h := idx, h
+
+				t.Logf("Loading handler %d in group %d", idx, h.group)
+				d.AddHandlerToGroup(handlers.NewMessage(message.All, func(b *gotgbot.Bot, ctx *ext.Context) error {
+					if !h.shouldRun {
+						t.Errorf("handler %d in group %d should not have run", idx, h.group)
+						t.FailNow()
+					}
+
+					t.Logf("handler %d in group %d has run, as expected", idx, h.group)
+					events = append(events, idx)
+					return h.returnVal
+				}), h.group)
+			}
+
+			t.Log("Processing one update...")
+			d.ProcessUpdate(nil, &gotgbot.Update{
+				Message: &gotgbot.Message{Text: "test text"},
+			}, nil)
+
+			// ensure events handled in order
+			if !sort.IntsAreSorted(events) {
+				t.Errorf("order of events is not sorted: %v", events)
+			}
+			if len(events) != testParams.numMatches {
+				t.Errorf("got %d matches, expected %d ", len(events), testParams.numMatches)
+			}
+		})
+	}
+}
+
+func TestDispatcher_RemoveHandlerFromGroup(t *testing.T) {
+	d := ext.NewDispatcher(nil)
+
+	const removeMe = "remove_me"
+	const group = 0
+
+	d.AddHandlerToGroup(handlers.NewNamedhandler(removeMe, handlers.NewMessage(message.All, nil)), group)
+
+	if found := d.RemoveHandlerFromGroup(removeMe, group); !found {
+		t.Errorf("RemoveHandlerFromGroup() = %v, want true", found)
+	}
+}
+
+func TestDispatcher_RemoveOneHandlerFromGroup(t *testing.T) {
+	d := ext.NewDispatcher(nil)
+
+	const removeMe = "remove_me"
+	const group = 0
+
+	// Load handler twice.
+	d.AddHandlerToGroup(handlers.NewNamedhandler(removeMe, handlers.NewMessage(message.All, nil)), group)
+	d.AddHandlerToGroup(handlers.NewNamedhandler(removeMe, handlers.NewMessage(message.All, nil)), group)
+
+	// Remove handler twice.
+	if found := d.RemoveHandlerFromGroup(removeMe, group); !found {
+		t.Errorf("RemoveHandlerFromGroup() = %v, want true", found)
+	}
+	if found := d.RemoveHandlerFromGroup(removeMe, group); !found {
+		t.Errorf("RemoveHandlerFromGroup() = %v, want true", found)
+	}
+	// fail! only 2 in there.
+	if found := d.RemoveHandlerFromGroup(removeMe, group); found {
+		t.Errorf("RemoveHandlerFromGroup() = %v, want false", found)
+	}
+}
+
+func TestDispatcher_RemoveHandlerNonExistingHandlerFromGroup(t *testing.T) {
+	d := ext.NewDispatcher(nil)
+
+	const keepMe = "keep_me"
+	const removeMe = "remove_me"
+	const group = 0
+
+	d.AddHandlerToGroup(handlers.NewNamedhandler(keepMe, handlers.NewMessage(message.All, nil)), group)
+
+	if found := d.RemoveHandlerFromGroup(removeMe, group); found {
+		t.Errorf("RemoveHandlerFromGroup() = %v, want false", found)
+	}
+}
+
+func TestDispatcher_RemoveHandlerHandlerFromNonExistingGroup(t *testing.T) {
+	d := ext.NewDispatcher(nil)
+
+	const removeMe = "remove_me"
+	const group = 0
+	const wrongGroup = 1
+	d.AddHandlerToGroup(handlers.NewNamedhandler(removeMe, handlers.NewMessage(message.All, nil)), group)
+
+	if found := d.RemoveHandlerFromGroup(removeMe, wrongGroup); found {
+		t.Errorf("RemoveHandlerFromGroup() = %v, want false", found)
+	}
+}
diff --git a/ext/handlers/named.go b/ext/handlers/named.go
index c734a520..ca91ca68 100644
--- a/ext/handlers/named.go
+++ b/ext/handlers/named.go
@@ -14,3 +14,10 @@ type Named struct {
 func (n Named) Name() string {
 	return n.CustomName
 }
+
+func NewNamedhandler(name string, handler ext.Handler) Named {
+	return Named{
+		CustomName: name,
+		Handler:    handler,
+	}
+}

From 250bec67c8d22262b9d7650d3a093c5a699d5f9c Mon Sep 17 00:00:00 2001
From: Paul Larsen <pnvlarsen@gmail.com>
Date: Mon, 2 May 2022 22:07:46 +0100
Subject: [PATCH 03/11] fix lint

---
 .golangci.yaml | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/.golangci.yaml b/.golangci.yaml
index a23b0843..74f8eef7 100644
--- a/.golangci.yaml
+++ b/.golangci.yaml
@@ -62,7 +62,7 @@ linters:
     #    - nilassign
     - nilerr
     - noctx
-    #    - nolintlint
+    - nolintlint
     #    - paralleltest
     - prealloc
     - promlinter
@@ -90,6 +90,9 @@ issues:
         - gosec
       text: "G404:" # warning about insecure math/rand. We dont care about this in tests!
       path: "\\w*_test.go"
+    - linters:
+        - gosimple
+      text: "S1023:" # allow redundant return statements. They can be nice for readability.
 
   # Enable default excludes, for common sense values.
   exclude-use-default: true

From c5f59e29d0a330985d50e6430850799907d15e1c Mon Sep 17 00:00:00 2001
From: Paul Larsen <pnvlarsen@gmail.com>
Date: Mon, 2 May 2022 22:21:23 +0100
Subject: [PATCH 04/11] Add warnings that methods are not thread safe

---
 ext/dispatcher.go | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/ext/dispatcher.go b/ext/dispatcher.go
index 6fc0e54f..9ab78dda 100644
--- a/ext/dispatcher.go
+++ b/ext/dispatcher.go
@@ -223,11 +223,13 @@ func (d *Dispatcher) Stop() {
 
 // AddHandler adds a new handler to the dispatcher. The dispatcher will call CheckUpdate() to see whether the handler
 // should be executed, and then HandleUpdate() to execute it.
+// Warning: Not thread safe.
 func (d *Dispatcher) AddHandler(handler Handler) {
 	d.AddHandlerToGroup(handler, 0)
 }
 
 // AddHandlerToGroup adds a handler to a specific group; lowest number will be processed first.
+// Warning: Not thread safe.
 func (d *Dispatcher) AddHandlerToGroup(handler Handler, group int) {
 	currHandlers, ok := d.handlers[group]
 	if !ok {
@@ -240,6 +242,7 @@ func (d *Dispatcher) AddHandlerToGroup(handler Handler, group int) {
 // RemoveHandlerFromGroup removes a handler by name from the specified group.
 // If multiple handlers have the same name, only the first one is removed.
 // Returns true if the handler was successfully removed.
+// Warning: Not thread safe.
 func (d *Dispatcher) RemoveHandlerFromGroup(handlerName string, group int) bool {
 	currHandlers, ok := d.handlers[group]
 	if !ok {
@@ -265,6 +268,7 @@ func (d *Dispatcher) RemoveHandlerFromGroup(handlerName string, group int) bool
 
 // RemoveGroup removes an entire group from the dispatcher's processing.
 // If group can't be found, this is a noop.
+// Warning: Not thread safe.
 func (d *Dispatcher) RemoveGroup(group int) {
 	if _, ok := d.handlers[group]; !ok {
 		// Group doesn't exist in map, so already removed.

From e8914248db85d649567256fa9b51b94d7c284316 Mon Sep 17 00:00:00 2001
From: Paul Larsen <pnvlarsen@gmail.com>
Date: Wed, 23 Aug 2023 02:01:07 +0200
Subject: [PATCH 05/11] Fix lints

---
 .golangci.yaml             | 2 +-
 ext/dispatcher_ext_test.go | 5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/.golangci.yaml b/.golangci.yaml
index 74f8eef7..ef70c4fb 100644
--- a/.golangci.yaml
+++ b/.golangci.yaml
@@ -62,7 +62,7 @@ linters:
     #    - nilassign
     - nilerr
     - noctx
-    - nolintlint
+    #    - nolintlint
     #    - paralleltest
     - prealloc
     - promlinter
diff --git a/ext/dispatcher_ext_test.go b/ext/dispatcher_ext_test.go
index efa5695a..6a122e8f 100644
--- a/ext/dispatcher_ext_test.go
+++ b/ext/dispatcher_ext_test.go
@@ -100,9 +100,12 @@ func TestDispatcher(t *testing.T) {
 			}
 
 			t.Log("Processing one update...")
-			d.ProcessUpdate(nil, &gotgbot.Update{
+			err := d.ProcessUpdate(nil, &gotgbot.Update{
 				Message: &gotgbot.Message{Text: "test text"},
 			}, nil)
+			if err != nil {
+				t.Errorf("Unexpected error while processing updates: %s", err.Error())
+			}
 
 			// ensure events handled in order
 			if !sort.IntsAreSorted(events) {

From 7cb7820a616b70fde1b96c1ec888d99c174d40c1 Mon Sep 17 00:00:00 2001
From: Paul Larsen <pnvlarsen@gmail.com>
Date: Wed, 23 Aug 2023 15:43:04 +0200
Subject: [PATCH 06/11] Add threadsafe mutexes to protect against undefined
 behaviour from concurrent modification

---
 ext/dispatcher.go           |  62 +++---------------
 ext/handler_mapping.go      |  94 +++++++++++++++++++++++++++
 ext/handler_mapping_test.go | 122 ++++++++++++++++++++++++++++++++++++
 3 files changed, 226 insertions(+), 52 deletions(-)
 create mode 100644 ext/handler_mapping.go
 create mode 100644 ext/handler_mapping_test.go

diff --git a/ext/dispatcher.go b/ext/dispatcher.go
index 9ab78dda..9fbd5e7f 100644
--- a/ext/dispatcher.go
+++ b/ext/dispatcher.go
@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"log"
 	"runtime/debug"
-	"sort"
 	"strings"
 	"sync"
 
@@ -71,10 +70,8 @@ type Dispatcher struct {
 	// If nil, logging is done via the log package's standard logger.
 	ErrorLog *log.Logger
 
-	// handlerGroups represents the list of available handler groups, numerically sorted.
-	handlerGroups []int
-	// handlers represents all available handles, split into groups (see handlerGroups).
-	handlers map[int][]Handler
+	// handlers represents all available handlers.
+	handlers handlerMappings
 
 	// limiter is how we limit the maximum number of goroutines for handling updates.
 	// if nil, this is a limitless dispatcher.
@@ -152,7 +149,7 @@ func NewDispatcher(opts *DispatcherOpts) *Dispatcher {
 		Panic:            panicHandler,
 		UnhandledErrFunc: unhandledErrFunc,
 		ErrorLog:         errLog,
-		handlers:         make(map[int][]Handler),
+		handlers:         handlerMappings{},
 		limiter:          limiter,
 		waitGroup:        sync.WaitGroup{},
 	}
@@ -230,13 +227,8 @@ func (d *Dispatcher) AddHandler(handler Handler) {
 
 // AddHandlerToGroup adds a handler to a specific group; lowest number will be processed first.
 // Warning: Not thread safe.
-func (d *Dispatcher) AddHandlerToGroup(handler Handler, group int) {
-	currHandlers, ok := d.handlers[group]
-	if !ok {
-		d.handlerGroups = append(d.handlerGroups, group)
-		sort.Ints(d.handlerGroups)
-	}
-	d.handlers[group] = append(currHandlers, handler)
+func (d *Dispatcher) AddHandlerToGroup(h Handler, group int) {
+	d.handlers.add(h, group)
 }
 
 // RemoveHandlerFromGroup removes a handler by name from the specified group.
@@ -244,48 +236,14 @@ func (d *Dispatcher) AddHandlerToGroup(handler Handler, group int) {
 // Returns true if the handler was successfully removed.
 // Warning: Not thread safe.
 func (d *Dispatcher) RemoveHandlerFromGroup(handlerName string, group int) bool {
-	currHandlers, ok := d.handlers[group]
-	if !ok {
-		// group does not exist; removal failed.
-		return false
-	}
-
-	for i, handler := range currHandlers {
-		if handler.Name() == handlerName {
-			d.handlers[group] = append(currHandlers[:i], currHandlers[i+1:]...)
-
-			// Had one item, we removed one, so none left.
-			if len(currHandlers) == 1 {
-				// No more handlers, so group should also be removed.
-				d.RemoveGroup(group)
-			}
-			return true
-		}
-	}
-	// handler not found - removal failed.
-	return false
+	return d.handlers.remove(handlerName, group)
 }
 
 // RemoveGroup removes an entire group from the dispatcher's processing.
 // If group can't be found, this is a noop.
 // Warning: Not thread safe.
-func (d *Dispatcher) RemoveGroup(group int) {
-	if _, ok := d.handlers[group]; !ok {
-		// Group doesn't exist in map, so already removed.
-		return
-	}
-
-	groups := d.handlerGroups
-	for j, handlerGroup := range groups {
-		if handlerGroup == group {
-			d.handlerGroups = append(groups[:j], groups[j+1:]...)
-			delete(d.handlers, group)
-			// Group found, and deleted. Success!
-			return
-		}
-	}
-	// Group not found in list - so already removed.
-	return
+func (d *Dispatcher) RemoveGroup(group int) bool {
+	return d.handlers.removeGroup(group)
 }
 
 // processRawUpdate takes a JSON update to be unmarshalled and processed by Dispatcher.ProcessUpdate.
@@ -325,8 +283,8 @@ func (d *Dispatcher) ProcessUpdate(b *gotgbot.Bot, u *gotgbot.Update, data map[s
 }
 
 func (d *Dispatcher) iterateOverHandlerGroups(b *gotgbot.Bot, ctx *Context) error {
-	for _, groupNum := range d.handlerGroups {
-		for _, handler := range d.handlers[groupNum] {
+	for _, groups := range d.handlers.getGroups() {
+		for _, handler := range groups {
 			if !handler.CheckUpdate(b, ctx) {
 				// Handler filter doesn't match this update; continue.
 				continue
diff --git a/ext/handler_mapping.go b/ext/handler_mapping.go
new file mode 100644
index 00000000..2709273e
--- /dev/null
+++ b/ext/handler_mapping.go
@@ -0,0 +1,94 @@
+package ext
+
+import (
+	"sort"
+	"sync"
+)
+
+type handlerMappings struct {
+	// mutex is used to ensure everything threadsafe.
+	mutex sync.RWMutex
+
+	// handlerGroups represents the list of available handler groups, numerically sorted.
+	handlerGroups []int
+	// handlers represents all available handlers, split into groups (see handlerGroups).
+	handlers map[int][]Handler
+}
+
+func (m *handlerMappings) add(h Handler, group int) {
+	m.mutex.Lock()
+	defer m.mutex.Unlock()
+
+	if m.handlers == nil {
+		m.handlers = map[int][]Handler{}
+	}
+	currHandlers, ok := m.handlers[group]
+	if !ok {
+		m.handlerGroups = append(m.handlerGroups, group)
+		sort.Ints(m.handlerGroups)
+	}
+	m.handlers[group] = append(currHandlers, h)
+}
+
+func (m *handlerMappings) remove(name string, group int) bool {
+	m.mutex.Lock()
+	defer m.mutex.Unlock()
+
+	currHandlers, ok := m.handlers[group]
+	if !ok {
+		// group does not exist; removal failed.
+		return false
+	}
+
+	for i, handler := range currHandlers {
+		if handler.Name() != name {
+			continue
+		}
+
+		// Only one item left, so just delete the group entirely.
+		if len(currHandlers) == 1 {
+			m.handlerGroups = append(m.handlerGroups[:group], m.handlerGroups[group+1:]...)
+			delete(m.handlers, group)
+			return true
+		}
+
+		m.handlers[group] = append(currHandlers[:i], currHandlers[i+1:]...)
+		return true
+	}
+	// handler not found - removal failed.
+	return false
+}
+
+func (m *handlerMappings) removeGroup(group int) bool {
+	m.mutex.Lock()
+	defer m.mutex.Unlock()
+
+	if _, ok := m.handlers[group]; !ok {
+		// Group doesn't exist in map, so already removed.
+		return false
+	}
+
+	for j, handlerGroup := range m.handlerGroups {
+		if handlerGroup != group {
+			continue
+		}
+
+		m.handlerGroups = append(m.handlerGroups[:j], m.handlerGroups[j+1:]...)
+		delete(m.handlers, group)
+		// Group found, and deleted. Success!
+		return true
+	}
+	// Group not found in list - so already removed.
+	return false
+}
+
+func (m *handlerMappings) getGroups() [][]Handler {
+	m.mutex.RLock()
+	defer m.mutex.RUnlock()
+
+	var handlers [][]Handler
+	for _, num := range m.handlerGroups {
+		handlers = append(handlers, m.handlers[num])
+	}
+	return handlers
+}
diff --git a/ext/handler_mapping_test.go b/ext/handler_mapping_test.go
new file mode 100644
index 00000000..1eb4810d
--- /dev/null
+++ b/ext/handler_mapping_test.go
@@ -0,0 +1,122 @@
+package ext
+
+import (
+	"testing"
+
+	"github.com/PaulSonOfLars/gotgbot/v2"
+)
+
+type dummy struct {
+	f    func(bot *gotgbot.Bot, ctx *Context) error
+	name string
+}
+
+func (d dummy) CheckUpdate(b *gotgbot.Bot, ctx *Context) bool {
+	return true
+}
+
+func (d dummy) HandleUpdate(b *gotgbot.Bot, ctx *Context) error {
+	return d.f(b, ctx)
+}
+
+func (d dummy) Name() string {
+	return "dummy" + d.name
+}
+
+// This test should demonstrate that once obtained, a list will not be changed by any additions/removals to that list by another call.
+func Test_handlerMappings_getGroupsConcurrentSafe(t *testing.T) {
+	m := handlerMappings{}
+	firstHandler := dummy{name: "first"}
+	secondHandler := dummy{name: "second"}
+
+	// We expect 0 groups at the start
+	startGroups := m.getGroups()
+	if len(startGroups) != 0 {
+		t.Errorf("failed predicate group layout")
+	}
+
+	// Add one handler.
+	m.add(firstHandler, 0)
+	currGroups := m.getGroups()
+	if len(currGroups) != 1 && len(startGroups) != 0 {
+		t.Errorf("Start groups should be 0, curr groups should be 1; got %d and %d", len(startGroups), len(currGroups))
+	}
+	if len(currGroups[0]) != 1 {
+		t.Errorf("length of group 0 in the groups list should be 1; got %d", len(currGroups[0]))
+	}
+
+	// Add a second handler.
+	m.add(secondHandler, 0)
+	newGroups := m.getGroups()
+	if len(currGroups[0]) != 1 {
+		t.Errorf("length of group 0 in currGroups should be 1; got %d", len(currGroups[0]))
+	}
+	if len(newGroups[0]) != 2 {
+		t.Errorf("length of group 0 in the newGroups 1; got %d", len(newGroups[0]))
+	}
+
+	// Remove second handler..
+	ok := m.remove(secondHandler.Name(), 0)
+	if !ok {
+		t.Errorf("failed to remove second handler")
+	}
+	delGroups := m.getGroups()
+	if len(currGroups[0]) != 1 {
+		t.Errorf("length of group 0 in currGroups should be 1; got %d", len(currGroups[0]))
+	}
+	if len(newGroups[0]) != 2 {
+		t.Errorf("length of group 0 in the newGroups 1; got %d", len(newGroups[0]))
+	}
+	if len(delGroups[0]) != 1 {
+		t.Errorf("length of group 0 in delGroups should be 1; got %d", len(delGroups[0]))
+	}
+
+	// Re-add second handler.
+	m.add(secondHandler, 0)
+	reAddedGroups := m.getGroups()
+	if len(currGroups[0]) != 1 &&
+		currGroups[0][0].Name() == firstHandler.Name() {
+		t.Errorf("length of group 0 in currGroups should be 1; got %d", len(currGroups[0]))
+	}
+	if len(newGroups[0]) != 2 &&
+		newGroups[0][1].Name() == secondHandler.Name() {
+		t.Errorf("length of group 0 in the newGroups 1; got %d", len(newGroups[0]))
+	}
+	if len(delGroups[0]) != 1 &&
+		delGroups[0][0].Name() == firstHandler.Name() {
+		t.Errorf("length of group 0 in delGroups should be 1; got %d", len(delGroups[0]))
+	}
+	if len(reAddedGroups[0]) != 2 &&
+		newGroups[0][0].Name() == firstHandler.Name() &&
+		newGroups[0][1].Name() == secondHandler.Name() {
+		t.Errorf("length of group 0 in reAddedGroups should be 2; got %d", len(reAddedGroups[0]))
+	}
+
+	// Remove first handler.
+	ok = m.remove(firstHandler.Name(), 0)
+	if !ok {
+		t.Errorf("failed to remove second handler")
+	}
+	noFirstGroups := m.getGroups()
+	if len(currGroups[0]) != 1 &&
+		currGroups[0][0].Name() == firstHandler.Name() {
+		t.Errorf("length of group 0 in currGroups should be 1; got %d", len(currGroups[0]))
+	}
+	if len(newGroups[0]) != 2 &&
+		newGroups[0][1].Name() == secondHandler.Name() {
+		t.Errorf("length of group 0 in the newGroups 1; got %d", len(newGroups[0]))
+	}
+	if len(delGroups[0]) != 1 &&
+		delGroups[0][0].Name() == firstHandler.Name() {
+		t.Errorf("length of group 0 in delGroups should be 1; got %d", len(delGroups[0]))
+	}
+	if len(reAddedGroups[0]) != 2 &&
+		reAddedGroups[0][0].Name() == firstHandler.Name() &&
+		reAddedGroups[0][1].Name() == secondHandler.Name() {
+		t.Errorf("length of group 0 in reAddedGroups should be 2; got %d", len(reAddedGroups[0]))
+	}
+	if len(noFirstGroups[0]) != 1 &&
+		noFirstGroups[0][0].Name() == secondHandler.Name() {
+		t.Errorf("length of group 0 in noFirstGroups should be 2; got %d", len(noFirstGroups[0]))
+	}
+}

From e6771ba4ba118a676d5a0a6f6873cbeac75ed659 Mon Sep 17 00:00:00 2001
From: Paul Larsen <pnvlarsen@gmail.com>
Date: Wed, 23 Aug 2023 16:01:14 +0200
Subject: [PATCH 07/11] Improve tests, and fix the error that they uncovered

---
 ext/handler_mapping.go      |  7 +++-
 ext/handler_mapping_test.go | 80 ++++++++++++-------------------------
 2 files changed, 31 insertions(+), 56 deletions(-)

diff --git a/ext/handler_mapping.go b/ext/handler_mapping.go
index 2709273e..2237407c 100644
--- a/ext/handler_mapping.go
+++ b/ext/handler_mapping.go
@@ -52,7 +52,12 @@ func (m *handlerMappings) remove(name string, group int) bool {
 			return true
 		}
 
-		m.handlers[group] = append(currHandlers[:i], currHandlers[i+1:]...)
+		// Make sure to copy the handler list to ensure we don't change the values of the underlying arrays, which
+		// could cause slice access issues when used concurrently.
+		newHandlers := make([]Handler, len(m.handlers[group]))
+		copy(newHandlers, m.handlers[group])
+
+		m.handlers[group] = append(newHandlers[:i], newHandlers[i+1:]...)
 		return true
 	}
 	// handler not found - removal failed.
diff --git a/ext/handler_mapping_test.go b/ext/handler_mapping_test.go
index 1eb4810d..6d452569 100644
--- a/ext/handler_mapping_test.go
+++ b/ext/handler_mapping_test.go
@@ -41,19 +41,13 @@ func Test_handlerMappings_getGroupsConcurrentSafe(t *testing.T) {
 	if len(currGroups) != 1 && len(startGroups) != 0 {
 		t.Errorf("Start groups should be 0, curr groups should be 1; got %d and %d", len(startGroups), len(currGroups))
 	}
-	if len(currGroups[0]) != 1 {
-		t.Errorf("length of group 0 in the groups list should be 1; got %d", len(currGroups[0]))
-	}
+	checkList(t, "currGroups", currGroups[0], firstHandler)
 
 	// Add a second handler.
 	m.add(secondHandler, 0)
 	newGroups := m.getGroups()
-	if len(currGroups[0]) != 1 {
-		t.Errorf("length of group 0 in currGroups should be 1; got %d", len(currGroups[0]))
-	}
-	if len(newGroups[0]) != 2 {
-		t.Errorf("length of group 0 in the newGroups 1; got %d", len(newGroups[0]))
-	}
+	checkList(t, "newgroups;currGroups", currGroups[0], firstHandler)
+	checkList(t, "newgroups;newGroups", newGroups[0], firstHandler, secondHandler)
 
 	// Remove second handler..
 	ok := m.remove(secondHandler.Name(), 0)
@@ -61,36 +55,17 @@ func Test_handlerMappings_getGroupsConcurrentSafe(t *testing.T) {
 		t.Errorf("failed to remove second handler")
 	}
 	delGroups := m.getGroups()
-	if len(currGroups[0]) != 1 {
-		t.Errorf("length of group 0 in currGroups should be 1; got %d", len(currGroups[0]))
-	}
-	if len(newGroups[0]) != 2 {
-		t.Errorf("length of group 0 in the newGroups 1; got %d", len(newGroups[0]))
-	}
-	if len(delGroups[0]) != 1 {
-		t.Errorf("length of group 0 in delGroups should be 1; got %d", len(delGroups[0]))
-	}
+	checkList(t, "delgroups;currGroups", currGroups[0], firstHandler)
+	checkList(t, "delgroups;newGroups", newGroups[0], firstHandler, secondHandler)
+	checkList(t, "delgroups;delGroups", delGroups[0], firstHandler)
 
 	// Re-add second handler.
 	m.add(secondHandler, 0)
 	reAddedGroups := m.getGroups()
-	if len(currGroups[0]) != 1 &&
-		currGroups[0][0].Name() == firstHandler.Name() {
-		t.Errorf("length of group 0 in currGroups should be 1; got %d", len(currGroups[0]))
-	}
-	if len(newGroups[0]) != 2 &&
-		newGroups[0][1].Name() == secondHandler.Name() {
-		t.Errorf("length of group 0 in the newGroups 1; got %d", len(newGroups[0]))
-	}
-	if len(delGroups[0]) != 1 &&
-		delGroups[0][0].Name() == firstHandler.Name() {
-		t.Errorf("length of group 0 in delGroups should be 1; got %d", len(delGroups[0]))
-	}
-	if len(reAddedGroups[0]) != 2 &&
-		newGroups[0][0].Name() == firstHandler.Name() &&
-		newGroups[0][1].Name() == secondHandler.Name() {
-		t.Errorf("length of group 0 in reAddedGroups should be 2; got %d", len(reAddedGroups[0]))
-	}
+	checkList(t, "readded;currGroups", currGroups[0], firstHandler)
+	checkList(t, "readded;newGroups", newGroups[0], firstHandler, secondHandler)
+	checkList(t, "readded;delGroups", delGroups[0], firstHandler)
+	checkList(t, "readded;reAddedGroups", reAddedGroups[0], firstHandler, secondHandler)
 
 	// Remove first handler.
 	ok = m.remove(firstHandler.Name(), 0)
@@ -98,25 +73,20 @@ func Test_handlerMappings_getGroupsConcurrentSafe(t *testing.T) {
 		t.Errorf("failed to remove second handler")
 	}
 	noFirstGroups := m.getGroups()
-	if len(currGroups[0]) != 1 &&
-		currGroups[0][0].Name() == firstHandler.Name() {
-		t.Errorf("length of group 0 in currGroups should be 1; got %d", len(currGroups[0]))
-	}
-	if len(newGroups[0]) != 2 &&
-		newGroups[0][1].Name() == secondHandler.Name() {
-		t.Errorf("length of group 0 in the newGroups 1; got %d", len(newGroups[0]))
-	}
-	if len(delGroups[0]) != 1 &&
-		delGroups[0][0].Name() == firstHandler.Name() {
-		t.Errorf("length of group 0 in delGroups should be 1; got %d", len(delGroups[0]))
-	}
-	if len(reAddedGroups[0]) != 2 &&
-		reAddedGroups[0][0].Name() == firstHandler.Name() &&
-		reAddedGroups[0][1].Name() == secondHandler.Name() {
-		t.Errorf("length of group 0 in reAddedGroups should be 2; got %d", len(reAddedGroups[0]))
-	}
-	if len(noFirstGroups[0]) != 1 &&
-		noFirstGroups[0][0].Name() == secondHandler.Name() {
-		t.Errorf("length of group 0 in noFirstGroups should be 2; got %d", len(noFirstGroups[0]))
+	checkList(t, "nofirst;currGroups", currGroups[0], firstHandler)
+	checkList(t, "nofirst;newGroups", newGroups[0], firstHandler, secondHandler)
+	checkList(t, "nofirst;delGroups", delGroups[0], firstHandler)
+	checkList(t, "nofirst;reAddedGroups", reAddedGroups[0], firstHandler, secondHandler)
+	checkList(t, "nofirst;noFirstGroups", noFirstGroups[0], secondHandler)
+}
+
+func checkList(t *testing.T, name string, got []Handler, expected ...Handler) {
+	if len(got) != len(expected) {
+		t.Errorf("mismatch on length of expected outputs for %s - got %d, expected %d", name, len(got), len(expected))
+	}
+	for idx, v := range got {
+		if v.Name() != expected[idx].Name() {
+			t.Errorf("unexpected output name for %s - IDX %d got %s, expected %s", name, idx, v.Name(), expected[idx].Name())
+		}
 	}
 }

From 107669aa4a0e2b3755458117cea2bf7c5415b7ba Mon Sep 17 00:00:00 2001
From: Paul Larsen <pnvlarsen@gmail.com>
Date: Wed, 23 Aug 2023 16:01:55 +0200
Subject: [PATCH 08/11] pre-allocate handlers when using getGroups

---
 ext/handler_mapping.go | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ext/handler_mapping.go b/ext/handler_mapping.go
index 2237407c..f4a28112 100644
--- a/ext/handler_mapping.go
+++ b/ext/handler_mapping.go
@@ -91,9 +91,9 @@ func (m *handlerMappings) getGroups() [][]Handler {
 	m.mutex.RLock()
 	defer m.mutex.RUnlock()
 
-	var handlers [][]Handler
-	for _, num := range m.handlerGroups {
-		handlers = append(handlers, m.handlers[num])
+	allHandlers := make([][]Handler, len(m.handlerGroups))
+	for idx, num := range m.handlerGroups {
+		allHandlers[idx] = m.handlers[num]
 	}
-	return handlers
+	return allHandlers
 }

From 2f333e37dc84fc52fa97bdcad51db31214011340 Mon Sep 17 00:00:00 2001
From: Paul Larsen <pnvlarsen@gmail.com>
Date: Thu, 31 Aug 2023 21:30:10 +0100
Subject: [PATCH 09/11] Add additional tests for handler mappings

---
 ext/handler_mapping.go      | 23 ++++++++++++++-----
 ext/handler_mapping_test.go | 44 +++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 5 deletions(-)

diff --git a/ext/handler_mapping.go b/ext/handler_mapping.go
index f4a28112..c25994e5 100644
--- a/ext/handler_mapping.go
+++ b/ext/handler_mapping.go
@@ -40,14 +40,18 @@ func (m *handlerMappings) remove(name string, group int) bool {
 		return false
 	}
 
-	for i, handler := range currHandlers {
+	for idx, handler := range currHandlers {
 		if handler.Name() != name {
 			continue
 		}
 
 		// Only one item left, so just delete the group entirely.
 		if len(currHandlers) == 1 {
-			m.handlerGroups = append(m.handlerGroups[:group], m.handlerGroups[group+1:]...)
+			// get index of the current group to remove it from the list of handlergroups
+			gIdx := getIndex(group, m.handlerGroups)
+			if gIdx != -1 {
+				m.handlerGroups = append(m.handlerGroups[:gIdx], m.handlerGroups[gIdx+1:]...)
+			}
 			delete(m.handlers, group)
 			return true
 		}
@@ -57,13 +61,22 @@ func (m *handlerMappings) remove(name string, group int) bool {
 		newHandlers := make([]Handler, len(m.handlers[group]))
 		copy(newHandlers, m.handlers[group])
 
-		m.handlers[group] = append(newHandlers[:i], newHandlers[i+1:]...)
+		m.handlers[group] = append(newHandlers[:idx], newHandlers[idx+1:]...)
 		return true
 	}
 	// handler not found - removal failed.
 	return false
 }
 
+func getIndex(find int, is []int) int {
+	for i, v := range is {
+		if v == find {
+			return i
+		}
+	}
+	return -1
+}
+
 func (m *handlerMappings) removeGroup(group int) bool {
 	m.mutex.Lock()
 	defer m.mutex.Unlock()
@@ -73,12 +86,12 @@ func (m *handlerMappings) removeGroup(group int) bool {
 		return false
 	}
 
-	for j, handlerGroup := range m.handlerGroups {
+	for idx, handlerGroup := range m.handlerGroups {
 		if handlerGroup != group {
 			continue
 		}
 
-		m.handlerGroups = append(m.handlerGroups[:j], m.handlerGroups[j+1:]...)
+		m.handlerGroups = append(m.handlerGroups[:idx], m.handlerGroups[idx+1:]...)
 		delete(m.handlers, group)
 		// Group found, and deleted. Success!
 		return true
diff --git a/ext/handler_mapping_test.go b/ext/handler_mapping_test.go
index 6d452569..a8fcc1a6 100644
--- a/ext/handler_mapping_test.go
+++ b/ext/handler_mapping_test.go
@@ -90,3 +90,47 @@ func checkList(t *testing.T, name string, got []Handler, expected ...Handler) {
 		}
 	}
 }
+
+func Test_handlerMappings_remove(t *testing.T) {
+	m := &handlerMappings{}
+	handler := dummy{name: "test"}
+
+	t.Run("nonExistent", func(t *testing.T) {
+		// removing an item that doesnt exist returns "false"
+		if got := m.remove(handler.Name(), 0); got {
+			t.Errorf("remove() = %v, want false", got)
+		}
+	})
+
+	t.Run("removalSuccess", func(t *testing.T) {
+		m.add(handler, 0)
+		// removing an item that DOES exist, returns true
+		if got := m.remove(handler.Name(), 0); !got {
+			t.Errorf("remove() = %v, want true", got)
+		}
+		// And so the second time, it returns false
+		if got := m.remove(handler.Name(), 0); got {
+			t.Errorf("remove() = %v, want false", got)
+		}
+	})
+
+	t.Run("removalSuccess", func(t *testing.T) {
+		m.add(handler, 0)
+		// removing an item that DOES exist, returns true
+		if got := m.remove(handler.Name(), 0); !got {
+			t.Errorf("remove() = %v, want true", got)
+		}
+		// And so the second time, it returns false
+		if got := m.remove(handler.Name(), 0); got {
+			t.Errorf("remove() = %v, want false", got)
+		}
+	})
+
+	t.Run("removalDifferentIndexes", func(t *testing.T) {
+		m.add(handler, 1)
+		m.add(handler, 2)
+		if got := m.remove(handler.Name(), 2); !got {
+			t.Errorf("remove() = %v, want true", got)
+		}
+	})
+}

From c741c6e0d698c1351dbdc2e68252d5eab9a392f5 Mon Sep 17 00:00:00 2001
From: Paul Larsen <pnvlarsen@gmail.com>
Date: Sat, 2 Sep 2023 16:07:05 +0100
Subject: [PATCH 10/11] remove invalid thread safety comments on group
 add/remove

---
 ext/dispatcher.go | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/ext/dispatcher.go b/ext/dispatcher.go
index 9fbd5e7f..f64e32ed 100644
--- a/ext/dispatcher.go
+++ b/ext/dispatcher.go
@@ -220,13 +220,11 @@ func (d *Dispatcher) Stop() {
 
 // AddHandler adds a new handler to the dispatcher. The dispatcher will call CheckUpdate() to see whether the handler
 // should be executed, and then HandleUpdate() to execute it.
-// Warning: Not thread safe.
 func (d *Dispatcher) AddHandler(handler Handler) {
 	d.AddHandlerToGroup(handler, 0)
 }
 
 // AddHandlerToGroup adds a handler to a specific group; lowest number will be processed first.
-// Warning: Not thread safe.
 func (d *Dispatcher) AddHandlerToGroup(h Handler, group int) {
 	d.handlers.add(h, group)
 }
@@ -234,14 +232,12 @@ func (d *Dispatcher) AddHandlerToGroup(h Handler, group int) {
 // RemoveHandlerFromGroup removes a handler by name from the specified group.
 // If multiple handlers have the same name, only the first one is removed.
 // Returns true if the handler was successfully removed.
-// Warning: Not thread safe.
 func (d *Dispatcher) RemoveHandlerFromGroup(handlerName string, group int) bool {
 	return d.handlers.remove(handlerName, group)
 }
 
 // RemoveGroup removes an entire group from the dispatcher's processing.
 // If group can't be found, this is a noop.
-// Warning: Not thread safe.
 func (d *Dispatcher) RemoveGroup(group int) bool {
 	return d.handlers.removeGroup(group)
 }

From abfedf2c11710f48541f8c925e4aa68032aaa8ac Mon Sep 17 00:00:00 2001
From: Paul Larsen <pnvlarsen@gmail.com>
Date: Sat, 2 Sep 2023 16:11:03 +0100
Subject: [PATCH 11/11] remove redundant dummy redeclaration

---
 ext/common_test.go          |  3 ++-
 ext/handler.go              |  4 +---
 ext/handler_mapping_test.go | 25 +++----------------------
 3 files changed, 6 insertions(+), 26 deletions(-)

diff --git a/ext/common_test.go b/ext/common_test.go
index 2ca8594c..c82affd6 100644
--- a/ext/common_test.go
+++ b/ext/common_test.go
@@ -6,6 +6,7 @@ import (
 
 type DummyHandler struct {
 	F func(b *gotgbot.Bot, ctx *Context) error
+	N string
 }
 
 func (d DummyHandler) CheckUpdate(b *gotgbot.Bot, ctx *Context) bool {
@@ -17,7 +18,7 @@ func (d DummyHandler) HandleUpdate(b *gotgbot.Bot, ctx *Context) error {
 }
 
 func (d DummyHandler) Name() string {
-	return "dummy"
+	return "dummy" + d.N
 }
 
 func (u *Updater) InjectUpdate(token string, upd gotgbot.Update) error {
diff --git a/ext/handler.go b/ext/handler.go
index 85d96f9f..23096d96 100644
--- a/ext/handler.go
+++ b/ext/handler.go
@@ -1,8 +1,6 @@
 package ext
 
-import (
-	"github.com/PaulSonOfLars/gotgbot/v2"
-)
+import "github.com/PaulSonOfLars/gotgbot/v2"
 
 type Handler interface {
 	// CheckUpdate checks whether the update should handled by this handler.
diff --git a/ext/handler_mapping_test.go b/ext/handler_mapping_test.go
index a8fcc1a6..77d5cee1 100644
--- a/ext/handler_mapping_test.go
+++ b/ext/handler_mapping_test.go
@@ -2,32 +2,13 @@ package ext
 
 import (
 	"testing"
-
-	"github.com/PaulSonOfLars/gotgbot/v2"
 )
 
-type dummy struct {
-	f    func(bot *gotgbot.Bot, ctx *Context) error
-	name string
-}
-
-func (d dummy) CheckUpdate(b *gotgbot.Bot, ctx *Context) bool {
-	return true
-}
-
-func (d dummy) HandleUpdate(b *gotgbot.Bot, ctx *Context) error {
-	return d.f(b, ctx)
-}
-
-func (d dummy) Name() string {
-	return "dummy" + d.name
-}
-
 // This test should demonstrate that once obtained, a list will not be changed by any additions/removals to that list by another call.
 func Test_handlerMappings_getGroupsConcurrentSafe(t *testing.T) {
 	m := handlerMappings{}
-	firstHandler := dummy{name: "first"}
-	secondHandler := dummy{name: "second"}
+	firstHandler := DummyHandler{N: "first"}
+	secondHandler := DummyHandler{N: "second"}
 
 	// We expect 0 groups at the start
 	startGroups := m.getGroups()
@@ -93,7 +74,7 @@ func checkList(t *testing.T, name string, got []Handler, expected ...Handler) {
 
 func Test_handlerMappings_remove(t *testing.T) {
 	m := &handlerMappings{}
-	handler := dummy{name: "test"}
+	handler := DummyHandler{N: "test"}
 
 	t.Run("nonExistent", func(t *testing.T) {
 		// removing an item that doesnt exist returns "false"