Skip to content

Commit

Permalink
refactor: dry and other pr comments
Browse files Browse the repository at this point in the history
PR-URL: #391
Credit: @JamieMagee
Close: #391
Reviewed-by: @isaacs
  • Loading branch information
JamieMagee authored and isaacs committed Sep 5, 2023
1 parent eeba222 commit 336fa8f
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 36 deletions.
30 changes: 16 additions & 14 deletions lib/pack.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,23 +79,25 @@ const Pack = warner(class Pack extends Minipass {

this.portable = !!opt.portable
this.zip = null
if (opt.gzip) {
if (typeof opt.gzip !== 'object') {
opt.gzip = {}
if (opt.gzip || opt.brotli) {
if (opt.gzip && opt.brotli) {
throw new TypeError('gzip and brotli are mutually exclusive')
}
if (this.portable) {
opt.gzip.portable = true
if (opt.gzip) {
if (typeof opt.gzip !== 'object') {
opt.gzip = {}
}
if (this.portable) {
opt.gzip.portable = true
}
this.zip = new zlib.Gzip(opt.gzip)
}
this.zip = new zlib.Gzip(opt.gzip)
this.zip.on('data', chunk => super.write(chunk))
this.zip.on('end', _ => super.end())
this.zip.on('drain', _ => this[ONDRAIN]())
this.on('resume', _ => this.zip.resume())
} else if (opt.brotli) {
if (typeof opt.brotli !== 'object') {
opt.brotli = {}
if (opt.brotli) {
if (typeof opt.brotli !== 'object') {
opt.brotli = {}
}
this.zip = new zlib.BrotliCompress(opt.brotli)
}
this.zip = new zlib.BrotliCompress(opt.brotli)
this.zip.on('data', chunk => super.write(chunk))
this.zip.on('end', _ => super.end())
this.zip.on('drain', _ => this[ONDRAIN]())
Expand Down
22 changes: 4 additions & 18 deletions lib/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ module.exports = warner(class Parser extends EE {
this.filter = typeof opt.filter === 'function' ? opt.filter : noop
// Unlike gzip, brotli doesn't have any magic bytes to identify it
// Users need to explicitly tell us they're extracting a brotli file
this.brotli = opt.brotli
// Or we infer from the file extension
this.brotli = opt.brotli || (opt.file && (opt.file.endsWith('.tar.br') || opt.file.endsWith('.tbr')))

// have to set this so that streams are ok piping into it
this.writable = true
Expand Down Expand Up @@ -359,30 +360,15 @@ module.exports = warner(class Parser extends EE {
this[BUFFER] = chunk
return true
}
if (this[UNZIP] === null && this.brotli) {
const ended = this[ENDED]
this[ENDED] = false
this[UNZIP] = new zlib.BrotliDecompress()
this[UNZIP].on('data', chunk => this[CONSUMECHUNK](chunk))
this[UNZIP].on('error', er => this.abort(er))
this[UNZIP].on('end', _ => {
this[ENDED] = true
this[CONSUMECHUNK]()
})
this[WRITING] = true
const ret = this[UNZIP][ended ? 'end' : 'write'](chunk)
this[WRITING] = false
return ret
}
for (let i = 0; this[UNZIP] === null && i < gzipHeader.length; i++) {
if (chunk[i] !== gzipHeader[i]) {
this[UNZIP] = false
}
}
if (this[UNZIP] === null) {
if (this[UNZIP] === null || (this[UNZIP] === false && this.brotli)) {
const ended = this[ENDED]
this[ENDED] = false
this[UNZIP] = new zlib.Unzip()
this[UNZIP] = this.brotli ? new zlib.BrotliDecompress() : new zlib.Unzip()
this[UNZIP].on('data', chunk => this[CONSUMECHUNK](chunk))
this[UNZIP].on('error', er => this.abort(er))
this[UNZIP].on('end', _ => {
Expand Down
21 changes: 17 additions & 4 deletions test/extract.js
Original file line number Diff line number Diff line change
Expand Up @@ -323,12 +323,25 @@ t.test('brotli', async t => {
await rimraf(dir)
})

t.test('fails if brotli', async t => {
const expect = new Error("TAR_BAD_ARCHIVE: Unrecognized archive format")
t.throws(_ => x({ sync: true, file: file }), expect)
t.test('fails if unknown file extension', async t => {
const filename = path.resolve(__dirname, 'brotli/example.unknown')
const f = fs.openSync(filename, 'a')
fs.closeSync(f)

const expect = new Error('TAR_BAD_ARCHIVE: Unrecognized archive format')

t.throws(_ => x({ sync: true, file: filename }), expect)
})

t.test('succeeds based on file extension', t => {
x({ sync: true, file: file, C: dir })

t.same(fs.readdirSync(dir + '/x').sort(),
['1', '10', '2', '3', '4', '5', '6', '7', '8', '9'])
t.end()
})

t.test('succeeds', t => {
t.test('succeeds when passed explicit option', t => {
x({ sync: true, file: file, C: dir, brotli: true })

t.same(fs.readdirSync(dir + '/x').sort(),
Expand Down
6 changes: 6 additions & 0 deletions test/pack.js
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,12 @@ t.test('if brotli is truthy, make it an object', t => {
t.end()
})

t.test('throws if both gzip and brotli are truthy', t => {
const opt = { gzip: true, brotli: true }
t.throws(_ => new Pack(opt), new TypeError('gzip and brotli are mutually exclusive'))
t.end()
})

t.test('gzip, also a very deep path', t => {
const out = []

Expand Down
22 changes: 22 additions & 0 deletions test/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,28 @@ t.test('fixture tests', t => {
bs.end(zlib.gzipSync(tardata))
})

t.test('compress with brotli based on filename .tar.br', t => {
const p = new Parse({
maxMetaEntrySize: maxMeta,
filter: filter ? (path, entry) => entry.size % 2 !== 0 : null,
strict: strict,
file: 'example.tar.br',
})
trackEvents(t, expect, p)
p.end(zlib.brotliCompressSync(tardata))
})

t.test('compress with brotli based on filename .tbr', t => {
const p = new Parse({
maxMetaEntrySize: maxMeta,
filter: filter ? (path, entry) => entry.size % 2 !== 0 : null,
strict: strict,
file: 'example.tbr',
})
trackEvents(t, expect, p)
p.end(zlib.brotliCompressSync(tardata))
})

t.test('compress with brotli all at once', t => {
const p = new Parse({
maxMetaEntrySize: maxMeta,
Expand Down

0 comments on commit 336fa8f

Please sign in to comment.