emberjs / data

A lightweight reactive data library for web applications. Designed over composable primitives.
https://api.emberjs.com/ember-data/release
Other
3.03k stars 1.33k forks source link

Does async belongsTo support related model assignment? #1542

Closed toranb closed 9 years ago

toranb commented 10 years ago

I have a simple hasMany / belongsTo relationship that's async

App.Appointment = DS.Model.extend({
  details: attr('string'),
  customer: belongsTo('customer', { async: true})
});

App.Customer = DS.Model.extend({
  name: attr('string'),
  appointments: hasMany()
});

When I'm creating a new appointment object I need to fetch a customer first and set it like so

this.store.find('customer', 1).then(function(customer) {
  var appointment = { details: 'foo', customer: customer.get('id') };
  this.store.createRecord('appointment', appointment).save();
});

When I do this, I can log the ember appointment object and it looks solid

Object {details: "asd", customer: 1}

But when I pause in the "createRecord" method itself, I see this "customer" is no longer an integer but instead a promise. And when the adapter tries to serialize this in the "serializeBelongsTo" method it fails to capture this (as it's a promise and not a concrete object or integer value)

Does belongsTo support this type of behavior when it's async? If yes, what should be done / what can I do to help? I have an adapter that I use with the latest master build and I've found this is an issue (but I assume it's also a core issue to even the RESTAdapter / Serializer that ships with ember-data core)

For reference, here are a few integration tests that show this behavior

https://github.com/toranb/ember-data-django-rest-adapter/pull/63/files

jherdman commented 10 years ago

Possibly related #1535?

toranb commented 10 years ago

@jherdman ah yes! And my biggest issue (handling this myself) has been the issue you raised in the discussion

"It seems like store.serialize() is finishing before polymorphic type stuff is done — it's async after all!"

As this "works" but the serlializer won't wait for the async work to finish

Ember.RSVP.resolve(belongsTo).then(function (rec) {
    //finish the serialize work ... but wait ... it's async :(
});

Did you get any feedback yet about how to solve this issue?

jherdman commented 10 years ago

@toranb a little! We're hammering out some details, specifically if we're willing to make serialize() return a promise.

toranb commented 10 years ago

I'm waiting for the #1535 to be pulled into master but here are 2 tests I used to drive out this behavior in my stand alone adapter

asyncTest('async belongsTo relationship will POST serialized data', function() {
    var store,
    assertion;

    store = App.__container__.lookup('store:main');
    Ember.run(App, function(){
        store.serializerFor('customer').pushSinglePayload(store, 'customer', {"id": 10, "name": "toranb", "appointments": []});
    });

    visit("/").then(function() {
        return store.find('customer', 10);
    }).then(function(customer){
        var appointment;
        appointment = store.createRecord('appointment', {
            start: 'today',
            details: 'more',
            customer: customer
        });
        return appointment.save().then(
                assertion,
                assertion
            ); // check the assertion regardless of success or failure of appointment.save()
    });

    assertion = function() {
        equal(ajaxHash.data, '{"start":"today","details":"more","customer":"10"}');
        start(); // end the asyncTest
    };
});

asyncTest('async belongsTo relationship will PUT serialized data', function() {
    var store,
    assertion;

    store = App.__container__.lookup('store:main');
    Ember.run(App, function(){
        store.serializerFor('customer').pushSinglePayload(store, 'customer', {"id": 10, "name": "toranb", "appointments": [2]});
        store.serializerFor('appointment').pushSinglePayload(store, 'appointment', {"id": 2, "start": "x", "details": "y", "customer": 10});
    });

    visit("/").then(function() {
        return store.find('appointment', 2);
    }).then(function(appointment){
        appointment.set('start', 'updated');
        return appointment.save().then(
                assertion,
                assertion
            ); // check the assertion regardless of success or failure of appointment.save()
    });

    assertion = function() {
        equal(ajaxHash.data, '{"start":"updated","details":"y","customer":"10"}');
        start(); // end the asyncTest
    };
});

The implementation is hacky at best but this got the tests passing for the above (totally subject to change as you know the core of this project best)

    serializeBelongsTo: function(record, json, relationship) {
        var self = this;
        var key = relationship.key;
        var belongsTo = record.get(key);
        var finalizer = function () { return json; };

        if (Ember.isNone(belongsTo)) {
          json[key] = belongsTo;
        } else {
            return Ember.RSVP.resolve(belongsTo).then(function(record) {
                json[key] = record.get('id');
            }).then(finalizer);
        }

        if (relationship.options.polymorphic) {
          return Ember.RSVP.resolve(belongsTo).then(function(record) {
                   self.serializePolymorphicType(record, json, relationship);
                 }).then(finalizer);
        }
    }
toranb commented 10 years ago

also here are the models I used in the test code above (could be put into a single PR once I get to work on this)

App.Customer = DS.Model.extend({
    name: DS.attr('string'),
    appointments: DS.hasMany('appointment', { async: true})
});

App.Appointment = DS.Model.extend({
    start: DS.attr('string'),
    details: DS.attr('string'),
    customer: DS.belongsTo('customer', { async: true}),
});
toranb commented 10 years ago

I'm seeing this issue pop up more and more -anyone on the core team aware of it? (this just came up on discuss yesterday but I've been using a fork of ember-data for a month+ now)

http://discuss.emberjs.com/t/how-should-async-belongsto-relationships-be-serialized/3858

stefanpenner commented 10 years ago

Sorry I havent been been to busy to dig into this one yet.

toranb commented 10 years ago

No worries -I'm tight for time myself this month, I just wanted to see if anyone else had noticed this was or wasn't an issue in ember-data 1.0 beta 5

I'm currently waiting on the related issue to be pulled in as it changes the serializer used here (returns a promise now as you've seen). When that PR is pulled in I think this change should be fairly simple (just resolve the belongsTo or hasMany essentially)

aBuder commented 10 years ago

Hi friends is the bug with the belongsTo relationship solved? I have the same issue.

kingpin2k commented 10 years ago

@aBuder, no, they are still hashing out implementation in https://github.com/emberjs/data/pull/1535

toranb commented 10 years ago

Agreed - I'm using a custom fork if you really need to ship and can't wait for the issue @kingpin2k mentioned. Ping me at my gmail and I'll pass you a link -I shipped a commercial app using it last month and so far it's been bug free for async hassMany/belongsTo (non-polymorphic) relationships

bmac commented 10 years ago

I just ran into this problem too. :( Looks like there are about 4 or 5 open issues for this bug.

ashrafhasson commented 10 years ago

I believe I have hit this same problem using ember-data.1.0.0-beta.8. Having a hasMany/belongsTo relationship, when I attempt to save the model object that has the belongsTo definition, the PUT request shows a null value for the belongsTo relationship key. Is this related to the issue above?

bmac commented 10 years ago

@ashrafhasson that sounds exactly like this issue. One workaround is to get the async property before saving.

model.set('asyncRelationship', record);
model.get('asyncRelationship').then(function() {
  model.save();
});
johanneswuerbach commented 9 years ago

Sadly this doesn't work anymore after ssot in beta10. Is there another workaround to set / unset and async belongsTo?

//cc @igorT

Update: the new workaround:

model.set('asyncRelationship.id', null);
model.save()
sly7-7 commented 9 years ago

I think this is fixed in canary now. cc/ @igorT

sly7-7 commented 9 years ago

@johanneswuerbach Is there any chance you could test with current canary ?

johanneswuerbach commented 9 years ago

@sly7-7 yes, unassigning works now for me. :+1:

sly7-7 commented 9 years ago

@johanneswuerbach Great, thank you. @igorT this could be closed

shaunc commented 9 years ago

Hmm... is it possible its not fixed in the case that the relationship is polymorphic as well as async?

sly7-7 commented 9 years ago

@shaunc Would it be possible for you to open a new issue demonstrating the bug please ? Currently, a test like

test("set polymorphic async belongsTo", function() {
  expect(2);
  Contact = DS.model.extend({
    posts: DS.hasMany('post')
  });
  Email = Contact.extend();
  Post = DS.Model.extend({
    contact: DS.belongsTo('contact', { async: true, polymorphic: true })
  });

  Ember.run(function () {
    email = env.store.createRecord('email');
    post = env.store.createRecord('post', {
      contact: email
    });
  });

  post.get('contact').then(async(function(contact){
    equal(contact, email, 'The polymorphic belongsTo is set up correctly');
    equal(get(email, 'posts.length'), 1, "The inverse has many is set up correctly on the email side.");
  }));
});

is passing. Maybe it does not reflect your use case though.

shaunc commented 9 years ago

I'll try to get one up -- the problem is that, in RESTSerializer. serializePolymorphicType,

  json[key + "Type"] = Ember.String.camelize(belongsTo.constructor.typeKey);

will fail when "belongsTo" is a PromiseObject. Using:

belongsTo.content.constructor.typeKey

in this case works. (I'm trying to run your tests now, but is getting sort of late at night here. :))

sly7-7 commented 9 years ago

@shaunc Ok, so your issue is a well known one, see https://github.com/emberjs/data/pull/2571

shaunc commented 9 years ago

thanks!