There are 2 different issues connected to advagg operations with Drush:

1. advagg-clear-all-files, advagg-force-new-aggregates, advagg-enable, advagg-disable commands throw Exception:

Call to undefined method Drush\Config\DrushConfig::save()

Reason: protected \Drupal\Core\Config\Config $config variable defined in the AdvaggCommands class, but DrushCommands class implemets ConfigAwareInterface , and it defines getConfig() getter to get Drush config. So \Drush\Config\DrushConfig is set as $config variable.

The issue is reproducible only for Drush 9.x

2. Lot of errors
The file public://css/optimized//FILE_NAME.css was not deleted because it does not exist.
are thrown when advagg-clear-all-files and advagg-force-new-aggregates commands are called. The same issue exists when execution clear from UI.

Reason: file_unmanaged_delete_recursive() is called with path that contains backslash (it shouldnt)

The issue is reproducible for external file_systems like Amazon S3 and internal file systems that dont process double slashes as one slash both for Drush 8.x and 9.x

CommentFileSizeAuthor
#9 advagg-broken-drush-9-commands-2949100-9.patch4.29 KBjmagunia
#5 advagg-broken-drush-9-commands-2949100-5.patch4.25 KBjoelstein
#2 advagg-broken-drush-9-commands-2949100-1.patch4.12 KBSer_Mir

Comments

Ser_Mir created an issue. See original summary.

Ser_Mir’s picture

Ser_Mir commented
StatusFileSize
new advagg-broken-drush-9-commands-2949100-1.patch4.12 KB

Try my patch in attachment please. It resolves both problems.

Ser_Mir’s picture

Ser_Mir commented
Status: Active » Needs review
idebr’s picture

idebr commented
Status: Needs review » Needs work

@Ser_Mir Thanks for the patch! It works great, however the change has a Drupal coding standard violation:

+++ b/src/Commands/AdvaggCommands.php
@@ -17,7 +17,7 @@ class AdvaggCommands extends DrushCommands {
    *
    * @var \Drupal\Core\Config\Config
    */
-  protected $config;
+  protected $advagg_config;

Class property $advagg_config should use lowerCamel naming without underscores

joelstein’s picture

joelstein commented
Status: Needs work » Needs review
StatusFileSize
new advagg-broken-drush-9-commands-2949100-5.patch4.25 KB

Removing the back slashes didn't prevent the "was not deleted because it does not exist" notice for me.

I updated the patch to simply check if the path exists before attempting to delete it, because that's what file_unmanaged_delete() does before attempting to delete it (and that's where the "was not deleted because it does not exist" error is logged).

I also updated the patch to replace "advagg_config" with "advaggConfig".

norman.lol’s picture

norman.lol
he/him
Primary language German
Location Berlin
commented
Status: Needs review » Reviewed & tested by the community

Nice patches! Thanks for the contributions. It absolutely solved my problem when I needed to force new aggregates after Drupal setup in a Docker container before making screenshots with Selenium. Without it the assets weren't loading correctly.

In the end it turned out it wasn't the re-aggregation that fixed my issue. I simply had to chmod/chown the files folder correctly... Thanks for everything anyway. :)

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented

Thanks guys!
But this patch needs re-roll:

error: patch failed: src/Commands/AdvaggCommands.php:35
error: src/Commands/AdvaggCommands.php: patch does not apply
error: patch failed: src/Form/OperationsForm.php:235
error: src/Form/OperationsForm.php: patch does not apply
thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented
Status: Reviewed & tested by the community » Needs work
jmagunia’s picture

jmagunia
Primary language English
Location Philadelphia 🇺🇸
commented
Status: Needs work » Needs review
StatusFileSize
new advagg-broken-drush-9-commands-2949100-9.patch4.29 KB

Reroll patch attached. There was a conflict which I had to sort through and I had to modify the original patch slightly: there was a call to $this->config instead of $this->advaggConfig. I hope its correct.

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented

We need someone to test.

jmagunia’s picture

jmagunia
Primary language English
Location Philadelphia 🇺🇸
commented
Issue tags: +Needs manual testing

Adding tag so it can be more easily found.

andrey.troeglazov’s picture

andrey.troeglazov commented

You have 'public://js/optimized', 'public://css/optimized' paths hardcoded in several places, its not good.
Its better to move it to some constants and use where is necessary.

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented

I suggest creating an interface for this constants in another issues.

@andrey.troeglazov, after the modifications made by @pmagunia does the module work normally?

Thanks @andrey.troeglazov!

  • thalles committed 5fdc01e on 8.x-3.x authored by pmagunia 
    Issue #2949100 by Ser_Mir, pmagunia, joelstein, thalles, idebr, leymannx...
thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented
Status: Needs review » Fixed
thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented

Fixed! Thank's @all for the hard work!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.