Fixing 3.0 compatibility problem with ResourceInterface::isFresh()

[weaverryan]

Hi guys!

The bundle is not ready for 3.0 yet, because of AsseticResource. Implementing isFresh() on ResourceInterface is deprecated: we need to also implement SelfCheckingResourceInterface.

But this patch clearly has issues: it makes AsseticBundle require symfony/config 2.8 (and worse, beta currently). Is there a way to make the bundle compatible with 2.7 (where SelfCheckingResourceInterface doesn't exist) AND 3.0 (where SelfCheckingResourceInterface is required) at the same time?

Thanks!

[weaverryan]

Ping @Ener-Getick - I've seen you add some very nice 3.0 compat in other places - I wonder if you've seen any other places where there is an example of how to correctly handle this situation. Thanks :)

[GuilhemN]

I don't remember any example but you can create a new @internal class (containing the AsseticRessource functions) which will be extended by AsseticRessource with the right interface (you have to place the class definition in a condition that checks the existence of SelfCheckingResourceInterface).

[stof]

Dropping support for LTS versions is not an option.

A better solution would be to register a checker able to handle AsseticResource (I'm planning to refactor these resources to avoid serializing twig in them anyway)

[weaverryan]

Thanks for the advice! This PR is updated and should be ready to be merged. I used @Ener-Getick's advice to create a sub-class that implements the interface and use that when the interface is available. This can be a short-term solution before properly refactoring the resource checking.

I also tested this locally on a 2.8 project and the deprecation warnings indeed went away.

[GuilhemN]

I was actually more thinking to something like:

<?php

namespace Foo;

/**
 * @internal
 */
class InternalFoo {
    function foo() {
        // ...
    }

    // other functions
}

if(interface_exists('Bar\BarInterface')) {
    class Foo extends InternalFoo implements \Bar\BarInterface { }
} else {
    class Foo extends InternalFoo { }
}

But I don't know if this solution is acceptable... What do you think @stof ?

[stof]

typo here

[weaverryan]

@stof fixed

[weaverryan]

This is ready to be merged. Failure is on php7 and is unrelated.

[xabbuh]

👍

[stof]

Thanks for fixing this bug @weaverryan.

[stof]

I will wait a bit (either this evening or this weekend depending on my time this evening) before making a release, as I would like to check how much time my refactoring idea would require. If it is not quick, I will release this workaround in the meantime.

[weaverryan]

👍 sounds very reasonable - thanks :)

[Restless-ET]

@stof Will this change be tagged soon or should I just track the master branch for now?

[jdemangeon]

👍

[weaverryan]

@stof what are your thoughts now? I'm waiting to record a KnpU episode on upgrading to 3.0 - this is my blocker. Thx :)

[stof]

I wanted to do it this weekend at first, but I ended up not doing anything related to code. Will do this evening

[weaverryan]

Me too ;)

[msvrtan]

Any chance of tagging a new release with this fix? 2.7.1 is still throwing deprecated warnings which my CI doesn't like :)

[Ninir]

@stof Any news? a release would be really appreciable :-)

[Wirone]

I'm also interested, it's probably last thing triggering deprecation errors in our app.

[fabpico]

I'm also waiting for a patch! 👍

[weaverryan]

@stof friendly ping - if you tag, I will buy you 🍨 in Berlin ;)

[kreaton85]

I hope the patch will come out asap!

[Madeyes44]

We are stuck ! A patch would be really appreciated !

[Babacooll]

@stof Same here, last deprecation error too :) ! A patch would also be appreciated !

[x3ro]

👍

[RubenHarms]

@stof, do you have any news about a patch or release tag?

[Oliboy50]

Hope a 2.7.2 or 2.8.0 tag will come soon to be able to use this bundle using a stable release in SF3 ;)

[rvanlaak]

Also hope @stof did like @weaverryan 's 🍨

This results in around 500 deprecation notices in my error mails Monolog sends 👍

[mynamedia]

Is this ready to deploy?

[netmikey]

Let me join the crowd: @stof it's been a while man, would be really nice not to let people hang on here any longer ;-)

[DHager]

I expect there's a large crowd of volunteers for any last-minute tasks that could hurry this along 😀

[mynamedia]

^ this.

[mynamedia]

From looking at thread #251, it seems that using the master branch fixes the issue.

This is also talked about on #401.

I installed the master branch and my deprecation notices went away.

[netmikey]

While technically correct, it's generally agreed on that using a library's master development branch is bad practice. What if they merge a broken commit and you happen to update your site right at that moment? Sure you could rely on a specific commit hash, but let's not go there... ;-)

[Wirone]

You can always force exact commit with

{
    "require": {
        "symfony/assetic-bundle": "dev-master#aa5b4f8"
    }
}

and it will write that commit in composer.lock ensuring that no newer commit will be pulled. It acts like release. So maybe it's a solution while @stof does not respond :-)

[arderyp]

Thanks for the workaround @Wirone, though I'm afraid to use it and forget about it. @stof, are there some additional tasks to be completed, code to be changed, etc before this release can be tagged and published? I know you mentioned a refactoring project above. If there is still work to be done, I think some of us would be happy to take a look. Hope all is well. Thanks.

[clytemnestra]

I have a 2.8 project and want to upgrade it to 3.0, but it uses assetic. My log file is full of this deprecation notice, like hundreds of it. Would this error affect the project or it still works? Since 3.0 removed backward compatibility...

[arderyp]

@clytemnestra, I believe that upgrading to 3.0 would break your application in those places where assetic is used, as the isFresh() and other deprecated functions/methods/classes will be removed in 3.0, meaning there will be no more notices to throw, just errors when your app tries to call a non-existent function/method/class. Someone please correct me if I am wrong, but I believe that's how it will work.

[timwhite]

👍

[clytemnestra]

That's how I believe it will work, too. I think Assetic is the only bundle that's keeping me from upgrading my project to 3.0, that's why I'm not doing it yet.

[rvanlaak]

@clytemnestra you could try requiring dev-master of this bundle to see or that prevents your project from crashing on 3.0