Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use unixfs exporter to traverse DAGs #455

Merged
merged 7 commits into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions packages/interop/src/verified-fetch-unixfs-dir.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,7 @@ describe('@helia/verified-fetch - unixfs directory', () => {
})
})

// TODO: find a smaller car file so the test doesn't timeout locally or flake on CI
describe.skip('HAMT-sharded directory', () => {
describe('HAMT-sharded directory', () => {
before(async () => {
// from https://github.com/ipfs/gateway-conformance/blob/193833b91f2e9b17daf45c84afaeeae61d9d7c7e/fixtures/trustless_gateway_car/single-layer-hamt-with-multi-block-files.car
await loadFixtureDataCar(controller, 'bafybeidbclfqleg2uojchspzd4bob56dqetqjsj27gy2cq3klkkgxtpn4i-single-layer-hamt-with-multi-block-files.car')
Expand Down
2 changes: 1 addition & 1 deletion packages/unixfs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@
"ipfs-unixfs": "^11.1.3",
"ipfs-unixfs-exporter": "^13.5.0",
"ipfs-unixfs-importer": "^15.2.4",
"it-all": "^3.0.4",
"it-glob": "^2.0.6",
"it-last": "^3.0.4",
"it-pipe": "^3.0.1",
Expand All @@ -183,7 +184,6 @@
"blockstore-core": "^4.4.0",
"delay": "^6.0.0",
"iso-url": "^1.2.1",
"it-all": "^3.0.4",
"it-drain": "^3.0.5",
"it-first": "^3.0.4",
"it-to-buffer": "^4.0.5",
Expand Down
57 changes: 9 additions & 48 deletions packages/unixfs/src/commands/utils/resolve.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { logger } from '@libp2p/logger'
import { exporter } from 'ipfs-unixfs-exporter'
import { DoesNotExistError, InvalidParametersError } from '../../errors.js'
import { walkPath } from 'ipfs-unixfs-exporter'
import all from 'it-all'
import { DoesNotExistError } from '../../errors.js'
import { addLink } from './add-link.js'
import { cidToDirectory } from './cid-to-directory.js'
import { cidToPBLink } from './cid-to-pblink.js'
Expand Down Expand Up @@ -37,57 +38,17 @@
return { cid }
}

log('resolve "%s" under %c', path, cid)

const parts = path.split('/').filter(Boolean)
const segments: Segment[] = [{
name: '',
cid,
size: 0n
}]

for (let i = 0; i < parts.length; i++) {
const part = parts[i]
const result = await exporter(cid, blockstore, options)

log('resolving "%s"', part, result)

if (result.type === 'file') {
if (i < parts.length - 1) {
throw new InvalidParametersError('Path was invalid')
}

cid = result.cid
} else if (result.type === 'directory') {
let dirCid: CID | undefined

for await (const entry of result.content()) {
if (entry.name === part) {
dirCid = entry.cid
break
}
}

if (dirCid == null) {
throw new DoesNotExistError('Could not find path in directory')
}

cid = dirCid

segments.push({
name: part,
cid,
size: result.size
})
} else {
throw new InvalidParametersError('Could not resolve path')
}
const p = `/ipfs/${cid}${path == null ? '' : `/${path}`}`
const segments = await all(walkPath(p, blockstore, options))

if (segments.length === 0) {
throw new DoesNotExistError('Could not find path in directory')

Check warning on line 45 in packages/unixfs/src/commands/utils/resolve.ts

View check run for this annotation

Codecov / codecov/patch

packages/unixfs/src/commands/utils/resolve.ts#L45

Added line #L45 was not covered by tests
}
Comment on lines +41 to 46
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is so much cleaner.


log('resolved %s to %c', path, cid)

return {
cid,
cid: segments[segments.length - 1].cid,
path,
segments
}
Expand Down
39 changes: 39 additions & 0 deletions packages/unixfs/test/cat.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import { expect } from 'aegir/chai'
import { MemoryBlockstore } from 'blockstore-core'
import all from 'it-all'
import drain from 'it-drain'
import toBuffer from 'it-to-buffer'
import { unixfs, type UnixFS } from '../src/index.js'
Expand Down Expand Up @@ -92,4 +93,42 @@ describe('cat', () => {

expect(bytes).to.deep.equal(smallFile)
})

it('should only load blocks necessary to traverse a HAMT', async () => {
const [, scriptFile, styleFile, imageFile, dir] = await all(fs.addAll([{
path: 'index.html',
content: Uint8Array.from([0, 1, 2])
}, {
path: 'script.js',
content: Uint8Array.from([3, 4, 5])
}, {
path: 'style.css',
content: Uint8Array.from([6, 7, 8])
}, {
path: 'image.png',
content: Uint8Array.from([9, 0, 1])
}], {
shardSplitThresholdBytes: 1,
wrapWithDirectory: true
}))

const dirStat = await fs.stat(dir.cid)
expect(dirStat.unixfs?.type).to.equal('hamt-sharded-directory')

// remove all blocks that aren't the index file
await drain(blockstore.deleteMany([
scriptFile.cid,
styleFile.cid,
imageFile.cid
]))

// should be able to cat the index file without loading the other files
// in the shard - the blockstore is offline so will throw if requested
// blocks are not present
const bytes = await toBuffer(fs.cat(dir.cid, {
Comment on lines +125 to +128
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Nice 👍

path: 'index.html'
}))

expect(bytes).to.equalBytes(Uint8Array.from([0, 1, 2]))
})
})
8 changes: 4 additions & 4 deletions packages/unixfs/test/rm.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ describe('rm', () => {
await expect(fs.stat(updatedDirCid, {
path
})).to.eventually.be.rejected
.with.property('code', 'ERR_DOES_NOT_EXIST')
.with.property('code', 'ERR_NOT_FOUND')
})

it('removes a directory', async () => {
Expand All @@ -49,7 +49,7 @@ describe('rm', () => {
await expect(fs.stat(updatedDirCid, {
path
})).to.eventually.be.rejected
.with.property('code', 'ERR_DOES_NOT_EXIST')
.with.property('code', 'ERR_NOT_FOUND')
})

it('removes a sharded directory inside a normal directory', async () => {
Expand All @@ -67,7 +67,7 @@ describe('rm', () => {
await expect(fs.stat(updatedContainingDirCid, {
path: dirName
})).to.eventually.be.rejected
.with.property('code', 'ERR_DOES_NOT_EXIST')
.with.property('code', 'ERR_NOT_FOUND')
})

it('removes a sharded directory inside a sharded directory', async () => {
Expand All @@ -86,7 +86,7 @@ describe('rm', () => {
await expect(fs.stat(updatedContainingDirCid, {
path: dirName
})).to.eventually.be.rejected
.with.property('code', 'ERR_DOES_NOT_EXIST')
.with.property('code', 'ERR_NOT_FOUND')

expect(updatedContainingDirCid.toString()).to.equal(shardedDirCid.toString(), 'adding and removing a file from a sharded directory did not result in the original sharded CID')
})
Expand Down
Loading