Update Guzzle to 6.3 by phil-davis · Pull Request #29773 · owncloud/core

phil-davis · 2017-12-06T15:54:11Z

Description

Update Guzzle to 6.3, and also other libraries (deleted and regenerated composer.lock)

Related Issue

Motivation and Context

Guzzle 5.3 is outdated, needs update.
Also some libs used by apps will need Guzzle > 6.

How Has This Been Tested?

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

This is a newer version of #29211 made by cherry-pick and reorganize of the commits from there, to massage it all into these "logical" commits.

codecov · 2017-12-07T01:04:32Z

Codecov Report

Merging #29773 into master will decrease coverage by 0.24%.
The diff coverage is 50%.

@@             Coverage Diff              @@
##             master   #29773      +/-   ##
============================================
- Coverage     64.01%   63.77%   -0.25%     
- Complexity    18560    18563       +3     
============================================
  Files          1171     1171              
  Lines         69841    69814      -27     
  Branches       1267     1267              
============================================
- Hits          44709    44521     -188     
- Misses        24762    24923     +161     
  Partials        370      370

Flag	Coverage Δ	Complexity Δ
#javascript	`52.84% <ø> (ø)`	`0 <ø> (ø)`	⬇️
#phpunit	`65.02% <50%> (-0.28%)`	`18563 <1> (+3)`

Impacted Files	Coverage Δ	Complexity Δ
lib/private/Files/Storage/DAV.php	`81.09% <ø> (ø)`	`0 <0> (ø)`	⬇️
lib/private/Http/Client/Response.php	`53.84% <100%> (ø)`	`6 <1> (ø)`	⬇️
lib/private/Http/Client/Client.php	`79.62% <44.44%> (-2.38%)`	`19 <0> (+2)`
...ivate/Files/ObjectStore/HomeObjectStoreStorage.php	`0% <0%> (-86.67%)`	`8% <0%> (ø)`
lib/private/Files/Storage/Wrapper/Availability.php	`10.38% <0%> (-38.97%)`	`80% <0%> (ø)`
lib/private/Files/ObjectStore/NoopScanner.php	`61.53% <0%> (-23.08%)`	`5% <0%> (ø)`
lib/private/Preview/MP3.php	`45.16% <0%> (-12.91%)`	`13% <0%> (ø)`
...b/private/Files/ObjectStore/ObjectStoreStorage.php	`71.37% <0%> (-12.55%)`	`93% <0%> (ø)`
lib/private/App/Platform.php	`21.73% <0%> (-8.7%)`	`9% <0%> (ø)`
apps/files_sharing/lib/Scanner.php	`90% <0%> (-5%)`	`7% <0%> (ø)`
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bc75987...91363af. Read the comment docs.

phil-davis · 2018-07-18T15:09:14Z

Note: I will rebase and sort this out late Friday.

phil-davis · 2018-07-23T13:47:32Z

Rebased. ToDo:

look into bonus unit test fails https://drone.owncloud.com/owncloud/core/8801/158

phil-davis · 2018-07-25T12:23:53Z

@PVince81 @DeepDiver1975 or whoever. Can you comment on #29773 (comment) - the "ordinary" 5 unit test fails.

And now I am getting other unit tests fails only in the objectstore job:
https://drone.owncloud.com/owncloud/core/8909/158
that seems to be coming from:
https://github.com/owncloud/core/pull/29773/files#diff-95a3c3582e6c3c585d01ba70836cd761R65

$config = $this->client->getConfig();

I suspect that one of you will know straight away why this is, rather than me spending lots of time trying to work out what is going on...

Edit add: and now a look at it again, it looks like there are changes needed for Guzzle v6 in files_primary_s3, which is enabled for a job in the test matrix. So that (like lots of other app stuff) will need coordination...

PVince81 · 2018-08-08T08:02:26Z

@phil-davis maybe we first need to make phan happy:

lib/private/Http/Client/Response.php:56 PhanUndeclaredClassMethod Call to method getBody from undeclared class \OC\Http\Client\GuzzleResponse

DeepDiver1975

left some minor comments ....

DeepDiver1975 · 2018-08-14T13:14:58Z

 		$exception = $this->getMockBuilder('\GuzzleHttp\Exception\ClientException')
 			->disableOriginalConstructor()->getMock();
-		$response = $this->getMockBuilder('\GuzzleHttp\Message\ResponseInterface')
+		$response = $this->getMockBuilder('\Psr\Http\Message\ResponseInterface')


Should all getMockBuilder() calls use ::class - e.g. 1 line above here that has not been touched by the PR (yet)?

anywhere a class name is referenced please use ::class

Usually I do this when touching any test case - and then for all occurrences.
Please have a look at https://plugins.jetbrains.com/plugin/7622-php-inspections-ea-extended- - help with this on file level.

DeepDiver1975 · 2018-08-14T13:15:08Z

 		$exception = $this->getMockBuilder('\GuzzleHttp\Exception\ClientException')
 			->disableOriginalConstructor()->getMock();
-		$response = $this->getMockBuilder('\GuzzleHttp\Message\ResponseInterface')
+		$response = $this->getMockBuilder('\Psr\Http\Message\ResponseInterface')


DeepDiver1975 · 2018-08-14T13:18:33Z

@@ -191,16 +192,17 @@ public function userHasCreatedACalendarNamed($user, $name) {
 		$davUrl = $this->featureContext->getBaseUrl()
 			. '/remote.php/dav/calendars/' . $user . '/' . $name;


"/remote.php/dav/calendars/$user/ $name";

This was not introduced in the PR.
Refactoring of string concatenation for acceptance test code is in PR #32336

phil-davis · 2018-08-14T13:26:59Z

 	public function setUp() {
 		parent::setUp();
 		$this->config = $this->createMock('\OCP\IConfig');
+		$this->response = $this->createMock('\Psr\Http\Message\ResponseInterface');


@DeepDiver1975 I was "hacking around" here sorting out what happened in Guzzle6 to the client and responses...

Is this the right sort of thing to do for the unit tests here?

Should this also have ::class? or it's OK being an interface?

createMock looks good

and yes - use ::class here as well

phil-davis · 2018-08-15T03:46:40Z

@DeepDiver1975 I have fixed up what I can.
https://drone.owncloud.com/owncloud/core/9460/158
The drone test job on objectstore clones the existing files_primary_s3 master and that, of course, has Guzzle5 in it. The job gets some errors in the unit tests.

Maybe next we need to make a PR/branch in files_primary_s3 that also has the Guzzle6 changes, and run CI here in core with that files_primary_s3 branch?

DeepDiver1975 · 2018-08-16T08:59:29Z

all apps which use guzzle need adoptions .... files-primary_s3, google, onedrive .......

phil-davis · 2018-08-16T09:09:51Z

@PVince81 @DeepDiver1975 @patrickjahns
I inherited this PR for some reason lost in the mists of time.
How do we proceed now with resource allocation and priority of getting all the needed app changes ready and planning implementation?

DeepDiver1975 · 2018-08-16T09:13:31Z

How do we proceed now with resource allocation and priority of getting all the needed app changes ready and planning implementation?

I suggest we plan this for the next server sprint - which starts next week. Vincent will be back by then as well.

phil-davis · 2018-08-31T15:10:07Z

See PR #32537 for the next iteration.

That is the Guzzle V6 code rebased and refactored for the underlying refactoring of acceptance test code that happened today. That introduced HttpRequestHelper which vastly reduced the places in acceptance test code where Guzzle client code is use. And thus it also vastly reduced the code diffs for upgrading to Guzzzle V6, which is "a god thing" (TM).

phil-davis added the 2 - Developing label Dec 6, 2017

phil-davis self-assigned this Dec 6, 2017

phil-davis changed the title ~~Update guzzle 2~~ Update Guzzle to 6.3 Dec 6, 2017

phil-davis force-pushed the update-guzzle-2 branch from 7dd50a4 to 94e00fd Compare December 7, 2017 01:04

phil-davis added 3 - To Review and removed 2 - Developing labels Dec 7, 2017

phil-davis added this to the triage milestone Dec 7, 2017

phil-davis requested review from DeepDiver1975, PVince81 and SergioBertolinSG December 7, 2017 01:06

phil-davis mentioned this pull request Dec 7, 2017

Update guzzle to 6.3 #29211

Closed

9 tasks

phil-davis force-pushed the update-guzzle-2 branch 4 times, most recently from 658c8ed to 5fb9844 Compare December 13, 2017 09:36

phil-davis force-pushed the update-guzzle-2 branch 3 times, most recently from f82f06d to ea11974 Compare December 27, 2017 07:48

ownclouders added the status/STALE label Jan 4, 2018

phil-davis force-pushed the update-guzzle-2 branch from ea11974 to 6285911 Compare January 8, 2018 06:56

ownclouders removed the status/STALE label Jan 8, 2018

phil-davis force-pushed the update-guzzle-2 branch 3 times, most recently from 453a58f to 2897446 Compare January 11, 2018 20:20

ownclouders added the status/STALE label Jan 19, 2018

phil-davis force-pushed the update-guzzle-2 branch from 2897446 to 1f79f03 Compare January 20, 2018 15:53

ownclouders removed the status/STALE label Jan 20, 2018

phil-davis force-pushed the update-guzzle-2 branch 2 times, most recently from 7846bc1 to 028e69a Compare January 25, 2018 06:31

phil-davis force-pushed the update-guzzle-2 branch 5 times, most recently from 4889295 to 6d4013e Compare April 27, 2018 06:18

phil-davis force-pushed the update-guzzle-2 branch 4 times, most recently from b72ff7b to 9dd3caa Compare May 7, 2018 07:58

phil-davis force-pushed the update-guzzle-2 branch 2 times, most recently from 9166ebb to 4141e88 Compare May 10, 2018 13:49

Vincent Petry and others added 6 commits August 14, 2018 16:27

Update libraries, and Guzzle to 6.3

953e85e

Adjust core for Guzzle V6

d5a1be1

Adjust integration tests for Guzzle V6

23afce2

Fix new getArrayOfAppInfoResponded xml method

a470427

Fixup ResponseInterface in Response.php

3e04b19

Fixup ClientTest

1ef9ff0

DeepDiver1975 reviewed Aug 14, 2018

View reviewed changes

phil-davis commented Aug 14, 2018

View reviewed changes

Add ::class in tests

91363af

phil-davis mentioned this pull request Aug 14, 2018

Style of acceptance test string concatenation #32336

Merged

7 tasks

		@@ -191,16 +192,17 @@ public function userHasCreatedACalendarNamed($user, $name) {
		$davUrl = $this->featureContext->getBaseUrl()
		. '/remote.php/dav/calendars/' . $user . '/' . $name;

Uh oh!

Conversation

phil-davis commented Dec 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Uh oh!

codecov Bot commented Dec 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

phil-davis commented Jul 18, 2018

Uh oh!

phil-davis commented Jul 23, 2018

Uh oh!

phil-davis commented Jul 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PVince81 commented Aug 8, 2018

Uh oh!

DeepDiver1975 left a comment

Choose a reason for hiding this comment

Uh oh!

DeepDiver1975 Aug 14, 2018

Choose a reason for hiding this comment

Uh oh!

phil-davis Aug 14, 2018

Choose a reason for hiding this comment

Uh oh!

DeepDiver1975 Aug 14, 2018

Choose a reason for hiding this comment

Uh oh!

DeepDiver1975 Aug 14, 2018

Choose a reason for hiding this comment

Uh oh!

DeepDiver1975 Aug 14, 2018

Choose a reason for hiding this comment

Uh oh!

phil-davis Aug 14, 2018

Choose a reason for hiding this comment

Uh oh!

phil-davis Aug 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DeepDiver1975 Aug 14, 2018

Choose a reason for hiding this comment

Uh oh!

phil-davis commented Aug 15, 2018

Uh oh!

DeepDiver1975 commented Aug 16, 2018

Uh oh!

phil-davis commented Aug 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DeepDiver1975 commented Aug 16, 2018

Uh oh!

phil-davis commented Aug 31, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

phil-davis commented Dec 6, 2017 •

edited

Loading

codecov Bot commented Dec 7, 2017 •

edited

Loading

phil-davis commented Jul 25, 2018 •

edited

Loading

phil-davis Aug 14, 2018 •

edited

Loading

phil-davis commented Aug 16, 2018 •

edited

Loading