Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Cursor based pagination #39

Merged
merged 46 commits into from
Sep 20, 2021
Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
7b71eea
COR-1597 Add get tx's per page
Yinon-Hever Aug 16, 2021
f906fe4
COR-1597 Add log for visibility
Yinon-Hever Aug 16, 2021
d7816ac
COR-1597 Add log for visibility
Yinon-Hever Aug 16, 2021
b682034
COR-1597 Add log for visibility
Yinon-Hever Aug 16, 2021
6d2e81b
COR-1597 Remove logs
Yinon-Hever Aug 16, 2021
ebfc605
COR-1597 Remove val
Yinon-Hever Aug 16, 2021
5dc9011
COR-1597 Add headers val's
Yinon-Hever Aug 16, 2021
d31748b
COR-1597 Add headers val's
Yinon-Hever Aug 16, 2021
1a04429
COR-1597 Add log
Yinon-Hever Aug 16, 2021
9518de9
COR-1597 Add log
Yinon-Hever Aug 16, 2021
d2cc41b
COR-1597 Add log
Yinon-Hever Aug 16, 2021
3c30348
COR-1597 Add log
Yinon-Hever Aug 16, 2021
53288ac
COR-1597 Add log
Yinon-Hever Aug 16, 2021
f034240
COR-1597 Add log
Yinon-Hever Aug 16, 2021
f0a7811
COR-1597 Add header's values
Yinon-Hever Aug 16, 2021
e1c32b3
COR-1597 Cleaning
Yinon-Hever Aug 16, 2021
514d887
COR-1597 Undefined check
Yinon-Hever Aug 16, 2021
8706b58
COR-1597 Undefined check
Yinon-Hever Aug 16, 2021
181a431
COR-1597 Refactor return val's
Yinon-Hever Aug 17, 2021
caca81f
COR-1597 Add log
Yinon-Hever Aug 17, 2021
bfa8889
COR-1597 Add getNextPage test
Yinon-Hever Aug 17, 2021
6c19c7a
COR-1597 Add getNextPage test
Yinon-Hever Aug 17, 2021
4de78ba
COR-1597 Add logs
Yinon-Hever Aug 17, 2021
38ded0c
COR-1597 Add logs
Yinon-Hever Aug 17, 2021
54f056c
COR-1597 Add logs
Yinon-Hever Aug 17, 2021
19f424c
COR-1597 Add logs
Yinon-Hever Aug 17, 2021
cb61a3f
COR-1597 Add logs
Yinon-Hever Aug 17, 2021
39a7a08
COR-1597 Add logs
Yinon-Hever Aug 17, 2021
ef869e5
COR-1597 Cleaning
Yinon-Hever Aug 18, 2021
ebf3691
COR-1597 Cleaning
Yinon-Hever Aug 18, 2021
cf4a375
COR-1597 Rename api
Yinon-Hever Aug 18, 2021
32f356f
COR-1597 log
Yinon-Hever Aug 18, 2021
2b5c5c3
COR-1597 Refactor
Yinon-Hever Aug 18, 2021
dc26a6a
COR-1597 Refactor
Yinon-Hever Aug 18, 2021
99b86df
COR-1597 Add interface specific per page req
Yinon-Hever Aug 19, 2021
c816582
COR-1597 Refactor desc
Yinon-Hever Aug 19, 2021
e542ceb
Refactor cursor name
Yinon-Hever Sep 6, 2021
da09f8b
Refactor page details
Yinon-Hever Sep 6, 2021
60a514b
debug log
Yinon-Hever Sep 6, 2021
fee6e32
Rename page header
Yinon-Hever Sep 6, 2021
e71e4ab
Refactor getTx's
Yinon-Hever Sep 13, 2021
1eeca42
Refactor get tx's with page info
Yinon-Hever Sep 13, 2021
acb7a59
Refactor description
Yinon-Hever Sep 13, 2021
38b64a0
Cleaning
Yinon-Hever Sep 13, 2021
3daefaa
Merge branch 'master' of https://github.com/fireblocks/fireblocks-sdk…
Yinon-Hever Sep 20, 2021
aa5742b
COR-1597 merge
Yinon-Hever Sep 20, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions src/api-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,29 @@ import { RequestOptions } from "./types";
export class ApiClient {
constructor(private authProvider: IAuthProvider, private apiBaseUrl: string) { }

public async issueGetRequest(path: string) {
public async issueGetRequest(path: string, pageMode: boolean = false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

consider creating a separate function for this specific logic

Copy link
Contributor Author

@Yinon-Hever Yinon-Hever Aug 19, 2021

Choose a reason for hiding this comment

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

The tradeoff here is to duplicate same code as a result of resolveWithFullResponse: true on line 17 and write GET func specific for page or edit all func's using issueGetRequest and extract body, BTW requestPromise lib deprecated, so we can fix it when we use any replacement lib.
@tomervil, what approach do u think we need to take here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But i totally agree that isn't ideal at all, just a quick win

const token = this.authProvider.signJwt(path);

return await requestPromise.get({
const res = await requestPromise.get({
uri: this.apiBaseUrl + path,
headers: {
"X-API-Key": this.authProvider.getApiKey(),
"Authorization": `Bearer ${token}`
},
json: true
json: true,
resolveWithFullResponse: true
});

if (pageMode) {
return {
transactions: res.body,
pageDetails: {
previous: res.headers["previous"] ? res.headers["previous"].toString() : "",
next: res.headers["next"] ? res.headers["next"].toString() : "",
}
};
}

return res.body;
}

public async issuePostRequest(path: string, body: any, requestOptions?: RequestOptions) {
Expand Down
27 changes: 25 additions & 2 deletions src/fireblocks-sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ import {
VaultBalancesFilter,
ValidateAddressResponse,
CreateVaultAssetResponse,
RequestOptions, AllocateFundsRequest, DeallocateFundsRequest, AssetTypeResponse
RequestOptions, AllocateFundsRequest, DeallocateFundsRequest, AssetTypeResponse,
TransactionPageResponse
} from "./types";

export * from "./types";
Expand Down Expand Up @@ -272,7 +273,29 @@ export class FireblocksSDK {
* @param filter.orderBy Determines the order of the results
*/
public async getTransactions(filter: TransactionFilter): Promise<TransactionResponse[]> {
return await this.apiClient.issueGetRequest(`/v1/transactions?${queryString.stringify(filter)}`);
return await this.apiClient.issueGetRequest(`/v1/transactions?${queryString.stringify(filter)}`) as TransactionResponse[];
}

/**
* Gets a list of transactions per page matching the given filter, `orderBy` field not allowed
* @param filter.before Only gets transactions created before a given timestamp (in milliseconds)
* @param filter.after Only gets transactions created after a given timestamp (in milliseconds)
* @param filter.status Only gets transactions with the spcified status
* @param filter.limit Limit the amount of returned results. If not specified, a limit of 200 results will be used
*/
public async getTransactionsWithPageInfo(filter: TransactionFilter): Promise<TransactionPageResponse> {
Yinon-Hever marked this conversation as resolved.
Show resolved Hide resolved
filter.orderBy = undefined;
return await this.apiClient.issueGetRequest(`/v1/transactions?${queryString.stringify(filter)}`, true) as TransactionPageResponse;
}

/**
* Get next or previous page of transactions matching a given path
* @param nextOrPreviousPath Each of path from pageDetails `getTransactionsWithPageInfo` response
*/
public async getTransactionsByPagePath(nextOrPreviousPath: string): Promise<TransactionPageResponse> {
const index = nextOrPreviousPath.indexOf("/v1/");
const path = nextOrPreviousPath.substring(index, nextOrPreviousPath.length);
return await this.apiClient.issueGetRequest(path, true) as TransactionPageResponse;
}

/**
Expand Down
10 changes: 10 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,16 @@ export interface FiatAccountResponse {
assets: AssetResponse[];
}

export interface TransactionPageResponse {
transactions: TransactionResponse[];
pageDetails: PageDetails;
}

export interface PageDetails {
previous: string;
next: string;
}

export interface TransactionResponse {
id: string;
assetId: string;
Expand Down