diff --git a/spec/model-view-selection-spec.coffee b/spec/list-selection-spec.coffee similarity index 88% rename from spec/model-view-selection-spec.coffee rename to spec/list-selection-spec.coffee index 1c1b8ca51..bb959a96c 100644 --- a/spec/model-view-selection-spec.coffee +++ b/spec/list-selection-spec.coffee @@ -54,35 +54,44 @@ describe "ListSelection", -> describe "remove", -> beforeEach -> - @selection.set([@items[2], @items[4]]) + @selection.set([@items[2], @items[4], @items[7]]) it "should do nothing if called without a valid item", -> @selection.remove(null) @selection.remove(undefined) @selection.remove(false) - expect(@selection.ids()).toEqual(['2', '4']) + expect(@selection.ids()).toEqual(['2', '4', '7']) it "should remove the item from the set", -> @selection.remove(@items[2]) - expect(@selection.ids()).toEqual(['4']) + expect(@selection.ids()).toEqual(['4', '7']) - it "should throw an exception if the item passed is not a model", -> + it "should throw an exception if any item passed is not a model", -> expect( => @selection.remove('hi')).toThrow() + it "should accept an array of models as well as a single item", -> + @selection.remove([@items[2], @items[4]]) + expect(@selection.ids()).toEqual(['7']) + it "should trigger", -> @selection.remove() expect(@trigger).toHaveBeenCalled() - describe "updateModelReferences", -> + describe "_applyChangeRecord", -> it "should replace items in the selection with the matching provided items, if present", -> @selection.set([@items[2], @items[4], @items[7]]) expect(@selection.items()[0]).toBe(@items[2]) - expect(@selection.items()[0].subject).toBe(undefined) newItem2 = new Thread(id: '2', subject:'Hello world!') - @selection.updateModelReferences([newItem2]) + @selection._applyChangeRecord({objectClass: 'Thread', objects: [newItem2], type: 'persist'}) expect(@selection.items()[0].subject).toBe('Hello world!') + it "should rremove items in the selection if type is unpersist", -> + @selection.set([@items[2], @items[4], @items[7]]) + newItem2 = new Thread(id: '2', subject:'Hello world!') + @selection._applyChangeRecord({objectClass: 'Thread', objects: [newItem2], type: 'unpersist'}) + expect(@selection.ids()).toEqual(['4', '7']) + describe "toggle", -> beforeEach -> @selection.set([@items[2]]) diff --git a/spec/models/query-subscription-spec.coffee b/spec/models/query-subscription-spec.coffee index 429797572..9c5443cf6 100644 --- a/spec/models/query-subscription-spec.coffee +++ b/spec/models/query-subscription-spec.coffee @@ -96,14 +96,14 @@ describe "QuerySubscription", -> mustUpdate: true mustRefetchAllIds: true },{ - name: 'Item saved - does not match query clauses' + name: 'Item saved - does not match query clauses, offset > 0' change: objectClass: Thread.name objects: [new Thread(accountId: 'b', clientId: '5', lastMessageReceivedTimestamp: 5)] type: 'persist' nextModels: 'unchanged' - mustUpdate: false - mustRefetchAllIds: false + mustUpdate: true + mustRefetchAllIds: true },{ name: 'Item saved - matches query clauses' change: diff --git a/src/flux/attributes/matcher.coffee b/src/flux/attributes/matcher.coffee index 0a501c52a..05523f02c 100644 --- a/src/flux/attributes/matcher.coffee +++ b/src/flux/attributes/matcher.coffee @@ -46,28 +46,33 @@ class Matcher @val evaluate: (model) -> - value = model[@attr.modelKey] - value = value() if value instanceof Function + modelValue = model[@attr.modelKey] + modelValue = modelValue() if modelValue instanceof Function + matcherValue = @val + + # Given an array of strings or models, and a string or model search value, + # will find if a match exists. + modelArrayContainsValue = (array, searchItem) -> + asId = (v) -> if v and v.id then v.id else v + search = asId(searchItem) + for item in array + return true if asId(item) == search + return false switch @comparator - when '=' then return value == @val - when '<' then return value < @val - when '>' then return value > @val - when 'in' then return value in @val + when '=' then return modelValue == matcherValue + when '<' then return modelValue < matcherValue + when '>' then return modelValue > matcherValue + when 'in' then return modelValue in matcherValue when 'contains' - # You can provide an ID or an object, and an array of IDs or an array of objects - # Assumes that `value` is an array of items - !!_.find value, (x) => - @val == x?.id || @val == x || @val?.id == x || @val?.id == x?.id - when 'containsAny' - # You can provide an ID or an object, and an array of IDs or an array of objects - # Assumes that `value` is an array of items - _.some @val, (subvalue) => - !!_.find value, (x) => - subvalue == x?.id || subvalue == x || subvalue?.id == x || subvalue?.id == x?.id + !!modelArrayContainsValue(modelValue, matcherValue) - when 'startsWith' then return value.startsWith(@val) - when 'like' then value.search(new RegExp(".*#{@val}.*", "gi")) >= 0 + when 'containsAny' + _.any matcherValue, (submatcherValue) -> + !!modelArrayContainsValue(modelValue, submatcherValue) + + when 'startsWith' then return modelValue.startsWith(matcherValue) + when 'like' then modelValue.search(new RegExp(".*#{matcherValue}.*", "gi")) >= 0 else throw new Error("Matcher.evaulate() not sure how to evaluate @{@attr.modelKey} with comparator #{@comparator}") diff --git a/src/flux/models/mutable-query-result-set.coffee b/src/flux/models/mutable-query-result-set.coffee index 022686b4f..e47d7a94e 100644 --- a/src/flux/models/mutable-query-result-set.coffee +++ b/src/flux/models/mutable-query-result-set.coffee @@ -9,6 +9,7 @@ class MutableQueryResultSet extends QueryResultSet set = new QueryResultSet({ _ids: [].concat(@_ids) _modelsHash: _.extend({}, @_modelsHash) + _query: @_query _offset: @_offset }) Object.freeze(set) @@ -70,4 +71,8 @@ class MutableQueryResultSet extends QueryResultSet delete @_modelsHash[item.id] @_ids.splice(idx, 1) + setQuery: (query) -> + @_query = query.clone() + @_query.finalize() + module.exports = MutableQueryResultSet diff --git a/src/flux/models/query-result-set.coffee b/src/flux/models/query-result-set.coffee index a8a71da15..575c3f37d 100644 --- a/src/flux/models/query-result-set.coffee +++ b/src/flux/models/query-result-set.coffee @@ -36,12 +36,14 @@ class QueryResultSet constructor: (other = {}) -> @_modelsHash = other._modelsHash ? {} @_offset = other._offset ? null + @_query = other._query ? null @_ids = other._ids ? [] clone: -> new @constructor({ _ids: [].concat(@_ids) _modelsHash: _.extend({}, @_modelsHash) + _query: @_query _offset: @_offset }) @@ -51,6 +53,9 @@ class QueryResultSet range: -> new QueryRange(offset: @_offset, limit: @_ids.length) + query: -> + @_query + count: -> @_ids.length diff --git a/src/flux/models/query-subscription.coffee b/src/flux/models/query-subscription.coffee index d02216ca8..9e2e37116 100644 --- a/src/flux/models/query-subscription.coffee +++ b/src/flux/models/query-subscription.coffee @@ -89,6 +89,13 @@ class QuerySubscription impactCount += 1 mustRefetchAllIds = true if @_itemSortOrderHasChanged(oldItem, item) + # If we're not at the top of the result set, we can't be sure whether an + # item previously matched the set and doesn't anymore, impacting the items + # in the query range. We need to refetch IDs to be sure our set is correct. + if @_query.range().offset > 0 and impactCount < record.objects.length + impactCount += 1 + mustRefetchAllIds = true + if impactCount > 0 if mustRefetchAllIds @log("Clearing result set - mustRefetchAllIds") @@ -179,6 +186,7 @@ class QuerySubscription throw new Error("QuerySubscription: result set contains duplicate ids.") if @_options.asResultSet + @_set.setQuery(@_query) @_lastResult = @_set.immutableClone() else @_lastResult = @_query.formatResult(@_set.models()) diff --git a/src/flux/models/query.coffee b/src/flux/models/query.coffee index 0802ca88b..f46d07b6b 100644 --- a/src/flux/models/query.coffee +++ b/src/flux/models/query.coffee @@ -294,11 +294,15 @@ class ModelQuery # Private: Marks the object as final, preventing any changes to the where # clauses, orders, etc. finalize: -> + return if @_finalized + if @_orders.length is 0 natural = @_klass.naturalSortOrder() @_orders.push(natural) if natural + if @_returnOne and not @_range.limit @limit(1) + @_finalized = true @ diff --git a/src/flux/modules/reflux-coffee.coffee b/src/flux/modules/reflux-coffee.coffee index f33e696e6..6ac4c04d9 100644 --- a/src/flux/modules/reflux-coffee.coffee +++ b/src/flux/modules/reflux-coffee.coffee @@ -151,7 +151,7 @@ module.exports = setupEmitter: -> return if @_emitter @_emitter ?= new EventEmitter() - @_emitter.setMaxListeners(50) + @_emitter.setMaxListeners(20) listen: (callback, bindContext) -> if not callback diff --git a/src/flux/stores/database-store.coffee b/src/flux/stores/database-store.coffee index 285bb919b..be497d268 100644 --- a/src/flux/stores/database-store.coffee +++ b/src/flux/stores/database-store.coffee @@ -89,6 +89,9 @@ class DatabaseStore extends NylasStore @_open = false @_waiting = [] + @setupEmitter() + @_emitter.setMaxListeners(100) + if NylasEnv.inSpecMode() @_databasePath = path.join(NylasEnv.getConfigDirPath(),'edgehill.test.db') else diff --git a/src/flux/stores/list-data-source.coffee b/src/flux/stores/list-data-source.coffee index 87c6022ae..44d33614a 100644 --- a/src/flux/stores/list-data-source.coffee +++ b/src/flux/stores/list-data-source.coffee @@ -7,6 +7,7 @@ class ListDataSource constructor: -> @_emitter = new EventEmitter() + @_cleanedup = false @selection = new ListSelection(@, @trigger) @ @@ -16,10 +17,18 @@ class ListDataSource @_emitter.emit('trigger', arg) listen: (callback, bindContext) -> + if @_cleanedup is true + throw new Error("ListDataSource: You cannot listen again after removing the last listener. This is an implementation detail.") + eventHandler = -> callback.apply(bindContext, arguments) @_emitter.addListener('trigger', eventHandler) - return => @_emitter.removeListener('trigger', eventHandler) + + return => + @_emitter.removeListener('trigger', eventHandler) + if @_emitter.listenerCount('trigger') is 0 + @_cleanedup = true + @cleanup() loaded: -> throw new Error("ListDataSource base class does not implement loaded()") @@ -44,3 +53,6 @@ class ListDataSource setRetainedRange: ({start, end}) -> throw new Error("ListDataSource base class does not implement setRetainedRange()") + + cleanup: -> + @selection.cleanup() diff --git a/src/flux/stores/list-selection.coffee b/src/flux/stores/list-selection.coffee index 129504e73..d00049f70 100644 --- a/src/flux/stores/list-selection.coffee +++ b/src/flux/stores/list-selection.coffee @@ -1,4 +1,5 @@ Model = require '../models/model' +DatabaseStore = require './database-store' _ = require 'underscore' module.exports = @@ -6,8 +7,12 @@ class ListSelection constructor: (@_view, @trigger) -> throw new Error("new ListSelection(): You must provide a view.") unless @_view + @_unlisten = DatabaseStore.listen(@_applyChangeRecord, @) @_items = [] + cleanup: -> + @_unlisten() + count: -> @_items.length @@ -29,17 +34,6 @@ class ListSelection @_items.push(item) @trigger(@) - updateModelReferences: (items = []) -> - for newer in items - unless newer instanceof Model - console.error(JSON.stringify(newer)) - throw new Error("updateModelReferences must be called with Models") - - for existing, idx in @_items - if existing.id is newer.id - @_items[idx] = newer - break - toggle: (item) -> return unless item throw new Error("toggle must be called with a Model") unless item instanceof Model @@ -61,11 +55,18 @@ class ListSelection @_items = updated @trigger(@) - remove: (item) -> - return unless item - throw new Error("remove must be called with a Model") unless item instanceof Model + remove: (items) -> + return unless items - without = _.reject @_items, (t) -> t.id is item.id + items = [items] unless items instanceof Array + + for item in items + unless item instanceof Model + throw new Error("remove: Must be passed a model or array of models") + + itemIds = _.pluck(items, 'id') + + without = _.reject @_items, (t) -> t.id in itemIds if without.length < @_items.length @_items = without @trigger(@) @@ -128,3 +129,20 @@ class ListSelection @_items.push(next) @trigger() + + _applyChangeRecord: (change) -> + return if @_items.length is 0 + return if change.objectClass isnt @_items[0].constructor.name + + if change.type is 'unpersist' + @remove(change.objects) + else if change.type is 'persist' + touched = 0 + for newer in change.objects + for existing, idx in @_items + if existing.id is newer.id + @_items[idx] = newer + touched += 1 + break + if touched > 0 + @trigger(@) diff --git a/src/flux/stores/observable-list-data-source.coffee b/src/flux/stores/observable-list-data-source.coffee index 23ca26b87..6fb32988a 100644 --- a/src/flux/stores/observable-list-data-source.coffee +++ b/src/flux/stores/observable-list-data-source.coffee @@ -22,7 +22,7 @@ class ObservableListDataSource extends ListDataSource @_resultSet = null @_resultDesiredLast = null - $resultSetObservable.subscribe (nextResultSet) => + @_subscription = $resultSetObservable.subscribe (nextResultSet) => if nextResultSet.range().end is @_resultDesiredLast @_countEstimate = Math.max(@_countEstimate, nextResultSet.range().end + 1) else @@ -31,7 +31,12 @@ class ObservableListDataSource extends ListDataSource previousResultSet = @_resultSet @_resultSet = nextResultSet - @selection.updateModelReferences(@_resultSet.models()) + # If the result set is derived from a query, remove any items in the selection + # that do not match the query. This ensures that items "removed from the view" + # are removed from the selection. + query = nextResultSet.query() + @selection.removeItemsNotMatching(query.matchers()) if query + @trigger({previous: previousResultSet, next: nextResultSet}) setRetainedRange: ({start, end}) -> @@ -64,5 +69,9 @@ class ObservableListDataSource extends ListDataSource return [] unless @_resultSet @_resultSet.models().filter(matchFn) + cleanup: -> + @_subscription?.dispose() + super + module.exports = ObservableListDataSource