Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Introduce traversePaginatedSourceByHasMore() #340

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Introduce traversePaginatedSourceByHasMore() #340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions packages/app/api-common/src/paginationUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,3 +201,22 @@ export async function getPaginatedEntriesByHasMore<T extends Record<string, unkn

return resultArray
}

export async function traversePaginatedSourceByHasMore<
Copy link
Author

@freiondrej freiondrej Sep 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it stands, this is pretty much just a copy of getPaginatedEntriesByHasMore, with an addition of the processPage() call.

In the future, I can imagine something more parallelized, with apiCall being called in a promise pool as fast as the processPage() is able to process it (as I assume processPage will be the bottleneck), so that we wouldn't need to keep things in memory.

EDIT: https://superchargejs.com/docs/3.x/promise-pool#create-a-promise-pool-from-an-async-iterable might work nicely :) I'll try it if I get a chance, but cannot make any promises ATM.

Comment on lines +204 to +205
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Could we add JSDoc as in the other methods? just to explain clearly how the method works and how it is intended to be used

T extends Record<string, unknown>,
>(
pagination: OptionalPaginationParams,
apiCall: (params: OptionalPaginationParams) => Promise<PaginatedResponse<T>>,
processPage: (pageData: PaginatedResponse<T>['data']) => Promise<void>,
): Promise<void> {
Comment on lines +210 to +211
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Could be interesting to be able to return the results of the processing step? something like

export async function traversePaginatedSourceByHasMore<
  T extends Record<string, unknown>,
  ReturnType = void,
>(
  pagination: OptionalPaginationParams,
  apiCall: (params: OptionalPaginationParams) => Promise<PaginatedResponse<T>>,
  processPage: (pageData: PaginatedResponse<T>['data']) => Promise<ReturnType> | ReturnType,
): Promise<ReturnType> {

An example usage could be to insert something on DB and return all new IDs. What do you think?

let hasMore: boolean | undefined
let currentCursor: string | undefined = pagination.after

do {
const pageResult = await apiCall({ ...pagination, after: currentCursor })
hasMore = pageResult.meta.hasMore
currentCursor = pageResult.meta.cursor

await processPage(pageResult.data)
} while (hasMore)
}
44 changes: 44 additions & 0 deletions packages/app/api-common/test/paginationUtils.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
getPaginatedEntries,
getPaginatedEntriesByHasMore,
} from '../src'
import { traversePaginatedSourceByHasMore } from '../src'

describe('paginationUtils', () => {
describe('createPaginatedResponse', () => {
Expand Down Expand Up @@ -276,6 +277,49 @@ describe('paginationUtils', () => {
expect(result).toEqual([{ id: 'red' }])
})
})
describe('traversePaginatedSourceByHasMore', () => {
it.only('should perform callback per each page', async () => {
// Given
const spy = vi
.spyOn(market, 'getApples')
.mockResolvedValueOnce({
data: [{ id: 'red' }],
meta: {
count: 1,
cursor: 'red',
hasMore: true,
},
})
.mockResolvedValueOnce({
data: [{ id: 'blue' }],
meta: {
count: 1,
cursor: 'blue',
hasMore: false,
},
})

const processPage = vi.fn()

// When
await traversePaginatedSourceByHasMore(
{ limit: 1 },
(params) => {
return market.getApples(params)
},
processPage,
)

// Then
expect(spy).toHaveBeenCalledTimes(2)
expect(spy).toHaveBeenNthCalledWith(1, { limit: 1 })
expect(spy).toHaveBeenNthCalledWith(2, { after: 'red', limit: 1 })

expect(processPage).toHaveBeenCalledTimes(2)
expect(processPage).toHaveBeenNthCalledWith(1, [{ id: 'red' }])
expect(processPage).toHaveBeenNthCalledWith(2, [{ id: 'blue' }])
})
})
})

type Entity = {
Expand Down
Loading