Update libraries, and Guzzle to 6.2

[PVince81]

Description

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

Expecting many problems as there were API changes in that lib.

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?

Screenshots (if appropriate):

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.

[PVince81]

First fail:

.......WWWWWPHP Fatal error:  Class 'GuzzleHttp\Message\Response' not found in /srv/www/htdocs/owncloud/tests/lib/Http/Client/ResponseTest.php on line 26

With a bit of luck if all our code uses the Http\Client wrapper, we only need to adjust that one.

[PVince81]

Guide here: https://github.com/guzzle/guzzle/blob/master/UPGRADING.md

• setDefaultOption has been removed but is used in Http\Client\Client

[PVince81]

Needs retesting:

• TEST: connecting to federated share with proxy server
• TEST: federated sharing (incoming, outgoing, etc)
• TEST: Ext storages that use Guzzle:
  • S3
  • GDrive
  • others ?

[PVince81]

• TEST: with PHP 5.6 (if we want to backport this monster to 10.0.x ...)

[PVince81]

unit tests passed for me locally, but I expected that some of these API changes aren't covered.

• TODO: adjust use statements from Guzzle: MessageInterface

[PVince81]

Took me a while to adjust the integration test code to work with Guzzle v6, especially because none of it is properly wrapped with utility functions.

Let's see, integration tests should go further now. @SergioBertolinSG

[PVince81]

• TEST: market connection
• TEST: certificate stuff

[PVince81]

• TEST: smashbox (better run it before merging, just in case, it also covers from federated sharing cases AFAIK)

[PVince81]

added more fixes, apparently you can only read the body once.

[PVince81]

Added more fixes, also in core for "POST". Federated sharing integration tests passed locally, so it looks promising.

• update market app guzzle too ??

[PVince81]

Great, CI passed!

Now still need to manually test everything I listed above.

[PVince81]

• automated tests of amazon s3 passed (make test-external TEST_EXTERNAL_ENV=amazons3-ceph) but I suspect they did because the code paths used by the tests are going through the storage code directly so it loads the old Guzzle bundled by aws-php-sdk.

In general we should anyway unbundle these external storages: #20116

[PVince81]

did some tests with federated shares and all seemed to work fine:

• TEST: incoming share
• TEST: outgoing share (because it sends remote requests)
• TEST: mount public link
• TEST: remove remote share as sharer (notifies counterpart)
• TEST: remove remote share as recipient (notifies counterpart)

[PVince81]

• TEST: GDrive 🚫 cannot set it up Cannot configure GoogleDrive as external Storage #27456 (comment)

[PVince81]

@felixboehm @DeepDiver1975 I think this PR is a good example of what is likely too risky to put in a maintenance release, considering that many market apps might also be using the old Guzzle API.

[PVince81]

Moving to backlog for now.

@SergioBertolinSG spoke about moving the Guzzle stuff into a utility wrapper, in which case we'll only need to adjust the wrapper and not all the tests like I did here.

[Hemant-Mann]

@PVince81 the new dropbox app also requires guzzlehttp/guzzle >= 6.0 due to a dependency in the UnOfficial Dropbox SDK https://github.com/kunalvarma05/dropbox-php-sdk/blob/master/composer.json#L13

[PVince81]

@felixboehm @pmaier1 @DeepDiver1975 looks like updating the Guzzle lib is becoming more pressing, see previous comment... Also see #28063 (comment).

[PVince81]

The updated GDrive SDK also requires Guzzle v6: owncloud/files_external_gdrive#4

[Hemant-Mann]

Hey so I can work with Guzzle 5.3 for the new dropbox app but in that way I'll have to manually manage the dependency of the app by committing vendor (3rd party) files in the repo which I dont think is a good practise

Thoughts @PVince81 @DeepDiver1975 ?

[PVince81]

@Hemant-Mann does this even work with Guzzle 5.3 ? Can you have a quick try ? Note that the APIs are not compatible so if the flysystem adapter has no version compatible with Guzzle 5.3, might need to hack that one too... ugly...

cc @DeepDiver1975

[Hemant-Mann]

@PVince81 The Guzzle dependency is in the SDK not in flysystem
But the SDK allows us to provide our own HTTP Client to send requests to the Dropbox API (Plug and Play type)
So if we can Supply a Guzzle 5.3 client to the SDK everything still works (I have tested my flysystem-adapter)
See this https://github.com/Hemant-Mann/flysystem-dropbox/blob/master/composer.json#L15 my adapter does not require Guzzle as such by the dependency kunalvarma05/dropbox-php-sdk requires Guzzle ^6.0 which is the problem

[PVince81]

Rebased and solved conflicts. I didn't rerun all tests, so let's see if all still passes.

The next step will be to extract the bits that are compatible with Guzzle v5.3 and merge them to master, so we don't need to change these again once switching to Guzzle v6.

[PVince81]

argh... of course... with Jenkins down I won't get test results

[PVince81]

hmm seems it wouldn't be that easy as we don't have GuzzleHttp\Psr7\Request with the old Guzzle, so would need to use yet another approach which works with both versions

[PVince81]

another fix: WebdavHelper used the obsolete setBody().

Let's hope all CI will pass this time.

[PVince81]

superseded by #29211

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.