プロジェクト

全般

プロフィール

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

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

Youichi Kimuraほぼ12年前に追加. ほぼ12年前に更新.

ステータス:
Fixed(完了)
優先度:
Normal(通常)
担当者:
対象バージョン:
開始日:
2012-04-04
期日:
進捗率:

100%


説明

Overview (概要)

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

Spec (仕様)

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


関連するチケット

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

関係しているリビジョン

リビジョン 5bd27b44 (差分)
Shouta Kashiwagiほぼ12年前に追加

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

リビジョン 08f8be6c (差分)
Shouta Kashiwagiほぼ12年前に追加

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

リビジョン bfc258b3 (差分)
Shouta Kashiwagiほぼ12年前に追加

added opDoctrineEvent class (fixes #2916)

リビジョン 8dfe7feb (差分)
Shouta Kashiwagiほぼ12年前に追加

added opDoctrineEvent class (fixes #2916)

リビジョン 4ad01d6c (差分)
Shouta Kashiwagiほぼ12年前に追加

(fixes #2916) modified to get array access.

リビジョン 6807fa54 (差分)
Shouta Kashiwagiほぼ12年前に追加

(fixes #2916) modified to get array access.

履歴

#1 Shouta Kashiwagiほぼ12年前に更新

  • 対象バージョンOpenPNE 3.8beta1 にセット

#2 Shouta Kashiwagiほぼ12年前に更新

  • ステータスNew(新規) から Pending Review(レビュー待ち) に変更
  • 進捗率0 から 50 に変更

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

#3 Shouta Kashiwagiほぼ12年前に更新

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

#4 Yuya Watanabeほぼ12年前に更新

  • ステータスPending Review(レビュー待ち) から 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 Shouta Kashiwagiほぼ12年前に更新

  • ステータスRejected(差し戻し) から Pending Review(レビュー待ち) に変更

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

#6 Shouta Kashiwagiほぼ12年前に更新

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

#7 Yuya Watanabeほぼ12年前に更新

  • ステータスPending Review(レビュー待ち) から Pending Testing(テスト待ち) に変更
  • 進捗率50 から 70 に変更

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

改善内容

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

#8 Kiwa Sakaiほぼ12年前に更新

  • ステータスPending Testing(テスト待ち) から Fixed(完了) に変更
  • 進捗率70 から 100 に変更

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

#9 Youichi Kimuraほぼ12年前に更新

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 Youichi Kimuraほぼ12年前に更新

  • ステータスFixed(完了) から Rejected(差し戻し) に変更
  • 進捗率100 から 50 に変更

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

#11 Yuya Watanabeほぼ12年前に更新

Youichi Kimura は書きました:

note-4 の指摘について。

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

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

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

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

#12 Youichi Kimuraほぼ12年前に更新

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

#13 Yuya Watanabeほぼ12年前に更新

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

#14 Shouta Kashiwagiほぼ12年前に更新

修正案

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 Shouta Kashiwagiほぼ12年前に更新

  • ステータスRejected(差し戻し) から Accepted(着手) に変更
  • 進捗率50 から 0 に変更

#16 Yuya Watanabeほぼ12年前に更新

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

#17 Shouta Kashiwagiほぼ12年前に更新

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

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

#18 Shouta Kashiwagiほぼ12年前に更新

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

#19 Shouta Kashiwagiほぼ12年前に更新

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

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

#20 Yuya Watanabeほぼ12年前に更新

  • ステータスPending Review(レビュー待ち) から Pending Testing(テスト待ち) に変更
  • 進捗率50 から 70 に変更

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

master

6807fa548f8167ba08ffe9efa2376e3a666e7e97

release-3.8beta1

4ad01d6ca9ed72bc3d4fe304ed77d18a527a710c

#21 Shouta Kashiwagiほぼ12年前に更新

  • ステータスPending Testing(テスト待ち) から Fixed(完了) に変更
  • 進捗率70 から 100 に変更

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

他の形式にエクスポート: Atom PDF