issues/1651

Config/Migration/1635299667_add_colum_execute_time.php

@@ -0,0 +1,67 @@
+<?php
+/**
+ * タイミングによっては同じ記事が複数メール送信されてしまうバグ修正のためカラム追加
+ *
+ * @author Shohei Nakajima <nakajimashouhei@gmail.com>
+ * @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;
+	}
+}

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'),

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;
+	}
 }

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__);
+		}
 	}
 }

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);

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);
 
 		//チェック

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,
 		),
 	);