Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,37 @@ export namespace gzip {

}

export namespace xz {

function compressFile(source: sourceType, dest: destType, opts?: any): Promise<void>

function uncompress(source: sourceType, dest: destType, opts?: any): Promise<void>

function decompress(source: sourceType, dest: destType, opts?: any): Promise<void>
Comment on lines +58 to +62

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For better type safety, the opts parameter for these functions should be more specific than any. compressFile accepts preset and threads options, while uncompress and decompress don't have specific options. Please consider defining the types for these options inline to improve developer experience and prevent potential bugs.

Suggested change
function compressFile(source: sourceType, dest: destType, opts?: any): Promise<void>
function uncompress(source: sourceType, dest: destType, opts?: any): Promise<void>
function decompress(source: sourceType, dest: destType, opts?: any): Promise<void>
function compressFile(source: sourceType, dest: destType, opts?: { preset?: number, threads?: number }): Promise<void>
function uncompress(source: sourceType, dest: destType, opts?: {}): Promise<void>
function decompress(source: sourceType, dest: destType, opts?: {}): Promise<void>


export class FileStream extends ReadStream {

constructor(opts?: {
preset?: number,
threads?: number,
source?: sourceType
});

}

export class UncompressStream extends WriteStream {

constructor(opts?: {
source?: sourceType
});

on(event: string, listener: (...args: any[]) => void): this
on(event: 'error', listener: (err: Error) => void): this

}

}

export namespace tar {

function compressFile(source: sourceType, dest: destType, opts?: any): Promise<void>
Expand Down
1 change: 1 addition & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ exports.zip = require('./lib/zip');
exports.gzip = require('./lib/gzip');
exports.tar = require('./lib/tar');
exports.tgz = require('./lib/tgz');
exports.xz = require('./lib/xz');
42 changes: 42 additions & 0 deletions lib/xz/file_stream.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
'use strict';

const fs = require('fs');
const lzma = require('lzma-native');
const utils = require('../utils');
const streamifier = require('streamifier');

class XzFileStream extends lzma.Compressor {
constructor(opts) {
opts = opts || {};
const lzmaOpts = {
preset: opts.preset || 6,
threads: opts.threads || 0,
};
super(lzmaOpts);

const sourceType = utils.sourceType(opts.source);

if (sourceType === 'file') {
const stream = fs.createReadStream(opts.source, opts.fs);
stream.on('error', err => this.emit('error', err));
stream.pipe(this);
return;
}

if (sourceType === 'buffer') {
const stream = streamifier.createReadStream(opts.source, opts.streamifier);
stream.on('error', err => this.emit('error', err));
stream.pipe(this);
return;
}

if (sourceType === 'stream') {
opts.source.on('error', err => this.emit('error', err));
opts.source.pipe(this);
}
Comment on lines +17 to +36

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logic for handling different source types (file, buffer, stream) is nearly identical to the logic in lib/xz/uncompress_stream.js. To improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle, this logic should be extracted into a shared utility function. This would centralize the source handling logic, making future modifications easier and less error-prone.


// else undefined: do nothing
}
}

module.exports = XzFileStream;
11 changes: 11 additions & 0 deletions lib/xz/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
'use strict';

const utils = require('../utils');
const XzFileStream = require('./file_stream');
const XzUncompressStream = require('./uncompress_stream');

exports.FileStream = XzFileStream;
exports.UncompressStream = XzUncompressStream;
exports.compressFile = utils.makeFileProcessFn(XzFileStream);
exports.uncompress = utils.makeFileProcessFn(XzUncompressStream);
exports.decompress = utils.makeFileProcessFn(XzUncompressStream);
38 changes: 38 additions & 0 deletions lib/xz/uncompress_stream.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
'use strict';

const fs = require('fs');
const lzma = require('lzma-native');
const utils = require('../utils');
const streamifier = require('streamifier');

class XzUncompressStream extends lzma.Decompressor {
constructor(opts) {
opts = opts || {};
super();

const sourceType = utils.sourceType(opts.source);

if (sourceType === 'file') {
const stream = fs.createReadStream(opts.source, opts.fs);
stream.on('error', err => this.emit('error', err));
stream.pipe(this);
return;
}

if (sourceType === 'buffer') {
const stream = streamifier.createReadStream(opts.source, opts.streamifier);
stream.on('error', err => this.emit('error', err));
stream.pipe(this);
return;
}

if (sourceType === 'stream') {
opts.source.on('error', err => this.emit('error', err));
opts.source.pipe(this);
}
Comment on lines +13 to +32

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This logic for handling different source types is duplicated from lib/xz/file_stream.js. As mentioned in the comment for that file, this should be refactored into a shared utility function to reduce code duplication and improve maintainability.

Comment on lines +15 to +32
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Prefer destroying on upstream errors to ensure teardown and proper pipeline propagation

Forwarding errors with this.emit('error', err) works, but using this.destroy(err) tears down the decompressor and guarantees pipeline() propagates the error cleanly and promptly. Apply across all three source types for consistency.

 if (sourceType === 'file') {
   const stream = fs.createReadStream(opts.source, opts.fs);
-  stream.on('error', err => this.emit('error', err));
+  stream.on('error', err => this.destroy(err));
   stream.pipe(this);
   return;
 }

 if (sourceType === 'buffer') {
   const stream = streamifier.createReadStream(opts.source, opts.streamifier);
-  stream.on('error', err => this.emit('error', err));
+  stream.on('error', err => this.destroy(err));
   stream.pipe(this);
   return;
 }

 if (sourceType === 'stream') {
-  opts.source.on('error', err => this.emit('error', err));
+  opts.source.on('error', err => this.destroy(err));
   opts.source.pipe(this);
+  return;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (sourceType === 'file') {
const stream = fs.createReadStream(opts.source, opts.fs);
stream.on('error', err => this.emit('error', err));
stream.pipe(this);
return;
}
if (sourceType === 'buffer') {
const stream = streamifier.createReadStream(opts.source, opts.streamifier);
stream.on('error', err => this.emit('error', err));
stream.pipe(this);
return;
}
if (sourceType === 'stream') {
opts.source.on('error', err => this.emit('error', err));
opts.source.pipe(this);
}
if (sourceType === 'file') {
const stream = fs.createReadStream(opts.source, opts.fs);
stream.on('error', err => this.destroy(err));
stream.pipe(this);
return;
}
if (sourceType === 'buffer') {
const stream = streamifier.createReadStream(opts.source, opts.streamifier);
stream.on('error', err => this.destroy(err));
stream.pipe(this);
return;
}
if (sourceType === 'stream') {
opts.source.on('error', err => this.destroy(err));
opts.source.pipe(this);
return;
}
🤖 Prompt for AI Agents
In lib/xz/uncompress_stream.js around lines 15 to 32, the upstream error
handlers call this.emit('error', err) which only forwards the error; change each
handler to call this.destroy(err) so the decompressor is torn down and the error
is propagated cleanly through pipeline(): replace stream.on('error', err =>
this.emit('error', err)) and opts.source.on('error', err => this.emit('error',
err)) with handlers that invoke this.destroy(err) (ensuring the handler context
is the decompressor) for the file, buffer and stream source branches to keep
behavior consistent.


// else: waiting to be piped
}
}

module.exports = XzUncompressStream;
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
"flushwritable": "^1.0.0",
"get-ready": "^1.0.0",
"iconv-lite": "^0.5.0",
"lzma-native": "^8.0.6",
"streamifier": "^0.1.1",
"tar-stream": "^1.5.2",
"yazl": "^2.4.2"
Expand Down
113 changes: 113 additions & 0 deletions test/xz/file_stream.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
const fs = require('fs');
const os = require('os');
const path = require('path');
const uuid = require('uuid');
const { pipeline: pump } = require('stream');
const compressing = require('../..');
const assert = require('assert');

describe('test/xz/file_stream.test.js', () => {
it('should be a transform stream', done => {
const sourceFile = path.join(__dirname, '..', 'fixtures', 'xx.log');
const sourceStream = fs.createReadStream(sourceFile);
const destFile = path.join(os.tmpdir(), uuid.v4() + '.log.xz');
// console.log('destFile', destFile);
const xzStream = new compressing.xz.FileStream();
const destStream = fs.createWriteStream(destFile);
pump(sourceStream, xzStream, destStream, err => {
assert(!err);
assert(fs.existsSync(destFile));
done();
});
});
Comment on lines +10 to +22

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This test uses the done callback, while other tests in the suite use async/await. For consistency and improved readability, it's better to use async/await throughout the test suite. You can promisify the stream pipeline to achieve this.

  it('should be a transform stream', async () => {
    const sourceFile = path.join(__dirname, '..', 'fixtures', 'xx.log');
    const sourceStream = fs.createReadStream(sourceFile);
    const destFile = path.join(os.tmpdir(), uuid.v4() + '.log.xz');
    // console.log('destFile', destFile);
    const xzStream = new compressing.xz.FileStream();
    const destStream = fs.createWriteStream(destFile);
    await new Promise((resolve, reject) => {
      pump(sourceStream, xzStream, destStream, err => {
        if (err) return reject(err);
        resolve();
      });
    });
    assert(fs.existsSync(destFile));
  });


it('should compress according to file path', done => {
const sourceFile = path.join(__dirname, '..', 'fixtures', 'xx.log');
const destFile = path.join(os.tmpdir(), uuid.v4() + '.log.xz');
// console.log('destFile', destFile);
const xzStream = new compressing.xz.FileStream({ source: sourceFile });
const destStream = fs.createWriteStream(destFile);
pump(xzStream, destStream, err => {
assert(!err);
assert(fs.existsSync(destFile));
done();
});
});

it('should compress file into Buffer', async () => {
const sourceFile = path.join(__dirname, '..', 'fixtures', 'xx.log');
const xzStream = new compressing.xz.FileStream({ source: sourceFile });
const xzChunks = [];
for await (const chunk of xzStream) {
xzChunks.push(chunk);
}

const destFile = path.join(os.tmpdir(), uuid.v4() + '.log.xz');
await fs.promises.writeFile(destFile, Buffer.concat(xzChunks));
// console.log(destFile);
});

it('should compress buffer', done => {
const sourceFile = path.join(__dirname, '..', 'fixtures', 'xx.log');
const sourceBuffer = fs.readFileSync(sourceFile);
const destFile = path.join(os.tmpdir(), uuid.v4() + '.log.xz');
// console.log('destFile', destFile);
const destStream = fs.createWriteStream(destFile);
const xzStream = new compressing.xz.FileStream({ source: sourceBuffer });
pump(xzStream, destStream, err => {
assert(!err);
assert(fs.existsSync(destFile));
done();
});

});

it('should compress stream', done => {
const sourceFile = path.join(__dirname, '..', 'fixtures', 'xx.log');
const sourceStream = fs.createReadStream(sourceFile);
const destFile = path.join(os.tmpdir(), uuid.v4() + '.log.xz');
// console.log('destFile', destFile);
const destStream = fs.createWriteStream(destFile);
const xzStream = new compressing.xz.FileStream({ source: sourceStream });
pump(xzStream, destStream, err => {
assert(!err);
assert(fs.existsSync(destFile));
done();
});
});

it('should compress with custom level', done => {
const sourceFile = path.join(__dirname, '..', 'fixtures', 'xx.log');
const destFile = path.join(os.tmpdir(), uuid.v4() + '.log.xz');
const xzStream = new compressing.xz.FileStream({
source: sourceFile,
level: 6,
});
const destStream = fs.createWriteStream(destFile);
pump(xzStream, destStream, err => {
assert(!err);
assert(fs.existsSync(destFile));
done();
});
});

it('should emit error if sourceFile does not exit', done => {
const sourceFile = 'file-not-exist';
const xzStream = new compressing.xz.FileStream({ source: sourceFile });
xzStream.on('error', err => {
assert(err);
done();
});
});

it('should emit error if sourceStream emit error', done => {
const sourceFile = 'file-not-exist';
const sourceStream = fs.createReadStream(sourceFile);
const xzStream = new compressing.xz.FileStream({ source: sourceStream });
xzStream.on('error', err => {
assert(err && err.code === 'ENOENT');
done();
});
});

});
Loading
Loading