[HttpKernel] [HttpCache] Fix deprecated error in HttpCache#getSurrogate #15306
Conversation
|
Note the fabbot.io check that fails here is not introduced by this pull request, but I would be happy to add a separate commit for this (one line white space change) if desired. |
|
👍 looks reasonable to me, it just should be merged into the Status: Reviewed |
|
Thanks for the thumbs up @xabbuh. Sorry about the wrong branch, I wasn't exactly sure how to interpret the Choose the right Branch section of the Contributing guide for something like this. Should I close and re-open a PR against 2.7, or was that note for the person that would be merging this? |
|
@m14t I guess we can safely apply the changes on the |
|
👍 |
|
Thank you @m14t. |
…#getSurrogate (m14t) This PR was submitted for the 2.8 branch but it was merged into the 2.7 branch instead (closes #15306). Discussion ---------- [HttpKernel] [HttpCache] Fix deprecated error in HttpCache#getSurrogate | Q | A | ------------- | --- | Bug fix? | yes? - I could not find an open issue, but it does appear to be a but to throw a `E_USER_DEPRECATED` when calling a non-depreciated method. | New feature? | no | BC breaks? | no | Deprecations? | no - but related to | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | Currently calls to `HttpCache#getEsi` correctly trigger a `E_USER_DEPRECATED` error and inform the user that they should instead use `HttpCache#getSurrogate`. Unfortunately `HttpCache#getSurrogate` currently calls `$this->getEsi();` which will result in the `E_USER_DEPRECATED` still being triggered. This pull request simply moves the logic that was previously in `getEsi` to `getSurrogate`, and leaves `getEsi` as a wrapper around `getSurrogate` with the addition of also triggering this warning. This pull request also effects the 2.7 branch. Commits ------- 32d964b Fix calls to HttpCache#getSurrogate triggering E_USER_DEPRECATED errors.
| return $this->getEsi(); | ||
| if (!$this->surrogate instanceof Esi) { | ||
| throw new \LogicException('This instance of HttpCache was not set up to use ESI as surrogate handler. You must overwrite and use createSurrogate'); | ||
| } |
There was a problem hiding this comment.
I might be missing something, but does it really make sense to have this condition in getSurrogate() which is suppose to work with any surrogate?
There was a problem hiding this comment.
you're right! would you open a PR?
There was a problem hiding this comment.
hum, maybe not: previously, only ESI was allowed (by calling getEsi())
So allowing more surrogates would be a new feature.
E_USER_DEPRECATEDwhen calling a non-depreciated method.Currently calls to
HttpCache#getEsicorrectly trigger aE_USER_DEPRECATEDerror and inform the user that they should instead useHttpCache#getSurrogate.Unfortunately
HttpCache#getSurrogatecurrently calls$this->getEsi();which will result in theE_USER_DEPRECATEDstill being triggered.This pull request simply moves the logic that was previously in
getEsitogetSurrogate, and leavesgetEsias a wrapper aroundgetSurrogatewith the addition of also triggering this warning.This pull request also effects the 2.7 branch.