2
\$\begingroup\$

I'm using the following code to unzip file which is working OK. I'm getting file with the request (express) and unzip it in specified folder

_unzip: function (req) {
        return new Promise((resolve, reject) =>{
            var noOfFiles = 0;
            let filePath = path.join(req.file.destination, req.file.filename);
            logger.info("Unzip filePath: " + filePath);
            if (req.file) {
                yauzl.open(filePath, function (err, zipFile) {
                    if (err) throw err;
                    zipFile.on('entry', (entry) =>{
                        //console.log(entry.fileName);
                        if (/\/$/.test(entry.fileName)) {
                            return;
                        }
                        zipFile.openReadStream(entry, (err, readStream) => {
                            if (err) {
                                logger.info(err);
                                reject(err);
                            } else {
                                // ensure parent directory exists, and then:
                                let destinationFileName = "./" + entry.fileName;

                                let directoryName = path.dirname(destinationFileName);
                                if (!fs.existsSync(directoryName)) {
                                    mkdirp(directoryName, (err) =>{
                                        if (err) {
                                            logger.info(err);
                                            reject(err);
                                        } else {
                                            writeIntoFile(readStream, destinationFileName, reject);
                                            noOfFiles++;
                                        }
                                    });
                                } else {
                                    writeIntoFile(readStream, destinationFileName, reject);
                                    noOfFiles++;
                                }
                            }
                        });
                    }).once('error', (err) =>{
                        logger.info(err);
                        reject(err);
                    }).once('close', () =>{
                        logger.info("Unpacked " + noOfFiles + " files");
                        resolve();
                    });
                });
            }
    });
}

https://github.com/thejoshwolfe/yauzl

\$\endgroup\$
3
\$\begingroup\$
  • Check for req.file before using its properties
  • Return/reject early instead of nesting a big else clause
  • Since you're using ES2015 syntax do it consistently:
    • template strings via backticks
    • arrow functions in callbacks that don't utilize this of the callee
    • string methods with self-explanatory names like endsWith instead of regexps
    • let instead of var (there was one in your code)
  • Deduplicate reject+logger combo and writeIntoFile
  • Use single or double quotes consistently

return new Promise((resolve, reject) => {
    if (!req || !req.file) {
        __rejectAndLog('No file provided');
        return;
    }
    let numberOfFiles = 0;
    let filePath = path.join(req.file.destination, req.file.filename);
    logger.info(`Unzip filePath: ${filePath}`);

    yauzl.open(filePath, (err, zipFile) => {
        if (err) {
            __rejectAndLog(err);
            return;
        }
        zipFile.on('entry', entry => __processEntry(zipFile, entry))
            .once('error', __rejectAndLog)
            .once('close', () => {
                logger.info(`Unpacked ${numberOfFiles} files`);
                resolve();
            });
    });

    function __processEntry(zipFile, entry) {
        if (entry.fileName.endsWith('/')) {
            return;
        }

        zipFile.openReadStream(entry, (err, readStream) => {
            if (err) {
                __rejectAndLog(err);
                return;
            }

            let destinationFileName = './' + entry.fileName;
            let directoryName = path.dirname(destinationFileName);
            if (fs.existsSync(directoryName)) {
                __writeFile();
                return;
            }

            mkdirp(directoryName, (err) => {
                if (err) {
                    __rejectAndLog(err);
                } else {
                    __writeFile();
                }
            });

            function __writeFile() {
                writeIntoFile(readStream, destinationFileName, reject);
                numberOfFiles++;
            }
        });
    }

    function __rejectAndLog(err) {
        logger.info(err);
        reject(err);
    }
});
  • entry-processing function is extracted to make the zipFile flow more obvious
  • __ prefix is used for subfunctions following your naming of _unzip
\$\endgroup\$
  • \$\begingroup\$ Thanks I've tried your proposal and I'm getting error TypeError: Cannot read property 'openReadStream' of undefined this is since it doesn't have the zipFile entry , any idea? \$\endgroup\$ – Jenny M Nov 13 '16 at 9:52
  • \$\begingroup\$ Well, obviously I overlooked zipFile variable scope. I've fixed the code, but you really should be able to see what is the issue, moreover my answer was an example not intended to be used as-is because I didn't test it. \$\endgroup\$ – wOxxOm Nov 13 '16 at 13:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.