From ec2c97e61a9741741ad1c319aa4e8eee8e9c5b03 Mon Sep 17 00:00:00 2001 From: Andrew Chou Date: Mon, 13 Nov 2023 11:02:12 -0500 Subject: [PATCH 1/2] integrate icon plugin into MediaServer (#369) --- src/blob-api.js | 9 +- src/icon-api.js | 11 +-- src/mapeo-project.js | 38 +++++--- src/media-server.js | 7 ++ test-e2e/media-server.js | 192 +++++++++++++++++++++++++++++++++------ tests/blob-api.js | 12 +-- tests/icon-api.js | 18 ++-- 7 files changed, 210 insertions(+), 77 deletions(-) diff --git a/src/blob-api.js b/src/blob-api.js index 35415f09c..52a62e211 100644 --- a/src/blob-api.js +++ b/src/blob-api.js @@ -10,18 +10,15 @@ import b4a from 'b4a' export class BlobApi { #blobStore #getMediaBaseUrl - #projectPublicId /** * @param {object} options - * @param {string} options.projectPublicId * @param {import('./blob-store/index.js').BlobStore} options.blobStore * @param {() => Promise} options.getMediaBaseUrl */ - constructor({ projectPublicId, blobStore, getMediaBaseUrl }) { + constructor({ blobStore, getMediaBaseUrl }) { this.#blobStore = blobStore this.#getMediaBaseUrl = getMediaBaseUrl - this.#projectPublicId = projectPublicId } /** @@ -38,9 +35,7 @@ export class BlobApi { base += '/' } - return ( - base + `${this.#projectPublicId}/${driveId}/${type}/${variant}/${name}` - ) + return base + `${driveId}/${type}/${variant}/${name}` } /** diff --git a/src/icon-api.js b/src/icon-api.js index 97d94c56b..50ce7d5b4 100644 --- a/src/icon-api.js +++ b/src/icon-api.js @@ -21,7 +21,6 @@ const MIME_TO_EXTENSION = { } export class IconApi { - #projectId #dataType #dataStore #getMediaBaseUrl @@ -36,13 +35,11 @@ export class IconApi { * import('@mapeo/schema').IconValue * >} opts.iconDataType * @param {import('./datastore/index.js').DataStore<'config'>} opts.iconDataStore - * @param {string} opts.projectId * @param {() => Promise} opts.getMediaBaseUrl */ - constructor({ iconDataType, iconDataStore, projectId, getMediaBaseUrl }) { + constructor({ iconDataType, iconDataStore, getMediaBaseUrl }) { this.#dataType = iconDataType this.#dataStore = iconDataStore - this.#projectId = projectId this.#getMediaBaseUrl = getMediaBaseUrl } @@ -110,15 +107,13 @@ export class IconApi { base += '/' } - base += `${this.#projectId}/${iconId}/` - const mimeExtension = MIME_TO_EXTENSION[opts.mimeType] if (opts.mimeType === 'image/svg+xml') { - return base + `${opts.size}${mimeExtension}` + return base + `${iconId}/${opts.size}${mimeExtension}` } - return base + `${opts.size}@${opts.pixelDensity}x${mimeExtension}` + return base + `${iconId}/${opts.size}@${opts.pixelDensity}x${mimeExtension}` } } diff --git a/src/mapeo-project.js b/src/mapeo-project.js index ba06a1e87..25bd118fa 100644 --- a/src/mapeo-project.js +++ b/src/mapeo-project.js @@ -62,7 +62,6 @@ export class MapeoProject { #capabilities #ownershipWriteDone #memberApi - #projectPublicId #iconApi #syncApi #l @@ -98,7 +97,6 @@ export class MapeoProject { this.#l = Logger.create('project', logger) this.#deviceId = getDeviceId(keyManager) this.#projectId = projectKeyToId(projectKey) - this.#projectPublicId = projectKeyToPublicId(projectKey) ///////// 1. Setup database const sqlite = new Database(dbPath) @@ -137,6 +135,7 @@ export class MapeoProject { coreOwnershipTable, roleTable, deviceInfoTable, + iconTable, ], sqlite, getWinner, @@ -219,16 +218,6 @@ export class MapeoProject { }), } - this.#blobStore = new BlobStore({ - coreManager: this.#coreManager, - }) - - this.$blobs = new BlobApi({ - projectPublicId: this.#projectPublicId, - blobStore: this.#blobStore, - getMediaBaseUrl: async () => getMediaBaseUrl('blobs'), - }) - this.#coreOwnership = new CoreOwnership({ dataType: this.#dataTypes.coreOwnership, }) @@ -254,13 +243,32 @@ export class MapeoProject { }, }) + const projectPublicId = projectKeyToPublicId(projectKey) + + this.#blobStore = new BlobStore({ + coreManager: this.#coreManager, + }) + + this.$blobs = new BlobApi({ + blobStore: this.#blobStore, + getMediaBaseUrl: async () => { + let base = await getMediaBaseUrl('blobs') + if (!base.endsWith('/')) { + base += '/' + } + return base + projectPublicId + }, + }) + this.#iconApi = new IconApi({ iconDataStore: this.#dataStores.config, iconDataType: this.#dataTypes.icon, - projectId: this.#projectId, - // TODO: Update after merging https://github.com/digidem/mapeo-core-next/pull/365 getMediaBaseUrl: async () => { - throw new Error('Not yet implemented') + let base = await getMediaBaseUrl('icons') + if (!base.endsWith('/')) { + base += '/' + } + return base + projectPublicId }, }) diff --git a/src/media-server.js b/src/media-server.js index c1acc2688..cbfebcdee 100644 --- a/src/media-server.js +++ b/src/media-server.js @@ -5,6 +5,8 @@ import pTimeout from 'p-timeout' import StateMachine from 'start-stop-state-machine' import BlobServerPlugin from './fastify-plugins/blobs.js' +import IconServerPlugin from './fastify-plugins/icons.js' + import { kBlobStore } from './mapeo-project.js' export const BLOBS_PREFIX = 'blobs' @@ -44,6 +46,11 @@ export class MediaServer { }, }) + this.#fastify.register(IconServerPlugin, { + prefix: ICONS_PREFIX, + getProject, + }) + this.#serverState = new StateMachine({ start: this.#startServer.bind(this), stop: this.#stopServer.bind(this), diff --git a/test-e2e/media-server.js b/test-e2e/media-server.js index 8d7543c6d..39a68eada 100644 --- a/test-e2e/media-server.js +++ b/test-e2e/media-server.js @@ -1,9 +1,10 @@ import { test } from 'brittle' +import { randomBytes } from 'crypto' import { join } from 'path' import { fileURLToPath } from 'url' import { KeyManager } from '@mapeo/crypto' import FakeTimers from '@sinonjs/fake-timers' -import { Agent, fetch } from 'undici' +import { Agent, fetch as uFetch } from 'undici' import fs from 'fs/promises' import RAM from 'random-access-memory' @@ -13,7 +14,7 @@ const BLOB_FIXTURES_DIR = fileURLToPath( new URL('../tests/fixtures/blob-api/', import.meta.url) ) -test('retrieving blobs urls', async (t) => { +test('retrieving blobs using url', async (t) => { const clock = FakeTimers.install({ shouldAdvanceTime: true }) t.teardown(() => clock.uninstall()) @@ -25,16 +26,13 @@ test('retrieving blobs urls', async (t) => { const project = await manager.getProject(await manager.createProject()) - const blobId = await project.$blobs.create( - { - original: join(BLOB_FIXTURES_DIR, 'original.png'), - }, - { mimeType: 'image/png' } - ) + await project.ready() const exceptionPromise1 = t.exception(async () => { await project.$blobs.getUrl({ - ...blobId, + driveId: randomBytes(32).toString('hex'), + name: 'foo', + type: 'photo', variant: 'original', }) }, 'getting blob url fails if manager.start() has not been called yet') @@ -44,36 +42,174 @@ test('retrieving blobs urls', async (t) => { await manager.start() - const blobUrl = await project.$blobs.getUrl({ - ...blobId, - variant: 'original', + await t.test('blob does not exist', async (st) => { + const blobUrl = await project.$blobs.getUrl({ + driveId: randomBytes(32).toString('hex'), + name: 'foo', + type: 'photo', + variant: 'original', + }) + + st.ok( + new URL(blobUrl), + 'retrieving url based on media server resolves after starting it' + ) + + const response = await fetch(blobUrl) + + st.is(response.status, 404, 'response is 404') }) - t.ok( - new URL(blobUrl), - 'retrieving url based on media server resolves after starting it' - ) + await t.test('blob exists', async (st) => { + const blobId = await project.$blobs.create( + { original: join(BLOB_FIXTURES_DIR, 'original.png') }, + { mimeType: 'image/png' } + ) - const response = await fetch(blobUrl, { - // Noticed that the process was hanging (on Node 18, at least) after calling manager.stop() further below - // Probably related to https://github.com/nodejs/undici/issues/2348 - // Adding the below seems to fix it - dispatcher: new Agent({ keepAliveMaxTimeout: 100 }), + const blobUrl = await project.$blobs.getUrl({ + ...blobId, + variant: 'original', + }) + + st.ok( + new URL(blobUrl), + 'retrieving url based on media server resolves after starting it' + ) + + const response = await fetch(blobUrl) + + st.is(response.status, 200, 'response status ok') + st.is( + response.headers.get('content-type'), + 'image/png', + 'matching content type header' + ) + + const expected = await fs.readFile(join(BLOB_FIXTURES_DIR, 'original.png')) + const body = Buffer.from(await response.arrayBuffer()) + + st.alike(body, expected, 'matching reponse body') }) - t.is(response.status, 200) - t.is(response.headers.get('content-type'), 'image/png') - const expected = await fs.readFile(join(BLOB_FIXTURES_DIR, 'original.png')) - const body = Buffer.from(await response.arrayBuffer()) - t.alike(body, expected) + await manager.stop() + + const exceptionPromise2 = t.exception(async () => { + await project.$blobs.getUrl({ + driveId: randomBytes(32).toString('hex'), + name: 'foo', + type: 'photo', + variant: 'original', + }) + }, 'getting url after manager.stop() has been called fails') + clock.tick(100_000) + await exceptionPromise2 +}) + +test('retrieving icons using url', async (t) => { + const clock = FakeTimers.install({ shouldAdvanceTime: true }) + t.teardown(() => clock.uninstall()) + + const manager = new MapeoManager({ + rootKey: KeyManager.generateRootKey(), + dbFolder: ':memory:', + coreStorage: () => new RAM(), + }) + + const project = await manager.getProject(await manager.createProject()) + + await project.ready() + + const exceptionPromise1 = t.exception(async () => { + await project.$icons.getIconUrl(randomBytes(32).toString('hex'), { + mimeType: 'image/png', + pixelDensity: 1, + size: 'small', + }) + }, 'getting icon url fails if manager.start() has not been called yet') + + clock.tick(100_000) + await exceptionPromise1 + + await manager.start() + + await t.test('icon does not exist', async (st) => { + const nonExistentIconId = randomBytes(32).toString('hex') + + const iconUrl = await project.$icons.getIconUrl(nonExistentIconId, { + size: 'small', + mimeType: 'image/png', + pixelDensity: 1, + }) + + st.ok( + new URL(iconUrl), + 'retrieving url based on media server resolves after starting it' + ) + + const response = await fetch(iconUrl) + + st.is(response.status, 404, 'response is 404') + }) + + await t.test('icon exists', async (st) => { + const iconBuffer = randomBytes(128) + + const iconId = await project.$icons.create({ + name: 'foo', + variants: [ + { + blob: iconBuffer, + mimeType: 'image/png', + pixelDensity: 1, + size: 'small', + }, + ], + }) + + const iconUrl = await project.$icons.getIconUrl(iconId, { + size: 'small', + mimeType: 'image/png', + pixelDensity: 1, + }) + + st.ok( + new URL(iconUrl), + 'retrieving url based on media server resolves after starting it' + ) + + const response = await fetch(iconUrl) + + st.is(response.status, 200, 'response status ok') + st.is( + response.headers.get('content-type'), + 'image/png', + 'matching content type header' + ) + const body = Buffer.from(await response.arrayBuffer()) + st.alike(body, iconBuffer, 'matching response body') + }) await manager.stop() const exceptionPromise2 = t.exception(async () => { - await project.$blobs.getUrl({ ...blobId, variant: 'original' }) + await project.$icons.getIconUrl(randomBytes(32).toString('hex'), { + mimeType: 'image/png', + pixelDensity: 1, + size: 'small', + }) }, 'getting url after manager.stop() has been called fails') clock.tick(100_000) await exceptionPromise2 }) -// TODO: Add icon urls test here +/** + * @param {string} url + */ +async function fetch(url) { + return uFetch(url, { + // Noticed that the process was hanging (on Node 18, at least) after calling manager.stop() further below + // Probably related to https://github.com/nodejs/undici/issues/2348 + // Adding the below seems to fix it + dispatcher: new Agent({ keepAliveMaxTimeout: 100 }), + }) +} diff --git a/tests/blob-api.js b/tests/blob-api.js index 4d0d5d278..29e653e90 100644 --- a/tests/blob-api.js +++ b/tests/blob-api.js @@ -1,11 +1,10 @@ // @ts-check import { join } from 'node:path' import * as fs from 'node:fs/promises' -import { createHash, randomBytes } from 'node:crypto' +import { createHash } from 'node:crypto' import { fileURLToPath } from 'url' import test from 'brittle' import { BlobApi } from '../src/blob-api.js' -import { projectKeyToPublicId } from '../src/utils.js' import { createBlobStore } from './helpers/blob-store.js' @@ -13,7 +12,6 @@ test('create blobs', async (t) => { const { blobStore } = createBlobStore() const blobApi = new BlobApi({ - projectPublicId: projectKeyToPublicId(randomBytes(32)), blobStore, getMediaBaseUrl: async () => 'http://127.0.0.1:8080/blobs', }) @@ -43,7 +41,6 @@ test('create blobs', async (t) => { }) test('get url from blobId', async (t) => { - const projectPublicId = projectKeyToPublicId(randomBytes(32)) const type = 'photo' const variant = 'original' const name = '1234' @@ -55,7 +52,6 @@ test('get url from blobId', async (t) => { let prefix = undefined const blobApi = new BlobApi({ - projectPublicId, blobStore, getMediaBaseUrl: async () => `http://127.0.0.1:${port}/${prefix || ''}`, }) @@ -70,7 +66,7 @@ test('get url from blobId', async (t) => { t.is( url, - `http://127.0.0.1:${port}/${projectPublicId}/${blobStore.writerDriveId}/${type}/${variant}/${name}` + `http://127.0.0.1:${port}/${blobStore.writerDriveId}/${type}/${variant}/${name}` ) } @@ -87,7 +83,7 @@ test('get url from blobId', async (t) => { t.is( url, - `http://127.0.0.1:${port}/${projectPublicId}/${blobStore.writerDriveId}/${type}/${variant}/${name}` + `http://127.0.0.1:${port}/${blobStore.writerDriveId}/${type}/${variant}/${name}` ) } @@ -104,7 +100,7 @@ test('get url from blobId', async (t) => { t.is( url, - `http://127.0.0.1:${port}/${prefix}/${projectPublicId}/${blobStore.writerDriveId}/${type}/${variant}/${name}` + `http://127.0.0.1:${port}/${prefix}/${blobStore.writerDriveId}/${type}/${variant}/${name}` ) } }) diff --git a/tests/icon-api.js b/tests/icon-api.js index 51c736d4b..d60bc9881 100644 --- a/tests/icon-api.js +++ b/tests/icon-api.js @@ -152,7 +152,7 @@ test('[kGetIconBlob]()', async (t) => { test(`getIconUrl()`, async (t) => { let mediaBaseUrl = 'http://127.0.0.1:8080/icons/' - const { iconApi, projectId } = setup({ + const { iconApi } = setup({ getMediaBaseUrl: async () => mediaBaseUrl, }) @@ -167,7 +167,7 @@ test(`getIconUrl()`, async (t) => { t.is( url, - mediaBaseUrl + `${projectId}/${iconId}/small@1x.png`, + mediaBaseUrl + `${iconId}/small@1x.png`, 'returns expected bitmap icon url' ) } @@ -180,13 +180,13 @@ test(`getIconUrl()`, async (t) => { t.is( url, - mediaBaseUrl + `${projectId}/${iconId}/small.svg`, + mediaBaseUrl + `${iconId}/small.svg`, 'returns expected svg icon url' ) } - // Change media base url (e.g. port changes) - mediaBaseUrl = 'http://127.0.0.1:3000/' + // Change media base url (e.g. host or port changes) + mediaBaseUrl = 'http://0.0.0.0:3000/icons/' { const url = await iconApi.getIconUrl(iconId, { @@ -197,7 +197,7 @@ test(`getIconUrl()`, async (t) => { t.is( url, - mediaBaseUrl + `${projectId}/${iconId}/medium@2x.png`, + mediaBaseUrl + `${iconId}/medium@2x.png`, 'returns expected bitmap icon url after media base url changes' ) } @@ -210,7 +210,7 @@ test(`getIconUrl()`, async (t) => { t.is( url, - mediaBaseUrl + `${projectId}/${iconId}/large.svg`, + mediaBaseUrl + `${iconId}/large.svg`, 'returns expected svg icon url after media base url changes' ) } @@ -649,17 +649,13 @@ function setup({ db, }) - const projectId = randomBytes(32).toString('hex') - const iconApi = new IconApi({ iconDataStore, iconDataType, - projectId, getMediaBaseUrl, }) return { - projectId, iconApi, iconDataType, } From 4a18a9cbd80664b5cd010f23e1e136acc91960f9 Mon Sep 17 00:00:00 2001 From: Andrew Chou Date: Mon, 13 Nov 2023 11:14:02 -0500 Subject: [PATCH 2/2] fix: fix writing to blob store when also calculating content hash (#370) Co-authored-by: Gregor MacLennan --- src/blob-api.js | 103 ++++++++++++++++------------------------------ tests/blob-api.js | 5 +-- 2 files changed, 38 insertions(+), 70 deletions(-) diff --git a/src/blob-api.js b/src/blob-api.js index 52a62e211..ce2b1032b 100644 --- a/src/blob-api.js +++ b/src/blob-api.js @@ -1,8 +1,7 @@ import fs from 'node:fs' -import { pipeline } from 'node:stream/promises' -import { createHash } from 'node:crypto' -import sodium from 'sodium-universal' -import b4a from 'b4a' +// @ts-expect-error - pipelinePromise missing from streamx types +import { Transform, pipelinePromise as pipeline } from 'streamx' +import { createHash, randomBytes } from 'node:crypto' /** @typedef {import('./types.js').BlobId} BlobId */ /** @typedef {import('./types.js').BlobType} BlobType */ @@ -47,85 +46,55 @@ export class BlobApi { async create(filepaths, metadata) { const { original, preview, thumbnail } = filepaths const { mimeType } = metadata - const blobType = getType(mimeType) - const hash = b4a.alloc(8) - sodium.randombytes_buf(hash) - const name = hash.toString('hex') - - const contentHash = createHash('sha256') - - await this.writeFile( - original, - { - name: `${name}`, - variant: 'original', - type: blobType, - }, - metadata - // contentHash + const type = getType(mimeType) + const name = randomBytes(8).toString('hex') + const hash = createHash('sha256') + + const ws = this.#blobStore.createWriteStream( + { type, variant: 'original', name }, + { metadata } ) + const writePromises = [ + pipeline(fs.createReadStream(original), hashTransform(hash), ws), + ] if (preview) { - await this.writeFile( - preview, - { - name: `${name}`, - variant: 'preview', - type: blobType, - }, - metadata + const ws = this.#blobStore.createWriteStream( + { type, variant: 'preview', name }, + { metadata } ) + writePromises.push(pipeline(fs.createReadStream(preview), ws)) } if (thumbnail) { - await this.writeFile( - thumbnail, - { - name: `${name}`, - variant: 'thumbnail', - type: blobType, - }, - metadata + const ws = this.#blobStore.createWriteStream( + { type, variant: 'thumbnail', name }, + { metadata } ) + writePromises.push(pipeline(fs.createReadStream(thumbnail), ws)) } + await Promise.all(writePromises) + return { driveId: this.#blobStore.writerDriveId, name, - type: blobType, - hash: contentHash.digest('hex'), + type, + hash: hash.digest('hex'), } } +} - /** - * @param {string} filepath - * @param {Omit} options - * @param {object} metadata - * @param {string} metadata.mimeType - * @param {import('node:crypto').Hash} [hash] - */ - async writeFile(filepath, { name, variant, type }, metadata, hash) { - if (hash) { - // @ts-ignore TODO: return value types don't match pipeline's expectations, though they should - await pipeline( - fs.createReadStream(filepath), - hash, - - // @ts-ignore TODO: remove driveId property from createWriteStream - this.#blobStore.createWriteStream({ type, variant, name }, { metadata }) - ) - - return { name, variant, type, hash } - } - - // @ts-ignore TODO: return value types don't match pipeline's expectations, though they should - await pipeline( - fs.createReadStream(filepath), - this.#blobStore.createWriteStream({ type, variant, name }, { metadata }) - ) - - return { name, variant, type } - } +/** + * @param {import('node:crypto').Hash} hash + */ +function hashTransform(hash) { + return new Transform({ + transform: (data, cb) => { + hash.update(data) + cb(null, data) + }, + }) } /** diff --git a/tests/blob-api.js b/tests/blob-api.js index 29e653e90..15063fd47 100644 --- a/tests/blob-api.js +++ b/tests/blob-api.js @@ -22,6 +22,7 @@ test('create blobs', async (t) => { const hash = createHash('sha256') const originalContent = await fs.readFile(join(directory, 'original.png')) + hash.update(originalContent) const attachment = await blobApi.create( @@ -35,9 +36,7 @@ test('create blobs', async (t) => { t.is(attachment.driveId, blobStore.writerDriveId) t.is(attachment.type, 'photo') - // TODO: Need to fix BlobApi implementation - // https://github.com/digidem/mapeo-core-next/pull/365#pullrequestreview-1716846341 - // t.alike(attachment.hash, hash.digest('hex')) + t.alike(attachment.hash, hash.digest('hex')) }) test('get url from blobId', async (t) => {