Allow to decorate the UploaderHelper

[VincentLanglet]

Hi @garak.

I recently tried the 1.19 version and got an issue.

Since the UploaderHelper is/will be final, I cannot extends it.
But since this class is required (and not an interface) in the construct of the UploaderExtension, I cannot use decoration either.

This PR solve this issue.

[garak]

I don't think we'll need that in the next major version.
UploaderHelper is just a legacy class from the time when we supported non-twig templates, probably it will be removed. Its logic will be moved directly into UploaderExtension.

[garak]

I have to correct myself: UploaderHelper can be directly used, indeed.

Anyway, I think that we can delay the move of adding a new interface to the next major version.

[VincentLanglet]

Anyway, I think that we can delay the move of adding a new interface to the next major version.

Why should we delay this ?

Adding the interface in the 1.x version, allows people to solve deprecation/message about the fact the UploaderHelper is final. Currently I have two solutions

• Sticking with 1.18 version
• Updating to 1.19 version and getting static-analysis errors about the fact I extend a @final method.

With the interface I could use decoration, solve my issue and update the library.

It seems better to give a migration path rather than having to solve all the BC break in the next major.

[garak]

The problem is that we would change the signature of the UploaderExtension constructor, and this can break if a user extended it.

[VincentLanglet]

The problem is that we would change the signature of the UploaderExtension constructor, and this can break if a user extended it.

Changing UploaderHelper to UploaderHelperInterface is not a BC-break since UploaderHelper implements UploaderHelperInterface.

Prior to this, if you pass UploaderHelper to the constructor it will still works.
And if you pass CustomUploaderHelper extends UploaderHelper it will still works since it implements the UploaderHelperInterface interface thanks to UploaderHelper.

[garak]

Changing UploaderHelper to UploaderHelperInterface is not a BC-break since UploaderHelper implements UploaderHelperInterface.

That's a good point.
So, I guess you only need to make tests passing.

[VincentLanglet]

Changing UploaderHelper to UploaderHelperInterface is not a BC-break since UploaderHelper implements UploaderHelperInterface.

That's a good point. So, I guess you only need to make tests passing.

Tests are fixed.

[VincentLanglet]

@garak I tried to add some deprecations. Tell me if it's ok for you.

[garak]

This is already deprecated in the helper. If we keep it here too, user will get a duplicate deprecation warning, right?

[VincentLanglet]

Only if you don't use a custom UploaderHelper.

[VincentLanglet]

If you want to avoid duplicate deprecation, I can add a check !$this->uploaderHelper instanceof UploadHelper.
WDYT ?

[VincentLanglet]

@garak I avoid the duplicate deprecations now

[garak]

Thank you @VincentLanglet

[VincentLanglet]

Thank you @VincentLanglet

You're welcome ; do you have time for a release ? :)

[garak]

As soon as we sort up #1273

[VincentLanglet]

Now #1273 is solved, do you have time to create a new release @garak ?

[garak]

Here you are https://github.com/dustin10/VichUploaderBundle/releases/tag/1.20.0