Skip to content

Comments

Teaching zipstreamer new tricks: addFileFromString & addFileOpen/Write/Close#36

Open
rivimey wants to merge 25 commits intoMcNetic:masterfrom
rivimey:master
Open

Teaching zipstreamer new tricks: addFileFromString & addFileOpen/Write/Close#36
rivimey wants to merge 25 commits intoMcNetic:masterfrom
rivimey:master

Conversation

@rivimey
Copy link

@rivimey rivimey commented Jul 14, 2016

We needed a PHP Zip archive creator that would stream all the way - so no temporary files at any point. I have based some code on your work that adds two facilities we needed: adding a file from immediate data (i.e. a php string) and adding a file a chunk at a time. The use-case is for the Excel-writer library Spout, which I am also modifying to use ZipStreamer rather than ZipArchive.

I have included some additional tests, and modified the manual.md to describe the new api functions.

The new test does fail, but I think the problem is the test itself: the file size in the directory is expected to be 0 and (in this case) is 808. I believe this is because for the addFileFromString the code has all the data to hand and so sets the length in the file header, rather than appending a file extension record. I am, however, not sure on this point, nor how to modify the test such that it works properly :( so I would appreciate your input if you are able.

ZipStreamer\TestZipStreamer::testZipfileStreamed with data set #1 (array(true, 0, 1), array(ZipStreamer\File Object (...)), 'one empty dir (options = arra...RMAL))')
Z64EIF size
Failed asserting that two objects are equal.
--- Expected
+++ Actual
@@ @@
ZipStreamer\Count64_64 Object (

  • 'value' => 0
  • 'value' => 808
    'limit32Bit' => false
    )

.../PHPZipStreamer/test/ZipComponents.php:83
.../PHPZipStreamer/test/ZipStreamerTest.php:218
.../PHPZipStreamer/test/ZipStreamerTest.php:443

I have included the .idea directory from phpstorm - just as a helpful thing, and I'll remove it from the pull if you wish.

One thing I would like to do would be make it psr-4 compliant, but I think this would be quite a big change in the files so I've not done anything!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant