Project

General

Profile

Bug(バグ) #965

プラグインを無効にした状態でmigrateを実行するとフォームに不要な要素が表示されるようになる

Added by Kiwa Sakai over 11 years ago. Updated over 11 years ago.

Status:
Fixed(完了)
Priority:
Immediate(今すぐ)
Target version:
Start date:
2010-04-19
Due date:
% Done:

100%

3.6 で発生するか:
Unknown (未調査)
3.8 で発生するか:
Unknown (未調査)

Description

発生確認バージョン

OpenPNE 3.4.x

手順

1. プラグインAを無効にする
2. migrateを実行する
3. プラグインAを有効にする
4. プラグインAのフォームに不要な要素が表示されるようになる


Related issues

Related to OpenPNE 3 - Backport(バックポート) #1045: プラグインを無効にした状態でmigrateを実行するとフォームに不要な要素が表示されるようになる Fixed(完了) 2010-05-11

Associated revisions

Revision 35aeb0a6 (diff)
Added by Shogo Kawahara over 11 years ago

fixed the process of configuration, bacause doctrine:build task generate some unnecessary file of form class (fixes #965)

Revision 0260664a (diff)
Added by Shogo Kawahara over 11 years ago

fixed the process of configuration to the performance (fixes #965)

History

#1 Updated by Shogo Kawahara over 11 years ago

  • Assignee deleted (Shogo Kawahara)

#2 Updated by Shogo Kawahara over 11 years ago

  • Assignee set to Shogo Kawahara

#3 Updated by Shogo Kawahara over 11 years ago

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

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

#4 Updated by Kousuke Ebihara over 11 years ago

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

問題ないように見えますが、パフォーマンスの観点から以下二点指摘させてください(細かいですが……)

opProjectConfiguration に正規表現でマッチングをおこなっている箇所がありますが、sf_task_name をセットしたい状況では $subject にはタスクのクラスインスタンスが格納されているのですから、 instanceof 演算子で sfTask を継承しているかどうかをチェックすればよいはずです。

$subject = $event->getSubject();
if (preg_match('/Task$/', get_class($subject)))
{
  sfConfig::set('sf_task_name', get_class($subject));
}

また、 opApplicationConfiguration にも正規表現でマッチングをおこなっている箇所がありますが、前述のようなコードでは sf_task_name にタスクのクラス名以外の文字列が渡ることは考えにくいため、 Task で終わっている文字列かどうかをチェックする必要はないと思われます。
Task で終了しているかどうかのチェックが不要であるという前提だと、 sfDoctrineBuild で開始されているかどうかをチェックするだけになるので、正規表現よりも strpos() でのチェックの方がお手軽かつパフォーマンス的に有利ということになると思いますがどうでしょうか。

if (!preg_match('/^sfDoctrineBuild.*Task$/', sfConfig::get('sf_task_name')))

#5 Updated by Shogo Kawahara over 11 years ago

  • Status changed from Rejected(差し戻し) to Accepted(着手)

確かにその通りです。修正します。

Kousuke Ebihara は書きました:

問題ないように見えますが、パフォーマンスの観点から以下二点指摘させてください(細かいですが……)

opProjectConfiguration に正規表現でマッチングをおこなっている箇所がありますが、sf_task_name をセットしたい状況では $subject にはタスクのクラスインスタンスが格納されているのですから、 instanceof 演算子で sfTask を継承しているかどうかをチェックすればよいはずです。
[...]

また、 opApplicationConfiguration にも正規表現でマッチングをおこなっている箇所がありますが、前述のようなコードでは sf_task_name にタスクのクラス名以外の文字列が渡ることは考えにくいため、 Task で終わっている文字列かどうかをチェックする必要はないと思われます。
Task で終了しているかどうかのチェックが不要であるという前提だと、 sfDoctrineBuild で開始されているかどうかをチェックするだけになるので、正規表現よりも strpos() でのチェックの方がお手軽かつパフォーマンス的に有利ということになると思いますがどうでしょうか。
[...]

#6 Updated by Shogo Kawahara over 11 years ago

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

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

#7 Updated by Kousuke Ebihara over 11 years ago

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

#8 Updated by Kiwa Sakai over 11 years ago

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

Also available in: Atom PDF