Skip to content
This repository has been archived by the owner on Dec 15, 2023. It is now read-only.

Update method names #13

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Update method names #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
99a1c23
Update method names
Trudeaucj May 22, 2017
2206f04
Updating to wrapper for backwards compat
Trudeaucj May 22, 2017
1359714
Update index.js
Trudeaucj May 23, 2017
873ed01
Adding delete test
May 23, 2017
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
9 changes: 8 additions & 1 deletion src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ module.exports = function(config) {
* @returns {{get: get, save: save, all: all, allById: allById}}
*/
function getStorageObj(client, namespace) {
function delete(id, cb) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm running this locally, turns out delete is a reserve word, could you change it to something else? Also you won't need a semi-colon after a function declaration. A little whitespace after the function would be nice too 😄 Otherwise things look great!

client.hdel(namespace, [id], cb);
};
return {
get: function(id, cb) {
client.hget(namespace, id, function(err, res) {
Expand All @@ -48,8 +51,12 @@ function getStorageObj(client, namespace) {

client.hset(namespace, object.id, JSON.stringify(object), cb);
},
//remove is a non-standard name, but for compatbility included here
remove: function(id, cb) {
client.hdel(namespace, [id], cb);
return delete(id, cb);
},
delete: function(id, cb) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you mind covering this new method with tests? You can just copy the tests for remove and rename the describe and method call, since these should behave identically.

Copy link
Author

Choose a reason for hiding this comment

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

Done

return delete(id, cb);
},
all: function(cb, options) {
client.hgetall(namespace, function(err, res) {
Expand Down
110 changes: 74 additions & 36 deletions tests/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
var should = require('should'), sinon = require('sinon'), proxyquire = require('proxyquire').noCallThru();
var should = require('should'),
sinon = require('sinon'),
proxyquire = require('proxyquire').noCallThru();

require('should-sinon');

describe('Redis', function() {
var Storage,
redisMock,
redisClientMock,
defaultNamespace;
var Storage, redisMock, redisClientMock, defaultNamespace;

beforeEach(function() {
defaultNamespace = 'botkit:store';
Expand All @@ -15,26 +14,25 @@ describe('Redis', function() {
hget: sinon.stub(),
hset: sinon.stub(),
hdel: sinon.stub(),
hgetall: sinon.stub(),
hgetall: sinon.stub()
};

redisMock = {
createClient: sinon.stub().returns(redisClientMock)
};

Storage = proxyquire('../src/index', {'redis': redisMock});
Storage = proxyquire('../src/index', { redis: redisMock });
});

describe('initialization', function() {

it('should initialize redis with the config', function() {
var config = {};
Storage(config);
redisMock.createClient.should.be.calledWith(config);
});

it('should set a custom namespace', function() {
var config = {namespace: 'custom'};
var config = { namespace: 'custom' };
Storage(config);
config.should.have.property('namespace', 'custom');
});
Expand All @@ -47,8 +45,10 @@ describe('Redis', function() {

it('should create a default config', function() {
Storage();
redisMock.createClient.should.be.calledWith({namespace: defaultNamespace,
methods: []});
redisMock.createClient.should.be.calledWith({
namespace: defaultNamespace,
methods: []
});
});
});

Expand All @@ -62,7 +62,6 @@ describe('Redis', function() {
});

describe('get', function() {

beforeEach(function() {
sinon.spy(JSON, 'parse');
});
Expand All @@ -72,14 +71,16 @@ describe('Redis', function() {
});

it('should get by ID', function() {
var result = '{}',
cb = sinon.stub();
var result = '{}', cb = sinon.stub();

redisClientMock.hget.yields(null, result);

storageInterface[method].get('walterwhite', cb);

redisClientMock.hget.should.be.calledWithMatch(hash, 'walterwhite');
redisClientMock.hget.should.be.calledWithMatch(
hash,
'walterwhite'
);
JSON.parse.should.be.calledWith(result);
cb.should.be.calledWith(null, {});
});
Expand All @@ -91,7 +92,10 @@ describe('Redis', function() {

storageInterface[method].get('walterwhite', cb);

redisClientMock.hget.should.be.calledWithMatch(hash, 'walterwhite');
redisClientMock.hget.should.be.calledWithMatch(
hash,
'walterwhite'
);
JSON.parse.should.not.be.called;
cb.should.be.calledWith(null, null);
});
Expand All @@ -103,14 +107,16 @@ describe('Redis', function() {

storageInterface[method].get('walterwhite', cb);

redisClientMock.hget.should.be.calledWithMatch(hash, 'walterwhite');
redisClientMock.hget.should.be.calledWithMatch(
hash,
'walterwhite'
);
JSON.parse.should.not.be.called;
cb.should.be.calledWith(err, null);
});
});

describe('save', function() {

beforeEach(function() {
sinon.spy(JSON, 'stringify');
});
Expand All @@ -120,8 +126,7 @@ describe('Redis', function() {
});

it('should throw an error if ID is not provided', function() {
var obj = {},
cb = sinon.stub();
var obj = {}, cb = sinon.stub();

storageInterface[method].save(obj, cb);

Expand All @@ -130,7 +135,7 @@ describe('Redis', function() {
});

it('should save to redis', function() {
var obj = {id: 'heisenberg'}, cb = sinon.stub();
var obj = { id: 'heisenberg' }, cb = sinon.stub();

storageInterface[method].save(obj, cb);

Expand All @@ -146,7 +151,6 @@ describe('Redis', function() {
});

describe('remove', function() {

beforeEach(function() {
sinon.spy(JSON, 'stringify');
});
Expand All @@ -168,9 +172,29 @@ describe('Redis', function() {
});
});

describe('delete', function() {
beforeEach(function() {
sinon.spy(JSON, 'stringify');
});

describe('all', function() {
afterEach(function() {
JSON.stringify.restore();
});

it('should delete from redis', function() {
var id = 'heisenberg', cb = sinon.stub();

storageInterface[method].delete(id, cb);

redisClientMock.hdel.should.be.calledWith(
defaultNamespace + ':' + method,
['heisenberg'],
cb
);
});
});

describe('all', function() {
beforeEach(function() {
sinon.spy(JSON, 'parse');
});
Expand All @@ -180,8 +204,7 @@ describe('Redis', function() {
});

it('should call callback with error if redis fails', function() {
var cb = sinon.stub(),
err = new Error('OOPS!');
var cb = sinon.stub(), err = new Error('OOPS!');

redisClientMock.hgetall.yields(err);

Expand All @@ -193,44 +216,59 @@ describe('Redis', function() {

it('should return an array by default', function() {
var cb = sinon.stub(),
result = ['{"walterwhite":"heisenberg"}', '{"jessepinkman":"capncook"}'];
result = [
'{"walterwhite":"heisenberg"}',
'{"jessepinkman":"capncook"}'
];

redisClientMock.hgetall.yields(null, result);

storageInterface[method].all(cb);

redisClientMock.hgetall.should.be.calledWithMatch(hash);
JSON.parse.should.be.calledTwice;
cb.should.be.calledWithMatch(null, [{'walterwhite': 'heisenberg'}, {'jessepinkman': 'capncook'}]);
cb.should.be.calledWithMatch(null, [
{ walterwhite: 'heisenberg' },
{ jessepinkman: 'capncook' }
]);
});

it('should return an object if specified in options', function() {
var cb = sinon.stub(),
result = {key1: '{"walterwhite":"heisenberg"}', key2: '{"jessepinkman":"capncook"}'};
result = {
key1: '{"walterwhite":"heisenberg"}',
key2: '{"jessepinkman":"capncook"}'
};

redisClientMock.hgetall.yields(null, result);

storageInterface[method].all(cb, {type: 'object'});
storageInterface[method].all(cb, { type: 'object' });

redisClientMock.hgetall.should.be.calledWithMatch(hash);
JSON.parse.should.be.calledTwice;
cb.should.be.calledWithMatch(
null,
{key1: {'walterwhite': 'heisenberg'}, key2: {'jessepinkman': 'capncook'}}
);
cb.should.be.calledWithMatch(null, {
key1: { walterwhite: 'heisenberg' },
key2: { jessepinkman: 'capncook' }
});
});

it('should return an array if something other than object is specified in options', function() {
var cb = sinon.stub(),
result = ['{"walterwhite":"heisenberg"}', '{"jessepinkman":"capncook"}'];
result = [
'{"walterwhite":"heisenberg"}',
'{"jessepinkman":"capncook"}'
];

redisClientMock.hgetall.yields(null, result);

storageInterface[method].all(cb, {type: 'notobject'});
storageInterface[method].all(cb, { type: 'notobject' });

redisClientMock.hgetall.should.be.calledWithMatch(hash);
JSON.parse.should.be.calledTwice;
cb.should.be.calledWithMatch(null, [{'walterwhite': 'heisenberg'}, {'jessepinkman': 'capncook'}]);
cb.should.be.calledWithMatch(null, [
{ walterwhite: 'heisenberg' },
{ jessepinkman: 'capncook' }
]);
});
});
});
Expand Down