3

Closed

async on version 2013.01 and caching

description

From https://spservices.codeplex.com/discussions/444799



Marc,
This is going to be long… I did not want to post yesterday night because I wanted to first do some testing so that I could prove my theories/observations... :)

(on a side note, before I begin: good job today at SPChat)

Issue 1: When cacheXML is true, completefunc() is not provided with the ajax “status”

This one is an easy fix… You already have the Deferred object, which is resolved because it was cached… Just “add yourself” on to the chain and call the completefunc… Because the Deferred is resolved, your callback is executed immediately. Here is the code:
// Call the completefunc if there is one
if(thisHasCompletefunc) {
        
    cachedPromise.done(function(xData, status){

        opt.completefunc(xData, status);
      
    });
        
}
// Return the cached promise
return cachedPromise;
You don’t want to force the status to be successful because you don’t currently have code that prevents the caching of the request if it status is ‘error’… So even if the request failed to be sent to the server (ex. invalid XML) it would still be cached (more on caching below)…

When I initially saw this yesterday, I was concerned as to why I have not seen this in the application I’m mostly working with now a days… Specially since I do have instances where I use cacheXML true (ex. retrieving list information) and I normally check the status field for “error” or “success”... Answer: I’m still using v0.7.2 which does not have this issue – you were using the cacheXML input option to determine if it should be cached or not.

(I also checked the “Black Magic for White Hats SharePoint” project I recently did and looks like I check for “error” explicitly and don’t do something like “If not success”, so I’m good there as well)

Issue 2: SPServices is always caching requests

This one is the more concerning one and the one I wanted to do more testing on… SPServices currently caches all Ajax requests regardless if the user sets cacheXML or not! (all operations – queries and updates a like)

My concerns are around memory and performance on the browser… From a functional standpoint, you still return the proper response based on cacheXML setting.
For a “casual” user, this will not be an issue – they make a few calls to SPServices… no problem… but… What about SPA’s (Single page applications)? These can make hundreds, if not thousands of calls for data during the life of the browser (tab) session… The memory footprint can grow to be very large. This all seems to have been introduced with the move to supporting Promises.

(At this point I should say: shame on me… for not contributing to your request for feedback when you first implemented the cacheXML and Promises functionality… specially since I have a vested interest in this library for my own projects :) )

I have a code suggestion which supports the following (personal) feeling toward caching – which echo’s the principle behind this original post about the async flag:
  1. Only cache when asked (cacheXML: true)
  2. Keep cached version until a subsequent request comes through with cacheXML == false… at this point: delete the cached version from memory.
I’m not going to worry about “cacheXML” being true by default (in the defaults object), which would make updates (ex. UpdateListItems) not actually make updates in some cases… (in SPA pages, it could happen). I would suggest adding something to the documentation under the cacheXML option that most likely 1) it should not be set globally... and 2) should probably not be used on updates.


Proposed Changes:

The change is fairly simple and only moves around some of your variables that you already defined… basically, you assign the ajax object to cachedPromise and only add that to promisesCache[] object if cacheXML is true. I took your code from v2013.01 and made only a few edits... Here it is:
// Check to see if we've already cached the results
var cachedPromise;
if(opt.cacheXML) {
    cachedPromise = promisesCache[msg];
}

// If we don't have a completefunc, then we won't attempt to call it
var thisHasCompletefunc = $.isFunction(opt.completefunc);

if(typeof cachedPromise === "undefined") {

    // Finally, make the Ajax call
    cachedPromise = $.ajax({
        // The relative URL for the AJAX call
        url: ajaxURL,
        // By default, the AJAX calls are asynchronous.  You can specify false to require a synchronous call.
        async: thisHasCompletefunc ? false : opt.async,
        // Before sending the msg, need to send the request header
        beforeSend: function (xhr) {
            // If we need to pass the SOAPAction, do so
            if(WSops[opt.operation][1]) {
                xhr.setRequestHeader("SOAPAction", SOAPAction);
            }
        },
        // Always a POST
        type: "POST",
        // Here is the SOAP request we've built above
        data: msg,
        // We're getting XML; tell jQuery so that it doesn't need to do a best guess
        dataType: "xml",
        // and this is its content type
        contentType: "text/xml;charset='utf-8'",
        complete: function(xData, Status) {
            // When the call is complete, call the completefunc if there is one
            if(thisHasCompletefunc) {
                opt.completefunc(xData, Status);
            }
        }
    });
    
    // If cache is true, then store the ajax request
    if (opt.cacheXML) {
    
        promisesCache[msg] = cachedPromise;
    
    // ELSE, cache must be false... Remove cached version (case it exists)
    } else if (promisesCache[msg]) {
        
        delete promisesCache[msg];
        
    }
    
    // Return the promise
    return cachedPromise;

}
I wish I had line number to tell you exactly what's I'm suggesting... Hopefully its not to hard to pick out...

Testing Code:

I create the script below that I loaded dynamically through firebug and played with from the console by calling the functions…
/**
 * Validate theory of v2013.01:
 *      1.  Always caches requests, regardless of cacheXML settings.
 *          Memory/Performance impact.
 *      2.  completefunc() receives different input params when using cached request.
 * 
 *
 */

// Load script via dom so that we can see it in firebug
function loadSPServices(url) {

    // Insert SPSErvices into page
    // Need to use a non-minified version for debuging/stop points.
    var head    = document.head || document.getElementsByTagName("head")[0] || document.documentElement,
        s       = document.createElement("script");
    s.type      = "text/javascript";
    s.src       = url;
    s.onload = s.onreadystatechange = function( _, isAbort ) {
        if ( isAbort || !s.readyState || /loaded|complete/.test( s.readyState ) ) {
        
            s.onload = s.onreadystatechange = null;
            s = undefined;
            
            console.log( "SPServices v" + $().SPServices.Version() );

        }
    };
    head.insertBefore(s, head.firstChild);
        
}

function makeSPCall(options, loopCount, list) {
    
    if (!loopCount) {
        
        loopCount = 1;
        
    }
        
    var n   = 1,
        opt = $.extend(
                {},
                {
                    operation: "GetListItems",
                    listName:   "Announcements",
                    cacheXML:   false,
                    CAMLRowLimit: 1,
                    completefunc: function(xData, status){
                        
                        console.log("status: " + status);
                        console.log("xData: ");
                        console.log(xData);
                        
                    }
                },
                options
            );
    
    
    
    for(; n<=loopCount; n++){
      
        $().SPServices( $.extend(opt, { CAMLRowLimit: n }) );
      
    };
    
}


function makeUpdate(itemId, cache) {
    
    $().SPServices({
        operation:  "UpdateListItems",
        listName:   "Announcements",
        cacheXML:   (cache === true ? true : false),
        batchCmd:   "Update",
        ID:         itemId,
        valuepairs: [
            ["Title", "Title updated..."]
        ],
        completefunc: function(xData, status){
            
            console.log("status: " + status);
            console.log("xData: ");
            console.log(xData);
            
        }
    });

}


console.log("TEST FILE: Loaded");
Hope this helps in explaining my concerns and in testing... On a plus side: I used Chrome's profile tools to do Heap snapshots... I eventually crashed Chrome... :) (a few times)


Paul.

file attachments

Closed Dec 16, 2014 at 5:34 PM by sympmarc
Fixed/deployed in 2014.02

comments

ptavares wrote Jun 1, 2014 at 3:21 PM

Hey Marc.
Ran across one of the items on this issue (callback not given correct input params on cached requests). This reminded me how dangerous the other issue here (always caching) is to SPAs... It significantly increases the memory footprint over time because all requests are being cached regardless of the cache flag.
For the time being I have created my own GetListItem function and made changes to a local copy of SPServices I use in my SPWidgets project.

sympmarc wrote Jun 2, 2014 at 1:22 PM

Paul:

This one slipped through the cracks on me - so much for marking something with high impact. Obviously your concerns here are valid (and may well explain a few things I've seen). I'm interested in the changes you've made to patch your copy. I could implement them and give you a pointer to my dev copy for testing.

M.

ptavares wrote Jun 3, 2014 at 3:15 PM

I'll check my version again, but I'm pretty sure the change I made are the one detailed above in the post... I'll try to do that tonight... :)

sympmarc wrote Jun 3, 2014 at 3:51 PM

Thanks, Paul.

ptavares wrote Jun 7, 2014 at 3:44 PM

Marc,
Sorry for the delay... See attached for the version that includes the fixes I mention in this post.

ptavares wrote Jun 8, 2014 at 3:35 PM

Marc,
Was looking at my proposed fix here and playing with it... Specifically the proposed code above to remove the promise from Cache... In the proposal, the cached version is removed if a subsequent call is made with Cache = False, which I now realize is probably not the best approach... A better one is to update the cached version with the latest promise... So I changed that if statement to this:
            // If cache is true
            // OR
            // we have this message cached from a prior call,
            // then update it with this most current version
            if (opt.cacheXML || promisesCache[msg]) {

                promisesCache[msg] = cachedPromise;

            }
Hope this helps.