Problem/Motivation
Guzzle 7 is compatible with PSR-18 out of the box, but PSR-17 compatibility is only available with the 2.x branch of the guzzlehttp/psr7 component. (Yes, they acknowledge that the naming is confusing.)
Drupal currently uses Diactoros for its PSR-17 implementation. However, Laminas projects have recently adopted a policy of placing an upper bound on PHP version compatibility, meaning we need to wait for them to do a release after every PHP minor in order to support that minor, which means we could go 6+ months without supporting the latest PHP version. Therefore, it is desirable to look for alternatives to the Laminas packages.
Proposed resolution
Test guzzlehttp/psr7 2.1.0 by explicitly requiring it.
After we land this issue, we move to #3255243: Replace Diactoros' PSR-17 implementation with Guzzle's.
Remaining tasks
TBD
User interface changes
N/A
API changes
TBD (but the PSR interfaces are supposed to be the public API, so...)
Data model changes
TBD
Release notes snippet
guzzlehttp/psr7 has been updated to 2.1.0
| Comment | File | Size | Author |
|---|---|---|---|
| #16 | 3220220-16.patch | 4.26 KB | longwave |
Issue fork drupal-3220220
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3220220-psr17
changes, plain diff MR !836
Comments
Comment #3
xjmComment #4
xjmThe two fails are the two cases of
ProviderRepositoryTest::testEmptyProviderList()(checking for the case where either an empty string or an empty array is returned as the JSON content of the provider database:The test method code:
GuzzleHttp\Psr7\MessageTrait::getBody()is changed in the 2.x branch to have a return typehint ofStreamInterface. (There was no return typehint in the 1.x branch.) This means that the expected exception can't ever be thrown because there's aTypeErrorbefore we get to that point.It makes me suspect the test wasn't quite testing the correct thing to begin with -- even in 1.x, the docs say
getBody()should only returnStreamInterfaceorNULL. I suspect what we want to test is that the contents of the response evaluate to the empty values. I don't think we want to change the test to expectTypeErrorbecause the point is to test our custom exception handling.Comment #5
xjmComment #6
phenaproximaYeah, I suspect this was just an oversight. I would suggest we change the test to return a stream! I would think this will work:
Comment #7
xjmLocally, #6 does the trick (when the correct namespace for
Utilsis added which I missed at first). Thanks @phenaproxima!Comment #8
xjm(Assuming that passes, I'll split it into its own issue since it's wrong-ish in HEAD as well.)
Comment #9
andypost2.0 released
Comment #10
andypostRebased and upgraded
guzzlehttp/psr7to 2.0.0Comment #11
gábor hojtsyNow that the closer alpha1 requirements issue exists, reparenting to that: #3251854: [META] Requirements for tagging Drupal 10.0.0-alpha1.
Comment #13
andypostComment #14
xjmForgot to mention here what I mentioned on the D10 roadmap. Let's split this into two issues:
guzzlehttp/psr7ones.Comment #15
gábor hojtsy2.1.0 is available as of two months ago, should we update to that directly? I assume so.
Opened #3255243: Replace Diactoros' PSR-17 implementation with Guzzle's and updated this issue summary.
Comment #16
longwaveComment #17
andypostThere's follow-up and tests pass
Comment #19
catchCommitted aa1ef1a and pushed to 10.0.x. Thanks!
Comment #20
longwaveComment #21
gábor hojtsyAlso needs release note snippet to be copied :D
Comment #22
catchAdded a release note snippet.