Adding 'has' method

[david-byng]

Adding the .has method, used to ensure a key is available, to handle cases where .set("key", foo.bar) might result in a falsy value. EG:

function loadPreferencesForModule( moduleName ) {
    var key = "arbitraryPrefixString:" + moduleName;
    if ( ! myCache.has(key) ) {
        var promise = 
            fetchDataFromSomewhereQuiteSlow()
                .then( extractAKeyFromTheDataThatMightBeMissing );
                .then( function ( value ) {
                    return myCache.set(key, value);
                });
    } else {
        return new Promise(function(resolve, reject) {
            resolve( myCache.get(key) );
        });
    }
}

• so if we fetched a preferences object with fetchDataFromSomewhereQuiteSlow, then extracted the preferences for a module that hasn't had any preferences set yet, the correct value would be undefined. Rather than wait for the preferences to load every time we want to know about this module's preferences, we would want to avoid the call over the network and just return undefined until the cached value expires.

( If this is merged, please be aware I have assumed the next version is 3.0.2 in the README.md file. )

[mpneuried]

Since version 2.1.0 node-cache will return a undefined if the key was not found.
So wouldn't it be better to use just one .get()?

With your solution ( .has() + .get() method ) on false node-cache internally has to check key twice? Plus there is, of course a quite unlikely, risk to fall into a race-condition if the key expires while creating the Promise.

See .get() and _check()

Example:

function loadPreferencesForModule( moduleName ) {
    var key = "arbitraryPrefixString:" + moduleName;
    // get key and return it as Promise if existend
    var _val = myCache.get(key);
    // check for an existing value
    if ( typeof _val === "undefined" || _val === null ) {
        var promise = 
            fetchDataFromSomewhereQuiteSlow()
                .then( extractAKeyFromTheDataThatMightBeMissing );
                .then( function ( value ) {
                    return myCache.set(key, value);
                });
    } else {
        return new Promise(function(resolve, reject) {
            // return the already received value
            resolve( _val );
        });
    }
}

[david-byng]

Hi! Thanks for the example, but the problem is, what if undefined really is the correct value? Because its unwrapped inside the .get function, the caller can't tell the difference between "the value was missing so I returned undefined" and "you set the value to undefined, so I returned undefined". At present, the only way to check for the existence of a key who's value is undefined is to ask for .keys and check if .indexOf(key) > -1 (which doesn't run _check, so might return expired values anyway!).

I see your point about the race condition though; maybe an option to throwOnMissing (or similar) would make more sense. Then as the caller, wrap it in a try-catch block (or a promise body) and handle missing cache entries like any other error (or recover by creating them). I'll look into creating that as a separate PR.

[mpneuried]

Now i understand your problem.

What if you prevent to store undefined?

function loadPreferencesForModule( moduleName ) {
    var key = "arbitraryPrefixString:" + moduleName;
    // get key and return it as Promise if existend
    var _val = myCache.get(key);
    // check if the value is undefined
    if ( _val === undefined ) {
        var promise = 
            fetchDataFromSomewhereQuiteSlow()
                .then( extractAKeyFromTheDataThatMightBeMissing );
                .then( function ( value ) {
                    // prevent storing undefined by converting it to null
                    return myCache.set(key,  ( value === undefined ? null : value ) );
                });
    } else {
        return new Promise(function(resolve, reject) {
            // return the already received value
            resolve( _val );
        });
    }
}

node-cache will return undefined if the key not exists.
If you just store null you can decide if you have to call you script and set the cache.

I will think about and discus adding some code like value === undefined ? null : value to the .set() method.
So i could guaranty that a .get() with a return of undefined is a miss.

What do you think?

[david-byng]

That would work for a one-off, but it'd be better if the logic for that could be kept inside the cache; you can imagine someone might simply forget to put in the 'swap undefined to null' check. I've opened a PR for a throwOnMissing option at #46 , which I think makes more sense with reference to the race condition you mentioned.

[mpneuried]

Hi,
i checked your code and refactored it a little bit.
I renamed the option to errorOnMissing because in case of a callback the error will not "thrown".
So it's a more general name.

An i refactored the error handling and used the existing _error method.

[mpneuried]

Hope that's OK.