Project

General

Profile

Enhancement(機能追加・改善) #2916

op_doctrine.pre_save 等のnotifyイベントで、発生元のモデルオブジェクトを受信側で取得できるようにする

Added by Youichi Kimura over 7 years ago. Updated over 7 years ago.

Status:
Fixed(完了)
Priority:
Normal(通常)
Target version:
Start date:
2012-04-04
Due date:
% Done:

100%


Description

Overview (概要)

lib/util/opDoctrineEventNotifier.class.php で送出される op_doctrine.pre_save 等のイベントで、現行の実装では sfEvent のパラメータにイベント発生元のオブジェクトが渡されていないため受信側で対象となるオブジェクト (pre_save の場合は永続化されようとしているモデルのインスタンス) を操作することができない。
これを改善するために、notifyイベント送出時に発生元のオブジェクトをパラメータとして渡すように修正する。

Spec (仕様)

sfEvent オブジェクト生成時に array('object' => $doctrineEvent->getInvoker()) をパラメータとして渡すように変更する。


Related issues

Related to OpenPNE 3 - Enhancement(機能追加・改善) #933: Notify Doctrine_Event as symfony's one (Doctrineのイベントをsymfonyのイベントとして通知する) Fixed(完了) 2010-04-06

Associated revisions

Revision 5bd27b44 (diff)
Added by Shouta Kashiwagi over 7 years ago

add object parameter on op_doctrine event. (fixes #2916)

Revision 08f8be6c (diff)
Added by Shouta Kashiwagi over 7 years ago

add object parameter on op_doctrine event. (fixes #2916)

Revision bfc258b3 (diff)
Added by Shouta Kashiwagi over 7 years ago

added opDoctrineEvent class (fixes #2916)

Revision 8dfe7feb (diff)
Added by Shouta Kashiwagi over 7 years ago

added opDoctrineEvent class (fixes #2916)

Revision 4ad01d6c (diff)
Added by Shouta Kashiwagi over 7 years ago

(fixes #2916) modified to get array access.

Revision 6807fa54 (diff)
Added by Shouta Kashiwagi over 7 years ago

(fixes #2916) modified to get array access.

History

#1 Updated by Shouta Kashiwagi over 7 years ago

  • Target version set to OpenPNE 3.8beta1

#2 Updated by Shouta Kashiwagi over 7 years ago

  • Status changed from New(新規) to Pending Review(レビュー待ち)
  • % Done changed from 0 to 50

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

#3 Updated by Shouta Kashiwagi over 7 years ago

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

#4 Updated by Yuya Watanabe over 7 years ago

  • Status changed from Pending Review(レビュー待ち) to Rejected(差し戻し)

差し戻し理由

object という名称が適切ではないのではないかと思いました.これが適切なのであればこのままもう一度ステータスをレビュー待ちにしてください.

lib/util/opDoctrineEventNotifier.class.php

 28     $dispatcher->notify(new sfEvent(null, sprintf('op_doctrine.%s_%s_%s', $when, $action, get_class($doctrineEvent->getInvoker())), array('object' => $doctrineEvent->getInvoker())));

メモ

パラメータとして渡された値を取得するには以下のようにする?

  $event->offsetGet('object');

lib/vendor/symfony/lib/event_dispatcher/sfEvent.php

125   /**
126    * Returns a parameter value (implements the ArrayAccess interface).
127    *
128    * @param  string  $name  The parameter name
129    *
130    * @return mixed  The parameter value
131    */
132   public function offsetGet($name)
133   {
134     if (!array_key_exists($name, $this->parameters))
135     {
136       throw new InvalidArgumentException(sprintf('The event "%s" has no "%s" parameter.', $this->name, $name));
137     }
138 
139     return $this->parameters[$name];
140   }

#5 Updated by Shouta Kashiwagi over 7 years ago

  • Status changed from Rejected(差し戻し) to Pending Review(レビュー待ち)

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

#6 Updated by Shouta Kashiwagi over 7 years ago

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

#7 Updated by Yuya Watanabe over 7 years ago

  • Status changed from Pending Review(レビュー待ち) to Pending Testing(テスト待ち)
  • % Done changed from 50 to 70

レビューOKとします.下記に改善内容を示します.

改善内容

sfEvent を継承した opDoctrineEvent というDoctrineのレコードを扱うイベントを新たに作成し,これを投げるように変更.
この opDoctrineEvent は変更を行ったモデルのオブジェクトを Subject として保持し,利用できるような仕様となっている,もとものと Doctrine_Event が必要な場合は getDoctrineEvent() メソッドを呼び出すことで利用可能となっている.

#8 Updated by Kiwa Sakai over 7 years ago

  • Status changed from Pending Testing(テスト待ち) to Fixed(完了)
  • % Done changed from 70 to 100

テスト対象外のためこのチケットは閉じます

#9 Updated by Youichi Kimura over 7 years ago

note-4 の指摘について。

まず僕がチケットに記載した仕様はsymfonyのadminモジュールで使われている doctrine.admin.save_object 等の既存のイベントに似せて書いたものです。
パラメータの object というキーも既存のものに合わせた名前となっています。これについては record なり適切な名称に変更することは問題ないと思います。

しかし、修正コミット (bfc258b3) に含まれている opDoctrineEvent を用いた方法は適切ではないと思います。これは、 opDoctrineEvent というクラスを作成する設計が悪いという意味ではなく、「他のイベントがそうではない」ためです。現状の、パラメータに連想配列を使うスタイルが綺麗でないとして別の方法を使ったとしても、以前のスタイルに慣れ親しんでいる側としては $event->getHogehoge() という書き方はいささか不自然に感じます。それに、イベントによって $event["hogehoge"]$event->getHogehoge() が混在することは決して良いとは思いません。

要約: 下記の修正で十分ではないでしょうか。

@@ -25,7 +25,7 @@ class opDoctrineEventNotifier extends Doctrine_Record_Listener
     }

     $dispatcher = sfContext::getInstance()->getEventDispatcher();
-    $dispatcher->notify(new sfEvent(null, sprintf('op_doctrine.%s_%s_%s', $when, $action, get_class($doctrineEvent->getInvoker())), array('object' => $doctrineEvent->getInvoke
+    $dispatcher->notify(new sfEvent(null, sprintf('op_doctrine.%s_%s_%s', $when, $action, get_class($doctrineEvent->getInvoker())), array('record' => $doctrineEvent->getInvoke
   }

   public function preSave(Doctrine_Event $event)

#10 Updated by Youichi Kimura over 7 years ago

  • Status changed from Fixed(完了) to Rejected(差し戻し)
  • % Done changed from 100 to 50

note-9の理由で、一旦ステータスを戻します。

#11 Updated by Yuya Watanabe over 7 years ago

Youichi Kimura は書きました:

note-4 の指摘について。

まず僕がチケットに記載した仕様はsymfonyのadminモジュールで使われている doctrine.admin.save_object 等の既存のイベントに似せて書いたものです。
パラメータの object というキーも既存のものに合わせた名前となっています。これについては record なり適切な名称に変更することは問題ないと思います。

しかし、修正コミット (bfc258b3) に含まれている opDoctrineEvent を用いた方法は適切ではないと思います。これは、 opDoctrineEvent というクラスを作成する設計が悪いという意味ではなく、「他のイベントがそうではない」ためです。現状の、パラメータに連想配列を使うスタイルが綺麗でないとして別の方法を使ったとしても、以前のスタイルに慣れ親しんでいる側としては $event->getHogehoge() という書き方はいささか不自然に感じます。それに、イベントによって $event["hogehoge"]$event->getHogehoge() が混在することは決して良いとは思いません。

要約: 下記の修正で十分ではないでしょうか。
[...]

以前のスタイルとして連想配列から値を取ってくるという使い方を提供すべきという点については理解しました.これについては note-9 の方法もいいと思います.ただ今回の修正が悪いというわけでもないため,今回の修正に以前のスタイルの値の取得方法を提供するという案でどうでしょうか?

#12 Updated by Youichi Kimura over 7 years ago

note-11は opDoctrineEvent にArrayAccessによるパラメータの取得とgetterメソッドを両方備える折衷案という解釈でよろしいでしょうか? もしそうであれば僕は問題ないと思います。

#13 Updated by Yuya Watanabe over 7 years ago

note-12 の通りです.懸念点としては getSubject() の値と ArrayAccess で取得された内容に差異が出る可能性があるため, getSubject() をオーバーライドして ArrayAccess で得られる値を返すようにすべきという点でしょうか.

#14 Updated by Shouta Kashiwagi over 7 years ago

修正案

diff --git a/lib/event/opDoctrineEvent.class.php b/lib/event/opDoctrineEvent.class.php
index 912c28e..1fc78a9 100644
--- a/lib/event/opDoctrineEvent.class.php
+++ b/lib/event/opDoctrineEvent.class.php
@@ -21,7 +21,10 @@ class opDoctrineEvent extends sfEvent

   public function __construct(Doctrine_Event $event, $when, $action, $parameters = array())
   {
-    parent::__construct($event->getInvoker(), sprintf('op_doctrine.%s_%s_%s', $when, $action, get_class($event->getInvoker())), $parameters);
+    $invoker = $event->getInvoker();
+    $parameters['record'] = $invoker;
+
+    parent::__construct($invoker, sprintf('op_doctrine.%s_%s_%s', $when, $action, get_class($invoker)), $parameters);
     $this->doctrineEvent = $event;
   }

@@ -29,4 +32,9 @@ class opDoctrineEvent extends sfEvent
   {
     return $this->doctrineEvent;
   }
+
+  public function getSubject()
+  {
+    return $this['record'];
+  }
 }

#15 Updated by Shouta Kashiwagi over 7 years ago

  • Status changed from Rejected(差し戻し) to Accepted(着手)
  • % Done changed from 50 to 0

#16 Updated by Yuya Watanabe over 7 years ago

sfEvent では subject を変更する手段がコンストラクタでのみ提供されていますが, opDoctrineRecord の note-14 の実装だと $parameters の値を変更することで subject を変更で来てしまうというハックができてしまいますが,これは想定されない使い方として扱うものとします.

#17 Updated by Shouta Kashiwagi over 7 years ago

  • Status changed from Accepted(着手) to Pending Review(レビュー待ち)
  • % Done changed from 0 to 50

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

#18 Updated by Shouta Kashiwagi over 7 years ago

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

#19 Updated by Shouta Kashiwagi over 7 years ago

  static function tetete(opDoctrineEvent $event)
  {
    var_dump($event->getSubject()->getId());
    var_dump($event['record']->getId());
 }

↑のような使い方をすることによって,期待する値を取得することができます.

#20 Updated by Yuya Watanabe over 7 years ago

  • Status changed from Pending Review(レビュー待ち) to Pending Testing(テスト待ち)
  • % Done changed from 50 to 70

note-9 で指摘された内容も考慮されたものと考えてレビューOKとします.

master

6807fa548f8167ba08ffe9efa2376e3a666e7e97

release-3.8beta1

4ad01d6ca9ed72bc3d4fe304ed77d18a527a710c

#21 Updated by Shouta Kashiwagi over 7 years ago

  • Status changed from Pending Testing(テスト待ち) to Fixed(完了)
  • % Done changed from 70 to 100

テストOKです.
(note-19コードサンプルで検証済み)

Also available in: Atom PDF