Skip to content

Commit

Permalink
fix(hash-stream-node): do not create copy of file stream (#3380)
Browse files Browse the repository at this point in the history
  • Loading branch information
trivikr authored Mar 1, 2022
1 parent 30a3d62 commit 89a70ae
Show file tree
Hide file tree
Showing 6 changed files with 5 additions and 123 deletions.
46 changes: 0 additions & 46 deletions packages/hash-stream-node/src/fsCreateReadStream.spec.ts

This file was deleted.

9 changes: 0 additions & 9 deletions packages/hash-stream-node/src/fsCreateReadStream.ts

This file was deleted.

34 changes: 0 additions & 34 deletions packages/hash-stream-node/src/isFileStream.spec.ts

This file was deleted.

7 changes: 0 additions & 7 deletions packages/hash-stream-node/src/isFileStream.ts

This file was deleted.

20 changes: 1 addition & 19 deletions packages/hash-stream-node/src/readableStreamHasher.spec.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,10 @@
import { Hash } from "@aws-sdk/types";
import { createReadStream } from "fs";
import { Readable, Writable } from "stream";

import { fsCreateReadStream } from "./fsCreateReadStream";
import { HashCalculator } from "./HashCalculator";
import { isFileStream } from "./isFileStream";
import { readableStreamHasher } from "./readableStreamHasher";

jest.mock("./fsCreateReadStream");
jest.mock("./HashCalculator");
jest.mock("./isFileStream");
jest.mock("fs");

describe(readableStreamHasher.name, () => {
const mockDigest = jest.fn();
Expand Down Expand Up @@ -44,25 +38,13 @@ describe(readableStreamHasher.name, () => {
(HashCalculator as unknown as jest.Mock).mockImplementation(
(hash) => new MockHashCalculator(hash, mockHashCalculatorWrite, mockHashCalculatorEnd)
);
(isFileStream as unknown as jest.Mock).mockReturnValue(false);
mockDigest.mockResolvedValue(mockHash);
});

afterEach(() => {
jest.clearAllMocks();
});

it("creates a copy in case of fileStream", () => {
(fsCreateReadStream as jest.Mock).mockReturnValue(new Readable({ read: (size) => {} }));
(isFileStream as unknown as jest.Mock).mockReturnValue(true);

const fsReadStream = createReadStream(__filename);
readableStreamHasher(mockHashCtor, fsReadStream);

expect(isFileStream).toHaveBeenCalledWith(fsReadStream);
expect(fsCreateReadStream).toHaveBeenCalledWith(fsReadStream);
});

it("computes hash for a readable stream", async () => {
const readableStream = new Readable({ read: (size) => {} });
const hashPromise = readableStreamHasher(mockHashCtor, readableStream);
Expand All @@ -86,7 +68,7 @@ describe(readableStreamHasher.name, () => {
expect(mockHashCalculatorEnd).toHaveBeenCalledTimes(1);
});

it("throws if readable stream is not a file stream and has started reading", async () => {
it("throws if readable stream has started reading", async () => {
const readableStream = new Readable({ read: (size) => {} });
// Simulate readableFlowing to true.
readableStream.resume();
Expand Down
12 changes: 4 additions & 8 deletions packages/hash-stream-node/src/readableStreamHasher.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,20 @@
import { HashConstructor, StreamHasher } from "@aws-sdk/types";
import { Readable } from "stream";

import { fsCreateReadStream } from "./fsCreateReadStream";
import { HashCalculator } from "./HashCalculator";
import { isFileStream } from "./isFileStream";

export const readableStreamHasher: StreamHasher<Readable> = (hashCtor: HashConstructor, readableStream: Readable) => {
// Throw if readableStream is already flowing and it's not a file stream.
if (!isFileStream(readableStream) && readableStream.readableFlowing !== null) {
// Throw if readableStream is already flowing.
if (readableStream.readableFlowing !== null) {
throw new Error("Unable to calculate hash for flowing readable stream");
}

const streamToPipe = isFileStream(readableStream) ? fsCreateReadStream(readableStream) : readableStream;

const hash = new hashCtor();
const hashCalculator = new HashCalculator(hash);
streamToPipe.pipe(hashCalculator);
readableStream.pipe(hashCalculator);

return new Promise((resolve, reject) => {
streamToPipe.on("error", (err: Error) => {
readableStream.on("error", (err: Error) => {
// if the source errors, the destination stream needs to manually end
hashCalculator.end();
reject(err);
Expand Down

0 comments on commit 89a70ae

Please sign in to comment.