Skip to content

Add support for PSR-18 HTTP clients#777

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

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

Conversation

@Art4
Copy link
Contributor

@Art4 Art4 commented Jan 27, 2023

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)
Copy link
Contributor Author

@Art4 Art4 Feb 16, 2023

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
Copy link
Contributor Author

Art4 commented Feb 17, 2023

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

Copy link
Member

@jtojnar jtojnar left a comment

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.

$requestedUrl = $response->getHeaderLine('Location');

if ($statusCode === 301) {
$permanentUrl = $requestedUrl;
Copy link
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
Copy link
Member

mblaney commented Aug 25, 2023

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

@Art4
Copy link
Contributor Author

Art4 commented Aug 25, 2023

@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