Added the support of the CacheBustingWorker

[stof]

This adds the configuration for kriswallsmith/assetic#297

I'm wondering if the worker should be enabled by default or no.

[fran6co]

+1 for enabled by default. I think it adheres to the principle of least surprise better than the current behaviour, things just work in a cache aggressive environment. For other cases it works just the same.

[datiecher]

+1 for going with CacheBusting by default. Front end developers will love it!

[arnaud-lb]

Does it affects development too ? (when assets aren't dumped on disk). If the filename changes with content, that may break javascript debuggers.

[stof]

@arnaud-lb I haven't checked the javascript debuggers, but I have checked that it does not break the routing when using the controller.

and if you don't want it in development, you would still be able to disable it in config_dev.yml :)

[shieldo]

+1 for being on by default - agree it's the path of least surprise, and is so beneficial I think it can't not be default.

[craue]

I'm having several issues after enabling it which didn't occur before. Though, I'm not sure if I'm doing something wrong here.

With an empty cache (not warmed up), I'm getting
Class 'EM50544f9001781_546a8d27f194334ee012bfe64f629947b07e4919\\__CG__\\Doctrine\\ORM\\EntityManager' not found in app\\cache\\prod\\appProdProjectContainer.php on line 1151.

For prod env, even with a warmed-up cache and dumped assets, assets seem to be re-dumped when accessing their URLs.
For dev env, even with a warmed-up cache and dumped assets, asset URLs just result in HTTP 500s with Uncaught exception 'Symfony\\Component\\Routing\\Exception\\ResourceNotFoundException' in app\\cache\\prod\\appprodUrlMatcher.php:3709.

In the end, my assets didn't work at all and I reverted to the old behavior.

Is there a known trap I just fell into? If not, I'd rather keep this feature disabled by default until it's working reliably.
...Or just enable it and see if there'll be some more complaints. ;)

[stof]

@craue For the prod environment, assets are never dumped on the fly. they should not even be served by a controller. So I don't understand how you could have them being re-dumped when accessing the file.

and for the other issue, it has nothing to do with AsseticBundle. It looks like a proxy generated by JMSAopBundle

[craue]

@stof: Indeed. According to my config, they aren't served by a controller. But I noticed Java being run to invoke the YUI Compressor. I admit it's pretty strange...

As long as I could even explicitly disable this feature, I'm also fine with enabling it by default.

[tristanbes]

+1 For enabling this by default
+1 for merging it ASAP ;-)

[liuggio]

I like this feature, and we need it.
If you enable by default will you brake backward comp?

[stof]

@liuggio it would simply mean you have to run assetic:dump again.

[liuggio]

We currently use different web servers and only one cdn bucket. If we run assetic:dump on each server the same file will have different name? How could we keep the same file with the same name on all servers? Am I missing something? Thanks.

[stof]

@liuggio when using the modification strategy, you would end with different names indeed. But the content strategy will generate the same hash on all servers (assuming all servers have the same content for their assets). this is exactly the goal of the content strategy

[liuggio]

thanks a lot for the explanation, 👍 without doubt

[acasademont]

works nicely! +1 for merging this :)

[datiecher]

Tried the content strategy today and it works really nice. I'm in favor of enabling this feature and using the content strategy by default as it will work even on scenarios like @liuggio described, so it should be a safe bet.

[tristanbes]

Why this PR isn't merged yet ? :'( ping @stof

[leek]

Been using this for a few days - anyone else noticing that using the content strategy causes assetic:dump to take almost 5x longer? /cc @stof

[stof]

Using the content strategy has indeed a performance impact as the worker is doing a dump to be able to have the content to create the name.

[acasademont]

Maybe we could improve the performance of the whole process if instead of dumping the asset in the hash_update($hash, $asset->dump() function we did a hash_update_file($hash, $asset->getSourceRoot()).

What do you think?

[datiecher]

@acasademont I do think it's worth trying, nice suggestion.

[stof]

@acasademont This would not work:

• $asset->getSourceRoot() is not the path to the file
• assets may not be local files (there is a built-in HttpAsset and you could implement the AssetInterface to load them from other places too)
• filters may import other files and a change in one of them should change the hash too (and without doing the dump, we cannot have this information currently, see the discussion in Refactored Asset dependency check kriswallsmith/assetic#259 regarding the modified date)

[acasademont]

True, I currently have some .less files that import other .less files. A change in one of the imported l.ess files would not change the content of the main .less file, thus the hash would remain the same. Forget about it!

[acasademont]

Still not sure about the PR?

[stof]

@acasademont given the performance impact of the content strategy, I'm not sure at all it should be used by default.

[alexcesaro]

I think the "enabled" node in the configuration should be renamed "disabled" and the default value be %kernel.debug%. Cache busting would be enabled in production mode because that is what you usually want in a production environment and you often do not care about the performance impact since it is only used when deploying. And it would be disabled in development mode since the performance matters and this feature is useless in that environment.

[fran6co]

There is a bug when calling assetic:dump with CacheBustingWorker enabled and assets with vars. kriswallsmith/assetic#312

[acasademont]

Besides the bug, I have been testing the performance of assetic with the cache busting enabled and here are my results. The test case consists of one .less file wich imports some other less files in turn, some javascripts and a .sass file compiled with compass. In total there are 240KB of less, 196KB of sass and 560KB of javascript

No busting: 6.9s avg
Modification date strategy: 7.0s avg
Content strategy: 9.1s avg

So there is a performance penaly when using the content strategy, a penalty that is dependant on the files size. But as you can see, the penalty is very acceptable, just 32% slower. @leek could you please tell us your results? I didn't get even close to 2x penalty, so 5x seems a lot to me.

However, I am -1 on enabling this by default, it is an extra feature that maybe not everybody wants/needs and although the penalty is not terrible, ther is some. The PR as is now does not enable the busting feature by default, fine with that!

[liuggio]

Who cares about the speed, it happens only in production when you deploy, is not a runtime delay, isn't?

[kriswallsmith]

The worker no longer has multiple strategies. Can someone update?

[estahn]

👍

[aur1mas]

@stof, @kriswallsmith Is anything happening on this problem? If no - what are suggested ways for cache busting?

[rincedd]

Any news on whether this might be merged soon? 👍

[kingcrunch]

@stof Are you still on this? (No offense, you are just the one who opened this PR)

[nurikabe]

+1

[sandermarechal]

@stof @kriswallsmith Any idea if/when this will be merged?

[tomiford]

+1

[wwotoole]

[rison]

+1

[kyaw-myo-thein]

+1 Without automatic cache busting, it's quite painful to manually clear cache in rapid deployment approach.

[javiercejudo]

👍

[kedrap]

👍

[jdeniau]

👍 , but there is no need for strategy anymore since kriswallsmith/assetic@8b43be9

Is there a "clean" way to do a similar thing the time this PR is merged ?

[jdeniau]

Just to answer my last question, I found the answer : http://stackoverflow.com/questions/14399288/how-to-use-assetic-with-cachebustingworker-and-twig/19495577#19495577

[sandermarechal]

In that case, should this PR be reworked so that it supports the new way to add the CacheBustingWorker? It would be nice to be able to set it from config.yml and maybe even enable it by default.

[kingcrunch]

Would prefer to not enable it by default (actually just because of BC), but a simple

cache_buster: true

would be nice 😄

[sandermarechal]

I have forked and updated this PR in #240. Let's hope it makes it in this time!

[Ninir]

@stof @kriswallsmith Any news on this one please?

[merk]

fwiw I got this working with a simple service definition

services:
    assetic.worker.cache_busting:
        class: Assetic\Factory\Worker\CacheBustingWorker
        public: false
        tags:
            - { name: assetic.factory_worker }

[Ninir]

@merk Works like a charm Tim, thanks :)

[stof]

Closing in favor of the updated PR

[kingcrunch]

IMO thats worth two different tests

• They test two different cases, not just different values for the same case, or similar cases.
• You'll probably never add more values, to cover other cases, because there only two boolean values.

[stof]

@kingcrunch After the merge of the other PR and 6ac666e tweaking it, the shortest config is

assetic:
    workers:
        cache_busting: ~