Generate identifiers for event documents rather than deriving them

[tpaulshippy]

When writing event documents to the underlying store, the ID should be able to be a consistent known format, rather than derived from the commit sequence appended to the commit ID. From a data store perspectives, if someone is using UUIDs, they expect UUIDs everywhere. Or if it's mongo ObjectIds, they should be consistent. This enables data portability with consistent data types for identifiers.

This PR should address the asynchronous support issue from my previous PR.

Thanks @nanov for maintaining this project.

[nanov]

Thank you very much!

In short, iI really don't want to mess with the id logic right now. I also don't see any specific gains from this change and this could possibly lead to some confusion with existing consumers.

Just a few points:

• Ids are stored as strings anyways ( for now ), i=I really don't see the necessity of this ( possibly breaking for some consumers ) change right now.

• The current implementation as it is, relays on the DB driver to generate ids, this means that those won't be consistent between DBs anyways. ( certainly something that needs to be improved ).

• I suggest that you take a look at the position option, this gives each id a unique, auto-incremented position property, which can be used for ordering, or identification.

[tpaulshippy]

• Ids are stored as strings anyways ( for now ), i=I really don't see the necessity of this ( possibly breaking for some consumers ) change right now.

Obviously, we disagree. It is only because the Ids are currently stored as strings that this code even works. If Ids were stored as any other data type, this code would break down. Two examples off the top of my head: integers and UUIDs. In both cases, this code would be problematic.

• The current implementation as it is, relays on the DB driver to generate ids, this means that those won't be consistent between DBs anyways. ( certainly something that needs to be improved ).

If the current implementation did rely on the DB driver to generate ids, I wouldn't have requested this change. The truth is the current implementation relies on the driver-specific class to generate ids EXCEPT for the event id (which is the most visible id because it is the id of the top level item in the data store). That id and that id alone is generated by appending two strings together (a generated id and an integer).

• I suggest that you take a look at the position option, this gives each id a unique, auto-incremented position property, which can be used for ordering, or identification.

I'm not sure how another property on the event document (position) will help alleviate my concerns about the id of that document. Interesting functionality, but it doesn't really help me here.

If this were an opt-in that didn't affect existing consumers, would you be more willing to consider it? What if it were an optional way to override line 335 of mongodb.js? Let me explain my exact use case. Our team wants all documents to by identified by a version 4 UUID in our mongo data stores and I'd really rather not have my own version of mongodb.js. I'm already monkey patching the getNewId function. But monkey patching a one line function (getNewId) is a lot more palatable than a 78 line function (addEvents).

Thank you for your consideration and for engaging in this discussion.

[nanov]

Obviously, we disagree. It is only because the Ids are currently stored as strings that this code even works. If Ids were stored as any other data type, this code would break down. Two examples off the top of my head: integers and UUIDs. In both cases, this code would be problematic.

There is a reason why IDs are stored as strings.

If the current implementation did rely on the DB driver to generate ids, I wouldn't have requested this change. The truth is the current implementation relies on the driver-specific class to generate ids EXCEPT for the event id (which is the most visible id because it is the id of the top level item in the data store). That id and that id alone is generated by appending two strings together (a generated id and an integer).

I do get your point here, you are right.

I'm not sure how another property on the event document (position) will help alleviate my concerns about the id of that document. Interesting functionality, but it doesn't really help me here.

My idea was that you could use it as ID, never mind.

If this were an opt-in that didn't affect existing consumers, would you be more willing to consider it? What if it were an optional way to override line 335 of mongodb.js? Let me explain my exact use case. Our team wants all documents to by identified by a version 4 UUID in our mongo data stores and I'd really rather not have my own version of mongodb.js. I'm already monkey patching the getNewId function. But monkey patching a one line function (getNewId) is a lot more palatable than a 78 line function (addEvents).

I have a few concurs, first and main one is that Mongo is just one of the drivers ( I do know for fact a few production projects relaying on Redis stores ), the other one is that at this point I don't want to introduce a change that is somehow not backward-compatible ( ie. would require some kind of migration from existing users).

Having those in mind, lets try and make those two ( commit and event ) ids ( generators ) configurable and leave the current behavior the default one.

Because of historical reasons the options object should stay a POJO one ( ie without functions in it ), and additional functionality should be added trough helper functions ( something like defineEventIdGenerator, and defineCommitIdGenerator ), this way you could achieve what your after without monkey patching.

I will try find time today to draft something quick.

Thank you for interesting discussion!