Support for Redis Sentinel

[XVincentX]

Connects #499

ioredis

Mock library issues

• The mock it's throwing exception when there's no data
• The mock is missing implementation for scan command, which we're using.
• The mock is missing support for reply transformers

Solution

• Get rid of the emulate option and use Redis for everything, even for testing on CircleCI (we should do that anyway) 4deacf9

Compatibility problems

• All methods already return a Promise, there's no need for promisify/Async version 5efe867

• hgetall reply must be modified to guarantee compatibility 6c12cfd

• exec function is using a Pipelining feature and has a different response type fix

•  bluebird is overriding the default Promise implementation and this is preventing the migrate framework to work detect a Promise and react accordingly issue 4ad3cfc

[XVincentX]

Interesting followups:

1. Take advantage of transparent key prefixing so we can remove some boilerplate code
2. Scan commands can return a stream — it is a great idea in order to avoid killing the server (both redis and the gateway)
3. Cluster support. This is a different connection object with a different set of connection options as well.

[DrMegavolt]

See one comment for tls test. once fixed\reimplemented let's merge (probably a matter of rebase)

[DrMegavolt]

this file was added to test ssl connection just recently why it is removed ?

[XVincentX]

Good question.
I deleted it temporarily because I'll have to figure out how to test that part.

The problem here is — ioredis gets created using a constructor and not using a function such as Redis.createClient()

sinon, the stub library we're using, does not support spies on constructors.

I deleted this test just to verify it was working correctly. I'll now figure out another way to test this part.

[DrMegavolt]

👍

[DrMegavolt]

If the emulate option is working I'm thinking that we can land this massive PR, and break in memory a bit. so it will be another bug to fix it

[XVincentX]

The emulate option is working correctly, yes.

I'm not really sure we should land this thing into master as it is.

I'm imagining somebody that wants to contribute or even simply trying the source code: they would clone the repository, install and run it: they would have a broke experience by default (as in-memory is not working and they would need a Redis instance), which is not very pleasant. I'd rather keep this branch here and resurrect it once ioredis-mock will be ready.

[agonbina]

Just for the record, I am using(developing against) this branch to run EG and I haven't run into any issues when running with a single redis host or the sentinel mode.

[DrMegavolt]

@agonbina thanks for active testing. the only reason we cannot merge it to master is some issues with InMemory mode. Which is implemented as mock of redis client. once fixed it will land into master

[XVincentX]

I've updated the ioredis-mock package and rebased with latest master.

Two points still have to be solved — see here

[XVincentX]

Rebase is going to be fun.

[DrMegavolt]

squash manually, it will be way easier.

[codecov]

Codecov Report

Merging #501 into master will decrease coverage by 0.07%.
The diff coverage is 90.9%.

@@            Coverage Diff             @@
##           master     #501      +/-   ##
==========================================
- Coverage   88.61%   88.54%   -0.08%     
==========================================
  Files         114      114              
  Lines        3540     3553      +13     
==========================================
+ Hits         3137     3146       +9     
- Misses        403      407       +4

Impacted Files	Coverage Δ
lib/services/consumers/application.service.js	91.3% <100%> (+0.49%)	⬆️
lib/services/tokens/token.dao.js	95.61% <100%> (+0.03%)	⬆️
lib/services/consumers/user.dao.js	97.82% <100%> (+0.04%)	⬆️
lib/services/consumers/application.dao.js	98.59% <100%> (+0.06%)	⬆️
lib/db.js	76.66% <75%> (-20%)	⬇️
...ices/authorization-codes/authorization-code.dao.js	90.47% <83.33%> (ø)	⬆️
lib/services/credentials/credential.dao.js	95.32% <96%> (+0.13%)	⬆️
lib/rest/routes/scopes.js	75.67% <0%> (+2.7%)	⬆️
bin/generators/scopes/create.js	100% <0%> (+7.69%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff7c212...447d910. Read the comment docs.

[DrMegavolt]

LGTM

[agonbina]

Nice to see this was merged! I will switch to using the master branch now 👍 I'm curious as to when is the next release planned?

[XVincentX]

Hard work, but we made it!

These two images will summarize last couple of hours here:

[DrMegavolt]

@agonbina waiting for multi-auth merge.
Expected Thursday Nov 16. If something goes wrong, then next week