Merge pull request #179 from mocktools/develop

Golang smtpmock v2.3.0

.circleci/linter_configs/.commitspell.yml

@@ -0,0 +1,44 @@
+---
+
+enableGlobDot: true
+
+patterns:
+  - name: GithubUser
+    pattern: /\[@.+\]/gmx
+
+languageSettings:
+  - languageId: markdown
+    ignoreRegExpList:
+      - Email
+      - GithubUser
+
+words:
+  - autobuilds
+  - bestwebua
+  - codecov
+  - codesmells
+  - commitspell
+  - consts
+  - crtypto
+  - funcs
+  - unexported
+  - golangci
+  - gomod
+  - gotestsum
+  - goreleaser
+  - lefthook
+  - ldflags
+  - markdownlint
+  - mocktools
+  - punycode
+  - rubocop
+  - shellcheck
+  - shortcuting
+  - smtpmock
+  - sigquit
+  - struct
+  - structs
+  - yamlint
+  - rset
+  - rcptto
+  - helo

.circleci/linter_configs/.lefthook.yml

@@ -6,6 +6,8 @@ skip_output:
 
 linters:
   commands:
+    commitspell:
+      run: .circleci/scripts/commitspell.sh -c '.circleci/linter_configs/.commitspell.yml'
     cspell:
       run: cspell-cli lint -c '.circleci/linter_configs/.cspell.yml' '**/*.{txt,md}'
     golangci:

.circleci/scripts/commitspell.sh

@@ -0,0 +1,22 @@
+#!/bin/sh
+set -e
+
+configuration=$(if [ "$2" = "" ]; then echo "$2"; else echo " $1 $2"; fi)
+latest_commit=$(git rev-parse HEAD)
+
+spellcheck_info() {
+  echo "Checking the spelling of the latest commit ($latest_commit) message..."
+}
+
+compose_cspell_command() {
+  echo "cspell-cli lint stdin$configuration"
+}
+
+cspell="$(compose_cspell_command)"
+
+spellcheck_latest_commit() {
+  git log -1 --pretty=%B | $cspell
+}
+
+spellcheck_info
+spellcheck_latest_commit

.tool-versions

@@ -1 +1 @@
-golang 1.21.5
+golang 1.22.0

CHANGELOG.md

@@ -2,6 +2,22 @@
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [2.3.0] - 2024-03-03
+
+### Added
+
+- Added ability to message purge when retrieving messages, `server.MessagesAndPurge()`. Thanks [@mitar](https://github.com/mitar) for PR
+- Added `commitspell` linter
+
+### Fixed
+
+- Fixed issue with data race condition between newMessage() and Messages(). Thanks [@mitar](https://github.com/mitar) for PR
+
+### Updated
+
+- Updated `lefthook` config
+- Updated project documentation
+
 ## [2.2.1] - 2024-01-25
 
 ### Added

README.md

@@ -314,9 +314,13 @@ func main() {
   client.Close()
 
   // Each result of SMTP session will be saved as message.
-  // To get access to server messages use Messages() method
+  // To get access for server messages copies use Messages() method
   server.Messages()
 
+  // To get access for server messages copies and purge it on server after
+  // use MessagesAndPurge() method
+  server.MessagesAndPurge()
+
   // To stop the server use Stop() method. Please note, smtpmock uses graceful shutdown.
   // It means that smtpmock will end all sessions after client responses or by session
   // timeouts immediately.

handler_rset.go

@@ -31,7 +31,7 @@ func (handler *handlerRset) run(request string) {
 func (handler *handlerRset) clearMessage() {
 	messageWithData, configuration := handler.message, handler.configuration
 
-	if !(configuration.multipleMessageReceiving && messageWithData.isConsistent()) {
+	if !(configuration.multipleMessageReceiving && messageWithData.IsConsistent()) {
 		clearedMessage := &Message{
 			heloRequest:  messageWithData.heloRequest,
 			heloResponse: messageWithData.heloResponse,

message.go

@@ -121,13 +121,6 @@ func (message Message) IsConsistent() bool {
 	return message.mailfrom && message.rcptto && message.data && message.msg
 }
 
-// Message pointer consistency status predicate. Returns true for case
-// when message struct is consistent. It means that MAILFROM, RCPTTO, DATA
-// commands and message context were successful. Otherwise returns false
-func (message *Message) isConsistent() bool {
-	return message.mailfrom && message.rcptto && message.data && message.msg
-}
-
 // Message RCPTTO successful response predicate. Returns true when at least one
 // successful RCPTTO response exists. Otherwise returns false
 func (message *Message) isIncludesSuccessfulRcpttoResponse(targetSuccessfulResponse string) bool {
@@ -145,16 +138,43 @@ var zeroMessage = &Message{}
 
 // Concurrent type that can be safely shared between goroutines
 type messages struct {
-	sync.Mutex
+	sync.RWMutex
 	items []*Message
 }
 
 // messages methods
 
-// Addes new message pointer into concurrent messages slice
+// Adds new message pointer into concurrent messages slice
 func (messages *messages) append(item *Message) {
 	messages.Lock()
 	defer messages.Unlock()
-
 	messages.items = append(messages.items, item)
 }
+
+// Returns a copy of all messages
+func (messages *messages) copy() []Message {
+	messages.RLock()
+	defer messages.RUnlock()
+	return messages.copyInternal()
+}
+
+// Copy messages without a lock
+func (messages *messages) copyInternal() []Message {
+	copiedMessages := []Message{}
+	for index := range messages.items {
+		copiedMessages = append(copiedMessages, *messages.items[index])
+	}
+
+	return copiedMessages
+}
+
+// Returns all messages and removes them at the same time
+func (messages *messages) purge() []Message {
+	messages.Lock()
+	defer messages.Unlock()
+
+	copiedMessages := messages.copyInternal()
+	messages.items = nil
+
+	return copiedMessages
+}

message_test.go

@@ -193,30 +193,30 @@ func TestMessagePointerIsConsistent(t *testing.T) {
 	t.Run("when consistent", func(t *testing.T) {
 		message := &Message{mailfrom: true, rcptto: true, data: true, msg: true}
 
-		assert.True(t, message.isConsistent())
+		assert.True(t, message.IsConsistent())
 	})
 
 	t.Run("when not consistent MAILFROM", func(t *testing.T) {
 
-		assert.False(t, new(Message).isConsistent())
+		assert.False(t, new(Message).IsConsistent())
 	})
 
 	t.Run("when not consistent RCPTTO", func(t *testing.T) {
 		message := &Message{mailfrom: true}
 
-		assert.False(t, message.isConsistent())
+		assert.False(t, message.IsConsistent())
 	})
 
 	t.Run("when not consistent DATA", func(t *testing.T) {
 		message := &Message{mailfrom: true, rcptto: true}
 
-		assert.False(t, message.isConsistent())
+		assert.False(t, message.IsConsistent())
 	})
 
 	t.Run("when not consistent MSG", func(t *testing.T) {
 		message := &Message{mailfrom: true, rcptto: true, data: true}
 
-		assert.False(t, message.isConsistent())
+		assert.False(t, message.IsConsistent())
 	})
 }
 
@@ -239,6 +239,31 @@ func TestMessagesAppend(t *testing.T) {
 		message, messages := new(Message), new(messages)
 		messages.append(message)
 
+		messages.RLock()
 		assert.Same(t, message, messages.items[0])
+		messages.RUnlock()
+	})
+}
+
+func TestMessagesCopy(t *testing.T) {
+	t.Run("copies messages", func(t *testing.T) {
+		message, messages := new(Message), new(messages)
+		message.heloRequest = "foobar"
+		messages.append(message)
+		copyMessages := messages.copy()
+
+		assert.Len(t, copyMessages, 1)
+		assert.Equal(t, *message, copyMessages[0])
+	})
+}
+
+func TestMessagesPurge(t *testing.T) {
+	t.Run("purges messages from items slice", func(t *testing.T) {
+		message, messages := new(Message), new(messages)
+		messages.append(message)
+
+		assert.Len(t, messages.copy(), 1)
+		assert.Len(t, messages.purge(), 1)
+		assert.Len(t, messages.copy(), 0)
 	})
 }

server.go

@@ -121,14 +121,14 @@ func (server *Server) Stop() (err error) {
 // Public interface to get access to server messages.
 // Returns slice with copy of messages
 func (server *Server) Messages() []Message {
-	server.Lock()
-	defer server.Unlock()
-	copiedMessages, messages := []Message{}, server.messages.items
-	for index := range messages {
-		copiedMessages = append(copiedMessages, *messages[index])
-	}
+	return server.messages.copy()
+}
 
-	return copiedMessages
+// Public interface to get access to server messages
+// and at the same time removes them.
+// Returns slice with copy of messages
+func (server *Server) MessagesAndPurge() []Message {
+	return server.messages.purge()
 }
 
 // Thread-safe getter of server port.
@@ -175,19 +175,13 @@ func (server *Server) stop() {
 	server.started = false
 }
 
-// Creates and assigns new message to server.messages
-func (server *Server) newMessage() *Message {
-	newMessage := new(Message)
-	server.messages.append(newMessage)
-	return newMessage
-}
-
 // Creates and assigns new message with helo context from other message to server.messages
 func (server *Server) newMessageWithHeloContext(otherMessage *Message) *Message {
-	newMessage := server.newMessage()
+	newMessage := new(Message)
 	newMessage.heloRequest = otherMessage.heloRequest
 	newMessage.heloResponse = otherMessage.heloResponse
 	newMessage.helo = otherMessage.helo
+	server.messages.append(otherMessage)
 	return newMessage
 }
 
@@ -221,7 +215,10 @@ func (server *Server) isAbleToEndSession(message *Message, session sessionInterf
 //nolint:gocyclo // SMTP client-server session handler
 func (server *Server) handleSession(session sessionInterface) {
 	defer session.finish()
-	message, configuration := server.newMessage(), server.configuration
+	message, configuration := new(Message), server.configuration
+	defer func() {
+		server.messages.append(message)
+	}()
 	session.writeResponse(configuration.msgGreeting, defaultSessionResponseDelay)
 
 	for {
@@ -244,7 +241,7 @@ func (server *Server) handleSession(session sessionInterface) {
 			case "HELO", "EHLO":
 				newHandlerHelo(session, message, configuration).run(request)
 			case "MAIL":
-				if configuration.multipleMessageReceiving && message.rset && message.isConsistent() {
+				if configuration.multipleMessageReceiving && message.rset && message.IsConsistent() {
 					message = server.newMessageWithHeloContext(message)
 				}