CacheService may try to write value for deleted key

[Zsar]

Currently the "CacheService" does not perform the service of dropping outdated keys. Instead this must happen from the outside via invalidate. This means that keys may spontaneously be deleted silently and at any time.

return this._cacheContainer[key][1] = Promise.resolve(object); then fails because this._cacheContainer[key] is undefined.

Workaround: Always optional-chain.

Proper solution: Let the CacheService do the dropping, such that it may handle pending replies to dropped keys appropriately.

Design issue: How do we handle responses which only arrive after their key has already timed out. Either/or:

1. What we actually want to cache is values, not keys. We should TTL the values, not the keys. Only when the key is written to should TTL start counting down for it.
2. When a key times out before the value is written, discard the write (and emit a warning, I guess): This cache is not properly configured to handle the response times encountered, so they will never be cached.

... I prefer 1). @mreiche : Opinions?

[mreiche]

Fact is, the CacheService doesn't invalidate the outdated keys by its own, that is right. The documentation should be more clear here that you should implement your "garbage collector" on your own like that.

window.setInterval(() => {
    this._cacheService.invalidate(this._cacheService.outdatedKeys);
}, 10000);

1. What we actually want to cache is values, not keys. We should TTL the values, not the keys. Only when the key is written to should TTL start counting down for it.

I dont know what you mean. This is a key-value-store where you only can TTL the key.

2. When a key times out before the value is written, discard the write (and emit a warning, I guess): This cache is not properly configured to handle the response times encountered, so they will never be cached.

A write always refreshes the key TTL.
There is only one real issue for a race condition, when the loading callback takes too long.

this._cacheContainer[key] = [
    now,
    loadingFunction().then(object=>{
        return this._cacheContainer[key][1] = Promise.resolve(object);
    })
];

when the loadingFunction() takes more than the cacheTtlSeconds, the key could be deleted in the meantime and this._cacheContainer[key] became undefined. In this case, the better option should be to write:

this._cacheContainer[key] = [
    now,
    loadingFunction().then(object=>{
        const now = (Date.now()/1000);
        return this._cacheContainer[key] = [now, Promise.resolve(object)];
    })
];

That should fix it.

Opinions?

We should have tests for that.

[Zsar]

Mmh. Maybe I should demonstrate... created #42 .
... If the TTL is lower than the time to answer, then TTL-ing the key will result in never receiving it. So I figure, maybe we'd better TTL the value.