🎨(query-subscription): Update query subscription

Summary:
- Applies code changes that we discussed with @bengotow when debugging
  #1327
- Minor refactoring and reorganization + update specs

Test Plan: - Unit tests

Reviewers: evan, bengotow

Reviewed By: bengotow

Subscribers: bengotow

Differential Revision: https://phab.nylas.com/D2612
This commit is contained in:
Juan Tejada 2016-02-21 00:28:47 -08:00
parent 2d279eeb05
commit be6242a44c
2 changed files with 79 additions and 60 deletions

View file

@ -208,16 +208,16 @@ describe "QuerySubscription", ->
describe "update", ->
beforeEach ->
spyOn(QuerySubscription.prototype, '_fetchRange').andCallFake ->
spyOn(QuerySubscription.prototype, '_fetchMissingRange').andCallFake ->
@_set ?= new MutableQueryResultSet()
Promise.resolve()
describe "when the query has an infinite range", ->
it "should call _fetchRange for the entire range", ->
it "should call _fetchMissingRange for the entire range", ->
subscription = new QuerySubscription(DatabaseStore.findAll(Thread))
subscription.update()
advanceClock()
expect(subscription._fetchRange).toHaveBeenCalledWith(QueryRange.infinite(), {entireModels: true, version: 1})
expect(subscription._fetchMissingRange).toHaveBeenCalledWith(QueryRange.infinite(), {fetchEntireModels: true, version: 1})
it "should fetch full full models only when the previous set is empty", ->
subscription = new QuerySubscription(DatabaseStore.findAll(Thread))
@ -225,35 +225,51 @@ describe "QuerySubscription", ->
subscription._set.addModelsInRange([new Thread()], new QueryRange(start: 0, end: 1))
subscription.update()
advanceClock()
expect(subscription._fetchRange).toHaveBeenCalledWith(QueryRange.infinite(), {entireModels: false, version: 1})
expect(subscription._fetchMissingRange).toHaveBeenCalledWith(QueryRange.infinite(), {fetchEntireModels: false, version: 1})
describe "when the query has a range", ->
beforeEach ->
@query = DatabaseStore.findAll(Thread).limit(10)
describe "when we have no current range", ->
it "should call _fetchRange for the entire range and fetch full models", ->
it "should call _fetchMissingRange for the entire range and fetch full models", ->
subscription = new QuerySubscription(@query)
subscription._set = null
subscription.update()
advanceClock()
expect(subscription._fetchRange).toHaveBeenCalledWith(@query.range(), {entireModels: true, version: 1})
expect(subscription._fetchMissingRange).toHaveBeenCalledWith(@query.range(), {fetchEntireModels: true, version: 1})
describe "when we have a previous range", ->
it "should call _fetchRange for the ranges representing the difference", ->
customRange1 = jasmine.createSpy('customRange1')
customRange2 = jasmine.createSpy('customRange2')
spyOn(QueryRange, 'rangesBySubtracting').andReturn [customRange1, customRange2]
it "should call _fetchMissingRange with the missingRange", ->
customRange = jasmine.createSpy('customRange1')
spyOn(QueryRange, 'rangesBySubtracting').andReturn [customRange]
subscription = new QuerySubscription(@query)
subscription._set = new MutableQueryResultSet()
subscription._set.addModelsInRange([new Thread()], new QueryRange(start: 0, end: 1))
advanceClock()
subscription._fetchRange.reset()
subscription._fetchMissingRange.reset()
subscription._updateInFlight = false
subscription.update()
advanceClock()
expect(subscription._fetchRange.callCount).toBe(2)
expect(subscription._fetchRange.calls[0].args).toEqual([customRange1, {entireModels: true, version: 1}])
expect(subscription._fetchRange.calls[1].args).toEqual([customRange2, {entireModels: true, version: 1}])
expect(subscription._fetchMissingRange.callCount).toBe(1)
expect(subscription._fetchMissingRange.calls[0].args).toEqual([customRange, {fetchEntireModels: true, version: 1}])
it "should call _fetchMissingRange for the entire query range when the missing range encompasses more than one range", ->
customRange1 = jasmine.createSpy('customRange1')
customRange2 = jasmine.createSpy('customRange2')
spyOn(QueryRange, 'rangesBySubtracting').andReturn [customRange1, customRange2]
range = new QueryRange(start: 0, end: 1)
subscription = new QuerySubscription(@query)
subscription._set = new MutableQueryResultSet()
subscription._set.addModelsInRange([new Thread()], range)
advanceClock()
subscription._fetchMissingRange.reset()
subscription._updateInFlight = false
subscription.update()
advanceClock()
expect(subscription._fetchMissingRange.callCount).toBe(1)
expect(subscription._fetchMissingRange.calls[0].args).toEqual([@query.range(), {fetchEntireModels: true, version: 1}])

View file

@ -59,6 +59,10 @@ class QuerySubscription
@_queuedChangeRecords.push(record)
@_processChangeRecords() unless @_updateInFlight
cancelPendingUpdate: =>
@_queryVersion += 1
@_updateInFlight = false
# Scan through change records and apply them to the last result set.
# - Returns true if changes did / will result in new result set being created.
# - Returns false if no changes were made.
@ -122,7 +126,7 @@ class QuerySubscription
return true
else if knownImpacts > 0
@_createResultAndTrigger()
return true
return false
else
return false
@ -138,71 +142,70 @@ class QuerySubscription
return false
update: =>
desiredRange = @_query.range()
currentRange = @_set?.range()
@_updateInFlight = true
version = @_queryVersion
desiredRange = @_query.range()
currentRange = @_set?.range()
areNotInfinite = currentRange and not currentRange.isInfinite() and not desiredRange.isInfinite()
previousResultIsEmpty = not @_set or @_set.modelCacheCount() is 0
missingRange = @_getMissingRange(desiredRange, currentRange)
fetchEntireModels = if areNotInfinite then true else previousResultIsEmpty
@_fetchMissingRange(missingRange, {version, fetchEntireModels})
_getMissingRange: (desiredRange, currentRange) =>
if currentRange and not currentRange.isInfinite() and not desiredRange.isInfinite()
ranges = QueryRange.rangesBySubtracting(desiredRange, currentRange)
entireModels = true
missingRange = if ranges.length > 1 then desiredRange else ranges[0]
else
ranges = [desiredRange]
entireModels = not @_set or @_set.modelCacheCount() is 0
missingRange = desiredRange
return missingRange
Promise.each ranges, (range) =>
return unless @_queryVersion is version
@_fetchRange(range, {entireModels, version})
.then =>
return unless @_queryVersion is version
ids = @_set.ids().filter (id) => not @_set.modelWithId(id)
return if ids.length is 0
return DatabaseStore.findAll(@_query._klass, {id: ids}).then (models) =>
return unless @_queryVersion is version
@_set.replaceModel(m) for m in models
.then =>
return unless @_queryVersion is version
@_updateInFlight = false
# Trigger if A) no changes came in during the update, or B) applying
# those changes has no effect on the result set, and this one is
# still good.
if @_queuedChangeRecords.length is 0 or not @_processChangeRecords()
@_createResultAndTrigger()
cancelPendingUpdate: =>
@_queryVersion += 1
@_updateInFlight = false
_fetchRange: (range, {entireModels, version} = {}) ->
rangeQuery = undefined
unless range.isInfinite()
_getQueryForRange: (range, fetchEntireModels) =>
rangeQuery = null
if not range.isInfinite()
rangeQuery ?= @_query.clone()
rangeQuery.offset(range.offset).limit(range.limit)
unless entireModels
if not fetchEntireModels
rangeQuery ?= @_query.clone()
rangeQuery.idsOnly()
rangeQuery ?= @_query
return rangeQuery
DatabaseStore.run(rangeQuery, {format: false}).then (results) =>
_fetchMissingRange: (missingRange, {version, fetchEntireModels}) ->
missingRangeQuery = @_getQueryForRange(missingRange, fetchEntireModels)
DatabaseStore.run(missingRangeQuery, {format: false})
.then (results) =>
return unless @_queryVersion is version
if @_set and not @_set.range().isContiguousWith(range)
if @_set and not @_set.range().isContiguousWith(missingRange)
@_set = null
@_set ?= new MutableQueryResultSet()
if entireModels
@_set.addModelsInRange(results, range)
else
@_set.addIdsInRange(results, range)
# Create result and trigger
# if A) no changes have come in during querying the missing range,
# or B) applying those changes has no effect on the result set, and this one is
# still good.
if @_queuedChangeRecords.length is 0 or @_processChangeRecords() is false
if fetchEntireModels
@_set.addModelsInRange(results, missingRange)
else
@_set.addIdsInRange(results, missingRange)
@_set.clipToRange(@_query.range())
@_set.clipToRange(@_query.range())
ids = @_set.ids().filter (id) => not @_set.modelWithId(id)
if ids.length > 0
return DatabaseStore.findAll(@_query._klass, {id: ids}).then (models) =>
return unless @_queryVersion is version
@_set.replaceModel(m) for m in models
@_updateInFlight = false
@_createResultAndTrigger()
else
@_updateInFlight = false
@_createResultAndTrigger()
_createResultAndTrigger: =>
allCompleteModels = @_set.isComplete()