Existing JSON formatted data broken in v0.1.10 and above

[Bringer128]

The tl;dr: Data stored from v0.1.9 and below is surrounded by extra quotes and needs to be migrated to work in v0.1.10 and above. Could this be documented in the README.md?

I'm currently using redisdown with level-sublevel and levelup to store JSON objects in a Redis DB.

I upgraded today from redisdown 0.1.6 to 0.1.12. and found my calls to get JSON encoded data (levelup feature) has started returning JSON strings instead of objects.

I did a quick upgrade through the versions and found that it was introduced in 0.1.10 which was the release for binary support. Debugging showed me that the diff was in the ._get method on the RedisDown prototype:

Old version:

RedisDown.prototype._get = function (key, options, cb) {
    this.db.hget(this.location+':h', key, function(e, v) {
        if (e) { return cb(e); }
        if (v === null || v === undefined) { return cb(new Error('NotFound error ' + key)); }
        var json;
    if (v === '') {
      json = v;
    } else {
        try {
            json = JSON.parse(v);
        } catch(e) {
            return cb(e);
        }
    }
      if (options.asBuffer === false || options.raw) {
        cb(null, json);
    } else if (json === null || json === undefined) {
            cb(null, new Buffer(''));
      } else {
            cb(null, new Buffer(json));
        }
    });
};

New version:

RedisDown.prototype._get = function (key, options, cb) {
    this.db.hget(this.location+':h', key, function(e, v) {
        if (e) { return cb(e); }
        if (v === null || v === undefined) { return cb(new Error('NotFound error ' + key)); }

      if (options.asBuffer === false || options.raw) {
        cb(null, String(v || ''));
    } else if (v === null || v === undefined) {
            cb(null, new Buffer(''));
      } else {
            cb(null, new Buffer(v));
        }
    });
};

It looks like the crux of the difference is that you no longer call JSON.parse(v).

Data created in v0.1.10 is parsed out correctly in v0.1.10 so it looks like a data migration issue.

Could you put up a warning on the README at a minimum about needing to upgrade existing data from pre-v0.1.10?

[hmalphettes]

@Bringer128 many thanks for pin-pointing this.

Indeed I did not realise we were breaking backward compatibility right there.: #17 (comment)

I am working on a warning.

Would it be useful to document how to override RedisDown.prototype._get to provide backward compatibility?

[hmalphettes]

Here is the suggested warning:
https://github.com/hmalphettes/redisdown/blob/master/README.md#warning-data-migration-from-redisdown-v019

[Bringer128]

The warning looks good.

What was your suggestion regarding overriding _get? I could see any attempt to handle it transparently clashing if anyone is legitimately storing JSON strings.

Perhaps the easiest solution is to provide a data migration utility or suggestions on how to write one? It should be sufficient in a data migration to just read out the value as a string, call JSON.parse(value) and write the resulting JSON object back.

[hmalphettes]

@Bringer128 I was trying to draft a workaround but indeed failed!
So iterate over the values that are expected to be JSON objects. Parse them and rewrite them.
Thanks for the hint.