Moodle 2 zip handling - inefficient extraction?

Moodle 2 zip handling - inefficient extraction?

by Paul Nicholls -
Number of replies: 0

After experiencing difficulty getting a Moodle backup (mbz) to extract and restore on a test server, I've discovered what appears to be a large inefficiency in Moodle 2's handling of zip files.  In my case, it manifested itself by stopping extraction of a ~60MB backup after extracting 21MB of files, with no errors to be found anywhere.  There may well be a good reason that things are being done as they are, but I can't see any sign of it - commit logs and tracker tickets only refer to implementing unzipping support...

The zip_packer class has a file-by-file extraction process in extract_to_pathname() rather than simply using ZipArchive's extractTo() function directly.  The only difference is that extract_to_pathname() builds an array of files and folders, which can then be looped over to find out whether an individual file or folder failed to extract.  The array's only used in extract_zip() in lib/deprecatedlib.php, where it simply loops over and returns false if any individual element in the array isn't (boolean) true.  It's already checking the overall falseness of the return value, so if we leave it at that, everything will be all set to handle pure boolean values and we can switch to extractTo() without breaking anything (in core, at least).  This is what I ended up doing to get the backup to restore - it now works perfectly, and seems to be faster as well.

I'm certainly happy to create a tracker ticket and submit a pull request for the changes, but before I do so, I'm looking for feedback from other developers before I do so - particularly anyone who has any knowledge of why it was implemented this way (Petr, if you're reading this, I see you committed it - your input would be appreciated, if you can remember August 2008 wink).

 

-Paul

Average of ratings: -