Issue with MGet (optional flag and cache)

[kefniark]

Description

Issue found with Archivist.MGet when using the flag optional = true in the current code, MGet crash on the first null element of the result and throw a nullReferenceException.

This happen because this element with a null value is not saved in the cache.

How to Test

In the following code, if any of the seasonId in queries doesn't exist in archivist, the MGet callback will return ("NullReferenceException", null)

var queries = new JObject();

foreach (var seasonId in seasonIds) {
    var rankingHistory = new JObject() {
        {"topic", new JValue(topicId)}, {
            "index", new JObject() { {"rankingId", new JValue(seasonId) } }
        }
    };

    // Creating Query
    queries.Add(seasonId, rankingHistory);
}

// Add Option: Optional
var options = new JObject();
options.Add("optional", new JValue(true));

Mage.Archivist.MGet(queries, options, callback);

Comment

• This PR is about MGet(), Archivist.Get() seems to not support this optional flag (and probably have the same cache issue). We should probably open a new issue to implement this feature later.
• Is it necessary to add something in the responseObject anyway?
• Does that solve the issue with Smartbeat?
• Is it fine and match the current JS Implementation ? @ronkorving @AlmirKadric

[ronkorving]

Re. the JS implementation, if a value does not exist, it is cached with value undefined. So when doing a 2nd optional get, it will simply return that undefined and not hit the server again.

[kefniark]

After discussion, I changed a little bit this PR based on the JS Version:

• MGet add the null value in the response object
• VaultValue stores the null values in the cache

Tested, it seems to properly work on our project

[AlmirKadric]

Completely my fault for not being more involved in the beginning. We fixed the problem in the wrong way. The issue is that when ValueDel is called we should be instantiating an empty VaultValue into the cache before null'ing it out. This is what ValueSet does. The delete logic is required as it throws other events that we need.

[kefniark]

ok let's restart another PR from scratch if it's to remove everything