PDA

View Full Version : Reader onMetaChange now always resets the _model.$className 4.2 to 5.0



JakeBrown
9 May 2014, 8:54 AM
We use metadata in our app to pull in extra data.

In 4.2, we could have metadata, and if a reader already had a model, it would continue to assume that model was still good.

In 5.0, the reader's onMetaChange always creates a new base model of Ext.data.Model.

We put in a temporary hack to override onMetaChange to wrap the newModel code to

if(meta.fields) {
...newmodel and set proxy.
}


Are we using metadata incorrectly, or is this an bug?

Thanks,
Jake

dfrederick
9 May 2014, 9:44 AM
To expand on that, it causes several issues. For example, it wipes out associations that were defined in the model. Any custom methods or configs that were set in the model declaration are also wiped out.
Here's a fiddle, it requires changing the root/rootProperty in the store depending on version
https://fiddle.sencha.com/#fiddle/5n7

evant
9 May 2014, 1:52 PM
Can you elaborate on the use case for this? That is to say, why aren't you using a fixed model type if they're sharing these things (custom methods, associations).

If the model is coming back as an anonymously defined model, the thinking was that it's an ephemeral kind of construct.

dfrederick
9 May 2014, 6:17 PM
Hi Evan,
I think you're misunderstanding. We are using a fixed model, configured in the store at design time. Well, at least we're trying to, but the reader isn't respecting that. Whenever metadata is encountered, the reader is throwing away the configured model and creating a new instance of Ext.data.Model instead of our extension. This is the Ext.data.reader.Reader's onMetaChange source taken directly from docs.sencha.com (critical section in red)



/** * @private * Reconfigures the meta data tied to this Reader */ onMetaChange : function(meta) { var me = this, fields = meta.fields || me.getFields(), model, newModel, clientIdProperty, proxy; // save off the raw meta data me.metaData = meta; // set any reader-specific configs from meta if available if (meta.root) { me.setRootProperty(meta.root); } if (meta.totalProperty) { me.setTotalProperty(meta.totalProperty); } if (meta.successProperty) { me.setSuccessProperty(meta.successProperty); } if (meta.messageProperty) { me.setMessageProperty(meta.messageProperty); } clientIdProperty = meta.clientIdProperty; newModel = Ext.define(null, { extend: 'Ext.data.Model', fields: fields, clientIdProperty: clientIdProperty }); me.setModel(newModel); proxy = me.getProxy(); if (proxy) { proxy.setModel(newModel); } me.buildExtractors(true); },

4.2.2 accounted for this and continued to respect the model type that was configured and only used it's own class if none was defined.



if (me.model) { me.model.setFields(fields, me.idProperty, clientIdProperty); me.setModel(me.model, true); } else { newModel = Ext.define("Ext.data.reader.Json-Model" + Ext.id(), { extend: 'Ext.data.Model', fields: fields, clientIdProperty: clientIdProperty }); if (me.idProperty) { // We only do this if the reader actually has a custom idProperty set, // otherwise let the model use its own default value. It is valid for // the reader idProperty to be undefined, in which case it will use the // model's idProperty (in getIdProperty()). newModel.idProperty = me.idProperty; } me.setModel(newModel, true); }

If you really need a use case, we have a bunch.

One is we have a grid that is paged and remote filtered.  It has a summary header that shows a rollup of all of the records passing the filter criteria, but the data is paged so we aren't able to calculate that client side. We do the calculation server side, and send the summary info back as metadata every time the filter is changed. But, the presence of the metadata ultimately means every record in our store is an instance of Ext.data.Model instead of my model class.

Here's another, we have an admin page that has configurable columns for multiple groups and rows for security objects. Each cell has complex data within it that is represented by multiple fields in the model for that security object and the group it belongs to. To avoid having a mess of dynamic model fields, and a mess in the renderer to figure out which fields to look at ex.
[
{name: 'id'},
{name: 'name'},
{name: 'group1field1'},
{name: 'group1field2'},
{name: 'group1field3'},
{name: 'group1field4'},
{name: 'group3field1'},
{name: 'group3field2'},
{name: 'group3field3'},
{name: 'group3field4'},
....
]
we're leveraging one to many associations and have this split up into 2 stores/models. ex.
SecurityObject fields
[
{name: 'id'},
{name: 'name'}
]
GroupPermission fields
[
{name: 'objectId'},
{name: 'groupId'},
{name: 'field1'},
{name: 'field2'},
{name: 'field3'},
{name: 'field4'}
]
where every SecurityObject has many GroupPermission records associated to it.

The column simply has to pivot the SecurityObject on the association and find the group that corresponds to the column that's being rendered to get the correct associated GroupPermission record and the renderer always gets to use the same fields 'field1', 'field2', 'field3', etc. But... in this case, when send metadata to reconfigure the grid with different columns for different groups - the reader no longer uses our model, and instead creates an instance of Ext.data.Model, but with the fields copied from our model, and we no longer have associations in the records from our main store.

evant
9 May 2014, 7:09 PM
Ok, so it certainly should conditionalize the fields check and only create the new model if necessary, something like:



onMetaChange : function(meta) {
var me = this,
fields = meta.fields,
model,
newModel,
clientIdProperty,
proxy;

// save off the raw meta data
me.metaData = meta;

// set any reader-specific configs from meta if available
if (meta.root) {
me.setRootProperty(meta.root);
}

if (meta.totalProperty) {
me.setTotalProperty(meta.totalProperty);
}

if (meta.successProperty) {
me.setSuccessProperty(meta.successProperty);
}

if (meta.messageProperty) {
me.setMessageProperty(meta.messageProperty);
}

clientIdProperty = meta.clientIdProperty;

if (fields) {
newModel = Ext.define(null, {
extend: 'Ext.data.Model',
fields: fields,
clientIdProperty: clientIdProperty
});
me.setModel(newModel);
proxy = me.getProxy();
if (proxy) {
proxy.setModel(newModel);
}
}
me.buildExtractors(true);
}


I'm not really following about associations though. A model is essentially defined its fields, changing the fields of a particular model seems like a non-standard use case. Creating a new anonymous model type, I can see that, but changing an existing known model would be akin to dynamically changing a database table as your app runs. Especially in 5.0.x because associations will be driven by the model fields.

Perhaps we could add a template method so it could look like:



onMetaChange : function(meta) {
var me = this,
fields = meta.fields,
model,
newModel,
clientIdProperty,
proxy;

// save off the raw meta data
me.metaData = meta;

// set any reader-specific configs from meta if available
if (meta.root) {
me.setRootProperty(meta.root);
}

if (meta.totalProperty) {
me.setTotalProperty(meta.totalProperty);
}

if (meta.successProperty) {
me.setSuccessProperty(meta.successProperty);
}

if (meta.messageProperty) {
me.setMessageProperty(meta.messageProperty);
}

clientIdProperty = meta.clientIdProperty;

if (fields) {
this.createMetaModel(fields);
}
me.buildExtractors(true);
},

createMetaModel: function() {
newModel = Ext.define(null, {
extend: 'Ext.data.Model',
fields: fields,
clientIdProperty: clientIdProperty
});
me.setModel(newModel);
proxy = me.getProxy();
if (proxy) {
proxy.setModel(newModel);
}
},

dfrederick
9 May 2014, 9:50 PM
Having the option for a template method goes a long way, but I don't understand the reason to disregard the model type the developer configured and inject an anonymous mode instead. It feels like having to add boilerplate. I don't think the new model should be defined/instantiated at all unless reader.getModel() === undefined

I think I confused and distracted from the issue by bringing associations into this, that was just one symptom, the real issue is that any metadata that is sent causes the configured model type to no longer be used.

This fiddle demonstrates it better. https://fiddle.sencha.com/#fiddle/5no
(https://fiddle.sencha.com/#fiddle/5no)
if you run it as saved, you see that user.getId()===user.get('userId') is true because 'userId' is the configured idProperty and is what I expect.

But if you uncomment these lines it goes bad, user.get('userId') still equals 201123, but user.getId() is now generated 'extModel29-1'
metaData: {
otherdata: 'this data is not used to reconfigure the reader/store/proxy/model, but is sent for other purposes'
}

Maybe I don't understand where Sencha is going with metadata, but it seems this is a big step backwards. Regardless of what the metadata is that is being sent, not respecting the specific model type that was configured by the developer is a big issue because the fields collection isn't the only important thing on a model, not to mention the side effects are totally unexpected and it took us about 2 days of tracing to finally come across the onMetaChange as being the culprit.

In this case, the idProperty 'userId' specified in the User class isn't being used anymore. Instead a generated id is being used and stuffed into the default 'id' field. If I had configured a proxy on my User class and tried to call record.save() on one of the resulting records, I'd get an error about not having a proxy. If I had a custom schema defined on my User class, the resulting records would be in the default schema instead.

Even limiting the anonymous model to be used only when fields is sent in the metadata is extremely restrictive. Essentially that means if I need to reconfigure a grid with dynamic columns sent from the server (or for that matter use metaData for anything), I better not extend model or configure anything but fields, and every model I return from the server needs to use 'id' for the unique id, and I can't have a proxy on my models, and essentially I really can't do anything with a model beyond the built in functionality and what is constrained by the default configuration.

Artur Bodera (Joust)
14 Aug 2014, 5:58 AM
+1 to that.

I've just lost 3h debugging this issue. As soon as I've defined "metaProperty" on my Json reader, associations stopped working. Debugging the whole stack revealed that onMetadataChange() happily clears _model and injects some bogus placeholder.

kevinvdheuvel
21 Oct 2015, 5:47 AM
Hi all,Did anyone manage to fix this issue? Im facing the same issue as explained here.. Regards,Kevin