1. #1
    Sencha User
    Join Date
    Oct 2009
    Posts
    23
    Vote Rating
    2
    ivanleblanc is on a distinguished road

      0  

    Default commitRecords not working with multiple server records

    commitRecords not working with multiple server records


    Ext.data.Operations.commitRecords does not seem to be syncing the server data to the client record when the server returns multiple records.

    This is for a multiple create operation, I have not tried a multiple update operation.

    4.0.7 worked perfectly handling multiple records but as soon as I swap the API out with 4.1.0 rc3 it does not work. I noticed that the commitRecords function was changed quite a bit from 4.0.7 but I have not been able to determine why is has stopped working.

    Again this only happens when multiple records are returned from the server, when one is returned it works fine in both 4.0.7 and 4.1.0 because of these lines (taken from 4.1.0 commitRecords):
    Code:
    clientRec = clientRecords[0];
    serverRec = serverRecords[0];
    The code doesn't seem to be able to match the server records back to the client records when there are multiple records returned form the server.

  2. #2
    Sencha User
    Join Date
    Aug 2008
    Location
    Brasov, Romania
    Posts
    34
    Vote Rating
    1
    nan21eu is on a distinguished road

      0  

    Default


    Do you have a clientId field in your model ? This may be a reason as the model uses a clientIdProperty:"clientId" to do this matching . I had a similar problem, as i use clientId as a tenant identificator and is present in most of my models, so my fix was:

    Code:
    Ext.override(Ext.data.Model, {
        clientIdProperty : "__clientRecordId__"
    });

  3. #3
    Sencha User
    Join Date
    Oct 2009
    Posts
    23
    Vote Rating
    2
    ivanleblanc is on a distinguished road

      0  

    Default


    No I do not.
    I just don't see how the commitRecords function works I guess.
    If anyone can shed some light on it that would be great. So this is what i understand of it.
    commitRecords code from 4.1.0 rc3
    Code:
    var me = this,
        mc, index, clientRecords, serverRec, clientRec;
    
    
    if (!me.actionSkipSyncRe.test(me.action)) {
        clientRecords = me.records;
        if (clientRecords && clientRecords.length) {
            if(clientRecords.length > 1) {
                // if this operation has multiple records, client records need to be matched up with server records
                // so that any data returned from the server can be updated in the client records.
                mc = new Ext.util.MixedCollection();
                mc.addAll(serverRecords);
    
    
                for (index = clientRecords.length; index--; ) {
                    clientRec = clientRecords[index];
                    serverRec = mc.findBy(function(record) {
                        var clientRecordId = clientRec.getId();
                        if(clientRecordId && record.getId() === clientRecordId) {
                            return true;
                        }
    This is where I get confused. So at this point the function knows that I have multiple records to commit and it
     has already determined that this is a new record not an update record.
    But how can record.internalId === clientRec.internalId ever be true??
    The clientRec.internalId is usually "ModelName"-{some random number Ext assigns if its a phantom record}
    The serverRec.internalId is "ModelName"-{the unique id my server gives the new record}
                        // if the server record cannot be found by id, find by internalId.
                        // this allows client records that did not previously exist on the server
                        // to be updated with the correct server id and data.
                        return record.internalId === clientRec.internalId;
                    });
                    
                    // replace client record data with server record data
                    me.updateClientRecord(clientRec, serverRec);
                }
            } else {
                // operation only has one record, so just match the first client record up with the first server record
                clientRec = clientRecords[0];
                serverRec = serverRecords[0];
                // if the client record is not a phantom, make sure the ids match before replacing the client data with server data.
                if(serverRec && (clientRec.phantom || clientRec.getId() === serverRec.getId())) {
                    me.updateClientRecord(clientRec, serverRec);
                }
            }
    
    
            if (me.actionCommitRecordsRe.test(me.action)) {
                for (index = clientRecords.length; index--; ) {
                    clientRecords[index].commit();
                }
            }
        }
    }

  4. #4
    Ext JS Premium Member dnorman's Avatar
    Join Date
    Jan 2011
    Posts
    101
    Vote Rating
    54
    dnorman is a jewel in the rough dnorman is a jewel in the rough dnorman is a jewel in the rough

      0  

    Default


    Just smacked right into this problem myself.

    Here's what I think is going on.
    In 4.0.7, Ext.data.Store formerly did the following. (Note the very relevant inline comment)
    Code:
     
        onCreateRecords: function(records, operation, success) {
            if (success) {
                var i = 0,
                    data = this.data,
                    snapshot = this.snapshot,
                    length = records.length,
                    originalRecords = operation.records,
                    record,
                    original,
                    index;
    
    
                /*
                 * Loop over each record returned from the server. Assume they are
                 * returned in order of how they were sent. If we find a matching
                 * record, replace it with the newly created one.
                 */
                for (; i < length; ++i) {
                    record = records[i];
                    original = originalRecords[i];
                    if (original) {
                        index = data.indexOf(original);
                        if (index > -1) {
                            data.removeAt(index);
                            data.insert(index, record);
                        }
    /*  SNIP  */
    This code is completely gone now, thank goodness!
    The whole remove + insert thing royally sucked. Good riddance.

    This is now being handled through commitRecords from Ext.data.Operation.
    However, it would appear that this is crazy FiFo business ( seriously, who does that? gahh ) has fallen out of fashion, and all stores are expected to pass my clientId through to the other side, which would then be used to set internalId and match up the records based on that, yada yada yada. Seeing as how I'd rather not hack the living bujeesuz out of my server app, I thought it better to patch it.

    Just a single line added below.
    Bear in mind that I'm still using 4.1rc1 for the time being, so your version may differ slightly.
    Code:
    Ext.define('MyApp.patch.Operation.Omg.Wtf', {
        override: 'Ext.data.Operation',
        commitRecords: function (serverRecords) {
            var me = this,
                mc, index, clientRecords, serverRec, clientRec;
    
    
            if (!me.actionSkipSyncRe.test(me.action)) {
                clientRecords = me.records;
    
    
                if (clientRecords && clientRecords.length) {
                    if(clientRecords.length > 1) {
                        // if this operation has multiple records, client records need to be matched up with server records
                        // so that any data returned from the server can be updated in the client records.
                        mc = new Ext.util.MixedCollection();
                        mc.addAll(serverRecords);
    
    
                        for (index = clientRecords.length; index--; ) {
                            clientRec = clientRecords[index];
                            serverRec = mc.findBy(function(record) {
                                var clientRecordId = clientRec.getId();
                                if(clientRecordId && record.getId() === clientRecordId) {
                                    return true;
                                }
                                // if the server record cannot be found by id, find by internalId.
                                // this allows client records that did not previously exist on the server
                                // to be updated with the correct server id and data.
                                return record.internalId === clientRec.internalId;
                            });
                            
                            if(!serverRec && me.action == 'create') serverRec = serverRecords[index]; // MODIFIED - Oh the horror!
                            
                            // replace client record data with server record data
                            me.updateClientRecord(clientRec, serverRec);
                        }
                    } else {
                        // operation only has one record, so just match the first client record up with the first server record
                        clientRec = clientRecords[0];
                        serverRec = serverRecords[0];
                        // if the client record is not a phantom, make sure the ids match before replacing the client data with server data.
                        if(serverRec && (clientRec.phantom || clientRec.getId() === serverRec.getId())) {
                            me.updateClientRecord(clientRec, serverRec);
                        }
                    }
    
    
                    if (me.actionCommitRecordsRe.test(me.action)) {
                        for (index = clientRecords.length; index--; ) {
                            clientRecords[index].commit();
                        }
                    }
                }
            }
        }
    });

  5. #5
    Sencha User
    Join Date
    Oct 2009
    Posts
    23
    Vote Rating
    2
    ivanleblanc is on a distinguished road

      0  

    Default


    Hey great fix!
    As far as I can tell that one liner makes it work for now. (in 4.1.0 rc3)

  6. #6
    Ext JS Premium Member dnorman's Avatar
    Join Date
    Jan 2011
    Posts
    101
    Vote Rating
    54
    dnorman is a jewel in the rough dnorman is a jewel in the rough dnorman is a jewel in the rough

      0  

    Default


    I'm glad it worked for you.

    Sencha: Any way we can slip this one-liner into the GA release?
    Otherwise it's going to break a buttload of applications when people switch to 4.1.

  7. #7
    Sencha User
    Join Date
    Oct 2009
    Posts
    23
    Vote Rating
    2
    ivanleblanc is on a distinguished road

      0  

    Default


    So the 4.1 GA release still seemed to not work on its own, but it does work with your fix as far as I can tell. The commitRecord code does need to be copied and pasted though from the 4.1 GA release because it has changed form the beta releases.

  8. #8
    Ext JS Premium Member dnorman's Avatar
    Join Date
    Jan 2011
    Posts
    101
    Vote Rating
    54
    dnorman is a jewel in the rough dnorman is a jewel in the rough dnorman is a jewel in the rough

      0  

    Default


    Does sencha have a position on this issue?

Thread Participants: 2