Skip to content

Commit

Permalink
fix: remove disposer
Browse files Browse the repository at this point in the history
promises have finally. actually
  • Loading branch information
wraithgar committed Apr 27, 2022
1 parent e77658c commit 76ab648
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 214 deletions.
65 changes: 30 additions & 35 deletions lib/content/write.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,34 +13,37 @@ const path = require('path')
const rimraf = util.promisify(require('rimraf'))
const ssri = require('ssri')
const uniqueFilename = require('unique-filename')
const { disposer } = require('./../util/disposer')
const fsm = require('fs-minipass')

const writeFile = util.promisify(fs.writeFile)

module.exports = write

function write (cache, data, opts = {}) {
async function write (cache, data, opts = {}) {
const { algorithms, size, integrity } = opts
if (algorithms && algorithms.length > 1) {
throw new Error('opts.algorithms only supports a single algorithm for now')
}

if (typeof size === 'number' && data.length !== size) {
return Promise.reject(sizeError(size, data.length))
throw sizeError(size, data.length)
}

const sri = ssri.fromData(data, algorithms ? { algorithms } : {})
if (integrity && !ssri.checkData(data, integrity, opts)) {
return Promise.reject(checksumError(integrity, sri))
throw checksumError(integrity, sri)
}

return disposer(makeTmp(cache, opts), makeTmpDisposer,
(tmp) => {
return writeFile(tmp.target, data, { flag: 'wx' })
.then(() => moveToDestination(tmp, cache, sri, opts))
})
.then(() => ({ integrity: sri, size: data.length }))
const tmp = await makeTmp(cache, opts)
try {
await writeFile(tmp.target, data, { flag: 'wx' })
await moveToDestination(tmp, cache, sri, opts)
return { integrity: sri, size: data.length }
} finally {
if (!tmp.moved) {
await rimraf(tmp.target)
}
}
}

module.exports.stream = writeStream
Expand Down Expand Up @@ -94,18 +97,22 @@ function writeStream (cache, opts = {}) {
return new CacacheWriteStream(cache, opts)
}

function handleContent (inputStream, cache, opts) {
return disposer(makeTmp(cache, opts), makeTmpDisposer, (tmp) => {
return pipeToTmp(inputStream, cache, tmp.target, opts)
.then((res) => {
return moveToDestination(
tmp,
cache,
res.integrity,
opts
).then(() => res)
})
})
async function handleContent (inputStream, cache, opts) {
const tmp = await makeTmp(cache, opts)
try {
const res = await pipeToTmp(inputStream, cache, tmp.target, opts)
await moveToDestination(
tmp,
cache,
res.integrity,
opts
)
return res
} finally {
if (!tmp.moved) {
await rimraf(tmp.target)
}
}
}

function pipeToTmp (inputStream, cache, tmpTarget, opts) {
Expand Down Expand Up @@ -136,11 +143,7 @@ function pipeToTmp (inputStream, cache, tmpTarget, opts) {
outStream
)

return pipeline.promise()
.then(() => ({ integrity, size }))
.catch(er => rimraf(tmpTarget).then(() => {
throw er
}))
return pipeline.promise().then(() => ({ integrity, size }))
}

function makeTmp (cache, opts) {
Expand All @@ -151,14 +154,6 @@ function makeTmp (cache, opts) {
}))
}

function makeTmpDisposer (tmp) {
if (tmp.moved) {
return Promise.resolve()
}

return rimraf(tmp.target)
}

function moveToDestination (tmp, cache, sri, opts) {
const destination = contentPath(cache, sri)
const destDir = path.dirname(destination)
Expand Down
8 changes: 6 additions & 2 deletions lib/entry-index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ const path = require('path')
const ssri = require('ssri')
const uniqueFilename = require('unique-filename')

const { disposer } = require('./util/disposer')
const contentPath = require('./content/path')
const fixOwner = require('./util/fix-owner')
const hashToSegments = require('./util/hash-to-segments')
Expand Down Expand Up @@ -102,7 +101,12 @@ async function compact (cache, key, matchFn, opts = {}) {
}

// write the file atomically
await disposer(setup(), teardown, write)
const tmp = await setup()
try {
await write(tmp)
} finally {
await teardown(tmp)
}

// we reverse the list we generated such that the newest
// entries come first in order to make looping through them easier
Expand Down
31 changes: 0 additions & 31 deletions lib/util/disposer.js

This file was deleted.

26 changes: 22 additions & 4 deletions test/content.write.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ t.test('cleans up tmp on successful completion', (t) => {
}))
})

t.test('cleans up tmp on error', (t) => {
t.test('cleans up tmp on streaming error', (t) => {
const CONTENT = 'foobarbaz'
const CACHE = t.testdir()
return t.rejects(
Expand All @@ -233,6 +233,25 @@ t.test('cleans up tmp on error', (t) => {
}))
})

t.test('cleans up tmp on non streaming error', (t) => {
// mock writefile and make it reject
const CONTENT = 'foobarbaz'
const CACHE = t.testdir({ 'content-v2': 'oh no a file' })
return t.rejects(write(CACHE, CONTENT))
.then(() => new Promise((resolve, reject) => {
const tmp = path.join(CACHE, 'tmp')
fs.readdir(tmp, function (err, files) {
if (!err || (err && err.code === 'ENOENT')) {
files = files || []
t.same(files, [], 'nothing in the tmp dir!')
resolve()
} else {
reject(err)
}
})
}))
})

t.test('checks the size of stream data if opts.size provided', (t) => {
const CONTENT = 'foobarbaz'
let int1 = null
Expand Down Expand Up @@ -270,12 +289,11 @@ t.test('checks the size of stream data if opts.size provided', (t) => {
})
})

t.test('only one algorithm for now', t => {
t.test('only one algorithm for now', async t => {
const CACHE = t.testdir()
t.throws(() => write(CACHE, 'foo', { algorithms: [1, 2] }), {
await t.rejects(() => write(CACHE, 'foo', { algorithms: [1, 2] }), {
message: 'opts.algorithms only supports a single algorithm for now',
})
t.end()
})

t.test('writes to cache with default options', t => {
Expand Down
142 changes: 0 additions & 142 deletions test/disposer.js

This file was deleted.

0 comments on commit 76ab648

Please sign in to comment.