Added support for 'ReplacingFile' to 'upload' files not uploaded via HTTP

[UlrichThomasGabor]

This fixes #1296.

I removed most of the dependencies of files in /src/Storage on UploadedFile because it makes no sense in my opinion.

Going beyond this PR I even think that this should be replaced with a copy call, because all other concrete implementations of Storage indeed only copy and do not remove the original. In practice this might not be relevant though.

The new class ReplacingFile provides a method getClientOriginalName to be somewhat compatible with UploadedFile. I changed all other occurrences of UploadedFile to alternatively expect an ReplacingFile. If language level would be PHP 8.1., this could be a union type in all occurrences, but I guess this is not possible at the moment.

I just used it this way, and it works. I did not run the test-suite though (and did not write a test), because "make tests" just throws an error:

make tests
make vichuploader-image
docker build -t dustin10/vichuploader_php74 -f ./docker/Dockerfile74 .
make[1]: docker: No such file or directory
make[1]: *** [vichuploader-image] Error 1
make: *** [tests] Error 2

[UlrichThomasGabor]

I added one test-case. There should be a functional one too I guess (something simulating a Command/Controller/Service creating a DummyEntity and inserting a ReplacingFile), I am not sure how to do that best. Maybe you can provide some insight.

It's also still unclear to me what steps to perform to actually run the tests? Like I said in my opening post, I only receive an error message when I try to run the tests.

[garak]

For testing, installing docker should be enough

[UlrichThomasGabor]

Ah my docker installation was broken. Fixed it, but still no luck:

make tests
make vichuploader-image
docker build -t dustin10/vichuploader_php74 -f ./docker/Dockerfile74 .
[+] Building 474.6s (11/11) FINISHED                                                                                                                
 => [internal] load build definition from Dockerfile74                                                                                         0.1s
 => => transferring dockerfile: 964B                                                                                                           0.0s
 => [internal] load .dockerignore                                                                                                              0.0s
 => => transferring context: 2B                                                                                                                0.0s
 => [internal] load metadata for docker.io/library/php:7.4-alpine                                                                              2.8s
 => [1/7] FROM docker.io/library/php:7.4-alpine@sha256:bfa39ec02c24e3b3b6260b0c69e40f6d6a08bf156515e25f62d6b099e0fc567d                        9.4s
 => => resolve docker.io/library/php:7.4-alpine@sha256:bfa39ec02c24e3b3b6260b0c69e40f6d6a08bf156515e25f62d6b099e0fc567d                        0.0s
 => => sha256:530afca65e2ea04227630ae746e0c85b2bd1a179379cbf2b6501b49c4cab2ccc 2.80MB / 2.80MB                                                 0.4s
 => => sha256:09cfb8455962e8dee2ec89d1aca6167797ee8fb124669b4f37e54aa82accbd11 1.71MB / 1.71MB                                                 0.6s
 => => sha256:bfa39ec02c24e3b3b6260b0c69e40f6d6a08bf156515e25f62d6b099e0fc567d 1.65kB / 1.65kB                                                 0.0s
 => => sha256:7027dc4933fbab58053bb54f6c47b082536438bf286f84de01ce56bf6d3a2745 2.20kB / 2.20kB                                                 0.0s
 => => sha256:8067d339ef450d9529042bed23a0dff00a3870f494393ad8bf46fd938189c4ab 8.92kB / 8.92kB                                                 0.0s
 => => sha256:b72bc08afeabf24400f8800b72194e328506b82a27cefb34b8b9f573393b8bc2 1.26kB / 1.26kB                                                 0.5s
 => => extracting sha256:530afca65e2ea04227630ae746e0c85b2bd1a179379cbf2b6501b49c4cab2ccc                                                      1.3s
 => => sha256:7fb7a732550aeef70ff8ae6f2fdc1f988da20f7adbd4fa74770ad6e4f15be150 268B / 268B                                                     0.6s
 => => sha256:45484466a2a50b56b0c5b88f3cf0605d4247fac17de1b136c4d98779fea574b2 10.44MB / 10.44MB                                               2.2s
 => => sha256:104dedd8731096601de57cba82168617d4cc7fb16c94c63f9d051641ba43f3c9 497B / 497B                                                     0.9s
 => => sha256:b7c9161ecde185086e1611e16fb002d52966932ac9ef9ee5fe0d6fed4a1cf4da 15.07MB / 15.07MB                                               3.2s
 => => sha256:4b863eddc75ba099989bb942c79064d98c8dc86012bbb32f392a8bd8977f3a0b 2.45kB / 2.45kB                                                 1.3s
 => => sha256:c94bd3dd2a3168323021c913c688038d3d861956dc21d61c4b2b00b5335d4e26 18.37kB / 18.37kB                                               1.6s
 => => extracting sha256:09cfb8455962e8dee2ec89d1aca6167797ee8fb124669b4f37e54aa82accbd11                                                      1.3s
 => => extracting sha256:b72bc08afeabf24400f8800b72194e328506b82a27cefb34b8b9f573393b8bc2                                                      0.0s
 => => extracting sha256:7fb7a732550aeef70ff8ae6f2fdc1f988da20f7adbd4fa74770ad6e4f15be150                                                      0.0s
 => => extracting sha256:45484466a2a50b56b0c5b88f3cf0605d4247fac17de1b136c4d98779fea574b2                                                      0.4s
 => => extracting sha256:104dedd8731096601de57cba82168617d4cc7fb16c94c63f9d051641ba43f3c9                                                      0.0s
 => => extracting sha256:b7c9161ecde185086e1611e16fb002d52966932ac9ef9ee5fe0d6fed4a1cf4da                                                      2.1s
 => => extracting sha256:4b863eddc75ba099989bb942c79064d98c8dc86012bbb32f392a8bd8977f3a0b                                                      0.0s
 => => extracting sha256:c94bd3dd2a3168323021c913c688038d3d861956dc21d61c4b2b00b5335d4e26                                                      0.0s
 => [internal] load build context                                                                                                              7.0s
 => => transferring context: 36.19MB                                                                                                           6.9s
 => [2/7] RUN set -eux;  apk add --no-cache --virtual .build-deps   autoconf   dpkg-dev dpkg   file   g++   gcc   libc-dev   make   pkgconf  456.6s
 => [3/7] RUN curl -s https://getcomposer.org/installer | php                                                                                  1.2s 
 => [4/7] RUN mv composer.phar /usr/local/bin/composer                                                                                         0.7s 
 => [5/7] WORKDIR /srv/vich-uploader                                                                                                           0.1s 
 => [6/7] COPY . ./                                                                                                                            2.7s 
 => ERROR [7/7] RUN set -eux;  composer install --prefer-dist -v;  composer clear-cache                                                        0.8s 
------                                                                                                                                              
 > [7/7] RUN set -eux;  composer install --prefer-dist -v;      composer clear-cache:                                                               
#11 0.439 + composer install --prefer-dist -v
#11 0.604 Do not run Composer as root/super user! See https://getcomposer.org/root for details
#11 0.746 Installing dependencies from lock file (including require-dev)
#11 0.752 Verifying lock file contents can be installed on current platform.
#11 0.787 Dependency resolution completed in 0.000 seconds
#11 0.791 Your lock file does not contain a compatible set of packages. Please run composer update.
#11 0.791 
#11 0.791   Problem 1
#11 0.791     - psr/log is locked to version 3.0.0 and an update of this package was not requested.
#11 0.791     - psr/log 3.0.0 requires php >=8.0.0 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 2
#11 0.791     - symfony/config is locked to version v6.1.0 and an update of this package was not requested.
#11 0.791     - symfony/config v6.1.0 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 3
#11 0.791     - symfony/console is locked to version v6.1.2 and an update of this package was not requested.
#11 0.791     - symfony/console v6.1.2 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 4
#11 0.791     - symfony/dependency-injection is locked to version v6.1.2 and an update of this package was not requested.
#11 0.791     - symfony/dependency-injection v6.1.2 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 5
#11 0.791     - symfony/deprecation-contracts is locked to version v3.1.1 and an update of this package was not requested.
#11 0.791     - symfony/deprecation-contracts v3.1.1 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 6
#11 0.791     - symfony/error-handler is locked to version v6.1.0 and an update of this package was not requested.
#11 0.791     - symfony/error-handler v6.1.0 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 7
#11 0.791     - symfony/event-dispatcher is locked to version v6.1.0 and an update of this package was not requested.
#11 0.791     - symfony/event-dispatcher v6.1.0 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 8
#11 0.791     - symfony/event-dispatcher-contracts is locked to version v3.1.1 and an update of this package was not requested.
#11 0.791     - symfony/event-dispatcher-contracts v3.1.1 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 9
#11 0.791     - symfony/filesystem is locked to version v6.1.0 and an update of this package was not requested.
#11 0.791     - symfony/filesystem v6.1.0 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 10
#11 0.791     - symfony/http-foundation is locked to version v6.1.2 and an update of this package was not requested.
#11 0.791     - symfony/http-foundation v6.1.2 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 11
#11 0.791     - symfony/http-kernel is locked to version v6.1.2 and an update of this package was not requested.
#11 0.791     - symfony/http-kernel v6.1.2 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 12
#11 0.791     - symfony/mime is locked to version v6.1.1 and an update of this package was not requested.
#11 0.791     - symfony/mime v6.1.1 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 13
#11 0.791     - symfony/property-access is locked to version v6.1.0 and an update of this package was not requested.
#11 0.791     - symfony/property-access v6.1.0 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 14
#11 0.791     - symfony/property-info is locked to version v6.1.2 and an update of this package was not requested.
#11 0.791     - symfony/property-info v6.1.2 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 15
#11 0.791     - symfony/service-contracts is locked to version v3.1.1 and an update of this package was not requested.
#11 0.791     - symfony/service-contracts v3.1.1 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 16
#11 0.791     - symfony/string is locked to version v6.1.2 and an update of this package was not requested.
#11 0.791     - symfony/string v6.1.2 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 17
#11 0.791     - symfony/var-dumper is locked to version v6.1.0 and an update of this package was not requested.
#11 0.791     - symfony/var-dumper v6.1.0 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 18
#11 0.791     - mongodb/mongodb is locked to version 1.13.0 and an update of this package was not requested.
#11 0.791     - mongodb/mongodb 1.13.0 requires ext-mongodb ^1.14.0 -> it has the wrong version installed (1.8.2).
#11 0.791   Problem 19
#11 0.791     - psr/cache is locked to version 3.0.0 and an update of this package was not requested.
#11 0.791     - psr/cache 3.0.0 requires php >=8.0.0 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 20
#11 0.791     - symfony/asset is locked to version v6.1.0 and an update of this package was not requested.
#11 0.791     - symfony/asset v6.1.0 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 21
#11 0.791     - symfony/browser-kit is locked to version v6.1.0 and an update of this package was not requested.
#11 0.791     - symfony/browser-kit v6.1.0 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 22
#11 0.791     - symfony/cache is locked to version v6.1.2 and an update of this package was not requested.
#11 0.791     - symfony/cache v6.1.2 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 23
#11 0.791     - symfony/cache-contracts is locked to version v3.1.1 and an update of this package was not requested.
#11 0.791     - symfony/cache-contracts v3.1.1 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 24
#11 0.791     - symfony/css-selector is locked to version v6.1.0 and an update of this package was not requested.
#11 0.791     - symfony/css-selector v6.1.0 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 25
#11 0.791     - symfony/doctrine-bridge is locked to version v6.1.2 and an update of this package was not requested.
#11 0.791     - symfony/doctrine-bridge v6.1.2 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 26
#11 0.791     - symfony/dom-crawler is locked to version v6.1.0 and an update of this package was not requested.
#11 0.791     - symfony/dom-crawler v6.1.0 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 27
#11 0.791     - symfony/finder is locked to version v6.1.0 and an update of this package was not requested.
#11 0.791     - symfony/finder v6.1.0 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 28
#11 0.791     - symfony/form is locked to version v6.1.2 and an update of this package was not requested.
#11 0.791     - symfony/form v6.1.2 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 29
#11 0.791     - symfony/framework-bundle is locked to version v6.1.2 and an update of this package was not requested.
#11 0.791     - symfony/framework-bundle v6.1.2 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 30
#11 0.791     - symfony/options-resolver is locked to version v6.1.0 and an update of this package was not requested.
#11 0.791     - symfony/options-resolver v6.1.0 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 31
#11 0.791     - symfony/password-hasher is locked to version v6.1.0 and an update of this package was not requested.
#11 0.791     - symfony/password-hasher v6.1.0 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 32
#11 0.791     - symfony/routing is locked to version v6.1.1 and an update of this package was not requested.
#11 0.791     - symfony/routing v6.1.1 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 33
#11 0.791     - symfony/security-core is locked to version v6.1.2 and an update of this package was not requested.
#11 0.791     - symfony/security-core v6.1.2 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 34
#11 0.791     - symfony/security-csrf is locked to version v6.1.0 and an update of this package was not requested.
#11 0.791     - symfony/security-csrf v6.1.0 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 35
#11 0.791     - symfony/translation is locked to version v6.1.0 and an update of this package was not requested.
#11 0.791     - symfony/translation v6.1.0 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 36
#11 0.791     - symfony/translation-contracts is locked to version v3.1.1 and an update of this package was not requested.
#11 0.791     - symfony/translation-contracts v3.1.1 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 37
#11 0.791     - symfony/twig-bridge is locked to version v6.1.2 and an update of this package was not requested.
#11 0.791     - symfony/twig-bridge v6.1.2 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 38
#11 0.791     - symfony/twig-bundle is locked to version v6.1.1 and an update of this package was not requested.
#11 0.791     - symfony/twig-bundle v6.1.1 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 39
#11 0.791     - symfony/validator is locked to version v6.1.1 and an update of this package was not requested.
#11 0.791     - symfony/validator v6.1.1 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 40
#11 0.791     - symfony/var-exporter is locked to version v6.1.1 and an update of this package was not requested.
#11 0.791     - symfony/var-exporter v6.1.1 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 41
#11 0.791     - symfony/yaml is locked to version v6.1.2 and an update of this package was not requested.
#11 0.791     - symfony/yaml v6.1.2 requires php >=8.1 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791   Problem 42
#11 0.791     - psr/log 3.0.0 requires php >=8.0.0 -> your php version (7.4.30) does not satisfy that requirement.
#11 0.791     - doctrine/dbal 3.3.7 requires psr/log ^1|^2|^3 -> satisfiable by psr/log[3.0.0].
#11 0.791     - doctrine/dbal is locked to version 3.3.7 and an update of this package was not requested.
#11 0.791 
#11 0.791 To enable extensions, verify that they are enabled in your .ini files:
#11 0.791     - 
#11 0.791     - /usr/local/etc/php/conf.d/docker-php-ext-mongodb.ini
#11 0.791     - /usr/local/etc/php/conf.d/docker-php-ext-sodium.ini
#11 0.791 You can also run `php --ini` in a terminal to see which files are used by PHP in CLI mode.
#11 0.791 Alternatively, you can run Composer with `--ignore-platform-req=ext-mongodb` to temporarily ignore these required extensions.
------
executor failed running [/bin/sh -c set -eux;   composer install --prefer-dist -v;  composer clear-cache]: exit code: 2
make[1]: *** [vichuploader-image] Error 1
make: *** [tests] Error 2

[garak]

Try removing your composer.lock file before running. Probably we should add a step to our Makefile to do that.

[UlrichThomasGabor]

Yes, this works.

I've taken a look again at the test, but I do not really see how to perform useful tests in the current test setup. The test I added already fails, because copy does not have a file it can copy and I do not see a how to use a fixture in that test case and there is no pretty way to mock a global function.

Secondly, the more relevant test is still missing: A use-case, pushing a ReplacingFile into an entity, persisting it, and observe if it's done correctly.

I've tried to understand the existing tests for quite a while now, but I don't see where and how to add both tests elegantly. I've added VichUploaderBundleTest::testReplacingFileIsCorrectlyUploaded which passes, but actually tests only a subset of what should be tested.

[UlrichThomasGabor]

The static analysis must have failed before. I did not structurally changed the if clauses and my editor complains about the same code position on master too. I corrected them now anyway.

I asked two times how to implement the tests, but did not get any help. I can remove the failing test, if this is your wish.

[garak]

The static analysis must have failed before.

It didn't. The error is clearly introduced by your changes.

[UlrichThomasGabor]

My editor reports a problem on line 36 of AbstractStorage.php on master already.

[garak]

[UlrichThomasGabor]

The return type of the method differs from the return type given in the PhpDoc, that was the case before my changes already too: https://github.com/dustin10/VichUploaderBundle/blob/23ee93cd3b84afaa20ce69ba8399a9098bdb07f6/src/Mapping/PropertyMapping.php#L75-L79

Fixed it anyway.

[garak]

The return type of the method differs from the return type given in the PhpDoc, that was the case before my changes already too:

https://github.com/dustin10/VichUploaderBundle/blob/23ee93cd3b84afaa20ce69ba8399a9098bdb07f6/src/Mapping/PropertyMapping.php#L75-L79

Fixed it anyway.

I don't understand why you're trying to "fix" something that is not broken.

[UlrichThomasGabor]

phpstan is happy now, rest is here:

I've taken a look again at the test, but I do not really see how to perform useful tests in the current test setup. The test I added already fails, because copy does not have a file it can copy and I do not see a how to use a fixture in that test case and there is no pretty way to mock a global function.

Secondly, the more relevant test is still missing: A use-case, pushing a ReplacingFile into an entity, persisting it, and observe if it's done correctly.

I've tried to understand the existing tests for quite a while now, but I don't see where and how to add both tests elegantly. I've added VichUploaderBundleTest::testReplacingFileIsCorrectlyUploaded which passes, but actually tests only a subset of what should be tested.

Please tell me how you want to proceed.

[garak]

You can take a look to https://github.com/dustin10/VichUploaderBundle/blob/master/tests/Storage/StorageTestCase.php to see how to deal with filesystem in tests

[UlrichThomasGabor]

I fixed the second test. Please take a look.

[garak]

We're almost there! You only need to add a documentation page

[UlrichThomasGabor]

You want 8.1 readonly and change the pipeline or should I remove it?

[garak]

You want 8.1 readonly and change the pipeline or should I remove it?

Let's stay on the safe side and keep the docs back-compatible.

[UlrichThomasGabor]

Is this still needing something from me here?