better media conversions generation

ElegantEngineeringTech · Nov 4, 2024 · 371564c · 371564c
1 parent c6301d2
commit 371564c
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 40 deletions.
diff --git a/src/Commands/GenerateMediaConversionsCommand.php b/src/Commands/GenerateMediaConversionsCommand.php
@@ -2,6 +2,7 @@
 
 namespace Elegantly\Media\Commands;
 
+use Elegantly\Media\Definitions\MediaConversionDefinition;
 use Elegantly\Media\Models\Media;
 use Illuminate\Console\Command;
 use Illuminate\Database\Eloquent\Builder;
@@ -28,6 +29,18 @@ public function handle(): int
         /** @var string[] $collections */
         $collections = (array) $this->option('collections');
 
+        $filter = function (MediaConversionDefinition $definition) use ($immediate) {
+
+            if (
+                $immediate === false &&
+                ! $definition->immediate
+            ) {
+                return false;
+            }
+
+            return true;
+        };
+
         /**
          * @var class-string<Media> $model
          */
@@ -47,7 +60,7 @@ public function handle(): int
 
         $progress = new Progress('Dispatching Media conversions', $count);
 
-        $query->chunkById(5_000, function ($items) use ($progress, $force, $immediate, $conversions) {
+        $query->chunkById(5_000, function ($items) use ($progress, $force, $conversions, $filter) {
 
             foreach ($items as $media) {
                 /** @var Media $media */
@@ -59,26 +72,26 @@ public function handle(): int
                         );
                     }
                 } else {
-                    $media->dispatchConversions(
+
+                    /**
+                     * Generate missing children conversions
+                     */
+                    $media->conversions->each(function ($conversion) use ($media, $force, $filter) {
+                        $media->generateConversions(
+                            parent: $conversion,
+                            queued: true,
+                            force: $force,
+                            filter: $filter,
+                        );
+                    });
+
+                    /**
+                     * Generate missing root conversions
+                     */
+                    $media->generateConversions(
                         queued: true,
-                        filter: function ($definition) use ($media, $force, $immediate) {
-
-                            if (
-                                $immediate === false &&
-                                ! $definition->immediate
-                            ) {
-                                return false;
-                            }
-
-                            if (
-                                $force === false &&
-                                $media->hasConversion($definition->name)
-                            ) {
-                                return false;
-                            }
-
-                            return true;
-                        }
+                        force: $force,
+                        filter: $filter,
                     );
                 }
 

diff --git a/src/Concerns/HasMedia.php b/src/Concerns/HasMedia.php
@@ -189,8 +189,9 @@ public function addMedia(
             );
         }
 
-        $media->dispatchConversions(
-            filter: fn ($definition) => $definition->immediate
+        $media->generateConversions(
+            filter: fn ($definition) => $definition->immediate,
+            force: true,
         );
 
         return $media;

diff --git a/src/Models/Media.php b/src/Models/Media.php
@@ -241,6 +241,10 @@ public function getChildrenConversionsDefinitions(string $name): array
         return $this->getConversionDefinition($name)?->conversions ?? [];
     }
 
+    /**
+     * Dispatch any conversion while generating missing parents.
+     * Will check for 'shouldExecute' definition method.
+     */
     public function dispatchConversion(
         string $conversion,
         bool $force = true,
@@ -264,6 +268,10 @@ public function dispatchConversion(
         return null;
     }
 
+    /**
+     * Execute any conversion while generating missing parents.
+     * Will check for 'shouldExecute' definition method.
+     */
     public function executeConversion(
         string $conversion,
         bool $force = true,
@@ -284,6 +292,10 @@ public function executeConversion(
                 $parent = null;
             }
 
+            if (! $definition->shouldExecute($this, $parent)) {
+                return null;
+            }
+
             return $definition->execute($this, $parent);
 
         }
@@ -356,9 +368,10 @@ public function replaceConversion(
             );
         }
 
-        $this->dispatchConversions(
+        $this->generateConversions(
             parent: $conversion,
-            filter: fn ($definition) => $definition->immediate
+            filter: fn ($definition) => $definition->immediate,
+            force: true,
         );
 
         return $conversion;
@@ -413,23 +426,27 @@ public function addConversion(
             $this->conversions->push($conversion);
         }
 
-        $this->dispatchConversions(
+        $this->generateConversions(
             parent: $conversion,
-            filter: fn ($definition) => $definition->immediate
+            filter: fn ($definition) => $definition->immediate,
+            force: true,
         );
 
         return $conversion;
     }
 
     /**
+     * Execute or dispatch first level conversions based on their definition
+     *
      * @param  null|(Closure(MediaConversionDefinition $definition):bool)  $filter
      * @param  ?bool  $queued  force queueing the conversions
      * @return $this
      */
-    public function dispatchConversions(
+    public function generateConversions(
         ?MediaConversion $parent = null,
         ?Closure $filter = null,
         ?bool $queued = null,
+        bool $force = false,
     ): static {
         if ($parent) {
             $definitions = $this->getChildrenConversionsDefinitions($parent->conversion_name);
@@ -443,28 +460,23 @@ public function dispatchConversions(
                 continue;
             }
 
-            if (! $definition->shouldExecute(
-                media: $this,
-                parent: $parent
-            )) {
-                continue;
-            }
+            $conversion = $parent ? "{$parent->conversion_name}.{$definition->name}" : $definition->name;
 
             if ($queued ?? $definition->queued) {
-                $definition->dispatch(
-                    media: $this,
-                    parent: $parent,
+                $this->dispatchConversion(
+                    conversion: $conversion,
+                    force: $force,
                 );
+
             } else {
-                $definition->execute(
-                    media: $this,
-                    parent: $parent
+                $this->executeConversion(
+                    conversion: $conversion,
+                    force: $force
                 );
             }
         }
 
         return $this;
-
     }
 
     /**