Skip to content

Commit c08a42f

Browse files
authored
fix: correct hamt structure when modifying deep sub-shards (ipfs#46)
If you have a shard like: ``` F1 A0 C8myfile ``` And you add a file that causes `C8myfile` to become a subshard, previously you would end up with this: ``` F1 A0 C8 C8 B3myfile 82newfile ``` This PR ensures you get the correct structure: ``` F1 A0 C8 B3myfile 82newfile ``` When we update a shard, we re-create a portion of the shard in order to update the existing structure, this avoids loading the entire shard which could be expensive. Previously we weren't descending into the newly created sub-shard to add the correct portion to the existing shard. Fixes ipfs#45
1 parent a389cbf commit c08a42f

File tree

3 files changed

+94
-10
lines changed

3 files changed

+94
-10
lines changed

src/core/utils/add-link.js

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const CID = require('cids')
88
const waterfall = require('async/waterfall')
99
const DirSharded = require('ipfs-unixfs-importer/src/importer/dir-sharded')
1010
const series = require('async/series')
11+
const whilst = require('async/whilst')
1112
const log = require('debug')('ipfs:mfs:core:utils:add-link')
1213
const UnixFS = require('ipfs-unixfs')
1314
const {
@@ -127,22 +128,22 @@ const addToShardedDirectory = (context, options, callback) => {
127128
return waterfall([
128129
(cb) => generatePath(context, options.name, options.parent, cb),
129130
({ rootBucket, path }, cb) => {
130-
updateShard(context, path, {
131+
updateShard(context, path.reverse(), {
131132
name: options.name,
132133
cid: options.cid,
133134
size: options.size
134-
}, options, (err, result = {}) => cb(err, { rootBucket, ...result }))
135+
}, 0, options, (err, result = {}) => cb(err, { rootBucket, ...result }))
135136
},
136137
({ rootBucket, node }, cb) => updateHamtDirectory(context, node.links, rootBucket, options, cb)
137138
], callback)
138139
}
139140

140-
const updateShard = (context, positions, child, options, callback) => {
141+
const updateShard = (context, positions, child, index, options, callback) => {
141142
const {
142143
bucket,
143144
prefix,
144145
node
145-
} = positions.pop()
146+
} = positions[index]
146147

147148
const link = node.links
148149
.find(link => link.name.substring(0, 2) === prefix && link.name !== `${prefix}${child.name}`)
@@ -163,9 +164,41 @@ const updateShard = (context, positions, child, options, callback) => {
163164
multihash: child.cid.buffer
164165
}], {}, done),
165166
({ node: { links: [ shard ] } }, done) => {
166-
return context.ipld.get(shard.cid, (err, result) => {
167-
done(err, { cid: shard.cid, node: result && result.value })
168-
})
167+
let position = 0
168+
169+
// step through the shard until we find the newly created sub-shard
170+
return whilst(
171+
() => position < positions.length - 1,
172+
(next) => {
173+
const shardPrefix = positions[position].prefix
174+
175+
log(`Prefix at position ${position} is ${shardPrefix} - shard.name ${shard.name}`)
176+
177+
if (shard.name.substring(0, 2) !== shardPrefix) {
178+
return next(new Error(`Unexpected prefix ${shard.name} !== ${shardPrefix}, position ${position}`))
179+
}
180+
181+
position++
182+
183+
context.ipld.get(shard.cid, (err, result) => {
184+
if (err) {
185+
return next(err)
186+
}
187+
188+
if (position < positions.length) {
189+
const nextPrefix = positions[position].prefix
190+
const nextShard = result.value.links.find(link => link.name.substring(0, 2) === nextPrefix)
191+
192+
if (nextShard) {
193+
shard = nextShard
194+
}
195+
}
196+
197+
next(err, { cid: result && result.cid, node: result && result.value })
198+
})
199+
},
200+
done
201+
)
169202
},
170203
(result, cb) => updateShardParent(context, bucket, node, link.name, result.node, result.cid, prefix, options, cb)
171204
], cb)
@@ -175,7 +208,7 @@ const updateShard = (context, positions, child, options, callback) => {
175208
log(`Descending into sub-shard ${link.name} for ${child.name}`)
176209

177210
return waterfall([
178-
(cb) => updateShard(context, positions, child, options, cb),
211+
(cb) => updateShard(context, positions, child, index + 1, options, cb),
179212
(result, cb) => updateShardParent(context, bucket, node, link.name, result.node, result.cid, prefix, options, cb)
180213
], cb)
181214
}

test/helpers/create-shard.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ const createShard = (ipld, files, shardSplitThreshold = 10) => {
1717
}),
1818
collect((err, files) => {
1919
if (err) {
20-
return reject(files)
20+
return reject(err)
2121
}
2222

2323
const dir = files[files.length - 1]

test/write.spec.js

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,11 @@ const {
1515
createMfs,
1616
cidAtPath,
1717
createShardedDirectory,
18-
createTwoShards
18+
createTwoShards,
19+
createShard
1920
} = require('./helpers')
2021
const CID = require('cids')
22+
const crypto = require('crypto')
2123

2224
let fs, tempWrite
2325

@@ -881,4 +883,53 @@ describe('write', () => {
881883
expect(stats.type).to.equal('hamt-sharded-directory')
882884
expect(updatedDirCid.toBaseEncodedString()).to.deep.equal(dirWithAllFiles.toBaseEncodedString())
883885
})
886+
887+
it('results in the same hash as a sharded directory created by the importer when causing a subshard of a subshard to be created', async function () {
888+
this.timeout(60000)
889+
890+
const dir = `/some-dir-${Date.now()}`
891+
892+
const nodeGrContent = Buffer.from([0, 1, 2, 3, 4])
893+
const superModuleContent = Buffer.from([5, 6, 7, 8, 9])
894+
895+
const dirCid = await createShard(mfs.ipld, [{
896+
path: `${dir}/node-gr`,
897+
content: nodeGrContent
898+
}, {
899+
path: `${dir}/yanvoidmodule`,
900+
content: crypto.randomBytes(5)
901+
}, {
902+
path: `${dir}/methodify`,
903+
content: crypto.randomBytes(5)
904+
}, {
905+
path: `${dir}/fis-msprd-style-loader_0_13_1`,
906+
content: crypto.randomBytes(5)
907+
}, {
908+
path: `${dir}/js-form`,
909+
content: crypto.randomBytes(5)
910+
}, {
911+
path: `${dir}/vivanov-sliceart`,
912+
content: crypto.randomBytes(5)
913+
}], 1)
914+
915+
await mfs.cp(`/ipfs/${dirCid.toBaseEncodedString()}`, dir)
916+
917+
await mfs.write(`${dir}/supermodule_test`, superModuleContent, {
918+
create: true
919+
})
920+
921+
await mfs.stat(`${dir}/supermodule_test`)
922+
await mfs.stat(`${dir}/node-gr`)
923+
924+
expect(await mfs.read(`${dir}/node-gr`)).to.deep.equal(nodeGrContent)
925+
expect(await mfs.read(`${dir}/supermodule_test`)).to.deep.equal(superModuleContent)
926+
927+
await mfs.rm(`${dir}/supermodule_test`)
928+
929+
try {
930+
await mfs.stat(`${dir}/supermodule_test`)
931+
} catch (err) {
932+
expect(err.message).to.contain('not exist')
933+
}
934+
})
884935
})

0 commit comments

Comments
 (0)