From 36e061c4bda8d79f657dc24b1dcf1937f31d7efe Mon Sep 17 00:00:00 2001
From: Russell Frank <russ.frank@rutgers.edu>
Date: Tue, 1 May 2012 22:31:53 -0400
Subject: [PATCH] Fixes to `Provider.save()` and tests.

Fixed `Provider.save()` to properly ignore stores which do not provide
a saveSync method.  Also, fixed `save()` to properly save asynchronously
when an async `save()` method on a store is provided.

Removed the tests from `nconf-test.js` which expected `save()` to throw
or return an error when a store without `save()` methods was
encountered. Also removed a `console.log` from `provider-test.js`.
---
 lib/nconf/common.js   |  2 +-
 lib/nconf/provider.js | 52 ++++++++++++++++++++++++++-----------------
 test/nconf-test.js    | 17 +-------------
 test/provider-test.js |  3 +--
 4 files changed, 35 insertions(+), 39 deletions(-)

diff --git a/lib/nconf/common.js b/lib/nconf/common.js
index 730de70..b6e16d9 100644
--- a/lib/nconf/common.js
+++ b/lib/nconf/common.js
@@ -108,4 +108,4 @@ common.merge = function (objs) {
 //
 common.capitalize = function (str) {
   return str && str[0].toUpperCase() + str.slice(1);
-};
\ No newline at end of file
+};
diff --git a/lib/nconf/provider.js b/lib/nconf/provider.js
index c1a5de7..1ce89ed 100644
--- a/lib/nconf/provider.js
+++ b/lib/nconf/provider.js
@@ -373,11 +373,14 @@ Provider.prototype.load = function (callback) {
 };
 
 //
-// ### function save (value, callback)
-// #### @value {Object} **Optional** Config object to set for this instance
-// #### @callback {function} Continuation to respond to when complete.
-// Removes any existing configuration settings that may exist in this
-// instance and then adds all key-value pairs in `value`.
+// ### function save (callback)
+// #### @callback {function} **optional**  Continuation to respond to when 
+// complete.
+// Instructs each provider to save.  If a callback is provided, we will attempt
+// asynchronous saves on the providers, falling back to synchronous saves if
+// this isn't possible.  If a provider does not know how to save, it will be
+// ignored.  Returns an object consisting of all of the data which was
+// actually saved.
 //
 Provider.prototype.save = function (value, callback) {
   if (!callback && typeof value === 'function') {
@@ -388,32 +391,41 @@ Provider.prototype.save = function (value, callback) {
   var self = this,
       names = Object.keys(this.stores);
 
-  function saveStoreSync(name) {
+  function saveStoreSync(memo, name) {
     var store = self.stores[name];
 
     //
     // If the `store` doesn't have a `saveSync` method,
     // just ignore it and continue.
     //
-    return store.saveSync
-      ? store.saveSync()
-      : null;
+    if (store.saveSync) {
+      var ret = store.saveSync();
+      if (typeof ret == 'object' && ret !== null) {
+        memo.push(ret);
+      }
+    }
+    return memo;
   }
 
-  function saveStore(name, next) {
+  function saveStore(memo, name, next) {
     var store = self.stores[name];
 
     //
     // If the `store` doesn't have a `save` or saveSync`
     // method(s), just ignore it and continue.
     //
-    if (!store.save && !store.saveSync) {
-      return next();
-    }
 
-    return store.saveSync
-      ? next(null, store.saveSync())
-      : store.save(next);
+    if (store.save) {
+      store.save(function (err, data) {
+        if (err) return next(err);
+        if (typeof data == 'object' && data !== null) {
+          memo.push(data);
+        }
+      });
+    } else if (store.saveSync) {
+      memo.push(store.saveSync());
+    }
+    next(null, memo);
   }
 
   //
@@ -422,11 +434,11 @@ Provider.prototype.save = function (value, callback) {
   // then do so.
   //
   if (!callback) {
-    return common.merge(names.map(saveStoreSync));
+    return common.merge(names.reduce(saveStoreSync, []));
   }
 
-  async.map(names, saveStore, function (err, objs) {
-    return err ? callback(err) : callback();
+  async.reduce(names, [], saveStore, function (err, objs) {
+    return err ? callback(err) : callback(null, common.merge(objs));
   });
 };
 
@@ -488,4 +500,4 @@ function onError(err, callback) {
   }
 
   throw err;
-}
\ No newline at end of file
+}
diff --git a/test/nconf-test.js b/test/nconf-test.js
index 00682bd..111a4f0 100644
--- a/test/nconf-test.js
+++ b/test/nconf-test.js
@@ -101,22 +101,7 @@ vows.describe('nconf').addBatch({
             });
           }
         }
-      },
-      "the save() method": {
-        "without a callback": {
-          "should throw an exception": function () {
-            assert.throws(function () { nconf.save() });
-          }
-        },
-        "with a callback": {
-          topic: function () {
-            nconf.save(this.callback.bind(null, null)); 
-          },
-          "should respond with an error": function (ign, err) {
-            assert.isNotNull(err);
-          }
-        }
       }
     }
   }
-}).export(module);
\ No newline at end of file
+}).export(module);
diff --git a/test/provider-test.js b/test/provider-test.js
index 60e2783..158b40c 100644
--- a/test/provider-test.js
+++ b/test/provider-test.js
@@ -99,7 +99,6 @@ vows.describe('nconf/provider').addBatch({
 
             helpers.assertMerged(null, merged);
             assert.equal(merged.candy.something, 'file1');
-            console.log(provider.sources);
           }
         },
         "when multiple stores are used": {
@@ -117,4 +116,4 @@ vows.describe('nconf/provider').addBatch({
       }
     }
   }
-}).export(module);
\ No newline at end of file
+}).export(module);