Skip to content

Fix failed unserialize#2865

Merged
kiy0taka merged 4 commits intoEC-CUBE:masterfrom
nanasess:fix-failed-unserialize
Feb 26, 2018
Merged

Fix failed unserialize#2865
kiy0taka merged 4 commits intoEC-CUBE:masterfrom
nanasess:fix-failed-unserialize

Conversation

@nanasess
Copy link
Copy Markdown
Contributor

@nanasess nanasess commented Feb 23, 2018

概要(Overview・Refs Issue)

方針(Policy)

  • Customer 及び Member に Serializable インターフェイスを実装
  • ShoppingService::setNonMember() を追加

実装に関する補足(Appendix)

  • Symfony のドキュメントでは、ユーザーの有効無効判定をするには AdvancedUserInterface を実装し、active プロパティも serialize/unserialize しろとあるが、 EC-CUBE の場合はUserProviderInterface::loadUserByUsername() で有効無効判定をしているため不要
  • 非会員の場合、 Customer オブジェクトをセッションに保存するが、 serialize 対象のプロパティしか保存されないため、 ShoppingService::setNonMember(), ShoppingService::getNonMember() を使用する必要がある

テスト(Test)

  • テストを行っている範囲など、レビューアが安心できるような情報

相談(Discussion)

  • 特に無し

マイナーバージョン互換性保持のための制限事項チェックリスト

  • マイナーバージョンでは、機能・プラグイン・デザインテンプレート互換性を損なう変更は原則取り込みません。
  • 既存機能の仕様変更
  • フックポイントの呼び出しタイミングの変更
  • フックポイントのパラメータの削除・データ型の変更
  • twigファイルに渡しているパラメータの削除・データ型の変更
  • Serviceクラスの公開関数の、引数の削除・データ型の変更
  • 入出力ファイル(CSVなど)のフォーマット変更

@chihiro-adachi chihiro-adachi added this to the 3.n.0 milestone Feb 23, 2018
@nanasess
Copy link
Copy Markdown
Contributor Author

nanasess commented Feb 23, 2018

Serializable を実装した関係で、Customer または Member 同士の比較が serialize 対象のキーのみとなってしまったので EquatableInterface の実装が必要そうです テストが通らないのはテストケースのバグだったので、 不要でした

https://symfony.com/doc/2.7/security/entity_provider.html#understanding-serialize-and-how-a-user-is-saved-in-the-session

@nanasess nanasess force-pushed the fix-failed-unserialize branch from 28fa832 to 3a2d0d4 Compare February 26, 2018 02:15
@kiy0taka kiy0taka merged commit 8d1ade7 into EC-CUBE:master Feb 26, 2018
@nanasess
Copy link
Copy Markdown
Contributor Author

experimental/sf ブランチでは、 Doctrine Proxy を読み込むタイミングの関係で、この修正をしなくても動作する模様。
Serializable は Symfony のドキュメント通り実装しておいた方が良いと思われる

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants