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

Add request(), hasNextPage(), and requestNextPage() #887

Merged
merged 9 commits into from
May 10, 2018
Merged

Conversation

intelliot
Copy link
Collaborator

@intelliot intelliot commented Apr 25, 2018

request() enables access to all rippled APIs.

While marker can be used directly for pagination, these methods are provided for convenience:

  • hasNextPage(currentResponse) returns true when there are more pages available.
  • requestNextPage(command, params, currentResponse) returns a promise that resolves to the next page of data.

@intelliot intelliot requested a review from mDuo13 April 25, 2018 19:46
src/api.ts Outdated
@@ -225,7 +241,7 @@ class RippleAPI extends EventEmitter {
// NOTE: We have to generalize the `this._request()` function signature
Copy link
Collaborator

Choose a reason for hiding this comment

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

_request -> request

return this.api.request('ledger_data').then(response => {
return this.api.requestNextPage('ledger_data', {}, response);
}).then(response => {
assert.equal(response.state[0].index, '000B714B790C3C79FEE00D17C4DEB436B375466F29679447BA64F265FD63D731')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this instead use the fixture to compare?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since the value comes from the fixture, I think we want to compare with this hardcoded value in order to test that the next page of data was actually retrieved.

assert(false, 'Should reject');
}).catch(error => {
assert(error instanceof Error);
assert.equal(error.message, 'response does not have a next page')
Copy link
Collaborator

Choose a reason for hiding this comment

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

It'd be nice to see (either here or in the hasNextPage test above) that hasNextPage returns false for the same response that requestNextPage rejects.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

Type | Description
---- | -----------
`ledgerClosed` | Sent by the `ledger` stream when the consensus process declares a new fully validated ledger. The message identifies the ledger and provides some information about its contents.
`validationReceived` | Sent by the validations stream when the server receives validation messages, also called validation votes, from validators it trusts.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This stream will include all validations received, not just those the rippled trusts.
XRPLF/xrpl-dev-portal#341


Requests the next page of data.

You can use this convenience method, or include `currentResponse.marker` in your `params` yourself.
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe add "when using/calling request" at the end


Refer to [rippled APIs](https://ripple.com/build/rippled-apis/) for commands and options. All XRP amounts must be specified in drops. One drop is equal to 0.000001 XRP. See [Specifying Currency Amounts](https://ripple.com/build/rippled-apis/#specifying-currency-amounts).

Most commands return data for the `current` (in-progress, open) ledger by default. Do not rely on this. Always specify a ledger version in your request. In the example below, the 'validated' ledger is requested, which is the most recent ledger that has been validated by the whole network. See [Specifying Ledgers](https://ripple.com/build/rippled-apis/#specifying-ledgers).
Copy link
Collaborator

Choose a reason for hiding this comment

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

The top of this file still says

RippleAPI only provides access to validated, immutable transaction data.

https://github.com/ripple/ripple-lib/blob/develop/docs/index.md#introduction

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed

@intelliot
Copy link
Collaborator Author

Thanks @wilsonianb ! Please have a second look.

`ledgerClosed` | Sent by the `ledger` stream when the consensus process declares a new fully validated ledger. The message identifies the ledger and provides some information about its contents.
`validationReceived` | Sent by the `validations` stream when the server receives a validation message, also called a validation vote, regardless of whether the server trusts the validator.
`transaction` | Sent by many subscriptions including `transactions`, `transactions_proposed`, `accounts`, `accounts_proposed`, and `book` (Order Book). See [Transaction Streams](https://ripple.com/build/rippled-apis/#transaction-streams) for details.
`peerStatusChange` | Admin-only. Reports a large amount of information on the activities of other `rippled` servers to which the server is connected.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great catch!

@intelliot intelliot requested a review from wilsonianb May 10, 2018 06:37
@intelliot intelliot mentioned this pull request May 10, 2018
docs/index.md Outdated
@@ -750,6 +753,183 @@ signature | string | *Optional* Signed claim authorizing withdrawal of XRP from
```


# rippled APIs

ripple-lib relies on [rippled APIs](https://ripple.com/build/rippled-apis/) for all online functionality. With ripple-lib version 0.22.0 and higher, you can easily access rippled APIs through ripple-lib. Use the `request()`, `hasNextPage()`, and `requestNextPage()` methods:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Don't worry about the indicated version number, I will update this to be accurate before the release goes out.

Copy link
Collaborator

@wilsonianb wilsonianb left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@intelliot intelliot merged commit b2b6715 into develop May 10, 2018
@intelliot intelliot deleted the request branch May 11, 2018 20:05
@FredKSchott
Copy link
Contributor

@intelliot this looks great! Sorry I wasn't able to review, awesome work getting this pushed out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants