Add some improvements for SongZipArchive

This commit is contained in:
Phan An 2019-06-30 11:44:39 +02:00
parent 756b325147
commit 9efd232daf
6 changed files with 44 additions and 75 deletions

View file

@ -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()

View file

@ -14,14 +14,14 @@ 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
@ -29,26 +29,11 @@ class SongZipArchive
*
* @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 @@ class SongZipArchive
{
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 @@ class SongZipArchive
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
{
return $this->archive;
$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
{
// 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());
}
}

View file

@ -56,7 +56,7 @@ class DownloadService
$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 {

View file

@ -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"

View file

@ -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 @@ class SongZipArchiveTest extends TestCase
]),
]);
// 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));
}

View file

@ -1,17 +0,0 @@
<?php
namespace Tests\Unit\Models;
use App\Models\SongZipArchive;
use Tests\TestCase;
class SongZipArchiveTest extends TestCase
{
/** @test */
public function it_can_be_instantiated()
{
$songZipArchive = new SongZipArchive();
$this->assertInstanceOf(SongZipArchive::class, $songZipArchive);
$this->assertInstanceOf(\ZipArchive::class, $songZipArchive->getArchive());
}
}