[TEX-537] Refactor compression #555
Conversation
timdawborn
force-pushed
the
TEX-537-refactor-compression
branch
from
2f512a1
to
eb337d7
Compare
| @@ -37,6 +37,7 @@ export function setupServers({ | |||
| defaultTapeName, | |||
| host: TEST_SERVER_HOST, | |||
| timeout: 100, | |||
| enableLogging: true, | |||
There was a problem hiding this comment.
Unrelated change: enable logging on the proxay instance running in the integration tests.
| data: string; | ||
| }; | ||
|
|
||
| export type CompressionAlgorithm = "br" | "gzip" | "none"; |
There was a problem hiding this comment.
Moved into compression.ts.
| @@ -37,8 +38,6 @@ export type PersistedBuffer = | |||
| } | |||
| | { | |||
| encoding: "utf8"; | |||
| compression: CompressionAlgorithm; | |||
| compression: CompressionAlgorithm | undefined; | |||
There was a problem hiding this comment.
Older tapes don't have the compression attribute serialized.
| let compression: CompressionAlgorithm = "none"; | ||
| if (contentEncoding === "br") { | ||
| buffer = Buffer.from(brotli.decompress(buffer)); | ||
| compression = "br"; | ||
| } | ||
| if (contentEncoding === "gzip") { | ||
| buffer = gunzipSync(buffer); | ||
| compression = "gzip"; | ||
| } |
There was a problem hiding this comment.
Moved this common logic into compression.ts.
| if (persisted.compression === "br") { | ||
| // TODO: Find a workaround for the new compressed message not necessarily | ||
| // being identical to what was originally sent (update Content-Length?). | ||
| const compressed = brotli.compress(buffer); | ||
| if (compressed) { | ||
| buffer = Buffer.from(compressed); | ||
| } else { | ||
| throw new Error(`Brotli compression failed!`); | ||
| } | ||
| } | ||
| if (persisted.compression === "gzip") { | ||
| // TODO: Find a workaround for the new compressed message not necessarily | ||
| // being identical to what was originally sent (update Content-Length?). | ||
| buffer = gzipSync(buffer); | ||
| } |
There was a problem hiding this comment.
Moved this common logic into compression.ts.
| switch (contentEncoding) { | ||
| case "": | ||
| return request.body; | ||
| case "br": | ||
| return Buffer.from(brotli.decompress(request.body)); | ||
| case "gzip": | ||
| return zlib.gunzipSync(request.body); | ||
| default: | ||
| throw Error(`Unhandled content-encoding value "${contentEncoding}"`); |
There was a problem hiding this comment.
Moved this common logic into compression.ts.
| @@ -51,7 +51,6 @@ | |||
| }, | |||
| "dependencies": { | |||
| "assert-never": "^1.2.1", | |||
| "brotli": "^1.3.3", | |||
There was a problem hiding this comment.
Replacing this third-party library with the brotli implementation provided by the NodeJS builtin zlib module.
timdawborn
force-pushed
the
TEX-537-refactor-compression
branch
from
538ab65
to
b1d7b23
Compare
timdawborn
force-pushed
the
TEX-537-refactor-compression
branch
from
b1d7b23
to
1ca870c
Compare
| @@ -221,7 +222,7 @@ describe("Persistence", () => { | |||
| }, | |||
| body: { | |||
| encoding: "base64", | |||
| data: "GxcAAI6UrMm1WkAERl0HoDFuCn3CIekc", | |||
| data: "GxcA+I+UrMm1WkAERl0HoDFuCn3CAZLOAQ==", | |||
There was a problem hiding this comment.
The brotli encoding default compression configuration differs between the old library and the new NodeJS builtin implementation. This is why hard-coded expected encoding here differs.
| @@ -233,7 +234,7 @@ describe("Persistence", () => { | |||
| }, | |||
| body: { | |||
| encoding: "base64", | |||
| data: "GxcAAI6UrPmFmgFmOV+HoM3+C33CIe4U", | |||
| data: "GxcA+I+UrPmFmgFmOV+HoM3+C33CAeJOAQ==", | |||
There was a problem hiding this comment.
The brotli encoding default compression configuration differs between the old library and the new NodeJS builtin implementation. This is why hard-coded expected encoding here differs.
This reverts commit fa3152b.
This PR continues to cleanup this codebase ahead of supporting richer, content-type-aware, request body matching.
This PR refactors all of the logic that deals with compression into a singular location, and removes an old, outdated third-party library in favour of the builtin NodeJS implementation.