toystars / node-elasticsearch-sync

ElasticSearch and MongoDB sync module for node
MIT License
17 stars 11 forks source link

Document id bson vs. string #13

Closed nuest closed 7 years ago

nuest commented 7 years ago

I've had an issue inserting documents using a BSON id straight from MongoDB. If found a solution, but wonder if there is a particular reason why here

https://github.com/toystars/node-elasticsearch-sync/blob/master/lib/util/crud.js#L39

document.id is used "as-is" in the insert function, while in the remove and update function, id.toString() is used, see

https://github.com/toystars/node-elasticsearch-sync/blob/master/lib/util/crud.js#L62 https://github.com/toystars/node-elasticsearch-sync/blob/master/lib/util/crud.js#L86

Wouldn't it be OK to use id: document.id.toString() in the insert function as well?

toystars commented 7 years ago

Using id: document.id.toString() also in the insert function should be the ideal way to go. That must have slipped by. There is no reason why document.id is used as-is. Thanks for the observation.

nuest commented 7 years ago

Ok, I'll create a PR...