Merge pull request #1043 from pokt-foundation/phd-client-remove-model…

…-fields

T-18925 fix: removed unneeded checks for required fields from PHD Client

src/controllers/blockchains.controller.ts

@@ -17,10 +17,7 @@ export class BlockchainsController {
     },
   })
   async count(): Promise<Count> {
-    return this.phdClient.count({
-      path: PHDPaths.Blockchain,
-      model: Blockchains,
-    })
+    return this.phdClient.count({ path: PHDPaths.Blockchain })
   }
 
   @get('/blockchains', {
@@ -39,12 +36,7 @@ export class BlockchainsController {
     },
   })
   async find(@param.filter(Blockchains) filter?: Filter<Blockchains>): Promise<BlockchainsResponse[]> {
-    return (
-      await this.phdClient.find<Blockchains>({
-        path: PHDPaths.Blockchain,
-        model: Blockchains,
-      })
-    ).map((bl) => blockchainToBlockchainResponse(bl))
+    return (await this.phdClient.find<Blockchains>({ path: PHDPaths.Blockchain })).map(blockchainToBlockchainResponse)
   }
 
   @get('/blockchains/{id}', {
@@ -60,13 +52,7 @@ export class BlockchainsController {
     },
   })
   async findById(@param.path.string('id') id: string): Promise<BlockchainsResponse> {
-    return blockchainToBlockchainResponse(
-      await this.phdClient.findById({
-        path: PHDPaths.Blockchain,
-        id,
-        model: Blockchains,
-      })
-    )
+    return blockchainToBlockchainResponse(await this.phdClient.findById({ path: PHDPaths.Blockchain, id }))
   }
   @get('/blockchains/ids', {
     responses: {
@@ -85,7 +71,6 @@ export class BlockchainsController {
   async idsMapping(@param.filter(Blockchains) filter?: Filter<Blockchains>): Promise<object> {
     const blockchains = await this.phdClient.find<Blockchains>({
       path: PHDPaths.Blockchain,
-      model: Blockchains,
     })
 
     const aliases = {}

src/controllers/v1.controller.ts

@@ -669,12 +669,7 @@ export class V1Controller {
 
     if (!cachedLoadBalancer) {
       try {
-        return await this.phdClient.findById({
-          path: PHDPaths.LoadBalancer,
-          id,
-          model: LoadBalancers,
-          cache: this.cache,
-        })
+        return await this.phdClient.findById({ path: PHDPaths.LoadBalancer, id, cache: this.cache })
       } catch (e) {
         return undefined
       }
@@ -688,12 +683,7 @@ export class V1Controller {
 
     if (!cachedApplication) {
       try {
-        return await this.phdClient.findById({
-          path: PHDPaths.Application,
-          id,
-          model: Applications,
-          cache: this.cache,
-        })
+        return await this.phdClient.findById({ path: PHDPaths.Application, id, cache: this.cache })
       } catch (e) {
         return undefined
       }

src/services/phd-client.ts

@@ -10,28 +10,20 @@ const logger = require('./logger')
 
 const FAILURE_ERROR = 'Data fetching from Pocket HTTP DB failed'
 
-type ModelRef = new (...args: any[]) => any //eslint-disable-line
-interface ModelProps extends ModelRef {
-  definition?: { properties: { [key: string]: { required?: boolean } } }
-}
-
 interface FindParams {
   path: string
-  model: ModelRef
   cacheKey?: string
   cache?: Cache
 }
 
 interface FindOneParams {
   path: string
   id: string
-  model: ModelRef
   cache?: Cache
 }
 
 interface CountParams {
   path: string
-  model: ModelRef
 }
 
 interface PostgresGatewayAAT {
@@ -63,41 +55,29 @@ class PHDClient {
     this.apiKey = apiKey
   }
 
-  async find<T extends Entity>({ path, model, cache, cacheKey }: FindParams): Promise<T[]> {
+  async find<T extends Entity>({ path, cache, cacheKey }: FindParams): Promise<T[]> {
     if (cache && !cacheKey) {
       throw new Error(`cacheKey not set for path ${path}`)
     }
 
     const url = `${this.baseUrl}/v1/${path}`
-    const modelFields = this.getRequiredModelFields(model)
-    const modelsData: T[] = []
 
     try {
-      const { data: documents } = await axios.get(url, { headers: { authorization: this.apiKey } })
+      const { data: documents } = await axios.get<T[]>(url, { headers: { authorization: this.apiKey } })
+
+      if (cache && cacheKey) {
+        await cache.set(cacheKey, JSON.stringify(documents), 'EX', 60)
+      }
 
-      documents.forEach((document) => {
-        if (this.hasAllRequiredModelFields(document, modelFields)) {
-          modelsData.push(new model(document))
-        } else {
-          throw new Error('data not instance of model')
-        }
-      })
+      return documents
     } catch (error) {
       logger.log('error', FAILURE_ERROR, { error })
       throw newHttpError(error)
     }
-
-    if (cache && cacheKey) {
-      await cache.set(cacheKey, JSON.stringify(modelsData), 'EX', 60)
-    }
-
-    return modelsData
   }
 
-  async findById<T extends Entity>({ path, id, model, cache }: FindOneParams): Promise<T> {
+  async findById<T extends Entity>({ path, id, cache }: FindOneParams): Promise<T> {
     const url = `${this.baseUrl}/v1/${path}/${id}`
-    const modelFields = this.getRequiredModelFields(model)
-    let modelData: T
 
     try {
       const { data: document } = await axios.get(url, { headers: { authorization: this.apiKey } })
@@ -109,21 +89,15 @@ class PHDClient {
 
       const processedDocument = processMethod?.() || document
 
-      if (this.hasAllRequiredModelFields<T>(processedDocument, modelFields)) {
-        modelData = new model(processedDocument)
-      } else {
-        throw new Error('data not instance of model')
+      if (cache) {
+        await cache.set(id, JSON.stringify(processedDocument), 'EX', 60)
       }
+
+      return processedDocument
     } catch (error) {
       logger.log('error', FAILURE_ERROR, { error })
       throw newHttpError(error)
     }
-
-    if (cache) {
-      await cache.set(id, JSON.stringify(modelData), 'EX', 60)
-    }
-
-    return modelData
   }
 
   async count({ path }: CountParams): Promise<Count> {
@@ -166,19 +140,6 @@ class PHDClient {
 
     return document
   }
-
-  /** Gets a string array of all the fields marked as required by the Loopback model */
-  private getRequiredModelFields(model: ModelProps): string[] {
-    return Object.entries(model.definition.properties)
-      .filter(([_, { required }]) => required)
-      .map(([key]) => key)
-  }
-
-  /** Checks that the data returned from the PHD has all required fields used by the
-      Portal API code, meaning all required fields declared by the Loopbak model. */
-  private hasAllRequiredModelFields<T>(data: T, modelFields: string[]): boolean {
-    return modelFields.every((key) => Object.keys(data).includes(key))
-  }
 }
 
 function newHttpError(error): HttpErrors.HttpError {

src/utils/relayer.ts

@@ -44,12 +44,7 @@ export async function loadBlockchain(
 
   if (!cachedBlockchains) {
     try {
-      blockchains = await phdClient.find({
-        path: PHDPaths.Blockchain,
-        model: Blockchains,
-        cache,
-        cacheKey: 'blockchains',
-      })
+      blockchains = await phdClient.find({ path: PHDPaths.Blockchain, cache, cacheKey: 'blockchains' })
     } catch (e) {
       throw new ErrorObject(rpcID, e)
     }
@@ -154,12 +149,7 @@ export async function getBlockchainAliasesByDomain(
   let blockchains: Blockchains[]
 
   if (!cachedBlockchains) {
-    blockchains = await phdClient.find({
-      path: PHDPaths.Blockchain,
-      model: Blockchains,
-      cache: redis,
-      cacheKey: PHDCacheKeys.Blockchain,
-    })
+    blockchains = await phdClient.find({ path: PHDPaths.Blockchain, cache: redis, cacheKey: PHDCacheKeys.Blockchain })
   } else {
     blockchains = JSON.parse(cachedBlockchains)
   }

tests/acceptance/phd-client.acceptance.ts

@@ -6,24 +6,6 @@ import { PHDClient, PHDPaths } from '../../src/services/phd-client'
 
 const logger = require('../../src/services/logger')
 
-const blockchainRequiredFields = [
-  'id',
-  'ticker',
-  'chainID',
-  'chainIDCheck',
-  'enforceResult',
-  'network',
-  'blockchain',
-  'blockchainAliases',
-  'active',
-  'syncCheckOptions',
-  'logLimitBlocks',
-  'path',
-  'altruist',
-]
-const loadBalancerRequiredFields = ['user', 'requestTimeout', 'applicationIDs']
-const applicationRequiredFields = ['id', 'freeTierApplicationAccount', 'gatewayAAT', 'gatewaySettings']
-
 const integrationDescribe = process.env.INTEGRATION_TEST === 'true' ? describe : describe.skip
 integrationDescribe('Pocket HTTP DB Client', () => {
   let phdClient: PHDClient
@@ -47,16 +29,10 @@ integrationDescribe('Pocket HTTP DB Client', () => {
   describe('find', () => {
     describe('blockchains', () => {
       it('fetches blockchains from PHD', async () => {
-        const blockchains = await phdClient.find<Blockchains>({
-          path: PHDPaths.Blockchain,
-          model: Blockchains,
-        })
+        const blockchains = await phdClient.find<Blockchains>({ path: PHDPaths.Blockchain })
 
-        expect(logSpy.calledOnceWith('warn')).to.be.false()
+        expect(logSpy.calledOnceWith('error')).to.be.false()
         expect(blockchains.length).to.be.above(1)
-        blockchains.forEach((chain) => {
-          expect(chain).to.have.properties(blockchainRequiredFields)
-        })
       })
     })
   })
@@ -66,63 +42,45 @@ integrationDescribe('Pocket HTTP DB Client', () => {
       it('fetches a blockchain from PHD', async () => {
         const testId = '0001'
 
-        const blockchain = await phdClient.findById<Blockchains>({
-          path: PHDPaths.Blockchain,
-          id: testId,
-          model: Blockchains,
-        })
+        const blockchain = await phdClient.findById<Blockchains>({ path: PHDPaths.Blockchain, id: testId })
 
-        expect(logSpy.calledOnceWith('warn')).to.be.false()
+        expect(logSpy.calledOnceWith('error')).to.be.false()
         expect(blockchain).not.to.be.undefined()
         expect(blockchain.ticker).to.equal('POKT')
-        expect(blockchain).to.have.properties(blockchainRequiredFields)
       })
     })
 
     describe('load_balancer', () => {
       it('fetches a load balancer from PHD', async () => {
         const testId = '280023ecacf59129e9497bc2'
 
-        const loadBalancer = await phdClient.findById<LoadBalancers>({
-          path: PHDPaths.LoadBalancer,
-          id: testId,
-          model: LoadBalancers,
-        })
+        const loadBalancer = await phdClient.findById<LoadBalancers>({ path: PHDPaths.LoadBalancer, id: testId })
 
-        expect(logSpy.calledOnceWith('warn')).to.be.false()
+        expect(logSpy.calledOnceWith('error')).to.be.false()
         expect(loadBalancer).not.to.be.undefined()
         expect(loadBalancer.name).to.equal('Pascals_test_app_DO-NOT-DELETE')
-        expect(loadBalancer).to.have.properties(loadBalancerRequiredFields)
       })
     })
 
     describe('application', () => {
       it('fetches an application from PHD', async () => {
         const testId = '6307c50471e59c00380027c9'
 
-        const application = await phdClient.findById<Applications>({
-          path: PHDPaths.Application,
-          id: testId,
-          model: Applications,
-        })
+        const application = await phdClient.findById<Applications>({ path: PHDPaths.Application, id: testId })
 
-        expect(logSpy.calledOnceWith('warn')).to.be.false()
+        expect(logSpy.calledOnceWith('error')).to.be.false()
         expect(application).not.to.be.undefined()
         expect(application.name).to.equal('PascalsTestApp')
-        expect(application).to.have.properties(applicationRequiredFields)
       })
     })
   })
 
   describe('count', () => {
     describe('blockchains', () => {
       it('fetches the count of blockchains from PHD', async () => {
-        const { count } = await phdClient.count({
-          path: PHDPaths.Blockchain,
-          model: Blockchains,
-        })
+        const { count } = await phdClient.count({ path: PHDPaths.Blockchain })
 
-        expect(logSpy.calledOnceWith('warn')).to.be.false()
+        expect(logSpy.calledOnceWith('error')).to.be.false()
         expect(count).not.to.be.undefined()
         expect(count).to.be.above(1)
       })