diff --git a/components/ILIAS/Filesystem/src/Util/Archive/Zip.php b/components/ILIAS/Filesystem/src/Util/Archive/Zip.php index 173f94ea7930..d712eef5d179 100755 --- a/components/ILIAS/Filesystem/src/Util/Archive/Zip.php +++ b/components/ILIAS/Filesystem/src/Util/Archive/Zip.php @@ -32,23 +32,24 @@ class Zip use PathHelper; public const DOT_EMPTY = '.empty'; + public const ITERATION_FACTOR = 0.9; private string $zip_output_file = ''; protected \ZipArchive $zip; private int $iteration_limit; private int $store_counter = 1; private int $path_counter = 1; + private bool $zip_opened = false; + /** * @var FileStream[] */ - private array $streams; + private array $streams = []; public function __construct( protected ZipOptions $options, - ...$streams + FileStream ...$streams ) { - $this->streams = array_filter($streams, fn($stream): bool => $stream instanceof FileStream); - if ($options->getZipOutputPath() !== null && $options->getZipOutputName() !== null) { $this->zip_output_file = $this->ensureDirectorySeperator( $options->getZipOutputPath() @@ -70,7 +71,24 @@ public function __construct( if (!file_exists($this->zip_output_file)) { touch($this->zip_output_file); } - if ($this->zip->open($this->zip_output_file, \ZipArchive::OVERWRITE) !== true) { + + $this->maybeOpenZip(\ZipArchive::OVERWRITE); + foreach ($streams as $path_inside_zip => $stream) { + $path_inside_zip = is_int($path_inside_zip) ? basename((string) $stream->getMetadata('uri')) : $path_inside_zip; + $this->addStream($stream, basename($path_inside_zip)); + } + } + + private function maybeOpenZip(int $flags = 0): void + { + if (!$this->zip_opened) { + if ($flags === 0) { + $this->zip_opened = $this->zip->open($this->zip_output_file) === true; + } else { + $this->zip_opened = $this->zip->open($this->zip_output_file, $flags) === true; + } + } + if (!$this->zip_opened) { throw new \Exception("cannot open <$this->zip_output_file>\n"); } } @@ -99,7 +117,7 @@ private function storeZIPtoFilesystem(): void foreach ($this->streams as $path_inside_zip => $stream) { $path = $stream->getMetadata('uri'); if ($this->store_counter === 0) { - $this->zip->open($this->zip_output_file); + $this->maybeOpenZip(); } if (is_int($path_inside_zip)) { $path_inside_zip = basename((string) $path); @@ -117,9 +135,10 @@ private function storeZIPtoFilesystem(): void if ( $this->store_counter === $this->iteration_limit - || count(get_resources('stream')) > ($this->iteration_limit * 0.9) + || count(get_resources('stream')) > ($this->iteration_limit * self::ITERATION_FACTOR) ) { $this->zip->close(); + $this->zip_opened = false; $this->store_counter = 0; } else { $this->store_counter++; @@ -129,9 +148,11 @@ private function storeZIPtoFilesystem(): void public function get(): Stream { + $this->maybeOpenZip(); $this->storeZIPtoFilesystem(); $this->zip->close(); + $this->zip_opened = false; return Streams::ofResource(fopen($this->zip_output_file, 'rb')); } @@ -156,7 +177,9 @@ public function destroy(): void */ public function addPath(string $path, ?string $path_inside_zip = null): void { - $path_inside_zip = $path_inside_zip ?? basename($path); + $path_inside_zip ??= basename($path); + + $this->maybeOpenZip(); // create directory if it does not exist $this->zip->addEmptyDir(rtrim(dirname($path_inside_zip), '/') . '/'); @@ -170,7 +193,7 @@ public function addPath(string $path, ?string $path_inside_zip = null): void public function addStream(FileStream $stream, string $path_inside_zip): void { // we remove the "empty zip file" now if possible - if (count($this->streams) === 1 && isset($this->streams[self::DOT_EMPTY])) { + if (isset($this->streams[self::DOT_EMPTY])) { unset($this->streams[self::DOT_EMPTY]); } @@ -179,7 +202,7 @@ public function addStream(FileStream $stream, string $path_inside_zip): void if ( $this->path_counter === $this->iteration_limit - || count(get_resources('stream')) > ($this->iteration_limit * 0.9) + || count(get_resources('stream')) > ($this->iteration_limit * self::ITERATION_FACTOR) ) { $this->storeZIPtoFilesystem(); $this->streams = []; @@ -225,7 +248,7 @@ public function addDirectory(string $directory_to_zip): void /** @var $file \SplFileInfo */ if ($file->isDir()) { // add directory to zip if it's empty - $sub_items = array_filter(scandir($pathname), static fn($d): bool => !str_contains((string) $d, '.DS_Store')); + $sub_items = array_filter(scandir($pathname), static fn($d): bool => !str_contains($d, '.DS_Store')); if (count($sub_items) === 2) { $this->zip->addEmptyDir($path_inside_zip); } diff --git a/components/ILIAS/Filesystem/tests/Util/ZipTest.php b/components/ILIAS/Filesystem/tests/Util/ZipTest.php index 9a592d1b6b60..c910e4374bea 100755 --- a/components/ILIAS/Filesystem/tests/Util/ZipTest.php +++ b/components/ILIAS/Filesystem/tests/Util/ZipTest.php @@ -27,12 +27,12 @@ use ILIAS\Filesystem\Util\Archive\ZipOptions; /** - * @author Fabian Schmid + * @author Fabian Schmid * * @runTestsInSeparateProcesses // This is required for the test to work since we define some constants in the test - * @preserveGlobalState disabled - * @backupGlobals disabled - * @backupStaticAttributes disabled + * @preserveGlobalState disabled + * @backupGlobals disabled + * @backupStaticAttributes disabled */ class ZipTest extends TestCase { @@ -69,7 +69,39 @@ public function testZip(): void $this->assertGreaterThan(0, $zip_stream->getSize()); $unzip_again = new Unzip(new UnzipOptions(), $zip_stream); - $this->assertEquals(2, $unzip_again->getAmountOfFiles()); + $this->assertSame(2, $unzip_again->getAmountOfFiles()); + } + + public function testZip2(): void + { + $zip_options = new ZipOptions(); + $streams = [ + Streams::ofResource(fopen($this->zips_dir . '1_folder_1_file_mac.zip', 'r')), + ]; + $zip = new Zip($zip_options, ...$streams); + $zip_stream = $zip->get(); + $this->assertGreaterThan(0, $zip_stream->getSize()); + + $unzip_again = new Unzip(new UnzipOptions(), $zip_stream); + $this->assertSame(1, $unzip_again->getAmountOfFiles()); + } + + public function testZip3(): void + { + $zip_options = new ZipOptions(); + $streams = [ + Streams::ofResource(fopen($this->zips_dir . '1_folder_1_file_mac.zip', 'r')), + Streams::ofResource(fopen($this->zips_dir . '1_folder_mac.zip', 'r')), + Streams::ofResource(fopen($this->zips_dir . '1_folder_win.zip', 'r')), + Streams::ofResource(fopen($this->zips_dir . '3_folders_mac.zip', 'r')), + Streams::ofResource(fopen($this->zips_dir . '3_folders_win.zip', 'r')), + ]; + $zip = new Zip($zip_options, ...$streams); + $zip_stream = $zip->get(); + $this->assertGreaterThan(0, $zip_stream->getSize()); + + $unzip_again = new Unzip(new UnzipOptions(), $zip_stream); + $this->assertSame(5, $unzip_again->getAmountOfFiles()); } public function testLegacyZip(): void @@ -85,8 +117,19 @@ public function testLegacyZip(): void $legacy->zip($this->zips_dir, $this->unzips_dir . self::ZIPPED_ZIP, false); $this->assertFileExists($this->unzips_dir . self::ZIPPED_ZIP); - $unzip_again = new Unzip(new UnzipOptions(), Streams::ofResource(fopen($this->unzips_dir . self::ZIPPED_ZIP, 'r'))); - $this->assertEquals(5, $unzip_again->getAmountOfFiles()); + $unzip_again = new Unzip( + new UnzipOptions(), + Streams::ofResource(fopen($this->unzips_dir . self::ZIPPED_ZIP, 'r')) + ); + $this->assertSame([ + 0 => './', + 1 => '3_folders_win.zip', + 2 => '1_folder_mac.zip', + 3 => '3_folders_mac.zip', + 4 => '1_folder_win.zip', + 5 => '1_folder_1_file_mac.zip' + ], iterator_to_array($unzip_again->getPaths())); + $this->assertSame(5, $unzip_again->getAmountOfFiles()); $depth = 0; foreach ($unzip_again->getPaths() as $path) { @@ -111,8 +154,11 @@ public function LegacyZipWithTop(): void $legacy->zip($this->zips_dir, $this->unzips_dir . self::ZIPPED_ZIP, true); $this->assertFileExists($this->unzips_dir . self::ZIPPED_ZIP); - $unzip_again = new Unzip(new UnzipOptions(), Streams::ofResource(fopen($this->unzips_dir . self::ZIPPED_ZIP, 'r'))); - $this->assertEquals(5, $unzip_again->getAmountOfFiles()); + $unzip_again = new Unzip( + new UnzipOptions(), + Streams::ofResource(fopen($this->unzips_dir . self::ZIPPED_ZIP, 'r')) + ); + $this->assertSame(5, $unzip_again->getAmountOfFiles()); $depth = 0; foreach ($unzip_again->getPaths() as $path) { @@ -145,10 +191,10 @@ public function testUnzip( $unzip = new Unzip($options, $stream); $this->assertFalse($unzip->hasZipReadingError()); - $this->assertEquals($has_multiple_root_entries, $unzip->hasMultipleRootEntriesInZip()); - $this->assertEquals($expected_amount_directories, $unzip->getAmountOfDirectories()); + $this->assertSame($has_multiple_root_entries, $unzip->hasMultipleRootEntriesInZip()); + $this->assertSame($expected_amount_directories, $unzip->getAmountOfDirectories()); $this->assertEquals($expected_directories, iterator_to_array($unzip->getDirectories())); - $this->assertEquals($expected_amount_files, $unzip->getAmountOfFiles()); + $this->assertSame($expected_amount_files, $unzip->getAmountOfFiles()); $this->assertEquals($expected_files, iterator_to_array($unzip->getFiles())); } @@ -159,14 +205,13 @@ public function testWrongZip(): void $unzip = new Unzip($options, $stream); $this->assertTrue($unzip->hasZipReadingError()); $this->assertFalse($unzip->hasMultipleRootEntriesInZip()); - $this->assertEquals(0, iterator_count($unzip->getFiles())); - $this->assertEquals(0, iterator_count($unzip->getDirectories())); - $this->assertEquals(0, iterator_count($unzip->getPaths())); - $this->assertEquals([], iterator_to_array($unzip->getDirectories())); - $this->assertEquals([], iterator_to_array($unzip->getFiles())); + $this->assertCount(0, iterator_to_array($unzip->getFiles())); + $this->assertCount(0, iterator_to_array($unzip->getDirectories())); + $this->assertCount(0, iterator_to_array($unzip->getPaths())); + $this->assertSame([], iterator_to_array($unzip->getDirectories())); + $this->assertSame([], iterator_to_array($unzip->getFiles())); } - public function testLargeZIPs(): void { // get ulimit @@ -264,15 +309,13 @@ private function directoryToArray(string $path_to_directory): array // PROVIDERS - public static function getZips(): array + public static function getZips(): \Iterator { - return [ - ['1_folder_mac.zip', false, 10, self::$directories_one, 15, self::$files_one], - ['1_folder_win.zip', false, 10, self::$directories_one, 15, self::$files_one], - ['3_folders_mac.zip', true, 9, self::$directories_three, 12, self::$files_three], - ['3_folders_win.zip', true, 9, self::$directories_three, 12, self::$files_three], - ['1_folder_1_file_mac.zip', true, 3, self::$directories_mixed, 5, self::$files_mixed] - ]; + yield ['1_folder_mac.zip', false, 10, self::$directories_one, 15, self::$files_one]; + yield ['1_folder_win.zip', false, 10, self::$directories_one, 15, self::$files_one]; + yield ['3_folders_mac.zip', true, 9, self::$directories_three, 12, self::$files_three]; + yield ['3_folders_win.zip', true, 9, self::$directories_three, 12, self::$files_three]; + yield ['1_folder_1_file_mac.zip', true, 3, self::$directories_mixed, 5, self::$files_mixed]; } protected static array $files_mixed = [