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

BatchMaxQueries #1659

Merged
merged 17 commits into from
Jun 20, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,4 @@ Miroslav Simulcik <simulcik.miro@gmail.com>
Stephen Potter <me@stevepotter.me>
Michaël De Boey <info@michaeldeboey.be>
Andreas Bergenwall <abergenw@gmail.com>
Michiel Westerbeek <happylinks@gmail.com>
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
### vNEXT

- `batchInterval` now has a default value of 10 ms [PR #1793](https://github.com/apollographql/apollo-client/pull/1793)
- Added `batchMax` to allow you to limit the amount of queries in one batch. [PR #1659](https://github.com/apollographql/apollo-client/pull/1659)

### 1.4.2
- Improved error messages for writeToStore, readFragment and writeFragment [PR #1766](https://github.com/apollographql/apollo-client/pull/1766), [PR #1722](https://github.com/apollographql/apollo-client/pull/1722)
Expand Down
27 changes: 24 additions & 3 deletions src/transport/batchedNetworkInterface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,30 @@ export class HTTPBatchedNetworkInterface extends BaseNetworkInterface {
public _afterwares: BatchAfterwareInterface[];
private batcher: QueryBatcher;

constructor(uri: string, batchInterval: number = 10, fetchOpts: RequestInit) {
constructor({
uri,
batchInterval = 10,
batchMax = 0,
fetchOpts,
}: {
uri: string,
batchInterval?: number,
batchMax?: number,
fetchOpts: RequestInit,
}) {
super(uri, fetchOpts);

if (typeof batchInterval !== 'number') {
throw new Error(`batchInterval must be a number, got ${batchInterval}`);
}

if (typeof batchMax !== 'number') {
throw new Error(`batchMax must be a number, got ${batchMax}`);
}

this.batcher = new QueryBatcher({
batchInterval: batchInterval,
batchMax: batchMax,
batchFetchFunction: this.batchQuery.bind(this),
});
}
Expand Down Expand Up @@ -217,13 +232,19 @@ export class HTTPBatchedNetworkInterface extends BaseNetworkInterface {

export interface BatchingNetworkInterfaceOptions {
uri: string;
batchInterval: number;
batchInterval?: number;
batchMax?: number;
opts?: RequestInit;
}

export function createBatchingNetworkInterface(options: BatchingNetworkInterfaceOptions): HTTPNetworkInterface {
if (! options) {
throw new Error('You must pass an options argument to createNetworkInterface.');
}
return new HTTPBatchedNetworkInterface(options.uri, options.batchInterval, options.opts || {});
return new HTTPBatchedNetworkInterface({
uri: options.uri,
batchInterval: options.batchInterval,
batchMax: options.batchMax,
fetchOpts: options.opts || {},
});
}
17 changes: 15 additions & 2 deletions src/transport/batching.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,24 @@ export class QueryBatcher {
// Queue on which the QueryBatcher will operate on a per-tick basis.
public queuedRequests: QueryFetchRequest[] = [];

private batchInterval: Number;
private batchInterval: number;
private batchMax: number;

//This function is called to the queries in the queue to the server.
private batchFetchFunction: (request: Request[]) => Promise<ExecutionResult[]>;

constructor({
batchInterval,
batchMax = 0,
batchFetchFunction,
}: {
batchInterval: number,
batchMax?: number,
batchFetchFunction: (request: Request[]) => Promise<ExecutionResult[]>,
}) {
this.queuedRequests = [];
this.batchInterval = batchInterval;
this.batchMax = batchMax;
this.batchFetchFunction = batchFetchFunction;
}

Expand All @@ -56,6 +60,11 @@ export class QueryBatcher {
this.scheduleQueueConsumption();
}

// When amount of requests reaches `batchMax`, trigger the queue consumption without waiting on the `batchInterval`.
if (this.queuedRequests.length === this.batchMax) {
this.consumeQueue();
}

return fetchRequest.promise;
}

Expand All @@ -76,6 +85,7 @@ export class QueryBatcher {
});

this.queuedRequests = [];

const batchedPromise = this.batchFetchFunction(requests);

batchedPromise.then((results) => {
Expand All @@ -87,12 +97,15 @@ export class QueryBatcher {
rejecters[index](error);
});
});

return promises;
}

private scheduleQueueConsumption(): void {
setTimeout(() => {
this.consumeQueue();
if (this.queuedRequests.length) {
this.consumeQueue();
}
}, this.batchInterval);
}
}
17 changes: 14 additions & 3 deletions test/batchedNetworkInterface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,11 @@ describe('HTTPBatchedNetworkInterface', () => {
opts?: RequestInit,
}) => {
const url = 'http://fake.com/graphql';
const batchedNetworkInterface = new HTTPBatchedNetworkInterface(url, 10, opts);
const batchedNetworkInterface = new HTTPBatchedNetworkInterface({
uri: url,
batchInterval: 10,
fetchOpts: opts,
});

batchedNetworkInterface.use(middlewares);
batchedNetworkInterface.useAfter(afterwares);
Expand Down Expand Up @@ -113,7 +117,11 @@ describe('HTTPBatchedNetworkInterface', () => {
it('should construct itself correctly', () => {
const url = 'http://notreal.com/graphql';
const opts = {};
const batchedNetworkInterface = new HTTPBatchedNetworkInterface(url, 10, opts);
const batchedNetworkInterface = new HTTPBatchedNetworkInterface({
uri: url,
batchInterval: 10,
fetchOpts: opts,
});
assert(batchedNetworkInterface);
assert.equal(batchedNetworkInterface._uri, url);
assert.deepEqual(batchedNetworkInterface._opts, opts);
Expand All @@ -124,7 +132,10 @@ describe('HTTPBatchedNetworkInterface', () => {
it('should have a default value of 10ms for batchInterval', () => {
const url = 'http://notreal.com/graphql';
const opts = {};
const batchedNetworkInterface = new HTTPBatchedNetworkInterface(url, undefined, opts);
const batchedNetworkInterface = new HTTPBatchedNetworkInterface({
uri: url,
fetchOpts: opts,
});
const queryBatcher = batchedNetworkInterface['batcher'];
assert.equal(queryBatcher['batchInterval'], 10);
});
Expand Down
198 changes: 198 additions & 0 deletions test/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2201,6 +2201,204 @@ describe('client', () => {
});
});

it('should limit the amount of queries in a batch according to the batchMax value', (done) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This test looks good! After making batchMax optional, please also add a test that checks that no batching happens if batchMax is set to zero.

const authorQuery = gql`
query {
author {
firstName
}
}`;
const authorResult = {
data: {
author: {
firstName: 'John',
},
},
};
const personQuery = gql`
query {
person {
name
}
}`;
const personResult = {
data: {
person: {
name: 'Jane Smith',
},
},
};

const url = 'http://not-a-real-url.com';

const networkInterface = createBatchingNetworkInterface({
uri: url,
batchInterval: 5,
batchMax: 2,
opts: {},
});

const oldFetch = fetch;
fetch = createMockFetch({
url,
opts: {
body: JSON.stringify([
{ query: print(authorQuery) },
{ query: print(personQuery) },
]),
headers: {
Accept: '*/*',
'Content-Type': 'application/json',
},
method: 'POST',
},
result: createMockedIResponse([authorResult, personResult]),
}, {
url,
opts: {
body: JSON.stringify([
{ query: print(authorQuery) },
{ query: print(personQuery) },
]),
headers: {
Accept: '*/*',
'Content-Type': 'application/json',
},
method: 'POST',
},
result: createMockedIResponse([authorResult, personResult]),
}, {
url,
opts: {
body: JSON.stringify([
{ query: print(authorQuery) },
]),
headers: {
Accept: '*/*',
'Content-Type': 'application/json',
},
method: 'POST',
},
result: createMockedIResponse([authorResult]),
});

Promise.all([
networkInterface.query({ query: authorQuery }),
networkInterface.query({ query: personQuery }),
networkInterface.query({ query: authorQuery }),
networkInterface.query({ query: personQuery }),
networkInterface.query({ query: authorQuery }),
]).then((results) => {
assert.deepEqual<[
ExecutionResult,
ExecutionResult,
ExecutionResult,
ExecutionResult,
ExecutionResult
]>(results, [
authorResult,
personResult,
authorResult,
personResult,
authorResult,
]);
fetch = oldFetch;
done();
}).catch( e => {
console.error(e);
});

});

it('should not limit the amount of queries in a batch when batchMax is not set', (done) => {
const authorQuery = gql`
query {
author {
firstName
}
}`;
const authorResult = {
data: {
author: {
firstName: 'John',
},
},
};
const personQuery = gql`
query {
person {
name
}
}`;
const personResult = {
data: {
person: {
name: 'Jane Smith',
},
},
};

const url = 'http://not-a-real-url.com';

const networkInterface = createBatchingNetworkInterface({
uri: url,
batchInterval: 5,
opts: {},
});

const oldFetch = fetch;
fetch = createMockFetch({
url,
opts: {
body: JSON.stringify([
{ query: print(authorQuery) },
{ query: print(personQuery) },
{ query: print(authorQuery) },
{ query: print(personQuery) },
{ query: print(authorQuery) },
]),
headers: {
Accept: '*/*',
'Content-Type': 'application/json',
},
method: 'POST',
},
result: createMockedIResponse([
authorResult,
personResult,
authorResult,
personResult,
authorResult,
]),
});

Promise.all([
networkInterface.query({ query: authorQuery }),
networkInterface.query({ query: personQuery }),
networkInterface.query({ query: authorQuery }),
networkInterface.query({ query: personQuery }),
networkInterface.query({ query: authorQuery }),
]).then((results) => {
assert.deepEqual<[
ExecutionResult,
ExecutionResult,
ExecutionResult,
ExecutionResult,
ExecutionResult
]>(results, [
authorResult,
personResult,
authorResult,
personResult,
authorResult,
]);
fetch = oldFetch;
done();
}).catch( e => {
console.error(e);
});
});

it('should enable dev tools logging', () => {
const query = gql`
query people {
Expand Down