Add some improvements for SongZipArchive

app/Facades/Download.php

@@ -2,8 +2,12 @@
 
 namespace App\Facades;
 
+use App\Models\Song;
 use Illuminate\Support\Facades\Facade;
 
+/**
+ * @method static fromSong(Song $song)
+ */
 class Download extends Facade
 {
     protected static function getFacadeAccessor()

app/Models/SongZipArchive.php

@@ -14,41 +14,26 @@ class SongZipArchive
     /**
      * @var ZipArchive
      */
-    protected $archive;
+    private $archive;
 
     /**
      * Path to the zip archive.
      *
      * @var string
      */
-    protected $path;
+    private $path;
 
     /**
      * Names of the files in the archive
      * Format: [file-name.mp3' => currentFileIndex].
      *
      * @var array
      */
-    protected $fileNames = [];
+    private $fileNames = [];
 
-    /**
-     * @param string $path
-     *
-     * @throws RuntimeException
-     */
-    public function __construct($path = '')
+    public function __construct(string $path = '')
     {
-        if (!class_exists('ZipArchive')) {
-            throw new RuntimeException('Downloading multiple files requires ZipArchive module.');
-        }
-
-        if ($path) {
-            $this->path = $path;
-        } else {
-            // We use system's temp dir instead of storage_path() here, so that the generated files
-            // can be cleaned up automatically after server reboot.
-            $this->path = sprintf('%s%skoel-download-%s.zip', sys_get_temp_dir(), DIRECTORY_SEPARATOR, uniqid());
-        }
+        $this->path = $path ? $path : self::generateRandomArchivePath();
 
         $this->archive = new ZipArchive();
 
@@ -74,22 +59,7 @@ public function addSong(Song $song): self
     {
         try {
             $path = Download::fromSong($song);
-
-            // We add all files into the zip archive as a flat structure.
-            // As a result, there can be duplicate file names.
-            // The following several lines are to make sure each file name is unique.
-            $name = basename($path);
-            if (array_key_exists($name, $this->fileNames)) {
-                $this->fileNames[$name]++;
-                $parts = explode('.', $name);
-                $ext = $parts[count($parts) - 1];
-                $parts[count($parts) - 1] = $this->fileNames[$name].".$ext";
-                $name = implode('.', $parts);
-            } else {
-                $this->fileNames[$name] = 1;
-            }
-
-            $this->archive->addFile($path, $name);
+            $this->archive->addFile($path, $this->generateZipContentFileNameFromPath($path));
         } catch (Exception $e) {
             Log::error($e);
         }
@@ -107,16 +77,37 @@ public function finish(): self
         return $this;
     }
 
-    /**
-     * Get the path to the archive.
-     */
     public function getPath(): string
     {
         return $this->path;
     }
 
-    public function getArchive(): ZipArchive
+    /**
+     * We add all files into the zip archive as a flat structure.
+     * As a result, there can be duplicate file names.
+     * This method makes sure each file name is unique in the zip archive.
+     */
+    private function generateZipContentFileNameFromPath(string $path): string
+    {
+        $name = basename($path);
+
+        if (array_key_exists($name, $this->fileNames)) {
+            $this->fileNames[$name]++;
+            $parts = explode('.', $name);
+            $ext = $parts[count($parts) - 1];
+            $parts[count($parts) - 1] = $this->fileNames[$name] . ".$ext";
+            $name = implode('.', $parts);
+        } else {
+            $this->fileNames[$name] = 1;
+        }
+
+        return $name;
+    }
+
+    private static function generateRandomArchivePath(): string
     {
-        return $this->archive;
+        // We use system's temp dir instead of storage_path() here, so that the generated files
+        // can be cleaned up automatically after server reboot.
+        return sprintf('%s%skoel-download-%s.zip', sys_get_temp_dir(), DIRECTORY_SEPARATOR, uniqid());
     }
 }

app/Services/DownloadService.php

@@ -56,7 +56,7 @@ public function fromSong(Song $song): string
 
             $localPath = sys_get_temp_dir().DIRECTORY_SEPARATOR.basename($s3Params['key']);
 
-            // The following function require allow_url_fopen to be ON.
+            // The following function requires allow_url_fopen to be ON.
             // We're just assuming that to be the case here.
             copy($url, $localPath);
         } else {

composer.json

@@ -38,6 +38,9 @@
     "php-mock/php-mock-mockery": "^1.3",
     "mpociot/laravel-apidoc-generator": "^3.1"
   },
+  "suggest": {
+    "ext-zip": "Allow downloading multiple songs as Zip archives"
+  },
   "autoload": {
     "classmap": [
       "database"

tests/Integration/Models/SongZipArchiveTest.php

@@ -11,27 +11,20 @@ class SongZipArchiveTest extends TestCase
     /** @test */
     public function a_song_can_be_added_into_an_archive()
     {
-        // Given a song
         $song = factory(Song::class)->create([
             'path' => realpath(__DIR__.'/../../songs/full.mp3'),
         ]);
 
-        // When I add the song into the archive
-        $songArchive = new SongZipArchive();
-        $songArchive->addSong($song);
+        $archive = new SongZipArchive();
+        $archive->addSong($song);
 
-        // Then I see the archive contains one file
-        $archive = $songArchive->getArchive();
         $this->assertEquals(1, $archive->numFiles);
-
-        // and the file is our song
         $this->assertEquals('full.mp3', $archive->getNameIndex(0));
     }
 
     /** @test */
     public function multiple_songs_can_be_added_into_an_archive()
     {
-        // Given some songs
         $songs = collect([
             factory(Song::class)->create([
                 'path' => realpath(__DIR__.'/../../songs/full.mp3'),
@@ -41,15 +34,10 @@ public function multiple_songs_can_be_added_into_an_archive()
             ]),
         ]);
 
-        // When I add the songs into the archive
-        $songArchive = new SongZipArchive();
-        $songArchive->addSongs($songs);
+        $archive = new SongZipArchive();
+        $archive->addSongs($songs);
 
-        // Then I see the archive contains two files
-        $archive = $songArchive->getArchive();
         $this->assertEquals(2, $archive->numFiles);
-
-        // and the files are our songs
         $this->assertEquals('full.mp3', $archive->getNameIndex(0));
         $this->assertEquals('lorem.mp3', $archive->getNameIndex(1));
     }