Browse Source

storage-node-v2: Review fixes.

Shamil Gadelshin 3 years ago
parent
commit
f7c2143701

+ 12 - 6
storage-node-v2/src/api-spec/openapi.yaml

@@ -52,14 +52,20 @@ paths:
               schema:
                 type: string
                 format: binary
-        404:
-          description: File not found
+        400:
+          description: Bad request
           content:
             application/json:
               schema:
                 $ref: '#/components/schemas/ErrorResponse'
-        410:
-          description: File request problem
+        401:
+          description: Unauthorized
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ErrorResponse'
+        404:
+          description: File not found
           content:
             application/json:
               schema:
@@ -81,10 +87,10 @@ paths:
       responses:
         200:
           description: Ok
+        400:
+          description: Bad request
         404:
           description: File not found
-        410:
-          description: Header request problem
         500:
           description: Unknown server error
   /files:

+ 1 - 3
storage-node-v2/src/commands/dev/upload.ts

@@ -1,5 +1,4 @@
 import { flags } from '@oclif/command'
-import BN from 'bn.js'
 import { uploadDataObjects } from '../../services/runtime/extrinsics'
 import ApiCommandBase from '../../command-base/ApiCommandBase'
 import logger from '../../services/logger'
@@ -41,8 +40,7 @@ export default class DevUpload extends ApiCommandBase {
 
     const api = await this.getApi()
 
-    // Must be a number.
-    const dataFee = (await api.query.storage.dataObjectPerMegabyteFee()) as BN
+    const dataFee = await api.query.storage.dataObjectPerMegabyteFee()
 
     logger.info(`Current data fee: ${dataFee}`)
 

+ 3 - 5
storage-node-v2/src/commands/leader/update-dynamic-bag-policy.ts

@@ -3,6 +3,7 @@ import { updateNumberOfStorageBucketsInDynamicBagCreationPolicy } from '../../se
 import { flags } from '@oclif/command'
 import logger from '../../services/logger'
 import { parseDynamicBagType } from '../../services/helpers/bagTypes'
+import { DynamicBagTypeKey } from '@joystream/types/storage'
 
 /**
  * CLI command:
@@ -22,7 +23,7 @@ export default class LeaderUpdateDynamicBagPolicy extends ApiCommandBase {
       required: true,
       description: 'New storage buckets number',
     }),
-    bagType: flags.enum({
+    bagType: flags.enum<DynamicBagTypeKey>({
       char: 't',
       description: 'Dynamic bag type (Channel, Member).',
       options: ['Channel', 'Member'],
@@ -42,11 +43,8 @@ export default class LeaderUpdateDynamicBagPolicy extends ApiCommandBase {
     const account = this.getAccount(flags)
     const newNumber = flags.number
 
-    // Verified by enum argument parser.
-    const dynamicBagTypeString = flags.bagType as 'Member' | 'Channel'
-
     const api = await this.getApi()
-    const dynamicBagType = parseDynamicBagType(api, dynamicBagTypeString)
+    const dynamicBagType = parseDynamicBagType(api, flags.bagType)
     const success = await updateNumberOfStorageBucketsInDynamicBagCreationPolicy(
       api,
       account,

+ 8 - 12
storage-node-v2/src/services/runtime/api.ts

@@ -148,14 +148,7 @@ export async function sendAndFollowNamedTx<T>(
   sudoCall = false,
   eventParser: ((result: ISubmittableResult) => T) | null = null
 ): Promise<T | void> {
-  const description = tx.toHuman() as {
-    method: {
-      method: string
-      section: string
-    }
-  }
-
-  logger.debug(`Sending ${description?.method?.section}.${description?.method?.method} extrinsic...`)
+  logger.debug(`Sending ${tx.method.section}.${tx.method.method} extrinsic...`)
 
   if (sudoCall) {
     tx = api.tx.sudo.sudo(tx)
@@ -180,14 +173,17 @@ export async function sendAndFollowNamedTx<T>(
  * @param api - API promise
  * @param account - KeyPair instance
  * @param tx - prepared extrinsic with arguments
+ * @param eventParser - defines event parsing function (null by default) for
+ * getting any information from the successful extrinsic events.
  * @returns void promise.
  */
-export async function sendAndFollowSudoNamedTx(
+export async function sendAndFollowSudoNamedTx<T>(
   api: ApiPromise,
   account: KeyringPair,
-  tx: SubmittableExtrinsic<'promise'>
-): Promise<void> {
-  return sendAndFollowNamedTx(api, account, tx, true)
+  tx: SubmittableExtrinsic<'promise'>,
+  eventParser: ((result: ISubmittableResult) => T) | null = null
+): Promise<T | void> {
+  return sendAndFollowNamedTx(api, account, tx, true, eventParser)
 }
 
 /**

+ 1 - 2
storage-node-v2/src/services/runtime/extrinsics.ts

@@ -4,7 +4,6 @@ import { KeyringPair } from '@polkadot/keyring/types'
 import { ApiPromise } from '@polkadot/api'
 import { BagId, DynamicBagType } from '@joystream/types/storage'
 import logger from '../../services/logger'
-import BN from 'bn.js'
 
 /**
  * Creates storage bucket.
@@ -36,7 +35,7 @@ export async function createStorageBucket(
     const tx = api.tx.storage.createStorageBucket(invitedWorkerValue, allowedNewBags, sizeLimit, objectsLimit)
     bucketId = await sendAndFollowNamedTx(api, account, tx, false, (result) => {
       const event = getEvent(result, 'storage', 'StorageBucketCreated')
-      const bucketId = event?.data[0] as BN
+      const bucketId = event?.data[0]
 
       return bucketId.toNumber()
     })

+ 21 - 12
storage-node-v2/src/services/runtime/hireLead.ts

@@ -1,7 +1,5 @@
-import { sendAndFollowSudoNamedTx, sendAndFollowNamedTx } from './api'
+import { sendAndFollowSudoNamedTx, sendAndFollowNamedTx, getEvent } from './api'
 import { getAlicePair } from './accounts'
-import { Option, Vec } from '@polkadot/types'
-import { WorkerId, OpeningId, ApplicationId } from '@joystream/types/working-group'
 import { MemberId } from '@joystream/types/members'
 import { ApiPromise } from '@polkadot/api'
 import logger from '../../services/logger'
@@ -21,19 +19,19 @@ export async function hireStorageWorkingGroupLead(api: ApiPromise): Promise<void
   const LeadKeyPair = getAlicePair()
 
   // Create membership if not already created
-  const members = (await api.query.members.memberIdsByControllerAccountId(LeadKeyPair.address)) as Vec<MemberId>
+  const members = await api.query.members.memberIdsByControllerAccountId(LeadKeyPair.address)
 
-  let memberId: MemberId | undefined = members.toArray()[0] as MemberId
+  let memberId: MemberId | undefined = members.toArray()[0]
 
   if (memberId === undefined) {
     logger.info('Preparing member account creation extrinsic...')
-    memberId = (await api.query.members.nextMemberId()) as MemberId
+    memberId = await api.query.members.nextMemberId()
     const tx = api.tx.members.buyMembership(0, 'alice', null, null)
     await sendAndFollowNamedTx(api, LeadKeyPair, tx)
   }
 
   // Create a new lead opening.
-  const currentLead = (await api.query.storageWorkingGroup.currentLead()) as Option<WorkerId>
+  const currentLead = await api.query.storageWorkingGroup.currentLead()
   if (currentLead.isSome) {
     logger.info('Storage lead already exists, skipping...')
     return
@@ -41,9 +39,6 @@ export async function hireStorageWorkingGroupLead(api: ApiPromise): Promise<void
 
   logger.info(`Making member id: ${memberId} the content lead.`)
 
-  const newOpeningId = (await api.query.storageWorkingGroup.nextOpeningId()) as OpeningId
-  const newApplicationId = (await api.query.storageWorkingGroup.nextApplicationId()) as ApplicationId
-
   // Create curator lead opening
   logger.info('Preparing Create Storage Lead Opening extrinsic...')
   let tx = api.tx.storageWorkingGroup.addOpening(
@@ -52,7 +47,16 @@ export async function hireStorageWorkingGroupLead(api: ApiPromise): Promise<void
     'storage opening', // human_readable_text
     'Leader' // opening_type
   )
-  await sendAndFollowSudoNamedTx(api, SudoKeyPair, tx)
+  const newOpeningId = await sendAndFollowSudoNamedTx(api, SudoKeyPair, tx, (result) => {
+    const event = getEvent(result, 'storageWorkingGroup', 'OpeningAdded')
+    const bucketId = event?.data[0]
+
+    return bucketId.toNumber()
+  })
+
+  if (typeof newOpeningId !== 'number') {
+    throw Error('Cannot create opening.')
+  }
 
   // Apply to lead opening
   logger.info('Preparing Apply to Storage Lead Opening extrinsic...')
@@ -64,7 +68,12 @@ export async function hireStorageWorkingGroupLead(api: ApiPromise): Promise<void
     null, // opt appl. stake
     'bootstrap opening' // human_readable_text
   )
-  await sendAndFollowNamedTx(api, LeadKeyPair, tx)
+  const newApplicationId = await sendAndFollowNamedTx(api, LeadKeyPair, tx, false, (result) => {
+    const event = getEvent(result, 'storageWorkingGroup', 'AppliedOnOpening')
+    const bucketId = event?.data[1]
+
+    return bucketId.toNumber()
+  })
 
   // Begin review period
   logger.info('Preparing Begin Applicant Review extrinsic...')

+ 50 - 35
storage-node-v2/src/services/webApi/controllers/publicApi.ts

@@ -1,4 +1,5 @@
 import { acceptPendingDataObjects } from '../../runtime/extrinsics'
+import { ExtrinsicFailedError } from '../../runtime/api'
 import {
   RequestData,
   UploadTokenRequest,
@@ -17,8 +18,14 @@ import * as express from 'express'
 import fs from 'fs'
 import path from 'path'
 import send from 'send'
+import { CLIError } from '@oclif/errors'
 const fsPromises = fs.promises
 
+/**
+ * Dedicated error for the web api requests.
+ */
+export class WebApiError extends CLIError {}
+
 /**
  * A public endpoint: serves files by CID.
  */
@@ -67,11 +74,7 @@ export async function getFileHeaders(req: express.Request, res: express.Response
 
     res.status(200).send()
   } catch (err) {
-    if (isNofileError(err)) {
-      res.status(404).send()
-    } else {
-      res.status(410).send()
-    }
+    res.status(getHttpStatusCodeByError(err)).send()
   }
 }
 
@@ -110,10 +113,7 @@ export async function uploadFile(req: express.Request, res: express.Response): P
   } catch (err) {
     await cleanupFileOnError(cleanupFileName, err.toString())
 
-    res.status(410).json({
-      type: 'upload',
-      message: err.toString(),
-    })
+    sendResponseWithError(res, err, 'upload')
   }
 }
 
@@ -139,10 +139,7 @@ export async function authTokenForUploading(req: express.Request, res: express.R
       token: signedToken,
     })
   } catch (err) {
-    res.status(410).json({
-      type: 'authtoken',
-      message: err.toString(),
-    })
+    sendResponseWithError(res, err, 'authtoken')
   }
 }
 
@@ -163,7 +160,7 @@ function getFileObject(req: express.Request): Express.Multer.File {
     return files[0]
   }
 
-  throw new Error('No file uploaded')
+  throw new WebApiError('No file uploaded')
 }
 
 /**
@@ -178,7 +175,7 @@ function getWorkerId(res: express.Response): number {
     return res.locals.workerId
   }
 
-  throw new Error('No Joystream worker ID loaded.')
+  throw new WebApiError('No Joystream worker ID loaded.')
 }
 
 /**
@@ -193,7 +190,7 @@ function getUploadsDir(res: express.Response): string {
     return res.locals.uploadsDir
   }
 
-  throw new Error('No upload directory path loaded.')
+  throw new WebApiError('No upload directory path loaded.')
 }
 
 /**
@@ -208,7 +205,7 @@ function getAccount(res: express.Response): KeyringPair {
     return res.locals.storageProviderAccount
   }
 
-  throw new Error('No Joystream account loaded.')
+  throw new WebApiError('No Joystream account loaded.')
 }
 
 /**
@@ -223,7 +220,7 @@ function getApi(res: express.Response): ApiPromise {
     return res.locals.api
   }
 
-  throw new Error('No Joystream API loaded.')
+  throw new WebApiError('No Joystream API loaded.')
 }
 
 /**
@@ -239,7 +236,7 @@ function getCid(req: express.Request): string {
     return cid
   }
 
-  throw new Error('No CID provided.')
+  throw new WebApiError('No CID provided.')
 }
 
 /**
@@ -255,7 +252,7 @@ function getTokenRequest(req: express.Request): UploadTokenRequest {
     return tokenRequest
   }
 
-  throw new Error('No token request provided.')
+  throw new WebApiError('No token request provided.')
 }
 
 /**
@@ -270,12 +267,12 @@ async function validateTokenRequest(api: ApiPromise, tokenRequest: UploadTokenRe
   const result = verifyTokenSignature(tokenRequest, tokenRequest.data.accountId)
 
   if (!result) {
-    throw new Error('Invalid upload token request signature.')
+    throw new WebApiError('Invalid upload token request signature.')
   }
 
   const membership = await api.query.members.membershipById(tokenRequest.data.memberId)
   if (membership.controller_account.toString() !== tokenRequest.data.accountId) {
-    throw new Error(`Provided controller account and member id don't match.`)
+    throw new WebApiError(`Provided controller account and member id don't match.`)
   }
 }
 
@@ -289,7 +286,7 @@ function verifyFileSize(fileSize: number) {
   const MAX_FILE_SIZE = 1000000 // TODO: Get this const from the runtime
 
   if (fileSize > MAX_FILE_SIZE) {
-    throw new Error('Max file size exceeded.')
+    throw new WebApiError('Max file size exceeded.')
   }
 }
 
@@ -325,7 +322,7 @@ async function verifyFileMimeType(filePath: string): Promise<void> {
   const correctMimeType = allowedMimeTypes.some((allowedType) => fileInfo.mimeType.startsWith(allowedType))
 
   if (!correctMimeType) {
-    throw new Error(`Incorrect mime type detected: ${fileInfo.mimeType}`)
+    throw new WebApiError(`Incorrect mime type detected: ${fileInfo.mimeType}`)
   }
 }
 
@@ -338,19 +335,11 @@ async function verifyFileMimeType(filePath: string): Promise<void> {
  * @returns void promise.
  */
 function sendResponseWithError(res: express.Response, err: Error, errorType: string): void {
-  const errorString = err.toString()
-  // Special case - file not found.
-  if (isNofileError(err)) {
-    res.status(404).json({
-      type: errorType,
-      message: `File not found.`,
-    })
-    return
-  }
+  const message = isNofileError(err) ? `File not found.` : err.toString()
 
-  res.status(410).json({
+  res.status(getHttpStatusCodeByError(err)).json({
     type: errorType,
-    message: errorString,
+    message,
   })
 }
 
@@ -363,3 +352,29 @@ function sendResponseWithError(res: express.Response, err: Error, errorType: str
 function isNofileError(err: Error): boolean {
   return err.toString().includes('ENOENT')
 }
+
+/**
+ * Get the status code by error.
+ *
+ * @param err - error
+ * @returns HTTP status code
+ */
+function getHttpStatusCodeByError(err: Error): number {
+  if (isNofileError(err)) {
+    return 404
+  }
+
+  if (err instanceof ExtrinsicFailedError) {
+    return 400
+  }
+
+  if (err instanceof WebApiError) {
+    return 400
+  }
+
+  if (err instanceof CLIError) {
+    return 400
+  }
+
+  return 500
+}