Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ZipStreamWriter to stream-write zip archives on PHP 7.2+ #103

Merged
merged 5 commits into from
Jun 6, 2024

Conversation

adamziel
Copy link
Collaborator

@adamziel adamziel commented Jun 4, 2024

Summary

This pull request introduces enhancements to the ZipStreamWriter class, enabling efficient file streaming and compression when writing ZIP archives. Key features include:

  1. Streaming File into ZIP: Implements a method to stream a file from disk directly into a ZIP archive without loading the entire file into memory.
  2. Handling Central Directory: Implements a method to write the central directory entries and the end-of-central-directory record, finalizing the ZIP archive.
  3. Deflate Compression: Supports optional deflate compression for files being added to the ZIP archive.

Closes #88

Major Changes

  1. writeFileFromPath Method:

    • Reads the source file from disk in two passes:
      • First pass: Computes CRC32, uncompressed size, and compressed size without buffering the entire file.
      • Second pass: Streams the file's compressed data directly into the ZIP archive.
    • Supports deflate compression using deflate_add.
  2. flush_directory_index Method:

    • Collects and writes central directory entries to the ZIP stream.
    • Writes the end-of-central-directory record to finalize the ZIP structure.

Example Usage

use WordPress\Zip\ZipStreamWriter;

// File paths
$sourcePathOnDisk = '/path/to/source/file.txt';
$targetPathInZip = 'archive/file.txt';

// Create a file pointer for the output ZIP file
$zipFilePointer = fopen('output.zip', 'wb');

// Instantiate the ZipStreamWriter
$zipWriter = new ZipStreamWriter($zipFilePointer);

// Write a file from the filesystem into the ZIP archive
$zipWriter->writeFileFromPath($sourcePathOnDisk, $targetPathInZip, true); // Use 'false' for no compression

// Finalize the ZIP file
$zipWriter->flush_directory_index();

fclose($zipFilePointer);

How to Test

  1. Clone the repository and checkout the branch with these changes.
  2. Ensure PHPUnit is installed.
  3. Run the test suite using the command:
    vendor/bin/phpunit
  4. Verify that all tests pass, indicating the functionality works as expected.

Notes

  • This implementation aims to handle large files efficiently by streaming data in chunks.
  • The flush_directory_index method should be called once all file entries have been written to ensure the ZIP archive is finalized correctly.

Feel free to provide any feedback or request further modifications as needed.

@adamziel adamziel added enhancement New feature or request V1 labels Jun 4, 2024
@adamziel
Copy link
Collaborator Author

adamziel commented Jun 4, 2024

PHP 8.2 and 8.3 fails on windows due to:

Class ZipArchive not found

It seems like the zip extension is missing, cc @reimic. I may merge anyway some time soon and it will break Ci on trunk.

adamziel added a commit to WordPress/playground-tools that referenced this pull request Jun 4, 2024
Ports the
[ZipStreamWriter](WordPress/blueprints-library#103)
class from WordPress Blueprints PHP library to replace the zipstream-php
library that only works with PHP 8.2. It probably works with PHP 7.2,
too, but I haven't tested it there yet. cc @bgrgicak

 ## Testing instructions

Attempt cloning the site in wp-admin (Tools > Sandbox site), confirm
that it worked.
@reimic
Copy link
Collaborator

reimic commented Jun 4, 2024

Sure. I know what this is about. Seems like an easy fix. Give me a sec. I'll take care of it now.

@reimic
Copy link
Collaborator

reimic commented Jun 5, 2024

I'd prepared a tiny fix, but I seem to have issues with pushing it to this branch directly. We'll figure it out sooner or later.

In the meantime. Regarding the failing Windows runs. To phpunit-tests-run.yml just add:

      - name: Set up PHP
        uses: shivammathur/setup-php@v2
        with:
          php-version: '${{ inputs.php }}'
          tools: phpunit-polyfills
          extensions: zip // this

It's a bit funny, using a liberal definition of the word... setup-php has those detailed presets of extensions loaded up with each os and PHP version. And it has those tiny supposedly illogical inconsistencies, like zip is automatically added to all PHP <= 8.1, but for 8.2 and 8.3 it is gone. Why? I don't know... but with similar issues you can check for maps of os to version to extension here: https://github.com/shivammathur/setup-php/wiki

To keep things clean - this also relates to #60

- Run PHPUnit tests pipeline with zip extension for all Windows
versions.
- No longer run the pipeline with php 7.0 and 7.1.
@reimic
Copy link
Collaborator

reimic commented Jun 5, 2024

HA! Done. @adamziel

@adamziel adamziel merged commit f9fcb58 into trunk Jun 6, 2024
21 checks passed
@flexseth
Copy link

flexseth commented Jun 6, 2024

Nice work! Couple of questions:

  1. When new functions are added like this (or updated), do they automatically get added to Docusaurus and the official docs via doc blocks? Or is there a scheduled process that updates the docs?

  2. Code is PHP based, it is part of v1 spec, and represents a step towards v2?

Observations

  • Seems like something that could land in Core.
  • Code is clean and looks very simple to implement!

@reimic
Copy link
Collaborator

reimic commented Jun 6, 2024

Hi.

  1. That's the intention. But we are yet to automate it.
  2. Work on this lib is done in parallel with the "fleshing out" of v2. But the lib is not yet in active use. I also wonder... @adamziel - is the goal to establish v2, finalize "major" work here and then make playground (and other places) use it?
  3. Seeing this in core would be cool as heck.
  4. :)

@reimic reimic mentioned this pull request Jun 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request V1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Port ZIP Encoder from Playground
3 participants