Fix AssetQueue to only return newly processed assets, not existing ones

Co-authored-by: dkarlovi <[email protected]>

Author: Copilot

src/AssetQueue.php

@@ -41,6 +41,8 @@ public function add(AssetCopy|AssetFetch $specification): void
      */
     public function flush(?callable $callable = null): array
     {
+        $processedAssets = [];
+        
         foreach ($this->queue as $specification) {
             $destination = $this->buildDir.'/'.mb_ltrim($specification->destination, '/');
             if (file_exists($destination)) {
@@ -67,17 +69,18 @@ public function flush(?callable $callable = null): array
                 if ($callable !== null) {
                     $callable($specification);
                 }
+                $processedAssets[] = $specification;
                 continue;
             }
 
             $this->filesystem->copy($specification->source, $destination);
             if ($callable !== null) {
                 $callable($specification);
             }
+            $processedAssets[] = $specification;
         }
-        $queue = array_values($this->queue);
         $this->queue = [];
 
-        return $queue;
+        return $processedAssets;
     }
 }

tests/unit/AssetQueueTest.php

@@ -0,0 +1,136 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the Sigwin Yassg project.
+ *
+ * (c) sigwin.hr
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace Sigwin\YASSG\Test;
+
+use PHPUnit\Framework\TestCase;
+use Sigwin\YASSG\Asset\AssetCopy;
+use Sigwin\YASSG\Asset\AssetFetch;
+use Sigwin\YASSG\AssetQueue;
+use Symfony\Component\Filesystem\Filesystem;
+use Symfony\Contracts\HttpClient\HttpClientInterface;
+
+class AssetQueueTest extends TestCase
+{
+    private string $tempDir;
+    private AssetQueue $assetQueue;
+    private Filesystem $filesystem;
+    private HttpClientInterface $httpClient;
+
+    protected function setUp(): void
+    {
+        $this->tempDir = sys_get_temp_dir() . '/yassg_test_' . uniqid();
+        mkdir($this->tempDir, 0777, true);
+        
+        $this->filesystem = new Filesystem();
+        $this->httpClient = $this->createMock(HttpClientInterface::class);
+        $this->assetQueue = new AssetQueue($this->tempDir, $this->filesystem, $this->httpClient);
+    }
+
+    protected function tearDown(): void
+    {
+        if (is_dir($this->tempDir)) {
+            $this->filesystem->remove($this->tempDir);
+        }
+    }
+
+    public function testFlushDoesNotReturnExistingAssets(): void
+    {
+        // Create a temporary source file
+        $sourceFile = $this->tempDir . '/source.txt';
+        file_put_contents($sourceFile, 'test content');
+        
+        // Create existing destination file  
+        $existingDestination = '/existing/asset.txt';
+        $existingDestinationPath = $this->tempDir . $existingDestination;
+        $this->filesystem->mkdir(dirname($existingDestinationPath));
+        file_put_contents($existingDestinationPath, 'existing content');
+        
+        // Create new destination path (doesn't exist yet)
+        $newDestination = '/new/asset.txt';
+        
+        // Add assets to queue
+        $existingAsset = new AssetCopy($sourceFile, $existingDestination);
+        $newAsset = new AssetCopy($sourceFile, $newDestination);
+        
+        $this->assetQueue->add($existingAsset);
+        $this->assetQueue->add($newAsset);
+        
+        // Track which assets were processed via callback
+        $processedAssets = [];
+        
+        // Flush the queue
+        $returnedAssets = $this->assetQueue->flush(function ($asset) use (&$processedAssets) {
+            $processedAssets[] = $asset;
+        });
+        
+        // Currently this test will FAIL because the method returns all assets
+        // After the fix, it should only return the new asset
+        $this->assertCount(1, $returnedAssets, 'Should only return newly processed assets, not existing ones');
+        $this->assertSame($newAsset, $returnedAssets[0], 'Should return the new asset');
+        
+        // Verify callback was only called for new asset
+        $this->assertCount(1, $processedAssets, 'Callback should only be called for newly processed assets');
+        $this->assertSame($newAsset, $processedAssets[0], 'Callback should be called with the new asset');
+        
+        // Verify the new asset was actually copied
+        $this->assertFileExists($this->tempDir . $newDestination, 'New asset should be copied');
+        $this->assertEquals('test content', file_get_contents($this->tempDir . $newDestination));
+        
+        // Verify existing file was not modified
+        $this->assertEquals('existing content', file_get_contents($existingDestinationPath));
+    }
+
+    public function testFlushWithNoExistingAssets(): void
+    {
+        // Create a temporary source file
+        $sourceFile = $this->tempDir . '/source.txt';
+        file_put_contents($sourceFile, 'test content');
+        
+        // Create assets that don't exist yet
+        $asset1 = new AssetCopy($sourceFile, '/new/asset1.txt');
+        $asset2 = new AssetCopy($sourceFile, '/new/asset2.txt');
+        
+        $this->assetQueue->add($asset1);
+        $this->assetQueue->add($asset2);
+        
+        // Track which assets were processed via callback
+        $processedAssets = [];
+        
+        // Flush the queue
+        $returnedAssets = $this->assetQueue->flush(function ($asset) use (&$processedAssets) {
+            $processedAssets[] = $asset;
+        });
+        
+        // Should return all assets since none existed
+        $this->assertCount(2, $returnedAssets, 'Should return all processed assets');
+        $this->assertContains($asset1, $returnedAssets);
+        $this->assertContains($asset2, $returnedAssets);
+        
+        // Verify callback was called for all assets
+        $this->assertCount(2, $processedAssets, 'Callback should be called for all processed assets');
+        $this->assertContains($asset1, $processedAssets);
+        $this->assertContains($asset2, $processedAssets);
+        
+        // Verify both assets were copied
+        $this->assertFileExists($this->tempDir . '/new/asset1.txt');
+        $this->assertFileExists($this->tempDir . '/new/asset2.txt');
+    }
+
+    public function testEmptyQueue(): void
+    {
+        $returnedAssets = $this->assetQueue->flush();
+        
+        $this->assertEmpty($returnedAssets, 'Empty queue should return empty array');
+    }
+}