Backport（バックポート） #2718: デイリーニュースにガジェットが追加されている場合、デイリーニュース配信タスク実行時にエラーになる - OpenPNE 3 - OpenPNE Issue Tracking System

操作

リンクをコピー

#1

Yuya Watanabe さんがほぼ13年前に更新

ステータス を New（新規） から Accepted（着手） に変更
担当者 を Yuya Watanabe にセット

操作

リンクをコピー

#2

Yuya Watanabe さんがほぼ13年前に更新

調査途中経過¶

3.6.1 での調査内容． 3.7.0-dev でも調査して同様ならばバックポート元にも記載します．

下記コマンドを実行して報告内容のエラーが発生するまでの流れ．

  $ ./symfony openpne:send-daily-news

lib/task/openpneSendDailyNewsTask.class.php の下記部分が実行され，61 行目が実行される．

 27   protected function execute($arguments = array(), $options = array())
 28   {
...
 56       $filteredGadgets = array();
 57       if ($gadgets)
 58       {
 59         foreach ($gadgets as $gadget)
 60         {
 61           if ($gadget->isEnabled())
 62           {
 63             $filteredGadgets[] = array(
 64               'component' => array('module' => $gadget->getComponentModule(), 'action' => $gadget->getComponentAction()),
 65               'gadget' => $gadget,
 66               'member' => $member,
 67             );
 68           } 
 69         } 
 70       }

lib/model/doctrine/Gadget.class.php の 80 行目でエラーが発生している．

 66   public function isEnabled()
 67   {
 68     $list = $this->getGadgetConfigList();
 69     if (empty($list[$this->name]))
 70     {
 71       return false;
 72     }
 73 
 74     $controller = sfContext::getInstance()->getController();
 75     if (!$controller->componentExists($this->getComponentModule(), $this->getComponentAction()))
 76     {
 77       return false;
 78     }
 79 
 80     $member = sfContext::getInstance()->getUser()->getMember();
 81     $isEnabled = $this->isAllowed($member, 'view');
 82 
 83     return $isEnabled;
 84   }

上記 80 行目で得られるクラスは以下のものである．

apps/api/lib/myUser.class.php

<?php

class myUser extends sfBasicSecurityUser
{
}

操作

リンクをコピー

#3

Yuya Watanabe さんがほぼ13年前に更新

説明を更新 (差分)

原因¶

下記コマンドを実行した時にデイリーニュース用のガジェットが表示可能かのロジックが正しくない．

  $ ./symfony openpne:send-daily-news

lib/task/openpneSendDailyNewsTask.class.php の下記部分が実行され，61 行目が実行される．

 27   protected function execute($arguments = array(), $options = array())
 28   {
...
 56       $filteredGadgets = array();
 57       if ($gadgets)
 58       {
 59         foreach ($gadgets as $gadget)
 60         {
 61           if ($gadget->isEnabled())
 62           {
 63             $filteredGadgets[] = array(
 64               'component' => array('module' => $gadget->getComponentModule(), 'action' => $gadget->getComponentAction()),
 65               'gadget' => $gadget,
 66               'member' => $member,
 67             );
 68           } 
 69         } 
 70       }

ここで isEnabled() を見てみると，sfContext で得られる getUser() で使えるかどうかを決定している．しかし，タスクで実行しているためここで得られる User は実際にメールを送信したい Member を含む User ではなくタスクを実行した時の User （ここでは apps/api/lib/myUser.class.php ）である．エラー自体はここで User から getMember() を呼び出すことができないという問題であるが，エラーが発生していなくても正しく動作しないものと思われる．

lib/model/doctrine/Gadget.class.php

 66   public function isEnabled()
 67   {
 68     $list = $this->getGadgetConfigList();
 69     if (empty($list[$this->name]))
 70     {
 71       return false;
 72     }
 73 
 74     $controller = sfContext::getInstance()->getController();
 75     if (!$controller->componentExists($this->getComponentModule(), $this->getComponentAction()))
 76     {
 77       return false;
 78     }
 79 
 80     $member = sfContext::getInstance()->getUser()->getMember();
 81     $isEnabled = $this->isAllowed($member, 'view');
 82 
 83     return $isEnabled;
 84   }

修正案¶

修正案1: Gadget の isEnabled() メソッドを用いるのではなく直接 isAllowed() で表示するかどうかを決定する．

diff --git a/lib/task/openpneSendDailyNewsTask.class.php b/lib/task/openpneSendDailyNewsTask.class.php
index f70f082..890ec4a 100644
--- a/lib/task/openpneSendDailyNewsTask.class.php
+++ b/lib/task/openpneSendDailyNewsTask.class.php
@@ -58,7 +58,7 @@ EOF;
       {
         foreach ($gadgets as $gadget)
         {
-          if ($gadget->isEnabled())
+          if ($gadget->isAllowed($member, 'view'))
           {
             $filteredGadgets[] = array(
               'component' => array('module' => $gadget->getComponentModule(), 'action' => $gadget->getComponentAction()),

修正案2: 表示可能かどうかを見たいメンバを Gadgetの isEnable() メソッドの引数に与えるようにする．

diff --git a/lib/model/doctrine/Gadget.class.php b/lib/model/doctrine/Gadget.class.php
index 4cb5053..80ab1cd 100644
--- a/lib/model/doctrine/Gadget.class.php
+++ b/lib/model/doctrine/Gadget.class.php
@@ -63,7 +63,7 @@ class Gadget extends BaseGadget implements opAccessControlRecordInterface
     return $list[$this->name]['component'][1];
   }

-  public function isEnabled()
+  public function isEnabled($member = null)
   {
     $list = $this->getGadgetConfigList();
     if (empty($list[$this->name]))
@@ -77,7 +77,10 @@ class Gadget extends BaseGadget implements opAccessControlRecordInterface
       return false;
     }

-    $member = sfContext::getInstance()->getUser()->getMember();
+    if (is_null($member))
+    {
+      $member = sfContext::getInstance()->getUser()->getMember();
+    }
     $isEnabled = $this->isAllowed($member, 'view');

     return $isEnabled;

備考¶

下記 myUser クラスは sfBasicSecurityUser を継承しているが，他のアプリケーションの myUser と同様に OpenPNE 内の User クラスを継承したほうがよいか？

apps/api/lib/myUser.class.php

  1 <?php
  2 
  3 class myUser extends sfBasicSecurityUser
  4 {
  5 }

参考

apps/pc_frontend/lib/myUser.class.php

 10 
 11 class myUser extends opSecurityUser
 12 {

apps/pc_backend/lib/myUser.class.php

 10 
 11 class myUser extends opBaseSecurityUser
 12 {

apps/mobile_frontend/lib/myUser.class.php

 10 
 11 class myUser extends opSecurityUser
 12 {

apps/mobile_mail_frontend/lib/myUser.class.php

 10 
 11 class myUser extends sfBasicSecurityUser
 12 {

操作

リンクをコピー

#4

Yuya Watanabe さんがほぼ13年前に更新

今回は note-3 に記述された２つの修正案のうち２番目を採用する．

操作

リンクをコピー

#5

Yuya Watanabe さんがほぼ13年前に更新

ステータス を Accepted（着手） から Pending Review（レビュー待ち） に変更
進捗率 を 0 から 50 に変更

更新履歴 ec98cc465d39d414bd65435e95f459905936dafa で適用されました。

操作

リンクをコピー

#6

Kousuke Ebihara さんがほぼ13年前に更新

ステータス を Pending Review（レビュー待ち） から Pending Testing（テスト待ち） に変更
進捗率 を 50 から 70 に変更

操作

リンクをコピー

#7

Yuma Sakata さんがほぼ13年前に更新

ステータス を Pending Testing（テスト待ち） から Rejected（差し戻し） に変更
進捗率 を 70 から 50 に変更

テスト実施しましたが、気になる点がありましたので確認お願いします。

フリーエリアガジェットを追加した場合、携帯メールアドレス宛てにデイリーニュース送信できるか確認¶

試験手順
1. 管理画面のデイリーニュース（携帯メールアドレス向け）ガジェット設定ページ(/pc_backend.php/design/gadget)で、フリーエリアガジェットを追加する
2. サーバーの 3.6 ディレクトリにアクセスする
3. $./symfony openpne:send-daily-news 実行する

試験結果
以下エラーが出て携帯メールアドレス宛てにデイリーニュース送信できない。

PHP Fatal error:  Cannot redeclare class defaultComponents in /home/hoge/sns/36.example.com/apps/mobile_frontend/modules/default/actions/components.class.php on line 50

修正方針
フリーエリアガジェットを追加した場合、携帯メールアドレス宛てにデイリーニュース送信できるように修正お願いします。

操作

リンクをコピー

#8

Yuya Watanabe さんがほぼ13年前に更新

ステータス を Rejected（差し戻し） から Accepted（着手） に変更
進捗率 を 50 から 0 に変更

問題2¶

携帯メールアドレスのみを持つメンバを追加する
携帯デイリーニュースにガジェットを追加する

「symfony openpne:send-daily-news」を実行する

下記エラーが発生する

PHP Fatal error:  Cannot redeclare class defaultComponents in /home/hoge/sns/36.example.com/apps/mobile_frontend/modules/default/actions/components.class.php on line 50

原因¶

$gadget->isEnabled() の中の sfContext::getInstance()->getController()->componentExists() でコンポーネントの存在確認される際にロードされるコンポーネントのアプリケーションが実際に送信する際のコンテキストとは別の場合があることが原因であると思われる．そのため，最初に pc_frontend で sfContext::createInstance() が呼び出されているので携帯メールアドレス向けにデイリー・ニュースを送信しようとするとエラーが発生する．直前の sfContext::createInstance() と componentExists() のインスタンスが一致する場合はエラーが発生しないため，PCメールアドレスに送信するときにエラーが発生しない．

具体的には，member_id=1 のメンバが $member->getEmailAddress() でPCメールアドレスを取得でき，member_id=2 のメンバが携帯メールアドレスを取得できるとするとき，下記のような感じで直前に生成されたコンテキストと送信時のコンテキストが一段階ずつずれている．

最初にpc_frontendがコンテキストで設定される（openpneSendDailyNewsTask.class.php 31行目）
- componentExists時: 未送信時: 未
componentExists() が呼び出される（openpneSendDailyNewsTask.class.php 61行目）
- componentExists時: pc_frontend 送信時: 未
opBaseMailTask::getContextByEmailAddress() 内で sfContext::createInstance() が呼び出される（openpneSendDailyNewsTask.class.php 72行目）
- componentExists時: pc_frontend 送信時: pc_frontend
opMailSend::sendTemplateMail() が呼び出される（openpneSendDailyNewsTask.class.php 80行目）
- componentExists時: pc_frontend 送信時: pc_frontend -> メールが送信される
opBaseMailTask::getContextByEmailAddress() 内で sfContext::createInstance() が呼び出される（openpneSendDailyNewsTask.class.php 72行目）
- componentExists時: pc_frontend 送信時: mobile_frontend
opMailSend::sendTemplateMail() が呼び出される（openpneSendDailyNewsTask.class.php 80行目）
- componentExists時: pc_frontend 送信時: mobile_frontend -> エラーが発生する

lib/task/openpneSendDailyNewsTask.class.php

 30 
 31     sfContext::createInstance($this->createConfiguration('pc_frontend', 'prod'), 'pc_frontend');
 32
...
 60         {
 61           if ($gadget->isEnabled($member))
 62           {
...
 71       
 72       $context = $this->getContextByEmailAddress($address);
 73       $params = array(
 74         'member'  => $member,
 75         'gadgets' => $filteredGadgets,
 76         'subject' => $context->getI18N()->__('デイリーニュース'),
 77         'today'   => time(),
 78       );
 79
 80       opMailSend::sendTemplateMail('dailyNews', $address, opConfig::get('admin_mail_address'), $params, $context);

lib/task/opBaseSendMailTask.class.php

 44   protected function getContextByEmailAddress($address)
 45   {
 46     $application = 'pc_frontend';
 47     if (opToolkit::isMobileEmailAddress($address))
 48     {
 49       $application = 'mobile_frontend';
 50     }
 51 
 52     if (!sfContext::hasInstance($application))
 53     {
 54       $context = sfContext::createInstance($this->createConfiguration($application, 'prod'), $application);
 55     }
 56     else
 57     {
 58       $context = sfContext::getInstance($application);
 59     }
 60 
 61     return $context;
 62   }

修正案２¶

コンテキストの変更を送信時ではなく $gadget->isEnabled() よりも前に行うことで原因で発生するような齟齬が発生しなくなる．

diff --git a/lib/task/openpneSendDailyNewsTask.class.php b/lib/task/openpneSendDailyNewsTask.class.php
index 3a7d9a9..e1c511f 100644
--- a/lib/task/openpneSendDailyNewsTask.class.php
+++ b/lib/task/openpneSendDailyNewsTask.class.php
@@ -48,6 +48,7 @@ EOF;
       }
       $address = $member->getEmailAddress();
       $gadgets = $pcGadgets['dailyNewsContents'];
+      $context = $this->getContextByEmailAddress($address);
       if (opToolkit::isMobileEmailAddress($address))
       {
         $gadgets = $mobileGadgets['mobileDailyNewsContents'];
@@ -69,7 +70,6 @@ EOF;
         }
       }

-      $context = $this->getContextByEmailAddress($address);
       $params = array(
         'member'  => $member,
         'gadgets' => $filteredGadgets,

操作

リンクをコピー

#9