Problem/Motivation

Guzzle was update to version 5 in #2348365: Update to Guzzle 5

Guzzle 5 does not use getBody(TRUE) to return a string. Instead you need to call getBody() and cast it to a string.

Proposed resolution

Convert all uses of getBody(TRUE)

Remaining tasks

Do it.

User interface changes

None

API changes

None

CommentFileSizeAuthor
#8 2401159-8.patch2.74 KBJaesin
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 98,792 pass(es). View
#7 2401159-get-body-7.patch2.74 KBkim.pepper
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,376 pass(es). View
#1 2401159-get-body-1.patch3.48 KBkim.pepper
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,351 pass(es). View
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

kim.pepper’s picture

Status: Active » Needs review
Related issues: +#2348365: Update to Guzzle 5
FileSize
3.48 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,351 pass(es). View
kim.pepper’s picture

This already caused one known issue #2401113: Update Zend Feed to latest stable

webchick’s picture

Issue tags: +Needs tests

Dang. Seems like we're missing a bunch of test coverage here, then. :(

larowlan’s picture

loose typing--, many of the places are implicitly cast to string already depending on usage - there was one that caused fails in the original issue we had to update.
Thanks for doing the rest.
In terms of tests - the fails in #2401113: Update Zend Feed to latest stable would indicate we are ok - the change in ZendFeed lost the implicit cast I assume?

kim.pepper’s picture

Yep. I think the additional check in zend-feed 2.3.* if (!is_string($string) || empty($trimmed)) { is the trigger as is_string will be false whereas it was being implicitly cast further down.

kim.pepper’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

Needs reroll after #2401113: Update Zend Feed to latest stable was committed.

kim.pepper’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
2.74 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,376 pass(es). View

Re-roll.

Jaesin’s picture

FileSize
2.74 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 98,792 pass(es). View

Re-roll.

jibran queued 8: 2401159-8.patch for re-testing.

dawehner’s picture

So now that we have guzzle 6, this parameter no longer exists, see http://www.php-fig.org/psr/psr-7/

I guess then the casting, what we currently do is exactly the right thing we do.

Berdir queued 8: 2401159-8.patch for re-testing.

Berdir’s picture

The parameter was already removed in Guzzle 5, so nothing changed I think...

dawehner’s picture

OH yeah, you are right.

Are we sure that the test coverage really makes sense? We are probably casting somewhere, but not explicit to the string later?

Berdir’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

Yes. I'm pretty sure nothing is actually broken here, all things things are covered by tests.

We're just removing a bogus argument and doing the string cast very explicitly instead of relying that it happens somewhere down the line, e.g. while writing it out to a file.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed efd4da3 and pushed to 8.0.x. Thanks!

  • alexpott committed efd4da3 on 8.0.x
    Issue #2401159 by kim.pepper, Jaesin: Convert instances of Guzzle...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.