Skip to content

Commit be94893

Browse files
committed
fix: pr feedback
1 parent c73f9d2 commit be94893

9 files changed

+66
-89
lines changed

src/default-options-browser.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
// Default configuration for a repo in the browser
44
module.exports = {
5-
autoMigrate: true,
65
lock: 'memory',
76
storageBackends: {
87
root: require('datastore-level'),

src/default-options.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
// Default configuration for a repo in node.js
44
module.exports = {
5-
autoMigrate: true,
65
lock: 'fs',
76
storageBackends: {
87
root: require('datastore-fs'),

src/errors/index.js

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@ class LockExistsError extends Error {
77
constructor (message) {
88
super(message)
99
this.name = 'LockExistsError'
10-
this.code = 'ERR_LOCK_EXISTS'
11-
this.message = message
10+
this.code = LockExistsError.code
1211
}
1312
}
1413

@@ -22,8 +21,7 @@ class NotFoundError extends Error {
2221
constructor (message) {
2322
super(message)
2423
this.name = 'NotFoundError'
25-
this.code = 'ERR_NOT_FOUND'
26-
this.message = message
24+
this.code = NotFoundError.code
2725
}
2826
}
2927

@@ -37,8 +35,7 @@ class InvalidRepoVersionError extends Error {
3735
constructor (message) {
3836
super(message)
3937
this.name = 'InvalidRepoVersionError'
40-
this.code = 'ERR_INVALID_REPO_VERSION'
41-
this.message = message
38+
this.code = InvalidRepoVersionError.code
4239
}
4340
}
4441

src/index.js

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -251,8 +251,7 @@ class IpfsRepo {
251251
*/
252252
async stat (options) {
253253
options = Object.assign({}, { human: false }, options)
254-
let storageMax, blocks, version, datastore, keys
255-
[storageMax, blocks, version, datastore, keys] = await Promise.all([
254+
const [storageMax, blocks, version, datastore, keys] = await Promise.all([
256255
this._storageMaxStat(),
257256
this._blockStat(),
258257
this.version.get(),
@@ -276,6 +275,10 @@ class IpfsRepo {
276275
}
277276

278277
async _isAutoMigrationEnabled () {
278+
if (this.options.autoMigrate !== undefined) {
279+
return this.options.autoMigrate
280+
}
281+
279282
let autoMigrateConfig
280283
try {
281284
autoMigrateConfig = await this.config.get(AUTO_MIGRATE_CONFIG_KEY)
@@ -287,7 +290,7 @@ class IpfsRepo {
287290
}
288291
}
289292

290-
return autoMigrateConfig && this.options.autoMigrate
293+
return autoMigrateConfig
291294
}
292295

293296
async _migrate (toVersion) {
@@ -327,7 +330,7 @@ class IpfsRepo {
327330
}
328331

329332
async function getSize (queryFn) {
330-
let sum = new Big(0)
333+
const sum = new Big(0)
331334
for await (const block of queryFn.query({})) {
332335
sum.plus(block.value.byteLength)
333336
.plus(block.key._buf.byteLength)

test/blockstore-test.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,11 @@ module.exports = (repo) => {
8585
close () {
8686

8787
}
88+
8889
has () {
8990
return true
9091
}
92+
9193
batch () {
9294
return {
9395
put () {
@@ -217,6 +219,7 @@ module.exports = (repo) => {
217219
close () {
218220

219221
}
222+
220223
get (c) {
221224
if (c.toString() === key.toString()) {
222225
throw err

test/browser.js

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,15 @@
44

55
const IPFSRepo = require('../src')
66

7-
async function createTempRepo ({ dontOpen, opts }) {
7+
async function createTempRepo (options = {}) {
88
const date = Date.now().toString()
99
const repoPath = 'test-repo-for-' + date
1010

11-
const repo = new IPFSRepo(repoPath, opts)
11+
const repo = new IPFSRepo(repoPath, options)
1212
await repo.init({})
13+
await repo.open()
1314

14-
if (!dontOpen) {
15-
await repo.open()
16-
}
17-
18-
return {
19-
path: repoPath,
20-
instance: repo
21-
}
15+
return repo
2216
}
2317

2418
describe('IPFS Repo Tests on the Browser', () => {

test/migrations-test.js

Lines changed: 42 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -36,65 +36,65 @@ module.exports = (createTempRepo) => {
3636
})
3737

3838
beforeEach(async () => {
39-
({ instance: repo } = await createTempRepo({}))
39+
repo = await createTempRepo()
4040
sinon.reset()
4141
})
4242

43-
it('should migrate by default', async () => {
44-
migrateStub.resolves()
45-
repoVersionStub.value(8)
46-
getLatestMigrationVersionStub.returns(9)
47-
48-
await repo.version.set(7)
49-
await repo.close()
43+
// Testing migration logic
44+
const migrationLogic = [
45+
{ config: true, option: true, result: true },
46+
{ config: true, option: false, result: false },
47+
{ config: true, option: undefined, result: true },
48+
{ config: false, option: true, result: true },
49+
{ config: false, option: false, result: false },
50+
{ config: false, option: undefined, result: false },
51+
{ config: undefined, option: true, result: true },
52+
{ config: undefined, option: false, result: false },
53+
{ config: undefined, option: undefined, result: true }
54+
]
55+
56+
migrationLogic.forEach(({ config, option, result }) => {
57+
it(`should ${result ? '' : 'not '}migrate when config=${config} and option=${option}`, async () => {
58+
migrateStub.resolves()
59+
repoVersionStub.value(8)
60+
getLatestMigrationVersionStub.returns(9)
61+
62+
if (config !== undefined) {
63+
await repo.config.set('repoAutoMigrate', config)
64+
}
65+
await repo.version.set(7)
66+
await repo.close()
67+
68+
const newOpts = Object.assign({}, repo.options)
69+
newOpts.autoMigrate = option
70+
const newRepo = new IPFSRepo(repo.path, newOpts)
5071

51-
expect(migrateStub.called).to.be.false()
72+
expect(migrateStub.called).to.be.false()
5273

53-
await repo.open()
74+
try {
75+
await newRepo.open()
76+
if (!result) expect.fail('should have thrown error')
77+
} catch (err) {
78+
expect(err.code).to.equal(errors.InvalidRepoVersionError.code)
79+
}
5480

55-
expect(migrateStub.called).to.be.true()
81+
expect(migrateStub.called).to.eq(result)
82+
})
5683
})
5784

58-
it('should not migrate when option autoMigrate is false', async () => {
85+
it('should migrate by default', async () => {
5986
migrateStub.resolves()
60-
repoVersionStub.resolves(8)
87+
repoVersionStub.value(8)
6188
getLatestMigrationVersionStub.returns(9)
6289

6390
await repo.version.set(7)
6491
await repo.close()
6592

66-
const newOpts = Object.assign({}, repo.options)
67-
newOpts.autoMigrate = false
68-
const newRepo = new IPFSRepo(repo.path, newOpts)
69-
70-
expect(migrateStub.called).to.be.false()
71-
try {
72-
await newRepo.open()
73-
expect.fail('should have thrown error')
74-
} catch (err) {
75-
expect(err.code).to.equal(errors.InvalidRepoVersionError.code)
76-
}
77-
7893
expect(migrateStub.called).to.be.false()
79-
})
80-
81-
it('should not migrate when config option repoAutoMigrate is false', async () => {
82-
migrateStub.resolves()
83-
repoVersionStub.resolves(8)
84-
getLatestMigrationVersionStub.returns(9)
8594

86-
await repo.config.set('repoAutoMigrate', false)
87-
await repo.version.set(7)
88-
await repo.close()
95+
await repo.open()
8996

90-
expect(migrateStub.called).to.be.false()
91-
try {
92-
await repo.open()
93-
expect.fail('should have thrown error')
94-
} catch (err) {
95-
expect(migrateStub.called).to.be.false()
96-
expect(err.code).to.equal(errors.InvalidRepoVersionError.code)
97-
}
97+
expect(migrateStub.called).to.be.true()
9898
})
9999

100100
it('should not migrate when versions matches', async () => {

test/node.js

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -17,27 +17,13 @@ const fsstat = promisify(fs.stat)
1717

1818
const IPFSRepo = require('../src')
1919

20-
async function createTempRepo ({ init, dontOpen, opts }) {
21-
const testRepoPath = path.join(__dirname, 'test-repo')
20+
async function createTempRepo (options = {}) {
2221
const date = Date.now().toString()
2322
const repoPath = path.join(os.tmpdir(), 'test-repo-for-' + date)
24-
25-
const repo = new IPFSRepo(repoPath, opts)
26-
27-
if (init) {
28-
await repo.init({})
29-
} else {
30-
await asyncNcp(testRepoPath, repoPath)
31-
}
32-
33-
if (!dontOpen) {
34-
await repo.open()
35-
}
36-
37-
return {
38-
path: repoPath,
39-
instance: repo
40-
}
23+
await asyncNcp(path.join(__dirname, 'test-repo'), repoPath)
24+
const repo = new IPFSRepo(repoPath, options)
25+
await repo.open()
26+
return repo
4127
}
4228

4329
describe('IPFS Repo Tests onNode.js', () => {
@@ -98,7 +84,7 @@ describe('IPFS Repo Tests onNode.js', () => {
9884
repos.forEach((r) => describe(r.name, () => {
9985
const testRepoPath = path.join(__dirname, 'test-repo')
10086
const date = Date.now().toString()
101-
const repoPath = testRepoPath + '-for-' + date
87+
const repoPath = path.join(os.tmpdir(), 'test-repo-for-' + date)
10288

10389
const repo = new IPFSRepo(repoPath, r.opts)
10490

@@ -112,10 +98,7 @@ describe('IPFS Repo Tests onNode.js', () => {
11298
})
11399

114100
after(async () => {
115-
try {
116-
await repo.close()
117-
} catch (e) {
118-
}
101+
await repo.close()
119102
await asyncRimraf(repoPath)
120103
})
121104

test/options-test.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@ function noop () {}
6666

6767
function expectedRepoOptions () {
6868
const options = {
69-
autoMigrate: true,
7069
lock: process.browser ? 'memory' : 'fs',
7170
storageBackends: {
7271
// packages are exchanged to browser-compatible

0 commit comments

Comments
 (0)