Request::hasSession()の現在の実装の意図
- （意図A）クライアントから送られてきた（本来の意味の）リクエストがセッション有効状態か（＝Cookieを持っているか）を調べる
- （意図B）単にセッションオブジェクトがあるかないかという判定
- （意図C）セッションオブジェクトがあり、開始済みかどうか
現在のhasSession()メソッドは、意図Aの実装であると考えられる。ただし、以下に挙げる呼び出し箇所では、意図Bなどが混在して用いられていると思われる。
- Request::hasSession()の使われ方
- a. RequestListener L70 上記の意図で判定に用いている。
- b. RequestDataCollector L67 メソッドの意図と異なる使われ方と思われる。null !== $request->getSession() が適切ではないか。
- c. SecurityComponent Http/Firewall/ContextListener L65 メソッドの意図と異なる使われ方と思われる。null !== $request->getSession() が適切ではないか。
- d. SecurityComponent Http/Firewall/ExceptionListener L154 用いている意図がやや不明確。
Created
May 8, 2011 00:45
-
-
Save hidenorigoto/960995 to your computer and use it in GitHub Desktop.
Request::hasSession()について
修正してみました。以前のブランチはいったん破棄して、新たに変更を行っています（そのためSession::isStarted()は実装していません）。
https://github.com/fivestar/symfony/commits/session
fivestar/symfony@ae0ef9e
合わせてExceptionListenerで行っていた箇所を、EntryPointへ移動しました。
既に修正パッチがあがっているようでした。EntryPointへ移動した部分は、Johannes曰く、話し合いの結果ExceptionListenerから動かさないことを決めたとのことです。
あがっていたパッチではhasSession()はセッションオブジェクトの有無を確かめるメソッドに変更されており、CookieがあるかどうかをhasPreviousSession()として定義していました。
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
RequestListenerへの修正の適用の件は、私の勘違いでしたね。すみませんでした、理解しました。
現時点では利用されている箇所も限られているので、hasSession()を廃止候補にした上で、書かれているコード例のように判定を各箇所で展開するという提案がよさそうですね。