fix: db client improvements (#546)

Co-authored-by: Alan Shaw <[email protected]>

packages/api/src/car.js

@@ -3,16 +3,15 @@ import { PutObjectCommand } from '@aws-sdk/client-s3'
 import { CarBlockIterator } from '@ipld/car'
 import { toString } from 'uint8arrays'
 import { Block } from 'multiformats/block'
-import { CID } from 'multiformats/cid'
 import { sha256 } from 'multiformats/hashes/sha2'
 import * as raw from 'multiformats/codecs/raw'
 import * as cbor from '@ipld/dag-cbor'
 import * as pb from '@ipld/dag-pb'
 import retry from 'p-retry'
 import { GATEWAY, LOCAL_ADD_THRESHOLD, MAX_BLOCK_SIZE } from './constants.js'
-import { ErrorInvalidCid } from './errors.js'
 import { JSONResponse } from './utils/json-response.js'
 import { toPinStatusEnum } from './utils/pin.js'
+import { normalizeCid } from './utils/normalize-cid.js'
 
 /**
  * @typedef {import('multiformats/cid').CID} CID
@@ -136,14 +135,15 @@ export async function handleCarUpload (request, env, ctx, car, uploadType = 'Car
     name = `Upload at ${new Date().toISOString()}`
   }
 
+  const normalizedCid = normalizeCid(cid)
   // Store in DB
   // Retried because it's possible to receive the error:
   // "Transaction was aborted due to detection of concurrent modification."
-  const { createUpload: upload } = await retry(() => (
+  await retry(() => (
     env.db.createUpload({
       user: user._id,
       authKey: authToken?._id,
-      contentCid: parseCid(cid),
+      contentCid: normalizedCid,
       sourceCid: cid,
       name,
       type: uploadType,
@@ -179,7 +179,7 @@ export async function handleCarUpload (request, env, ctx, car, uploadType = 'Car
         if (!okPins.length) continue
 
         for (const pin of okPins) {
-          await env.db.upsertPin(upload.content._id, pin)
+          await env.db.upsertPin(normalizedCid, pin)
         }
         return
       }
@@ -347,17 +347,3 @@ function toPins (peerMap) {
     location: { peerId, peerName }
   }))
 }
-
-/**
- * Parse CID and return v1 and original
- *
- * @param {string} cid
- */
-function parseCid (cid) {
-  try {
-    const c = CID.parse(cid)
-    return c.toV1().toString()
-  } catch (err) {
-    throw new ErrorInvalidCid(cid)
-  }
-}

packages/api/src/errors.js

@@ -56,15 +56,15 @@ export class MagicTokenRequiredError extends HTTPError {
 }
 MagicTokenRequiredError.CODE = 'ERROR_MAGIC_TOKEN_REQUIRED'
 
-export class ErrorInvalidCid extends Error {
+export class InvalidCidError extends Error {
   /**
    * @param {string} cid
    */
   constructor (cid) {
     super(`Invalid CID: ${cid}`)
     this.name = 'InvalidCid'
     this.status = 400
-    this.code = ErrorInvalidCid.CODE
+    this.code = InvalidCidError.CODE
   }
 }
-ErrorInvalidCid.CODE = 'ERROR_INVALID_CID'
+InvalidCidError.CODE = 'ERROR_INVALID_CID'

packages/api/src/status.js

@@ -1,4 +1,5 @@
 import { JSONResponse, notFound } from './utils/json-response.js'
+import { normalizeCid } from './utils/normalize-cid.js'
 
 /**
  * Returns pin and deal status info for a given CID.
@@ -10,11 +11,15 @@ import { JSONResponse, notFound } from './utils/json-response.js'
  */
 export async function statusGet (request, env) {
   const cid = request.params.cid
-  const res = await env.db.getStatus(cid)
+  const normalizedCid = normalizeCid(cid)
+  const res = await env.db.getStatus(normalizedCid)
 
   if (!res) {
     return notFound()
   }
 
+  // replace content cid for source cid in response
+  res.cid = cid
+
   return new JSONResponse(res)
 }

packages/api/src/user.js

@@ -196,7 +196,7 @@ export async function userUploadsDelete (request, env) {
   const cid = request.params.cid
   const user = request.auth.user._id
 
-  const res = await env.db.deleteUpload(cid, user)
+  const res = await env.db.deleteUpload(user, cid)
   return new JSONResponse(res)
 }
 
@@ -211,6 +211,6 @@ export async function userUploadsRename (request, env) {
   const { cid } = request.params
   const { name } = await request.json()
 
-  const res = await env.db.renameUpload(cid, user, name)
+  const res = await env.db.renameUpload(user, cid, name)
   return new JSONResponse(res)
 }

packages/api/src/utils/normalize-cid.js

@@ -0,0 +1,16 @@
+import { CID } from 'multiformats/cid'
+import { InvalidCidError } from '../errors.js'
+
+/**
+ * Parse CID and return normalized b32 v1
+ *
+ * @param {string} cid
+ */
+export function normalizeCid (cid) {
+  try {
+    const c = CID.parse(cid)
+    return c.toV1().toString()
+  } catch (err) {
+    throw new InvalidCidError(cid)
+  }
+}

packages/api/test/fixtures/find-content-by-cid-no-deal.json

@@ -7,7 +7,7 @@
         "data": [{
           "dataModelSelector": "Links/0/Links",
           "aggregate": {
-            "dataCid": "nodeal",
+            "dataCid": "bafybeica6klnrhlrbx6z24icefykpbwyypouglnypvnwb5esdm6yzcie3q",
             "pieceCid": "baga",
             "deals": {
               "data" : []

packages/api/test/fixtures/find-content-by-cid.json

@@ -7,7 +7,7 @@
         "data": [{
           "dataModelSelector": "Links/0/Links",
           "aggregate": {
-            "dataCid": "testcid",
+            "dataCid": "bafybeifnfkzjeohjf2dch2iqqpef3bfjylwxlcjws2msvdfyze5bvdprfm",
             "pieceCid": "baga",
             "deals": {
               "data" : [ {

packages/api/test/mocks/db/post_graphql.js

@@ -72,11 +72,11 @@ module.exports = ({ body }) => {
   if (body.query.includes('findContentByCid')) {
     const { cid } = body.variables
     let res
-    if (cid === 'unknown') {
+    if (cid === 'bafybeihgrtet4vowd4t4iqaspzclxajrwwsesur7zllkahrbhcymfh7kyi') {
       res = require('../../fixtures/find-content-by-cid-unknown.json')
-    } else if (cid === 'noaggregate') {
+    } else if (cid === 'bafybeiaiipiibr7aletbbrzmpklw4l5go6sodl22xs6qtcqo3lqogfogy4') {
       res = require('../../fixtures/find-content-by-cid-no-aggregate.json')
-    } else if (cid === 'nodeal') {
+    } else if (cid === 'bafybeica6klnrhlrbx6z24icefykpbwyypouglnypvnwb5esdm6yzcie3q') {
       res = require('../../fixtures/find-content-by-cid-no-deal.json')
     } else {
       res = require('../../fixtures/find-content-by-cid.json')

packages/api/test/mocks/pgrest/get_content.js

@@ -6,11 +6,11 @@ module.exports = ({ query }) => {
   const cid = query.cid && query.cid.split('eq.')[1]
   let res
 
-  if (cid === 'unknown') {
+  if (cid === 'bafybeihgrtet4vowd4t4iqaspzclxajrwwsesur7zllkahrbhcymfh7kyi') {
     res = null
-  } else if (cid === 'noaggregate') {
+  } else if (cid === 'bafybeiaiipiibr7aletbbrzmpklw4l5go6sodl22xs6qtcqo3lqogfogy4') {
     res = require('../../fixtures/postgres/find-content-by-cid-no-aggregate.json')
-  } else if (cid === 'nodeal') {
+  } else if (cid === 'bafybeica6klnrhlrbx6z24icefykpbwyypouglnypvnwb5esdm6yzcie3q') {
     res = require('../../fixtures/postgres/find-content-by-cid-no-deal.json')
   } else {
     res = require('../../fixtures/postgres/find-content-by-cid.json')

packages/api/test/mocks/pgrest/post_rpc#find_deals_by_content_cids.js

@@ -4,23 +4,23 @@
  */
 module.exports = ({ body }) => {
   // main cid
-  if (body.cids.includes('testcid')) {
+  if (body.cids.includes('bafybeifnfkzjeohjf2dch2iqqpef3bfjylwxlcjws2msvdfyze5bvdprfm')) {
     return [{
       dealId: 12345,
       storageProvider: 'f99',
       status: 'Active',
       pieceCid: 'baga',
-      dataCid: 'testcid',
+      dataCid: 'bafybeifnfkzjeohjf2dch2iqqpef3bfjylwxlcjws2msvdfyze5bvdprfm',
       dataModelSelector: 'Links/0/Links',
       activation: '<iso timestamp>',
       created: '2021-07-14T19:27:14.934572Z',
       updated: '2021-07-14T19:27:14.934572Z'
     }]
-  } else if (body.cids.includes('nodeal')) {
+  } else if (body.cids.includes('bafybeica6klnrhlrbx6z24icefykpbwyypouglnypvnwb5esdm6yzcie3q')) {
     return [{
       status: 'Queued',
       pieceCid: 'baga',
-      dataCid: 'nodeal',
+      dataCid: 'bafybeica6klnrhlrbx6z24icefykpbwyypouglnypvnwb5esdm6yzcie3q',
       dataModelSelector: 'Links/0/Links'
     }]
   }

packages/api/test/mocks/pgrest/post_rpc#user_keys_list.js

@@ -0,0 +1,15 @@
+/**
+ * https://github.com/sinedied/smoke#javascript-mocks
+ */
+module.exports = () => {
+  return {
+    statusCode: 200,
+    body: [{
+      id: 1,
+      name: 'test-key',
+      secret: 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJ0ZXN0IiwiaXNzIjoid2ViMy1zdG9yYWdlIiwiaWF0IjoxNjMzOTU3Mzg5ODcyLCJuYW1lIjoidGVzdCJ9.KEH0jHUfJls44YWsj8uex_zj0dUIvdyqGalv2rhWnx8',
+      created: '2021-07-14T19:27:14.934572Z',
+      uploads: 0
+    }]
+  }
+}

packages/api/test/status.spec.js

@@ -4,12 +4,12 @@ import { endpoint } from './scripts/constants.js'
 
 describe('GET /status/:cid', () => {
   it('get pin and deal status', async () => {
-    const cid = 'testcid'
+    const cid = 'bafybeifnfkzjeohjf2dch2iqqpef3bfjylwxlcjws2msvdfyze5bvdprfm'
     const res = await fetch(new URL(`status/${cid}`, endpoint))
     assert(res.ok, `${JSON.stringify(res)}`)
     const json = await res.json()
     assert.deepStrictEqual(json, {
-      cid: 'testcid',
+      cid: 'bafybeifnfkzjeohjf2dch2iqqpef3bfjylwxlcjws2msvdfyze5bvdprfm',
       created: '2021-07-14T19:27:14.934572Z',
       dagSize: 101,
       pins: [{
@@ -24,7 +24,7 @@ describe('GET /status/:cid', () => {
         storageProvider: 'f99',
         status: 'Active',
         pieceCid: 'baga',
-        dataCid: 'testcid',
+        dataCid: 'bafybeifnfkzjeohjf2dch2iqqpef3bfjylwxlcjws2msvdfyze5bvdprfm',
         dataModelSelector: 'Links/0/Links',
         activation: '<iso timestamp>',
         created: '2021-07-14T19:27:14.934572Z',
@@ -34,7 +34,7 @@ describe('GET /status/:cid', () => {
   })
 
   it('get shows initial queued deal', async () => {
-    const cid = 'nodeal'
+    const cid = 'bafybeica6klnrhlrbx6z24icefykpbwyypouglnypvnwb5esdm6yzcie3q'
     const res = await fetch(new URL(`status/${cid}`, endpoint))
     assert(res.ok)
     const json = await res.json()
@@ -52,14 +52,14 @@ describe('GET /status/:cid', () => {
       deals: [{
         status: 'Queued',
         pieceCid: 'baga',
-        dataCid: 'nodeal',
+        dataCid: 'bafybeica6klnrhlrbx6z24icefykpbwyypouglnypvnwb5esdm6yzcie3q',
         dataModelSelector: 'Links/0/Links'
       }]
     })
   })
 
   it('get shows no deals before aggregate is ready', async () => {
-    const cid = 'noaggregate'
+    const cid = 'bafybeiaiipiibr7aletbbrzmpklw4l5go6sodl22xs6qtcqo3lqogfogy4'
     const res = await fetch(new URL(`status/${cid}`, endpoint))
     assert(res.ok)
     const json = await res.json()
@@ -79,7 +79,7 @@ describe('GET /status/:cid', () => {
   })
 
   it('get 404 for unknown cid', async () => {
-    const cid = 'unknown'
+    const cid = 'bafybeihgrtet4vowd4t4iqaspzclxajrwwsesur7zllkahrbhcymfh7kyi'
     const res = await fetch(new URL(`status/${cid}`, endpoint))
     assert(!res.ok)
     assert.strictEqual(res.status, 404)

packages/db/db-client-types.ts

@@ -48,9 +48,7 @@ export type AuthKey = {
 }
 
 export type AuthKeyItem = definitions['auth_key'] & {
-  uploads: Array<
-    Pick<definitions['upload'], 'id'>
-  >
+  uploads: number
 }
 
 export type AuthKeyItemOutput = {

packages/db/fauna/client.js

@@ -165,11 +165,11 @@ export class FaunaClient {
   /**
    * Rename an upload.
    *
-   * @param {string} cid
    * @param {number} user
+   * @param {string} cid
    * @param {string} name
    */
-  async renameUpload (cid, user, name) {
+  async renameUpload (user, cid, name) {
     const res = await this.query(gql`
     mutation RenameUserUpload($user: ID!, $cid: String!, $name: String!) {
       renameUserUpload(user: $user, cid: $cid, name: $name) {
@@ -184,10 +184,10 @@ export class FaunaClient {
   /**
    * Delete a user upload.
    *
-   * @param {string} cid
    * @param {number} userId
+   * @param {string} cid
    */
-  async deleteUpload (cid, userId) {
+  async deleteUpload (userId, cid) {
     const res = await this.query(gql`
       mutation DeleteUserUpload($user: ID!, $cid: String!) {
         deleteUserUpload(user: $user, cid: $cid) {

packages/db/index.d.ts

@@ -29,8 +29,8 @@ export class DBClient {
   createUpload (data: CreateUploadInput): Promise<CreateUploadOutput>
   getUpload (cid: string, userId: number): Promise<UploadItemOutput>
   listUploads (userId: number, opts?: ListUploadsOptions): Promise<UploadItemOutput[]>
-  renameUpload (cid: string, userId: number, name: string): Promise<{ name: string }>
-  deleteUpload(cid: string, userId: number): Promise<{ _id: number }>
+  renameUpload (userId: number, cid: string, name: string): Promise<{ name: string }>
+  deleteUpload (userId: number, cid: string): Promise<{ _id: number }>
   getStatus (cid: string): Promise<ContentItemOutput>
   getBackups(uploadId: number): Promise<Array<BackupOutput>>
   upsertPin (cid: string, pin: PinItemOutput): Promise<number>

packages/db/index.js

@@ -83,22 +83,22 @@ export class DBClient {
   /**
    * Rename an upload.
    *
-   * @param {string} cid
    * @param {number} userId
+   * @param {string} cid
    * @param {string} name
    */
-  renameUpload (cid, userId, name) {
-    return this._client.renameUpload(cid, userId, name)
+  renameUpload (userId, cid, name) {
+    return this._client.renameUpload(userId, cid, name)
   }
 
   /**
    * Delete a user upload.
    *
-   * @param {string} cid
    * @param {number} userId
+   * @param {string} cid
    */
-  deleteUpload (cid, userId) {
-    return this._client.deleteUpload(cid, userId)
+  deleteUpload (userId, cid) {
+    return this._client.deleteUpload(userId, cid)
   }
 
   /**