Skip to content

Commit fa26ab4

Browse files
wraithgarwraithgar
committed
fix: move to async functions where possible
This refactors the code to use async functions where possible. It also uses `@npmcli/fs` consistently (it was already a dependency, just not used). It also reorders the test files to match their sources in `./lib`. Tests were not refactored (except where needed to move to `@npmcli/fs`) in the interest of an easier PR review.
1 parent 36b8763 commit fa26ab4

31 files changed

+803
-1039
lines changed

‎.eslintrc.local.js

+3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1+
'use strict'
2+
13
module.exports = {
24
rules: {
5+
strict: 'error',
36
'no-shadow': 0, // XXX: fix this later
47
},
58
}

‎lib/content/read.js

+73-88
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,35 @@
11
'use strict'
22

3-
const util = require('util')
4-
5-
const fs = require('fs')
3+
const fs = require('@npmcli/fs')
64
const fsm = require('fs-minipass')
75
const ssri = require('ssri')
86
const contentPath = require('./path')
97
const Pipeline = require('minipass-pipeline')
108

11-
const lstat = util.promisify(fs.lstat)
12-
const readFile = util.promisify(fs.readFile)
13-
const copyFile = util.promisify(fs.copyFile)
14-
159
module.exports = read
1610

1711
const MAX_SINGLE_READ_SIZE = 64 * 1024 * 1024
18-
function read (cache, integrity, opts = {}) {
12+
async function read (cache, integrity, opts = {}) {
1913
const { size } = opts
20-
return withContentSri(cache, integrity, (cpath, sri) => {
14+
const { stat, cpath, sri } = await withContentSri(cache, integrity, async (cpath, sri) => {
2115
// get size
22-
return lstat(cpath).then(stat => ({ stat, cpath, sri }))
23-
}).then(({ stat, cpath, sri }) => {
24-
if (typeof size === 'number' && stat.size !== size) {
25-
throw sizeError(size, stat.size)
26-
}
16+
const stat = await fs.lstat(cpath)
17+
return { stat, cpath, sri }
18+
})
19+
if (typeof size === 'number' && stat.size !== size) {
20+
throw sizeError(size, stat.size)
21+
}
2722

28-
if (stat.size > MAX_SINGLE_READ_SIZE) {
29-
return readPipeline(cpath, stat.size, sri, new Pipeline()).concat()
30-
}
23+
if (stat.size > MAX_SINGLE_READ_SIZE) {
24+
return readPipeline(cpath, stat.size, sri, new Pipeline()).concat()
25+
}
3126

32-
return readFile(cpath, null).then((data) => {
33-
if (!ssri.checkData(data, sri)) {
34-
throw integrityError(sri, cpath)
35-
}
27+
const data = await fs.readFile(cpath, null)
28+
if (!ssri.checkData(data, sri)) {
29+
throw integrityError(sri, cpath)
30+
}
3631

37-
return data
38-
})
39-
})
32+
return data
4033
}
4134

4235
const readPipeline = (cpath, size, sri, stream) => {
@@ -77,16 +70,19 @@ module.exports.readStream = readStream
7770
function readStream (cache, integrity, opts = {}) {
7871
const { size } = opts
7972
const stream = new Pipeline()
80-
withContentSri(cache, integrity, (cpath, sri) => {
81-
// just lstat to ensure it exists
82-
return lstat(cpath).then((stat) => ({ stat, cpath, sri }))
83-
}).then(({ stat, cpath, sri }) => {
73+
// Set all this up to run on the stream and then just return the stream
74+
Promise.resolve().then(async () => {
75+
const { stat, cpath, sri } = await withContentSri(cache, integrity, async (cpath, sri) => {
76+
// just lstat to ensure it exists
77+
const stat = await fs.lstat(cpath)
78+
return { stat, cpath, sri }
79+
})
8480
if (typeof size === 'number' && size !== stat.size) {
8581
return stream.emit('error', sizeError(size, stat.size))
8682
}
8783

8884
readPipeline(cpath, stat.size, sri, stream)
89-
}, er => stream.emit('error', er))
85+
}).catch(err => stream.emit('error', err))
9086

9187
return stream
9288
}
@@ -96,7 +92,7 @@ module.exports.copy.sync = copySync
9692

9793
function copy (cache, integrity, dest) {
9894
return withContentSri(cache, integrity, (cpath, sri) => {
99-
return copyFile(cpath, dest)
95+
return fs.copyFile(cpath, dest)
10096
})
10197
}
10298

@@ -108,14 +104,17 @@ function copySync (cache, integrity, dest) {
108104

109105
module.exports.hasContent = hasContent
110106

111-
function hasContent (cache, integrity) {
107+
async function hasContent (cache, integrity) {
112108
if (!integrity) {
113-
return Promise.resolve(false)
109+
return false
114110
}
115111

116-
return withContentSri(cache, integrity, (cpath, sri) => {
117-
return lstat(cpath).then((stat) => ({ size: stat.size, sri, stat }))
118-
}).catch((err) => {
112+
try {
113+
return await withContentSri(cache, integrity, async (cpath, sri) => {
114+
const stat = await fs.lstat(cpath)
115+
return { size: stat.size, sri, stat }
116+
})
117+
} catch (err) {
119118
if (err.code === 'ENOENT') {
120119
return false
121120
}
@@ -128,7 +127,7 @@ function hasContent (cache, integrity) {
128127
return false
129128
}
130129
}
131-
})
130+
}
132131
}
133132

134133
module.exports.hasContent.sync = hasContentSync
@@ -159,61 +158,47 @@ function hasContentSync (cache, integrity) {
159158
})
160159
}
161160

162-
function withContentSri (cache, integrity, fn) {
163-
const tryFn = () => {
164-
const sri = ssri.parse(integrity)
165-
// If `integrity` has multiple entries, pick the first digest
166-
// with available local data.
167-
const algo = sri.pickAlgorithm()
168-
const digests = sri[algo]
169-
170-
if (digests.length <= 1) {
171-
const cpath = contentPath(cache, digests[0])
172-
return fn(cpath, digests[0])
173-
} else {
174-
// Can't use race here because a generic error can happen before
175-
// a ENOENT error, and can happen before a valid result
176-
return Promise
177-
.all(digests.map((meta) => {
178-
return withContentSri(cache, meta, fn)
179-
.catch((err) => {
180-
if (err.code === 'ENOENT') {
181-
return Object.assign(
182-
new Error('No matching content found for ' + sri.toString()),
183-
{ code: 'ENOENT' }
184-
)
185-
}
186-
return err
187-
})
188-
}))
189-
.then((results) => {
190-
// Return the first non error if it is found
191-
const result = results.find((r) => !(r instanceof Error))
192-
if (result) {
193-
return result
194-
}
195-
196-
// Throw the No matching content found error
197-
const enoentError = results.find((r) => r.code === 'ENOENT')
198-
if (enoentError) {
199-
throw enoentError
200-
}
201-
202-
// Throw generic error
203-
throw results.find((r) => r instanceof Error)
204-
})
161+
async function withContentSri (cache, integrity, fn) {
162+
const sri = ssri.parse(integrity)
163+
// If `integrity` has multiple entries, pick the first digest
164+
// with available local data.
165+
const algo = sri.pickAlgorithm()
166+
const digests = sri[algo]
167+
168+
if (digests.length <= 1) {
169+
const cpath = contentPath(cache, digests[0])
170+
return fn(cpath, digests[0])
171+
} else {
172+
// Can't use race here because a generic error can happen before
173+
// a ENOENT error, and can happen before a valid result
174+
const results = await Promise.all(digests.map(async (meta) => {
175+
try {
176+
return await withContentSri(cache, meta, fn)
177+
} catch (err) {
178+
if (err.code === 'ENOENT') {
179+
return Object.assign(
180+
new Error('No matching content found for ' + sri.toString()),
181+
{ code: 'ENOENT' }
182+
)
183+
}
184+
return err
185+
}
186+
}))
187+
// Return the first non error if it is found
188+
const result = results.find((r) => !(r instanceof Error))
189+
if (result) {
190+
return result
205191
}
206-
}
207192

208-
return new Promise((resolve, reject) => {
209-
try {
210-
tryFn()
211-
.then(resolve)
212-
.catch(reject)
213-
} catch (err) {
214-
reject(err)
193+
// Throw the No matching content found error
194+
const enoentError = results.find((r) => r.code === 'ENOENT')
195+
if (enoentError) {
196+
throw enoentError
215197
}
216-
})
198+
199+
// Throw generic error
200+
throw results.find((r) => r instanceof Error)
201+
}
217202
}
218203

219204
function withContentSriSync (cache, integrity, fn) {

‎lib/content/rm.js

+9-9
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,13 @@ const rimraf = util.promisify(require('rimraf'))
88

99
module.exports = rm
1010

11-
function rm (cache, integrity) {
12-
return hasContent(cache, integrity).then((content) => {
13-
// ~pretty~ sure we can't end up with a content lacking sri, but be safe
14-
if (content && content.sri) {
15-
return rimraf(contentPath(cache, content.sri)).then(() => true)
16-
} else {
17-
return false
18-
}
19-
})
11+
async function rm (cache, integrity) {
12+
const content = await hasContent(cache, integrity)
13+
// ~pretty~ sure we can't end up with a content lacking sri, but be safe
14+
if (content && content.sri) {
15+
await rimraf(contentPath(cache, content.sri))
16+
return true
17+
} else {
18+
return false
19+
}
2020
}

‎lib/content/write.js

+14-19
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ const util = require('util')
44

55
const contentPath = require('./path')
66
const fixOwner = require('../util/fix-owner')
7-
const fs = require('fs')
7+
const fs = require('@npmcli/fs')
88
const moveFile = require('../util/move-file')
99
const Minipass = require('minipass')
1010
const Pipeline = require('minipass-pipeline')
@@ -15,8 +15,6 @@ const ssri = require('ssri')
1515
const uniqueFilename = require('unique-filename')
1616
const fsm = require('fs-minipass')
1717

18-
const writeFile = util.promisify(fs.writeFile)
19-
2018
module.exports = write
2119

2220
async function write (cache, data, opts = {}) {
@@ -36,7 +34,7 @@ async function write (cache, data, opts = {}) {
3634

3735
const tmp = await makeTmp(cache, opts)
3836
try {
39-
await writeFile(tmp.target, data, { flag: 'wx' })
37+
await fs.writeFile(tmp.target, data, { flag: 'wx' })
4038
await moveToDestination(tmp, cache, sri, opts)
4139
return { integrity: sri, size: data.length }
4240
} finally {
@@ -115,7 +113,7 @@ async function handleContent (inputStream, cache, opts) {
115113
}
116114
}
117115

118-
function pipeToTmp (inputStream, cache, tmpTarget, opts) {
116+
async function pipeToTmp (inputStream, cache, tmpTarget, opts) {
119117
let integrity
120118
let size
121119
const hashStream = ssri.integrityStream({
@@ -143,30 +141,27 @@ function pipeToTmp (inputStream, cache, tmpTarget, opts) {
143141
outStream
144142
)
145143

146-
return pipeline.promise().then(() => ({ integrity, size }))
144+
await pipeline.promise()
145+
return { integrity, size }
147146
}
148147

149-
function makeTmp (cache, opts) {
148+
async function makeTmp (cache, opts) {
150149
const tmpTarget = uniqueFilename(path.join(cache, 'tmp'), opts.tmpPrefix)
151-
return fixOwner.mkdirfix(cache, path.dirname(tmpTarget)).then(() => ({
150+
await fixOwner.mkdirfix(cache, path.dirname(tmpTarget))
151+
return {
152152
target: tmpTarget,
153153
moved: false,
154-
}))
154+
}
155155
}
156156

157-
function moveToDestination (tmp, cache, sri, opts) {
157+
async function moveToDestination (tmp, cache, sri, opts) {
158158
const destination = contentPath(cache, sri)
159159
const destDir = path.dirname(destination)
160160

161-
return fixOwner
162-
.mkdirfix(cache, destDir)
163-
.then(() => {
164-
return moveFile(tmp.target, destination)
165-
})
166-
.then(() => {
167-
tmp.moved = true
168-
return fixOwner.chownr(cache, destination)
169-
})
161+
await fixOwner.mkdirfix(cache, destDir)
162+
await moveFile(tmp.target, destination)
163+
tmp.moved = true
164+
await fixOwner.chownr(cache, destination)
170165
}
171166

172167
function sizeError (expected, found) {

0 commit comments

Comments
 (0)