Solve race condition on Command replacement

bot_test.go

@@ -107,7 +107,7 @@ func TestDefaultBot_AppendCommand(t *testing.T) {
 	myBot.AppendCommand(command)
 
 	registeredCommands := myBot.commands
-	if len(*registeredCommands) != 1 {
+	if len(registeredCommands.collection) != 1 {
 		t.Errorf("1 registered command should exists: %#v.", registeredCommands)
 	}
 }
@@ -136,8 +136,8 @@ func TestDefaultBot_Respond_StorageAcquisitionError(t *testing.T) {
 
 func TestDefaultBot_Respond_WithCommandError(t *testing.T) {
 	expectedErr := errors.New("expected")
-	myBot := &defaultBot{
-		commands: &Commands{
+	commands := &Commands{
+		collection: []Command{
 			&DummyCommand{
 				MatchFunc: func(_ Input) bool {
 					return true
@@ -148,6 +148,9 @@ func TestDefaultBot_Respond_WithCommandError(t *testing.T) {
 			},
 		},
 	}
+	myBot := &defaultBot{
+		commands: commands,
+	}
 
 	err := myBot.Respond(context.TODO(), &DummyInput{})
 
@@ -213,7 +216,7 @@ func TestDefaultBot_Respond_WithContextButMessage(t *testing.T) {
 	isSent := false
 	myBot := &defaultBot{
 		userContextStorage: dummyStorage,
-		commands:           &Commands{command},
+		commands:           &Commands{collection: []Command{command}},
 		sendMessageFunc: func(_ context.Context, output Output) {
 			isSent = true
 		},
@@ -330,7 +333,7 @@ func TestDefaultBot_Respond_WithContextStorageSetError(t *testing.T) {
 			sendMessageCalled = true
 		},
 		userContextStorage: dummyStorage,
-		commands:           &Commands{cmd},
+		commands:           &Commands{collection: []Command{cmd}},
 	}
 
 	err := myBot.Respond(context.TODO(), &DummyInput{})
@@ -372,7 +375,7 @@ func TestDefaultBot_Respond_UserContextWithoutStorage(t *testing.T) {
 		sendMessageFunc: func(_ context.Context, output Output) {
 			sendMessageCalled = true
 		},
-		commands:           &Commands{cmd},
+		commands:           &Commands{collection: []Command{cmd}},
 		userContextStorage: nil,
 	}
 
@@ -433,7 +436,7 @@ func TestDefaultBot_Respond_Help(t *testing.T) {
 	}
 	myBot := &defaultBot{
 		userContextStorage: dummyStorage,
-		commands:           &Commands{cmd},
+		commands:           &Commands{collection: []Command{cmd}},
 		sendMessageFunc: func(_ context.Context, output Output) {
 			givenOutput = output
 		},

command.go

@@ -11,6 +11,7 @@ import (
 	"path"
 	"regexp"
 	"strings"
+	"sync"
 )
 
 var (
@@ -100,28 +101,37 @@ func StripMessage(pattern *regexp.Regexp, input string) string {
 }
 
 // Commands stashes all registered Command.
-type Commands []Command
+type Commands struct {
+	collection []Command
+	mutex      sync.RWMutex
+}
 
 // NewCommands creates and returns new Commands instance.
 func NewCommands() *Commands {
-	return &Commands{}
+	return &Commands{
+		collection: []Command{},
+		mutex:      sync.RWMutex{},
+	}
 }
 
 // Append let developers register new Command to its internal stash.
 // If any command is registered with the same ID, the old one is replaced in favor of new one.
 func (commands *Commands) Append(command Command) {
+	commands.mutex.Lock()
+	defer commands.mutex.Unlock()
+
 	// See if command with the same identifier exists.
-	for i, cmd := range *commands {
+	for i, cmd := range commands.collection {
 		if cmd.Identifier() == command.Identifier() {
 			log.Infof("replacing old command in favor of newly appending one: %s.", command.Identifier())
-			(*commands)[i] = command
+			commands.collection[i] = command
 			return
 		}
 	}
 
 	// Not stored, then append to the last.
 	log.Infof("appending new command: %s.", command.Identifier())
-	*commands = append(*commands, command)
+	commands.collection = append(commands.collection, command)
 }
 
 // FindFirstMatched look for first matching command by calling Command's Match method: First Command.Match to return true
@@ -130,7 +140,10 @@ func (commands *Commands) Append(command Command) {
 // This check is run in the order of Command registration: Earlier the Commands.Append is called, the command is checked
 // earlier. So register important Command first.
 func (commands *Commands) FindFirstMatched(input Input) Command {
-	for _, command := range *commands {
+	commands.mutex.RLock()
+	defer commands.mutex.RUnlock()
+
+	for _, command := range commands.collection {
 		if command.Match(input) {
 			return command
 		}
@@ -151,8 +164,11 @@ func (commands *Commands) ExecuteFirstMatched(ctx context.Context, input Input)
 
 // Helps returns underlying commands help messages in a form of *CommandHelps.
 func (commands *Commands) Helps() *CommandHelps {
+	commands.mutex.RLock()
+	defer commands.mutex.RUnlock()
+
 	helps := &CommandHelps{}
-	for _, command := range *commands {
+	for _, command := range commands.collection {
 		h := &CommandHelp{
 			Identifier:   command.Identifier(),
 			InputExample: command.InputExample(),

command_test.go

@@ -300,7 +300,7 @@ func TestCommands_FindFirstMatched(t *testing.T) {
 	irrelevantCommand2.MatchFunc = func(_ Input) bool {
 		return false
 	}
-	commands = &Commands{irrelevantCommand, echoCommand, irrelevantCommand2}
+	commands = &Commands{collection: []Command{irrelevantCommand, echoCommand, irrelevantCommand2}}
 
 	matchedCommand = commands.FindFirstMatched(&DummyInput{MessageValue: "echo"})
 	if matchedCommand == nil {
@@ -332,7 +332,7 @@ func TestCommands_ExecuteFirstMatched(t *testing.T) {
 	echoCommand.ExecuteFunc = func(_ context.Context, _ Input) (*CommandResponse, error) {
 		return &CommandResponse{Content: ""}, nil
 	}
-	commands = &Commands{echoCommand}
+	commands = &Commands{collection: []Command{echoCommand}}
 	response, err = commands.ExecuteFirstMatched(context.TODO(), input)
 	if err != nil {
 		t.Errorf("Unexpected error on command execution: %#v.", err)
@@ -361,27 +361,27 @@ func TestCommands_Append(t *testing.T) {
 
 	// First operation
 	commands.Append(command)
-	if len(*commands) == 0 {
+	if len(commands.collection) == 0 {
 		t.Fatal("Provided command was not appended.")
 	}
 
-	if (*commands)[0] != command {
-		t.Fatalf("Appended command is not the one provided: %#v", (*commands)[0])
+	if (commands.collection)[0] != command {
+		t.Fatalf("Appended command is not the one provided: %#v", commands.collection[0])
 	}
 
 	// Second operation with same command
 	commands.Append(command)
-	if len(*commands) != 1 {
-		t.Fatalf("Expected only one command to stay, but was: %d.", len(*commands))
+	if len(commands.collection) != 1 {
+		t.Fatalf("Expected only one command to stay, but was: %d.", len(commands.collection))
 	}
 
 	// Third operation with different command
 	anotherCommand := &DummyCommand{
 		IdentifierValue: "second",
 	}
 	commands.Append(anotherCommand)
-	if len(*commands) != 2 {
-		t.Fatalf("Expected 2 commands to stay, but was: %d.", len(*commands))
+	if len(commands.collection) != 2 {
+		t.Fatalf("Expected 2 commands to stay, but was: %d.", len(commands.collection))
 	}
 }
 
@@ -392,7 +392,7 @@ func TestCommands_Helps(t *testing.T) {
 			return "example"
 		},
 	}
-	commands := &Commands{cmd}
+	commands := &Commands{collection: []Command{cmd}}
 
 	helps := commands.Helps()
 	if len(*helps) != 1 {