From 83fb0ebb2c86dd58e58ec5396d0dc0e886a3f967 Mon Sep 17 00:00:00 2001 From: Piotr Sarnacki Date: Thu, 21 Feb 2013 02:05:13 +0100 Subject: [PATCH] Fix incomplete implementation --- assets/scripts/app/pusher.coffee | 1 - assets/scripts/app/routes.coffee | 2 +- assets/scripts/app/store.coffee | 109 +++++++++--------- assets/scripts/app/store/rest_adapter.coffee | 45 ++++++++ assets/scripts/lib/travis/model.coffee | 50 +++++--- .../scripts/spec/unit/incomplete_spec.coffee | 76 +++++++----- assets/scripts/spec/unit/merge_spec.coffee | 105 +++++++++++++++++ assets/scripts/travis.coffee | 5 +- public/index.html | 1 + public/spec.html | 4 +- 10 files changed, 288 insertions(+), 110 deletions(-) create mode 100644 assets/scripts/spec/unit/merge_spec.coffee diff --git a/assets/scripts/app/pusher.coffee b/assets/scripts/app/pusher.coffee index bd4a3e22..eeb46b57 100644 --- a/assets/scripts/app/pusher.coffee +++ b/assets/scripts/app/pusher.coffee @@ -35,7 +35,6 @@ $.extend Travis.Pusher.prototype, "#{Travis.Pusher.CHANNEL_PREFIX}#{channel}" receive: (event, data) -> - return return if event.substr(0, 6) == 'pusher' data = @normalize(event, data) if data.id diff --git a/assets/scripts/app/routes.coffee b/assets/scripts/app/routes.coffee index ac218a49..54be2f3b 100644 --- a/assets/scripts/app/routes.coffee +++ b/assets/scripts/app/routes.coffee @@ -374,7 +374,7 @@ Travis.OldRouter = Ember.Object.extend job.addObserver('log.id', observer) Ember.Router.reopen - location: Ember.HistoryLocation.create() + location: (if testMode? then Ember.HashLocation.create() else Ember.HistoryLocation.create()) Travis.Router.map -> @resource 'index', path: '/', -> diff --git a/assets/scripts/app/store.coffee b/assets/scripts/app/store.coffee index 64f836ea..e3ec1074 100644 --- a/assets/scripts/app/store.coffee +++ b/assets/scripts/app/store.coffee @@ -1,8 +1,6 @@ require 'store/rest_adapter' -DATA_PROXY = - get: (name) -> - @savedData[name] +coerceId = (id) -> if id == null then null else id+'' Travis.Store = DS.Store.extend revision: 11 @@ -11,20 +9,15 @@ Travis.Store = DS.Store.extend init: -> @_super.apply this, arguments @_loadedData = {} + @clientIdToComplete = {} - load: (type, id, hash) -> + load: (type, data, prematerialized) -> result = @_super.apply this, arguments - if result && result.clientId + if result && result.clientId && @clientIdToComplete[result.clientId] == undefined # I assume that everything that goes through load is complete record # representation, incomplete hashes from pusher go through merge() - record = @findByClientId type, result.clientId - record.set 'incomplete', false - record.set 'complete', true - # setting both incomplete and complete may be weird, but it's easier to - # work with both values. I need to check if record has already been completed - # and in order to do that, without having 'complete', I would need to check - # for incomplete == false, which looks worse + @clientIdToComplete[result.clientId] = true result @@ -34,38 +27,26 @@ Travis.Store = DS.Store.extend array.set('isLoaded', true) for array in @typeMapFor(type).recordArrays result - merge: (type, id, hash) -> - if hash == undefined - hash = id - primaryKey = type.proto().primaryKey - Ember.assert("A data hash was loaded for a record of type " + type.toString() + " but no primary key '" + primaryKey + "' was provided.", hash[primaryKey]) - id = hash[primaryKey] + merge: (type, data, incomplete) -> + id = coerceId data.id - typeMap = @typeMapFor(type) - dataCache = typeMap.cidToHash - clientId = typeMap.idToCid[id] - recordCache = @get('recordCache') - - if clientId != undefined - if (data = dataCache[clientId]) && (typeof data == 'object') - for key, value of hash - if ( descriptor = Object.getOwnPropertyDescriptor(data, key) ) && descriptor.set - Ember.set(data, key, value) - else - data[key] = value - else - dataCache[clientId] = hash - - if record = recordCache[clientId] - record.send('didChangeData') + typeMap = @typeMapFor(type) + clientId = typeMap.idToCid[id] + record = @recordCache[clientId] + if record + @get('adapter').merge(this, record, data) else - clientId = @pushHash(hash, id, type) + if (savedData = @clientIdToData[clientId]) && savedData.id? + $.extend(savedData, data) + else + result = @load(type, data, {id: data.id}) - if clientId - DATA_PROXY.savedData = hash - @updateRecordArrays(type, clientId, DATA_PROXY) + if result && result.clientId + clientId = result.clientId + if incomplete + @clientIdToComplete[result.clientId] = false - { id: id, clientId: clientId } + { clientId: clientId, id: id } isInStore: (type, id) -> !!@typeMapFor(type).idToCid[id] @@ -77,6 +58,7 @@ Travis.Store = DS.Store.extend mappings = @adapter.get('mappings') type = mappings[name] + if event == 'build:started' && data.build.commit # TODO: commit should be a sideload record on build, not mixed with it build = data.build @@ -114,14 +96,29 @@ Travis.Store = DS.Store.extend if type == Travis.Build && (json.repository || json.repo) @loadIncomplete(Travis.Repo, json.repository || json.repo) - @loadIncomplete(type, json[root]) + result = @loadIncomplete(type, json[root]) + if result.id + @find(type, result.id) addLoadedData: (type, clientId, hash) -> id = hash.id @_loadedData[type.toString()] ||= {} loadedData = (@_loadedData[type][clientId] ||= []) - for key of hash - loadedData.pushObject key unless loadedData.contains(key) + + serializer = @get('adapter.serializer') + + Ember.get(type, 'attributes').forEach( (name, meta) -> + value = @extractAttribute(type, hash, name) + if value != undefined + loadedData.pushObject name unless loadedData.contains(name) + , serializer) + + Ember.get(type, 'relationshipsByName').forEach( (name, relationship) -> + key = @_keyForBelongsTo(type, relationship.key) + value = @extractBelongsTo(type, hash, key) + if value != undefined + loadedData.pushObject name unless loadedData.contains(name) + , serializer) isDataLoadedFor: (type, clientId, key) -> if recordsData = @_loadedData[type.toString()] @@ -131,26 +128,32 @@ Travis.Store = DS.Store.extend loadIncomplete: (type, hash, options) -> options ?= {} - id = hash.id + id = coerceId hash.id typeMap = @typeMapFor(type) - dataCache = typeMap.cidToHash + cidToData = @clientIdToData clientId = typeMap.idToCid[id] - if dataCache[clientId] && options.skipIfExists + if clientId && cidToData[clientId] && options.skipIfExists return - result = @merge(type, hash) - + result = @merge(type, hash, true) if result && result.clientId @addLoadedData(type, result.clientId, hash) - record = @findByClientId(type, result.clientId) - unless record.get('complete') - record.loadedAsIncomplete() + # TODO: it will be probably needed to uncomment and fix this + #@_updateAssociations(type, type.singularName(), hash) - @_updateAssociations(type, type.singularName(), hash) + result - record + materializeRecord: (type, clientId, id) -> + record = @_super.apply this, arguments + + if @clientIdToComplete[clientId] != undefined && !@clientIdToComplete[clientId] + record.set 'incomplete', true + else + record.set 'incomplete', false + + record _loadMany: (store, type, json) -> root = type.pluralName() diff --git a/assets/scripts/app/store/rest_adapter.coffee b/assets/scripts/app/store/rest_adapter.coffee index 54352ee5..eb422316 100644 --- a/assets/scripts/app/store/rest_adapter.coffee +++ b/assets/scripts/app/store/rest_adapter.coffee @@ -6,7 +6,49 @@ DS.JSONTransforms['object'] = { serialize: (deserialized) -> deserialized } +Travis.Serializer = DS.RESTSerializer.extend + merge: (record, serialized) -> + data = record.get('data') + + # TODO: write test that ensures that we go to materializingData + # only if we can + state = record.get('stateManager.currentState.path') + unless state == "rootState.loaded.materializing" + record.send('materializingData') + + record.eachAttribute( (name, attribute) -> + value = @extractAttribute(record.constructor, serialized, name) + if value != undefined + value = @deserializeValue(value, attribute.type) + if value != data.attributes[name] + record.materializeAttribute(name, value) + record.notifyPropertyChange(name) + , this) + + record.eachRelationship( (name, relationship) -> + if relationship.kind == 'belongsTo' + key = @_keyForBelongsTo(record.constructor, relationship.key) + value = @extractBelongsTo(record.constructor, serialized, key) + + if value != undefined && data.belongsTo[name] != value + record.materializeBelongsTo name, value + record.notifyPropertyChange(name) + else if relationship.kind == 'hasMany' + key = @_keyForHasMany(record.constructor, relationship.key) + value = @extractHasMany(record.constructor, serialized, key) + + if value != undefined + record.materializeHasMany name, value + record.notifyPropertyChange(name) + , this) + + # TODO: add test that ensures that this line is called + # it should check if record goes into loaded.saved + # state after being in materializing + record.notifyPropertyChange('data') + Travis.RestAdapter = DS.RESTAdapter.extend + serializer: Travis.Serializer mappings: broadcasts: Travis.Broadcast repositories: Travis.Repo @@ -44,6 +86,9 @@ Travis.RestAdapter = DS.RESTAdapter.extend else @_super.apply this, arguments + merge: (store, record, serialized) -> + @get('serializer').merge(record, serialized) + Travis.RestAdapter.map 'Travis.Commit', {} Travis.RestAdapter.map 'Travis.Build', { diff --git a/assets/scripts/lib/travis/model.coffee b/assets/scripts/lib/travis/model.coffee index 11e2ee42..9084dd02 100644 --- a/assets/scripts/lib/travis/model.coffee +++ b/assets/scripts/lib/travis/model.coffee @@ -3,10 +3,22 @@ @loadedAttributes = [] @_super.apply this, arguments - refresh: -> - if id = @get('id') - store = @get('store') - store.adapter.find store, @constructor, id + getAttr: (key, options) -> + @needsCompletionCheck(key) + @_super.apply this, arguments + + getBelongsTo: (key, type, meta) -> + @needsCompletionCheck(key) + @_super.apply this, arguments + + getHasMany: (key, type, meta) -> + @needsCompletionCheck(key) + @_super.apply this, arguments + + needsCompletionCheck: (key) -> + if key && (@constructor.isAttribute(key) || @constructor.isRelationship(key)) && + @get('incomplete') && !@isAttributeLoaded(key) + @loadTheRest(key) update: (attrs) -> $.each attrs, (key, value) => @@ -14,14 +26,7 @@ this isAttributeLoaded: (name) -> - key = null - if meta = Ember.get(this.constructor, 'attributes').get(name) - key = meta.key(this.constructor) - else if meta = Ember.get(this.constructor, 'associationsByName').get(name) - key = meta.options.key || @get('namingConvention').foreignKey(name) - - if key - @get('store').isDataLoadedFor(this.constructor, @get('clientId'), key) + @get('store').isDataLoadedFor(this.constructor, @get('clientId'), name) isComplete: (-> if @get 'incomplete' @@ -44,14 +49,13 @@ return if @get('isCompleting') @set 'isCompleting', true - @refresh() + if @get('stateManager.currentState.path') != 'rootState.loaded.materializing' + @reload() + @set 'incomplete', false select: -> @constructor.select(@get('id')) - loadedAsIncomplete: () -> - @set 'incomplete', true - @Travis.Model.reopenClass find: -> if arguments.length == 0 @@ -86,5 +90,15 @@ Travis.store.adapter.pluralize(@singularName()) isAttribute: (name) -> - Ember.get(this, 'attributes').has(name) || - Ember.get(this, 'associationsByName').has(name) + Ember.get(this, 'attributes').has(name) + + isRelationship: (name) -> + Ember.get(this, 'relationshipsByName').has(name) + + isHasManyRelationship: (name) -> + if relationship = Ember.get(this, 'relationshipsByName').get(name) + relationship.kind == 'hasMany' + + isBelongsToRelationship: (name) -> + if relationship = Ember.get(this, 'relationshipsByName').get(name) + relationship.kind == 'belongsTo' diff --git a/assets/scripts/spec/unit/incomplete_spec.coffee b/assets/scripts/spec/unit/incomplete_spec.coffee index 676752c3..9957e587 100644 --- a/assets/scripts/spec/unit/incomplete_spec.coffee +++ b/assets/scripts/spec/unit/incomplete_spec.coffee @@ -1,27 +1,36 @@ -Travis.Foo = Travis.Model.extend - name: DS.attr('string') - description: DS.attr('string') - lastName: DS.attr('string') - - bar: DS.belongsTo('Travis.Bar') - niceBar: DS.belongsTo('Travis.Bar') - veryNiceBar: DS.belongsTo('Travis.Bar', key: 'very_nice_bar_indeed_id') - -Travis.Bar = Travis.Model.extend() - record = null store = null +adapterClass = null $.mockjax url: '/foos/1' responseTime: 10 responseText: { foo: { id: 1, name: 'foo', description: 'bar' } } -describe 'Travis.Model', -> +describe 'Travis.Model - incomplete', -> beforeEach -> - store = Travis.Store.create() + Travis.Foo = Travis.Model.extend + name: DS.attr('string') + description: DS.attr('string') + lastName: DS.attr('string') + + bar: DS.belongsTo('Travis.Bar') + niceBar: DS.belongsTo('Travis.Bar') + veryNiceBar: DS.belongsTo('Travis.Bar') + + Travis.Bar = Travis.Model.extend() + + adapterClass = Travis.RestAdapter.extend() + adapterClass.map 'Travis.Foo', + veryNiceBar: { key: 'very_nice_bar_indeed_id' } + niceBar: { key: 'nice_bar_id' } + + store = Travis.Store.create + adapter: adapterClass.create() afterEach -> + delete Travis.Foo + delete Travis.Bar store.destroy() describe 'with incomplete record with loaded associations', -> @@ -32,43 +41,45 @@ describe 'Travis.Model', -> nice_bar_id: 3 very_nice_bar_indeed_id: 4 } - record = store.loadIncomplete(Travis.Foo, attrs) + store.loadIncomplete(Travis.Foo, attrs) + record = store.find Travis.Foo, 1 store.load(Travis.Bar, id: 2) store.load(Travis.Bar, id: 3) store.load(Travis.Bar, id: 4) it 'does not load record on association access', -> - expect( record.get('bar.id') ).toEqual 2 - expect( record.get('niceBar.id') ).toEqual 3 - expect( record.get('veryNiceBar.id') ).toEqual 4 + expect( record.get('bar.id') ).toEqual '2' + expect( record.get('niceBar.id') ).toEqual '3' + expect( record.get('veryNiceBar.id') ).toEqual '4' waits 50 runs -> - expect( record.get('complete') ).toBeFalsy() + expect( record.get('incomplete') ).toBeTruthy() describe 'with incomplete record without loaded associations', -> beforeEach -> attrs = { id: 1 } - record = store.loadIncomplete(Travis.Foo, attrs) + store.loadIncomplete(Travis.Foo, attrs) + record = store.find Travis.Foo, 1 it 'loads record based on regular association key', -> record.get('bar') waits 50 runs -> - expect( record.get('complete') ).toBeTruthy() + expect( record.get('incomplete') ).toBeFalsy() it 'loads record based on camel case association key', -> record.get('niceBar') waits 50 runs -> - expect( record.get('complete') ).toBeTruthy() + expect( record.get('incomplete') ).toBeFalsy() it 'loads record based on ssociation with explicit key', -> record.get('veryNiceBar') waits 50 runs -> - expect( record.get('complete') ).toBeTruthy() + expect( record.get('incomplete') ).toBeFalsy() describe 'with incomplete record', -> beforeEach -> @@ -77,7 +88,8 @@ describe 'Travis.Model', -> name: 'foo' last_name: 'foobar' } - record = store.loadIncomplete(Travis.Foo, attrs) + store.loadIncomplete(Travis.Foo, attrs) + record = store.find Travis.Foo, 1 it 'shows if attribute is loaded', -> expect( record.isAttributeLoaded('name') ).toBeTruthy() @@ -87,7 +99,7 @@ describe 'Travis.Model', -> expect( record.get('name') ).toEqual 'foo' waits 50 runs -> - expect( record.get('complete') ).toBeFalsy() + expect( record.get('incomplete') ).toBeTruthy() it 'loads missing data if getPath is used', -> other = Em.Object.create(record: record) @@ -104,7 +116,6 @@ describe 'Travis.Model', -> waits 50 runs -> expect( record.get('description') ).toEqual 'bar' - expect( record.get('complete') ).toBeTruthy() expect( record.get('isComplete') ).toBeTruthy() it 'does not set incomplete on the record twice', -> @@ -124,28 +135,29 @@ describe 'Travis.Model', -> expect( record.get('lastName') ).toEqual 'foobar' waits 50 runs -> - expect( record.get('complete') ).toBeFalsy() + expect( record.get('incomplete') ).toBeTruthy() it 'adds takes into account additional data loaded as incomplete', -> - record = store.loadIncomplete(Travis.Foo, { id: 1, description: 'baz' }) + store.loadIncomplete(Travis.Foo, { id: 1, description: 'baz' }) + record = store.find Travis.Foo, 1 expect( record.get('description') ).toEqual 'baz' waits 50 runs -> - expect( record.get('complete') ).toBeFalsy() + expect( record.get('incomplete') ).toBeTruthy() describe 'with complete record', -> beforeEach -> - id = 5 + id = '5' attrs = { id: id name: 'foo' } - store.load(Travis.Foo, id, attrs) - record = Travis.Foo.find(id) + store.load(Travis.Foo, attrs, { id: attrs.id }) + record = store.find(Travis.Foo, id) it 'is marked as completed', -> - expect( record.get('complete') ).toBeTruthy() + expect( record.get('incomplete') ).toBeFalsy() it 'allows to get regular attribute', -> expect( record.get('name') ).toEqual 'foo' diff --git a/assets/scripts/spec/unit/merge_spec.coffee b/assets/scripts/spec/unit/merge_spec.coffee new file mode 100644 index 00000000..7feda345 --- /dev/null +++ b/assets/scripts/spec/unit/merge_spec.coffee @@ -0,0 +1,105 @@ +record = null +store = null + +describe 'Travis.Model - merge', -> + beforeEach -> + Travis.Foo = Travis.Model.extend + login: DS.attr('string') + firstName: DS.attr('string') + email: DS.attr('string') + + bar: DS.belongsTo('Travis.Bar') + + Travis.Bar = Travis.Model.extend + foos: DS.hasMany('Travis.Foo') + + store = Travis.Store.create() + + afterEach -> + delete Travis.Foo + delete Travis.Bar + store.destroy() + + it 'updates the attributes of materialized record', -> + data = { id: '1', firstName: 'Piotr', email: 'drogus@example.org' } + store.load(Travis.Foo, { id: '1' }, data) + record = store.find(Travis.Foo, '1') + + changes = 0 + + observer = -> + changes += 1 + record.addObserver 'firstName', observer + + Ember.run -> + store.merge(Travis.Foo, { id: '1', first_name: 'Peter', login: 'drogus' }) + + record.removeObserver 'firstName', observer + + expect(changes).toEqual(1) + expect(record.get('firstName')).toEqual('Peter') + expect(record.get('login')).toEqual('drogus') + expect(record.get('email')).toEqual('drogus@example.org') + + it 'updates belongsTo relationship of materialized record', -> + data = { id: '1', login: 'drogus', bar_id: '1' } + store.load(Travis.Foo, data, { id: '1' }) + store.load(Travis.Bar, { id: '1' }, { id: '1' }) + store.load(Travis.Bar, { id: '2' }, { id: '2' }) + record = store.find(Travis.Foo, '1') + + changed = false + + observer = -> + changed = true + record.addObserver 'bar', observer + + Ember.run -> + store.merge(Travis.Foo, { id: '1', bar_id: '2' }) + + record.removeObserver 'bar', observer + + bar = store.find(Travis.Bar, '2') + + expect(changed).toEqual(true) + expect(record.get('bar')).toEqual(bar) + + it 'updates hasMany relationship of materialized record', -> + data = { id: '1', foo_ids: [1] } + store.load(Travis.Bar, data, { id: '1' }) + store.load(Travis.Foo, { id: '1' }, { id: '1' }) + store.load(Travis.Foo, { id: '2' }, { id: '2' }) + + record = store.find(Travis.Bar, '1') + + changed = false + + observer = -> + changed = true + record.addObserver 'foos.length', observer + + Ember.run -> + store.merge(Travis.Bar, { id: '1', foo_ids: [1, 2] }) + + record.removeObserver 'foos.length', observer + + expect(changed).toEqual(true) + expect(record.get('foos.length')).toEqual(2) + expect(record.get('foos').mapProperty('id')).toEqual(['1', '2']) + + it 'loads given data if it\'s not in the store yet', -> + store.merge(Travis.Foo, { id: '1', login: 'drogus' }) + + record = store.find(Travis.Foo, 1) + + expect(record.get('login')).toEqual('drogus') + expect(record.get('email')).toEqual(null) + + it 'merges data if it\'s just loaded into store', -> + store.load(Travis.Foo, { id: '1', login: 'drogus', email: 'drogus@example.org' }, { id: '1' }) + store.merge(Travis.Foo, { id: '1', login: 'svenfuchs' }) + + record = store.find(Travis.Foo, 1) + + expect(record.get('login')).toEqual('svenfuchs') + expect(record.get('email')).toEqual('drogus@example.org') diff --git a/assets/scripts/travis.coffee b/assets/scripts/travis.coffee index d3ceacf4..bf99ca71 100644 --- a/assets/scripts/travis.coffee +++ b/assets/scripts/travis.coffee @@ -17,13 +17,12 @@ Storage = Em.Object.extend @set('storage', {}) window.Travis = Em.Application.extend(Ember.Evented, - autoinit: false authStateBinding: 'auth.state' signedIn: (-> @get('authState') == 'signed-in' ).property('authState') setup: -> @store = Travis.Store.create( - adapter: Travis.RestAdapter.create(serializer: DS.RESTSerializer) + adapter: Travis.RestAdapter.create() ) @store.loadMany(Travis.Sponsor, Travis.SPONSORS) @@ -73,6 +72,8 @@ window.Travis = Em.Application.extend(Ember.Evented, @autoSignIn() unless @get('signedIn') ).create() +Travis.deferReadiness() + $.extend Travis, config: api_endpoint: $('meta[rel="travis.api_endpoint"]').attr('href') diff --git a/public/index.html b/public/index.html index 9921d781..4bf319be 100644 --- a/public/index.html +++ b/public/index.html @@ -16,6 +16,7 @@ diff --git a/public/spec.html b/public/spec.html index 86d5decd..1293498e 100644 --- a/public/spec.html +++ b/public/spec.html @@ -11,6 +11,7 @@ @@ -28,8 +29,5 @@ jasmine.getEnv().addReporter(console_reporter); jasmine.getEnv().execute(); - -
-