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

auto-batching functionality #71

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
22 changes: 22 additions & 0 deletions src/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ module.exports = class Client {
this.caches = [nullCache()]
this.debug = false
this.client = this
this.autoBatchPool = {}
}

// Set the schema version
Expand Down Expand Up @@ -79,6 +80,27 @@ module.exports = class Client {
})
}

// Maintains a pool of static endpoints with auto-batching enabled
autoBatch (endpointName, batchDelay) {
if (this.autoBatchPool[endpointName]) {
return this.autoBatchPool[endpointName]
}

if (!this[endpointName]) {
throw new Error(`no enpoint ${endpointName} found`)
}

const resultEndpoint = this[endpointName]()
if (resultEndpoint.isBulk) {
this.autoBatchPool[endpointName] = resultEndpoint.enableAutoBatch(batchDelay)
}
else {
this.debugMessage(`${endpointName} is not bulk expanding, endpoint will not have any autobatch behavior`)
}

return resultEndpoint
}

// All the different API endpoints
account () {
return new endpoints.AccountEndpoint(this)
Expand Down
65 changes: 60 additions & 5 deletions src/endpoint.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ module.exports = class AbstractEndpoint {
this.isOptionallyAuthenticated = false
this.credentials = false

this._autoBatch = null

this._skipCache = false
}

Expand Down Expand Up @@ -74,6 +76,18 @@ module.exports = class AbstractEndpoint {
return this
}

// Turn on auto-batching for this endpoint
enableAutoBatch (batchDelay = 100) {
if (this._autoBatch === null) {
this._autoBatch = {
batchDelay: batchDelay,
idsForNextBatch: new Set(),
nextBatchPromise: null,
}
}
return this
}

// Get all ids
ids () {
this.debugMessage(`ids(${this.url}) called`)
Expand Down Expand Up @@ -156,7 +170,14 @@ module.exports = class AbstractEndpoint {

// Request the single id if the endpoint a bulk endpoint
if (this.isBulk && !url) {
return this._request(`${this.url}?id=${id}`)
if (this._autoBatch === null) {
return this._request(`${this.url}?id=${id}`)
}
else {
return this._autoBatchMany([id]).then((items) => {
return items[0]?items[0]:null
})
}
}

// We are dealing with a custom url instead
Expand All @@ -168,8 +189,34 @@ module.exports = class AbstractEndpoint {
return this._request(this.url)
}

_autoBatchMany (ids) {
if (this._autoBatch.idsForNextBatch.size === 0) {
this._autoBatch.nextBatchPromise = new Promise((resolve, reject) => {
setTimeout(() => {
const batchedIds = Array.from(this._autoBatch.idsForNextBatch)
this.debugMessage(`autoBatchMany called (${batchedIds.length} ids)`)
this._autoBatch.idsForNextBatch.clear()
return resolve(this.many(batchedIds, true))
}, this._autoBatch.batchDelay)
}).then(items => {
const indexedItems = {}
items.forEach(item => {
indexedItems[item.id] = item
})
return indexedItems
})
}

// Add the requested ids to the pending ids
ids.forEach(id => this._autoBatch.idsForNextBatch.add(id))
// Return the results based on the requested ids
return this._autoBatch.nextBatchPromise.then(indexedItems => {
return ids.map(id => indexedItems[id]).filter(x => x)
})
}

// Get multiple entries by ids
many (ids) {
many (ids, skipAutoBatch) {
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
many (ids, skipAutoBatch) {
many (ids, skipAutoBatch = false) {

this.debugMessage(`many(${this.url}) called (${ids.length} ids)`)

if (!this.isBulk) {
Expand All @@ -186,7 +233,7 @@ module.exports = class AbstractEndpoint {

// There is no cache time set, so always use the live data
if (!this.cacheTime) {
return this._many(ids)
return this._many(ids, undefined, skipAutoBatch)
}

// Get as much as possible out of the cache
Expand All @@ -201,7 +248,7 @@ module.exports = class AbstractEndpoint {

this.debugMessage(`many(${this.url}) resolving partially from cache (${cached.length} ids)`)
const missingIds = getMissingIds(ids, cached)
return this._many(missingIds, cached.length > 0).then(content => {
return this._many(missingIds, cached.length > 0, skipAutoBatch).then(content => {
const cacheContent = content.map(value => [this._cacheHash(value.id), value])
this._cacheSetMany(cacheContent)

Expand Down Expand Up @@ -230,9 +277,17 @@ module.exports = class AbstractEndpoint {
}

// Get multiple entries by ids from the live API
_many (ids, partialRequest = false) {
_many (ids, partialRequest = false, skipAutoBatch) {
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
_many (ids, partialRequest = false, skipAutoBatch) {
_many (ids, partialRequest = false, skipAutoBatch = false) {

this.debugMessage(`many(${this.url}) requesting from api (${ids.length} ids)`)

if (this._autoBatch !== null) {
if (!skipAutoBatch) {
return this._autoBatchMany(ids)
}
}
Comment on lines +289 to +293
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
if (this._autoBatch !== null) {
if (!skipAutoBatch) {
return this._autoBatchMany(ids)
}
}
if (this._autoBatch !== null && !skipAutoBatch) {
return this._autoBatchMany(ids)
}




// Chunk the requests to the max page size
const pages = chunk(ids, this.maxPageSize)
const requests = pages.map(page => `${this.url}?ids=${page.join(',')}`)
Expand Down
44 changes: 44 additions & 0 deletions tests/client.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,17 @@ const memoryCache = require('../src/cache/memory')
const Module = require('../src/client')
const wait = (ms) => new Promise((resolve) => setTimeout(resolve, ms))

async function expectError (callback) {
let err
try {
await callback()
} catch (e) {
err = e
}

expect(err).toBeInstanceOf(Error)
}

describe('client', () => {
let client
beforeEach(() => {
Expand Down Expand Up @@ -123,6 +134,39 @@ describe('client', () => {
client.build = tmp
})

describe('autobatch', () => {
const batchDelay = 10
it('can get an autobatching endpoint', () => {
let endpoint = client.autoBatch('items', batchDelay)
expect(endpoint.url).toEqual('/v2/items')
expect(endpoint._autoBatch.batchDelay).toEqual(batchDelay)
})

it('adds the endpoint to the pool', () => {
client.autoBatch('items', batchDelay)
expect(client.autoBatchPool.items).toBeDefined()
})

it('returns the same endpoint on subsequent calls', () => {
let endpoint1 = client.autoBatch('items', batchDelay)
endpoint1.arbitraryPropertyNameForTesting = 'test confirmed'
let endpoint2 = client.autoBatch('items', batchDelay)
expect(endpoint2.arbitraryPropertyNameForTesting).toEqual('test confirmed')
})

it(`errors if endpoint name doesn't exist`, async () => {
await expectError(() => client.autoBatch('does not exist'))
})

it(`debugMessage if endpoint is not bulk expanding`, () => {
const debugMessageMock = jest.fn()
client.debugMessage = debugMessageMock

const eventsAutoBatch = client.autoBatch('events')
expect(debugMessageMock.mock.calls.length).toBe(1)
})
})

it('can get the account endpoint', () => {
let endpoint = client.account()
expect(endpoint.url).toEqual('/v2/account')
Expand Down
84 changes: 84 additions & 0 deletions tests/endpoint.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,90 @@ describe('abstract endpoint', () => {
})
})

describe('auto batching', () => {
const batchDelay = 10

it('sets up _autoBatch variable', () => {
let x = endpoint.enableAutoBatch(batchDelay)
expect(x).toBeInstanceOf(Module)
expect(x._autoBatch.batchDelay).toEqual(batchDelay)
expect(x._autoBatch.idsForNextBatch).toBeDefined()
expect(x._autoBatch.nextBatchPromise).toBeNull()
})

it('has default batchDelay of 100', () => {
let x = endpoint.enableAutoBatch()
expect(x._autoBatch.batchDelay).toEqual(100)
})

it('enableAutoBatch does not overwrite _autoBatch variable', () => {
endpoint.enableAutoBatch()
endpoint.enableAutoBatch(batchDelay)
expect(endpoint._autoBatch.batchDelay).toEqual(100)
})

it('supports batching from get', async () => {
let content = [{ id: 1, name: 'foo' }, { id: 2, name: 'bar' }]
endpoint.isBulk = true
endpoint.url = '/v2/test'
endpoint.enableAutoBatch(batchDelay)
fetchMock.addResponse(content)

let [entry1, entry2] = await Promise.all([endpoint.get(1), endpoint.get(2)])
expect(fetchMock.lastUrl()).toEqual('https://api.guildwars2.com/v2/test?v=schema&ids=1,2')
expect(entry1).toEqual(content[0])
expect(entry2).toEqual(content[1])
})

it('returns null from get with no response', async () => {
let content = []
endpoint.isBulk = true
endpoint.url = '/v2/test'
endpoint.enableAutoBatch(batchDelay)
fetchMock.addResponse(content)

let [entry1, entry2] = await Promise.all([endpoint.get(1), endpoint.get(2)])
expect(fetchMock.lastUrl()).toEqual('https://api.guildwars2.com/v2/test?v=schema&ids=1,2')
expect(entry1).toEqual(null)
expect(entry2).toEqual(null)
})

it('supports batching from many', async () => {
let content = [{ id: 1, name: 'foo' }, { id: 2, name: 'bar' }, { id: 3, name: 'bar' }]
endpoint.isBulk = true
endpoint.url = '/v2/test'
endpoint.enableAutoBatch(batchDelay)
fetchMock.addResponse(content)

let [entry1, entry2] = await Promise.all([endpoint.many([1,2]), endpoint.many([2,3])])
expect(fetchMock.lastUrl()).toEqual('https://api.guildwars2.com/v2/test?v=schema&ids=1,2,3')
expect(entry1).toEqual([content[0],content[1]])
expect(entry2).toEqual([content[1],content[2]])
})

it('only batches requests during the batchDelay', async () => {
let content1 = [{ id: 1, name: 'foo' }]
let content2 = [{ id: 2, name: 'bar' }]
endpoint.isBulk = true
endpoint.url = '/v2/test'
endpoint.enableAutoBatch(batchDelay)
fetchMock.addResponse(content1)
fetchMock.addResponse(content2)

let [entry1, entry2] = await Promise.all([
endpoint.get(1),
new Promise((resolve) => {setTimeout(() => {resolve(endpoint.get(2))}, batchDelay+1)})
])
expect(fetchMock.urls()).toEqual([
'https://api.guildwars2.com/v2/test?v=schema&ids=1',
'https://api.guildwars2.com/v2/test?v=schema&ids=2'
])
expect(entry1).toEqual(content1[0])
expect(entry2).toEqual(content2[0])

})
})

describe('get', () => {
it('support for bulk expanding', async () => {
let content = { id: 1, name: 'foo' }
Expand Down