-
Notifications
You must be signed in to change notification settings - Fork 4
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: identity CIDs use contentTypeParser #49
fix: identity CIDs use contentTypeParser #49
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self review of test changes. added some info as comments
@@ -97,7 +97,7 @@ | |||
"@sgtpooki/file-type": "^1.0.1", | |||
"@types/sinon": "^17.0.3", | |||
"aegir": "^42.2.5", | |||
"blockstore-core": "^4.4.0", | |||
"blockstore-core": "^4.4.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update to get ipfs/js-stores#303
import Sinon from 'sinon' | ||
import { fromString as uint8ArrayFromString } from 'uint8arrays/from-string' | ||
import { createVerifiedFetch } from '../src/index.js' | ||
import { VerifiedFetch } from '../src/verified-fetch.js' | ||
import { createHelia } from './fixtures/create-offline-helia.js' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use the offline helia.
it('can properly set content type for identity CIDs', async () => { | ||
verifiedFetch = new VerifiedFetch({ | ||
helia | ||
}, { | ||
contentTypeParser: async (data) => { | ||
return 'text/plain' | ||
} | ||
}) | ||
const resp = await verifiedFetch.fetch(CID.parse('bafkqablimvwgy3y')) | ||
expect(resp.headers.get('content-type')).to.equal('text/plain') | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test for this fix
import { MemoryDatastore } from 'datastore-core' | ||
import type { HeliaHTTPInit } from '@helia/http' | ||
import type { Helia } from '@helia/interface' | ||
|
||
export async function createHelia (init: Partial<HeliaHTTPInit> = {}): Promise<Helia> { | ||
const datastore = new MemoryDatastore() | ||
const blockstore = new MemoryBlockstore() | ||
const blockstore = new IdentityBlockstore(new MemoryBlockstore()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
allow identity blocks when testing
CI action hung for some reason. cancelled and restarted |
## @helia/verified-fetch [1.3.9](https://github.com/ipfs/helia-verified-fetch/compare/@helia/verified-fetch-1.3.8...@helia/verified-fetch-1.3.9) (2024-04-11) ### Bug Fixes * identity CIDs use contentTypeParser ([#49](#49)) ([3014498](3014498))
🎉 This PR is included in version 1.3.9 🎉 The release is available on npm package (@latest dist-tag) Your semantic-release bot 📦🚀 |
🎉 This PR is included in version 1.16.0 🎉 The release is available on npm package (@latest dist-tag) Your semantic-release bot 📦🚀 |
🎉 This PR is included in version 1.0.0 🎉 The release is available on npm package (@latest dist-tag) Your semantic-release bot 📦🚀 |
Title
fix: identity CIDs use contentTypeParser
Description
latest update of
@helia/verified-fetch
caused failing tests in service-worker-gateway: ipfs/service-worker-gateway#187Fixes #48
Notes & open questions
This change normalizes how we handle setting content-type for raw CIDs and identity CIDs and was required to ensure that the content-type is set correctly for identity CIDs.
Change checklist