From c901c0833026b7f167a1c883f63f6e15bfe4bde7 Mon Sep 17 00:00:00 2001 From: Dhaivat Pandya Date: Thu, 14 Jul 2016 11:57:48 -0700 Subject: [PATCH 01/13] added some of the basic functions needed to do polling interval alignment --- src/scheduler.ts | 41 +++++++++++++++++++++++++++++++++++++++++ test/scheduler.ts | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/src/scheduler.ts b/src/scheduler.ts index 935f8898026..f10e7632c43 100644 --- a/src/scheduler.ts +++ b/src/scheduler.ts @@ -28,6 +28,14 @@ export class QueryScheduler { // Map going from queryIds to polling timers. private pollingTimers: { [queryId: string]: NodeJS.Timer | any }; // oddity in Typescript + // Map going from query ids to the query options associated with those queries. Contains all of + // the queries, both in flight and not in flight. + private registeredQueries: { [queryId: string]: WatchQueryOptions }; + + // Map going from polling interval with to the query ids that fire on that interval. + // These query ids are associated with a set of options in the this.registeredQueries. + public intervalQueries: { [interval: number]: string[] }; + constructor({ queryManager, }: { @@ -36,6 +44,7 @@ export class QueryScheduler { this.queryManager = queryManager; this.pollingTimers = {}; this.inFlightQueries = {}; + this.intervalQueries = {}; } public checkInFlight(queryId: string) { @@ -103,6 +112,38 @@ export class QueryScheduler { }); } + // Fires the all of the queries on a particular interval. Called on a setInterval. + public fireQueriesOnInterval(interval: number) { + this.intervalQueries[interval].forEach((queryId) => { + const queryOptions = this.registeredQueries[queryId]; + + const pollingOptions = assign({}, queryOptions) as WatchQueryOptions; + pollingOptions.forceFetch = true; + this.fetchQuery(queryId, pollingOptions).then(() => { + this.removeInFlight(queryId); + }); + }); + } + + // Adds a query on a particular interval to this.intervalQueries and then fires + // that query with all the other queries executing on that interval. Note that the query id + // and query options must have been added to this.registeredQueries before this function is called. + public addQueryOnInterval(queryId: string, queryOptions: WatchQueryOptions) { + const interval = queryOptions.pollInterval; + + // If there are other queries on this interval, this query will just fire with those + // and we don't need to create a new timer. + if (this.intervalQueries.hasOwnProperty(interval.toString())) { + this.intervalQueries[interval].push(queryId); + } else { + this.intervalQueries[interval] = [queryId]; + // set up the timer + this.pollingTimers[queryId] = setInterval(() => { + this.fireQueriesOnInterval(interval) + }, interval); + } + } + private addInFlight(queryId: string, options: WatchQueryOptions) { this.inFlightQueries[queryId] = options; } diff --git a/test/scheduler.ts b/test/scheduler.ts index 80a6ab2f294..db1fa1be601 100644 --- a/test/scheduler.ts +++ b/test/scheduler.ts @@ -332,4 +332,39 @@ describe('QueryScheduler', () => { done(); }, 100); }); + + it('should add queries on an interval correctly', () => { + const query = gql` + query { + fortuneCookie + }`; + const data = { + 'fortuneCookie': 'live long and prosper', + }; + const queryOptions = { + query, + pollInterval: 10000, + }; + const networkInterface = mockNetworkInterface( + { + request: queryOptions, + result: { data }, + } + ); + const queryManager = new QueryManager({ + networkInterface, + store: createApolloStore(), + reduxRootKey: 'apollo', + }); + const scheduler = new QueryScheduler({ + queryManager, + }); + const queryId = 'fake-id'; + scheduler.addQueryOnInterval(queryId, queryOptions); + assert.equal(Object.keys(scheduler.intervalQueries).length, 1); + assert.equal(Object.keys(scheduler.intervalQueries)[0], queryOptions.pollInterval.toString()); + const queries = scheduler.intervalQueries[queryOptions.pollInterval.toString()]; + assert.equal(queries.length, 1); + assert.equal(queries[0], queryId); + }); }); From a7e29ccaf93363cefe4f4f8d2eb4650eb84414af Mon Sep 17 00:00:00 2001 From: Dhaivat Pandya Date: Thu, 14 Jul 2016 14:38:58 -0700 Subject: [PATCH 02/13] corrected the rest of the scheduler so that queries on the same interval are batched together --- src/scheduler.ts | 59 +++++++++++++++++++++++++++-------------------- test/scheduler.ts | 3 ++- 2 files changed, 36 insertions(+), 26 deletions(-) diff --git a/src/scheduler.ts b/src/scheduler.ts index f10e7632c43..db54d6ab31c 100644 --- a/src/scheduler.ts +++ b/src/scheduler.ts @@ -25,8 +25,8 @@ export class QueryScheduler { // mechanism). private queryManager: QueryManager; - // Map going from queryIds to polling timers. - private pollingTimers: { [queryId: string]: NodeJS.Timer | any }; // oddity in Typescript + // Map going from polling interval widths to polling timers. + private pollingTimers: { [interval: number]: NodeJS.Timer | any }; // oddity in Typescript // Map going from query ids to the query options associated with those queries. Contains all of // the queries, both in flight and not in flight. @@ -44,6 +44,7 @@ export class QueryScheduler { this.queryManager = queryManager; this.pollingTimers = {}; this.inFlightQueries = {}; + this.registeredQueries = {}; this.intervalQueries = {}; } @@ -64,25 +65,18 @@ export class QueryScheduler { }); } - public startPollingQuery(options: WatchQueryOptions, listener: QueryListener, - queryId?: string): string { + public startPollingQuery( + options: WatchQueryOptions, + listener: QueryListener, + queryId?: string + ): string { if (!queryId) { queryId = this.queryManager.generateQueryId(); } // Fire an initial fetch before we start the polling query this.fetchQuery(queryId, options); this.queryManager.addQueryListener(queryId, listener); - - this.pollingTimers[queryId] = setInterval(() => { - const pollingOptions = assign({}, options) as WatchQueryOptions; - pollingOptions.forceFetch = true; - - // We only fire the query if another instance of this same polling query isn't - // already in flight. See top of this file for the reasoning as to why we do this. - if (!this.checkInFlight(queryId)) { - this.fetchQuery(queryId, pollingOptions); - } - }, options.pollInterval); + this.addQueryOnInterval(queryId, options); return queryId; } @@ -92,9 +86,9 @@ export class QueryScheduler { // further data returned. this.queryManager.removeQueryListener(queryId); - if (this.pollingTimers[queryId]) { - clearInterval(this.pollingTimers[queryId]); - } + // Remove the query options from one of the registered queries. + // The polling function will then take care of not firing it anymore. + delete this.registeredQueries[queryId]; // Fire a APOLLO_STOP_QUERY state change to the underlying store. this.queryManager.stopQueryInStore(queryId); @@ -114,15 +108,30 @@ export class QueryScheduler { // Fires the all of the queries on a particular interval. Called on a setInterval. public fireQueriesOnInterval(interval: number) { - this.intervalQueries[interval].forEach((queryId) => { - const queryOptions = this.registeredQueries[queryId]; + this.intervalQueries[interval] = this.intervalQueries[interval].filter((queryId) => { + // If queryOptions can't be found from registeredQueries, it means that this queryId + // is no longer registered and should be removed from the list of queries firing on this + // interval. + if (!this.registeredQueries.hasOwnProperty(queryId)) { + return false; + } + // Don't fire this instance of the polling query is one of the instances is already in + // flight. + if (this.checkInFlight(queryId)) { + return true; + } + + const queryOptions = this.registeredQueries[queryId]; const pollingOptions = assign({}, queryOptions) as WatchQueryOptions; pollingOptions.forceFetch = true; - this.fetchQuery(queryId, pollingOptions).then(() => { - this.removeInFlight(queryId); - }); + this.fetchQuery(queryId, pollingOptions); + return true; }); + + if (this.intervalQueries[interval].length == 0) { + clearInterval(this.pollingTimers[interval]); + } } // Adds a query on a particular interval to this.intervalQueries and then fires @@ -137,8 +146,8 @@ export class QueryScheduler { this.intervalQueries[interval].push(queryId); } else { this.intervalQueries[interval] = [queryId]; - // set up the timer - this.pollingTimers[queryId] = setInterval(() => { + // set up the timer for the function that will handle this interval + this.pollingTimers[interval] = setInterval(() => { this.fireQueriesOnInterval(interval) }, interval); } diff --git a/test/scheduler.ts b/test/scheduler.ts index db1fa1be601..f10e9f2c679 100644 --- a/test/scheduler.ts +++ b/test/scheduler.ts @@ -263,6 +263,7 @@ describe('QueryScheduler', () => { const queryOptions = { query, pollInterval: 70, + forceFetch: true, }; const networkInterface = mockNetworkInterface( { @@ -333,7 +334,7 @@ describe('QueryScheduler', () => { }, 100); }); - it('should add queries on an interval correctly', () => { + it('should add a query to an interval correctly', () => { const query = gql` query { fortuneCookie From 43a26dae67adb7cea6ff82a426df70c8d76f4c9c Mon Sep 17 00:00:00 2001 From: Dhaivat Pandya Date: Thu, 14 Jul 2016 15:31:55 -0700 Subject: [PATCH 03/13] added some more tests --- src/scheduler.ts | 2 +- test/scheduler.ts | 110 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 1 deletion(-) diff --git a/src/scheduler.ts b/src/scheduler.ts index db54d6ab31c..2e4c5609952 100644 --- a/src/scheduler.ts +++ b/src/scheduler.ts @@ -30,7 +30,7 @@ export class QueryScheduler { // Map going from query ids to the query options associated with those queries. Contains all of // the queries, both in flight and not in flight. - private registeredQueries: { [queryId: string]: WatchQueryOptions }; + public registeredQueries: { [queryId: string]: WatchQueryOptions }; // Map going from polling interval with to the query ids that fire on that interval. // These query ids are associated with a set of options in the this.registeredQueries. diff --git a/test/scheduler.ts b/test/scheduler.ts index f10e9f2c679..b8174f4973a 100644 --- a/test/scheduler.ts +++ b/test/scheduler.ts @@ -368,4 +368,114 @@ describe('QueryScheduler', () => { assert.equal(queries.length, 1); assert.equal(queries[0], queryId); }); + + it('should add multiple queries to an interval correctly', () => { + const query1 = gql` + query { + fortuneCookie + }`; + const data1 = { + 'fortuneCookie': 'live long and prosper', + }; + const query2 = gql` + query { + author { + firstName + lastName + } + }`; + const data2 = { + author: { + firstName: 'Dhaivat', + lastName: 'Pandya', + } + }; + const interval = 20000; + const queryOptions1 = { + query: query1, + pollInterval: interval, + }; + const queryOptions2 = { + query: query2, + pollInterval: interval, + }; + const queryManager = new QueryManager({ + networkInterface: mockNetworkInterface( + { + request: { query: query1 }, + result: { data: data1 }, + }, + { + request: { query: query2 }, + result: { data: data2 }, + } + ), + store: createApolloStore(), + reduxRootKey: 'apollo', + }); + const scheduler = new QueryScheduler({ + queryManager, + }); + const observable1 = scheduler.registerPollingQuery(queryOptions1); + observable1.subscribe({ + next(result) { + //do nothing + } + }); + + const observable2 = scheduler.registerPollingQuery(queryOptions2); + observable2.subscribe({ + next(result) { + //do nothing + } + }); + + const keys = Object.keys(scheduler.intervalQueries); + assert.equal(keys.length, 1); + assert.equal(keys[0], interval); + + const queryIds = scheduler.intervalQueries[keys[0]]; + assert.equal(queryIds.length, 2); + assert.deepEqual(scheduler.registeredQueries[queryIds[0]], queryOptions1); + assert.deepEqual(scheduler.registeredQueries[queryIds[1]], queryOptions2); + }); + + it('should remove queries from the interval list correctly', () => { + const query = gql` + query { + author { + firstName + lastName + } + }`; + const data = { + author: { + firstName: 'John', + lastName: 'Smith', + } + }; + const queryManager = new QueryManager({ + networkInterface: mockNetworkInterface( + { + request: { query }, + result: { data }, + } + ), + store: createApolloStore(), + reduxRootKey: 'apollo', + }); + const scheduler = new QueryScheduler({ + queryManager, + }); + let timesFired = 0; + const observable = scheduler.registerPollingQuery({ query, pollInterval: 20 }); + const subscription = observable.subscribe({ + next(result) { + timesFired += 1; + assert.deepEqual(result, { data }); + subscription.unsubscribe(); + assert.equal(Object.keys(scheduler.registeredQueries).length, 0); + } + }); + }); }); From 17a5a0ca11f4b6214fd330b3d73c31b5eebaa81c Mon Sep 17 00:00:00 2001 From: Dhaivat Pandya Date: Thu, 14 Jul 2016 16:15:16 -0700 Subject: [PATCH 04/13] pacified linter --- src/scheduler.ts | 18 +++++++++--------- test/scheduler.ts | 10 +++++----- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/scheduler.ts b/src/scheduler.ts index 2e4c5609952..60557841352 100644 --- a/src/scheduler.ts +++ b/src/scheduler.ts @@ -21,13 +21,6 @@ export class QueryScheduler { // Map going from queryIds to query options that are in flight. public inFlightQueries: { [queryId: string]: WatchQueryOptions }; - // We use this instance to actually fire queries (i.e. send them to the batching - // mechanism). - private queryManager: QueryManager; - - // Map going from polling interval widths to polling timers. - private pollingTimers: { [interval: number]: NodeJS.Timer | any }; // oddity in Typescript - // Map going from query ids to the query options associated with those queries. Contains all of // the queries, both in flight and not in flight. public registeredQueries: { [queryId: string]: WatchQueryOptions }; @@ -36,6 +29,13 @@ export class QueryScheduler { // These query ids are associated with a set of options in the this.registeredQueries. public intervalQueries: { [interval: number]: string[] }; + // We use this instance to actually fire queries (i.e. send them to the batching + // mechanism). + private queryManager: QueryManager; + + // Map going from polling interval widths to polling timers. + private pollingTimers: { [interval: number]: NodeJS.Timer | any }; // oddity in Typescript + constructor({ queryManager, }: { @@ -129,7 +129,7 @@ export class QueryScheduler { return true; }); - if (this.intervalQueries[interval].length == 0) { + if (this.intervalQueries[interval].length === 0) { clearInterval(this.pollingTimers[interval]); } } @@ -148,7 +148,7 @@ export class QueryScheduler { this.intervalQueries[interval] = [queryId]; // set up the timer for the function that will handle this interval this.pollingTimers[interval] = setInterval(() => { - this.fireQueriesOnInterval(interval) + this.fireQueriesOnInterval(interval); }, interval); } } diff --git a/test/scheduler.ts b/test/scheduler.ts index b8174f4973a..d25d8181e73 100644 --- a/test/scheduler.ts +++ b/test/scheduler.ts @@ -388,7 +388,7 @@ describe('QueryScheduler', () => { author: { firstName: 'Dhaivat', lastName: 'Pandya', - } + }, }; const interval = 20000; const queryOptions1 = { @@ -420,14 +420,14 @@ describe('QueryScheduler', () => { observable1.subscribe({ next(result) { //do nothing - } + }, }); const observable2 = scheduler.registerPollingQuery(queryOptions2); observable2.subscribe({ next(result) { //do nothing - } + }, }); const keys = Object.keys(scheduler.intervalQueries); @@ -452,7 +452,7 @@ describe('QueryScheduler', () => { author: { firstName: 'John', lastName: 'Smith', - } + }, }; const queryManager = new QueryManager({ networkInterface: mockNetworkInterface( @@ -475,7 +475,7 @@ describe('QueryScheduler', () => { assert.deepEqual(result, { data }); subscription.unsubscribe(); assert.equal(Object.keys(scheduler.registeredQueries).length, 0); - } + }, }); }); }); From 669f808d39b232000201c82a8d8387ca212b3561 Mon Sep 17 00:00:00 2001 From: Dhaivat Pandya Date: Thu, 14 Jul 2016 16:18:57 -0700 Subject: [PATCH 05/13] updated a test - should all be tested at this point --- test/scheduler.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/scheduler.ts b/test/scheduler.ts index d25d8181e73..370cb5717c6 100644 --- a/test/scheduler.ts +++ b/test/scheduler.ts @@ -440,7 +440,7 @@ describe('QueryScheduler', () => { assert.deepEqual(scheduler.registeredQueries[queryIds[1]], queryOptions2); }); - it('should remove queries from the interval list correctly', () => { + it('should remove queries from the interval list correctly', (done) => { const query = gql` query { author { @@ -468,7 +468,7 @@ describe('QueryScheduler', () => { queryManager, }); let timesFired = 0; - const observable = scheduler.registerPollingQuery({ query, pollInterval: 20 }); + const observable = scheduler.registerPollingQuery({ query, pollInterval: 10 }); const subscription = observable.subscribe({ next(result) { timesFired += 1; @@ -477,5 +477,10 @@ describe('QueryScheduler', () => { assert.equal(Object.keys(scheduler.registeredQueries).length, 0); }, }); + + setTimeout(() => { + assert.equal(timesFired, 1); + done(); + }, 100); }); }); From 4ad080f4cd0de6bde51c8af7d255f07a60983933 Mon Sep 17 00:00:00 2001 From: Dhaivat Pandya Date: Fri, 15 Jul 2016 11:55:02 -0700 Subject: [PATCH 06/13] all tests passing on basic alignment of polling intervals --- src/QueryManager.ts | 70 ++++++++++++++++++++++---------------------- src/scheduler.ts | 57 ++++++++++++++++++++---------------- test/QueryManager.ts | 6 ++-- test/scheduler.ts | 8 ++--- 4 files changed, 73 insertions(+), 68 deletions(-) diff --git a/src/QueryManager.ts b/src/QueryManager.ts index fae772bb55f..8cba1b49248 100644 --- a/src/QueryManager.ts +++ b/src/QueryManager.ts @@ -95,23 +95,29 @@ export class ObservableQuery extends Observable { public stopPolling: () => void; public startPolling: (p: number) => void; public options: WatchQueryOptions; - public queryManager: QueryManager; private queryId: string; + private scheduler: QueryScheduler; + private queryManager: QueryManager; constructor({ - queryManager, + scheduler, options, shouldSubscribe = true, }: { - queryManager: QueryManager, + scheduler: QueryScheduler, options: WatchQueryOptions, shouldSubscribe?: boolean, }) { - + const queryManager = scheduler.queryManager; const queryId = queryManager.generateQueryId(); + const isPollingQuery = !!options.pollInterval; + const subscriberFunction = (observer: Observer) => { const retQuerySubscription = { unsubscribe: () => { + if (isPollingQuery) { + scheduler.stopPollingQuery(queryId); + } queryManager.stopQuery(queryId); }, }; @@ -121,15 +127,23 @@ export class ObservableQuery extends Observable { queryManager.addQuerySubscription(queryId, retQuerySubscription); } + if (isPollingQuery) { + this.scheduler.startPollingQuery( + options, + queryId + ); + } queryManager.startQuery( queryId, options, queryManager.queryListenerForObserver(options, observer) ); + return retQuerySubscription; }; super(subscriberFunction); this.options = options; + this.scheduler = scheduler; this.queryManager = queryManager; this.queryId = queryId; @@ -147,8 +161,9 @@ export class ObservableQuery extends Observable { }; this.stopPolling = () => { - if (this.queryManager.pollingTimers[this.queryId]) { - clearInterval(this.queryManager.pollingTimers[this.queryId]); + this.queryManager.stopQuery(this.queryId); + if (isPollingQuery) { + this.scheduler.stopPollingQuery(this.queryId); } }; @@ -156,12 +171,12 @@ export class ObservableQuery extends Observable { if (this.options.noFetch) { throw new Error('noFetch option should not use query polling.'); } - this.queryManager.pollingTimers[this.queryId] = setInterval(() => { - const pollingOptions = assign({}, this.options) as WatchQueryOptions; - // subsequent fetches from polling always reqeust new data - pollingOptions.forceFetch = true; - this.queryManager.fetchQuery(this.queryId, pollingOptions); - }, pollInterval); + + if (isPollingQuery) { + this.scheduler.stopPollingQuery(this.queryId); + } + options.pollInterval = pollInterval; + this.scheduler.startPollingQuery(this.options, this.queryId, false); }; } @@ -187,6 +202,8 @@ export type QueryListener = (queryStoreValue: QueryStoreValue) => void; export class QueryManager { public pollingTimers: {[queryId: string]: NodeJS.Timer | any}; //oddity in Typescript + public scheduler: QueryScheduler; + private networkInterface: NetworkInterface; private store: ApolloStore; private reduxRootKey: string; @@ -195,7 +212,6 @@ export class QueryManager { private idCounter = 0; - private scheduler: QueryScheduler; private batcher: QueryBatcher; private batchInterval: number; @@ -410,7 +426,7 @@ export class QueryManager { getQueryDefinition(options.query); let observableQuery = new ObservableQuery({ - queryManager: this, + scheduler: this.scheduler, options: options, shouldSubscribe: shouldSubscribe, }); @@ -542,18 +558,11 @@ export class QueryManager { public startQuery(queryId: string, options: WatchQueryOptions, listener: QueryListener) { this.queryListeners[queryId] = listener; - this.fetchQuery(queryId, options); - if (options.pollInterval) { - if (options.noFetch) { - throw new Error('noFetch option should not use query polling.'); - } - this.pollingTimers[queryId] = setInterval(() => { - const pollingOptions = assign({}, options) as WatchQueryOptions; - // subsequent fetches from polling always reqeust new data - pollingOptions.forceFetch = true; - this.fetchQuery(queryId, pollingOptions); - }, options.pollInterval); + // If the pollInterval is present, the scheduler has already taken care of firing the first + // fetch so we don't have to worry about it here. + if (!options.pollInterval) { + this.fetchQuery(queryId, options); } return queryId; @@ -563,16 +572,7 @@ export class QueryManager { // XXX in the future if we should cancel the request // so that it never tries to return data delete this.queryListeners[queryId]; - - // if we have a polling interval running, stop it - if (this.pollingTimers[queryId]) { - clearInterval(this.pollingTimers[queryId]); - } - - this.store.dispatch({ - type: 'APOLLO_QUERY_STOP', - queryId, - }); + this.stopQueryInStore(queryId); } private fetchQueryOverInterface( diff --git a/src/scheduler.ts b/src/scheduler.ts index 60557841352..8ce86d2708a 100644 --- a/src/scheduler.ts +++ b/src/scheduler.ts @@ -31,7 +31,7 @@ export class QueryScheduler { // We use this instance to actually fire queries (i.e. send them to the batching // mechanism). - private queryManager: QueryManager; + public queryManager: QueryManager; // Map going from polling interval widths to polling timers. private pollingTimers: { [interval: number]: NodeJS.Timer | any }; // oddity in Typescript @@ -65,49 +65,43 @@ export class QueryScheduler { }); } + // The firstFetch option is used to denote whether we want to fire off a + // "first fetch" before we start polling. If startPollingQuery() is being called + // from an existing ObservableQuery, the first fetch has already been fired which + // means that firstFetch should be false. public startPollingQuery( options: WatchQueryOptions, - listener: QueryListener, - queryId?: string + queryId?: string, + firstFetch: boolean = true, + listener?: QueryListener ): string { if (!queryId) { queryId = this.queryManager.generateQueryId(); } + + this.registeredQueries[queryId] = options; + // Fire an initial fetch before we start the polling query - this.fetchQuery(queryId, options); - this.queryManager.addQueryListener(queryId, listener); + if (firstFetch) { + this.fetchQuery(queryId, options); + } + + if (listener) { + this.queryManager.addQueryListener(queryId, listener); + } this.addQueryOnInterval(queryId, options); return queryId; } public stopPollingQuery(queryId: string) { - // TODO should cancel in flight request so that there is no - // further data returned. - this.queryManager.removeQueryListener(queryId); - // Remove the query options from one of the registered queries. // The polling function will then take care of not firing it anymore. delete this.registeredQueries[queryId]; - - // Fire a APOLLO_STOP_QUERY state change to the underlying store. - this.queryManager.stopQueryInStore(queryId); - } - - // Tell the QueryScheduler to schedule the queries fired by a polling query. - public registerPollingQuery(options: WatchQueryOptions): ObservableQuery { - if (!options.pollInterval) { - throw new Error('Tried to register a non-polling query with the scheduler.'); - } - - return new ObservableQuery({ - queryManager: this.queryManager, - options: options, - }); } // Fires the all of the queries on a particular interval. Called on a setInterval. - public fireQueriesOnInterval(interval: number) { + public fetchQueriesOnInterval(interval: number) { this.intervalQueries[interval] = this.intervalQueries[interval].filter((queryId) => { // If queryOptions can't be found from registeredQueries, it means that this queryId // is no longer registered and should be removed from the list of queries firing on this @@ -148,11 +142,22 @@ export class QueryScheduler { this.intervalQueries[interval] = [queryId]; // set up the timer for the function that will handle this interval this.pollingTimers[interval] = setInterval(() => { - this.fireQueriesOnInterval(interval); + this.fetchQueriesOnInterval(interval); }, interval); } } + // Used only for unit testing. + public registerPollingQuery(queryOptions: WatchQueryOptions): ObservableQuery { + if (!queryOptions.pollInterval) { + throw new Error('Attempted to register a non-polling query with the scheduler.'); + } + return new ObservableQuery({ + scheduler: this, + options: queryOptions, + }); + } + private addInFlight(queryId: string, options: WatchQueryOptions) { this.inFlightQueries[queryId] = options; } diff --git a/test/QueryManager.ts b/test/QueryManager.ts index 2866bfcc64b..a374edcdc49 100644 --- a/test/QueryManager.ts +++ b/test/QueryManager.ts @@ -2367,8 +2367,8 @@ describe('QueryManager', () => { options: { query: query, }, - queryManager: queryManager, - } as ObservableQuery; + scheduler: queryManager.scheduler, + } as any as ObservableQuery; const queryId = 'super-fake-id'; queryManager.addObservableQuery(queryId, mockObservableQuery); @@ -2400,7 +2400,7 @@ describe('QueryManager', () => { }, options, queryManager: queryManager, - } as ObservableQuery; + } as any as ObservableQuery; const queryId = 'super-fake-id'; queryManager.addObservableQuery(queryId, mockObservableQuery); diff --git a/test/scheduler.ts b/test/scheduler.ts index 370cb5717c6..358dfbb6050 100644 --- a/test/scheduler.ts +++ b/test/scheduler.ts @@ -72,7 +72,7 @@ describe('QueryScheduler', () => { queryManager, }); let timesFired = 0; - const queryId = scheduler.startPollingQuery(queryOptions, (queryStoreValue) => { + const queryId = scheduler.startPollingQuery(queryOptions, 'fake-id', true, (queryStoreValue) => { timesFired += 1; }); setTimeout(() => { @@ -115,7 +115,7 @@ describe('QueryScheduler', () => { queryManager, }); let timesFired = 0; - let queryId = scheduler.startPollingQuery(queryOptions, (queryStoreValue) => { + let queryId = scheduler.startPollingQuery(queryOptions, 'fake-id', true, (queryStoreValue) => { timesFired += 1; scheduler.stopPollingQuery(queryId); }); @@ -252,7 +252,7 @@ describe('QueryScheduler', () => { }); }); - it.skip('should keep track of in flight queries', (done) => { + it('should keep track of in flight queries', (done) => { const query = gql` query { fortuneCookie @@ -298,7 +298,7 @@ describe('QueryScheduler', () => { }, 100); }); - it.skip('should not fire another query if one with the same id is in flight', (done) => { + it('should not fire another query if one with the same id is in flight', (done) => { const query = gql` query { fortuneCookie From 3b4e243c573884b44fd2089bb06c2e0682856517 Mon Sep 17 00:00:00 2001 From: Dhaivat Pandya Date: Fri, 15 Jul 2016 14:12:19 -0700 Subject: [PATCH 07/13] minor fixes in tests and source --- src/scheduler.ts | 4 ++++ test/scheduler.ts | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/scheduler.ts b/src/scheduler.ts index 8ce86d2708a..4d8bd978a0a 100644 --- a/src/scheduler.ts +++ b/src/scheduler.ts @@ -75,6 +75,10 @@ export class QueryScheduler { firstFetch: boolean = true, listener?: QueryListener ): string { + if (!options.pollInterval) { + throw new Error('Attempted to start a polling query without a polling interval.'); + } + if (!queryId) { queryId = this.queryManager.generateQueryId(); } diff --git a/test/scheduler.ts b/test/scheduler.ts index 358dfbb6050..dd4505e79ba 100644 --- a/test/scheduler.ts +++ b/test/scheduler.ts @@ -11,7 +11,7 @@ import mockNetworkInterface from './mocks/mockNetworkInterface'; import gql from 'graphql-tag'; describe('QueryScheduler', () => { - it('should throw an error if we try to register a non-polling query', () => { + it('should throw an error if we try to start polling a non-polling query', () => { const queryManager = new QueryManager({ networkInterface: mockNetworkInterface(), store: createApolloStore(), @@ -33,7 +33,7 @@ describe('QueryScheduler', () => { query, }; assert.throws(() => { - scheduler.registerPollingQuery(queryOptions); + scheduler.startPollingQuery(queryOptions); }); }); From d7810380976aec10fc0de6a07becb673107612f1 Mon Sep 17 00:00:00 2001 From: Dhaivat Pandya Date: Fri, 15 Jul 2016 14:22:16 -0700 Subject: [PATCH 08/13] updated changelog --- CHANGELOG.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ef8ab03dde..6ff27694fc4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,11 +4,10 @@ Expect active development and potentially significant breaking changes in the `0 ### vNEXT - Added the `batchInterval` option to ApolloClient that allows you to specify the width of the batching interval as per your app's needs. [Issue #394](https://github.com/apollostack/apollo-client/issues/394) and [PR #395](https://github.com/apollostack/apollo-client/pull/395). - - Stringify `storeObj` for error message in `diffFieldAgainstStore`. - Fix map function returning `undefined` in `removeRefsFromStoreObj`. [PR #393](https://github.com/apollostack/apollo-client/pull/393) - - Added a "noFetch" option to WatchQueryOptions that only returns available data from the local store (even it is incomplete). [Issue #225](https://github.com/apollostack/apollo-client/issues/225) and [PR #385](https://github.com/apollostack/apollo-client/pull/385). +- Integrated the scheduler so that polling queries on the same polling interval are batched together. [PR #403](https://github.com/apollostack/apollo-client/pull/403) and [Issue #401](https://github.com/apollostack/apollo-client/issues/401). ### v0.4.1 From d5b7fc4bdf27177a85394d995b4e657b3a86a370 Mon Sep 17 00:00:00 2001 From: Dhaivat Pandya Date: Tue, 19 Jul 2016 11:17:50 -0700 Subject: [PATCH 09/13] added tests and removed a dead line of code --- src/scheduler.ts | 4 ---- test/QueryManager.ts | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/src/scheduler.ts b/src/scheduler.ts index 4d8bd978a0a..524fe2a0677 100644 --- a/src/scheduler.ts +++ b/src/scheduler.ts @@ -79,10 +79,6 @@ export class QueryScheduler { throw new Error('Attempted to start a polling query without a polling interval.'); } - if (!queryId) { - queryId = this.queryManager.generateQueryId(); - } - this.registeredQueries[queryId] = options; // Fire an initial fetch before we start the polling query diff --git a/test/QueryManager.ts b/test/QueryManager.ts index a374edcdc49..ebe71723316 100644 --- a/test/QueryManager.ts +++ b/test/QueryManager.ts @@ -2686,6 +2686,46 @@ describe('QueryManager', () => { }); }); + it('should be able to unsubscribe from a polling query subscription', (done) => { + const query = gql` + query { + author { + firstName + lastName + } + }`; + const data = { + author: { + firstName: 'John', + lastName: 'Smith', + }, + }; + const networkInterface = mockNetworkInterface( + { + request: { query }, + result: { data }, + } + ); + const queryManager = new QueryManager({ + networkInterface, + store: createApolloStore(), + reduxRootKey: 'apollo', + }); + const observableQuery = queryManager.watchQuery({ query, pollInterval: 20 }); + let timesFired = 0; + const subscription = observableQuery.subscribe({ + next(result) { + timesFired += 1; + subscription.unsubscribe(); + }, + }); + + setTimeout(() => { + assert.equal(timesFired, 1); + done(); + }, 60); + }); + it('should not empty the store when a polling query fails due to a network error', (done) => { const query = gql` query { From 9318db4c103c566b7a594aeec0229527b079a11e Mon Sep 17 00:00:00 2001 From: Dhaivat Pandya Date: Tue, 19 Jul 2016 11:37:51 -0700 Subject: [PATCH 10/13] fixed the issues caused by the mergeS --- src/ObservableQuery.ts | 57 ++++++++++++++------ src/QueryManager.ts | 119 +---------------------------------------- 2 files changed, 42 insertions(+), 134 deletions(-) diff --git a/src/ObservableQuery.ts b/src/ObservableQuery.ts index 2be6f48ef31..5742d8e0e7d 100644 --- a/src/ObservableQuery.ts +++ b/src/ObservableQuery.ts @@ -1,35 +1,49 @@ -import assign = require('lodash.assign'); +import { WatchQueryOptions } from './watchQueryOptions'; import { Observable, Observer } from './util/Observable'; -import { ApolloQueryResult } from './index'; +import { + QueryScheduler, +} from './scheduler'; -import { WatchQueryOptions } from './watchQueryOptions'; +import { + QueryManager, +} from './QueryManager'; -import { QueryManager } from './QueryManager'; +import { + ApolloQueryResult, +} from './index'; + +import assign = require('lodash.assign'); export class ObservableQuery extends Observable { public refetch: (variables?: any) => Promise; public stopPolling: () => void; public startPolling: (p: number) => void; public options: WatchQueryOptions; - public queryManager: QueryManager; - public queryId: string; + private queryId: string; + private scheduler: QueryScheduler; + private queryManager: QueryManager; constructor({ - queryManager, + scheduler, options, shouldSubscribe = true, }: { - queryManager: QueryManager, + scheduler: QueryScheduler, options: WatchQueryOptions, shouldSubscribe?: boolean, }) { - + const queryManager = scheduler.queryManager; const queryId = queryManager.generateQueryId(); + const isPollingQuery = !!options.pollInterval; + const subscriberFunction = (observer: Observer) => { const retQuerySubscription = { unsubscribe: () => { + if (isPollingQuery) { + scheduler.stopPollingQuery(queryId); + } queryManager.stopQuery(queryId); }, }; @@ -39,15 +53,23 @@ export class ObservableQuery extends Observable { queryManager.addQuerySubscription(queryId, retQuerySubscription); } + if (isPollingQuery) { + this.scheduler.startPollingQuery( + options, + queryId + ); + } queryManager.startQuery( queryId, options, queryManager.queryListenerForObserver(queryId, options, observer) ); + return retQuerySubscription; }; super(subscriberFunction); this.options = options; + this.scheduler = scheduler; this.queryManager = queryManager; this.queryId = queryId; @@ -65,8 +87,9 @@ export class ObservableQuery extends Observable { }; this.stopPolling = () => { - if (this.queryManager.pollingTimers[this.queryId]) { - clearInterval(this.queryManager.pollingTimers[this.queryId]); + this.queryManager.stopQuery(this.queryId); + if (isPollingQuery) { + this.scheduler.stopPollingQuery(this.queryId); } }; @@ -74,12 +97,12 @@ export class ObservableQuery extends Observable { if (this.options.noFetch) { throw new Error('noFetch option should not use query polling.'); } - this.queryManager.pollingTimers[this.queryId] = setInterval(() => { - const pollingOptions = assign({}, this.options) as WatchQueryOptions; - // subsequent fetches from polling always reqeust new data - pollingOptions.forceFetch = true; - this.queryManager.fetchQuery(this.queryId, pollingOptions); - }, pollInterval); + + if (isPollingQuery) { + this.scheduler.stopPollingQuery(this.queryId); + } + options.pollInterval = pollInterval; + this.scheduler.startPollingQuery(this.options, this.queryId, false); }; } diff --git a/src/QueryManager.ts b/src/QueryManager.ts index b0106df6974..07b87ad0b56 100644 --- a/src/QueryManager.ts +++ b/src/QueryManager.ts @@ -4,7 +4,6 @@ import { } from './networkInterface'; import forOwn = require('lodash.forown'); -import assign = require('lodash.assign'); import isEqual = require('lodash.isequal'); import { @@ -81,123 +80,9 @@ import { ApolloError, } from './errors'; -export interface WatchQueryOptions { - query: Document; - variables?: { [key: string]: any }; - forceFetch?: boolean; - returnPartialData?: boolean; - noFetch?: boolean; - pollInterval?: number; - fragments?: FragmentDefinition[]; -} - -export class ObservableQuery extends Observable { - public refetch: (variables?: any) => Promise; - public stopPolling: () => void; - public startPolling: (p: number) => void; - public options: WatchQueryOptions; - private queryId: string; - private scheduler: QueryScheduler; - private queryManager: QueryManager; - - constructor({ - scheduler, - options, - shouldSubscribe = true, - }: { - scheduler: QueryScheduler, - options: WatchQueryOptions, - shouldSubscribe?: boolean, - }) { - const queryManager = scheduler.queryManager; - const queryId = queryManager.generateQueryId(); - const isPollingQuery = !!options.pollInterval; - - const subscriberFunction = (observer: Observer) => { - const retQuerySubscription = { - unsubscribe: () => { - if (isPollingQuery) { - scheduler.stopPollingQuery(queryId); - } - queryManager.stopQuery(queryId); - }, - }; - - if (shouldSubscribe) { - queryManager.addObservableQuery(queryId, this); - queryManager.addQuerySubscription(queryId, retQuerySubscription); - } - - if (isPollingQuery) { - this.scheduler.startPollingQuery( - options, - queryId - ); - } - queryManager.startQuery( - queryId, - options, - queryManager.queryListenerForObserver(options, observer) - ); - - return retQuerySubscription; - }; - super(subscriberFunction); - this.options = options; - this.scheduler = scheduler; - this.queryManager = queryManager; - this.queryId = queryId; - - this.refetch = (variables?: any) => { - // If no new variables passed, use existing variables - variables = variables || this.options.variables; - if (this.options.noFetch) { - throw new Error('noFetch option should not use query refetch.'); - } - // Use the same options as before, but with new variables and forceFetch true - return this.queryManager.fetchQuery(this.queryId, assign(this.options, { - forceFetch: true, - variables, - }) as WatchQueryOptions); - }; - - this.stopPolling = () => { - this.queryManager.stopQuery(this.queryId); - if (isPollingQuery) { - this.scheduler.stopPollingQuery(this.queryId); - } - }; - - this.startPolling = (pollInterval) => { - if (this.options.noFetch) { - throw new Error('noFetch option should not use query polling.'); - } - - if (isPollingQuery) { - this.scheduler.stopPollingQuery(this.queryId); - } - options.pollInterval = pollInterval; - this.scheduler.startPollingQuery(this.options, this.queryId, false); - }; - } +import { WatchQueryOptions } from './watchQueryOptions'; - public result(): Promise { - return new Promise((resolve, reject) => { - const subscription = this.subscribe({ - next(result) { - resolve(result); - setTimeout(() => { - subscription.unsubscribe(); - }, 0); - }, - error(error) { - reject(error); - }, - }); - }); - } - -} +import { ObservableQuery } from './ObservableQuery'; export type QueryListener = (queryStoreValue: QueryStoreValue) => void; From ad328d1106bb5e1b1defdecb2d30264c50a036eb Mon Sep 17 00:00:00 2001 From: Dhaivat Pandya Date: Tue, 19 Jul 2016 11:52:07 -0700 Subject: [PATCH 11/13] another merge fix --- CHANGELOG.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d8d18879ac..e8035ea69eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,13 +3,11 @@ Expect active development and potentially significant breaking changes in the `0.x` track. We'll try to be diligent about releasing a `1.0` version in a timely fashion (ideally within 3 to 6 months), to signal the start of a more stable API. ### vNEXT -<<<<<<< HEAD - Added the `batchInterval` option to ApolloClient that allows you to specify the width of the batching interval as per your app's needs. [Issue #394](https://github.com/apollostack/apollo-client/issues/394) and [PR #395](https://github.com/apollostack/apollo-client/pull/395). - Stringify `storeObj` for error message in `diffFieldAgainstStore`. - Fix map function returning `undefined` in `removeRefsFromStoreObj`. [PR #393](https://github.com/apollostack/apollo-client/pull/393) - Added a "noFetch" option to WatchQueryOptions that only returns available data from the local store (even it is incomplete). [Issue #225](https://github.com/apollostack/apollo-client/issues/225) and [PR #385](https://github.com/apollostack/apollo-client/pull/385). - Integrated the scheduler so that polling queries on the same polling interval are batched together. [PR #403](https://github.com/apollostack/apollo-client/pull/403) and [Issue #401](https://github.com/apollostack/apollo-client/issues/401). -======= ### v0.4.4 From 9596a0396d1ecc1272eae06d9016cb0bef2c504b Mon Sep 17 00:00:00 2001 From: Dhaivat Pandya Date: Tue, 19 Jul 2016 12:03:02 -0700 Subject: [PATCH 12/13] fixed the changelog again --- CHANGELOG.md | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8035ea69eb..ff003bb1df4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,11 +3,7 @@ Expect active development and potentially significant breaking changes in the `0.x` track. We'll try to be diligent about releasing a `1.0` version in a timely fashion (ideally within 3 to 6 months), to signal the start of a more stable API. ### vNEXT -- Added the `batchInterval` option to ApolloClient that allows you to specify the width of the batching interval as per your app's needs. [Issue #394](https://github.com/apollostack/apollo-client/issues/394) and [PR #395](https://github.com/apollostack/apollo-client/pull/395). -- Stringify `storeObj` for error message in `diffFieldAgainstStore`. -- Fix map function returning `undefined` in `removeRefsFromStoreObj`. [PR #393](https://github.com/apollostack/apollo-client/pull/393) -- Added a "noFetch" option to WatchQueryOptions that only returns available data from the local store (even it is incomplete). [Issue #225](https://github.com/apollostack/apollo-client/issues/225) and [PR #385](https://github.com/apollostack/apollo-client/pull/385). -- Integrated the scheduler so that polling queries on the same polling interval are batched together. [PR #403](https://github.com/apollostack/apollo-client/pull/403) and [Issue #401](https://github.com/apollostack/apollo-client/issues/401). +- - Integrated the scheduler so that polling queries on the same polling interval are batched together. [PR #403](https://github.com/apollostack/apollo-client/pull/403) and [Issue #401](https://github.com/apollostack/apollo-client/issues/401). ### v0.4.4 @@ -25,7 +21,6 @@ Expect active development and potentially significant breaking changes in the `0 - Fix map function returning `undefined` in `removeRefsFromStoreObj`. [PR #393](https://github.com/apollostack/apollo-client/pull/393) - Added deep result comparison so that observers are only fired when the data associated with a particular query changes. This change eliminates unnecessary re-renders and improves UI performance. [PR #402](https://github.com/apollostack/apollo-client/pull/402) and [Issue #400](https://github.com/apollostack/apollo-client/issues/400). - Added a "noFetch" option to WatchQueryOptions that only returns available data from the local store (even it is incomplete). The `ObservableQuery` returned from calling `watchQuery` now has `options`, `queryManager`, and `queryId`. The `queryId` can be used to read directly from the state of `apollo.queries`. [Issue #225](https://github.com/apollostack/apollo-client/issues/225), [Issue #342](https://github.com/apollostack/apollo-client/issues/342), and [PR #385](https://github.com/apollostack/apollo-client/pull/385). ->>>>>>> 378cbbe6054d0bae2b4ede2387d19dc2d1927567 ### v0.4.1 From 7ca776493b9e3f01dd1159bf969d38c4d4a8894f Mon Sep 17 00:00:00 2001 From: Dhaivat Pandya Date: Wed, 20 Jul 2016 14:41:03 -0700 Subject: [PATCH 13/13] added the noFetch error --- src/ObservableQuery.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/ObservableQuery.ts b/src/ObservableQuery.ts index 5742d8e0e7d..12582cf2518 100644 --- a/src/ObservableQuery.ts +++ b/src/ObservableQuery.ts @@ -54,6 +54,10 @@ export class ObservableQuery extends Observable { } if (isPollingQuery) { + if (options.noFetch) { + throw new Error('noFetch option should not use query polling.'); + } + this.scheduler.startPollingQuery( options, queryId