Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added setMany() functionality #74

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ Documentation
* [.getMany(keys, callback)](#module_storage.getMany)
* [.getAll(callback)](#module_storage.getAll)
* [.set(key, json, callback)](#module_storage.set)
* [.setMany(keys, callback)](#module_storage.setMany)
* [.has(key, callback)](#module_storage.has)
* [.keys(callback)](#module_storage.keys)
* [.remove(key, callback)](#module_storage.remove)
Expand Down Expand Up @@ -141,6 +142,37 @@ storage.set('foobar', { foo: 'bar' }, function(error) {
if (error) throw error;
});
```
<a name="module_storage.setMany"></a>

### storage.setMany(keys, callback)
**Kind**: static method of <code>[storage](#module_storage)</code>
**Summary**: Write multiple sets of user data
**Access:** public

| Param | Type | Description |
| --- | --- | --- |
| keys | <code>Object</code> | json object |
| callback | <code>function</code> | callback (error, finished) |

**Example**
```js
const storage = require('electron-json-storage');

storage.setMany({ foo: { bar: 'baz' }, baz: { bar: 'foo' } }, function(error, finished) {
if (error) {
console.log('Error saving ' + error.key);
throw error.error;
}
if (finished) console.log('finished');
});
```
The `error` property of the callback is an object with two properties:
- `error` (object), the error object
- `key` (string), the key that failed to save that failed to save).

This function will set each key one by one, for each error that occurs the callback will be invoked. Therefore the `finished`
parameter of the callback indicates if the function has finished saving all the data.

<a name="module_storage.has"></a>

### storage.has(key, callback)
Expand Down
54 changes: 54 additions & 0 deletions lib/storage.js
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,60 @@ exports.set = function(key, json, callback) {
], callback);
};

/**
* @summary Write multiple sets of user data
* @function
* @public
*
* @param {Object} keys
* @param {Function} callback - callback (error, done)
*
* @example
* const storage = require('electron-json-storage');
*
* storage.setMany({ foo: 'bar', bar: 'foo' }, function(error, done) {
* if (error) {
* console.log('Error saving ' + error.key);
* throw error.error;
* }
* if (finished) console.log('finished');
* });
*/
exports.setMany = function(keys, callback) {
if (!keys || keys.constructor !== Object) {
callback(new Error('Invalid keys'), true);
return;
}

const entries = _.map(keys, function(value, key) {
return { key: key, value: value };
});

function process() {
const entry = entries.shift();
const key = entry.key;
const value = entry.value;

exports.set(key, value, function(error) {
if (error) {
callback({ error: error, key: key }, entries.length === 0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be good if instead of simply returning if things were successful or not, we would return the list of properties that failed (or the ones that succeeded), so in the case of an error, the user knows which ones to retry?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was originally what I had planned. With the way I ended up doing it, the user will still know which ones to retry (it will set the keys one by one, and every time there is an error it will invoke the callback which is why the callback takes a second parameter finished to let the user know when all the keys have been set).

It's up to you. If we did it your way, would we simply return a list of failed keys (['foo', 'bar']) or the errors that they threw too ({ foo: error1, bar: error2 })?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I think we should return a list of failed keys along with their respective errors, and only call the final callback once.

}

if (entries.length > 0) {
process();
} else if (!error) {
callback(null, true);
}
});
}

if (entries.length > 0) {
process();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove a lot of boilerplate by reusing async's parallel function (http://caolan.github.io/async/docs.html#parallel), or even parallelLimit, so we only have X amount of writes at any given time.

Copy link
Author

@Rob-- Rob-- Jul 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I replied on #73 about using async, but the if statement is not necessary at all, it is just to ensure that in the event someone does storage.setMany({}) that an error will not be thrown. I've never used async.parallel so I'm unsure if would be able to handle an event in which no keys are passed to the function.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure if would be able to handle an event in which no keys are passed to the function.

Fair enough. The current approach looks good then :)

} else {
callback(null, true);
}
};

/**
* @summary Check if a key exists
* @function
Expand Down
39 changes: 39 additions & 0 deletions tests/storage.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,46 @@ describe('Electron JSON Storage', function() {
});

});

describe('.setMany()', function() {

it('should yield an error if no key', function(done) {
storage.setMany(null, function(error, finished) {
m.chai.expect(error).to.be.an.instanceof(Error);
m.chai.expect(error.message).to.equal('Invalid keys');
done();
});
});

it('should be able to store multiple valid JSON objects', function(done) {
async.waterfall([
function(callback) {
storage.setMany({
foo: { bar: 'baz' },
baz: { bar: 'foo' }
}, callback);
},
function(finished, callback) {
m.chai.expect(finished).to.be.true;
storage.getMany(['foo', 'baz'], callback);
}
], function(error, data) {
m.chai.expect(data.foo).to.deep.equal({ bar: 'baz' });
m.chai.expect(data.baz).to.deep.equal({ bar: 'foo' });
done();
});
});

it('should ignore keys that are empty objects', function(done) {
storage.setMany({}, function(error, finished) {
m.chai.expect(error).to.not.exist;
m.chai.expect(finished).to.be.true;
done();
});
})

});

describe('.has()', function() {

it('should yield an error if no key', function(done) {
Expand Down