Proposal: Class-centered Polymorphism (STI)

The Problem

At the time of writing Ember Data only handles polymorphic associations. This doesn’t cover many common use cases because it enforces the polymorphism to only be used in associations.

Example

Imagine I have a model called Work which has many tasks. There are many kinds of tasks, so naturally in OOP I would define Task as a super class with all the common properties and then extend it with specific properties with subclasses.

App.Work = DS.Model.extend({
    name: DS.attr('string'),
    tasks: DS.hasMany('task')
});
  
App.Task = DS.Model.extend({
    description: DS.attr('string')
    date: DS.attr('date')
});
   
App.RepairTask = App.Task.extend({
     timeRequired: DS.attr('number'),
     operator: DS.attr('string')
});
App.CleaningTask = App.Task.extend({
     usedWater: DS.attr('number')
});

When I request /task/1, for example, the server returns the JSON (I’m using ActiveModelSerializer for ED, but that’s irrelevant here):

{
    "task": {
        "id":1,
        "type":"repairTask",
        "description": "I'm repairing this right away!",
        "timeRequired":5.4,
        "operator":"Tom"
    }
}

The code this.store.find('task') returns a Task object and not a RepairTask with the additional properties (imagine, relationships) like desired.


Proposal

Querying

Whenever a query is made, if the type is a subclass of DS.Model and it has been subclassed, Ember Data should look for type information to determine the correct concrete bottommost class to instantiate. The string in a type property is a common way to go and should be the default, but ideally we should abstract the concrete type determination logic to the user.

Saving

Likewise, when we save a concrete subclassed model (RepairTask, for example), a POST request should be sent to /tasks along with its type information (in this case a type property with repairTask value).

Bottomline and other toughts

This approach would make polymorphic:true useless, since associations would be naturally covered as well. Accessing the tasks property would trigger all this behaviour since Task isn’t a concrete subclass of DS.Model.

This approach is common in JSON processors like [Jackson][1], and with [JPA][2] ORM’s like [Hibernate][3]. The typical way to deal with inheritance in JSON is to include the type information in a reserved field.

Before version 1 of Ember Data we were dealing with this problem like this: https://coderwall.com/p/hi6hya Basically we were overriding the materialize hook and then we changed the record.constructor and applied an existing mixin based on the type property.

With Ember Data 1 the nearest solution I found was this one (question has before v1, answer has v1). However, with that solution relationships don’t work, and logic related to the specific properties in the mixin are not applied (like converting to camel case, for example).

Am I missing something here? Thank you for your time.

9 Likes

I agree with this proposal. I spent the day trying to shim this functionality into ember-data beta 3, but things get complicated. What I’d love to have is the ability to query an endpoint from the persistence layer that is of the root type (/message, to use the common ember example for polymorphic types) that will return to me a list of objects that tell me what their subclass is (a Post or a Comment). So if I query the store for store.find('messages'), when that find promise is resolved, the record array that is returned to me is populated with a mixture of Post and Comment models, all of which pass model instanceof Message (the type of the RecordArray would be ‘message’). Similarly, when I save a Post model, it would persist to /message (though as you said, making this configurable would be very beneficial)

The problem comes down to getting things in and out of the store. All models are stored in the (ahem) store on a series of hashes that allow for easy ways to retrieve and query for specific models of a given type. The problem is that these internal typeMaps don’t know about polymorphism. If you push in a Post record, it will be stored in the Post typeMap and the Messages typeMap will have no idea that this record exists. So even if you properly parse everything and override store.push to use the correct constructor when the model is created, you will not be able to reliably retrieve that model from the store by querying for any of its parent types: If at the ember-data level, Message is thought of as an abstract class that never has an instantiated model, then store.filter('message') will never return anything, because as far as the store is concerned, it knows about no Message models; everything is either a Post or a Comment, nor does it do anything to check if these types have anything to do with polymorphism.

I’m not sure what the ember-data core team has in mind for their relationship refactor (and it seems things have been silent for the last few weeks), but I think the way to make this work would be to make the store’s internal typeMaps aware of polymorphism, either in their structure (typeMaps for subclasses are stored within the typeMap for the parent class) or via some sort of observers (typeMaps for parent classes place a reference to all subclass instances within them, although this would mean that no instances in this polymorphic family can share ids).

I’d love to hack on this concept more, but unfortunately I’m on the clock, so I’ll need to revert to a non-polymorphic way of managing things. My product owner couldn’t care less about whether or not the types are polymorphic or not (and rightfully so :smile: ).

Maybe I’ll make some spare time available to look into it further.

2 Likes

This proposal makes a lot of sense to me. Currently, my back-end uses jackson for polymorphic serialization/deserialization with @type as the type-delimiting field for polymorphism. Any word on an implementation plan?

1 Like

Sadly, this didn’t seem to draw much attention from the community. Six months have passed and I still think this makes a lot of sense and is the way to go regarding inheritance.

So, I don’t know of any plans regarding this proposal or anything related.

1 Like

Can this be done by extend and override store.find? Seems like the problem here is that find not returning expected values.

This would probably have a deeper impact because, for example, your save behaviour would be different too:

Saving

Likewise, when we save a concrete subclassed model (RepairTask, for example), a POST request should be sent to /tasks along with its type information (in this case a type property with repairTask value).

I, too, have run into this problem. Our team has made a few attempts at solving it, but haven’t been completely satisfied with any of them.

If you dig into the store, it seems the problem lies in the recordForId method. It currently wears two hats. That method finds a record of a type for an id, returning it if found (synchronously) or building a new one (of type task and with an id) and returning the “temporary” record. Upon the serializer/adapter’s promise resolving, it wants to then update that temporary record with the normalized payload.

If you do a this.get('store').find('task', 1), or worse, click a link from elsewhere that takes you to http://youremberapp.com/#/tasks/1 - you don’t necessarily know what type the record should be before hand. So, you end up with a record in the store of type task, and potentially a record of type foobarTask.

To me, it seems there are two possibilities:

  1. Store all types of tasks in the store as task, materializing them into their respective type when pulled out of the store.
  • Seems this is how it was originally done w/ mixins, but can’t be now because of internal API changes. Also, it seems like the inheritance (STI likely) on the server should be mirrored on the client?)
  1. Store the types in the store as their specific type of task, but somehow coalescing them into tasks (perhaps using existing polymorphic plumbing?)
  • This falls down because when finding, recordForId will have already built a placeholder task record

In either direction, the crux of the issue seems to lie in recordForId - which is called internally in several parts of the codebase (often times assuming a synchronous getById or assuming a new record is built).

Thoughts? Also, concerned on how the ssot branch will effect this discussion.

2 Likes

I’ll agree that both the recordForId thing and the structure of the store make it difficult to work with polymorphic records offhand.

One of our client projects uses STI to serve a large number of Task subtypes. In Ember we have a bunch of subtypes of App.Task. Currently we put different subtypes into the store based on a type attribute that comes with the task’s payload.

I’ve been trying to write up a document on how we’re doing things up to this point.

  • We store subtypes of Task in the store.
  • We save all tasks to the same endpoint.
  • We can call store.find('task', 1) and get back a subtype of task
  • We can’t call store.all('task') and get back all tasks for all subtypes
  • We use polymorphic hasManys in several places

We had to override store.push in order to remove the empty supertype record left by recordForId, and also to override the type parameter in some cases.

Task a look at the repo and I’ve got some sample code and a waaayy too long readme.

I created a simple(ish) solution that solved my immediate problem. In particular wrapping the functionality of store.find such that if I pass a ‘parent’ type in, I get back instances for that type and any ‘child’ instances.

Note that it also relies on Ember App Kit, more specifically Ember-Resolver. The key to the function is requirejs.entries, which ember-resolver uses in the backend. By enumerating all models, and then filtering for ones that depend on the model type given.

Note that these code blocks are ripped out of a larger project and have been re-typed for this comment.

findSubmodels function that locates child models:

function( baseModel, cb ){
  var _entries = requirejs.entries
  var _entries    = requirejs.entries;
  var _submodels  = [ ];
  var self = this;
  async.each( Object.keys( _entries ), function( _entry, cb ){
      if( ! _entry.match( App.modulePrefix + "/models/" ) ){
          return cb( null );
      }

      if( _entry.match( App.modulePrefix + "/models/" + baseModel.toLowerCase( ) ) ){
          var _split      = _entry.split("/");
          var _name       = _split[_split.length-1];
          _submodels.push( _name );
          return cb( null );
      }

      if( _entries[_entry].deps.indexOf( App.modulePrefix + "/models/" + baseModel ) >= 0 ){
          var _split      = _entry.split("/");            // jshint ignore: line
          var _name       = _split[_split.length-1];      // jshint ignore: line
          _submodels.push( _name );
      }
      return cb( null );
  }, function( err ){
    return cb( null, _submodels );
  } );
 };

And the relevant code that wraps .find:

var _promise = new Ember.RSVP.Promise( function( resolve, reject ){
    findSubmodels( modelName, function( err, subModels ){
        async.map( subModels, function( modelName, cb ){
          store.find( _modelName, query ).then( function( _res ){
            cb( null, _res );
          } );
        }, function( err, res ){
             res.forEach( function( _res ){
                 _res.content.forEach( function( _return_obj ){
                        _return.push( _return_obj );
                 } );
              } );
        } );
        resolve( _return );
} );

I would just like to add this workaround found on github issues: https://github.com/emberjs/data/issues/1766#issuecomment-50371667

Link to github issue: https://github.com/emberjs/data/issues/2407