Conversation
b82278d to
7441fae
Compare
909bd36 to
1a6aa8b
Compare
| * | ||
| * @return array<string, string[]> | ||
| */ | ||
| public function get_http_headers(): array |
There was a problem hiding this comment.
Not sure if we want to have this. Maybe exposing Response will be better.
There was a problem hiding this comment.
I think this new method is a reasonable compromise. Once we have exposed the Response class or implemented #190, we will be able to deprecate many methods.
There was a problem hiding this comment.
One issue is that the File turns all header names to lowercase, while Psr7Response keeps the original casing. So it is inconsistent between clients.
Ideally, we would keep the introduction of new methods minimal but with Psr7Response response behaviour, if user is interseted in a specific header, they would need to lowercase it themselves, which is inconvenient.
|
|
||
| $this->assertFalse($return); | ||
| $this->assertSame(429, $simplepie->status_code()); | ||
| $this->assertSame(['3600'], $simplepie->get_http_header('Retry-After')); |
There was a problem hiding this comment.
Maybe adding a test for SimplePie::get_http_headers() as well?
There was a problem hiding this comment.
Actually, looking at the headers, most of them are set by PHP test server itself so we probably cannot rely on them remaining consistent:
'Host' => Array &1 [
0 => '127.0.0.1:39477',
],
'Date' => Array &2 [
0 => 'Fri, 01 Aug 2025 08:57:44 GMT',
],
'Connection' => Array &3 [
0 => 'close',
],
'X-Powered-By' => Array &4 [
0 => 'PHP/8.4.8',
],
'Retry-After' => Array &5 [
0 => '3600',
],
'Content-type' => Array &6 [
0 => 'text/html; charset=UTF-8',
],There was a problem hiding this comment.
Could you add a unit test with a mocked response instead?
If we enable `CURLOPT_FAILONERROR`, `curl_exec` will return `false` on HTTP error so we would not get access to response and thus unable to parse headers. Reverts 9b5f209
c2b9531 to
98917bd
Compare
Currently, the `SimplePie::$data` property is only marked `@access private` using a PHPDoc comment so consumers can access HTTP headers in that. But when we mark it as private later, there would be no way to obtain HTTP headers.
…Response` with `Psr18Client`
98917bd to
d6077f7
Compare
Tries to fix #905 (comment)