Skip to content
This repository has been archived by the owner on Feb 11, 2020. It is now read-only.

Not correctly used LRANGE in redis persistence #151

Closed
mocheng opened this issue Jun 18, 2014 · 1 comment · Fixed by #154
Closed

Not correctly used LRANGE in redis persistence #151

mocheng opened this issue Jun 18, 2014 · 1 comment · Fixed by #154

Comments

@mocheng
Copy link
Contributor

mocheng commented Jun 18, 2014

In https://github.com/mcollina/mosca/blob/master/lib/persistence/redis.js line 319, it is

RedisPersistence.prototype.streamOfflinePackets = function(client, cb) {
  var that = this;

  if (this._cleanClient(client)) {
    return;
  }

  that._client.lrange("packets:" + client.id, -1, 10000, function(err, results) {
  }

It looks the usage of LRANGE is not correct.

According to http://redis.io/commands/lrange , the command "LRANGE packets:client_id -1 1000" would only get last element in "packets:client_id". As a result, streamOfflinePackets only send out one offline packet. I suppose this is not correct.

Please correct me if I'm wrong.

@mcollina
Copy link
Collaborator

You are right! It should be LRANGE packets:client_id 0 1000.
Can you please submit a pull request with a unit test?

@mocheng mocheng mentioned this issue Jun 23, 2014
mcollina added a commit that referenced this issue Jun 23, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants