Refactor code that sents notifications

Make sure no notification is sent twice in the same request.

classes/BlueChip/Security/Modules/Notifications/Mailman.php

@@ -28,28 +28,24 @@ abstract class Mailman
      *
      * @param string|string[] $to Email address(es) of notification recipient(s).
      * @param string $subject Subject of notification.
-     * @param string|string[] $message Body of notification.
+     * @param Message $message Body of notification.
      *
      * @return bool True if notification has been sent successfully, false otherwise.
      */
-    public static function send($to, string $subject, $message): bool
+    public static function send(array|string $to, string $subject, Message $message): bool
     {
-        return \wp_mail(
+        return wp_mail(
             $to,
             self::formatSubject($subject),
-            self::formatMessage(\is_array($message) ? $message : [$message])
+            self::formatMessage($message)
         );
     }
 
 
     /**
      * Strip any HTML tags from $message and add plugin boilerplate to it.
-     *
-     * @param string[] $message Message body as list of lines.
-     *
-     * @return string
      */
-    private static function formatMessage(array $message): string
+    private static function formatMessage(Message $message): string
     {
         $boilerplate_intro = [
             \sprintf(
@@ -71,7 +67,7 @@ private static function formatMessage(array $message): string
             ),
         ];
 
-        return \implode(self::EOL, \array_merge($boilerplate_intro, self::stripTags($message), $boilerplate_outro));
+        return \implode(self::EOL, \array_merge($boilerplate_intro, self::stripTags($message->getRaw()), $boilerplate_outro));
     }
 
 

classes/BlueChip/Security/Modules/Notifications/Message.php

@@ -0,0 +1,65 @@
+<?php
+
+declare(strict_types=1);
+
+namespace BlueChip\Security\Modules\Notifications;
+
+class Message
+{
+    /**
+     * @var string[] Message body as lines of text
+     */
+    private array $body = [];
+
+
+    public function __construct(string $text = '')
+    {
+        $this->body = ($text !== '') ? [$text] : [];
+    }
+
+
+    public function __toString(): string
+    {
+        return \implode(PHP_EOL, $this->body);
+    }
+
+
+    public function addEmptyLine(): self
+    {
+        return $this->addLine('');
+    }
+
+
+    public function addLine(string $text = ''): self
+    {
+        $this->body[] = $text;
+        return $this;
+    }
+
+
+    /**
+     * @param string[] $text
+     *
+     * @return self
+     */
+    public function addLines(array $text): self
+    {
+        $this->body = \array_merge($this->body, \array_is_list($text) ? $text : \array_values($text));
+        return $this;
+    }
+
+
+    public function getFingerprint(): string
+    {
+        return \sha1((string) $this);
+    }
+
+
+    /**
+     * @return string[]
+     */
+    public function getRaw(): array
+    {
+        return $this->body;
+    }
+}

classes/BlueChip/Security/Modules/Notifications/Watchman.php

@@ -23,7 +23,13 @@ class Watchman implements Activable, Initializable
     /**
      * @var string[] List of notifications recipients
      */
-    private $recipients;
+    private array $recipients;
+
+
+    /**
+     * @var string[] List of sent notifications.
+     */
+    private array $sent_notifications = [];
 
 
     /**
@@ -124,14 +130,16 @@ public function deactivate(): void
             $user = wp_get_current_user();
             if ($user->ID) {
                 // Name the bastard that turned us off!
-                $message = \sprintf(
-                    __('User "%s" had just deactivated BC Security plugin on your website!', 'bc-security'),
-                    $user->user_login
+                $message = new Message(
+                    \sprintf(
+                        __('User "%s" had just deactivated BC Security plugin on your website!', 'bc-security'),
+                        $user->user_login
+                    )
                 );
             } else {
                 // No user means plugin has been probably deactivated via WP-CLI.
                 // See: https://github.com/chesio/bc-security/issues/16#issuecomment-321541102
-                $message = __('BC Security plugin on your website has been deactivated!', 'bc-security');
+                $message = new Message(__('BC Security plugin on your website has been deactivated!', 'bc-security'));
             }
 
             $this->notify($subject, $message);
@@ -170,9 +178,11 @@ private function watchCoreUpdateAvailable($update_transient): void
         }
 
         $subject = __('WordPress update available', 'bc-security');
-        $message = \sprintf(
-            __('WordPress has an update to version %s available.', 'bc-security'),
-            $latest_version
+        $message = new Message(
+            \sprintf(
+                __('WordPress has an update to version %s available.', 'bc-security'),
+                $latest_version,
+            )
         );
 
         // Send notification.
@@ -206,7 +216,7 @@ private function watchPluginUpdatesAvailable($update_transient): void
         }
 
         $subject = __('Plugin updates available', 'bc-security');
-        $message = [];
+        $message = new Message();
 
         foreach ($plugin_updates as $plugin_file => $plugin_update_data) {
             $plugin_data = Plugin::getPluginData($plugin_file);
@@ -224,7 +234,7 @@ private function watchPluginUpdatesAvailable($update_transient): void
                 );
             }
 
-            $message[] = $plugin_message;
+            $message->addLine($plugin_message);
         }
 
         // Send notification.
@@ -260,14 +270,16 @@ private function watchThemeUpdatesAvailable($update_transient): void
         }
 
         $subject = __('Theme updates available', 'bc-security');
-        $message = [];
+        $message = new Message();
 
         foreach ($theme_updates as $theme_slug => $theme_update_data) {
             $theme = wp_get_theme($theme_slug);
-            $message[] = \sprintf(
-                __('Theme "%1$s" has an update to version %2$s available.', 'bc-security'),
-                $theme,
-                $theme_update_data['new_version']
+            $message->addLine(
+                \sprintf(
+                    __('Theme "%1$s" has an update to version %2$s available.', 'bc-security'),
+                    $theme,
+                    $theme_update_data['new_version'],
+                )
             );
         }
 
@@ -288,11 +300,13 @@ private function watchBadRequestBanEvents(string $remote_address, string $uri, B
     {
         if (\in_array($remote_address, $this->logger->getKnownIps(), true)) {
             $subject = __('Known IP locked out', 'bc-security');
-            $message = \sprintf(
-                __('A known IP address %1$s has been locked out due to bad request rule "%2$s" after someone tried to access following URL: %3$s', 'bc-security'),
-                self::formatRemoteAddress($remote_address),
-                $ban_rule->getName(),
-                $uri
+            $message = new Message(
+                \sprintf(
+                    __('A known IP address %1$s has been locked out due to bad request rule "%2$s" after someone tried to access following URL: %3$s', 'bc-security'),
+                    self::formatRemoteAddress($remote_address),
+                    $ban_rule->getName(),
+                    $uri,
+                )
             );
 
             $this->notify($subject, $message);
@@ -307,11 +321,13 @@ private function watchLockoutEvents(string $remote_address, string $username, in
     {
         if (\in_array($remote_address, $this->logger->getKnownIps(), true)) {
             $subject = __('Known IP locked out', 'bc-security');
-            $message = \sprintf(
-                __('A known IP address %1$s has been locked out for %2$d seconds after someone tried to log in with username "%3$s".', 'bc-security'),
-                self::formatRemoteAddress($remote_address),
-                $duration,
-                $username
+            $message = new Message(
+                \sprintf(
+                    __('A known IP address %1$s has been locked out for %2$d seconds after someone tried to log in with username "%3$s".', 'bc-security'),
+                    self::formatRemoteAddress($remote_address),
+                    $duration,
+                    $username,
+                )
             );
 
             $this->notify($subject, $message);
@@ -326,10 +342,12 @@ private function watchWpLogin(string $username, WP_User $user): void
     {
         if (Is::admin($user)) {
             $subject = __('Admin user login', 'bc-security');
-            $message = \sprintf(
-                __('User "%1$s" with administrator privileges just logged in to your WordPress site from IP address %2$s.', 'bc-security'),
-                $username,
-                self::formatRemoteAddress($this->remote_address)
+            $message = new Message(
+                \sprintf(
+                    __('User "%1$s" with administrator privileges just logged in to your WordPress site from IP address %2$s.', 'bc-security'),
+                    $username,
+                    self::formatRemoteAddress($this->remote_address),
+                )
             );
 
             $this->notify($subject, $message);
@@ -343,12 +361,17 @@ private function watchWpLogin(string $username, WP_User $user): void
     private function watchChecklistSingleCheckAlert(Check $check, CheckResult $result): void
     {
         $subject = __('Checklist monitoring alert', 'bc-security');
-        $preamble = [
-            \sprintf(__('An issue has been found during checklist monitoring of "%s" check:', 'bc-security'), $check->getName()),
-            '',
-        ];
+        $message = new Message(
+            \sprintf(
+                __('An issue has been found during checklist monitoring of "%s" check:', 'bc-security'),
+                $check->getName(),
+            )
+        );
+
+        $message->addEmptyLine();
+        $message->addLines($result->getMessage());
 
-        $this->notify($subject, \array_merge($preamble, $result->getMessage()));
+        $this->notify($subject, $message);
     }
 
 
@@ -360,13 +383,15 @@ private function watchChecklistSingleCheckAlert(Check $check, CheckResult $resul
     private function watchChecklistMultipleChecksAlert(array $issues): void
     {
         $subject = __('Checklist monitoring alert', 'bc-security');
-        $message = [
+        $message = new Message(
             __('Following checks had failed during checklist monitoring:', 'bc-security'),
-        ];
+        );
 
         foreach ($issues as $issue) {
-            $message[] = '';
-            $message[] = \sprintf("%s: %s", $issue['check']->getName(), $issue['result']->getMessageAsPlainText());
+            $message
+                ->addEmptyLine()
+                ->addLine(\sprintf("%s: %s", $issue['check']->getName(), $issue['result']->getMessageAsPlainText()))
+            ;
         }
 
         $this->notify($subject, $message);
@@ -377,12 +402,35 @@ private function watchChecklistMultipleChecksAlert(array $issues): void
      * Send email notification with given $subject and $message to recipients configured in plugin settings.
      *
      * @param string $subject
-     * @param string|string[] $message
+     * @param Message $message
      *
      * @return bool|null Null if there are no recipients configured. True if email has been sent, false otherwise.
      */
-    private function notify(string $subject, $message): ?bool
+    private function notify(string $subject, Message $message): ?bool
+    {
+        if ($this->hasMessageBeenSent($message)) {
+            // Given message has been sent already.
+            return true;
+        }
+
+        $status = empty($this->recipients) ? null : Mailman::send($this->recipients, $subject, $message);
+
+        if ($status) {
+            $this->markMessageAsSent($message);
+        }
+
+        return $status;
+    }
+
+
+    private function hasMessageBeenSent(Message $message): bool
+    {
+        return \in_array($message->getFingerprint(), $this->sent_notifications, true);
+    }
+
+
+    private function markMessageAsSent(Message $message): void
     {
-        return empty($this->recipients) ? null : Mailman::send($this->recipients, $subject, $message);
+        $this->sent_notifications[] = $message->getFingerprint();
     }
 }