Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Singleton cache object #452

Closed
carrbrpoa opened this issue Aug 26, 2015 · 13 comments
Closed

Singleton cache object #452

carrbrpoa opened this issue Aug 26, 2015 · 13 comments

Comments

@carrbrpoa
Copy link

Hello,

First of all, I opened another issue, related to another widget. Adding to the info provided in the issue, there's an aditional behavior that seems to redraw the table and call renderCell in every table resize.

Based on this and in the fact that I'm not a Dojo expert, I decided to create some kind of cache, Singleton and injectable anywhere in the application. Ended with a class called ObjectCacheManager:

define([
        "dojo/_base/declare",
        "dojo/_base/lang",
        "dojo/json"
    ], function (
        declare,
        lang,
        JSON) {
    var ObjectCacheManager = declare("gis.dijit.ObjectCacheManager", [], {
            cachedData : {},

            addCachedData : function (identifier, data) {
                this.addCachedDataInternal(this.cachedData, identifier, data);
            },

            addCachedDataInternal : function (target, identifiers, data) {
                var currentIdentifier = identifiers.shift();
                if (!currentIdentifier) {
                    return;
                }
                if (!target[currentIdentifier]) {
                    var stringCacheData = '{"' + currentIdentifier + '": {}}';
                    var jsonCacheData = JSON.parse(stringCacheData);
                    lang.mixin(target, jsonCacheData);
                }

                if (identifiers.length === 0) {
                    target[currentIdentifier] = data;
                    return;
                }

                this.addCachedDataInternal(target[currentIdentifier], identifiers, data);
            },

            removeCachedData : function (identifier) {
                if (!identifier || identifier.length === 0) {
                    return;
                }
                var previousIdentifier = identifier.length === 1
                     ? identifier[0]
                     : identifier[identifier.length - 2];
                var cacheItem = this.searchCacheIdentifier(this.cachedData, identifier, true);
                delete cacheItem[previousIdentifier];
            },

            getCachedData : function (identifier) {
                var cacheItem = this.searchCacheIdentifier(this.cachedData, identifier);
                return cacheItem;
            },

            searchCacheIdentifier : function (searchTarget, identifiers, returnPrevious) {
                if (!identifiers || !searchTarget) {
                    return undefined;
                }
                if (identifiers.length === 0 || (returnPrevious && identifiers.length === 1)) {
                    return searchTarget;
                }
                var currentIdentifier = identifiers.shift();
                return this.searchCacheIdentifier(searchTarget[currentIdentifier], identifiers);
            }
        });
    if (!_instance) {
        var _instance = new ObjectCacheManager();
    }
    return _instance;
});

Example of use with Search widget's config:

define([ 'dojo/on', 'dojo/date/locale', 'dojo/topic', 'dojo/request', 'dojo/_base/array',
'gis/dijit/ObjectCacheManager' ], function(on, locale, topic, request, array, objectCache) {
(...)
gridOptions : {
    columns : [{
            id : 'Documents',
            field : 'EU',
            label : 'Exp. Uni.',
            width : 100,
            sortable : true,
            exportable : true,
            renderCell : function (object, value, node) {

                var cachedData = objectCache.getCachedData(['attributesTable', 'pesquisaEU', 'EU', value]);
                if (cachedData) {
                    (...)
                    return;
                }

                request.get("http://.../webservices/documents/123456789", {
                    handleAs : "json"
                }).then(function (data) {
                    if (data.count > 0) {
                        var nodeContents = '';
                        array.forEach(data.documents, function (documento, i) {
                            nodeContents += '<a href="' + documento.trim() + '" target="_blank">' + 'Doc.' + (i + 1) + '</a> ';
                        });

                        objectCache.addCachedData(['attributesTable', 'pesquisaEU', 'EU', value], data.documents);
                        (...)
                    }
                });
            }
        },
        (...)

The previous call to objectCache.addCachedData results in a "cached" json like this:

{
    attributesTable : {
        pesquisaEU : {
            EU : {
                "202238.9" : [...]
            }
        }
    }
}

Subsequent calls with different identifiers aggregates to this:

{
    attributesTable : {
        pesquisaEU : {
            EU : {
                "202238.9" : [...],
                "303255.1" : [...],
                "400012.5" : [...]
            }
        }
    }
}

This avoids that my request.get inside renderCell be called several times with every resize, because now it's cached. In another part of the application (clicking Search button, for example), I can clear that specific cache that I added with the "starter level" called attributesTable.

Do you guys thinks it's contributable or is it too specific for this? Better ideas, other ways? :)

@green3g
Copy link
Member

green3g commented Aug 26, 2015

I like this idea a lot. I think it could be really useful for different widgets that perform the same queries as others. First, they could check the cache manager for a key which could be the url path, and if it exists, use the data. Otherwise, run the query and save it to the cache manager.

One example would be the legend data. That gets requested from at least 2 widgets, potentially more.

Something like this would really be useful as a core module in cmv.

Other thoughts though, I think browsers tend to do their own caching of requests like that as well. I've seen in the network logs, quite often chrome will use a cached json query as opposed to requesting that data again. So maybe this would just be redundant.

@carrbrpoa
Copy link
Author

Nice, @roemhildtg. About your concern, it's true, I've seen some requests being made that return http status 304, but in the case I presented, all of the requests being made returned http status 200; I could even see the table lagging with all those requests (like 1000 with a small resize, including OPTION and GET requests - needed to wait several seconds to see my cell - just one - being filled).

@tmcgee
Copy link
Member

tmcgee commented Aug 27, 2015

I understand the concept and like the implementation. Somehow, making the initial request in the renderCell method just seems so wrong. ;) Perhaps I should be publishing a topic at a different place in the workflow - like when iterating through the result records after the QueryTask has completed.

Specific to this situation, it seems you could store the result(s) of your call in another field in the data store record and check that you have a value before making the request.

@carrbrpoa
Copy link
Author

Perhaps I should be publishing a topic at a different place in the workflow - like when iterating through the result records after the QueryTask has completed. - @tmcgee

I'm not sure if I get you; you mean firing an event for each result record?

Specific to this situation, it seems you could store the result(s) of your call in another field in the data store record and check that you have a value before making the request. - @tmcgee

Some example of that? :)

@tmcgee
Copy link
Member

tmcgee commented Aug 27, 2015

@carrbrpoa, I'm thinking something like this untested conceptual deferred:

topic.subscribe(topicID + '/queryResults', function (queryResults) {
    var calls = [];

    // loop through the results
    array.forEach(queryResults.features, function (feature) {
        // add your call for each feature to the calls array
        // the result of each call would be used to update the queryResults
    });

    // all done, now populate the Grid
    all(calls).then(function () {
        topic.publish(topicID + '/populateGrid', queryResults);
    });
})

Caveat: I have not actually used the topics mentioned above beyond a basic test to see that they are publishing/subscribing. If they don't work as expected, give a holler or better yet, submit a fix via a PR. ;)

@carrbrpoa
Copy link
Author

Hey @tmcgee, I'd liked your idea, but don't understand where should I place this code.

@tmcgee
Copy link
Member

tmcgee commented Aug 28, 2015

@carrbrpoa Since the anonymous function is wrapped inside a topic.subscribe, I'd expect you could put it most anywhere. Since it is associated with the search, I would put it at the top of the same search.js file where the original code was. Something like this?

define([
    any required modules like topic, etc
], function (
    vars for modules
) {

    topic.subscribe(topicID + '/queryResults', function (queryResults) {
        var calls = [];

        // loop through the results
        array.forEach(queryResults.features, function (feature) {
            // add your call for each feature to the calls array
            // the result of each call would be used to update the queryResults
        });

        // all done, now populate the Grid
        all(calls).then(function () {
            topic.publish(topicID + '/populateGrid', queryResults);
        });
    });

    return {
        map: true,
        mapClickMode: true,

        layers: [
            {
                name: 'Damage Assessment',

...

@carrbrpoa
Copy link
Author

Perfect, @tmcgee!
It worked as you said. See the beginning of my config/search.js:

define([ 'dojo/on', 'dojo/date/locale', 'dojo/topic', 'dojo/request', 'dojo/_base/array', 'dojo/promise/all' ],
        function(on, locale, topic, request, array, all) {

            function formatDateTime(value) {
                if (!value)
                    return '';
                var date = new Date(value);
                return locale.format(date, {
                    formatLength : 'short'
                });
            }

            topic.subscribe('pesquisaEU' + '/queryResults', function(queryResults) {
                var calls = [];

                // loop through the results
                array.forEach(queryResults.features, function(feature) {
                    // add your call for each feature to the calls array
                    // the result of each call would be used to update the
                    // queryResults
                    var findDocuments = 
                        request.get("http://.../webservices/documentos/123456789", {
                            handleAs : "json"
                        }).then(function(data) {
                            var dataToCache = [];
                            if (data.count > 0) {
                                feature.attributes.imagedDocuments = data.documentos[0]; //first one just to test
                                return true;
                            } else {
                                return false;
                            }
                        }, function(err) {
                            return false;
                        });

                    calls.push(findDocuments);
                });

                // all done, now populate the Grid

                all(calls).then(function () { 
                    topic.publish('pesquisaEU' + '/populateGrid', queryResults); 
                });
            });

            return {
                map : true,
                mapClickMode : true,
(...)

Kind of hardcoded stuff but I don't see another way.

Then, I'll handle my feature.attributes.imagedDocuments in a column's formatter or something, since it's an array and I want to present each item as an icon:

title : 'Expediente Único',
topicID : 'pesquisaEU',
gridOptions : {
    columns : [{
            id : 'Documents',
            field : 'imagedDocuments',
            label : 'Exp. Uni.',
            width : 100,
            sortable : true,
            exportable : true,
            formatter : function (value) {
                ...
            }
        }
        ...
    ],
...

Of course, I checked that the anonymous function is not called a thousand times, just what I want.
Thank you very much for this tip!

Now, what about the ObjectCacheManager stuff? :)

@tmcgee
Copy link
Member

tmcgee commented Aug 28, 2015

@carrbrpoa That is excellent to hear. Not having used those topics in such a way, I wasn't 100% it would work.

Yes we (I?) did get distracted from the original question. :) I do like it - perhaps directly in cmv core or as a mixin for the controller or as a "widget". I'd like to hear more about possible use cases to consider how/where it fits in best. @roemhildtg mentioned some good use cases. @cmv/core-committers what do you folks think?

@tmcgee tmcgee added this to the v1.3.4 milestone Aug 28, 2015
@carrbrpoa
Copy link
Author

Actually, during my wanderings, I've found this guy and this other one but I decided to build the ObjectCacheManager as a POC because I didn't want to handle the complexities.. :)

@DavidSpriggs
Copy link
Member

This little gem created by one of my Esri peeps (@thollingsheadesri) would do the trick:
https://github.com/thollingsheadesri/arcgis-server-store
You can use it with the cache mixin as well.

@carrbrpoa
Copy link
Author

Nice @DavidSpriggs, bookmarked for study :)

@tmcgee
Copy link
Member

tmcgee commented Nov 23, 2015

@DavidSpriggs Is arcgis-server-store something we'd want to include with CMV (mixin?) for the use cases such as those discussed in this issue?

@tmcgee tmcgee removed this from the v1.3.4 milestone Nov 23, 2015
@tmcgee tmcgee closed this as completed Sep 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants