feat: make zip support larger files

chore: improve code readability
This commit is contained in:
Leangseu Kim
2022-03-15 09:49:58 -04:00
committed by leangseu-edx
parent 876f5d9413
commit d033fa310d
5 changed files with 83 additions and 79 deletions

36
package-lock.json generated
View File

@@ -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",

View File

@@ -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",

View File

@@ -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);
}),
}));
};

View File

@@ -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');
});
});
});

View File

@@ -106,3 +106,5 @@ jest.mock('@edx/paragon/icons', () => ({
jest.mock('data/constants/app', () => ({
locationId: 'fake-location-id',
}));
jest.mock('@zip.js/zip.js', () => ({}));