Use scoper to namespace dependencies #237

patrickjahns · 2019-08-13T15:44:07Z

This pullrequest tries to accomplish similar aspects as #142 - but utilizes humbug/scoper https://github.com/humbug/php-scoper

Currently scoping requires a 3 step process:

run php-scoper with the provided configuration file
dumpe the autoloader for the scoped files
fix the composer autoloader for loading static ( scoped files ) scoper-fix-autoloader.php

As this process touches composer and autoloading - the scoping can only happen in the build process of the artifact.

Thus the 3 steps have been included in the Makefile in two targets:

make dist
make dist-qa

For 🤖 - this also means, that now not the source code as it exists in the working space should/needs to be tested. This time we need to install the artifact and then perform our tests on top.

As the unit-tests are normally not included in an artifact - the dist-qa target was thus created.
So bot acceptance and unit tests are performed on top of the scoped files.

The drone.yml has been adjusted to reflect this behaviour - with the migration to drone 1.0 and starlark this probably needs to be migrated to the new format.
For this app ( and any other app that might use scoped dependencies ) it is of key essence, to test the final artifact, as the source code and the vendor files are touched during the scoping process.
For development this should not make any difference.

Important things noticed

AWS SDK
The AWS Library had issue being fully scoped initially, and thus it was needed to write patchers for humbug
- A patcher for reversing the issue of a gmdate string being changed 7c76e63
- A patcher for fixing the exception handling of the AWS SDK - as these are infered by the service name indirectly f2b144a
GUZZLE SDK
In order to correctly use guzzle with scoping - it was better to put the guzzle sdk to the vendor libraries and get them scoped, as all references in other required libraries would have otherwise been scoped or needed to be whitelisted. Additionally this now makes files_primary_s3 app independent from the core guzzle version used - as it has its own scoped version

In the guzzle SDK, there is a file called functions.php - loading these plain files in composer is based on a package+filename. These loaded files are registered globally and only loaded once for every package.
As Scoper does not change the packagename, the functions.php from guzzle wouldn't be loaded - hence it was necessary to write a "fixer" for the dumped composer autoloader, to tell composer that the functions.php referenced in this app is actually a different one compared to the one already loaded from any ohter app/core

Below some of the quirks were described in single posts - and some of the solutions as well

patrickjahns · 2019-08-20T12:09:26Z

Further investigation I run into this bug humbug/php-scoper#298 - meaning that because of having guzzle in core, the functions_include can not be easily scoped.

An unhandled exception has been thrown:
Error: Call to undefined function _PhpScoper1ae18a57724e\React\Promise\resolve() in /Users/patrickjahns/projects/owncloud/source/testing-files-primary-s3/owncloud/apps-external/files_primary_s3/vendor/react/promise/src/Promise.php:184
Stack trace:
#0 /Users/patrickjahns/projects/owncloud/source/testing-files-primary-s3/owncloud/apps-external/files_primary_s3/vendor/react/promise/src/Deferred.php(31): _PhpScoper1ae18a57724e\React\Promise\Promise::_PhpScoper1ae18a57724e\React\Promise\{closure}(Array)

Now I run into:

An unhandled exception has been thrown:
Error: Class 'Aws\S3\Exception\S3Exception' not found in /Users/patrickjahns/projects/owncloud/source/testing-files-primary-s3/owncloud/apps-external/files_primary_s3/vendor/aws/aws-sdk-php/src/WrappedHttpHandler.php:139
Stack trace:
#0 /Users/patrickjahns/projects/owncloud/source/testing-files-primary-s3/owncloud/apps-external/files_primary_s3/vendor/aws/aws-sdk-php/src/WrappedHttpHandler.php(78): _PhpScoper1ae18a57724e\Aws\WrappedHttpHandler->parseError(Array, Object(_PhpScoper1ae18a57724e\GuzzleHttp\Psr7\Request), Object(_PhpScoper1ae18a57724e\Aws\Command), Array)

Let's see why the exception functions are not properly scoped here

Went on manually dealing with this to see how far I would get when ignoring the initial error

patrickjahns · 2019-08-20T12:23:31Z

Figured out the S3Exception part - the magic in https://github.com/aws/aws-sdk-php/blob/master/src/AwsClient.php#L295 causes trouble

It can however be overridden when a client is created via:
https://github.com/aws/aws-sdk-php/blob/master/src/AwsClient.php#L182-L184

lib/s3storage.php

patrickjahns · 2019-08-22T07:46:18Z

Unit test failures where resolved by copying over the tests folder

patrickjahns · 2019-08-22T11:45:33Z

Remaining failures are related to trashbin tests


--- Failed scenarios:
--
541 |  
542 | /drone/server/tests/acceptance/features/apiTrashbin/trashbinRestore.feature:219
543 | /drone/server/tests/acceptance/features/apiTrashbin/trashbinRestore.feature:220
544 | /drone/server/tests/acceptance/features/apiTrashbin/trashbinRestore.feature:225
545 | /drone/server/tests/acceptance/features/apiTrashbin/trashbinRestore.feature:242

see: https://drone.owncloud.com/owncloud/files_primary_s3/1116/27/20

When looking inside the owncloud.log - the errors are


{"reqId":"UO9NnMP7EVcsw6LroqbQ","level":3,"time":"2019-08-22T09:39:09+00:00","remoteAddr":"172.18.0.10","user":"user0","app":"PHP","method":"MOVE","url":"\/remote.php\/dav\/trash-bin\/user0\/2147484862","message":"Error executing \"GetObject\" on \"http:\/\/ceph\/OWNCLOUD\/urn%3Aoid%3A2147484862\"; AWS HTTP error: Client error response [url] http:\/\/ceph\/OWNCLOUD\/urn%3Aoid%3A2147484862 [status code] 404 [reason phrase] Not Found NotFound (client): 404 Not Found (Request-ID: tx000000000000000000a24-005d5e62bd-100c-default) - <?xml version=\"1.0\" encoding=\"UTF-8\"?><Error><Code>NoSuchKey<\/Code><BucketName>OWNCLOUD<\/BucketName><RequestId>tx000000000000000000a24-005d5e62bd-100c-default<\/RequestId><HostId>100c-default-default<\/HostId><\/Error> at \/drone\/server\/apps-external\/files_primary_s3\/lib\/streamwrapper.php#599"}
--
101 | {"reqId":"UO9NnMP7EVcsw6LroqbQ","level":3,"time":"2019-08-22T09:39:09+00:00","remoteAddr":"172.18.0.10","user":"user0","app":"PHP","method":"MOVE","url":"\/remote.php\/dav\/trash-bin\/user0\/2147484862","message":"fopen(s3:\/\/OWNCLOUD\/urn:oid:2147484862): failed to open stream: &quot;OCA\\Files_Primary_S3\\StreamWrapper::stream_open&quot; call failed at \/drone\/server\/apps-external\/files_primary_s3\/lib\/s3storage.php#146"}
102 | {"reqId":"UO9NnMP7EVcsw6LroqbQ","level":3,"time":"2019-08-22T09:39:09+00:00","remoteAddr":"172.18.0.10","user":"user0","app":"PHP","method":"MOVE","url":"\/remote.php\/dav\/trash-bin\/user0\/2147484862","message":"fread() expects parameter 1 to be resource, boolean given at \/drone\/server\/lib\/private\/Files\/Storage\/Wrapper\/Encryption.php#893"}
103 | {"reqId":"UO9NnMP7EVcsw6LroqbQ","level":3,"time":"2019-08-22T09:39:09+00:00","remoteAddr":"172.18.0.10","user":"user0","app":"PHP","method":"MOVE","url":"\/remote.php\/dav\/trash-bin\/user0\/2147484862","message":"fclose() expects parameter 1 to be resource, boolean given at \/drone\/server\/lib\/private\/Files\/Storage\/Wrapper\/Encryption.php#894"}
104 | {"reqId":"UO9NnMP7EVcsw6LroqbQ","level":3,"time":"2019-08-22T09:39:09+00:00","remoteAddr":"172.18.0.10","user":"user0","app":"PHP","method":"MOVE","url":"\/remote.php\/dav\/trash-bin\/user0\/2147484862","message":"Error executing \"GetObject\" on \"http:\/\/ceph\/OWNCLOUD\/urn%3Aoid%3A2147484862\"; AWS HTTP error: Client error response [url] http:\/\/ceph\/OWNCLOUD\/urn%3Aoid%3A2147484862 [status code] 404 [reason phrase] Not Found NotFound (client): 404 Not Found (Request-ID: tx000000000000000000a25-005d5e62bd-100c-default) - <?xml version=\"1.0\" encoding=\"UTF-8\"?><Error><Code>NoSuchKey<\/Code><BucketName>OWNCLOUD<\/BucketName><RequestId>tx000000000000000000a25-005d5e62bd-100c-default<\/RequestId><HostId>100c-default-default<\/HostId><\/Error> at \/drone\/server\/apps-external\/files_primary_s3\/lib\/streamwrapper.php#599"}
105 | {"reqId":"UO9NnMP7EVcsw6LroqbQ","level":3,"time":"2019-08-22T09:39:09+00:00","remoteAddr":"172.18.0.10","user":"user0","app":"PHP","method":"MOVE","url":"\/remote.php\/dav\/trash-bin\/user0\/2147484862","message":"fopen(s3:\/\/OWNCLOUD\/urn:oid:2147484862): failed to open stream: &quot;OCA\\Files_Primary_S3\\StreamWrapper::stream_open&quot; call failed at \/drone\/server\/apps-external\/files_primary_s3\/lib\/s3storage.php#146"}
106 | {"reqId":"UO9NnMP7EVcsw6LroqbQ","level":3,"time":"2019-08-22T09:39:09+00:00","remoteAddr":"172.18.0.10","user":"user0","app":"PHP","method":"MOVE","url":"\/remote.php\/dav\/trash-bin\/user0\/2147484862","message":"fclose() expects parameter 1 to be resource, boolean given at \/drone\/server\/lib\/private\/Files\/Storage\/Common.php#584"}

Not sure why the object storage returns a 404 here - this needs investigation

individual-it · 2019-08-27T10:59:50Z

the reason is that API tests are run instead of webUI tests see https://github.com/owncloud/files_primary_s3/pull/237/files#diff-3216dfff0ed3e301453e6799e8c367e2R268

the detailed reason:
the failing tests are marked @skip_on_objectstore
but that is only set for API tests, not webUI see

files_primary_s3/.drone.yml

Line 234 in b9b5466

- OC_TEST_ON_OBJECTSTORE=1

in combination with
https://github.com/owncloud/core/blob/9505bffa2729a0232d89d7e03ef72f6b1dd41742/tests/acceptance/run.sh#L880
so running API tests in the webUI drone section messes it all up

patrickjahns · 2019-08-29T09:50:15Z

The failures on scality fail on tests that are tagged @skipOnStorage:ceph - so I assume that they would be green the other way and its not just a ceph issue - but a general objectstorage issue

Signed-off-by: Patrick Jahns <[email protected]> allow the usage of dev dependencies in order to install latest scoper

…asses

The autoloader was correctly fixed by php-scoper - however doing the recommended dump after scoper was used, the fixes for the autoloader where no longer present. The logic here has been moved into a seperate script now

Signed-off-by: Patrick Jahns <[email protected]>

…that can be installed for unit testing Signed-off-by: Patrick Jahns <[email protected]>

ianmjones

Hey @patrickjahns, thank you very much for your scoper-fix-autoloader.php file, I've "borrowed" it to fix the clashing hashes when embedding the SDKs that use the composer autoloader in a WordPress plugin.

I did notice that it prefixed all but the last hash key in the processed array, so here's a slight tweak that fixes that.

Thanks again, very much appreciated!

ianmjones · 2019-11-26T14:49:00Z

scoper-fix-autoloader.php

+$static_loader_path = $scoper_path.'/autoload_static.php';
+echo "Fixing $static_loader_path \n";
+$static_loader = file_get_contents($static_loader_path);
+$static_loader = \preg_replace('/\'([A-Za-z0-9]*?)\' => __DIR__ \. (.*?),/', '\'a$1\' => __DIR__ . $2,', $static_loader);


This doesn't catch the last element in the array that is followed by ) rather than ,, so should be like the following instead as each element ends with .php'...

$static_loader = \preg_replace('/\'([A-Za-z0-9]*?)\' => __DIR__ \. (.*?\.php\')/', '\'a$1\' => __DIR__ . $2', $static_loader);

ianmjones · 2019-11-26T14:51:02Z

scoper-fix-autoloader.php

+$files_loader_path = $scoper_path.'/autoload_files.php';
+echo "Fixing $files_loader_path \n";
+$files_loader = file_get_contents($files_loader_path);
+$files_loader = \preg_replace('/\'(.*?)\' => (.*?),/', '\'a$1\' => $2,', $files_loader);


This doesn't catch the last element in the array that is followed by ) rather than ,, so should be like the following instead as each element ends with .php'...

$files_loader = \preg_replace('/\'(.*?)\' => (.*?\.php\')/', '\'a$1\' => $2', $files_loader);

patrickjahns force-pushed the feature-scoped-vendor branch 2 times, most recently from 97db16f to 76abe84 Compare August 20, 2019 17:00

patrickjahns commented Aug 20, 2019

View reviewed changes

lib/s3storage.php Outdated Show resolved Hide resolved

patrickjahns force-pushed the feature-scoped-vendor branch 11 times, most recently from a035c3d to d0744a8 Compare August 20, 2019 19:34

patrickjahns force-pushed the feature-scoped-vendor branch from bcac537 to b9b5466 Compare August 22, 2019 08:48

patrickjahns force-pushed the feature-scoped-vendor branch 2 times, most recently from ebc2ed4 to 1052182 Compare August 27, 2019 15:15

patrickjahns added 7 commits August 29, 2019 14:24

prepare scoper

f0b589a

Signed-off-by: Patrick Jahns <[email protected]> allow the usage of dev dependencies in order to install latest scoper

do not prefix the app namespace

74ebbff

patch the wrongly scoped gmdate pattern in aws sdk

7c76e63

patch AWS exception reflection to properly expose scoped excaption cl…

f2b144a

…asses

allow loading files by prefixing the autoloader string

b9bd209

refactor autoloader fixing

08b0d22

The autoloader was correctly fixed by php-scoper - however doing the recommended dump after scoper was used, the fixes for the autoloader where no longer present. The logic here has been moved into a seperate script now

install built app

f491139

Signed-off-by: Patrick Jahns <[email protected]>

patrickjahns force-pushed the feature-scoped-vendor branch from c1965ae to 32dd3e0 Compare August 29, 2019 13:37

patrickjahns added 9 commits August 29, 2019 15:45

change workspace to get app loaded

ceebf26

Signed-off-by: Patrick Jahns <[email protected]>

allow overriding the test-runner path in the app

7c8eb7a

Signed-off-by: Patrick Jahns <[email protected]>

restructured app building and installing

77d19f5

fix path issue

13724b5

copy over configuration after app was installed

9a0d80a

run all acceptance tests again

098a56e

fix code style

02ca60e

run unit tests again

432ee31

introduce dist-qa target to have a tarball that contains test folder …

c055ad7

…that can be installed for unit testing Signed-off-by: Patrick Jahns <[email protected]>

patrickjahns force-pushed the feature-scoped-vendor branch from 32dd3e0 to c055ad7 Compare August 29, 2019 13:46

patrickjahns self-assigned this Aug 29, 2019

patrickjahns marked this pull request as ready for review August 29, 2019 14:11

patrickjahns requested review from phil-davis, micbar, PVince81 and DeepDiver1975 August 29, 2019 14:11

ianmjones reviewed Nov 26, 2019

View reviewed changes

patrickjahns removed their assignment Mar 3, 2020

patrickjahns closed this Apr 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use scoper to namespace dependencies #237

Use scoper to namespace dependencies #237

patrickjahns commented Aug 13, 2019 •

edited

Loading

patrickjahns commented Aug 20, 2019

patrickjahns commented Aug 20, 2019

patrickjahns commented Aug 22, 2019

patrickjahns commented Aug 22, 2019

individual-it commented Aug 27, 2019

patrickjahns commented Aug 29, 2019

ianmjones left a comment

ianmjones Nov 26, 2019

ianmjones Nov 26, 2019

Use scoper to namespace dependencies #237

Use scoper to namespace dependencies #237

Conversation

patrickjahns commented Aug 13, 2019 • edited Loading

Important things noticed

patrickjahns commented Aug 20, 2019

patrickjahns commented Aug 20, 2019

patrickjahns commented Aug 22, 2019

patrickjahns commented Aug 22, 2019

individual-it commented Aug 27, 2019

patrickjahns commented Aug 29, 2019

ianmjones left a comment

Choose a reason for hiding this comment

ianmjones Nov 26, 2019

Choose a reason for hiding this comment

ianmjones Nov 26, 2019

Choose a reason for hiding this comment

patrickjahns commented Aug 13, 2019 •

edited

Loading