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

Implement KeyValueAccessObject API #3

Merged
merged 2 commits into from
Aug 8, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
57 changes: 51 additions & 6 deletions lib/kv-redis.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
var assert = require('assert');
var Connector = require('loopback-connector').Connector;
var debug = require('debug')('loopback:connector:kv-redis');
var createPacker = require('./packer');
var Redis = require('ioredis');
var Command = Redis.Command;
var util = require('util');

exports.initialize = function initializeDataSource(dataSource, callback) {
Expand All @@ -23,17 +23,17 @@ exports.initialize = function initializeDataSource(dataSource, callback) {
.once('error', callback);
};

exports._Connector = RedisKeyValueConnector;

function RedisKeyValueConnector(settings, dataSource) {
Connector.call(this, 'kv-redis', settings);
this.dataSource = dataSource;

debug('Connector settings', settings);

this._client = new Redis(settings.url || settings);

this.DataAccessObject = function() {
// FIXME use KV DAO from juggler instead
};
this._packer = createPacker();
this.DataAccessObject = dataSource.juggler.KeyValueAccessObject;
};

util.inherits(RedisKeyValueConnector, Connector);
Expand All @@ -52,7 +52,7 @@ RedisKeyValueConnector.prototype.execute = function(command, args, cb) {
assert(typeof cb === 'function', 'callback must be a function');

debug('EXECUTE %j %j', command, args);
var cmd = new Command(command, args, 'utf8', function(err, result) {
var cmd = new Redis.Command(command, args, 'utf8', function(err, result) {
debug('RESULT OF %j -- %j', command, result);
cb(err, result);
});
Expand All @@ -62,3 +62,48 @@ RedisKeyValueConnector.prototype.execute = function(command, args, cb) {
RedisKeyValueConnector.prototype.disconnect = function(cb) {
this._client.quit(cb);
};

RedisKeyValueConnector.prototype.set =
function(modelName, key, value, options, callback) {
var composedKey = RedisKeyValueConnector._composeKey(modelName, key);
var rawData = this._packer.encode(value).slice();

var args = [composedKey, rawData];
if (options.ttl) {
args.push('PX');
args.push(options.ttl);
}

this.execute('SET', args, callback);
};

RedisKeyValueConnector.prototype.get =
function(modelName, key, options, callback) {
var composedKey = RedisKeyValueConnector._composeKey(modelName, key);
var packer = this._packer;
this.execute('GET', [composedKey], function(err, rawData) {
if (err) return callback(err);
var value = rawData !== null ? packer.decode(rawData) : null;
callback(null, value);
});
};

RedisKeyValueConnector.prototype.expire =
function(modelName, key, ttl, options, callback) {
var composedKey = RedisKeyValueConnector._composeKey(modelName, key);
this.execute('PEXPIRE', [composedKey, ttl], function(err, result) {
if (err) return callback(err);
if (!result) {
return callback(new Error(
'Key does not exist or the timeout cannot be set. ' +
'Model: ' + modelName + ' Key: ' + key));
}
callback();
});
};

RedisKeyValueConnector._composeKey = function(modelName, key) {
// FIXME escape values to prevent collision
// 'model' + 'foo:bar' vs 'model:foo' + 'bar'
return encodeURIComponent(modelName) + ':' + key;
};
17 changes: 17 additions & 0 deletions lib/packer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
'use strict';

var msgpack = require('msgpack5');

module.exports = function createPacker() {
var packer = msgpack({forceFloat64: true});
packer.register(1, Date, encodeDate, decodeDate);
return packer;
};

function encodeDate(obj) {
return new Buffer(obj.toISOString(), 'utf8');
}

function decodeDate(buf) {
return new Date(buf.toString('utf8'));
}
8 changes: 5 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,24 @@
},
"main": "lib/kv-redis.js",
"scripts": {
"test": "mocha test/integration/*.js",
"test": "mocha test/unit/*.js && mocha test/integration/*.js",
Copy link

Choose a reason for hiding this comment

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

I suspect this breaks the build-framework test runner. Can this not be run with a single mocha command?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, when running the tests locally, I prefer to run fast unit-tests first before the slower integration tests.

Is there any reasonable way how to use different test command for dev vs. CI while supporting Windows too?

Copy link

Choose a reason for hiding this comment

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

By listing them as mocha test/unit/*.js test/integration/*.js won't it test in that order? You could add --bail to get fail-fast, as well.

Copy link
Contributor

@superkhau superkhau Aug 16, 2016

Choose a reason for hiding this comment

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

I believe we should get the same effect by running mocha test/unit/*.js test/integration.js. However, --bail wouldn't have the same effect as it would only show a single error (the first one it fails on -- the current behaviour is it shows all failing unit tests or all integration tests together).

That said, I'm OK with --bail too as we should be fixing all failing tests anyways so whether we display them altogether at once or one by one via bail doesn't matter to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

By listing them as mocha test/unit/*.js test/integration/*.js won't it test in that order? You could add --bail to get fail-fast, as well.
I believe we should get the same effect by running mocha test/unit/*.js test/integration.js.

Interesting, it seems that this approach works. I'll use it then!

You could add --bail to get fail-fast, as well.

I prefer to see all test failures, it usually helps to understand whether it's a problem in the code (a single test failing) or perhaps something else (many tests failing because of CI environment problem).

"posttest": "npm run lint",
"lint": "eslint ."
},
"license": "MIT",
"dependencies": {
"debug": "^2.2.0",
"ioredis": "^2.2.0",
"loopback-connector": "^2.4.0"
"loopback-connector": "^2.4.0",
"msgpack5": "^3.4.0"
},
"devDependencies": {
"chai": "^3.5.0",
"dirty-chai": "^1.2.2",
"eslint": "^2.13.1",
"eslint-config-loopback": "^4.0.0",
"loopback-datasource-juggler": "^2.47.0",
"mocha": "^2.5.3"
"mocha": "^2.5.3",
"should": "^8.4.0"
}
}
8 changes: 8 additions & 0 deletions test/integration/juggler-api.integration.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
'use strict';

var createDataSource = require('../helpers/data-source-factory');
var expect = require('../helpers/expect');

describe('Juggler API', function() {
require('loopback-datasource-juggler/test/kvao.suite.js')(createDataSource);
});
26 changes: 26 additions & 0 deletions test/unit/compose-key.unit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
'use strict';

var composeKey = require('../..')._Connector._composeKey;
var expect = require('../helpers/expect');

describe('RedisKeyValueConnector._composeKey', function() {
it('honours the key', function() {
var key1 = composeKey('Car', 'vin');
var key2 = composeKey('Car', 'name');
expect(key1).to.not.equal(key2);
});

it('honours the model name', function() {
var key1 = composeKey('Product', 'name');
var key2 = composeKey('Category', 'name');
expect(key1).to.not.equal(key2);
});

it('encodes values', function() {
// This test is based on the knowledge that we are using ':' separator
// when building the composed string
var key1 = composeKey('a', 'b:c');
var key2 = composeKey('a:b', 'c');
expect(key1).to.not.equal(key2);
});
});