diff --git a/package-lock.json b/package-lock.json index 0a32cc9..87f6a5b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4541,6 +4541,11 @@ "integrity": "sha512-NuHqBY1PB/D8xU6s/thBgOAiAP7HOYDQ32+BFZILJ8ivkUkAHQnWfn6WhL79Owj1qmUnoN/YPhktdIoucipkAQ==", "dev": true }, + "@zip.js/zip.js": { + "version": "2.4.6", + "resolved": "https://registry.npmjs.org/@zip.js/zip.js/-/zip.js-2.4.6.tgz", + "integrity": "sha512-gP13tvMy1bhaTWw5I/Sm3mJAOU7J8S18e4sAcscGzYY8NVUF8FRirfY17eYq+rZhRBk8SNg5bFzzWgFR47qSyw==" + }, "JSONStream": { "version": "1.3.5", "resolved": "https://registry.npmjs.org/JSONStream/-/JSONStream-1.3.5.tgz", @@ -15891,27 +15896,6 @@ "object.assign": "^4.1.0" } }, - "jszip": { - "version": "3.7.1", - "resolved": "https://registry.npmjs.org/jszip/-/jszip-3.7.1.tgz", - "integrity": "sha512-ghL0tz1XG9ZEmRMcEN2vt7xabrDdqHHeykgARpmZ0BiIctWxM47Vt63ZO2dnp4QYt/xJVLLy5Zv1l/xRdh2byg==", - "requires": { - "lie": "~3.3.0", - "pako": "~1.0.2", - "readable-stream": "~2.3.6", - "set-immediate-shim": "~1.0.1" - }, - "dependencies": { - "lie": { - "version": "3.3.0", - "resolved": "https://registry.npmjs.org/lie/-/lie-3.3.0.tgz", - "integrity": "sha512-UaiMJzeWRlEujzAuw5LokY1L5ecNQYZKfmyZ9L7wDHb/p5etKaxXhohBcrw0EYby+G/NA52vRSN4N39dxHAIwQ==", - "requires": { - "immediate": "~3.0.5" - } - } - } - }, "junk": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/junk/-/junk-3.1.0.tgz", @@ -19689,11 +19673,6 @@ "resolved": "https://registry.npmjs.org/p-try/-/p-try-2.2.0.tgz", "integrity": "sha512-R4nPAVTAU0B9D35/Gk3uJf/7XYbQcyohSKdvAxIRSNghFl4e71hVoGnBNQz9cWaXxO2I10KTC+3jMdvvoKw6dQ==" }, - "pako": { - "version": "1.0.11", - "resolved": "https://registry.npmjs.org/pako/-/pako-1.0.11.tgz", - "integrity": "sha512-4hLB8Py4zZce5s4yd9XzopqwVv/yGNhV1Bl8NTmCq1763HeK2+EwVTv+leGeL13Dnh2wfbqowVPXCIO0z4taYw==" - }, "param-case": { "version": "3.0.4", "resolved": "https://registry.npmjs.org/param-case/-/param-case-3.0.4.tgz", @@ -22597,11 +22576,6 @@ "resolved": "https://registry.npmjs.org/set-blocking/-/set-blocking-2.0.0.tgz", "integrity": "sha1-BF+XgtARrppoA93TgrJDkrPYkPc=" }, - "set-immediate-shim": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/set-immediate-shim/-/set-immediate-shim-1.0.1.tgz", - "integrity": "sha1-SysbJ+uAip+NzEgaWOXlb1mfP2E=" - }, "set-value": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/set-value/-/set-value-2.0.1.tgz", diff --git a/package.json b/package.json index d3c5908..df40667 100755 --- a/package.json +++ b/package.json @@ -37,6 +37,7 @@ "@redux-beacon/segment": "^1.1.0", "@reduxjs/toolkit": "^1.6.1", "@testing-library/user-event": "^13.5.0", + "@zip.js/zip.js": "^2.4.6", "axios": "^0.21.4", "classnames": "^2.3.1", "core-js": "3.16.2", @@ -49,7 +50,6 @@ "font-awesome": "4.7.0", "history": "5.0.1", "html-react-parser": "^1.3.0", - "jszip": "^3.7.1", "lodash": "^4.17.21", "node-sass": "^6.0.1", "prop-types": "15.7.2", diff --git a/src/data/redux/thunkActions/download.js b/src/data/redux/thunkActions/download.js index 2a43dd9..3c1f822 100644 --- a/src/data/redux/thunkActions/download.js +++ b/src/data/redux/thunkActions/download.js @@ -1,4 +1,4 @@ -import JSZip from 'jszip'; +import * as zip from '@zip.js/zip.js'; import FileSaver from 'file-saver'; import { StrictDict } from 'utils'; @@ -36,13 +36,21 @@ export const zipFileName = () => { * @param {blob[]} blobs - file content blobs * @return {Promise} - zip async process promise. */ -export const zipFiles = (files, blobs) => { - const zip = new JSZip(); - zip.file('manifest.txt', module.genManifest(files)); - blobs.forEach((blob, i) => zip.file(files[i].name, blob)); - return zip.generateAsync({ type: 'blob' }).then( - zipFile => FileSaver.saveAs(zipFile, module.zipFileName()), - ); +export const zipFiles = async (files, blobs) => { + const zipWriter = new zip.ZipWriter(new zip.BlobWriter('application/zip')); + await zipWriter.add('manifest.txt', new zip.TextReader(module.genManifest(files))); + + // forEach or map will create additional thread. It is less readable if we create more + // promise or async function just to circumvent that. + for (let i = 0; i < blobs.length; i++) { + // eslint-disable-next-line no-await-in-loop + await zipWriter.add(files[i].name, new zip.BlobReader(blobs[i]), { + bufferedWrite: true, + }); + } + + const zipFile = await zipWriter.close(); + FileSaver.saveAs(zipFile, module.zipFileName()); }; /** @@ -73,7 +81,7 @@ export const downloadFiles = () => (dispatch, getState) => { if (blobs.some(blob => blob === null)) { throw Error(ERRORS.fetchFailed); } - module.zipFiles(files, blobs); + return module.zipFiles(files, blobs); }), })); }; diff --git a/src/data/redux/thunkActions/download.test.js b/src/data/redux/thunkActions/download.test.js index c0b21ca..fb0d4d3 100644 --- a/src/data/redux/thunkActions/download.test.js +++ b/src/data/redux/thunkActions/download.test.js @@ -1,21 +1,34 @@ -import JSZip from 'jszip'; +import * as zip from '@zip.js/zip.js'; import FileSaver from 'file-saver'; import { selectors } from 'data/redux'; import { RequestKeys } from 'data/constants/requests'; import * as download from './download'; +const mockBlobWriter = jest.fn().mockName('BlobWriter'); +const mockTextReader = jest.fn().mockName('TextReader'); +const mockBlobReader = jest.fn().mockName('BlobReader'); + +const mockZipAdd = jest.fn(); +const mockZipClose = jest.fn(); + +jest.mock('@zip.js/zip.js', () => { + const files = []; + return { + ZipWriter: jest.fn().mockImplementation(() => ({ + add: mockZipAdd.mockImplementation((file, content) => files.push([file, content])), + close: mockZipClose.mockImplementation(() => Promise.resolve(files)), + files, + })), + BlobWriter: () => mockBlobWriter, + TextReader: () => mockTextReader, + BlobReader: () => mockBlobReader, + }; +}); + jest.mock('file-saver', () => ({ saveAs: jest.fn(), })); -jest.mock('jszip', () => { - const file = jest.fn(); - const zipFile = 'test zip output'; - const generateAsync = jest.fn(() => new Promise((resolve) => resolve(zipFile))); - return function zip() { - return { file, zipFile, generateAsync }; - }; -}); jest.mock('./requests', () => ({ networkRequest: (args) => ({ networkRequest: args }), @@ -44,10 +57,12 @@ describe('download thunkActions', () => { const getState = () => testState; describe('genManifest', () => { test('returns a list of strings with filename and description for each file', () => { - expect(download.genManifest(response.files)).toEqual([ - `Filename: ${files[0].name}\nDescription: ${files[0].description}\nSize: ${files[0].size}`, - `Filename: ${files[1].name}\nDescription: ${files[1].description}\nSize: ${files[0].size}`, - ].join('\n\n')); + expect(download.genManifest(response.files)).toEqual( + [ + `Filename: ${files[0].name}\nDescription: ${files[0].description}\nSize: ${files[0].size}`, + `Filename: ${files[1].name}\nDescription: ${files[1].description}\nSize: ${files[0].size}`, + ].join('\n\n'), + ); }); }); describe('zipFileName', () => { @@ -55,18 +70,16 @@ describe('download thunkActions', () => { }); describe('zipFiles', () => { test('zips files and manifest', () => { - const mockZip = new JSZip(); - const mockFilename = 'mock-filename'; - download.genManifest = (testFiles) => ({ genManifest: testFiles }); - download.zipFileName = () => mockFilename; + const mockZipWriter = new zip.ZipWriter(); return download.zipFiles(files, blobs).then(() => { - expect(mockZip.file.mock.calls).toEqual([ - ['manifest.txt', download.genManifest(files)], - [files[0].name, blobs[0]], - [files[1].name, blobs[1]], + expect(mockZipWriter.files).toEqual([ + ['manifest.txt', mockTextReader], + [files[0].name, mockBlobReader], + [files[1].name, mockBlobReader], ]); - expect(mockZip.generateAsync).toHaveBeenCalledWith({ type: 'blob' }); - expect(FileSaver.saveAs).toHaveBeenCalledWith(mockZip.zipFile, mockFilename); + expect(mockZipAdd).toHaveBeenCalledTimes(mockZipWriter.files.length); + expect(mockZipClose).toHaveBeenCalledTimes(1); + expect(FileSaver.saveAs).toHaveBeenCalledTimes(1); }); }); }); @@ -82,20 +95,28 @@ describe('download thunkActions', () => { window.fetch = fetch; }); it('returns blob output if successful', () => { - window.fetch.mockReturnValue(new Promise(resolve => resolve({ ok: true, blob: () => blob }))); - return download.downloadFile(files[0]).then(val => expect(val).toEqual(blob)); + window.fetch.mockReturnValue( + new Promise((resolve) => resolve({ ok: true, blob: () => blob })), + ); + return download + .downloadFile(files[0]) + .then((val) => expect(val).toEqual(blob)); }); it('returns null if not successful', () => { - window.fetch.mockReturnValue(new Promise(resolve => resolve({ ok: false }))); - return download.downloadFile(files[0]).then(val => expect(val).toEqual(null)); + window.fetch.mockReturnValue( + new Promise((resolve) => resolve({ ok: false })), + ); + return download + .downloadFile(files[0]) + .then((val) => expect(val).toEqual(null)); }); }); describe('downloadBlobs', () => { it('returns a joing promise mapping all files to download action', async () => { - download.downloadFile = (file) => new Promise(resolve => resolve(file.name)); + download.downloadFile = (file) => new Promise((resolve) => resolve(file.name)); const responses = await download.downloadBlobs(files); - expect(responses).toEqual(files.map(file => file.name)); + expect(responses).toEqual(files.map((file) => file.name)); }); }); @@ -103,28 +124,27 @@ describe('download thunkActions', () => { beforeEach(() => { dispatch = jest.fn(); selectors.grading.selected.response = () => ({ files }); + module.zipFiles = jest.fn(); }); it('dispatches network request with downloadFiles key', () => { - download.downloadBlobs = () => new Promise(resolve => resolve(blobs)); + module.downloadBlobs = () => new Promise((resolve) => resolve(blobs)); download.downloadFiles()(dispatch, getState); const { networkRequest } = dispatch.mock.calls[0][0]; expect(networkRequest.requestKey).toEqual(RequestKeys.downloadFiles); }); it('dispatches network request for downloadFiles, zipping output of downloadBlobs', () => { - download.downloadBlobs = () => new Promise(resolve => resolve(blobs)); - const zipSpy = jest.spyOn(download, 'zipFiles').mockImplementation(() => ({})); + module.downloadBlobs = () => new Promise((resolve) => resolve(blobs)); download.downloadFiles()(dispatch, getState); const { networkRequest } = dispatch.mock.calls[0][0]; - return networkRequest.promise.then(() => { - expect(zipSpy).toHaveBeenCalledWith(files, blobs); + networkRequest.promise.then(() => { + expect(module.zipFile).toHaveBeenCalledWith(files, blobs); }); }); - it('throws an error if any of the blobs fail to download', () => { - jest.spyOn(download, 'zipFiles').mockImplementation(() => ({})); - download.downloadBlobs = () => new Promise((resolve) => resolve([1, null, 2])); + it('throws an error on failure', () => { + module.downloadBlobs = () => new Promise((resolve, reject) => reject()); download.downloadFiles()(dispatch, getState); const { networkRequest } = dispatch.mock.calls[0][0]; - return expect(networkRequest.promise).rejects.toThrow(download.ERRORS.fetchFailed); + expect(networkRequest.promise).rejects.toThrow('Fetch failed'); }); }); }); diff --git a/src/setupTest.js b/src/setupTest.js index d0eb257..97f697b 100755 --- a/src/setupTest.js +++ b/src/setupTest.js @@ -106,3 +106,5 @@ jest.mock('@edx/paragon/icons', () => ({ jest.mock('data/constants/app', () => ({ locationId: 'fake-location-id', })); + +jest.mock('@zip.js/zip.js', () => ({}));