8000 feature #19734 [HttpFoundation] Deprecate extending some methods (Ene… · symfony/symfony@65e254c · GitHub
[go: up one dir, main page]

Skip to content

Commit 65e254c

Browse files
committed
feature #19734 [HttpFoundation] Deprecate extending some methods (Ener-Getick)
This PR was merged into the 3.2-dev branch. Discussion ---------- [HttpFoundation] Deprecate extending some methods | Q | A | ------------- | --- | Branch? | "master" | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #19727 | License | MIT | Doc PR | It's really hard to change methods signature because of bc. I'm proposing to deprecate extending some getters/setters of `Response` because of this (and because extending them is not really useful). If you like this approach it could be used in other places to simplify bc in 4.0. Edit: This causes issues (warnings always triggered) when mocking `Response` entirely but does it matter as people should only mock needed methods? Commits ------- c0a26bc [HttpFoundation] Deprecate extending some methods
2 parents 4053283 + c0a26bc commit 65e254c

File tree

4 files changed

+137
-0
lines changed

4 files changed

+137
-0
lines changed

UPGRADE-3.2.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,29 @@ FrameworkBundle
4141
* The service `serializer.mapping.cache.doctrine.apc` is deprecated. APCu should now
4242
be automatically used when available.
4343

44+
HttpFoundation
45+
---------------
46+
47+
* Extending the following methods of `Response`
48+
is deprecated (these methods will be `final` in 4.0):
49+
50+
- `setDate`/`getDate`
51+
- `setExpires`/`getExpires`
52+
- `setLastModified`/`getLastModified`
53+
- `setProtocolVersion`/`getProtocolVersion`
54+
- `setStatusCode`/`getStatusCode`
55+
- `setCharset`/`getCharset`
56+
- `setPrivate`/`setPublic`
57+
- `getAge`
58+
- `getMaxAge`/`setMaxAge`
59+
- `setSharedMaxAge`
60+
- `getTtl`/`setTtl`
61+
- `setClientTtl`
62+
- `getEtag`/`setEtag`
63+
- `hasVary`/`getVary`/`setVary`
64+
- `isInvalid`/`isSuccessful`/`isRedirection`/`isClientError`/`isServerError`
65+
- `isOk`/`isForbidden`/`isNotFound`/`isRedirect`/`isEmpty`
66+
4467
Validator
4568
---------
4669

UPGRADE-4.0.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,29 @@ FrameworkBundle
121121
* The `Controller::getUser()` method has been removed in favor of the ability
122122
to typehint the security user object in the action.
123123

124+
HttpFoundation
125+
---------------
126+
127+
* Extending the following methods of `Response`
128+
is no longer possible (these methods are now `final`):
129+
130+
- `setDate`/`getDate`
131+
- `setExpires`/`getExpires`
132+
- `setLastModified`/`getLastModified`
133+
- `setProtocolVersion`/`getProtocolVersion`
134+
- `setStatusCode`/`getStatusCode`
135+
- `setCharset`/`getCharset`
136+
- `setPrivate`/`setPublic`
137+
- `getAge`
138+
- `getMaxAge`/`setMaxAge`
139+
- `setSharedMaxAge`
140+
- `getTtl`/`setTtl`
141+
- `setClientTtl`
142+
- `getEtag`/`setEtag`
143+
- `hasVary`/`getVary`/`setVary`
144+
- `isInvalid`/`isSuccessful`/`isRedirection`/`isClientError`/`isServerError`
145+
- `isOk`/`isForbidden`/`isNotFound`/`isRedirect`/`isEmpty`
146+
124147
HttpKernel
125148
----------
126149

src/Symfony/Component/HttpFoundation/Response.php

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,30 @@ class Response
186186
511 => 'Network Authentication Required', // RFC6585
187187
);
188188

189+
private static $deprecatedMethods = array(
190+
'setDate', 'getDate',
191+
'setExpires', 'getExpires',
192+
'setLastModified', 'getLastModified',
193+
'setProtocolVersion', 'getProtocolVersion',
194+
'setStatusCode', 'getStatusCode',
195+
'setCharset', 'getCharset',
196+
'setPrivate', 'setPublic',
197+
'getAge', 'getMaxAge', 'setMaxAge', 'setSharedMaxAge',
198+
'getTtl', 'setTtl', 'setClientTtl',
199+
'getEtag', 'setEtag',
200+
'hasVary', 'getVary', 'setVary',
201+
'isInvalid', 'isSuccessful', 'isRedirection',
202+
'isClientError', 'isOk', 'isForbidden',
203+
'isNotFound', 'isRedirect', 'isEmpty',
204+
);
205+
private static $deprecationsTriggered = array(
206+
__CLASS__ => true,
207+
BinaryFileResponse::class => true,
208+
JsonResponse::class => true,
209+
RedirectResponse::class => true,
210+
StreamedResponse::class => true,
211+
);
212+
189213
/**
190214
* Constructor.
191215
*
@@ -201,6 +225,23 @@ public function __construct($content = '', $status = 200, $headers = array())
201225
$this->setContent($content);
202226
$this->setStatusCode($status);
203227
$this->setProtocolVersion('1.0');
228+
229+
// Deprecations
230+
$class = get_class($this);
231+
if ($this instanceof \PHPUnit_Framework_MockObject_MockObject || $this instanceof \Prophecy\Doubler\DoubleInterface) {
232+
$class = get_parent_class($class);
233+
}
234+
if (isset(self::$deprecationsTriggered[$class])) {
235+
return;
236+
}
237+
238+
self::$deprecationsTriggered[$class] = true;
239+
foreach (self::$deprecatedMethods as $method) {
240+
$r = new \ReflectionMethod($class, $method);
241+
if (__CLASS__ !== $r->getDeclaringClass()->getName()) {
242+
@trigger_error(sprintf('Extending %s::%s() in %s is deprecated since version 3.2 and won\'t be supported anymore in 4.0 as it will be final.', __CLASS__, $method, $class), E_USER_DEPRECATED);
243+
}
244+
}
204245
}
205246

206247
/**

src/Symfony/Component/HttpFoundation/Tests/ResponseTest.php

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@
1111

1212
namespace Symfony\Component\HttpFoundation\Tests;
1313

14+
use Response\DefaultResponse;
15+
use Response\ExtendedResponse;
16+
use Symfony\Bridge\PhpUnit\ErrorAssert;
1417
use Symfony\Component\HttpFoundation\Request;
1518
use Symfony\Component\HttpFoundation\Response;
1619

@@ -843,6 +846,34 @@ public function testSettersAreChainable()
843846
}
844847
}
845848

849+
/**
850+
* @requires function Symfony\Bridge\PhpUnit\ErrorAssert::assertDeprecationsAreTriggered
851+
*/
852+
public function testNoDeprecations()
853+
{
854+
ErrorAssert::assertDeprecationsAreTriggered(array(), function () {
855+
new DefaultResponse();
856+
$this->getMock(Response::class);
857+
});
858+
}
859+
860+
/**
861+
* @requires function Symfony\Bridge\PhpUnit\ErrorAssert::assertDeprecationsAreTriggered
862+
*/
863+
public function testDeprecations()
864+
{
865+
$deprecationMessages = array();
866+
foreach (array('getDate', 'setLastModified') as $method) {
867+
$deprecationMessages[] = sprintf('Extending %s::%s() in Response\ExtendedResponse is deprecated', Response::class, $method);
868+
}
869+
ErrorAssert::assertDeprecationsAreTriggered($deprecationMessages, function () {
870+
new ExtendedResponse();
871+
872+
// Deprecations should not be triggered twice
873+
new ExtendedResponse();
874+
});
875+
}
876+
846877
public function validContentProvider()
847878
{
848879
return array(
@@ -891,3 +922,22 @@ public function __toString()
891922
return 'Foo';
892923
}
893924
}
925+
926+
namespace Response;
927+
928+
use Symfony\Component\HttpFoundation\Response;
929+
930+
class DefaultResponse extends Response
931+
{
932+
}
933+
934+
class ExtendedResponse extends Response
935+
{
936+
public function setLastModified(\DateTime $date = null)
937+
{
938+
}
939+
940+
public function getDate()
941+
{
942+
}
943+
}

0 commit comments

Comments
 (0)
0