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

Add indexers that allow to create ad-hoc indexes #140

Closed
wants to merge 11 commits into from
Closed

Conversation

albe
Copy link
Owner

@albe albe commented Feb 7, 2021

Sometimes it is necessary to iterate all events that fulfill a specific condition, which is not known at design time. A (suboptimal) example is events by type that are coming in from another system.
For those cases it was not possible to create on-the-fly streams easily and required code like this:

if (eventstore.getEventStream('type-' + event.payload.type) < 0) {
    eventstore.createEventStream('type-' + event.payload.type, { payload : { type : event.payload.type } });
}

For more complicated event matching that requires a matcher function, the function would have needed to be stringified and evaluated, so that variables from the incoming event are serialized into the matcher function, as that needs to be pure.

With this change the same can be achieved like that:

eventstore.createDynamicStream((event) => 'type-' + event.payload.type);

@albe albe added enhancement P: Storage Affects the storage layer labels Feb 7, 2021
@ghost
Copy link

ghost commented Feb 7, 2021

Congratulations 🎉. DeepCode analyzed your code in 3.997 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard | Configure the bot

@coveralls
Copy link

coveralls commented Feb 7, 2021

Coverage Status

Coverage decreased (-0.2%) to 97.087% when pulling 57659f8 on indexer into 11677db on master.

@albe
Copy link
Owner Author

albe commented Feb 20, 2021

After writing down the PR message, this whole change sounds rather edge-casey and introduces a whole new stream definition API into the EventStore and Storage layers. Also, that API does some clever (bad) stuff internally (eval a function string to generate a matcher dynamically). On top, it does not solve the issue of secondary indexes potentially running out of sync with the storage, see #129 (comment).
Also this new flag for the ensureIndex() method is a smell https://github.com/albe/node-event-storage/pull/140/files#diff-d51a47d660b3482194158f53ddf3b699b52e2f75038b190454a779194f2e4d7bR231

For the example of event type streams, the existing solution is only two lines of code and not really complicated. For the case that the matching is indeed more complicated, I'm not sure it is worth adding this API surface. Especially since an actual projection API with linkTo based on consumers as in #129 could provide the same, but with fully consistent streams.

# Conflicts:
#	src/Storage/WritableStorage.js
@albe albe added the postponed label Feb 20, 2021
@albe
Copy link
Owner Author

albe commented Feb 20, 2021

Closing this for now for the reasons given above.

@albe albe closed this Feb 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement P: Storage Affects the storage layer postponed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants