kurko / ember-sync

MIT License
282 stars 28 forks source link

hasMany array is lost #15

Closed tute closed 9 years ago

tute commented 10 years ago

If I define my DS.hasMany('child') as DS.attr('array') (I've got a trivial array Transform), then in ember-sync I can see the array of child ids in the parent object.

When I leave the hasMany call, then the attribute gets undefined. Debugging lib/ember-sync/query.js:

onlinePromise.then(function(results) {
          // ...
          results.forEach(function(onlineResult) {
                    onlineResult.get('childs') // => undefined

My onlineStore is the same as my ex-RESTAdapter, only moved to plugin ember-sync in the middle. API has no changes. Why could this be happening? Where should I continue digging into this issue?

tute commented 10 years ago

Problem may come from the suggested CustomOnlineSerializer of the README. I didn't specify any serializer before, do I even need to?

kurko commented 10 years ago

Do you think you could write a PR with a failing specs? That way it'd be pretty easy for me to look at it and try to fix, if there's an actual bug.

It might help just replacing this.emberSync.findQuery() with this.findQuery() just to see if that childs isn't undefined with ED itself. Ember Sync just returns whatever the ED returns.

I didn't specify any serializer before, do I even need to?

Some times you do have to specify it if you're defining custom stores like that. Maybe your situation is different. He it works fine.

tute commented 10 years ago

Indeed I didn't need to specify anyone, and the sample code was misleading to me as instead of ActiveModel I should have set RESTSerializer (or just none). README updated in PR #16 to reflect this no-needed. Thank you!

tute commented 10 years ago

I don't know if this issue is for ember-sync or ember-localstorage-adapter, but issue persists:

// In online store:
f = Client.__container__.lookup('store:online').find('parent')
// This returns a promise, and when I look into the models they have
// embedded hasMany objects

// In main (offline) store:
f = Client.__container__.lookup('store:main').find('parent')
// When I look into the models they don't even show the key for the hasMany
// relationship

Somewhere in between these libs the relationship is being lost.

kurko commented 10 years ago

Looks like

  1. LSAdapter has a bug
  2. RESTAdapter doesn't have a bug.
  3. Ember Sync is returning the instance generated by the offline database, which happens to be buggy.

You should try writing a failing spec as a PR for the LSAdapter repo.

tute commented 10 years ago

Finding the problem possibly in ember-sync. In the line https://github.com/kurko/ember-sync/blob/master/lib/ember-sync/persistence.js#L48, record has the relationship in it's _data, but after serialization it gets lost (serialized doesn't have it). Now investigating what's the specific serializer.

tute commented 10 years ago

Sharing work in progress, in case someone stumps with this problem: serialized.children = record.get('children').toArray() right in https://github.com/kurko/ember-sync/blob/master/lib/ember-sync/persistence.js#L50 keeps the related data. In a general case I scribbled there the following:

# Get the relationships of parent model, set in serialized hash
for key, value of record._data
  if typeof(value) is 'object'
    serialized[key] = value.toArray()
var key, value, _ref;
_ref = record._data;
for (key in _ref) {
  value = _ref[key];
  if (typeof value === 'object') {
    serialized[key] = value.toArray();
  }
}

I'm using DS.LSSerializer, which should handle these relationships automatically according to https://github.com/rpflorence/ember-localstorage-adapter/issues/38#issuecomment-35111725.

Looking forward to where/how to fix this in a nicer way. :)

kurko commented 9 years ago

I'm experiencing this in my app now. I'll try to come up with a path.

kurko commented 9 years ago

@tute any chance you can test this again? I believe I have fixed this in ca9797661e253842389d04056242aff3ea22eda9. If it doesn't work for you, please reopen the issue ;)