fix: Improve NFT fetch performance on rentals

src/adapters/sources/rentals.ts

@@ -12,6 +12,7 @@ import {
   Sortable,
 } from '../../ports/merger/types'
 import { INFTsComponent, NFTResult } from '../../ports/nfts/types'
+import { getId } from '../../ports/nfts/utils'
 import { IRentalsComponent } from '../../ports/rentals/types'
 
 export function createRentalsNFTSource(
@@ -21,25 +22,29 @@ export function createRentalsNFTSource(
   async function enhanceRentalListing(
     rentals: RentalListing[]
   ): Promise<Sortable<NFTResult, NFTSortBy>[]> {
-    const nftResults: NFTResult[] = await Promise.all(
-      rentals.map(async (rental) => {
-        const nftResult = await nfts.fetchOne(
-          rental.contractAddress,
-          rental.tokenId
-        )
-        if (!nftResult) {
+    const tokenIdsOfRentals = rentals.map((rental) => rental.nftId)
+    const nftResultsOfRentals = await nfts.fetchByTokenIds(tokenIdsOfRentals)
+    const nftResultsOfRentalsById = Object.fromEntries(
+      nftResultsOfRentals.map((nft) => [nft.nft.id, nft])
+    )
+    return rentals
+      .map((rental) => {
+        let nftResultForRental =
+          nftResultsOfRentalsById[getId(rental.contractAddress, rental.tokenId)]
+        if (!nftResultForRental) {
           throw new Error('NFT for the rental listing was not found')
         }
 
         return {
-          ...nftResult,
-          nft: { ...nftResult.nft, openRentalId: rental.id },
+          ...nftResultForRental,
+          nft: {
+            ...nftResultForRental.nft,
+            openRentalId: rental.id,
+          },
           rental,
         }
       })
-    )
-
-    return nftResults.map(convertNFTResultToSortableResult)
+      .map(convertNFTResultToSortableResult)
   }
 
   async function fetch(

src/logic/nfts/rentals.ts

@@ -31,8 +31,7 @@ export function shouldFetch(filters: NFTFilters): boolean {
     filters.category &&
     filters.category !== NFTCategory.ESTATE &&
     filters.category !== NFTCategory.PARCEL
-  const landFilterIsSetAndIsNotLAND =
-    filters.isLand !== undefined && !filters.isLand
+  const categoriesIsNotSetAndIsNotLand = !filters.category && !filters.isLand
   const prohibitedFilterIsSet = PROHIBITED_FILTERS.some(
     (prohibitedFilter) =>
       setFilters.includes(prohibitedFilter) &&
@@ -46,7 +45,7 @@ export function shouldFetch(filters: NFTFilters): boolean {
       prohibitedSortingIsSet ||
       prohibitedFilterIsSet ||
       categoriesIsSetAndIsNotLAND ||
-      landFilterIsSetAndIsNotLAND
+      categoriesIsNotSetAndIsNotLand
     ) && Boolean(filters.isOnRent)
   )
 }

src/ports/nfts/component.ts

@@ -1,7 +1,12 @@
 import { NFTFilters, NFTSortBy } from '@dcl/schemas'
 import { ISubgraphComponent } from '@well-known-components/thegraph-component'
 import { INFTsComponent, NFTResult } from './types'
-import { getFetchOneQuery, getFetchQuery, getQueryVariables } from './utils'
+import {
+  getByTokenIdQuery,
+  getFetchOneQuery,
+  getFetchQuery,
+  getQueryVariables,
+} from './utils'
 
 export function createNFTComponent<T extends { id: string }>(options: {
   subgraph: ISubgraphComponent
@@ -81,9 +86,25 @@ export function createNFTComponent<T extends { id: string }>(options: {
     }
   }
 
+  /**
+   * Fetches up to 1000 NFTs by their token IDs.
+   */
+  async function fetchByTokenIds(tokenIds: string[]): Promise<NFTResult[]> {
+    const query = getByTokenIdQuery(fragmentName, getFragment)
+    const variables = {
+      tokenIds,
+    }
+    const { nfts } = await subgraph.query<{
+      nfts: T[]
+    }>(query, variables)
+
+    return nfts.map(fromFragment)
+  }
+
   return {
     fetch,
     fetchOne,
+    fetchByTokenIds,
     count,
   }
 }

src/ports/nfts/types.ts

@@ -15,5 +15,6 @@ export type QueryVariables = Omit<NFTFilters, 'sortBy'> & {
 export interface INFTsComponent {
   fetch(filters: NFTFilters): Promise<NFTResult[]>
   fetchOne(contractAddress: string, tokenId: string): Promise<NFTResult | null>
+  fetchByTokenIds(tokenIds: string[]): Promise<NFTResult[]>
   count(filters: NFTFilters): Promise<number>
 }

src/ports/nfts/utils.ts

@@ -196,6 +196,23 @@ export function getFetchOneQuery(
     `
 }
 
+export function getByTokenIdQuery(
+  fragmentName: string,
+  getFragment: () => string
+) {
+  return `
+  query NFTByTokenId($tokenIds: [String!]) {
+    nfts(
+      where: { id_in: $tokenIds }
+      first: 1000
+      ) {
+        ...${fragmentName}
+      }
+    }
+    ${getFragment()}
+    `
+}
+
 export function getId(contractAddress: string, tokenId: string) {
   return `${contractAddress}-${tokenId}`
 }

src/tests/adapters/sources/nft-source.spec.ts

@@ -23,13 +23,15 @@ let options: {
 }
 let fetchMock: jest.Mock
 let fetchOneMock: jest.Mock
+let fetchByTokenIdsMock: jest.Mock
 let countMock: jest.Mock
 let shouldFetchMock: jest.Mock
 let getRentalsListingsMock: jest.Mock
 let getOpenRentalsListingsOfNFTsMock: jest.Mock
 
 beforeEach(() => {
   fetchMock = jest.fn()
+  fetchByTokenIdsMock = jest.fn()
   countMock = jest.fn()
   fetchOneMock = jest.fn()
   shouldFetchMock = jest.fn()
@@ -39,6 +41,7 @@ beforeEach(() => {
   nftComponentMock = {
     fetch: fetchMock,
     fetchOne: fetchOneMock,
+    fetchByTokenIds: fetchByTokenIdsMock,
     count: countMock,
   }
 

src/tests/adapters/sources/rentals-source.spec.ts

@@ -5,6 +5,7 @@ import { SignaturesServerPaginatedResponse } from '../../../ports/rentals/types'
 import { INFTsComponent, NFTResult } from '../../../ports/nfts/types'
 import { IRentalsComponent } from '../../../ports/rentals/types'
 import { convertNFTResultToSortableResult } from '../../../logic/nfts/utils'
+import { getId } from '../../../ports/nfts/utils'
 
 let rentalsNFTSource: IMergerComponent.Source<NFTResult, NFTFilters, NFTSortBy>
 let rentalsComponentMock: IRentalsComponent
@@ -13,12 +14,14 @@ let getRentalsListingsMock: jest.Mock
 let getOpenRentalsListingsOfNFTsMock: jest.Mock
 let fetchMock: jest.Mock
 let fetchOneMock: jest.Mock
+let fetchByTokenIdsMock: jest.Mock
 let countMock: jest.Mock
 let rentalsResponse: SignaturesServerPaginatedResponse<RentalListing[]>
 let nftResults: NFTResult[]
 
 beforeEach(() => {
   getRentalsListingsMock = jest.fn()
+  fetchByTokenIdsMock = jest.fn()
   getOpenRentalsListingsOfNFTsMock = jest.fn()
   fetchMock = jest.fn()
   fetchOneMock = jest.fn()
@@ -30,6 +33,7 @@ beforeEach(() => {
   nftsComponentsMock = {
     fetch: fetchMock,
     fetchOne: fetchOneMock,
+    fetchByTokenIds: fetchByTokenIdsMock,
     count: countMock,
   }
   rentalsNFTSource = createRentalsNFTSource(
@@ -54,6 +58,7 @@ describe('when fetching rented nfts', () => {
         data: {
           results: Array.from({ length: 5 }, (_, i) => ({
             id: i.toString(),
+            nftId: `parcel-${i}`,
             contractAddress: `contractAddress-${i}`,
             tokenId: `tokenId-${i}`,
             startedAt: Date.now(),
@@ -67,24 +72,24 @@ describe('when fetching rented nfts', () => {
       }
       nftResults = rentalsResponse.data.results.map((rental) => ({
         nft: {
+          id: getId(rental.contractAddress, rental.tokenId),
           name: `nft-${rental.id}`,
+          contractAddress: `contractAddress-${rental.id}`,
+          tokenId: `tokenId-${rental.id}`,
           openRentalId: rental.id,
           createdAt: Date.now(),
           soldAt: Date.now(),
         } as NFT,
         order: null,
         rental: null,
       }))
-      rentalsResponse.data.results.forEach((_, i) => {
-        fetchOneMock.mockResolvedValueOnce(nftResults[i])
-      })
-
+      fetchByTokenIdsMock.mockResolvedValueOnce(nftResults)
       getRentalsListingsMock.mockResolvedValueOnce(rentalsResponse)
     })
 
-    it('should return the fetched nfts', () => {
+    it('should return the fetched nfts', async () => {
       return expect(
-        rentalsNFTSource.fetch!({ isOnRent: true })
+        rentalsNFTSource.fetch!({ isOnRent: true, isLand: true })
       ).resolves.toEqual(
         nftResults
           .map((nftResult, i) => ({
@@ -105,7 +110,7 @@ describe('when fetching rented nfts', () => {
 
     it('should reject propagating the error', () => {
       return expect(
-        rentalsNFTSource.fetch!({ isOnRent: true })
+        rentalsNFTSource.fetch!({ isOnRent: true, isLand: true })
       ).rejects.toThrowError('An error occurred')
     })
   })
@@ -127,6 +132,7 @@ describe('when counting rented nfts', () => {
         data: {
           results: Array.from({ length: 5 }, (_, i) => ({
             id: i.toString(),
+            nftId: `nft-id-${i}`,
             contractAddress: `contractAddress-${i}`,
             tokenId: `tokenId-${i}`,
             startedAt: Date.now(),
@@ -144,7 +150,7 @@ describe('when counting rented nfts', () => {
 
     it('should return the number of fetched nfts', () => {
       return expect(
-        rentalsNFTSource.count({ isOnRent: true })
+        rentalsNFTSource.count({ isOnRent: true, isLand: true })
       ).resolves.toEqual(rentalsResponse.data.total)
     })
   })
@@ -158,7 +164,7 @@ describe('when counting rented nfts', () => {
 
     it('should reject propagating the error', () => {
       return expect(
-        rentalsNFTSource.count!({ isOnRent: true })
+        rentalsNFTSource.count!({ isOnRent: true, isLand: true })
       ).rejects.toThrowError('An error occurred')
     })
   })
@@ -183,6 +189,7 @@ describe('when fetching and counting rented nfts', () => {
         data: {
           results: Array.from({ length: 5 }, (_, i) => ({
             id: i.toString(),
+            nftId: `parcel-${i}`,
             contractAddress: `contractAddress-${i}`,
             tokenId: `tokenId-${i}`,
             startedAt: Date.now(),
@@ -196,6 +203,7 @@ describe('when fetching and counting rented nfts', () => {
       }
       nftResults = rentalsResponse.data.results.map((rental) => ({
         nft: {
+          id: getId(rental.contractAddress, rental.tokenId),
           name: `nft-${rental.id}`,
           openRentalId: rental.id,
           createdAt: Date.now(),
@@ -204,16 +212,14 @@ describe('when fetching and counting rented nfts', () => {
         order: null,
         rental: null,
       }))
-      rentalsResponse.data.results.forEach((_, i) => {
-        fetchOneMock.mockResolvedValueOnce(nftResults[i])
-      })
 
+      fetchByTokenIdsMock.mockResolvedValueOnce(nftResults)
       getRentalsListingsMock.mockResolvedValueOnce(rentalsResponse)
     })
 
     it('should return the count and the fetched nfts', () => {
       return expect(
-        rentalsNFTSource.fetchAndCount!({ isOnRent: true })
+        rentalsNFTSource.fetchAndCount!({ isOnRent: true, isLand: true })
       ).resolves.toEqual({
         data: nftResults
           .map((nftResult, i) => ({
@@ -235,7 +241,7 @@ describe('when fetching and counting rented nfts', () => {
 
     it('should reject propagating the error', () => {
       return expect(
-        rentalsNFTSource.fetchAndCount!({ isOnRent: true })
+        rentalsNFTSource.fetchAndCount!({ isOnRent: true, isLand: true })
       ).rejects.toThrowError('An error occurred')
     })
   })

src/tests/logic/nft-rentals.spec.ts

@@ -5,19 +5,23 @@ import {
   shouldFetch,
 } from '../../logic/nfts/rentals'
 
-describe('when checking if the rental should be checked', () => {
+describe('when checking if the rental should be fetched', () => {
   PROHIBITED_FILTERS.forEach((filter) => {
     describe(`and the filter ${filter} is set which is prohibited`, () => {
       it('should return false', () => {
-        expect(shouldFetch({ [filter]: true, isOnRent: true })).toBe(false)
+        expect(
+          shouldFetch({ [filter]: true, isOnRent: true, isLand: true })
+        ).toBe(false)
       })
     })
   })
 
   PROHIBITED_SORTING.forEach((sortBy) => {
     describe(`and the sort by filter is set to ${sortBy} which is prohibited`, () => {
       it('should return false', () => {
-        expect(shouldFetch({ sortBy, isOnRent: true })).toBe(false)
+        expect(shouldFetch({ sortBy, isOnRent: true, isLand: true })).toBe(
+          false
+        )
       })
     })
   })
@@ -43,15 +47,21 @@ describe('when checking if the rental should be checked', () => {
     })
   })
 
-  describe('and the isOnRent filter is not true', () => {
+  describe('and the isOnRent filter is not true but the isLand one is', () => {
     it('should return false', () => {
-      expect(shouldFetch({ isOnRent: false })).toBe(false)
+      expect(shouldFetch({ isOnRent: false, isLand: true })).toBe(false)
     })
   })
 
-  describe('and the isOnRent filter is set to true', () => {
+  describe('and the isOnRent and isLand filter are set to true', () => {
     it('should return true', () => {
-      expect(shouldFetch({ isOnRent: true })).toBe(true)
+      expect(shouldFetch({ isOnRent: true, isLand: true })).toBe(true)
+    })
+  })
+
+  describe('and the isOnRent filter is set but isLand is set to false', () => {
+    it('should return false', () => {
+      expect(shouldFetch({ isOnRent: true, isLand: false })).toBe(false)
     })
   })
   ;[(NFTCategory.ESTATE, NFTCategory.PARCEL)].forEach((category) => {
@@ -73,7 +83,9 @@ describe('when checking if the rental should be checked', () => {
   ].forEach((filter) => {
     describe(`and the filter ${filter} filter is set which is permitted`, () => {
       it('should return true', () => {
-        expect(shouldFetch({ isOnRent: true, [filter]: true })).toBe(true)
+        expect(
+          shouldFetch({ isOnRent: true, isLand: true, [filter]: true })
+        ).toBe(true)
       })
     })
   })