mirror of
https://github.com/outline/outline.git
synced 2026-08-03 13:27:25 +03:00
perf: Attach output stream before adding zip entries (#13167)
* perf: Attach output stream before adding zip entries * fix: Handle failure
This commit is contained in:
+1
-1
@@ -345,7 +345,7 @@
|
||||
"@types/utf8": "^3.0.3",
|
||||
"@types/validator": "^13.15.10",
|
||||
"@types/yauzl": "^2.10.3",
|
||||
"@types/yazl": "^2.4.6",
|
||||
"@types/yazl": "^3.3.1",
|
||||
"@vitest/ui": "^4.1.8",
|
||||
"browserslist-to-esbuild": "^1.2.0",
|
||||
"concurrently": "^8.2.2",
|
||||
|
||||
@@ -30,24 +30,46 @@ export default class HTMLHelper {
|
||||
contentType: string | null | undefined,
|
||||
buffer: Buffer
|
||||
): string | null {
|
||||
if (!contentType?.startsWith("image/")) {
|
||||
return null;
|
||||
}
|
||||
if (buffer.length === 0 || buffer.length > HTMLHelper.maxInlineImageSize) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const pattern = new RegExp(escapeRegExp(redirectUrl), "g");
|
||||
if ((html.match(pattern)?.length ?? 0) !== 1) {
|
||||
if (
|
||||
!HTMLHelper.canInlineImage(html, redirectUrl, contentType, buffer.length)
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return html.replace(
|
||||
pattern,
|
||||
new RegExp(escapeRegExp(redirectUrl), "g"),
|
||||
`data:${contentType};base64,${buffer.toString("base64")}`
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether an image meets the criteria for inlining, determined from its
|
||||
* size alone so that callers can avoid reading images into memory that
|
||||
* would be written to the export as an external file regardless.
|
||||
*
|
||||
* @param html The HTML content referencing the image.
|
||||
* @param redirectUrl The redirect URL of the image within the HTML.
|
||||
* @param contentType The content type of the image, e.g. "image/png".
|
||||
* @param size The size of the image in bytes.
|
||||
* @returns True if the image is a candidate for inlining.
|
||||
*/
|
||||
public static canInlineImage(
|
||||
html: string,
|
||||
redirectUrl: string,
|
||||
contentType: string | null | undefined,
|
||||
size: number
|
||||
): boolean {
|
||||
if (!contentType?.startsWith("image/")) {
|
||||
return false;
|
||||
}
|
||||
if (size === 0 || size > HTMLHelper.maxInlineImageSize) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const pattern = new RegExp(escapeRegExp(redirectUrl), "g");
|
||||
return (html.match(pattern)?.length ?? 0) === 1;
|
||||
}
|
||||
|
||||
/**
|
||||
* Move CSS styles from <style> tags to inline styles with default settings.
|
||||
*
|
||||
|
||||
@@ -84,39 +84,50 @@ export default abstract class ExportDocumentTreeTask extends ExportTask {
|
||||
}
|
||||
|
||||
const dir = path.dirname(pathInZip);
|
||||
let buffer: Buffer;
|
||||
try {
|
||||
buffer = await attachment.buffer;
|
||||
} catch (err) {
|
||||
Logger.warn(`Failed to read attachment from storage`, {
|
||||
attachmentId: attachment.id,
|
||||
teamId: attachment.teamId,
|
||||
error: errToString(err),
|
||||
});
|
||||
buffer = Buffer.from("");
|
||||
}
|
||||
|
||||
// Inline small images referenced a single time as base64 data URIs
|
||||
// rather than writing an external file. PDF export renders from HTML too.
|
||||
// Only these are read into memory; the rest are streamed into the archive.
|
||||
if (
|
||||
format === FileOperationFormat.HTMLZip ||
|
||||
format === FileOperationFormat.PDF
|
||||
) {
|
||||
const inlined = HTMLHelper.inlineImage(
|
||||
(format === FileOperationFormat.HTMLZip ||
|
||||
format === FileOperationFormat.PDF) &&
|
||||
HTMLHelper.canInlineImage(
|
||||
text,
|
||||
attachment.redirectUrl,
|
||||
attachment.contentType,
|
||||
buffer
|
||||
);
|
||||
attachment.size
|
||||
)
|
||||
) {
|
||||
let buffer: Buffer | undefined;
|
||||
try {
|
||||
buffer = await attachment.buffer;
|
||||
} catch (err) {
|
||||
Logger.warn(`Failed to read attachment from storage`, {
|
||||
attachmentId: attachment.id,
|
||||
teamId: attachment.teamId,
|
||||
error: errToString(err),
|
||||
});
|
||||
}
|
||||
|
||||
const inlined = buffer
|
||||
? HTMLHelper.inlineImage(
|
||||
text,
|
||||
attachment.redirectUrl,
|
||||
attachment.contentType,
|
||||
buffer
|
||||
)
|
||||
: null;
|
||||
if (inlined !== null) {
|
||||
text = inlined;
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
zip.addBuffer(buffer, path.join(dir, attachment.key), {
|
||||
mtime: attachment.updatedAt,
|
||||
});
|
||||
this.addAttachmentToArchive(
|
||||
zip,
|
||||
attachment,
|
||||
path.join(dir, attachment.key)
|
||||
);
|
||||
|
||||
text = text.replace(
|
||||
new RegExp(escapeRegExp(attachment.redirectUrl), "g"),
|
||||
@@ -165,33 +176,30 @@ export default abstract class ExportDocumentTreeTask extends ExportTask {
|
||||
* @returns The path to the zip file in tmp.
|
||||
*/
|
||||
protected async addCollectionsToArchive(
|
||||
zip: ZipFile,
|
||||
collections: Collection[],
|
||||
format: FileOperationFormat,
|
||||
includeAttachments = true
|
||||
) {
|
||||
const pathMap = this.createPathMap(collections, format);
|
||||
await this.addDocumentsToArchive({
|
||||
zip,
|
||||
pathMap,
|
||||
format,
|
||||
includeAttachments,
|
||||
});
|
||||
|
||||
return await ZipHelper.toTmpFile(zip);
|
||||
return await ZipHelper.toTmpFile((zip) =>
|
||||
this.addDocumentsToArchive({
|
||||
zip,
|
||||
pathMap,
|
||||
format,
|
||||
includeAttachments,
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
protected async addDocumentToArchive({
|
||||
document,
|
||||
format,
|
||||
documentStructure,
|
||||
zip,
|
||||
includeAttachments = true,
|
||||
}: {
|
||||
document: Document;
|
||||
format: FileOperationFormat;
|
||||
documentStructure: NavigationNode[];
|
||||
zip: ZipFile;
|
||||
includeAttachments?: boolean;
|
||||
}) {
|
||||
const pathMap = new Map<string, string>();
|
||||
@@ -209,14 +217,14 @@ export default abstract class ExportDocumentTreeTask extends ExportTask {
|
||||
format
|
||||
);
|
||||
|
||||
await this.addDocumentsToArchive({
|
||||
zip,
|
||||
pathMap,
|
||||
format,
|
||||
includeAttachments,
|
||||
});
|
||||
|
||||
return await ZipHelper.toTmpFile(zip);
|
||||
return await ZipHelper.toTmpFile((zip) =>
|
||||
this.addDocumentsToArchive({
|
||||
zip,
|
||||
pathMap,
|
||||
format,
|
||||
includeAttachments,
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
import { ZipFile } from "yazl";
|
||||
import type { NavigationNode } from "@shared/types";
|
||||
import { FileOperationFormat } from "@shared/types";
|
||||
import type { Collection, FileOperation } from "@server/models";
|
||||
@@ -10,10 +9,7 @@ export default class ExportHTMLZipTask extends ExportDocumentTreeTask {
|
||||
collections: Collection[],
|
||||
fileOperation: FileOperation
|
||||
) {
|
||||
const zip = new ZipFile();
|
||||
|
||||
return await this.addCollectionsToArchive(
|
||||
zip,
|
||||
collections,
|
||||
FileOperationFormat.HTMLZip,
|
||||
fileOperation.options?.includeAttachments ?? true
|
||||
@@ -25,13 +21,10 @@ export default class ExportHTMLZipTask extends ExportDocumentTreeTask {
|
||||
documentStructure: NavigationNode[],
|
||||
includeAttachments: boolean
|
||||
): Promise<string> {
|
||||
const zip = new ZipFile();
|
||||
|
||||
return await this.addDocumentToArchive({
|
||||
document,
|
||||
documentStructure,
|
||||
format: FileOperationFormat.HTMLZip,
|
||||
zip,
|
||||
includeAttachments,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -1,9 +1,7 @@
|
||||
import { ZipFile } from "yazl";
|
||||
import type { ZipFile } from "yazl";
|
||||
import { omit } from "es-toolkit/compat";
|
||||
import { errToString } from "@shared/utils/error";
|
||||
import type { NavigationNode } from "@shared/types";
|
||||
import env from "@server/env";
|
||||
import Logger from "@server/logging/Logger";
|
||||
import type { Collection, FileOperation } from "@server/models";
|
||||
import { Attachment, Document } from "@server/models";
|
||||
import { DocumentHelper } from "@server/models/helpers/DocumentHelper";
|
||||
@@ -20,29 +18,28 @@ export default class ExportJSONTask extends ExportTask {
|
||||
collections: Collection[],
|
||||
fileOperation: FileOperation
|
||||
) {
|
||||
const zip = new ZipFile();
|
||||
const usedFilenames = new Set<string>();
|
||||
|
||||
// serial to avoid overloading, slow and steady wins the race
|
||||
for (const collection of collections) {
|
||||
let filename = serializeFilename(collection.name);
|
||||
let i = 0;
|
||||
while (usedFilenames.has(filename)) {
|
||||
filename = `${serializeFilename(collection.name)} (${++i})`;
|
||||
return ZipHelper.toTmpFile(async (zip) => {
|
||||
// serial to avoid overloading, slow and steady wins the race
|
||||
for (const collection of collections) {
|
||||
let filename = serializeFilename(collection.name);
|
||||
let i = 0;
|
||||
while (usedFilenames.has(filename)) {
|
||||
filename = `${serializeFilename(collection.name)} (${++i})`;
|
||||
}
|
||||
usedFilenames.add(filename);
|
||||
|
||||
await this.addCollectionToArchive(
|
||||
zip,
|
||||
collection,
|
||||
fileOperation.options?.includeAttachments ?? true,
|
||||
filename
|
||||
);
|
||||
}
|
||||
usedFilenames.add(filename);
|
||||
|
||||
await this.addCollectionToArchive(
|
||||
zip,
|
||||
collection,
|
||||
fileOperation.options?.includeAttachments ?? true,
|
||||
filename
|
||||
);
|
||||
}
|
||||
|
||||
await this.addMetadataToArchive(zip, fileOperation);
|
||||
|
||||
return ZipHelper.toTmpFile(zip);
|
||||
await this.addMetadataToArchive(zip, fileOperation);
|
||||
});
|
||||
}
|
||||
|
||||
private async addMetadataToArchive(
|
||||
@@ -87,31 +84,18 @@ export default class ExportJSONTask extends ExportTask {
|
||||
attachments: {},
|
||||
};
|
||||
|
||||
async function addAttachments(attachments: Attachment[]) {
|
||||
const addAttachments = (attachments: Attachment[]) => {
|
||||
for (const attachment of attachments) {
|
||||
let buffer: Buffer;
|
||||
try {
|
||||
buffer = await attachment.buffer;
|
||||
} catch (err) {
|
||||
Logger.warn(`Failed to read attachment from storage`, {
|
||||
attachmentId: attachment.id,
|
||||
teamId: attachment.teamId,
|
||||
error: errToString(err),
|
||||
});
|
||||
buffer = Buffer.from("");
|
||||
}
|
||||
zip.addBuffer(buffer, attachment.key, {
|
||||
mtime: attachment.updatedAt,
|
||||
});
|
||||
this.addAttachmentToArchive(zip, attachment, attachment.key);
|
||||
|
||||
output.attachments[attachment.id] = {
|
||||
...omit(presentAttachment(attachment), "url"),
|
||||
key: attachment.key,
|
||||
};
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
async function addDocumentTree(nodes: NavigationNode[]) {
|
||||
const addDocumentTree = async (nodes: NavigationNode[]) => {
|
||||
for (const node of nodes) {
|
||||
const document = await Document.findByPk(node.id, {
|
||||
includeState: true,
|
||||
@@ -132,7 +116,7 @@ export default class ExportJSONTask extends ExportTask {
|
||||
})
|
||||
: [];
|
||||
|
||||
await addAttachments(documentAttachments);
|
||||
addAttachments(documentAttachments);
|
||||
|
||||
output.documents[document.id] = {
|
||||
id: document.id,
|
||||
@@ -157,7 +141,7 @@ export default class ExportJSONTask extends ExportTask {
|
||||
await addDocumentTree(node.children);
|
||||
}
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
const collectionAttachments = includeAttachments
|
||||
? await Attachment.findAll({
|
||||
@@ -170,7 +154,7 @@ export default class ExportJSONTask extends ExportTask {
|
||||
})
|
||||
: [];
|
||||
|
||||
await addAttachments(collectionAttachments);
|
||||
addAttachments(collectionAttachments);
|
||||
|
||||
if (collection.documentStructure) {
|
||||
await addDocumentTree(collection.documentStructure);
|
||||
|
||||
@@ -1,8 +1,12 @@
|
||||
import { Readable } from "node:stream";
|
||||
import fs from "fs-extra";
|
||||
import { vi } from "vitest";
|
||||
import FileStorage from "@server/storage/files";
|
||||
import ZipHelper from "@server/utils/ZipHelper";
|
||||
import {
|
||||
buildCollection,
|
||||
buildDocument,
|
||||
buildDocumentWithAttachment,
|
||||
buildFileOperation,
|
||||
buildTeam,
|
||||
buildUser,
|
||||
@@ -10,6 +14,10 @@ import {
|
||||
import ExportMarkdownZipTask from "./ExportMarkdownZipTask";
|
||||
|
||||
describe("ExportMarkdownZipTask", () => {
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("should not duplicate documents in the zip file", async () => {
|
||||
const team = await buildTeam();
|
||||
const user = await buildUser({ teamId: team.id });
|
||||
@@ -58,4 +66,88 @@ describe("ExportMarkdownZipTask", () => {
|
||||
await fs.remove(filePath);
|
||||
}
|
||||
});
|
||||
|
||||
it("should stream attachment contents into the zip file", async () => {
|
||||
const { collection, attachment, fileOperation } =
|
||||
await buildDocumentWithAttachment();
|
||||
|
||||
const getFileStream = vi
|
||||
.spyOn(FileStorage, "getFileStream")
|
||||
.mockResolvedValue(Readable.from(["image-", "bytes"]));
|
||||
|
||||
const task = new ExportMarkdownZipTask();
|
||||
const filePath = await task.exportCollections([collection], fileOperation);
|
||||
|
||||
try {
|
||||
const contents = await readZipContents(filePath);
|
||||
expect(contents[`${collection.name}/${attachment.key}`]).toBe(
|
||||
"image-bytes"
|
||||
);
|
||||
expect(getFileStream).toHaveBeenCalledWith(attachment.key);
|
||||
} finally {
|
||||
await fs.remove(filePath);
|
||||
}
|
||||
});
|
||||
|
||||
it("should fail the export when an attachment cannot be read", async () => {
|
||||
const { collection, fileOperation } = await buildDocumentWithAttachment();
|
||||
|
||||
vi.spyOn(FileStorage, "getFileStream").mockRejectedValue(
|
||||
new Error("storage unavailable")
|
||||
);
|
||||
|
||||
const task = new ExportMarkdownZipTask();
|
||||
await expect(
|
||||
task.exportCollections([collection], fileOperation)
|
||||
).rejects.toThrow("storage unavailable");
|
||||
});
|
||||
|
||||
it("should fail the export when an attachment read is interrupted", async () => {
|
||||
const { collection, fileOperation } = await buildDocumentWithAttachment();
|
||||
|
||||
vi.spyOn(FileStorage, "getFileStream").mockResolvedValue(
|
||||
new Readable({
|
||||
read() {
|
||||
this.push("partial");
|
||||
this.destroy(new Error("connection reset"));
|
||||
},
|
||||
})
|
||||
);
|
||||
|
||||
const task = new ExportMarkdownZipTask();
|
||||
await expect(
|
||||
task.exportCollections([collection], fileOperation)
|
||||
).rejects.toThrow("connection reset");
|
||||
});
|
||||
|
||||
it("should write an empty entry when an attachment is missing from storage", async () => {
|
||||
const { collection, attachment, fileOperation } =
|
||||
await buildDocumentWithAttachment();
|
||||
|
||||
vi.spyOn(FileStorage, "getFileStream").mockResolvedValue(null);
|
||||
|
||||
const task = new ExportMarkdownZipTask();
|
||||
const filePath = await task.exportCollections([collection], fileOperation);
|
||||
|
||||
try {
|
||||
const contents = await readZipContents(filePath);
|
||||
expect(contents[`${collection.name}/${attachment.key}`]).toBe("");
|
||||
} finally {
|
||||
await fs.remove(filePath);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
async function readZipContents(
|
||||
filePath: string
|
||||
): Promise<Record<string, string>> {
|
||||
const contents: Record<string, string> = {};
|
||||
await ZipHelper.walk(filePath, async (entry) => {
|
||||
if (!entry.isDirectory) {
|
||||
contents[entry.fileName] = (await entry.readBuffer(1024 * 1024)).toString(
|
||||
"utf8"
|
||||
);
|
||||
}
|
||||
});
|
||||
return contents;
|
||||
}
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
import { ZipFile } from "yazl";
|
||||
import type { NavigationNode } from "@shared/types";
|
||||
import { FileOperationFormat } from "@shared/types";
|
||||
import type { Collection, FileOperation } from "@server/models";
|
||||
@@ -10,10 +9,7 @@ export default class ExportMarkdownZipTask extends ExportDocumentTreeTask {
|
||||
collections: Collection[],
|
||||
fileOperation: FileOperation
|
||||
) {
|
||||
const zip = new ZipFile();
|
||||
|
||||
return await this.addCollectionsToArchive(
|
||||
zip,
|
||||
collections,
|
||||
FileOperationFormat.MarkdownZip,
|
||||
fileOperation.options?.includeAttachments
|
||||
@@ -25,13 +21,10 @@ export default class ExportMarkdownZipTask extends ExportDocumentTreeTask {
|
||||
documentStructure: NavigationNode[],
|
||||
includeAttachments: boolean
|
||||
): Promise<string> {
|
||||
const zip = new ZipFile();
|
||||
|
||||
return await this.addDocumentToArchive({
|
||||
document,
|
||||
documentStructure,
|
||||
format: FileOperationFormat.MarkdownZip,
|
||||
zip,
|
||||
includeAttachments,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
import { Readable } from "node:stream";
|
||||
import fs from "fs-extra";
|
||||
import { truncate } from "es-toolkit/compat";
|
||||
import type { ZipFile } from "yazl";
|
||||
import { toError } from "@shared/utils/error";
|
||||
import type { NavigationNode } from "@shared/types";
|
||||
import { FileOperationState, NotificationEventType } from "@shared/types";
|
||||
@@ -221,6 +223,49 @@ export default abstract class ExportTask extends BaseTask<Props> {
|
||||
includeAttachments: boolean
|
||||
): Promise<string>;
|
||||
|
||||
/**
|
||||
* Add an attachment to the archive, streaming its contents from storage as
|
||||
* the archive is written so that the file is never held in memory whole.
|
||||
*
|
||||
* A read that fails, or is interrupted part way through, fails the export —
|
||||
* a silently truncated archive is worse than none, as it looks complete.
|
||||
* An attachment that is simply absent from storage is written as an empty
|
||||
* entry, since a row outliving its file must not make export impossible.
|
||||
*
|
||||
* @param zip The archive to add the attachment to
|
||||
* @param attachment The attachment to add
|
||||
* @param pathInZip The path to store the attachment at within the archive
|
||||
*/
|
||||
protected addAttachmentToArchive(
|
||||
zip: ZipFile,
|
||||
attachment: Attachment,
|
||||
pathInZip: string
|
||||
) {
|
||||
zip.addReadStreamLazy(
|
||||
pathInZip,
|
||||
{ mtime: attachment.updatedAt },
|
||||
(callback) => {
|
||||
attachment.stream.then(
|
||||
(stream) => {
|
||||
if (!stream) {
|
||||
Logger.warn(`Attachment is missing from storage`, {
|
||||
attachmentId: attachment.id,
|
||||
teamId: attachment.teamId,
|
||||
});
|
||||
return callback(null, Readable.from([]));
|
||||
}
|
||||
|
||||
// yazl does not watch the streams it is handed, so an interrupted
|
||||
// read would otherwise stall the archive rather than fail it.
|
||||
stream.on("error", (err) => zip.emit("error", toError(err)));
|
||||
return callback(null, stream);
|
||||
},
|
||||
(err) => callback(toError(err), Readable.from([]))
|
||||
);
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Update the state of the underlying FileOperation in the database and send
|
||||
* an event to the client.
|
||||
|
||||
@@ -638,6 +638,35 @@ export async function buildAttachment(
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Build a collection holding one document that references one attachment,
|
||||
* along with a file operation to export it with.
|
||||
*
|
||||
* @param overrides Optional team and user to build the records under.
|
||||
* @returns the created collection, document, attachment and file operation.
|
||||
*/
|
||||
export async function buildDocumentWithAttachment(
|
||||
overrides: { teamId?: string; userId?: string } = {}
|
||||
) {
|
||||
const teamId = overrides.teamId ?? (await buildTeam()).id;
|
||||
const userId = overrides.userId ?? (await buildUser({ teamId })).id;
|
||||
|
||||
const collection = await buildCollection({ teamId, createdById: userId });
|
||||
const attachment = await buildAttachment({ teamId, userId });
|
||||
const document = await buildDocument({
|
||||
teamId,
|
||||
userId,
|
||||
collectionId: collection.id,
|
||||
title: "Test",
|
||||
text: ``,
|
||||
});
|
||||
await collection.addDocumentToStructure(document);
|
||||
|
||||
const fileOperation = await buildFileOperation({ teamId, userId });
|
||||
|
||||
return { collection, document, attachment, fileOperation };
|
||||
}
|
||||
|
||||
export async function buildEmoji(
|
||||
overrides: Partial<Emoji> = {}
|
||||
): Promise<Emoji> {
|
||||
|
||||
Vendored
+5
@@ -2,6 +2,11 @@ import "yazl";
|
||||
|
||||
declare module "yazl" {
|
||||
interface Options {
|
||||
/**
|
||||
* Upstream types only declare `fileComment` for entries added from a file
|
||||
* or stream, and only as a string. yazl reads it for every entry type —
|
||||
* `addBuffer` included — and accepts a Buffer as well.
|
||||
*/
|
||||
fileComment: string | Buffer;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
import { Readable } from "node:stream";
|
||||
import { pipeline } from "node:stream/promises";
|
||||
import fs from "fs-extra";
|
||||
import tmp from "tmp";
|
||||
import { vi } from "vitest";
|
||||
import { ZipFile } from "yazl";
|
||||
import ZipHelper from "./ZipHelper";
|
||||
|
||||
@@ -19,6 +21,125 @@ async function writeZip(
|
||||
return zipPath;
|
||||
}
|
||||
|
||||
/**
|
||||
* Watch for the temporary file toTmpFile writes to, so that cleanup can be
|
||||
* asserted against that exact path rather than by scanning the shared temp
|
||||
* directory, which tests running in parallel also write to.
|
||||
*/
|
||||
function watchTmpFile() {
|
||||
const createWriteStream = vi.spyOn(fs, "createWriteStream");
|
||||
return () => {
|
||||
expect(createWriteStream).toHaveBeenCalledTimes(1);
|
||||
return String(createWriteStream.mock.calls[0][0]);
|
||||
};
|
||||
}
|
||||
|
||||
async function readZip(filePath: string): Promise<Record<string, string>> {
|
||||
const contents: Record<string, string> = {};
|
||||
await ZipHelper.walk(filePath, async (entry) => {
|
||||
if (!entry.isDirectory) {
|
||||
contents[entry.fileName] = (await entry.readBuffer(1024)).toString(
|
||||
"utf8"
|
||||
);
|
||||
}
|
||||
});
|
||||
return contents;
|
||||
}
|
||||
|
||||
describe("ZipHelper.toTmpFile", () => {
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("writes buffered and streamed entries to a temporary file", async () => {
|
||||
const filePath = await ZipHelper.toTmpFile(async (zip) => {
|
||||
zip.addBuffer(Buffer.from("hello"), "a.txt");
|
||||
zip.addReadStreamLazy("b.txt", {}, (callback) =>
|
||||
callback(null, Readable.from(["wo", "rld"]))
|
||||
);
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
expect(await readZip(filePath)).toEqual({
|
||||
"a.txt": "hello",
|
||||
"b.txt": "world",
|
||||
});
|
||||
await fs.remove(filePath);
|
||||
});
|
||||
|
||||
it("opens each lazy stream only when its entry is written", async () => {
|
||||
const opened: string[] = [];
|
||||
const filePath = await ZipHelper.toTmpFile(async (zip) => {
|
||||
for (const name of ["a.txt", "b.txt", "c.txt"]) {
|
||||
zip.addReadStreamLazy(name, {}, (callback) => {
|
||||
opened.push(name);
|
||||
callback(null, Readable.from([name]));
|
||||
});
|
||||
}
|
||||
await Promise.resolve();
|
||||
// Nothing is read up front — sources are opened one at a time as the
|
||||
// archive is pumped, so only one file is ever held open.
|
||||
expect(opened.length).toBeLessThan(3);
|
||||
});
|
||||
|
||||
expect(opened).toEqual(["a.txt", "b.txt", "c.txt"]);
|
||||
await fs.remove(filePath);
|
||||
});
|
||||
|
||||
it("propagates errors thrown while adding entries and cleans up", async () => {
|
||||
const tmpFile = watchTmpFile();
|
||||
|
||||
await expect(
|
||||
ZipHelper.toTmpFile(async (zip) => {
|
||||
zip.addBuffer(Buffer.from("partial"), "a.txt");
|
||||
await Promise.resolve();
|
||||
throw new Error("boom");
|
||||
})
|
||||
).rejects.toThrow("boom");
|
||||
|
||||
expect(await fs.pathExists(tmpFile())).toBe(false);
|
||||
});
|
||||
|
||||
it("propagates errors raised when opening an entry and cleans up", async () => {
|
||||
const tmpFile = watchTmpFile();
|
||||
|
||||
await expect(
|
||||
ZipHelper.toTmpFile(async (zip) => {
|
||||
zip.addReadStreamLazy("a.txt", {}, (callback) =>
|
||||
callback(new Error("stream unavailable"), Readable.from([]))
|
||||
);
|
||||
await Promise.resolve();
|
||||
})
|
||||
).rejects.toThrow("stream unavailable");
|
||||
|
||||
expect(await fs.pathExists(tmpFile())).toBe(false);
|
||||
});
|
||||
|
||||
it("fails rather than stalls when an entry's stream errors mid-read", async () => {
|
||||
const tmpFile = watchTmpFile();
|
||||
|
||||
await expect(
|
||||
ZipHelper.toTmpFile(async (zip) => {
|
||||
const source = new Readable({
|
||||
read() {
|
||||
this.push("partial");
|
||||
this.destroy(new Error("connection reset"));
|
||||
},
|
||||
});
|
||||
// Mirrors how ExportTask surfaces an interrupted read, which yazl
|
||||
// does not watch for on the streams it is given.
|
||||
source.on("error", (err) => zip.emit("error", err));
|
||||
zip.addReadStreamLazy("a.txt", {}, (callback) =>
|
||||
callback(null, source)
|
||||
);
|
||||
await Promise.resolve();
|
||||
})
|
||||
).rejects.toThrow("connection reset");
|
||||
|
||||
expect(await fs.pathExists(tmpFile())).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("ZipHelper.toFileTree", () => {
|
||||
it("builds a nested tree with normalized pathInZip", async () => {
|
||||
const zipPath = await writeZip({
|
||||
|
||||
+50
-14
@@ -4,7 +4,7 @@ import fs from "fs-extra";
|
||||
import tmp from "tmp";
|
||||
import type { Entry } from "yauzl";
|
||||
import yauzl, { validateFileName } from "yauzl";
|
||||
import type { ZipFile } from "yazl";
|
||||
import { ZipFile } from "yazl";
|
||||
import { bytesToHumanReadable } from "@shared/utils/files";
|
||||
import { ValidationError } from "@server/errors";
|
||||
import Logger from "@server/logging/Logger";
|
||||
@@ -46,33 +46,56 @@ export default class ZipHelper {
|
||||
/**
|
||||
* Write a zip file to a temporary disk location.
|
||||
*
|
||||
* The caller is responsible for adding entries to the `ZipFile`; this method
|
||||
* calls `end()` and waits for the output stream to drain to disk.
|
||||
* Entries are added by the `addEntries` callback, which receives an archive
|
||||
* that is already draining to disk. Adding entries only after a reader is
|
||||
* attached keeps memory proportional to a single entry rather than to the
|
||||
* size of the whole archive.
|
||||
*
|
||||
* @param zip yazl ZipFile object with entries already added.
|
||||
* @param addEntries Callback that populates the archive.
|
||||
* @returns pathname of the temporary file where the zip was written to disk.
|
||||
* @throws if the archive could not be built or written to disk.
|
||||
*/
|
||||
public static async toTmpFile(zip: ZipFile): Promise<string> {
|
||||
public static async toTmpFile(
|
||||
addEntries: (zip: ZipFile) => Promise<void>
|
||||
): Promise<string> {
|
||||
Logger.debug("utils", "Creating tmp file…");
|
||||
const filePath = await createTmpFile({
|
||||
prefix: "export-",
|
||||
postfix: ".zip",
|
||||
});
|
||||
|
||||
const zip = new ZipFile();
|
||||
const writeStream = fs.createWriteStream(filePath);
|
||||
// yazl reports failures on the archive rather than on its output stream,
|
||||
// so route them into the pipeline to get a single failure path.
|
||||
zip.on("error", (error: Error) => writeStream.destroy(error));
|
||||
|
||||
const writing = pipeline(zip.outputStream, writeStream);
|
||||
// Resolve rather than reject, so that a write failure while entries are
|
||||
// still being added is never an unhandled rejection.
|
||||
const written = writing.then(
|
||||
() => undefined,
|
||||
(error: Error) => error
|
||||
);
|
||||
|
||||
try {
|
||||
const writing = pipeline(
|
||||
zip.outputStream,
|
||||
fs.createWriteStream(filePath)
|
||||
);
|
||||
await addEntries(zip);
|
||||
zip.end();
|
||||
await writing;
|
||||
} catch (error) {
|
||||
await fs.remove(filePath).catch((rmErr) => {
|
||||
Logger.error("Failed to remove tmp file", rmErr);
|
||||
});
|
||||
// Unblock the pipeline, which is still waiting on entries that will now
|
||||
// never arrive.
|
||||
writeStream.destroy();
|
||||
await written;
|
||||
await removeTmpFile(filePath);
|
||||
throw error;
|
||||
}
|
||||
|
||||
const writeError = await written;
|
||||
if (writeError) {
|
||||
await removeTmpFile(filePath);
|
||||
throw writeError;
|
||||
}
|
||||
|
||||
Logger.debug("utils", "Writing zip complete", { path: filePath });
|
||||
return filePath;
|
||||
}
|
||||
@@ -293,12 +316,25 @@ export default class ZipHelper {
|
||||
/**
|
||||
* Promisified wrapper around tmp.file.
|
||||
*
|
||||
* The descriptor tmp opens is discarded, as the file is written through a
|
||||
* separate stream — retaining it would leak a descriptor per call.
|
||||
*
|
||||
* @param options options passed through to tmp.
|
||||
* @returns the path of the created temporary file.
|
||||
*/
|
||||
const createTmpFile = (options: tmp.FileOptions) =>
|
||||
new Promise<string>((resolve, reject) => {
|
||||
tmp.file(options, (err, filePath) =>
|
||||
tmp.file({ ...options, discardDescriptor: true }, (err, filePath) =>
|
||||
err ? reject(err) : resolve(filePath)
|
||||
);
|
||||
});
|
||||
|
||||
/**
|
||||
* Delete a temporary file, logging rather than throwing on failure.
|
||||
*
|
||||
* @param filePath the path of the file to remove.
|
||||
*/
|
||||
const removeTmpFile = (filePath: string) =>
|
||||
fs.remove(filePath).catch((error) => {
|
||||
Logger.error("Failed to remove tmp file", error);
|
||||
});
|
||||
|
||||
@@ -7528,12 +7528,12 @@ __metadata:
|
||||
languageName: node
|
||||
linkType: hard
|
||||
|
||||
"@types/yazl@npm:^2.4.6":
|
||||
version: 2.4.6
|
||||
resolution: "@types/yazl@npm:2.4.6"
|
||||
"@types/yazl@npm:^3.3.1":
|
||||
version: 3.3.1
|
||||
resolution: "@types/yazl@npm:3.3.1"
|
||||
dependencies:
|
||||
"@types/node": "npm:*"
|
||||
checksum: 10c0/6145fd1025e592747ed1331d88edb6d57dfd0f514812420889ecc880e0728b22c4d4214d5062b36164c4c40ca271266968a1571118ba6521796c76ac0b18c99c
|
||||
checksum: 10c0/bc72a56c88d99021ecb80ee9c0a2eb3dceb5bec522c20fd4e160d5c52dcc156c37b2e956a05d54e89e7ececba82aca07a361a9e8f66e5ebbbfcdcd8d2c0dfc0e
|
||||
languageName: node
|
||||
linkType: hard
|
||||
|
||||
@@ -15545,7 +15545,7 @@ __metadata:
|
||||
"@types/utf8": "npm:^3.0.3"
|
||||
"@types/validator": "npm:^13.15.10"
|
||||
"@types/yauzl": "npm:^2.10.3"
|
||||
"@types/yazl": "npm:^2.4.6"
|
||||
"@types/yazl": "npm:^3.3.1"
|
||||
"@vitejs/plugin-react": "npm:^6.0.3"
|
||||
"@vitest/ui": "npm:^4.1.8"
|
||||
addressparser: "npm:^1.0.1"
|
||||
|
||||
Reference in New Issue
Block a user