Skip to content

Commit

Permalink
feature #32122 [HttpFoundation] deprecate HeaderBag::get() returning …
Browse files Browse the repository at this point in the history
…an array and add all($key) instead (Simperfit)

This PR was merged into the 4.4 branch.

Discussion
----------

[HttpFoundation] deprecate HeaderBag::get() returning an array and add all($key) instead

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | maybe <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #31317  <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | todo <!-- required for new features -->

<!--
Replace this notice by a short README for your feature/bugfix. This will help people
understand your PR and can be used as a start for the documentation.

Additionally (see https://symfony.com/roadmap):
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against branch 4.4.
 - Legacy code removals go to the master branch.
-->

the $first param has been deprecated in the get methid
and we are adding a $key parameter to all to get all values from a key as arrays
Do we deprecated the get method ? if so this will be a little bigger in terms of changes.

Commits
-------

2c5a8f1 [HttpFoundation] deprecate using $first in get and added key in all
  • Loading branch information
nicolas-grekas committed Aug 8, 2019
2 parents cf57007 + 2c5a8f1 commit 29dbbe1
Show file tree
Hide file tree
Showing 12 changed files with 53 additions and 30 deletions.
1 change: 1 addition & 0 deletions UPGRADE-4.4.md
Expand Up @@ -105,6 +105,7 @@ HttpFoundation
--------------

* `ApacheRequest` is deprecated, use `Request` class instead.
* Passing a third argument to `HeaderBag::get()` is deprecated since Symfony 4.4, use method `all()` instead

HttpKernel
----------
Expand Down
1 change: 1 addition & 0 deletions UPGRADE-5.0.md
Expand Up @@ -284,6 +284,7 @@ HttpFoundation
* The `FileinfoMimeTypeGuesser` class has been removed,
use `Symfony\Component\Mime\FileinfoMimeTypeGuesser` instead.
* `ApacheRequest` has been removed, use the `Request` class instead.
* The third argument of the `HeaderBag::get()` method has been removed, use method `all()` instead.

HttpKernel
----------
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Component/HttpFoundation/CHANGELOG.md
Expand Up @@ -6,6 +6,7 @@ CHANGELOG

* passing arguments to `Request::isMethodSafe()` is deprecated.
* `ApacheRequest` is deprecated, use the `Request` class instead.
* passing a third argument to `HeaderBag::get()` is deprecated, use method `all()` instead

4.3.0
-----
Expand Down
35 changes: 18 additions & 17 deletions src/Symfony/Component/HttpFoundation/HeaderBag.php
Expand Up @@ -58,10 +58,18 @@ public function __toString()
/**
* Returns the headers.
*
* @param string|null $key The name of the headers to return or null to get them all
*
* @return array An array of headers
*/
public function all()
public function all(/*string $key = null*/)
{
if (1 <= \func_num_args() && null !== $key = func_get_arg(0)) {
$key = str_replace('_', '-', strtolower($key));

return $this->headers[$key] ?? [];
}

return $this->headers;
}

Expand Down Expand Up @@ -103,28 +111,21 @@ public function add(array $headers)
*
* @param string $key The header name
* @param string|null $default The default value
* @param bool $first Whether to return the first value or all header values
*
* @return string|string[]|null The first header value or default value if $first is true, an array of values otherwise
* @return string|null The first header value or default value
*/
public function get($key, $default = null, $first = true)
public function get($key, $default = null)
{
$key = str_replace('_', '-', strtolower($key));
$headers = $this->all();
$headers = $this->all((string) $key);
if (2 < \func_num_args()) {
@trigger_error(sprintf('Passing a third argument to "%s()" is deprecated since Symfony 4.4, use method "all()" instead', __METHOD__), E_USER_DEPRECATED);

if (!\array_key_exists($key, $headers)) {
if (null === $default) {
return $first ? null : [];
if (!func_get_arg(2)) {
return $headers;
}

return $first ? $default : [$default];
}

if ($first) {
return \count($headers[$key]) ? $headers[$key][0] : $default;
}

return $headers[$key];
return $headers[0] ?? $default;
}

/**
Expand Down Expand Up @@ -181,7 +182,7 @@ public function has($key)
*/
public function contains($key, $value)
{
return \in_array($value, $this->get($key, null, false));
return \in_array($value, $this->all((string) $key));
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/HttpFoundation/Response.php
Expand Up @@ -1024,7 +1024,7 @@ public function hasVary(): bool
*/
public function getVary(): array
{
if (!$vary = $this->headers->get('Vary', null, false)) {
if (!$vary = $this->headers->all('Vary')) {
return [];
}

Expand Down
11 changes: 10 additions & 1 deletion src/Symfony/Component/HttpFoundation/ResponseHeaderBag.php
Expand Up @@ -87,10 +87,19 @@ public function replace(array $headers = [])

/**
* {@inheritdoc}
*
* @param string|null $key The name of the headers to return or null to get them all
*/
public function all()
public function all(/*string $key = null*/)
{
$headers = parent::all();

if (1 <= \func_num_args() && null !== $key = func_get_arg(0)) {
$key = str_replace('_', '-', strtolower($key));

return 'set-cookie' !== $key ? $headers[$key] ?? [] : array_map('strval', $this->getCookies());
}

foreach ($this->getCookies() as $cookie) {
$headers['set-cookie'][] = (string) $cookie;
}
Expand Down
Expand Up @@ -40,7 +40,7 @@ public function toString(): string
*/
protected function matches($response): bool
{
return $this->expectedValue === $response->headers->get($this->headerName, null, true);
return $this->expectedValue === $response->headers->get($this->headerName, null);
}

/**
Expand Down
18 changes: 14 additions & 4 deletions src/Symfony/Component/HttpFoundation/Tests/HeaderBagTest.php
Expand Up @@ -86,24 +86,34 @@ public function testGet()
$bag = new HeaderBag(['foo' => 'bar', 'fuzz' => 'bizz']);
$this->assertEquals('bar', $bag->get('foo'), '->get return current value');
$this->assertEquals('bar', $bag->get('FoO'), '->get key in case insensitive');
$this->assertEquals(['bar'], $bag->get('foo', 'nope', false), '->get return the value as array');
$this->assertEquals(['bar'], $bag->all('foo'), '->get return the value as array');

// defaults
$this->assertNull($bag->get('none'), '->get unknown values returns null');
$this->assertEquals('default', $bag->get('none', 'default'), '->get unknown values returns default');
$this->assertEquals(['default'], $bag->get('none', 'default', false), '->get unknown values returns default as array');
$this->assertEquals([], $bag->all('none'), '->get unknown values returns an empty array');

$bag->set('foo', 'bor', false);
$this->assertEquals('bar', $bag->get('foo'), '->get return first value');
$this->assertEquals(['bar', 'bor'], $bag->get('foo', 'nope', false), '->get return all values as array');
$this->assertEquals(['bar', 'bor'], $bag->all('foo'), '->get return all values as array');
}

/**
* @group legacy
* @expectedDeprecation Passing a third argument to "Symfony\Component\HttpFoundation\HeaderBag::get()" is deprecated since Symfony 4.4, use method "all()" instead
*/
public function testGetIsEqualToNewMethod()
{
$bag = new HeaderBag(['foo' => 'bar', 'fuzz' => 'bizz']);
$this->assertSame($bag->all('none'), $bag->get('none', [], false), '->get unknown values returns default as array');
}

public function testSetAssociativeArray()
{
$bag = new HeaderBag();
$bag->set('foo', ['bad-assoc-index' => 'value']);
$this->assertSame('value', $bag->get('foo'));
$this->assertEquals(['value'], $bag->get('foo', 'nope', false), 'assoc indices of multi-valued headers are ignored');
$this->assertSame(['value'], $bag->all('foo'), 'assoc indices of multi-valued headers are ignored');
}

public function testContains()
Expand Down
Expand Up @@ -89,13 +89,13 @@ public function testCacheControlHeader()

$bag = new ResponseHeaderBag();
$bag->set('Cache-Control', ['public', 'must-revalidate']);
$this->assertCount(1, $bag->get('Cache-Control', null, false));
$this->assertCount(1, $bag->all('Cache-Control'));
$this->assertEquals('must-revalidate, public', $bag->get('Cache-Control'));

$bag = new ResponseHeaderBag();
$bag->set('Cache-Control', 'public');
$bag->set('Cache-Control', 'must-revalidate', false);
$this->assertCount(1, $bag->get('Cache-Control', null, false));
$this->assertCount(1, $bag->all('Cache-Control'));
$this->assertEquals('must-revalidate, public', $bag->get('Cache-Control'));
}

Expand Down Expand Up @@ -166,7 +166,7 @@ public function testCookiesWithSameNames()
'foo=bar; path=/path/bar; domain=foo.bar; httponly; samesite=lax',
'foo=bar; path=/path/bar; domain=bar.foo; httponly; samesite=lax',
'foo=bar; path=/; httponly; samesite=lax',
], $bag->get('set-cookie', null, false));
], $bag->all('set-cookie'));

$this->assertSetCookieHeader('foo=bar; path=/path/foo; domain=foo.bar; httponly; samesite=lax', $bag);
$this->assertSetCookieHeader('foo=bar; path=/path/bar; domain=foo.bar; httponly; samesite=lax', $bag);
Expand Down
Expand Up @@ -123,7 +123,7 @@ public function testSessionWithNewSessionIdAndNewCookieDoesNotSendAnotherCookie(

$response = $this->filterResponse(new Request(), HttpKernelInterface::MASTER_REQUEST, $response);

$this->assertSame($expected, $response->headers->get('Set-Cookie', null, false));
$this->assertSame($expected, $response->headers->all()['set-cookie']);
}

public function anotherCookieProvider()
Expand Down
Expand Up @@ -47,7 +47,7 @@ public function testOnKernelResponse()
'</foo>; rel="preload"',
];

$this->assertEquals($expected, $response->headers->get('Link', null, false));
$this->assertEquals($expected, $response->headers->all()['link']);
}

public function testSubscribedEvents()
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/WebLink/composer.json
Expand Up @@ -24,7 +24,7 @@
"symfony/http-kernel": ""
},
"require-dev": {
"symfony/http-foundation": "^3.4|^4.0|^5.0",
"symfony/http-foundation": "^4.4|^5.0",
"symfony/http-kernel": "^4.3|^5.0"
},
"conflict": {
Expand Down

0 comments on commit 29dbbe1

Please sign in to comment.