diff --git a/Config/Migration/1635299667_add_colum_execute_time.php b/Config/Migration/1635299667_add_colum_execute_time.php new file mode 100644 index 0000000..1616e0b --- /dev/null +++ b/Config/Migration/1635299667_add_colum_execute_time.php @@ -0,0 +1,67 @@ + + * @link http://www.netcommons.org NetCommons Project + * @license http://www.netcommons.org/license.txt NetCommons License + * @copyright Copyright 2014, NetCommons Project + */ + +App::uses('NetCommonsMigration', 'NetCommons.Config/Migration'); + +/** + * タイミングによっては同じ記事が複数メール送信されてしまうバグ修正のためカラム追加 + * + * @package NetCommons\Mails\Config\Migration + * @see https://github.com/NetCommons3/NetCommons3/issues/1651 + */ +class AddColumExecuteTime extends NetCommonsMigration { + +/** + * Migration description + * + * @var string + */ + public $description = 'add_colum_execute_time'; + +/** + * Actions to be performed + * + * @var array $migration + */ + public $migration = array( + 'up' => array( + 'create_field' => array( + 'mail_queues' => array( + 'execute_time' => array('type' => 'datetime', 'null' => true, 'default' => null, 'comment' => '実行日時', 'after' => 'send_time'), + ), + ), + ), + 'down' => array( + 'drop_field' => array( + 'mail_queues' => array('execute_time'), + ), + ), + ); + +/** + * Before migration callback + * + * @param string $direction Direction of migration process (up or down) + * @return bool Should process continue + */ + public function before($direction) { + return true; + } + +/** + * After migration callback + * + * @param string $direction Direction of migration process (up or down) + * @return bool Should process continue + */ + public function after($direction) { + return true; + } +} diff --git a/Config/Schema/schema.php b/Config/Schema/schema.php index 1351435..e2ab13d 100644 --- a/Config/Schema/schema.php +++ b/Config/Schema/schema.php @@ -89,6 +89,7 @@ public function after($event = array()) { 'mail_subject' => array('type' => 'string', 'null' => false, 'default' => null, 'collate' => 'utf8_general_ci', 'comment' => 'メール件名', 'charset' => 'utf8'), 'mail_body' => array('type' => 'text', 'null' => false, 'default' => null, 'collate' => 'utf8_general_ci', 'comment' => 'メール本文', 'charset' => 'utf8'), 'send_time' => array('type' => 'datetime', 'null' => false, 'default' => null, 'key' => 'index', 'comment' => '送信日時'), + 'execute_time' => array('type' => 'datetime', 'null' => true, 'default' => null, 'comment' => '実行日時'), 'created_user' => array('type' => 'integer', 'null' => true, 'default' => null, 'unsigned' => false, 'comment' => '作成者'), 'created' => array('type' => 'datetime', 'null' => true, 'default' => null, 'comment' => '作成日時'), 'modified_user' => array('type' => 'integer', 'null' => true, 'default' => null, 'unsigned' => false, 'comment' => '更新者'), @@ -111,6 +112,7 @@ public function after($event = array()) { 'language_id' => array('type' => 'integer', 'null' => false, 'default' => null, 'length' => 6, 'unsigned' => false), 'is_origin' => array('type' => 'boolean', 'null' => false, 'default' => '1', 'comment' => 'オリジナルかどうか'), 'is_translation' => array('type' => 'boolean', 'null' => false, 'default' => '0', 'comment' => '翻訳したかどうか'), + 'is_original_copy' => array('type' => 'boolean', 'null' => false, 'default' => '0', 'comment' => 'オリジナルのコピー。言語を新たに追加したときに使用する'), 'plugin_key' => array('type' => 'string', 'null' => true, 'default' => null, 'collate' => 'utf8_general_ci', 'charset' => 'utf8'), 'block_key' => array('type' => 'string', 'null' => true, 'default' => null, 'key' => 'index', 'collate' => 'utf8_general_ci', 'charset' => 'utf8'), 'type_key' => array('type' => 'string', 'null' => false, 'default' => null, 'collate' => 'utf8_general_ci', 'comment' => '定型文の種類', 'charset' => 'utf8'), diff --git a/Console/Command/MailSendShell.php b/Console/Command/MailSendShell.php index 560f327..492b9f0 100644 --- a/Console/Command/MailSendShell.php +++ b/Console/Command/MailSendShell.php @@ -106,19 +106,21 @@ public function send() { return $this->_stop(); } - $now = NetCommonsTime::getNowDatetime(); + $now = (new DateTime('now', new DateTimeZone('UTC')))->format('Y-m-d H:i:s'); - // キュー取得＆ロック - シェル実行の排他を実現したいため、行ロックしている - // http://k-1blog.com/development/program/post-7407/ - // http://d.hatena.ne.jp/fat47/20140212/1392171784 - // 下記SQL（テーブル結合＆範囲条件）でSELECT FOR UPDATEを実行すると、テーブルロック + //メール送信する対象のメールキューの実行時間を更新する + $result = $this->_updateExecuteTime($now); + if (! $result) { + return $this->_stop(); + } + + //対象の実行時間のメールキューのみ処理する $sql = 'SELECT * FROM ' . $this->MailQueue->tablePrefix . 'mail_queues MailQueue, ' . $this->MailQueueUser->tablePrefix . 'mail_queue_users MailQueueUser ' . 'WHERE ' . 'MailQueue.id = MailQueueUser.mail_queue_id ' . - 'AND MailQueue.send_time <= ? ' . - 'FOR UPDATE '; + 'AND MailQueue.execute_time = ? '; $mailQueues = $this->MailQueue->query($sql, array($now)); if (empty($mailQueues)) { $this->out('MailQueue is empty. [' . __METHOD__ . '] '); @@ -182,4 +184,53 @@ protected function _isSendBlockType($mailQueue) { // ブロック非公開、期間外はメール送らない return $this->MailQueue->isSendBlockType($block); } + +/** + * 実行時間を更新する + * + * @param string $now 現在時刻 + * @return bool + */ + protected function _updateExecuteTime($now) { + try { + //トランザクションBegin + $this->MailQueue->begin(); + + // キュー取得＆ロック - シェル実行の排他を実現したいため、行ロックしている + // http://k-1blog.com/development/program/post-7407/ + // http://d.hatena.ne.jp/fat47/20140212/1392171784 + // 下記SQL（テーブル結合＆範囲条件）でSELECT FOR UPDATEを実行すると、テーブルロック + $sql = 'SELECT COUNT(*) FROM ' . + $this->MailQueue->tablePrefix . 'mail_queues MailQueue ' . + 'WHERE MailQueue.execute_time = ? ' . + 'FOR UPDATE '; + $count = $this->MailQueue->query($sql, array($now)); + + //全くの同時刻に実行されたものは無視する + if (isset($count[0][0]['COUNT(*)']) && + $count[0][0]['COUNT(*)'] > 0) { + $this->MailQueue->rollback(); + $this->out('MailQueue is executing ' . $now . ' [' . __METHOD__ . '] '); + return false; + } + + $update = [ + 'MailQueue.execute_time' => "'" . $now . "'" + ]; + $conditions = [ + 'MailQueue.execute_time' => null, + 'MailQueue.send_time <=' => $now + ]; + $this->MailQueue->updateAll($update, $conditions); + + //トランザクションCommit + $this->MailQueue->commit(); + } catch (Exception $ex) { + //トランザクションRollback + $this->MailQueue->rollback($ex); + return false; + } + + return true; + } } diff --git a/Model/Behavior/MailQueueDeleteBehavior.php b/Model/Behavior/MailQueueDeleteBehavior.php index a7abb9d..31ce3a0 100644 --- a/Model/Behavior/MailQueueDeleteBehavior.php +++ b/Model/Behavior/MailQueueDeleteBehavior.php @@ -85,16 +85,16 @@ public function deleteQueue(Model $model, $value, $deleteColum = 'content_key') 'MailQueueUser' => 'Mails.MailQueueUser', ]); - // キューの配信先 削除 - $conditions = array($model->MailQueueUser->alias . '.' . $deleteColum => $value); - if (! $model->MailQueueUser->deleteAll($conditions, false)) { - throw new InternalErrorException('Failed - MailQueueUser ' . __METHOD__); - } - // キュー 削除 $conditions = array($model->MailQueue->alias . '.' . $deleteColum => $value); if (! $model->MailQueue->deleteAll($conditions, false)) { throw new InternalErrorException('Failed - MailQueue ' . __METHOD__); } + + // キューの配信先 削除 + $conditions = array($model->MailQueueUser->alias . '.' . $deleteColum => $value); + if (! $model->MailQueueUser->deleteAll($conditions, false)) { + throw new InternalErrorException('Failed - MailQueueUser ' . __METHOD__); + } } } diff --git a/Test/Case/Utility/NetCommonsMail/BrReplaceTest.php b/Test/Case/Utility/NetCommonsMail/BrReplaceTest.php index 565b6af..89936a4 100644 --- a/Test/Case/Utility/NetCommonsMail/BrReplaceTest.php +++ b/Test/Case/Utility/NetCommonsMail/BrReplaceTest.php @@ -77,7 +77,9 @@ public function testBrReplaceHtml() { $this->mail->emailFormat('html'); //テスト実施 + error_reporting(0); $this->mail->brReplace(); + error_reporting(-1); //チェック $this->assertTextContains("\n", $this->mail->body); diff --git a/Test/Case/Utility/NetCommonsMail/SendQueueMailTest.php b/Test/Case/Utility/NetCommonsMail/SendQueueMailTest.php index 3c0c292..0d18e98 100644 --- a/Test/Case/Utility/NetCommonsMail/SendQueueMailTest.php +++ b/Test/Case/Utility/NetCommonsMail/SendQueueMailTest.php @@ -84,6 +84,7 @@ public function testSendQueueMail($index, $assert = 'assertNotEmpty') { $this->mail->initShell($mailQueue); //テスト実施 + $this->mail->emailFormat('text'); $result = $this->mail->sendQueueMail($mailQueueUser, $mailQueueLanguageId); //チェック @@ -134,6 +135,7 @@ public function testSendQueueMailBodyEmpty() { $mailQueueLanguageId = 2; //テスト実施 + $this->mail->emailFormat('text'); $result = $this->mail->sendQueueMail($mailQueueUser, $mailQueueLanguageId); //チェック diff --git a/Test/Fixture/MailQueueFixture.php b/Test/Fixture/MailQueueFixture.php index 9c79fc8..d18c11c 100644 --- a/Test/Fixture/MailQueueFixture.php +++ b/Test/Fixture/MailQueueFixture.php @@ -32,6 +32,7 @@ class MailQueueFixture extends CakeTestFixture { 'mail_subject' => array('type' => 'string', 'null' => false, 'default' => null, 'collate' => 'utf8_general_ci', 'comment' => 'メール件名', 'charset' => 'utf8'), 'mail_body' => array('type' => 'text', 'null' => false, 'default' => null, 'collate' => 'utf8_general_ci', 'comment' => 'メール本文', 'charset' => 'utf8'), 'send_time' => array('type' => 'datetime', 'null' => false, 'default' => null, 'comment' => '送信日時'), + 'execute_time' => array('type' => 'datetime', 'null' => true, 'default' => null, 'comment' => '実行日時'), 'created_user' => array('type' => 'integer', 'null' => true, 'default' => null, 'unsigned' => false, 'comment' => '作成者'), 'created' => array('type' => 'datetime', 'null' => true, 'default' => null, 'comment' => '作成日時'), 'modified_user' => array('type' => 'integer', 'null' => true, 'default' => null, 'unsigned' => false, 'comment' => '更新者'), @@ -61,6 +62,7 @@ class MailQueueFixture extends CakeTestFixture { 'mail_subject' => '件名', 'mail_body' => "本文１\r\n本文２\r\n本文３\r\n", 'send_time' => '2016-03-22 12:22:15', + 'execute_time' => null, ), array( 'id' => 2, @@ -72,6 +74,7 @@ class MailQueueFixture extends CakeTestFixture { 'mail_subject' => '件名2', 'mail_body' => "本文１\r\n本文２\r\n本文３\r\n", 'send_time' => '2016-03-22 12:22:15', + 'execute_time' => null, ), );