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

Empty arrays convert to objects with Redis backend since version 1.10 #96

Closed
juhoojala opened this issue Nov 30, 2016 · 5 comments
Closed

Comments

@juhoojala
Copy link

juhoojala commented Nov 30, 2016

Thanks for maintaining this project!

Since version 1.10, arrays convert to objects when stored with a redis backend. Version 1.9 works fine.

This is caused by the new Lua scripts in this commit:
401c537

See below link for more information:
http://openmymind.net/Lua-JSON-turns-empty-arrays-into-empty-hashes/

@adrai
Copy link
Contributor

adrai commented Dec 1, 2016

@rehia Can you take a look at it?

@rehia
Copy link
Contributor

rehia commented Dec 1, 2016

sure...
@juhoojala do you have an event as an example, so I can add a new test?

Thanks!

@rehia
Copy link
Contributor

rehia commented Dec 5, 2016

hi @juhoojala, I'd really need one of those examples.
I'd like to fix it with a test which reproduce it.
Thanks!

@juhoojala
Copy link
Author

juhoojala commented Dec 7, 2016

Hi @rehia, sorry for not getting back earlier!

package.json

{
  "name": "eventstore-bug",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "dependencies": {
    "eventstore": "^1.10.3",
    "redis": "^2.6.3"
  }
}

index.js:

var es = require('eventstore')({
  type: 'redis',
  host: 'localhost',                          // optional
  port: 16379,                                // note that standard port is 6379
  db: 0,                                      // optional
  prefix: 'eventstore',                       // optional
  eventsCollectionName: 'test-events',        // optional
  snapshotsCollectionName: 'snapshots',       // optional
  timeout: 10000                              // optional
});

es.init(function (err) {
  console.log('Initialized');
  es.getEventStream('streamId', function(err, stream) {
    stream.addEvent({ my: 'event', arr: []});
    stream.commit(function(err, stream) {
      console.log('Event added');
      es.getEvents('streamId', 0, 100, function(err, events) {
        console.log('Returned event:');
        console.log(events);
      });
    });
  });
});

Output

$ node index.js
Initialized
Event added
Returned event:
[ { restInCommitStream: 0,
    commitSequence: 0,
    aggregateId: 'streamId',
    commitStamp: 2016-12-07T08:06:12.794Z,
    streamRevision: 0,
    payload: { my: 'event', arr: {} },
    streamId: 'streamId',
    commitId: '3',
    id: '30' },
  next: [Function: nextFn] ]

I'm using node v.6.2.0 and the sameersbn/redis:latest docker image to run redis.

@rehia
Copy link
Contributor

rehia commented Dec 8, 2016

OK thanks @juhoojala for this.
The original event and the returned one will help.
Try to find some time in the coming days to find a fix.

rehia added a commit to rehia/node-eventstore that referenced this issue Jan 4, 2017

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
…ymore

Fixes thenativeweb#96
rehia added a commit to rehia/node-eventstore that referenced this issue Jan 4, 2017

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
…ymore

Fixes thenativeweb#96
rehia added a commit to rehia/node-eventstore that referenced this issue Jan 4, 2017

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
…ymore

Fixes thenativeweb#96
rehia added a commit to rehia/node-eventstore that referenced this issue Jan 4, 2017

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
…ymore

Fixes thenativeweb#96
@adrai adrai closed this as completed in #97 Jan 4, 2017
adrai pushed a commit that referenced this issue Jan 4, 2017

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
…ymore (#97)

Fixes #96
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants