Memory Store in the chain of stores causes save to fail

[davidcl64]

I am currently using a memory store definition passed to child objects to keep readonly semantics on a global configuration file. However, calling save at the provider level results in the following exception:

TypeError: Object.keys called on non-object
    at Function.keys (native)
    at C:\util\build\node_modules\nconf\lib\nconf\common.js:96:12
    at Array.forEach (native)
    at Object.merge (C:\util\build\node_modules\nconf\lib\nconf\common.js:95:8)
    at [object Object].save (C:\util\build\node_modules\nconf\lib\nconf\provider.js:425:19)
    at...

Using a configuration hierarchy that looks something like:

    provider = new nconf.Provider({
        stores: [
            { name: 'workspace', type: 'file', file: 'workspace.json' },
            { name: 'buildId',   type: 'file', file: 'buildId.json' },
            { name: 'global',    type: 'memory', loadFrom: ['global.json'] }
        ]
    }); 

Calling save on the provider in turn triggers this logic:

  //
  // If we don't have a callback and the current 
  // store is capable of saving synchronously
  // then do so.
  //
  if (!callback) {
    return common.merge(names.map(saveStoreSync));
  }

The map function triggers saveStoreSync does the following:

    //
    // If the `store` doesn't have a `saveSync` method, 
    // just ignore it and continue. 
    //
    return store.saveSync
      ? store.saveSync()
      : null;

resulting in a null instead of the store object when the method saveSync is undefined on a particular store.

When common.merge is then called:

common.merge = function (objs) {
  var store = new Memory();

  objs.forEach(function (obj) {
    Object.keys(obj).forEach(function (key) {
      store.merge(key, obj[key]);
    });
  });

  return store.store;
};

No check is done on the store prior to using it resulting in the exception listed above.

My workaround for now is to monkey patch the memory store with dummy save/saveSync methods.

[rf]

I encountered this same problem and traced the same bit through the code.

Looking at the vows tests, I found this:

  "When using nconf": {                                                            
    "with the memory store": {                                                     
...                                                                             
      "the save() method": {                                                       
        "without a callback": {                                                    
          "should throw an exception": function () {                               
            assert.throws(function () { nconf.save() });                           
          }                                                                        

It, of course, does throw an exception (Object.keys called on non-object), but that's probably not the exception it should be throwing.

I was about to fix this and submit a pull request, but I'm not really sure what the intended behavior is here.

[jlank]

+1

[davidcl64]

In some ways this is similar to issue #20

From the investigation: "When you save your configuration, it does so for all the stores, including the 'system' store, for which saving makes no sense. However, for your 'file' store it works fine. Hence, you see an error but it still saves."

The fix quote: "This is fixed in nconf@0.5.0 if you try to save when you have a store which has no .save() or .saveSync() methods, we will not throw an error, we will just ignore it and not save."

My preference would be to see the same in the memory store.

[rf]

This should be fixed as of PR #38, would you mind testing this @davidcl64 and @jlank?

[davidcl64]

Will do - it might take me a bit to get to it, but the provider changes look good.

[pksunkara]

Fixed by #38