From f9bf3b8d47ead2f4a862b37938bbd60af0e06e50 Mon Sep 17 00:00:00 2001 From: Pierre Rudloff Date: Sun, 21 Apr 2019 00:34:12 +0200 Subject: [PATCH] refactor: Use a StreamInterface for PlaylistArchiveStream It is much cleaner --- classes/PlaylistArchiveStream.php | 181 ++++++++++++++++++++-------- classes/VideoDownload.php | 24 ---- controllers/FrontController.php | 5 +- index.php | 2 - tests/LocaleManagerTest.php | 2 +- tests/LocaleTest.php | 2 +- tests/PlaylistArchiveStreamTest.php | 178 +++++++++++++++++++-------- tests/VideoDownloadStubsTest.php | 15 --- tests/VideoDownloadTest.php | 15 --- 9 files changed, 267 insertions(+), 157 deletions(-) diff --git a/classes/PlaylistArchiveStream.php b/classes/PlaylistArchiveStream.php index 08ee875..70dd68f 100644 --- a/classes/PlaylistArchiveStream.php +++ b/classes/PlaylistArchiveStream.php @@ -1,28 +1,29 @@ client = new Client(); $this->download = new VideoDownload($config); + + $this->format = $format; + $buffer = fopen('php://temp', 'r+'); + if ($buffer !== false) { + $this->buffer = $buffer; + } + foreach ($video->entries as $entry) { + $this->files[] = [ + 'url' => $entry->url, + 'headersSent' => false, + 'complete' => false, + 'stream' => null, + ]; + } } /** @@ -84,89 +101,152 @@ class PlaylistArchiveStream extends TarArchive // Add data to the buffer. fwrite($this->buffer, $data); if ($pos !== false) { - // Rewind so that stream_read() can later read this data. + // Rewind so that read() can later read this data. fseek($this->buffer, $pos); } } /** - * Called when fopen() is used on the stream. + * Write data to the stream. * - * @param string $path Playlist path (should be playlist://url1;url2;.../format) + * @param string $string The string that is to be written. * - * @return bool + * @return int */ - public function stream_open($path) + public function write($string) { - $this->format = ltrim(parse_url($path, PHP_URL_PATH), '/'); - $buffer = fopen('php://temp', 'r+'); - if ($buffer !== false) { - $this->buffer = $buffer; - } - foreach (explode(';', parse_url($path, PHP_URL_HOST)) as $url) { - $this->files[] = [ - 'url' => urldecode($url), - 'headersSent' => false, - 'complete' => false, - 'stream' => null, - ]; - } + throw new RuntimeException('This stream is not writeable.'); + } + /** + * Get the size of the stream if known. + * + * @return null + */ + public function getSize() + { + return null; + } + + /** + * Returns whether or not the stream is seekable. + * + * @return boolean + */ + public function isSeekable() + { + return false; + } + + /** + * Seek to the beginning of the stream. + * + * @return void + */ + public function rewind() + { + throw new RuntimeException('This stream is not seekable.'); + } + + /** + * Returns whether or not the stream is writable. + * + * @return boolean + */ + public function isWritable() + { + return false; + } + + /** + * Returns whether or not the stream is readable. + * + * @return boolean + */ + public function isReadable() + { return true; } /** - * Called when fwrite() is used on the stream. + * Returns the remaining contents in a string. * - * @return int + * @return string */ - public function stream_write() + public function getContents() { - //We don't support writing to a stream - return 0; + return stream_get_contents($this->buffer); } /** - * Called when fstat() is used on the stream. + * Get stream metadata as an associative array or retrieve a specific key. * - * @return array + * @param string $key string $key Specific metadata to retrieve. + * + * @return null */ - public function stream_stat() + public function getMetadata($key = null) { - //We need this so Slim won't try to get the size of the stream - return [ - 'mode' => 0010000, - ]; + return null; } /** - * Called when ftell() is used on the stream. + * Separates any underlying resources from the stream. + * + * @return resource + */ + public function detach() + { + $stream = $this->buffer; + $this->close(); + return $stream; + } + + /** + * Reads all data from the stream into a string, from the beginning to end. + * + * @return string + */ + public function __toString() + { + $string = ''; + + foreach ($this->files as $file) { + $string .= $file['url']; + } + + return $string; + } + + /** + * Returns the current position of the file read/write pointer * * @return int|false */ - public function stream_tell() + public function tell() { return ftell($this->buffer); } /** - * Called when fseek() is used on the stream. + * Seek to a position in the stream. * * @param int $offset Offset + * @param int $whence Specifies how the cursor position will be calculated * - * @return bool + * @return void */ - public function stream_seek($offset) + public function seek($offset, $whence = SEEK_SET) { - return fseek($this->buffer, $offset) == 0; + throw new RuntimeException('This stream is not seekable.'); } /** - * Called when feof() is used on the stream. + * Returns true if the stream is at the end of the stream. * * @return bool */ - public function stream_eof() + public function eof() { foreach ($this->files as $file) { if (!$file['complete']) { @@ -178,13 +258,13 @@ class PlaylistArchiveStream extends TarArchive } /** - * Called when fread() is used on the stream. + * Read data from the stream. * * @param int $count Number of bytes to read * * @return string|false */ - public function stream_read($count) + public function read($count) { if (!$this->files[$this->curFile]['headersSent']) { $urls = $this->download->getURL($this->files[$this->curFile]['url'], $this->format); @@ -211,14 +291,19 @@ class PlaylistArchiveStream extends TarArchive } /** - * Called when fclose() is used on the stream. + * Closes the stream and any underlying resources. * * @return void */ - public function stream_close() + public function close() { if (is_resource($this->buffer)) { fclose($this->buffer); } + foreach ($this->files as $file) { + if (is_resource($file['stream'])) { + fclose($file['stream']); + } + } } } diff --git a/classes/VideoDownload.php b/classes/VideoDownload.php index 56e0684..b32b4d8 100644 --- a/classes/VideoDownload.php +++ b/classes/VideoDownload.php @@ -481,30 +481,6 @@ class VideoDownload return $stream; } - /** - * Get a Tar stream containing every video in the playlist piped through the server. - * - * @param stdClass $video Video object returned by youtube-dl - * @param string $format Requested format - * - * @throws Exception If the popen stream was not created correctly - * - * @return resource - */ - public function getPlaylistArchiveStream(stdClass $video, $format) - { - $playlistItems = []; - foreach ($video->entries as $entry) { - $playlistItems[] = urlencode($entry->url); - } - $stream = fopen('playlist://'.implode(';', $playlistItems).'/'.$format, 'r'); - if (!is_resource($stream)) { - throw new Exception(_('Could not open fopen stream.')); - } - - return $stream; - } - /** * Get the stream of a converted video. * diff --git a/controllers/FrontController.php b/controllers/FrontController.php index 4e73858..9789583 100644 --- a/controllers/FrontController.php +++ b/controllers/FrontController.php @@ -10,6 +10,7 @@ use Alltube\EmptyUrlException; use Alltube\Locale; use Alltube\LocaleManager; use Alltube\PasswordException; +use Alltube\PlaylistArchiveStream; use Alltube\VideoDownload; use Aura\Session\Segment; use Aura\Session\SessionFactory; @@ -403,14 +404,14 @@ class FrontController { $video = $this->download->getJSON($url, $format, $password); if (isset($video->entries)) { - $stream = $this->download->getPlaylistArchiveStream($video, $format); + $stream = new PlaylistArchiveStream($this->config, $video, $format); $response = $response->withHeader('Content-Type', 'application/x-tar'); $response = $response->withHeader( 'Content-Disposition', 'attachment; filename="'.$video->title.'.tar"' ); - return $response->withBody(new Stream($stream)); + return $response->withBody($stream); } elseif ($video->protocol == 'rtmp') { $stream = $this->download->getRtmpStream($video); $response = $response->withHeader('Content-Type', 'video/'.$video->ext); diff --git a/index.php b/index.php index a3d4a27..34d8d99 100644 --- a/index.php +++ b/index.php @@ -15,8 +15,6 @@ if (isset($_SERVER['REQUEST_URI']) && strpos($_SERVER['REQUEST_URI'], '/index.ph die; } -stream_wrapper_register('playlist', PlaylistArchiveStream::class); - $app = new App(); $container = $app->getContainer(); $config = Config::getInstance(); diff --git a/tests/LocaleManagerTest.php b/tests/LocaleManagerTest.php index 718f5b2..9bf9837 100644 --- a/tests/LocaleManagerTest.php +++ b/tests/LocaleManagerTest.php @@ -10,7 +10,7 @@ use Alltube\LocaleManager; use PHPUnit\Framework\TestCase; /** - * Unit tests for the Config class. + * Unit tests for the LocaleManagerTest class. */ class LocaleManagerTest extends TestCase { diff --git a/tests/LocaleTest.php b/tests/LocaleTest.php index 3cf257b..ba18b84 100644 --- a/tests/LocaleTest.php +++ b/tests/LocaleTest.php @@ -9,7 +9,7 @@ use Alltube\Locale; use PHPUnit\Framework\TestCase; /** - * Unit tests for the Config class. + * Unit tests for the LocaleTest class. */ class LocaleTest extends TestCase { diff --git a/tests/PlaylistArchiveStreamTest.php b/tests/PlaylistArchiveStreamTest.php index 36a49d3..1c50c54 100644 --- a/tests/PlaylistArchiveStreamTest.php +++ b/tests/PlaylistArchiveStreamTest.php @@ -8,6 +8,8 @@ namespace Alltube\Test; use Alltube\Config; use Alltube\PlaylistArchiveStream; use PHPUnit\Framework\TestCase; +use stdClass; +use RuntimeException; /** * Unit tests for the ViewFactory class. @@ -31,7 +33,14 @@ class PlaylistArchiveStreamTest extends TestCase } else { $configFile = 'config_test.yml'; } - $this->stream = new PlaylistArchiveStream(Config::getInstance('config/'.$configFile)); + + $entry = new stdClass(); + $entry->url = 'BaW_jenozKc'; + + $video = new stdClass(); + $video->entries = [$entry, $entry]; + + $this->stream = new PlaylistArchiveStream(Config::getInstance('config/'.$configFile), $video, 'worst'); } /** @@ -41,71 +50,51 @@ class PlaylistArchiveStreamTest extends TestCase */ protected function tearDown() { - $this->stream->stream_close(); + $this->stream->close(); } /** - * Test the stream_open() function. + * Test the write() function. + * + * @return void + * @expectedException RuntimeException + */ + public function testWrite() + { + $this->stream->write('foo'); + } + + + /** + * Test the tell() function. * * @return void */ - public function testStreamOpen() + public function testTell() { - $this->assertTrue($this->stream->stream_open('playlist://foo')); + $this->assertInternalType('int', $this->stream->tell()); } /** - * Test the stream_write() function. + * Test the seek() function. * * @return void + * @expectedException RuntimeException */ - public function testStreamWrite() + public function testSeek() { - $this->assertEquals(0, $this->stream->stream_write()); + $this->stream->seek(42); } /** - * Test the stream_stat() function. + * Test the read() function. * * @return void */ - public function testStreamStat() + public function testRead() { - $this->assertEquals(['mode' => 4096], $this->stream->stream_stat()); - } - - /** - * Test the stream_tell() function. - * - * @return void - */ - public function testStreamTell() - { - $this->stream->stream_open('playlist://foo'); - $this->assertInternalType('int', $this->stream->stream_tell()); - } - - /** - * Test the stream_seek() function. - * - * @return void - */ - public function testStreamSeek() - { - $this->stream->stream_open('playlist://foo'); - $this->assertInternalType('bool', $this->stream->stream_seek(3)); - } - - /** - * Test the stream_read() function. - * - * @return void - */ - public function testStreamRead() - { - $this->stream->stream_open('playlist://BaW_jenozKc;BaW_jenozKc/worst'); - while (!$this->stream->stream_eof()) { - $result = $this->stream->stream_read(8192); + while (!$this->stream->eof()) { + $result = $this->stream->read(8192); $this->assertInternalType('string', $result); if (is_string($result)) { $this->assertLessThanOrEqual(8192, strlen($result)); @@ -114,13 +103,104 @@ class PlaylistArchiveStreamTest extends TestCase } /** - * Test the stream_eof() function. + * Test the eof() function. * * @return void */ - public function testStreamEof() + public function testEof() { - $this->stream->stream_open('playlist://foo'); - $this->assertFalse($this->stream->stream_eof()); + $this->assertFalse($this->stream->eof()); + } + + /** + * Test the getSize() function. + * + * @return void + */ + public function testGetSize() + { + $this->assertNull($this->stream->getSize()); + } + + /** + * Test the isSeekable() function. + * + * @return void + */ + public function testIsSeekable() + { + $this->assertFalse($this->stream->isSeekable()); + } + + /** + * Test the rewind() function. + * + * @return void + * @expectedException RuntimeException + */ + public function testRewind() + { + $this->stream->rewind(); + } + + /** + * Test the isWritable() function. + * + * @return void + */ + public function testIsWritable() + { + $this->assertFalse($this->stream->isWritable()); + } + + /** + * Test the isReadable() function. + * + * @return void + */ + public function testIsReadable() + { + $this->assertTrue($this->stream->isReadable()); + } + + /** + * Test the getContents() function. + * + * @return void + */ + public function testGetContents() + { + $this->assertInternalType('string', $this->stream->getContents()); + } + + /** + * Test the getMetadata() function. + * + * @return void + */ + public function testGetMetadata() + { + $this->assertNull($this->stream->getMetadata()); + } + + /** + * Test the detach() function. + * + * @return void + */ + public function testDetach() + { + $this->assertInternalType('resource', $this->stream->detach()); + } + + /** + * Test the __toString() function. + * + * @return void + */ + public function testToString() + { + $this->assertInternalType('string', $this->stream->__toString()); + $this->assertInternalType('string', (string) $this->stream); } } diff --git a/tests/VideoDownloadStubsTest.php b/tests/VideoDownloadStubsTest.php index 71a50ad..c97f5f0 100644 --- a/tests/VideoDownloadStubsTest.php +++ b/tests/VideoDownloadStubsTest.php @@ -110,21 +110,6 @@ class VideoDownloadStubsTest extends TestCase $this->download->getRemuxStream([$this->url, $this->url]); } - /** - * Test getPlaylistArchiveStream function with a buggy popen. - * - * @return void - * @expectedException Exception - */ - public function testGetPlaylistArchiveStreamWithPopenError() - { - $video = $this->download->getJSON( - 'https://www.youtube.com/playlist?list=PLgdySZU6KUXL_8Jq5aUkyNV7wCa-4wZsC', - 'best' - ); - $this->download->getPlaylistArchiveStream($video, 'best'); - } - /** * Test getConvertedStream function with a buggy popen. * diff --git a/tests/VideoDownloadTest.php b/tests/VideoDownloadTest.php index 87c0ce9..a16c15b 100644 --- a/tests/VideoDownloadTest.php +++ b/tests/VideoDownloadTest.php @@ -533,21 +533,6 @@ class VideoDownloadTest extends TestCase $download->getM3uStream($video); } - /** - * Test getPlaylistArchiveStream function. - * - * @return void - * @requires OS Linux - */ - public function testGetPlaylistArchiveStream() - { - $video = $this->download->getJSON( - 'https://www.youtube.com/playlist?list=PLgdySZU6KUXL_8Jq5aUkyNV7wCa-4wZsC', - 'best' - ); - $this->assertStream($this->download->getPlaylistArchiveStream($video, 'best')); - } - /** * Test getConvertedStream function without avconv. *