Skip to content

Add support for PSR-18 HTTP clients#777

Merged
mblaney merged 42 commits into
simplepie:masterfrom
Art4:add-psr18-http-client-support
Aug 26, 2023
Merged

Add support for PSR-18 HTTP clients#777
mblaney merged 42 commits into
simplepie:masterfrom
Art4:add-psr18-http-client-support

Conversation

@Art4

@Art4 Art4 commented Jan 27, 2023

Copy link
Copy Markdown
Contributor

Hi all 👋

This is a follow up of #774 and will add support for every PSR-18 HTTP client implementation. 🥳

This PR will fixes #520. I propose this PR for SimplePie 1.9.0, see #731.

* @param File $file Input file
*/
public function __construct(File $file)
public function __construct(/* File */ $file)

@Art4 Art4 Feb 16, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the type hint is not a breaking change because we have not released 1.9.0 yet.

@Art4

Art4 commented Feb 17, 2023

Copy link
Copy Markdown
Contributor Author

This PR is ready for review. Because this PR is build on #774 I recommend to merge this PR first. It will fix some BC breaks that comes with #774.

To show only the changes between #774 and #777 you can look at the diff between the branches: Art4/simplepie@split-file-into-client-and-response...Art4:simplepie:add-psr18-http-client-support

@jtojnar jtojnar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still need to resolve the question of non-absolute Location header and Redirect tracking.

Comment thread src/HTTP/Psr18Client.php
$requestedUrl = $response->getHeaderLine('Location');

if ($statusCode === 301) {
$permanentUrl = $requestedUrl;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need some kind of mechanism to not change the permanent URL after temporary redirects (as tested by #819). For example, this is what File does:

$this->permanentUrlMutable = $this->permanentUrlMutable && ($this->status_code == 301 || $this->status_code == 308);

@mblaney

mblaney commented Aug 25, 2023

Copy link
Copy Markdown
Member

thanks @Art4 this looks good, can we merge and handle the redirect case separately?

@Art4

Art4 commented Aug 25, 2023

Copy link
Copy Markdown
Contributor Author

@mblaney I resolved all conflicts and fixed all PHPStan errors.

@mblaney mblaney merged commit 8f45077 into simplepie:master Aug 26, 2023
@Art4 Art4 deleted the add-psr18-http-client-support branch August 26, 2023 09:18
jtojnar added a commit to jtojnar/simplepie that referenced this pull request Nov 6, 2023
…ent-support"

There are correctness issues as well as disorganized commit history.
The fixed changes are re-applied in the commits that follow.
@Art4 Art4 added this to the 1.9.0 milestone Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migration to a HTTP library

3 participants