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
84 changes: 50 additions & 34 deletions src/api.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as _ from 'lodash'
import {EventEmitter} from 'events'
import {Connection, errors, validate} from './common'
import {
Expand Down Expand Up @@ -87,25 +86,12 @@ function getCollectKeyFromCommand(command: string): string|undefined {
}
}

// prevent access to non-validated ledger versions
export class RestrictedConnection extends Connection {
request(request: any, timeout?: number) {
const ledger_index = request.ledger_index
if (ledger_index !== undefined && ledger_index !== 'validated') {
if (!_.isNumber(ledger_index) || ledger_index > this._ledgerVersion) {
return Promise.reject(new errors.LedgerVersionError(
`ledgerVersion ${ledger_index} is greater than server\'s ` +
`most recent validated ledger: ${this._ledgerVersion}`))
}
}
return super.request(request, timeout)
}
}

class RippleAPI extends EventEmitter {

_feeCushion: number
connection: RestrictedConnection

// New in > 0.21.0
// non-validated ledger versions are allowed, and passed to rippled as-is.
connection: Connection

// these are exposed only for use by unit tests; they are not part of the API.
static _PRIVATE = {
Expand All @@ -121,7 +107,7 @@ class RippleAPI extends EventEmitter {
this._feeCushion = options.feeCushion || 1.2
const serverURL = options.server
if (serverURL !== undefined) {
this.connection = new RestrictedConnection(serverURL, options)
this.connection = new Connection(serverURL, options)
this.connection.on('ledgerClosed', message => {
this.emit('ledger', formatLedgerClose(message))
})
Expand All @@ -137,48 +123,77 @@ class RippleAPI extends EventEmitter {
} else {
// use null object pattern to provide better error message if user
// tries to call a method that requires a connection
this.connection = new RestrictedConnection(null, options)
this.connection = new Connection(null, options)
}
}


async _request(command: 'account_info', params: AccountInfoRequest):
async request(command: 'account_info', params: AccountInfoRequest):
Promise<AccountInfoResponse>
async _request(command: 'account_lines', params: AccountLinesRequest):
async request(command: 'account_lines', params: AccountLinesRequest):
Promise<AccountLinesResponse>

/**
* Returns objects owned by an account.
* For an account's trust lines and balances,
* see `getTrustlines` and `getBalances`.
*/
async _request(command: 'account_objects', params: AccountObjectsRequest):
async request(command: 'account_objects', params: AccountObjectsRequest):
Promise<AccountObjectsResponse>

async _request(command: 'account_offers', params: AccountOffersRequest):
async request(command: 'account_offers', params: AccountOffersRequest):
Promise<AccountOffersResponse>
async _request(command: 'book_offers', params: BookOffersRequest):
async request(command: 'book_offers', params: BookOffersRequest):
Promise<BookOffersResponse>
async _request(command: 'gateway_balances', params: GatewayBalancesRequest):
async request(command: 'gateway_balances', params: GatewayBalancesRequest):
Promise<GatewayBalancesResponse>
async _request(command: 'ledger', params: LedgerRequest):
async request(command: 'ledger', params: LedgerRequest):
Promise<LedgerResponse>
async _request(command: 'ledger_entry', params: LedgerEntryRequest):
async request(command: 'ledger_entry', params: LedgerEntryRequest):
Promise<LedgerEntryResponse>

async request(command: string, params: object):
Promise<any>

/**
* Makes a request to the API with the given command and
* additional request body parameters.
*
* NOTE: This command is under development.
*/
async _request(command: string, params: object = {}) {
async request(command: string, params: object = {}) {
return this.connection.request({
...params,
command
})
}

/**
* Returns true if there are more pages of data.
*
* When there are more results than contained in the response, the response
* includes a `marker` field.
*
* See https://ripple.com/build/rippled-apis/#markers-and-pagination
*/
hasNextPage<T extends {marker?: string}>(currentResponse: T) {
return !!currentResponse.marker
}

async requestNextPage<T extends {marker?: string}>(
command: string,
params: object = {},
currentResponse: T
) {
if (!currentResponse.marker) {
return Promise.reject(
new errors.NotFoundError('response does not have a next page')
)
}
const nextPageParams = Object.assign({}, params, {
marker: currentResponse.marker
})
return this.request(command, nextPageParams)
}

/**
* Makes multiple paged requests to the API to return a given number of
* resources. _requestAll() will make multiple requests until the `limit`
Expand All @@ -188,8 +203,9 @@ class RippleAPI extends EventEmitter {
* If the command is unknown, an additional `collect` property is required to
* know which response key contains the array of resources.
*
* NOTE: This command is under development and should not yet be relied
* on by external consumers.
* NOTE: This command is used by existing methods and is not recommended for
* general use. Instead, use rippled's built-in pagination and make multiple
* requests as needed.
*/
async _requestAll(command: 'account_offers', params: AccountOffersRequest):
Promise<AccountOffersResponse[]>
Expand Down Expand Up @@ -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

// here until we add support for unknown commands (since command is some
// unknown string).
const singleResult = await (<Function>this._request)(command, repeatProps)
const singleResult = await (<Function>this.request)(command, repeatProps)
const collectedData = singleResult[collectKey]
marker = singleResult.marker
results.push(singleResult)
Expand Down
2 changes: 1 addition & 1 deletion src/common/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ class Connection extends EventEmitter {
function onDisconnect() {
clearTimeout(timer)
self.removeAllListeners(eventName)
reject(new DisconnectedError())
reject(new DisconnectedError('websocket was closed'))
}

function cleanup() {
Expand Down
2 changes: 1 addition & 1 deletion src/ledger/accountinfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export default async function getAccountInfo(
// 1. Validate
validate.getAccountInfo({address, options})
// 2. Make Request
const response = await this._request('account_info', {
const response = await this.request('account_info', {
account: address,
ledger_index: options.ledgerVersion || 'validated'
})
Expand Down
2 changes: 1 addition & 1 deletion src/ledger/accountobjects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export default async function getAccountObjects(
// through to rippled. rippled validates requests.

// Make Request
const response = await this._request('account_objects', removeUndefined({
const response = await this.request('account_objects', removeUndefined({
account: address,
type: options.type,
ledger_hash: options.ledgerHash,
Expand Down
2 changes: 1 addition & 1 deletion src/ledger/balance-sheet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ async function getBalanceSheet(
validate.getBalanceSheet({address, options})
options = await ensureLedgerVersion.call(this, options)
// 2. Make Request
const response = await this._request('gateway_balances', {
const response = await this.request('gateway_balances', {
account: address,
strict: true,
hotwallet: options.excludeAddresses,
Expand Down
2 changes: 1 addition & 1 deletion src/ledger/ledger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ async function getLedger(
// 1. Validate
validate.getLedger({options})
// 2. Make Request
const response = await this._request('ledger', {
const response = await this.request('ledger', {
ledger_index: options.ledgerVersion || 'validated',
expand: options.includeAllData,
transactions: options.includeTransactions,
Expand Down
2 changes: 1 addition & 1 deletion src/ledger/payment-channel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ async function getPaymentChannel(
// 1. Validate
validate.getPaymentChannel({id})
// 2. Make Request
const response = await this._request('ledger_entry', {
const response = await this.request('ledger_entry', {
index: id,
binary: false,
ledger_index: 'validated'
Expand Down
2 changes: 1 addition & 1 deletion src/ledger/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ async function getSettings(
// 1. Validate
validate.getSettings({address, options})
// 2. Make Request
const response = await this._request('account_info', {
const response = await this.request('account_info', {
account: address,
ledger_index: options.ledgerVersion || 'validated',
signer_lists: true
Expand Down
62 changes: 55 additions & 7 deletions test/api-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,54 @@ describe('RippleAPI', function () {
assert.strictEqual(error.inspect(), '[RippleError(mess, { data: 1 })]');
});

describe('pagination', function () {

describe('hasNextPage', function () {

it('returns true when there is another page', function () {
return this.api.request('ledger_data').then(response => {
assert(this.api.hasNextPage(response));
}
);
});

it('returns false when there are no more pages', function () {
return this.api.request('ledger_data').then(response => {
return this.api.requestNextPage('ledger_data', {}, response);
}).then(response => {
assert(!this.api.hasNextPage(response));
});
});

});

describe('requestNextPage', function () {

it('requests the next page', function () {
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.

});
});

it('rejects when there are no more pages', function () {
return this.api.request('ledger_data').then(response => {
return this.api.requestNextPage('ledger_data', {}, response);
}).then(response => {
return this.api.requestNextPage('ledger_data', {}, response);
}).then(() => {
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

});
});

});

});

describe('preparePayment', function () {

it('normal', function () {
Expand Down Expand Up @@ -1213,15 +1261,15 @@ describe('RippleAPI', function () {
});

it('request account_objects', function () {
return this.api._request('account_objects', {
return this.api.request('account_objects', {
account: address
}).then(response =>
checkResult(responses.getAccountObjects, 'AccountObjectsResponse', response));
});

it('request account_objects - invalid options', function () {
// Intentionally no local validation of these options
return this.api._request('account_objects', {
return this.api.request('account_objects', {
account: address,
invalid: 'options'
}).then(response =>
Expand Down Expand Up @@ -1528,12 +1576,12 @@ describe('RippleAPI', function () {
_.partial(checkResult, responses.getLedger.header, 'getLedger'));
});

// New in > 0.21.0
// future ledger versions are allowed, and passed to rippled as-is.
it('getLedger - future ledger version', function () {
return this.api.getLedger({ ledgerVersion: 14661789 }).then(() => {
assert(false, 'Should throw LedgerVersionError');
}).catch(error => {
assert(error instanceof this.api.errors.LedgerVersionError);
});
return this.api.getLedger({ ledgerVersion: 14661789 }).then(response => {
assert(response)
})
});

it('getLedger - with state as hashes', function () {
Expand Down
4 changes: 4 additions & 0 deletions test/fixtures/rippled/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ module.exports = {
usd_xrp: require('./book-offers-usd-xrp'),
xrp_usd: require('./book-offers-xrp-usd')
},
ledger_data: {
first_page: require('./ledger-data-first-page'),
last_page: require('./ledger-data-last-page')
},
ledger_entry: {
error: require('./ledger-entry-error')
},
Expand Down
40 changes: 40 additions & 0 deletions test/fixtures/rippled/ledger-data-first-page.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
{
"id": 0,
"status": "success",
"type": "response",
"result": {
"ledger_hash":
"102A6E70FFB18C18E97BB56E3047B0E45EA1BCC90BFCCB8CBB0D07BF0E2AB449",
"ledger_index": 38202000,
"marker":
"000B714B790C3C79FEE00D17C4DEB436B375466F29679447BA64F265FD63D730",
"state": [
{
"Flags": 0,
"Indexes": [
"B32769DB3BE790E959A96CF37A62414479E3EB20A5AEC7156B2BF8FD816DBFF8"
],
"LedgerEntryType": "DirectoryNode",
"Owner": "rwt5iiE1mRbBgNhH6spU4nKgHcE7xK9joN",
"RootIndex":
"0005C961C890079D3C4CC8317F9735D388C3CE3D9BCDC152D3C9A7C08F508D1B",
"index":
"0005C961C890079D3C4CC8317F9735D388C3CE3D9BCDC152D3C9A7C08F508D1B"
},
{
"Account": "rpzpyUjdWKmz7yyMvirk3abcaNvSPmDpJn",
"Balance": "91508000",
"Flags": 0,
"LedgerEntryType": "AccountRoot",
"OwnerCount": 0,
"PreviousTxnID":
"F62A5A5EC92DE4E52663B9C7B44A2B76DAB1371737C83A5A81127CBDA84DFE9E",
"PreviousTxnLgrSeq": 35672898,
"Sequence": 1,
"index":
"000B6A1287DB6174F61B1BF987E630CF41DA2A2131CFEB6C5C8143A8F539E9D1"
}
],
"validated": true
}
}
Loading