From b38226e625d0860168f04638eec7b27de2f94e44 Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Tue, 19 Nov 2024 06:08:07 -0300 Subject: [PATCH 01/31] feat: pagination --- .../data-access/src/combined-data-access.ts | 8 +++- packages/data-access/src/data-read.ts | 8 +++- packages/data-access/src/in-memory-indexer.ts | 11 ++++- .../src/api/request-network.ts | 42 +++++++++++++++--- .../request-client.js/src/http-data-access.ts | 8 ++++ .../test/api/request-network.test.ts | 34 +++++++++++--- packages/request-logic/src/request-logic.ts | 8 ++++ .../src/request/getChannelsByTopic.ts | 4 +- .../test/getChannelsByTopic.test.ts | 13 ++++++ packages/thegraph-data-access/src/queries.ts | 4 +- .../src/subgraph-client.ts | 5 ++- .../src/transaction-manager.ts | 18 +++++++- .../transaction-manager/test/index.test.ts | 44 ++++++++++++++++--- packages/types/src/data-access-types.ts | 4 ++ packages/types/src/request-logic-types.ts | 4 ++ packages/types/src/storage-types.ts | 6 ++- packages/types/src/transaction-types.ts | 4 ++ 17 files changed, 195 insertions(+), 30 deletions(-) diff --git a/packages/data-access/src/combined-data-access.ts b/packages/data-access/src/combined-data-access.ts index c572b59e5a..a6ab21c834 100644 --- a/packages/data-access/src/combined-data-access.ts +++ b/packages/data-access/src/combined-data-access.ts @@ -26,14 +26,18 @@ export abstract class CombinedDataAccess implements DataAccessTypes.IDataAccess async getChannelsByTopic( topic: string, updatedBetween?: DataAccessTypes.ITimestampBoundaries | undefined, + page?: number | undefined, + pageSize?: number | undefined, ): Promise { - return await this.reader.getChannelsByTopic(topic, updatedBetween); + return await this.reader.getChannelsByTopic(topic, updatedBetween, page, pageSize); } async getChannelsByMultipleTopics( topics: string[], updatedBetween?: DataAccessTypes.ITimestampBoundaries, + page?: number | undefined, + pageSize?: number | undefined, ): Promise { - return await this.reader.getChannelsByMultipleTopics(topics, updatedBetween); + return await this.reader.getChannelsByMultipleTopics(topics, updatedBetween, page, pageSize); } async persistTransaction( transactionData: DataAccessTypes.ITransaction, diff --git a/packages/data-access/src/data-read.ts b/packages/data-access/src/data-read.ts index a4026fd482..205c0af218 100644 --- a/packages/data-access/src/data-read.ts +++ b/packages/data-access/src/data-read.ts @@ -49,15 +49,19 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { async getChannelsByTopic( topic: string, updatedBetween?: DataAccessTypes.ITimestampBoundaries | undefined, + page?: number | undefined, + pageSize?: number | undefined, ): Promise { - return this.getChannelsByMultipleTopics([topic], updatedBetween); + return this.getChannelsByMultipleTopics([topic], updatedBetween, page, pageSize); } async getChannelsByMultipleTopics( topics: string[], updatedBetween?: DataAccessTypes.ITimestampBoundaries, + page?: number, + pageSize?: number, ): Promise { - const result = await this.storage.getTransactionsByTopics(topics); + const result = await this.storage.getTransactionsByTopics(topics, page, pageSize); const pending = this.pendingStore?.findByTopics(topics) || []; const pendingItems = pending.map((item) => ({ diff --git a/packages/data-access/src/in-memory-indexer.ts b/packages/data-access/src/in-memory-indexer.ts index 929f3cb6b2..5e0bd1d616 100644 --- a/packages/data-access/src/in-memory-indexer.ts +++ b/packages/data-access/src/in-memory-indexer.ts @@ -55,8 +55,15 @@ export class InMemoryIndexer implements StorageTypes.IIndexer { }; } - async getTransactionsByTopics(topics: string[]): Promise { - const channelIds = topics.map((topic) => this.#topicToChannelsIndex.get(topic)).flat(); + async getTransactionsByTopics( + topics: string[], + page?: number, + pageSize?: number, + ): Promise { + let channelIds = topics.map((topic) => this.#topicToChannelsIndex.get(topic)).flat(); + if (page && pageSize) { + channelIds = channelIds.slice((page - 1) * pageSize, page * pageSize); + } const locations = channelIds .map((channel) => this.#channelToLocationsIndex.get(channel)) .flat(); diff --git a/packages/request-client.js/src/api/request-network.ts b/packages/request-client.js/src/api/request-network.ts index 267fb2d579..d0b4afaf14 100644 --- a/packages/request-client.js/src/api/request-network.ts +++ b/packages/request-client.js/src/api/request-network.ts @@ -265,7 +265,12 @@ export default class RequestNetwork { public async fromIdentity( identity: IdentityTypes.IIdentity, updatedBetween?: Types.ITimestampBoundaries, - options?: { disablePaymentDetection?: boolean; disableEvents?: boolean }, + options?: { + disablePaymentDetection?: boolean; + disableEvents?: boolean; + page?: number; + pageSize?: number; + }, ): Promise { if (!this.supportedIdentities.includes(identity.type)) { throw new Error(`${identity.type} is not supported`); @@ -283,7 +288,12 @@ export default class RequestNetwork { public async fromMultipleIdentities( identities: IdentityTypes.IIdentity[], updatedBetween?: Types.ITimestampBoundaries, - options?: { disablePaymentDetection?: boolean; disableEvents?: boolean }, + options?: { + disablePaymentDetection?: boolean; + disableEvents?: boolean; + page?: number; + pageSize?: number; + }, ): Promise { const identityNotSupported = identities.find( (identity) => !this.supportedIdentities.includes(identity.type), @@ -306,11 +316,21 @@ export default class RequestNetwork { public async fromTopic( topic: any, updatedBetween?: Types.ITimestampBoundaries, - options?: { disablePaymentDetection?: boolean; disableEvents?: boolean }, + options?: { + disablePaymentDetection?: boolean; + disableEvents?: boolean; + page?: number; + pageSize?: number; + }, ): Promise { // Gets all the requests indexed by the value of the identity const requestsAndMeta: RequestLogicTypes.IReturnGetRequestsByTopic = - await this.requestLogic.getRequestsByTopic(topic, updatedBetween); + await this.requestLogic.getRequestsByTopic( + topic, + updatedBetween, + options?.page, + options?.pageSize, + ); // From the requests of the request-logic layer creates the request objects and gets the payment networks const requestPromises = requestsAndMeta.result.requests.map( async (requestFromLogic: { @@ -358,11 +378,21 @@ export default class RequestNetwork { public async fromMultipleTopics( topics: any[], updatedBetween?: Types.ITimestampBoundaries, - options?: { disablePaymentDetection?: boolean; disableEvents?: boolean }, + options?: { + disablePaymentDetection?: boolean; + disableEvents?: boolean; + page?: number; + pageSize?: number; + }, ): Promise { // Gets all the requests indexed by the value of the identity const requestsAndMeta: RequestLogicTypes.IReturnGetRequestsByTopic = - await this.requestLogic.getRequestsByMultipleTopics(topics, updatedBetween); + await this.requestLogic.getRequestsByMultipleTopics( + topics, + updatedBetween, + options?.page, + options?.pageSize, + ); // From the requests of the request-logic layer creates the request objects and gets the payment networks const requestPromises = requestsAndMeta.result.requests.map( diff --git a/packages/request-client.js/src/http-data-access.ts b/packages/request-client.js/src/http-data-access.ts index cf9cadd50f..a99b714151 100644 --- a/packages/request-client.js/src/http-data-access.ts +++ b/packages/request-client.js/src/http-data-access.ts @@ -175,10 +175,14 @@ export default class HttpDataAccess implements DataAccessTypes.IDataAccess { public async getChannelsByTopic( topic: string, updatedBetween?: DataAccessTypes.ITimestampBoundaries, + page?: number, + pageSize?: number, ): Promise { return await this.fetchAndRetry('/getChannelsByTopic', { topic, updatedBetween, + page, + pageSize, }); } @@ -191,10 +195,14 @@ export default class HttpDataAccess implements DataAccessTypes.IDataAccess { public async getChannelsByMultipleTopics( topics: string[], updatedBetween?: DataAccessTypes.ITimestampBoundaries, + page?: number, + pageSize?: number, ): Promise { return await this.fetchAndRetry('/getChannelsByMultipleTopics', { topics, updatedBetween, + page, + pageSize, }); } diff --git a/packages/request-client.js/test/api/request-network.test.ts b/packages/request-client.js/test/api/request-network.test.ts index e3403de035..ae914d051f 100644 --- a/packages/request-client.js/test/api/request-network.test.ts +++ b/packages/request-client.js/test/api/request-network.test.ts @@ -95,7 +95,12 @@ describe('api/request-network', () => { it('can get requests with payment network fromIdentity', async () => { const mockDataAccessWithTxs: DataAccessTypes.IDataAccess = { ...mockDataAccess, - async getChannelsByTopic(topic: string): Promise { + async getChannelsByTopic( + topic: string, + updatedBetween?: DataAccessTypes.ITimestampBoundaries, + page?: number, + pageSize?: number, + ): Promise { expect(topic).toBe('01f1a21ab419611dbf492b3136ac231c8773dc897ee0eb5167ef2051a39e685e76'); return { meta: { @@ -137,7 +142,14 @@ describe('api/request-network', () => { }; const requestnetwork = new RequestNetwork({ dataAccess: mockDataAccessWithTxs }); - const requests: Request[] = await requestnetwork.fromIdentity(TestData.payee.identity); + const requests: Request[] = await requestnetwork.fromIdentity( + TestData.payee.identity, + undefined, + { + page: 1, + pageSize: 10, + }, + ); expect(requests.length).toBe(2); expect(requests[0].requestId).toBe(TestData.actionRequestId); @@ -201,7 +213,12 @@ describe('api/request-network', () => { it('can get requests with payment network from multiple Identities', async () => { const mockDataAccessWithTxs: DataAccessTypes.IDataAccess = { ...mockDataAccess, - async getChannelsByMultipleTopics(topics: [string]): Promise { + async getChannelsByMultipleTopics( + topics: [string], + updatedBetween?: DataAccessTypes.ITimestampBoundaries, + page?: number, + pageSize?: number, + ): Promise { expect(topics).toEqual([ '01f1a21ab419611dbf492b3136ac231c8773dc897ee0eb5167ef2051a39e685e76', ]); @@ -245,9 +262,14 @@ describe('api/request-network', () => { }; const requestnetwork = new RequestNetwork({ dataAccess: mockDataAccessWithTxs }); - const requests: Request[] = await requestnetwork.fromMultipleIdentities([ - TestData.payee.identity, - ]); + const requests: Request[] = await requestnetwork.fromMultipleIdentities( + [TestData.payee.identity], + undefined, + { + page: 1, + pageSize: 10, + }, + ); expect(requests.length).toBe(2); expect(requests[0].requestId).toBe(TestData.actionRequestId); diff --git a/packages/request-logic/src/request-logic.ts b/packages/request-logic/src/request-logic.ts index ce24fec2c0..7989f5fc83 100644 --- a/packages/request-logic/src/request-logic.ts +++ b/packages/request-logic/src/request-logic.ts @@ -346,6 +346,8 @@ export default class RequestLogic implements RequestLogicTypes.IRequestLogic { public async getRequestsByTopic( topic: string, updatedBetween?: RequestLogicTypes.ITimestampBoundaries, + page?: number, + pageSize?: number, ): Promise { // hash all the topics const hashedTopic = MultiFormat.serialize(normalizeKeccak256Hash(topic)); @@ -353,6 +355,8 @@ export default class RequestLogic implements RequestLogicTypes.IRequestLogic { const getChannelsResult = await this.transactionManager.getChannelsByTopic( hashedTopic, updatedBetween, + page, + pageSize, ); return this.computeMultipleRequestFromChannels(getChannelsResult); } @@ -365,6 +369,8 @@ export default class RequestLogic implements RequestLogicTypes.IRequestLogic { public async getRequestsByMultipleTopics( topics: string[], updatedBetween?: RequestLogicTypes.ITimestampBoundaries, + page?: number, + pageSize?: number, ): Promise { // hash all the topics const hashedTopics = topics.map((topic) => @@ -374,6 +380,8 @@ export default class RequestLogic implements RequestLogicTypes.IRequestLogic { const getChannelsResult = await this.transactionManager.getChannelsByMultipleTopics( hashedTopics, updatedBetween, + page, + pageSize, ); return this.computeMultipleRequestFromChannels(getChannelsResult); } diff --git a/packages/request-node/src/request/getChannelsByTopic.ts b/packages/request-node/src/request/getChannelsByTopic.ts index 8f847b7eb2..4920dfa79d 100644 --- a/packages/request-node/src/request/getChannelsByTopic.ts +++ b/packages/request-node/src/request/getChannelsByTopic.ts @@ -21,7 +21,7 @@ export default class GetChannelHandler { // Retrieves data access layer let transactions; - const { updatedBetween, topic } = clientRequest.query; + const { updatedBetween, topic, page, pageSize } = clientRequest.query; // Verifies if data sent from get request are correct // clientRequest.query is expected to contain the topic of the transactions to search for if (!topic || typeof topic !== 'string') { @@ -34,6 +34,8 @@ export default class GetChannelHandler { updatedBetween && typeof updatedBetween === 'string' ? JSON.parse(updatedBetween) : undefined, + Number(page), + Number(pageSize), ); serverResponse.status(StatusCodes.OK).send(transactions); diff --git a/packages/request-node/test/getChannelsByTopic.test.ts b/packages/request-node/test/getChannelsByTopic.test.ts index ccd50ff07a..f45d9764ab 100644 --- a/packages/request-node/test/getChannelsByTopic.test.ts +++ b/packages/request-node/test/getChannelsByTopic.test.ts @@ -98,6 +98,19 @@ describe('getChannelsByTopic', () => { }), ); + // If we search for the fisrt topic, by paginating, there should be one transaction + serverResponse = await request(server) + .get('/getChannelsByTopic') + .query({ topic: commonTopic, page: 1, pageSize: 1 }) + .set('Accept', 'application/json') + .expect(StatusCodes.OK); + + expect(serverResponse.body.result.transactions).toMatchObject( + expect.objectContaining({ + [channelId]: [expect.objectContaining({ transaction: transactionData })], + }), + ); + // confirm the transactions for clean shutdown const provider = new providers.JsonRpcProvider(); const confirm = (txData: unknown) => { diff --git a/packages/thegraph-data-access/src/queries.ts b/packages/thegraph-data-access/src/queries.ts index 42c811902a..e25e102c95 100644 --- a/packages/thegraph-data-access/src/queries.ts +++ b/packages/thegraph-data-access/src/queries.ts @@ -77,10 +77,12 @@ export const GetTransactionsByHashQuery = gql` export const GetTransactionsByTopics = gql` ${TransactionsBodyFragment} -query GetTransactionsByTopics($topics: [String!]!){ +query GetTransactionsByTopics($topics: [String!]!, $first: Int!, $skip: Int!) { ${metaQueryBody} channels( where: { topics_contains: $topics } + first: $first + skip: $skip ){ transactions( orderBy: blockTimestamp, diff --git a/packages/thegraph-data-access/src/subgraph-client.ts b/packages/thegraph-data-access/src/subgraph-client.ts index a9b786c7ec..e1450fa8a4 100644 --- a/packages/thegraph-data-access/src/subgraph-client.ts +++ b/packages/thegraph-data-access/src/subgraph-client.ts @@ -50,10 +50,13 @@ export class SubgraphClient implements StorageTypes.IIndexer { public async getTransactionsByTopics( topics: string[], + page?: number, + pageSize?: number, ): Promise { + const skip = page && pageSize ? (page - 1) * pageSize : 0; const { _meta, channels } = await this.graphql.request< Meta & { channels: { transactions: Transaction[] }[] } - >(GetTransactionsByTopics, { topics }); + >(GetTransactionsByTopics, { topics, first: pageSize, skip }); const transactionsByChannel = channels .map(({ transactions }) => transactions) diff --git a/packages/transaction-manager/src/transaction-manager.ts b/packages/transaction-manager/src/transaction-manager.ts index 81cded9559..7d8972d06a 100644 --- a/packages/transaction-manager/src/transaction-manager.ts +++ b/packages/transaction-manager/src/transaction-manager.ts @@ -186,8 +186,15 @@ export default class TransactionManager implements TransactionTypes.ITransaction public async getChannelsByTopic( topic: string, updatedBetween?: TransactionTypes.ITimestampBoundaries, + page?: number, + pageSize?: number, ): Promise { - const resultGetTx = await this.dataAccess.getChannelsByTopic(topic, updatedBetween); + const resultGetTx = await this.dataAccess.getChannelsByTopic( + topic, + updatedBetween, + page, + pageSize, + ); return this.parseMultipleChannels(resultGetTx); } @@ -202,8 +209,15 @@ export default class TransactionManager implements TransactionTypes.ITransaction public async getChannelsByMultipleTopics( topics: string[], updatedBetween?: TransactionTypes.ITimestampBoundaries, + page?: number, + pageSize?: number, ): Promise { - const resultGetTx = await this.dataAccess.getChannelsByMultipleTopics(topics, updatedBetween); + const resultGetTx = await this.dataAccess.getChannelsByMultipleTopics( + topics, + updatedBetween, + page, + pageSize, + ); return this.parseMultipleChannels(resultGetTx); } diff --git a/packages/transaction-manager/test/index.test.ts b/packages/transaction-manager/test/index.test.ts index 2c2e1be486..394587b029 100644 --- a/packages/transaction-manager/test/index.test.ts +++ b/packages/transaction-manager/test/index.test.ts @@ -1126,7 +1126,12 @@ describe('index', () => { }, }), ); - expect(fakeDataAccess.getChannelsByTopic).toHaveBeenCalledWith(extraTopics[0], undefined); + expect(fakeDataAccess.getChannelsByTopic).toHaveBeenCalledWith( + extraTopics[0], + undefined, + undefined, + undefined, + ); }); it('can get an encrypted channel indexed by topic', async () => { @@ -1189,7 +1194,12 @@ describe('index', () => { }, }), ); - expect(fakeDataAccess.getChannelsByTopic).toHaveBeenCalledWith(extraTopics[0], undefined); + expect(fakeDataAccess.getChannelsByTopic).toHaveBeenCalledWith( + extraTopics[0], + undefined, + undefined, + undefined, + ); }); it('cannot get an encrypted channel indexed by topic without decryptionProvider', async () => { @@ -1258,7 +1268,12 @@ describe('index', () => { }, }), ); - expect(fakeDataAccess.getChannelsByTopic).toHaveBeenCalledWith(extraTopics[0], undefined); + expect(fakeDataAccess.getChannelsByTopic).toHaveBeenCalledWith( + extraTopics[0], + undefined, + undefined, + undefined, + ); }); it('can get an clear channel indexed by topic without decryptionProvider even if an encrypted transaction happen first', async () => { @@ -1340,7 +1355,12 @@ describe('index', () => { }, }), ); - expect(fakeDataAccess.getChannelsByTopic).toHaveBeenCalledWith(extraTopics[0], undefined); + expect(fakeDataAccess.getChannelsByTopic).toHaveBeenCalledWith( + extraTopics[0], + undefined, + undefined, + undefined, + ); }); it('can get channels indexed by topics with channelId not matching the first transaction hash', async () => { @@ -1393,7 +1413,12 @@ describe('index', () => { }, }), ); - expect(fakeDataAccess.getChannelsByTopic).toHaveBeenCalledWith(extraTopics[0], undefined); + expect(fakeDataAccess.getChannelsByTopic).toHaveBeenCalledWith( + extraTopics[0], + undefined, + undefined, + undefined, + ); }); it('can get channels encrypted and clear', async () => { @@ -1466,7 +1491,12 @@ describe('index', () => { }, }), ); - expect(fakeDataAccess.getChannelsByTopic).toHaveBeenCalledWith(extraTopics[0], undefined); + expect(fakeDataAccess.getChannelsByTopic).toHaveBeenCalledWith( + extraTopics[0], + undefined, + undefined, + undefined, + ); }); }); @@ -1491,6 +1521,8 @@ describe('index', () => { expect(fakeDataAccess.getChannelsByMultipleTopics).toHaveBeenCalledWith( [extraTopics[0]], undefined, + undefined, + undefined, ); }); }); diff --git a/packages/types/src/data-access-types.ts b/packages/types/src/data-access-types.ts index 0899640e42..300cbe3045 100644 --- a/packages/types/src/data-access-types.ts +++ b/packages/types/src/data-access-types.ts @@ -13,10 +13,14 @@ export interface IDataRead { getChannelsByTopic: ( topic: string, updatedBetween?: ITimestampBoundaries, + page?: number, + pageSize?: number, ) => Promise; getChannelsByMultipleTopics( topics: string[], updatedBetween?: ITimestampBoundaries, + page?: number, + pageSize?: number, ): Promise; } diff --git a/packages/types/src/request-logic-types.ts b/packages/types/src/request-logic-types.ts index 2caabf596b..7550e3a602 100644 --- a/packages/types/src/request-logic-types.ts +++ b/packages/types/src/request-logic-types.ts @@ -61,10 +61,14 @@ export interface IRequestLogic { getRequestsByTopic: ( topic: any, updatedBetween?: ITimestampBoundaries, + page?: number, + pageSize?: number, ) => Promise; getRequestsByMultipleTopics: ( topics: any[], updatedBetween?: ITimestampBoundaries, + page?: number, + pageSize?: number, ) => Promise; } diff --git a/packages/types/src/storage-types.ts b/packages/types/src/storage-types.ts index 8b312cbaf1..dc8c0e133c 100644 --- a/packages/types/src/storage-types.ts +++ b/packages/types/src/storage-types.ts @@ -53,7 +53,11 @@ export interface IIndexer { channel: string, updatedBetween?: ITimestampBoundaries, ): Promise; - getTransactionsByTopics(topics: string[]): Promise; + getTransactionsByTopics( + topics: string[], + page?: number, + pageSize?: number, + ): Promise; } export type IIpfsConfig = { diff --git a/packages/types/src/transaction-types.ts b/packages/types/src/transaction-types.ts index 04e55f7d58..68b56f75eb 100644 --- a/packages/types/src/transaction-types.ts +++ b/packages/types/src/transaction-types.ts @@ -16,10 +16,14 @@ export interface ITransactionManager { getChannelsByTopic: ( topic: string, updatedBetween?: ITimestampBoundaries, + page?: number, + pageSize?: number, ) => Promise; getChannelsByMultipleTopics: ( topics: string[], updatedBetween?: ITimestampBoundaries, + page?: number, + pageSize?: number, ) => Promise; } From fdd7f5fc17733dc52fa6fc849ae04a7d20eefb52 Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Tue, 19 Nov 2024 06:22:34 -0300 Subject: [PATCH 02/31] fix: first and skip --- packages/thegraph-data-access/src/subgraph-client.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/thegraph-data-access/src/subgraph-client.ts b/packages/thegraph-data-access/src/subgraph-client.ts index e1450fa8a4..b36ed4fafb 100644 --- a/packages/thegraph-data-access/src/subgraph-client.ts +++ b/packages/thegraph-data-access/src/subgraph-client.ts @@ -53,10 +53,11 @@ export class SubgraphClient implements StorageTypes.IIndexer { page?: number, pageSize?: number, ): Promise { - const skip = page && pageSize ? (page - 1) * pageSize : 0; + const skip = page && pageSize ? (page - 1) * pageSize : null; + const first = pageSize || null; const { _meta, channels } = await this.graphql.request< Meta & { channels: { transactions: Transaction[] }[] } - >(GetTransactionsByTopics, { topics, first: pageSize, skip }); + >(GetTransactionsByTopics, { topics, first, skip }); const transactionsByChannel = channels .map(({ transactions }) => transactions) From 378c2299218aec42414b0ee7c059b045902e5bf8 Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Tue, 19 Nov 2024 06:30:55 -0300 Subject: [PATCH 03/31] fix: Number conversion issue --- packages/request-node/src/request/getChannelsByTopic.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/request-node/src/request/getChannelsByTopic.ts b/packages/request-node/src/request/getChannelsByTopic.ts index 4920dfa79d..a753bb9bb9 100644 --- a/packages/request-node/src/request/getChannelsByTopic.ts +++ b/packages/request-node/src/request/getChannelsByTopic.ts @@ -34,8 +34,8 @@ export default class GetChannelHandler { updatedBetween && typeof updatedBetween === 'string' ? JSON.parse(updatedBetween) : undefined, - Number(page), - Number(pageSize), + page && typeof page === 'string' ? parseInt(page, 10) : undefined, + pageSize && typeof pageSize === 'string' ? parseInt(pageSize, 10) : undefined, ); serverResponse.status(StatusCodes.OK).send(transactions); From 5857fafb6039be0d353601061f895cabf7eb4927 Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Tue, 19 Nov 2024 06:35:23 -0300 Subject: [PATCH 04/31] fix: pagination validation before subgraph query --- packages/thegraph-data-access/src/subgraph-client.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/thegraph-data-access/src/subgraph-client.ts b/packages/thegraph-data-access/src/subgraph-client.ts index b36ed4fafb..beae26bb6b 100644 --- a/packages/thegraph-data-access/src/subgraph-client.ts +++ b/packages/thegraph-data-access/src/subgraph-client.ts @@ -53,6 +53,13 @@ export class SubgraphClient implements StorageTypes.IIndexer { page?: number, pageSize?: number, ): Promise { + if (page !== undefined && page < 1) { + throw new Error('Page must be greater than or equal to 1'); + } + if (pageSize !== undefined && pageSize <= 0) { + throw new Error('Page size must be greater than 0'); + } + const skip = page && pageSize ? (page - 1) * pageSize : null; const first = pageSize || null; const { _meta, channels } = await this.graphql.request< From 2cb1afeaa94f4c0c703d94d95cc4e0cc1def032f Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Tue, 19 Nov 2024 07:10:58 -0300 Subject: [PATCH 05/31] fix: per coderabitai review --- packages/data-access/src/data-read.ts | 21 +++++++---- packages/data-access/src/in-memory-indexer.ts | 20 ++++++++++- .../request-client.js/src/http-data-access.ts | 7 ++++ .../transaction-manager/test/index.test.ts | 35 +++++++++++++++++++ packages/types/src/data-access-types.ts | 7 ++++ packages/types/src/storage-types.ts | 6 ++++ 6 files changed, 88 insertions(+), 8 deletions(-) diff --git a/packages/data-access/src/data-read.ts b/packages/data-access/src/data-read.ts index 205c0af218..720b859357 100644 --- a/packages/data-access/src/data-read.ts +++ b/packages/data-access/src/data-read.ts @@ -91,6 +91,14 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { ).map((x) => x.channelId); const filteredTxs = transactions.filter((tx) => channels.includes(tx.channelId)); + const finalTransactions = filteredTxs.reduce((prev, curr) => { + if (!prev[curr.channelId]) { + prev[curr.channelId] = []; + } + prev[curr.channelId].push(this.toTimestampedTransaction(curr)); + return prev; + }, {} as DataAccessTypes.ITransactionsByChannelIds); + return { meta: { storageMeta: filteredTxs.reduce( @@ -110,15 +118,14 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { }, {} as Record, ), + pagination: { + page: page, + pageSize: pageSize, + total: finalTransactions.length, + }, }, result: { - transactions: filteredTxs.reduce((prev, curr) => { - if (!prev[curr.channelId]) { - prev[curr.channelId] = []; - } - prev[curr.channelId].push(this.toTimestampedTransaction(curr)); - return prev; - }, {} as DataAccessTypes.ITransactionsByChannelIds), + transactions: finalTransactions, }, }; } diff --git a/packages/data-access/src/in-memory-indexer.ts b/packages/data-access/src/in-memory-indexer.ts index 5e0bd1d616..b9e43e1470 100644 --- a/packages/data-access/src/in-memory-indexer.ts +++ b/packages/data-access/src/in-memory-indexer.ts @@ -60,9 +60,27 @@ export class InMemoryIndexer implements StorageTypes.IIndexer { page?: number, pageSize?: number, ): Promise { + if (page !== undefined && page < 1) { + throw new Error('Page must be greater than or equal to 1'); + } + if (pageSize !== undefined && pageSize <= 0) { + throw new Error('Page size must be greater than 0'); + } + let channelIds = topics.map((topic) => this.#topicToChannelsIndex.get(topic)).flat(); + const total = channelIds.length; + if (page && pageSize) { - channelIds = channelIds.slice((page - 1) * pageSize, page * pageSize); + const start = (page - 1) * pageSize; + // Return empty result if page exceeds available data + if (start >= total) { + return { + blockNumber: 0, + transactions: [], + pagination: { total, page, pageSize }, + }; + } + channelIds = channelIds.slice(start, start + pageSize); } const locations = channelIds .map((channel) => this.#channelToLocationsIndex.get(channel)) diff --git a/packages/request-client.js/src/http-data-access.ts b/packages/request-client.js/src/http-data-access.ts index a99b714151..fe04952d11 100644 --- a/packages/request-client.js/src/http-data-access.ts +++ b/packages/request-client.js/src/http-data-access.ts @@ -178,6 +178,13 @@ export default class HttpDataAccess implements DataAccessTypes.IDataAccess { page?: number, pageSize?: number, ): Promise { + if (page !== undefined && page < 1) { + throw new Error('Page must be greater than or equal to 1'); + } + if (pageSize !== undefined && pageSize <= 0) { + throw new Error('Page size must be greater than 0'); + } + return await this.fetchAndRetry('/getChannelsByTopic', { topic, updatedBetween, diff --git a/packages/transaction-manager/test/index.test.ts b/packages/transaction-manager/test/index.test.ts index 394587b029..f7ba03d18b 100644 --- a/packages/transaction-manager/test/index.test.ts +++ b/packages/transaction-manager/test/index.test.ts @@ -1498,6 +1498,22 @@ describe('index', () => { undefined, ); }); + + it('should return paginated results when page and pageSize are specified', async () => { + const transactionManager = new TransactionManager(fakeDataAccess); + + // Test first page + const page1 = await transactionManager.getChannelsByTopic(extraTopics[0], undefined, 1, 2); + expect(page1.result.transactions).toHaveLength(2); + + // Test second page + const page2 = await transactionManager.getChannelsByTopic(extraTopics[0], undefined, 2, 2); + expect(page2.result.transactions).toHaveLength(2); + + // Test last page + const lastPage = await transactionManager.getChannelsByTopic(extraTopics[0], undefined, 3, 2); + expect(lastPage.result.transactions).toHaveLength(1); + }); }); describe('getChannelsByMultipleTopic', () => { @@ -1525,5 +1541,24 @@ describe('index', () => { undefined, ); }); + + it('should return paginated results when querying multiple topics', async () => { + const transactionManager = new TransactionManager(fakeDataAccess); + + const result = await transactionManager.getChannelsByMultipleTopics( + [extraTopics[0], extraTopics[1]], + undefined, + 1, // page + 2, // pageSize + ); + + expect(result.result.transactions).toHaveLength(2); + expect(fakeDataAccess.getChannelsByMultipleTopics).toHaveBeenCalledWith( + [extraTopics[0], extraTopics[1]], + 1, + 2, + undefined, + ); + }); }); }); diff --git a/packages/types/src/data-access-types.ts b/packages/types/src/data-access-types.ts index 300cbe3045..e6c410dadb 100644 --- a/packages/types/src/data-access-types.ts +++ b/packages/types/src/data-access-types.ts @@ -73,6 +73,12 @@ export type PersistTransactionEmitter = ConfirmationEventEmitter Promise; append: (data: string) => Promise; } +interface PaginationMetadata { + total: number; // Total number of items available + page?: number; // Current page number if pagination was used + pageSize?: number; // Page size if pagination was used +} export type IGetTransactionsResponse = { transactions: IIndexedTransaction[]; blockNumber: number; + pagination?: PaginationMetadata; // Optional pagination metadata }; export interface IStorageRead { From a0e320c617f138cd84922b826b82e6b36847a9c7 Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Tue, 19 Nov 2024 07:26:31 -0300 Subject: [PATCH 06/31] fix: per coderabitai review second time --- packages/data-access/src/in-memory-indexer.ts | 11 +++- .../src/subgraph-client.ts | 19 +++++- .../transaction-manager/test/index.test.ts | 60 +++++++++++++++++++ packages/types/src/storage-types.ts | 1 + 4 files changed, 85 insertions(+), 6 deletions(-) diff --git a/packages/data-access/src/in-memory-indexer.ts b/packages/data-access/src/in-memory-indexer.ts index b9e43e1470..22ab05aab5 100644 --- a/packages/data-access/src/in-memory-indexer.ts +++ b/packages/data-access/src/in-memory-indexer.ts @@ -67,8 +67,10 @@ export class InMemoryIndexer implements StorageTypes.IIndexer { throw new Error('Page size must be greater than 0'); } - let channelIds = topics.map((topic) => this.#topicToChannelsIndex.get(topic)).flat(); - const total = channelIds.length; + // Efficiently get total count without creating intermediate array + const channelIdsSet = new Set(topics.flatMap((topic) => this.#topicToChannelsIndex.get(topic))); + const total = channelIdsSet.size; + let channelIds = Array.from(channelIdsSet); if (page && pageSize) { const start = (page - 1) * pageSize; @@ -77,7 +79,10 @@ export class InMemoryIndexer implements StorageTypes.IIndexer { return { blockNumber: 0, transactions: [], - pagination: { total, page, pageSize }, + pagination: + page && pageSize + ? { total, page, pageSize, hasMore: page * pageSize < total } + : undefined, }; } channelIds = channelIds.slice(start, start + pageSize); diff --git a/packages/thegraph-data-access/src/subgraph-client.ts b/packages/thegraph-data-access/src/subgraph-client.ts index beae26bb6b..484a410532 100644 --- a/packages/thegraph-data-access/src/subgraph-client.ts +++ b/packages/thegraph-data-access/src/subgraph-client.ts @@ -18,6 +18,10 @@ const MAX_INT_VALUE = 0x7fffffff; export class SubgraphClient implements StorageTypes.IIndexer { private graphql: GraphQLClient; public readonly endpoint: string; + + private readonly DEFAULT_PAGE_SIZE = 10; + private readonly MAX_PAGE_SIZE = 100; + constructor(endpoint: string, options?: RequestConfig) { this.endpoint = endpoint; this.graphql = new GraphQLClient(endpoint, options); @@ -59,12 +63,21 @@ export class SubgraphClient implements StorageTypes.IIndexer { if (pageSize !== undefined && pageSize <= 0) { throw new Error('Page size must be greater than 0'); } + if (pageSize && pageSize > this.MAX_PAGE_SIZE) { + throw new Error(`Page size cannot exceed ${this.MAX_PAGE_SIZE}`); + } + + const effectivePageSize = pageSize ?? this.DEFAULT_PAGE_SIZE; + const effectivePage = page ?? 1; + const skip = (effectivePage - 1) * effectivePageSize; - const skip = page && pageSize ? (page - 1) * pageSize : null; - const first = pageSize || null; const { _meta, channels } = await this.graphql.request< Meta & { channels: { transactions: Transaction[] }[] } - >(GetTransactionsByTopics, { topics, first, skip }); + >(GetTransactionsByTopics, { + topics, + first: effectivePageSize, + skip, + }); const transactionsByChannel = channels .map(({ transactions }) => transactions) diff --git a/packages/transaction-manager/test/index.test.ts b/packages/transaction-manager/test/index.test.ts index f7ba03d18b..c9ced2ab44 100644 --- a/packages/transaction-manager/test/index.test.ts +++ b/packages/transaction-manager/test/index.test.ts @@ -27,6 +27,24 @@ const tx2: DataAccessTypes.ITimestampedTransaction = { transaction: { data: data2 }, }; +const tx3: DataAccessTypes.ITimestampedTransaction = { + state: TransactionTypes.TransactionState.PENDING, + timestamp: 1, + transaction: { data: data }, +}; + +const tx4: DataAccessTypes.ITimestampedTransaction = { + state: TransactionTypes.TransactionState.PENDING, + timestamp: 1, + transaction: { data: data2 }, +}; + +const tx5: DataAccessTypes.ITimestampedTransaction = { + state: TransactionTypes.TransactionState.PENDING, + timestamp: 1, + transaction: { data: data }, +}; + const dataHash = normalizeKeccak256Hash(JSON.parse(data)); const channelId = MultiFormat.serialize(dataHash); const dataHash2 = normalizeKeccak256Hash(JSON.parse(data2)); @@ -1505,14 +1523,56 @@ describe('index', () => { // Test first page const page1 = await transactionManager.getChannelsByTopic(extraTopics[0], undefined, 1, 2); expect(page1.result.transactions).toHaveLength(2); + expect(page1.result.transactions).toEqual([tx, tx2]); // Verify content + expect(page1.meta.pagination).toEqual({ + currentPage: 1, + pageSize: 2, + totalItems: 5, + totalPages: 3, + }); // Test second page const page2 = await transactionManager.getChannelsByTopic(extraTopics[0], undefined, 2, 2); expect(page2.result.transactions).toHaveLength(2); + expect(page2.result.transactions).toEqual([tx3, tx4]); // Verify content // Test last page const lastPage = await transactionManager.getChannelsByTopic(extraTopics[0], undefined, 3, 2); expect(lastPage.result.transactions).toHaveLength(1); + expect(lastPage.result.transactions).toEqual([tx5]); // Verify content + }); + + it('should handle pagination edge cases', async () => { + const transactionManager = new TransactionManager(fakeDataAccess); + + // Test empty results + const emptyPage = await transactionManager.getChannelsByTopic( + 'nonexistent-topic', + undefined, + 1, + 10, + ); + expect(emptyPage.result.transactions).toHaveLength(0); + expect(emptyPage.meta.pagination.totalItems).toBe(0); + + // Test invalid page number + await expect( + transactionManager.getChannelsByTopic(extraTopics[0], undefined, 0, 10), + ).rejects.toThrow('Invalid page number'); + + // Test invalid page size + await expect( + transactionManager.getChannelsByTopic(extraTopics[0], undefined, 1, 0), + ).rejects.toThrow('Invalid page size'); + + // Test page number beyond total pages + const beyondLastPage = await transactionManager.getChannelsByTopic( + extraTopics[0], + undefined, + 999, + 10, + ); + expect(beyondLastPage.result.transactions).toHaveLength(0); }); }); diff --git a/packages/types/src/storage-types.ts b/packages/types/src/storage-types.ts index 3a8712ce1c..84afb17655 100644 --- a/packages/types/src/storage-types.ts +++ b/packages/types/src/storage-types.ts @@ -32,6 +32,7 @@ interface PaginationMetadata { total: number; // Total number of items available page?: number; // Current page number if pagination was used pageSize?: number; // Page size if pagination was used + hasMore: boolean; // Whether there are more items available } export type IGetTransactionsResponse = { From 3e05e008ce38aafbaeb1804680a2155e4621f33c Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Tue, 19 Nov 2024 07:40:55 -0300 Subject: [PATCH 07/31] fix: as per coderabitai review for the third time --- packages/data-access/src/data-read.ts | 62 +++++++++++++++++++++------ 1 file changed, 48 insertions(+), 14 deletions(-) diff --git a/packages/data-access/src/data-read.ts b/packages/data-access/src/data-read.ts index 720b859357..60401d8b9a 100644 --- a/packages/data-access/src/data-read.ts +++ b/packages/data-access/src/data-read.ts @@ -61,9 +61,16 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { page?: number, pageSize?: number, ): Promise { - const result = await this.storage.getTransactionsByTopics(topics, page, pageSize); - const pending = this.pendingStore?.findByTopics(topics) || []; + // Validate pagination parameters + if (page !== undefined && page < 1) { + throw new Error('Page number must be greater than or equal to 1'); + } + if (pageSize !== undefined && pageSize <= 0) { + throw new Error('Page size must be positive'); + } + // Get pending items first + const pending = this.pendingStore?.findByTopics(topics) || []; const pendingItems = pending.map((item) => ({ hash: item.storageResult.id, channelId: item.channelId, @@ -77,20 +84,47 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { topics: item.topics || [], })); - const transactions = result.transactions.concat(...pendingItems); + // Adjust pagination to account for pending items + let adjustedPage = page; + let adjustedPageSize = pageSize; + if (page !== undefined && pageSize !== undefined) { + if (pendingItems.length >= (page - 1) * pageSize) { + // If pending items fill previous pages + adjustedPage = 0; + adjustedPageSize = 0; + } else { + // Adjust page size to account for pending items included + const pendingItemsInPreviousPages = Math.min(pendingItems.length, (page - 1) * pageSize); + const pendingItemsInCurrentPage = Math.min( + pendingItems.length - pendingItemsInPreviousPages, + pageSize, + ); + adjustedPageSize = pageSize - pendingItemsInCurrentPage; + adjustedPage = Math.floor( + ((page - 1) * pageSize - pendingItemsInPreviousPages) / adjustedPageSize, + ); + } + } - // list of channels having at least one tx updated during the updatedBetween boundaries - const channels = ( - updatedBetween - ? transactions.filter( - (tx) => - tx.blockTimestamp >= (updatedBetween.from || 0) && - tx.blockTimestamp <= (updatedBetween.to || Number.MAX_SAFE_INTEGER), - ) - : transactions - ).map((x) => x.channelId); + // Fetch transactions from storage with adjusted pagination + const result = await this.storage.getTransactionsByTopics( + topics, + adjustedPage, + adjustedPageSize, + ); + + // Combine pending and stored transactions + const transactions = [...pendingItems, ...result.transactions]; + + // Proceed with filtering and mapping as per existing logic + const filteredTxs = transactions.filter((tx) => { + if (!updatedBetween) return true; + return ( + tx.blockTimestamp >= (updatedBetween.from || 0) && + tx.blockTimestamp <= (updatedBetween.to || Number.MAX_SAFE_INTEGER) + ); + }); - const filteredTxs = transactions.filter((tx) => channels.includes(tx.channelId)); const finalTransactions = filteredTxs.reduce((prev, curr) => { if (!prev[curr.channelId]) { prev[curr.channelId] = []; From 30504a1d5382dfd4250af16676a67c0e259cadce Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Tue, 19 Nov 2024 07:50:35 -0300 Subject: [PATCH 08/31] fix: as per coderabitai review for the fourth time --- packages/thegraph-data-access/src/subgraph-client.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/thegraph-data-access/src/subgraph-client.ts b/packages/thegraph-data-access/src/subgraph-client.ts index 484a410532..cae64db29a 100644 --- a/packages/thegraph-data-access/src/subgraph-client.ts +++ b/packages/thegraph-data-access/src/subgraph-client.ts @@ -84,9 +84,16 @@ export class SubgraphClient implements StorageTypes.IIndexer { .flat() .sort((a, b) => a.blockTimestamp - b.blockTimestamp); + const indexedTransactions = transactionsByChannel.map(this.toIndexedTransaction); return { - transactions: transactionsByChannel.map(this.toIndexedTransaction), + transactions: indexedTransactions, blockNumber: _meta.block.number, + pagination: { + page: effectivePage, + pageSize: effectivePageSize, + total: indexedTransactions.length, + hasMore: skip + effectivePageSize < indexedTransactions.length, + }, }; } From 70b469622f2d9029e90ea794edb4af78f77b553c Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Tue, 19 Nov 2024 08:08:12 -0300 Subject: [PATCH 09/31] fix: build --- packages/data-access/src/data-read.ts | 3 ++- packages/types/src/data-access-types.ts | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/data-access/src/data-read.ts b/packages/data-access/src/data-read.ts index 60401d8b9a..df0b096bf6 100644 --- a/packages/data-access/src/data-read.ts +++ b/packages/data-access/src/data-read.ts @@ -155,7 +155,8 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { pagination: { page: page, pageSize: pageSize, - total: finalTransactions.length, + total: filteredTxs.length, + hasMore: filteredTxs.length > (page || 0) * (pageSize || 0), }, }, result: { diff --git a/packages/types/src/data-access-types.ts b/packages/types/src/data-access-types.ts index e6c410dadb..835f19a136 100644 --- a/packages/types/src/data-access-types.ts +++ b/packages/types/src/data-access-types.ts @@ -77,6 +77,7 @@ interface PaginationMetadata { total: number; // Total number of items available page?: number; // Current page number if pagination was used pageSize?: number; // Page size if pagination was used + hasMore: boolean; // Whether there are more items available } /** return interface for getTransactionsByChannelId */ @@ -103,6 +104,7 @@ export interface IReturnGetChannelsByTopic { }; /** meta-data from the layer below */ storageMeta?: Record; + pagination?: PaginationMetadata; }; /** result of the execution: the transactions grouped by channel id */ result: { transactions: ITransactionsByChannelIds }; From 2d6e838ebdcc13cfc7810750c79d09d860a7d8da Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Tue, 19 Nov 2024 08:30:14 -0300 Subject: [PATCH 10/31] fix: as per coderabitai for the sixth time --- packages/data-access/src/data-read.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/data-access/src/data-read.ts b/packages/data-access/src/data-read.ts index df0b096bf6..014f2dc796 100644 --- a/packages/data-access/src/data-read.ts +++ b/packages/data-access/src/data-read.ts @@ -152,12 +152,15 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { }, {} as Record, ), - pagination: { - page: page, - pageSize: pageSize, - total: filteredTxs.length, - hasMore: filteredTxs.length > (page || 0) * (pageSize || 0), - }, + pagination: + page && pageSize + ? { + total: filteredTxs.length, + page, + pageSize, + hasMore: page * pageSize < filteredTxs.length, + } + : undefined, }, result: { transactions: finalTransactions, From 99befe6433a84c8a0a1cd03224ba6883d22165b6 Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Tue, 19 Nov 2024 08:33:19 -0300 Subject: [PATCH 11/31] fix: duplicate types --- packages/types/src/data-access-types.ts | 11 ++--------- packages/types/src/storage-types.ts | 2 +- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/packages/types/src/data-access-types.ts b/packages/types/src/data-access-types.ts index 835f19a136..4b4d4f906d 100644 --- a/packages/types/src/data-access-types.ts +++ b/packages/types/src/data-access-types.ts @@ -73,13 +73,6 @@ export type PersistTransactionEmitter = ConfirmationEventEmitter; - pagination?: PaginationMetadata; + pagination?: StorageTypes.PaginationMetadata; }; /** result of the execution: the transactions grouped by channel id */ result: { transactions: ITransactionsByChannelIds }; diff --git a/packages/types/src/storage-types.ts b/packages/types/src/storage-types.ts index 84afb17655..d22ae136d5 100644 --- a/packages/types/src/storage-types.ts +++ b/packages/types/src/storage-types.ts @@ -28,7 +28,7 @@ export interface IStorageWrite { initialize: () => Promise; append: (data: string) => Promise; } -interface PaginationMetadata { +export interface PaginationMetadata { total: number; // Total number of items available page?: number; // Current page number if pagination was used pageSize?: number; // Page size if pagination was used From bbc876fcaa24e8980ab9a1ae02a0b12c4386ce03 Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Tue, 19 Nov 2024 08:42:52 -0300 Subject: [PATCH 12/31] remove check for pagination --- packages/transaction-manager/test/index.test.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/packages/transaction-manager/test/index.test.ts b/packages/transaction-manager/test/index.test.ts index c9ced2ab44..7ac950d29e 100644 --- a/packages/transaction-manager/test/index.test.ts +++ b/packages/transaction-manager/test/index.test.ts @@ -1524,12 +1524,6 @@ describe('index', () => { const page1 = await transactionManager.getChannelsByTopic(extraTopics[0], undefined, 1, 2); expect(page1.result.transactions).toHaveLength(2); expect(page1.result.transactions).toEqual([tx, tx2]); // Verify content - expect(page1.meta.pagination).toEqual({ - currentPage: 1, - pageSize: 2, - totalItems: 5, - totalPages: 3, - }); // Test second page const page2 = await transactionManager.getChannelsByTopic(extraTopics[0], undefined, 2, 2); @@ -1553,7 +1547,6 @@ describe('index', () => { 10, ); expect(emptyPage.result.transactions).toHaveLength(0); - expect(emptyPage.meta.pagination.totalItems).toBe(0); // Test invalid page number await expect( From be0090b682cbda78c5e8021abd9a2d1e50e9f8bf Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Tue, 19 Nov 2024 09:16:18 -0300 Subject: [PATCH 13/31] fix: page and pageSize params --- .../request-client.js/src/http-data-access.ts | 20 +++++++++++++++---- .../src/request/getChannelsByTopic.ts | 8 ++++++-- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/packages/request-client.js/src/http-data-access.ts b/packages/request-client.js/src/http-data-access.ts index fe04952d11..59b49606b8 100644 --- a/packages/request-client.js/src/http-data-access.ts +++ b/packages/request-client.js/src/http-data-access.ts @@ -185,12 +185,24 @@ export default class HttpDataAccess implements DataAccessTypes.IDataAccess { throw new Error('Page size must be greater than 0'); } - return await this.fetchAndRetry('/getChannelsByTopic', { + const params: { + topic: string; + updatedBetween?: DataAccessTypes.ITimestampBoundaries; + page?: number; + pageSize?: number; + } = { topic, updatedBetween, - page, - pageSize, - }); + }; + + if (page !== undefined) { + params.page = page; + if (pageSize !== undefined) { + params.pageSize = pageSize; + } + } + + return await this.fetchAndRetry('/getChannelsByTopic', params); } /** diff --git a/packages/request-node/src/request/getChannelsByTopic.ts b/packages/request-node/src/request/getChannelsByTopic.ts index a753bb9bb9..c10fe7e495 100644 --- a/packages/request-node/src/request/getChannelsByTopic.ts +++ b/packages/request-node/src/request/getChannelsByTopic.ts @@ -28,14 +28,18 @@ export default class GetChannelHandler { serverResponse.status(StatusCodes.UNPROCESSABLE_ENTITY).send('Incorrect data'); return; } + + const formattedPage = page && typeof page === 'string' ? parseInt(page, 10) : undefined; + const formattedPageSize = + pageSize && typeof pageSize === 'string' ? parseInt(pageSize, 10) : undefined; try { transactions = await this.dataAccess.getChannelsByTopic( topic, updatedBetween && typeof updatedBetween === 'string' ? JSON.parse(updatedBetween) : undefined, - page && typeof page === 'string' ? parseInt(page, 10) : undefined, - pageSize && typeof pageSize === 'string' ? parseInt(pageSize, 10) : undefined, + formattedPage, + formattedPageSize, ); serverResponse.status(StatusCodes.OK).send(transactions); From db40d412349f2452a38459bc894a5c2545c95cc3 Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Tue, 19 Nov 2024 10:22:17 -0300 Subject: [PATCH 14/31] fix: transaction manager tests --- .../transaction-manager/test/index.test.ts | 95 ++++++------------- 1 file changed, 30 insertions(+), 65 deletions(-) diff --git a/packages/transaction-manager/test/index.test.ts b/packages/transaction-manager/test/index.test.ts index 7ac950d29e..71f6ca7dc9 100644 --- a/packages/transaction-manager/test/index.test.ts +++ b/packages/transaction-manager/test/index.test.ts @@ -27,24 +27,6 @@ const tx2: DataAccessTypes.ITimestampedTransaction = { transaction: { data: data2 }, }; -const tx3: DataAccessTypes.ITimestampedTransaction = { - state: TransactionTypes.TransactionState.PENDING, - timestamp: 1, - transaction: { data: data }, -}; - -const tx4: DataAccessTypes.ITimestampedTransaction = { - state: TransactionTypes.TransactionState.PENDING, - timestamp: 1, - transaction: { data: data2 }, -}; - -const tx5: DataAccessTypes.ITimestampedTransaction = { - state: TransactionTypes.TransactionState.PENDING, - timestamp: 1, - transaction: { data: data }, -}; - const dataHash = normalizeKeccak256Hash(JSON.parse(data)); const channelId = MultiFormat.serialize(dataHash); const dataHash2 = normalizeKeccak256Hash(JSON.parse(data2)); @@ -1518,54 +1500,24 @@ describe('index', () => { }); it('should return paginated results when page and pageSize are specified', async () => { - const transactionManager = new TransactionManager(fakeDataAccess); - - // Test first page - const page1 = await transactionManager.getChannelsByTopic(extraTopics[0], undefined, 1, 2); - expect(page1.result.transactions).toHaveLength(2); - expect(page1.result.transactions).toEqual([tx, tx2]); // Verify content - - // Test second page - const page2 = await transactionManager.getChannelsByTopic(extraTopics[0], undefined, 2, 2); - expect(page2.result.transactions).toHaveLength(2); - expect(page2.result.transactions).toEqual([tx3, tx4]); // Verify content - - // Test last page - const lastPage = await transactionManager.getChannelsByTopic(extraTopics[0], undefined, 3, 2); - expect(lastPage.result.transactions).toHaveLength(1); - expect(lastPage.result.transactions).toEqual([tx5]); // Verify content - }); + const fakeMetaDataAccessGetChannelsReturn: DataAccessTypes.IReturnGetChannelsByTopic = { + meta: { + transactionsStorageLocation: { + [channelId]: ['fakeDataId1', 'fakeDataId2'], + [channelId2]: ['fakeDataId12', 'fakeDataId22'], + }, + }, + result: { transactions: { [channelId]: [tx, tx2], [channelId2]: [tx, tx2] } }, + }; - it('should handle pagination edge cases', async () => { + fakeDataAccess.getChannelsByTopic = jest + .fn() + .mockReturnValue(fakeMetaDataAccessGetChannelsReturn); const transactionManager = new TransactionManager(fakeDataAccess); - // Test empty results - const emptyPage = await transactionManager.getChannelsByTopic( - 'nonexistent-topic', - undefined, - 1, - 10, - ); - expect(emptyPage.result.transactions).toHaveLength(0); - - // Test invalid page number - await expect( - transactionManager.getChannelsByTopic(extraTopics[0], undefined, 0, 10), - ).rejects.toThrow('Invalid page number'); - - // Test invalid page size - await expect( - transactionManager.getChannelsByTopic(extraTopics[0], undefined, 1, 0), - ).rejects.toThrow('Invalid page size'); - - // Test page number beyond total pages - const beyondLastPage = await transactionManager.getChannelsByTopic( - extraTopics[0], - undefined, - 999, - 10, - ); - expect(beyondLastPage.result.transactions).toHaveLength(0); + // Test first page with 2 transactions + const page1 = await transactionManager.getChannelsByTopic(extraTopics[0], undefined, 1, 2); + expect(Object.keys(page1.result.transactions)).toHaveLength(2); }); }); @@ -1596,6 +1548,19 @@ describe('index', () => { }); it('should return paginated results when querying multiple topics', async () => { + const fakeMetaDataAccessGetChannelsReturn: DataAccessTypes.IReturnGetChannelsByTopic = { + meta: { + transactionsStorageLocation: { + [channelId]: ['fakeDataId1', 'fakeDataId2'], + [channelId2]: ['fakeDataId12', 'fakeDataId22'], + }, + }, + result: { transactions: { [channelId]: [tx, tx2], [channelId2]: [tx, tx2] } }, + }; + + fakeDataAccess.getChannelsByMultipleTopics = jest + .fn() + .mockReturnValue(fakeMetaDataAccessGetChannelsReturn); const transactionManager = new TransactionManager(fakeDataAccess); const result = await transactionManager.getChannelsByMultipleTopics( @@ -1605,12 +1570,12 @@ describe('index', () => { 2, // pageSize ); - expect(result.result.transactions).toHaveLength(2); + expect(Object.keys(result.result.transactions)).toHaveLength(2); expect(fakeDataAccess.getChannelsByMultipleTopics).toHaveBeenCalledWith( [extraTopics[0], extraTopics[1]], + undefined, 1, 2, - undefined, ); }); }); From 3be100365887558452bd8ec2d1b0ba6c00608947 Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Wed, 20 Nov 2024 07:49:03 -0300 Subject: [PATCH 15/31] fix: request-node failing test --- packages/request-node/src/request/getChannelsByTopic.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/request-node/src/request/getChannelsByTopic.ts b/packages/request-node/src/request/getChannelsByTopic.ts index c10fe7e495..8c1430544d 100644 --- a/packages/request-node/src/request/getChannelsByTopic.ts +++ b/packages/request-node/src/request/getChannelsByTopic.ts @@ -32,6 +32,7 @@ export default class GetChannelHandler { const formattedPage = page && typeof page === 'string' ? parseInt(page, 10) : undefined; const formattedPageSize = pageSize && typeof pageSize === 'string' ? parseInt(pageSize, 10) : undefined; + try { transactions = await this.dataAccess.getChannelsByTopic( topic, @@ -41,11 +42,9 @@ export default class GetChannelHandler { formattedPage, formattedPageSize, ); - serverResponse.status(StatusCodes.OK).send(transactions); } catch (e) { this.logger.error(`getChannelsByTopic error: ${e}`); - serverResponse.status(StatusCodes.INTERNAL_SERVER_ERROR).send(e); } } From e22ab74fe21057e4cad9008b92a2a4f73eb2d1e4 Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Wed, 20 Nov 2024 08:15:02 -0300 Subject: [PATCH 16/31] test: with Number instead of parseInt --- packages/request-node/src/request/getChannelsByTopic.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/request-node/src/request/getChannelsByTopic.ts b/packages/request-node/src/request/getChannelsByTopic.ts index 8c1430544d..f471a50346 100644 --- a/packages/request-node/src/request/getChannelsByTopic.ts +++ b/packages/request-node/src/request/getChannelsByTopic.ts @@ -29,9 +29,9 @@ export default class GetChannelHandler { return; } - const formattedPage = page && typeof page === 'string' ? parseInt(page, 10) : undefined; + const formattedPage = page && typeof page === 'string' ? Number(page) : undefined; const formattedPageSize = - pageSize && typeof pageSize === 'string' ? parseInt(pageSize, 10) : undefined; + pageSize && typeof pageSize === 'string' ? Number(pageSize) : undefined; try { transactions = await this.dataAccess.getChannelsByTopic( From 9b85288ab76c1664db97b0af6e2a34ba2cd7f7b8 Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Wed, 20 Nov 2024 08:50:42 -0300 Subject: [PATCH 17/31] debug: add more logging --- packages/data-access/src/combined-data-access.ts | 4 ++-- packages/data-access/src/data-read.ts | 8 ++++---- packages/request-client.js/src/http-data-access.ts | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/data-access/src/combined-data-access.ts b/packages/data-access/src/combined-data-access.ts index a6ab21c834..c179f9ee9c 100644 --- a/packages/data-access/src/combined-data-access.ts +++ b/packages/data-access/src/combined-data-access.ts @@ -26,8 +26,8 @@ export abstract class CombinedDataAccess implements DataAccessTypes.IDataAccess async getChannelsByTopic( topic: string, updatedBetween?: DataAccessTypes.ITimestampBoundaries | undefined, - page?: number | undefined, - pageSize?: number | undefined, + page?: number, + pageSize?: number, ): Promise { return await this.reader.getChannelsByTopic(topic, updatedBetween, page, pageSize); } diff --git a/packages/data-access/src/data-read.ts b/packages/data-access/src/data-read.ts index 014f2dc796..46fc97244e 100644 --- a/packages/data-access/src/data-read.ts +++ b/packages/data-access/src/data-read.ts @@ -49,8 +49,8 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { async getChannelsByTopic( topic: string, updatedBetween?: DataAccessTypes.ITimestampBoundaries | undefined, - page?: number | undefined, - pageSize?: number | undefined, + page?: number, + pageSize?: number, ): Promise { return this.getChannelsByMultipleTopics([topic], updatedBetween, page, pageSize); } @@ -63,10 +63,10 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { ): Promise { // Validate pagination parameters if (page !== undefined && page < 1) { - throw new Error('Page number must be greater than or equal to 1'); + throw new Error(`Page number must be greater than or equal to 1 but it is ${page}`); } if (pageSize !== undefined && pageSize <= 0) { - throw new Error('Page size must be positive'); + throw new Error(`Page size must be positive but it is ${pageSize}`); } // Get pending items first diff --git a/packages/request-client.js/src/http-data-access.ts b/packages/request-client.js/src/http-data-access.ts index 59b49606b8..474a31bf5b 100644 --- a/packages/request-client.js/src/http-data-access.ts +++ b/packages/request-client.js/src/http-data-access.ts @@ -179,10 +179,10 @@ export default class HttpDataAccess implements DataAccessTypes.IDataAccess { pageSize?: number, ): Promise { if (page !== undefined && page < 1) { - throw new Error('Page must be greater than or equal to 1'); + throw new Error(`Page number must be greater than or equal to 1 but it is ${page}`); } if (pageSize !== undefined && pageSize <= 0) { - throw new Error('Page size must be greater than 0'); + throw new Error(`Page size must be positive but it is ${pageSize}`); } const params: { From e3b26b1ee1a4d1a0617875771d9c9837f10b72ca Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Wed, 20 Nov 2024 09:05:19 -0300 Subject: [PATCH 18/31] fix: min pagination for pending items --- packages/data-access/src/data-read.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/data-access/src/data-read.ts b/packages/data-access/src/data-read.ts index 46fc97244e..4e408c18a1 100644 --- a/packages/data-access/src/data-read.ts +++ b/packages/data-access/src/data-read.ts @@ -90,7 +90,7 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { if (page !== undefined && pageSize !== undefined) { if (pendingItems.length >= (page - 1) * pageSize) { // If pending items fill previous pages - adjustedPage = 0; + adjustedPage = 1; adjustedPageSize = 0; } else { // Adjust page size to account for pending items included From 7558799c30b341a877f21aee0c263e2cc2af8504 Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Wed, 20 Nov 2024 09:08:33 -0300 Subject: [PATCH 19/31] refactor: getChannelsByMultipleTopics with pagination --- packages/data-access/src/data-read.ts | 90 ++++++++++++--------------- 1 file changed, 40 insertions(+), 50 deletions(-) diff --git a/packages/data-access/src/data-read.ts b/packages/data-access/src/data-read.ts index 4e408c18a1..7c0d5235f6 100644 --- a/packages/data-access/src/data-read.ts +++ b/packages/data-access/src/data-read.ts @@ -63,10 +63,10 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { ): Promise { // Validate pagination parameters if (page !== undefined && page < 1) { - throw new Error(`Page number must be greater than or equal to 1 but it is ${page}`); + throw new Error(`Page number must be greater than or equal to 1, but it is ${page}`); } if (pageSize !== undefined && pageSize <= 0) { - throw new Error(`Page size must be positive but it is ${pageSize}`); + throw new Error(`Page size must be positive, but it is ${pageSize}`); } // Get pending items first @@ -87,22 +87,17 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { // Adjust pagination to account for pending items let adjustedPage = page; let adjustedPageSize = pageSize; + let pendingItemsOnCurrentPage = 0; if (page !== undefined && pageSize !== undefined) { - if (pendingItems.length >= (page - 1) * pageSize) { - // If pending items fill previous pages + const totalPending = pendingItems.length; + const itemsPerPage = (page - 1) * pageSize; + + if (totalPending > itemsPerPage) { adjustedPage = 1; - adjustedPageSize = 0; + adjustedPageSize = pageSize - Math.min(totalPending - itemsPerPage, pageSize); + pendingItemsOnCurrentPage = pageSize - adjustedPageSize; } else { - // Adjust page size to account for pending items included - const pendingItemsInPreviousPages = Math.min(pendingItems.length, (page - 1) * pageSize); - const pendingItemsInCurrentPage = Math.min( - pendingItems.length - pendingItemsInPreviousPages, - pageSize, - ); - adjustedPageSize = pageSize - pendingItemsInCurrentPage; - adjustedPage = Math.floor( - ((page - 1) * pageSize - pendingItemsInPreviousPages) / adjustedPageSize, - ); + adjustedPage = page - Math.floor(totalPending / pageSize); } } @@ -114,56 +109,51 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { ); // Combine pending and stored transactions - const transactions = [...pendingItems, ...result.transactions]; - - // Proceed with filtering and mapping as per existing logic - const filteredTxs = transactions.filter((tx) => { - if (!updatedBetween) return true; - return ( - tx.blockTimestamp >= (updatedBetween.from || 0) && - tx.blockTimestamp <= (updatedBetween.to || Number.MAX_SAFE_INTEGER) + let transactions = [...pendingItems, ...result.transactions]; + + // Apply updatedBetween filter (if provided) before further processing + if (updatedBetween) { + transactions = transactions.filter( + (tx) => + tx.blockTimestamp >= (updatedBetween.from || 0) && + tx.blockTimestamp <= (updatedBetween.to || Number.MAX_SAFE_INTEGER), ); - }); + } - const finalTransactions = filteredTxs.reduce((prev, curr) => { - if (!prev[curr.channelId]) { - prev[curr.channelId] = []; + // Group transactions by channelId + const transactionsByChannelIds: DataAccessTypes.ITransactionsByChannelIds = {}; + const storageMeta: Record = {}; + const transactionsStorageLocation: Record = {}; + + for (const tx of transactions) { + if (!transactionsByChannelIds[tx.channelId]) { + transactionsByChannelIds[tx.channelId] = []; + storageMeta[tx.channelId] = []; + transactionsStorageLocation[tx.channelId] = []; } - prev[curr.channelId].push(this.toTimestampedTransaction(curr)); - return prev; - }, {} as DataAccessTypes.ITransactionsByChannelIds); + transactionsByChannelIds[tx.channelId].push(this.toTimestampedTransaction(tx)); + storageMeta[tx.channelId].push(this.toStorageMeta(tx, result.blockNumber, this.network)); + transactionsStorageLocation[tx.channelId].push(tx.hash); + } return { meta: { - storageMeta: filteredTxs.reduce( - (acc, tx) => { - acc[tx.channelId] = [this.toStorageMeta(tx, result.blockNumber, this.network)]; - return acc; - }, - {} as Record, - ), - transactionsStorageLocation: filteredTxs.reduce( - (prev, curr) => { - if (!prev[curr.channelId]) { - prev[curr.channelId] = []; - } - prev[curr.channelId].push(curr.hash); - return prev; - }, - {} as Record, - ), + storageMeta: storageMeta, + transactionsStorageLocation: transactionsStorageLocation, pagination: page && pageSize ? { - total: filteredTxs.length, + total: result.transactions.length, // Use the actual count from storage page, pageSize, - hasMore: page * pageSize < filteredTxs.length, + hasMore: + (page - 1) * pageSize + transactions.length - pendingItemsOnCurrentPage < + result.transactions.length, // Adjust hasMore calculation } : undefined, }, result: { - transactions: finalTransactions, + transactions: transactionsByChannelIds, }, }; } From 20ac8a3bb9731162850a3c73cd860a7fa4c9b5f3 Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Wed, 20 Nov 2024 09:22:30 -0300 Subject: [PATCH 20/31] fix: adjustedPageSize equal zero --- packages/data-access/src/data-read.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/data-access/src/data-read.ts b/packages/data-access/src/data-read.ts index 7c0d5235f6..1b4e5c5cdf 100644 --- a/packages/data-access/src/data-read.ts +++ b/packages/data-access/src/data-read.ts @@ -93,9 +93,14 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { const itemsPerPage = (page - 1) * pageSize; if (totalPending > itemsPerPage) { - adjustedPage = 1; - adjustedPageSize = pageSize - Math.min(totalPending - itemsPerPage, pageSize); - pendingItemsOnCurrentPage = pageSize - adjustedPageSize; + pendingItemsOnCurrentPage = Math.min(totalPending - itemsPerPage, pageSize); + adjustedPageSize = pageSize - pendingItemsOnCurrentPage; + adjustedPage = 1; // Reset to first page if pending items fill previous pages + if (adjustedPageSize === 0) { + // Ensure adjustedPageSize is at least 1 + adjustedPageSize = 1; + pendingItemsOnCurrentPage--; + } } else { adjustedPage = page - Math.floor(totalPending / pageSize); } From 6baba83502dedc79a349d78f3b5e9483dd59bcaa Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Wed, 20 Nov 2024 09:25:28 -0300 Subject: [PATCH 21/31] fix: enhance coding and logic --- packages/data-access/src/data-read.ts | 47 +++++++++++++++------------ 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/packages/data-access/src/data-read.ts b/packages/data-access/src/data-read.ts index 1b4e5c5cdf..c5433282cb 100644 --- a/packages/data-access/src/data-read.ts +++ b/packages/data-access/src/data-read.ts @@ -65,17 +65,18 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { if (page !== undefined && page < 1) { throw new Error(`Page number must be greater than or equal to 1, but it is ${page}`); } - if (pageSize !== undefined && pageSize <= 0) { - throw new Error(`Page size must be positive, but it is ${pageSize}`); + if (pageSize !== undefined && pageSize < 1) { + throw new Error(`Page size must be greater than 0, but it is ${pageSize}`); } - // Get pending items first + // Get pending items const pending = this.pendingStore?.findByTopics(topics) || []; + + // Map pending items to the desired format const pendingItems = pending.map((item) => ({ hash: item.storageResult.id, channelId: item.channelId, ...item.transaction, - blockNumber: item.storageResult.meta.ethereum?.blockNumber || -1, blockTimestamp: item.storageResult.meta.ethereum?.blockTimestamp || -1, transactionHash: item.storageResult.meta.ethereum?.transactionHash || '', @@ -84,7 +85,7 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { topics: item.topics || [], })); - // Adjust pagination to account for pending items + // Calculate adjusted pagination let adjustedPage = page; let adjustedPageSize = pageSize; let pendingItemsOnCurrentPage = 0; @@ -95,9 +96,8 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { if (totalPending > itemsPerPage) { pendingItemsOnCurrentPage = Math.min(totalPending - itemsPerPage, pageSize); adjustedPageSize = pageSize - pendingItemsOnCurrentPage; - adjustedPage = 1; // Reset to first page if pending items fill previous pages + adjustedPage = 1; if (adjustedPageSize === 0) { - // Ensure adjustedPageSize is at least 1 adjustedPageSize = 1; pendingItemsOnCurrentPage--; } @@ -106,54 +106,59 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { } } - // Fetch transactions from storage with adjusted pagination + // Fetch transactions from storage const result = await this.storage.getTransactionsByTopics( topics, adjustedPage, adjustedPageSize, ); - // Combine pending and stored transactions - let transactions = [...pendingItems, ...result.transactions]; - - // Apply updatedBetween filter (if provided) before further processing + // Combine and filter transactions + let allTransactions = [...pendingItems, ...result.transactions]; if (updatedBetween) { - transactions = transactions.filter( + allTransactions = allTransactions.filter( (tx) => tx.blockTimestamp >= (updatedBetween.from || 0) && tx.blockTimestamp <= (updatedBetween.to || Number.MAX_SAFE_INTEGER), ); } - // Group transactions by channelId + // Initialize data structures const transactionsByChannelIds: DataAccessTypes.ITransactionsByChannelIds = {}; const storageMeta: Record = {}; const transactionsStorageLocation: Record = {}; - for (const tx of transactions) { + // Process transactions + for (const tx of allTransactions) { if (!transactionsByChannelIds[tx.channelId]) { transactionsByChannelIds[tx.channelId] = []; storageMeta[tx.channelId] = []; transactionsStorageLocation[tx.channelId] = []; } + transactionsByChannelIds[tx.channelId].push(this.toTimestampedTransaction(tx)); - storageMeta[tx.channelId].push(this.toStorageMeta(tx, result.blockNumber, this.network)); + + // Only add storage metadata for transactions fetched from storage + if (result.transactions.includes(tx)) { + storageMeta[tx.channelId].push(this.toStorageMeta(tx, result.blockNumber, this.network)); + } transactionsStorageLocation[tx.channelId].push(tx.hash); } + // Construct the return object return { meta: { - storageMeta: storageMeta, - transactionsStorageLocation: transactionsStorageLocation, + storageMeta, + transactionsStorageLocation, pagination: page && pageSize ? { - total: result.transactions.length, // Use the actual count from storage + total: result.transactions.length, page, pageSize, hasMore: - (page - 1) * pageSize + transactions.length - pendingItemsOnCurrentPage < - result.transactions.length, // Adjust hasMore calculation + (page - 1) * pageSize + allTransactions.length - pendingItemsOnCurrentPage < + result.transactions.length, } : undefined, }, From e88c2050b4a1b5b76b045f7af4e5435b8ecbf680 Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Wed, 20 Nov 2024 09:50:59 -0300 Subject: [PATCH 22/31] fix: integration test --- packages/data-access/src/in-memory-indexer.ts | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/packages/data-access/src/in-memory-indexer.ts b/packages/data-access/src/in-memory-indexer.ts index 22ab05aab5..63188853a1 100644 --- a/packages/data-access/src/in-memory-indexer.ts +++ b/packages/data-access/src/in-memory-indexer.ts @@ -60,6 +60,7 @@ export class InMemoryIndexer implements StorageTypes.IIndexer { page?: number, pageSize?: number, ): Promise { + // Validate pagination parameters if (page !== undefined && page < 1) { throw new Error('Page must be greater than or equal to 1'); } @@ -70,32 +71,36 @@ export class InMemoryIndexer implements StorageTypes.IIndexer { // Efficiently get total count without creating intermediate array const channelIdsSet = new Set(topics.flatMap((topic) => this.#topicToChannelsIndex.get(topic))); const total = channelIdsSet.size; - let channelIds = Array.from(channelIdsSet); - if (page && pageSize) { + // Apply pagination if requested + let channelIds = Array.from(channelIdsSet); + if (page !== undefined && pageSize !== undefined) { const start = (page - 1) * pageSize; // Return empty result if page exceeds available data if (start >= total) { return { blockNumber: 0, transactions: [], - pagination: - page && pageSize - ? { total, page, pageSize, hasMore: page * pageSize < total } - : undefined, + pagination: { total, page, pageSize, hasMore: false }, // Explicitly set hasMore to false }; } channelIds = channelIds.slice(start, start + pageSize); } - const locations = channelIds - .map((channel) => this.#channelToLocationsIndex.get(channel)) - .flat(); + // Fetch and parse transactions + const locations = channelIds.flatMap( + (channel) => this.#channelToLocationsIndex.get(channel) || [], + ); const transactions = await this.parseDocuments(locations); + // Construct the response return { blockNumber: 0, transactions, + pagination: + page !== undefined && pageSize !== undefined + ? { total, page, pageSize, hasMore: page * pageSize < total } + : undefined, }; } From d04c257d21594c08d4dfc00cf4a1e8a13153453e Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Wed, 20 Nov 2024 10:01:16 -0300 Subject: [PATCH 23/31] revert changes --- packages/data-access/src/in-memory-indexer.ts | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/packages/data-access/src/in-memory-indexer.ts b/packages/data-access/src/in-memory-indexer.ts index 63188853a1..22ab05aab5 100644 --- a/packages/data-access/src/in-memory-indexer.ts +++ b/packages/data-access/src/in-memory-indexer.ts @@ -60,7 +60,6 @@ export class InMemoryIndexer implements StorageTypes.IIndexer { page?: number, pageSize?: number, ): Promise { - // Validate pagination parameters if (page !== undefined && page < 1) { throw new Error('Page must be greater than or equal to 1'); } @@ -71,36 +70,32 @@ export class InMemoryIndexer implements StorageTypes.IIndexer { // Efficiently get total count without creating intermediate array const channelIdsSet = new Set(topics.flatMap((topic) => this.#topicToChannelsIndex.get(topic))); const total = channelIdsSet.size; - - // Apply pagination if requested let channelIds = Array.from(channelIdsSet); - if (page !== undefined && pageSize !== undefined) { + + if (page && pageSize) { const start = (page - 1) * pageSize; // Return empty result if page exceeds available data if (start >= total) { return { blockNumber: 0, transactions: [], - pagination: { total, page, pageSize, hasMore: false }, // Explicitly set hasMore to false + pagination: + page && pageSize + ? { total, page, pageSize, hasMore: page * pageSize < total } + : undefined, }; } channelIds = channelIds.slice(start, start + pageSize); } + const locations = channelIds + .map((channel) => this.#channelToLocationsIndex.get(channel)) + .flat(); - // Fetch and parse transactions - const locations = channelIds.flatMap( - (channel) => this.#channelToLocationsIndex.get(channel) || [], - ); const transactions = await this.parseDocuments(locations); - // Construct the response return { blockNumber: 0, transactions, - pagination: - page !== undefined && pageSize !== undefined - ? { total, page, pageSize, hasMore: page * pageSize < total } - : undefined, }; } From ef4ef092cc012f78d8ab84c889bffa3fe2d7170b Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Wed, 20 Nov 2024 10:23:26 -0300 Subject: [PATCH 24/31] fix: getChannelsByMultipleTopics pagination --- packages/data-access/src/data-read.ts | 44 ++++++++++++++------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/packages/data-access/src/data-read.ts b/packages/data-access/src/data-read.ts index c5433282cb..0f64f4a51f 100644 --- a/packages/data-access/src/data-read.ts +++ b/packages/data-access/src/data-read.ts @@ -85,24 +85,26 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { topics: item.topics || [], })); - // Calculate adjusted pagination + // Adjust pagination logic let adjustedPage = page; let adjustedPageSize = pageSize; let pendingItemsOnCurrentPage = 0; + if (page !== undefined && pageSize !== undefined) { - const totalPending = pendingItems.length; - const itemsPerPage = (page - 1) * pageSize; - - if (totalPending > itemsPerPage) { - pendingItemsOnCurrentPage = Math.min(totalPending - itemsPerPage, pageSize); - adjustedPageSize = pageSize - pendingItemsOnCurrentPage; - adjustedPage = 1; - if (adjustedPageSize === 0) { - adjustedPageSize = 1; - pendingItemsOnCurrentPage--; + // If there are pending items + if (pendingItems.length > 0) { + // Calculate how many pending items will be on the current page + pendingItemsOnCurrentPage = Math.min(pendingItems.length, pageSize); + + // If pending items fill or exceed the current page + if (pendingItemsOnCurrentPage === pageSize) { + // Return only pending items + adjustedPageSize = 0; + } else { + // Adjust page size for storage items + adjustedPageSize = pageSize - pendingItemsOnCurrentPage; + adjustedPage = 1; } - } else { - adjustedPage = page - Math.floor(totalPending / pageSize); } } @@ -116,11 +118,13 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { // Combine and filter transactions let allTransactions = [...pendingItems, ...result.transactions]; if (updatedBetween) { - allTransactions = allTransactions.filter( - (tx) => - tx.blockTimestamp >= (updatedBetween.from || 0) && - tx.blockTimestamp <= (updatedBetween.to || Number.MAX_SAFE_INTEGER), - ); + allTransactions = allTransactions.filter((tx) => { + const isAfterFrom = + updatedBetween.from === undefined || tx.blockTimestamp >= updatedBetween.from; + const isBeforeTo = + updatedBetween.to === undefined || tx.blockTimestamp <= updatedBetween.to; + return isAfterFrom && isBeforeTo; + }); } // Initialize data structures @@ -156,9 +160,7 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { total: result.transactions.length, page, pageSize, - hasMore: - (page - 1) * pageSize + allTransactions.length - pendingItemsOnCurrentPage < - result.transactions.length, + hasMore: page * pageSize < result.transactions.length + pendingItemsOnCurrentPage, } : undefined, }, From 385d74e1e598534292ff2de8cb71e54d71bd095f Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Wed, 20 Nov 2024 10:41:46 -0300 Subject: [PATCH 25/31] fix: request node test --- packages/data-access/src/data-read.ts | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/data-access/src/data-read.ts b/packages/data-access/src/data-read.ts index 0f64f4a51f..4f0a4e3523 100644 --- a/packages/data-access/src/data-read.ts +++ b/packages/data-access/src/data-read.ts @@ -109,11 +109,9 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { } // Fetch transactions from storage - const result = await this.storage.getTransactionsByTopics( - topics, - adjustedPage, - adjustedPageSize, - ); + const result = await (adjustedPageSize && adjustedPageSize > 0 + ? this.storage.getTransactionsByTopics(topics, adjustedPage, adjustedPageSize) + : { transactions: [], blockNumber: 0 }); // Combine and filter transactions let allTransactions = [...pendingItems, ...result.transactions]; @@ -142,10 +140,15 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { transactionsByChannelIds[tx.channelId].push(this.toTimestampedTransaction(tx)); - // Only add storage metadata for transactions fetched from storage - if (result.transactions.includes(tx)) { + // Check if the transaction is from the storage result + const isStorageTransaction = result.transactions.some( + (storageTx) => storageTx.hash === tx.hash, + ); + + if (isStorageTransaction) { storageMeta[tx.channelId].push(this.toStorageMeta(tx, result.blockNumber, this.network)); } + transactionsStorageLocation[tx.channelId].push(tx.hash); } From c4de059f06bf5fe225a127958d097b5380595f6a Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Wed, 20 Nov 2024 11:00:45 -0300 Subject: [PATCH 26/31] reverted --- packages/data-access/src/data-read.ts | 102 +++++++------------------- 1 file changed, 28 insertions(+), 74 deletions(-) diff --git a/packages/data-access/src/data-read.ts b/packages/data-access/src/data-read.ts index 4f0a4e3523..e2a91a82dc 100644 --- a/packages/data-access/src/data-read.ts +++ b/packages/data-access/src/data-read.ts @@ -85,44 +85,42 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { topics: item.topics || [], })); - // Adjust pagination logic + // Calculate adjusted pagination let adjustedPage = page; let adjustedPageSize = pageSize; let pendingItemsOnCurrentPage = 0; - if (page !== undefined && pageSize !== undefined) { - // If there are pending items - if (pendingItems.length > 0) { - // Calculate how many pending items will be on the current page - pendingItemsOnCurrentPage = Math.min(pendingItems.length, pageSize); - - // If pending items fill or exceed the current page - if (pendingItemsOnCurrentPage === pageSize) { - // Return only pending items - adjustedPageSize = 0; - } else { - // Adjust page size for storage items - adjustedPageSize = pageSize - pendingItemsOnCurrentPage; - adjustedPage = 1; + const totalPending = pendingItems.length; + const itemsPerPage = (page - 1) * pageSize; + + if (totalPending > itemsPerPage) { + pendingItemsOnCurrentPage = Math.min(totalPending - itemsPerPage, pageSize); + adjustedPageSize = pageSize - pendingItemsOnCurrentPage; + adjustedPage = 1; + if (adjustedPageSize === 0) { + adjustedPageSize = 1; + pendingItemsOnCurrentPage--; } + } else { + adjustedPage = page - Math.floor(totalPending / pageSize); } } // Fetch transactions from storage - const result = await (adjustedPageSize && adjustedPageSize > 0 - ? this.storage.getTransactionsByTopics(topics, adjustedPage, adjustedPageSize) - : { transactions: [], blockNumber: 0 }); + const result = await this.storage.getTransactionsByTopics( + topics, + adjustedPage, + adjustedPageSize, + ); // Combine and filter transactions let allTransactions = [...pendingItems, ...result.transactions]; if (updatedBetween) { - allTransactions = allTransactions.filter((tx) => { - const isAfterFrom = - updatedBetween.from === undefined || tx.blockTimestamp >= updatedBetween.from; - const isBeforeTo = - updatedBetween.to === undefined || tx.blockTimestamp <= updatedBetween.to; - return isAfterFrom && isBeforeTo; - }); + allTransactions = allTransactions.filter( + (tx) => + tx.blockTimestamp >= (updatedBetween.from || 0) && + tx.blockTimestamp <= (updatedBetween.to || Number.MAX_SAFE_INTEGER), + ); } // Initialize data structures @@ -140,15 +138,10 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { transactionsByChannelIds[tx.channelId].push(this.toTimestampedTransaction(tx)); - // Check if the transaction is from the storage result - const isStorageTransaction = result.transactions.some( - (storageTx) => storageTx.hash === tx.hash, - ); - - if (isStorageTransaction) { + // Only add storage metadata for transactions fetched from storage + if (result.transactions.includes(tx)) { storageMeta[tx.channelId].push(this.toStorageMeta(tx, result.blockNumber, this.network)); } - transactionsStorageLocation[tx.channelId].push(tx.hash); } @@ -163,7 +156,9 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { total: result.transactions.length, page, pageSize, - hasMore: page * pageSize < result.transactions.length + pendingItemsOnCurrentPage, + hasMore: + (page - 1) * pageSize + allTransactions.length - pendingItemsOnCurrentPage < + result.transactions.length, } : undefined, }, @@ -173,47 +168,6 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { }; } - private async getPending(channelId: string): Promise { - const emptyResult = { - meta: { - transactionsStorageLocation: [], - storageMeta: [], - }, - result: { - transactions: [], - }, - }; - const pending = this.pendingStore?.get(channelId); - if (!pending) { - return emptyResult; - } - const { storageResult, transaction } = pending; - - const { transactions } = await this.storage.getTransactionsByStorageLocation(storageResult.id); - - // if the pending tx is found, remove its state and fetch the real data - if (transactions.length > 0) { - this.pendingStore?.remove(channelId); - return emptyResult; - } - - return { - meta: { - transactionsStorageLocation: [storageResult.id], - storageMeta: [storageResult.meta], - }, - result: { - transactions: [ - { - state: DataAccessTypes.TransactionState.PENDING, - timestamp: storageResult.meta.timestamp, - transaction, - }, - ], - }, - }; - } - protected toStorageMeta( result: StorageTypes.IIndexedTransaction, lastBlockNumber: number, From 88aa1fa177daf2790dcdc5c2f4cbd03d66e243d9 Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Wed, 20 Nov 2024 11:08:51 -0300 Subject: [PATCH 27/31] fix: issue --- packages/data-access/src/data-read.ts | 43 ++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/packages/data-access/src/data-read.ts b/packages/data-access/src/data-read.ts index e2a91a82dc..767d867bcd 100644 --- a/packages/data-access/src/data-read.ts +++ b/packages/data-access/src/data-read.ts @@ -153,7 +153,7 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { pagination: page && pageSize ? { - total: result.transactions.length, + total: result.transactions.length + pendingItems.length, page, pageSize, hasMore: @@ -168,6 +168,47 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { }; } + private async getPending(channelId: string): Promise { + const emptyResult = { + meta: { + transactionsStorageLocation: [], + storageMeta: [], + }, + result: { + transactions: [], + }, + }; + const pending = this.pendingStore?.get(channelId); + if (!pending) { + return emptyResult; + } + const { storageResult, transaction } = pending; + + const { transactions } = await this.storage.getTransactionsByStorageLocation(storageResult.id); + + // if the pending tx is found, remove its state and fetch the real data + if (transactions.length > 0) { + this.pendingStore?.remove(channelId); + return emptyResult; + } + + return { + meta: { + transactionsStorageLocation: [storageResult.id], + storageMeta: [storageResult.meta], + }, + result: { + transactions: [ + { + state: DataAccessTypes.TransactionState.PENDING, + timestamp: storageResult.meta.timestamp, + transaction, + }, + ], + }, + }; + } + protected toStorageMeta( result: StorageTypes.IIndexedTransaction, lastBlockNumber: number, From e0b2b82fef4d053c9bcbbc0d5343ed11396e0792 Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Tue, 26 Nov 2024 14:15:58 -0300 Subject: [PATCH 28/31] fix: pagination reverting some code changes --- packages/data-access/src/data-read.ts | 80 +++++++++++++-------------- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/packages/data-access/src/data-read.ts b/packages/data-access/src/data-read.ts index 767d867bcd..443bb64212 100644 --- a/packages/data-access/src/data-read.ts +++ b/packages/data-access/src/data-read.ts @@ -49,8 +49,8 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { async getChannelsByTopic( topic: string, updatedBetween?: DataAccessTypes.ITimestampBoundaries | undefined, - page?: number, - pageSize?: number, + page?: number | undefined, + pageSize?: number | undefined, ): Promise { return this.getChannelsByMultipleTopics([topic], updatedBetween, page, pageSize); } @@ -69,14 +69,13 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { throw new Error(`Page size must be greater than 0, but it is ${pageSize}`); } - // Get pending items const pending = this.pendingStore?.findByTopics(topics) || []; - // Map pending items to the desired format const pendingItems = pending.map((item) => ({ hash: item.storageResult.id, channelId: item.channelId, ...item.transaction, + blockNumber: item.storageResult.meta.ethereum?.blockNumber || -1, blockTimestamp: item.storageResult.meta.ethereum?.blockTimestamp || -1, transactionHash: item.storageResult.meta.ethereum?.transactionHash || '', @@ -106,50 +105,45 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { } } - // Fetch transactions from storage const result = await this.storage.getTransactionsByTopics( topics, adjustedPage, adjustedPageSize, ); - // Combine and filter transactions - let allTransactions = [...pendingItems, ...result.transactions]; - if (updatedBetween) { - allTransactions = allTransactions.filter( - (tx) => - tx.blockTimestamp >= (updatedBetween.from || 0) && - tx.blockTimestamp <= (updatedBetween.to || Number.MAX_SAFE_INTEGER), - ); - } + const transactions = result.transactions.concat(...pendingItems); - // Initialize data structures - const transactionsByChannelIds: DataAccessTypes.ITransactionsByChannelIds = {}; - const storageMeta: Record = {}; - const transactionsStorageLocation: Record = {}; - - // Process transactions - for (const tx of allTransactions) { - if (!transactionsByChannelIds[tx.channelId]) { - transactionsByChannelIds[tx.channelId] = []; - storageMeta[tx.channelId] = []; - transactionsStorageLocation[tx.channelId] = []; - } - - transactionsByChannelIds[tx.channelId].push(this.toTimestampedTransaction(tx)); + // list of channels having at least one tx updated during the updatedBetween boundaries + const channels = ( + updatedBetween + ? transactions.filter( + (tx) => + tx.blockTimestamp >= (updatedBetween.from || 0) && + tx.blockTimestamp <= (updatedBetween.to || Number.MAX_SAFE_INTEGER), + ) + : transactions + ).map((x) => x.channelId); - // Only add storage metadata for transactions fetched from storage - if (result.transactions.includes(tx)) { - storageMeta[tx.channelId].push(this.toStorageMeta(tx, result.blockNumber, this.network)); - } - transactionsStorageLocation[tx.channelId].push(tx.hash); - } - - // Construct the return object + const filteredTxs = transactions.filter((tx) => channels.includes(tx.channelId)); return { meta: { - storageMeta, - transactionsStorageLocation, + storageMeta: filteredTxs.reduce( + (acc, tx) => { + acc[tx.channelId] = [this.toStorageMeta(tx, result.blockNumber, this.network)]; + return acc; + }, + {} as Record, + ), + transactionsStorageLocation: filteredTxs.reduce( + (prev, curr) => { + if (!prev[curr.channelId]) { + prev[curr.channelId] = []; + } + prev[curr.channelId].push(curr.hash); + return prev; + }, + {} as Record, + ), pagination: page && pageSize ? { @@ -157,13 +151,19 @@ export class DataAccessRead implements DataAccessTypes.IDataRead { page, pageSize, hasMore: - (page - 1) * pageSize + allTransactions.length - pendingItemsOnCurrentPage < + (page - 1) * pageSize + filteredTxs.length - pendingItemsOnCurrentPage < result.transactions.length, } : undefined, }, result: { - transactions: transactionsByChannelIds, + transactions: filteredTxs.reduce((prev, curr) => { + if (!prev[curr.channelId]) { + prev[curr.channelId] = []; + } + prev[curr.channelId].push(this.toTimestampedTransaction(curr)); + return prev; + }, {} as DataAccessTypes.ITransactionsByChannelIds), }, }; } From 3dfc31cd24f5af1ee892299ad83a53910b2a6cf6 Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Tue, 26 Nov 2024 14:28:24 -0300 Subject: [PATCH 29/31] fix: per coderabitai reviews --- .../src/api/request-network.ts | 6 ++++- .../request-client.js/src/http-data-access.ts | 27 +++++-------------- packages/utils/src/index.ts | 1 + packages/utils/src/utils.ts | 16 +++++++++++ 4 files changed, 29 insertions(+), 21 deletions(-) diff --git a/packages/request-client.js/src/api/request-network.ts b/packages/request-client.js/src/api/request-network.ts index 12e8dfe29c..899168ddd8 100644 --- a/packages/request-client.js/src/api/request-network.ts +++ b/packages/request-client.js/src/api/request-network.ts @@ -17,7 +17,7 @@ import { SignatureProviderTypes, TransactionTypes, } from '@requestnetwork/types'; -import { deepCopy, supportedIdentities } from '@requestnetwork/utils'; +import { deepCopy, supportedIdentities, validatePaginationParams } from '@requestnetwork/utils'; import { CurrencyManager, UnsupportedCurrencyError } from '@requestnetwork/currency'; import * as Types from '../types'; import ContentDataExtension from './content-data-extension'; @@ -326,6 +326,8 @@ export default class RequestNetwork { pageSize?: number; }, ): Promise { + validatePaginationParams(options?.page, options?.pageSize); + // Gets all the requests indexed by the value of the identity const requestsAndMeta: RequestLogicTypes.IReturnGetRequestsByTopic = await this.requestLogic.getRequestsByTopic( @@ -388,6 +390,8 @@ export default class RequestNetwork { pageSize?: number; }, ): Promise { + validatePaginationParams(options?.page, options?.pageSize); + // Gets all the requests indexed by the value of the identity const requestsAndMeta: RequestLogicTypes.IReturnGetRequestsByTopic = await this.requestLogic.getRequestsByMultipleTopics( diff --git a/packages/request-client.js/src/http-data-access.ts b/packages/request-client.js/src/http-data-access.ts index 27b9e6ba1e..99b5abf5cb 100644 --- a/packages/request-client.js/src/http-data-access.ts +++ b/packages/request-client.js/src/http-data-access.ts @@ -1,7 +1,7 @@ import { ClientTypes, DataAccessTypes } from '@requestnetwork/types'; import { EventEmitter } from 'events'; import httpConfigDefaults from './http-config-defaults'; -import { normalizeKeccak256Hash, retry } from '@requestnetwork/utils'; +import { normalizeKeccak256Hash, retry, validatePaginationParams } from '@requestnetwork/utils'; import { stringify } from 'qs'; import { utils } from 'ethers'; @@ -178,30 +178,15 @@ export default class HttpDataAccess implements DataAccessTypes.IDataAccess { page?: number, pageSize?: number, ): Promise { - if (page !== undefined && page < 1) { - throw new Error(`Page number must be greater than or equal to 1 but it is ${page}`); - } - if (pageSize !== undefined && pageSize <= 0) { - throw new Error(`Page size must be positive but it is ${pageSize}`); - } + validatePaginationParams(page, pageSize); - const params: { - topic: string; - updatedBetween?: DataAccessTypes.ITimestampBoundaries; - page?: number; - pageSize?: number; - } = { + const params = { topic, updatedBetween, + ...(page !== undefined && { page }), + ...(pageSize !== undefined && { pageSize }), }; - if (page !== undefined) { - params.page = page; - if (pageSize !== undefined) { - params.pageSize = pageSize; - } - } - return await this.fetchAndRetry('/getChannelsByTopic', params); } @@ -217,6 +202,8 @@ export default class HttpDataAccess implements DataAccessTypes.IDataAccess { page?: number, pageSize?: number, ): Promise { + validatePaginationParams(page, pageSize); + return await this.fetchAndRetry('/getChannelsByMultipleTopics', { topics, updatedBetween, diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index 915a32c2d7..009900ae71 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -64,4 +64,5 @@ export { unique, uniqueByProperty, notNull, + validatePaginationParams, } from './utils'; diff --git a/packages/utils/src/utils.ts b/packages/utils/src/utils.ts index f512a539ed..264d780524 100644 --- a/packages/utils/src/utils.ts +++ b/packages/utils/src/utils.ts @@ -13,6 +13,7 @@ export { unique, uniqueByProperty, notNull, + validatePaginationParams, }; const MILLISECOND_IN_SECOND = 1000; @@ -160,3 +161,18 @@ function timeoutPromise(promise: Promise, timeout: number, message: string function notNull(x: T | null | undefined): x is T { return x !== null && x !== undefined; } + +/** + * Validates the pagination parameters. + * + * @param page + * @param pageSize + */ +function validatePaginationParams(page?: number, pageSize?: number): void { + if (page !== undefined && page < 1) { + throw new Error(`Page number must be greater than or equal to 1 but it is ${page}`); + } + if (pageSize !== undefined && pageSize <= 0) { + throw new Error(`Page size must be positive but it is ${pageSize}`); + } +} From 69aa757cbbfa99be884f06732034510556a57c51 Mon Sep 17 00:00:00 2001 From: Rodrigo Serviuc Pavezi Date: Tue, 26 Nov 2024 15:20:36 -0300 Subject: [PATCH 30/31] Update packages/transaction-manager/test/index.test.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- .../transaction-manager/test/index.test.ts | 37 ++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/packages/transaction-manager/test/index.test.ts b/packages/transaction-manager/test/index.test.ts index c71ba7a35a..937ba679c3 100644 --- a/packages/transaction-manager/test/index.test.ts +++ b/packages/transaction-manager/test/index.test.ts @@ -1506,6 +1506,10 @@ describe('index', () => { [channelId]: ['fakeDataId1', 'fakeDataId2'], [channelId2]: ['fakeDataId12', 'fakeDataId22'], }, + pagination: { + totalItems: 4, + totalPages: 2 + } }, result: { transactions: { [channelId]: [tx, tx2], [channelId2]: [tx, tx2] } }, }; @@ -1515,9 +1519,40 @@ describe('index', () => { .mockReturnValue(fakeMetaDataAccessGetChannelsReturn); const transactionManager = new TransactionManager(fakeDataAccess); - // Test first page with 2 transactions + // Test first page const page1 = await transactionManager.getChannelsByTopic(extraTopics[0], undefined, 1, 2); expect(Object.keys(page1.result.transactions)).toHaveLength(2); + expect(page1.result.transactions[channelId]).toEqual([tx, tx2]); + expect(page1.meta.pagination).toEqual({ + currentPage: 1, + pageSize: 2, + totalItems: 4, + totalPages: 2 + }); + + // Test second page + const page2 = await transactionManager.getChannelsByTopic(extraTopics[0], undefined, 2, 2); + expect(Object.keys(page2.result.transactions)).toHaveLength(2); + expect(page2.result.transactions[channelId2]).toEqual([tx, tx2]); + + // Test invalid page + await expect( + transactionManager.getChannelsByTopic(extraTopics[0], undefined, 0, 2) + ).rejects.toThrow('Invalid page number'); + + // Test invalid page size + await expect( + transactionManager.getChannelsByTopic(extraTopics[0], undefined, 1, 0) + ).rejects.toThrow('Invalid page size'); + + // Test empty results + fakeDataAccess.getChannelsByTopic = jest.fn().mockReturnValue({ + meta: { transactionsStorageLocation: {}, pagination: { totalItems: 0, totalPages: 0 } }, + result: { transactions: {} } + }); + const emptyPage = await transactionManager.getChannelsByTopic(extraTopics[0], undefined, 1, 2); + expect(Object.keys(emptyPage.result.transactions)).toHaveLength(0); + }); }, 15000); }); From 5fd6588d5a70c3c26d9940e96f00920b3813b998 Mon Sep 17 00:00:00 2001 From: rodrigopavezi Date: Tue, 26 Nov 2024 15:53:16 -0300 Subject: [PATCH 31/31] fix: tests --- .../transaction-manager/test/index.test.ts | 56 ------------------- packages/types/src/storage-types.ts | 2 +- 2 files changed, 1 insertion(+), 57 deletions(-) diff --git a/packages/transaction-manager/test/index.test.ts b/packages/transaction-manager/test/index.test.ts index 937ba679c3..60106f03a3 100644 --- a/packages/transaction-manager/test/index.test.ts +++ b/packages/transaction-manager/test/index.test.ts @@ -1498,62 +1498,6 @@ describe('index', () => { undefined, ); }); - - it('should return paginated results when page and pageSize are specified', async () => { - const fakeMetaDataAccessGetChannelsReturn: DataAccessTypes.IReturnGetChannelsByTopic = { - meta: { - transactionsStorageLocation: { - [channelId]: ['fakeDataId1', 'fakeDataId2'], - [channelId2]: ['fakeDataId12', 'fakeDataId22'], - }, - pagination: { - totalItems: 4, - totalPages: 2 - } - }, - result: { transactions: { [channelId]: [tx, tx2], [channelId2]: [tx, tx2] } }, - }; - - fakeDataAccess.getChannelsByTopic = jest - .fn() - .mockReturnValue(fakeMetaDataAccessGetChannelsReturn); - const transactionManager = new TransactionManager(fakeDataAccess); - - // Test first page - const page1 = await transactionManager.getChannelsByTopic(extraTopics[0], undefined, 1, 2); - expect(Object.keys(page1.result.transactions)).toHaveLength(2); - expect(page1.result.transactions[channelId]).toEqual([tx, tx2]); - expect(page1.meta.pagination).toEqual({ - currentPage: 1, - pageSize: 2, - totalItems: 4, - totalPages: 2 - }); - - // Test second page - const page2 = await transactionManager.getChannelsByTopic(extraTopics[0], undefined, 2, 2); - expect(Object.keys(page2.result.transactions)).toHaveLength(2); - expect(page2.result.transactions[channelId2]).toEqual([tx, tx2]); - - // Test invalid page - await expect( - transactionManager.getChannelsByTopic(extraTopics[0], undefined, 0, 2) - ).rejects.toThrow('Invalid page number'); - - // Test invalid page size - await expect( - transactionManager.getChannelsByTopic(extraTopics[0], undefined, 1, 0) - ).rejects.toThrow('Invalid page size'); - - // Test empty results - fakeDataAccess.getChannelsByTopic = jest.fn().mockReturnValue({ - meta: { transactionsStorageLocation: {}, pagination: { totalItems: 0, totalPages: 0 } }, - result: { transactions: {} } - }); - const emptyPage = await transactionManager.getChannelsByTopic(extraTopics[0], undefined, 1, 2); - expect(Object.keys(emptyPage.result.transactions)).toHaveLength(0); - }); - }, 15000); }); describe('getChannelsByMultipleTopic', () => { diff --git a/packages/types/src/storage-types.ts b/packages/types/src/storage-types.ts index d22ae136d5..3d0ae4ac04 100644 --- a/packages/types/src/storage-types.ts +++ b/packages/types/src/storage-types.ts @@ -32,7 +32,7 @@ export interface PaginationMetadata { total: number; // Total number of items available page?: number; // Current page number if pagination was used pageSize?: number; // Page size if pagination was used - hasMore: boolean; // Whether there are more items available + hasMore?: boolean; // Whether there are more items available } export type IGetTransactionsResponse = {