-
Notifications
You must be signed in to change notification settings - Fork 276
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
[refactor] #1981, #4195: Refactor the event filters #4240
[refactor] #1981, #4195: Refactor the event filters #4240
Conversation
4f83035
to
1686240
Compare
This new API doesn't allow to represent the filter "account created events within domain wonderland", which the previous system allowed. This still can be implemented with the flattened approach by introducing typed id matchers. For example: pub enum AccountIdMatcher {
/// match any account id
ByAny,
/// match account id within a specific domain
ByDomain(DomainId),
/// match only a specific account id
ByAccount(AccountId),
} This functionality is somewhat related to the Anyways, I would prefer this to be implemented as a separate PR though |
c27784e
to
a1cdde5
Compare
a1cdde5
to
2ceed8e
Compare
8f4a4de
to
15ab511
Compare
15ab511
to
b2cc3e0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we consider case for filtering multiple events?
Like someone wants to count number of accounts, it would be possible by filtering events registered or unregistered (and increment or decrement counter respectively).
Current workaround would be to create multiple event streams.
b2cc3e0
to
90d66ec
Compare
…he event filters - rename the `Filter` trait to `EventFilter` - "manually" implement all the event filters circumventing the macro - as a consequence, move all the filters to a separate module - change the filter types naming scheme to be more logical: - AccountFilter -> AccountEventFilter (filters the event as a whole: by origin id and event type) - AccountEventFilter -> AccountEventMatcher (matches over a type of event) - flattens the event filter hierarchy (account event is a kind of domain event, but account event filter is not) Signed-off-by: Nikita Strygin <[email protected]>
… event filter derive macro Signed-off-by: Nikita Strygin <[email protected]>
…he event filters (even further) - Remove the FilterOpt combinator, fuse it with DataEventFilter - Rename DataEntityFilter to DataEventFilter - Add `Any` suffix to nested events matchers - Add doc-comments to event filters & their variants Signed-off-by: Nikita Strygin <[email protected]>
…terBox and TriggeringFilterBox to EventFilterBox and TriggeringEventFilterBox To make it consistent with the renamed EventFilter trait Signed-off-by: Nikita Strygin <[email protected]>
…n_for_events and friends implicitly convert event filter types Signed-off-by: Nikita Strygin <[email protected]>
…r-iroha#3068: Unify various event filter APIs, introduce a fluent builder API - All event filters implement Debug, Clone, Eq, Ord - All event filters (except TimeEventFilter) have a similar fluent builder API Event filter starts by accepting anything, with each method call limiting the accepted events - Structs with hidden fields provide getters & builder APIs to create them - Data event matchers are no longer prefixed with `By` to make them read better inside the `only_events` method call Signed-off-by: Nikita Strygin <[email protected]>
eaf54e7
to
21f2f9d
Compare
…r-iroha#3068: Add an event set proc macro This proc macro generates a wrapper around a bit mask to specifying a set of events to match Signed-off-by: Nikita Strygin <[email protected]>
…r-iroha#3068: Use the auto-generated event sets instead of handwritten matchers Signed-off-by: Nikita Strygin <[email protected]>
…r-iroha#3068: match more exhaustively over events in DataEventFilter This will produce a compile error in case a new top-level data event is introduced. It will not catch new Domain event types (like Account, Asset, AssetDefinition). Achieving that will require listing all (even non-hierarchical) event types, which is probably too much boilerplate Signed-off-by: Nikita Strygin <[email protected]> [refactor] hyperledger-iroha#1981, hyperledger-iroha#4195, hyperledger-iroha#3068: match more exhaustively over events in DataEventFilter This will produce a compile error in case a new top-level data event is introduced. It will not catch new Domain event types (like Account, Asset, AssetDefinition). Achieving that will require listing all (even non-hierarchical) event types, which is probably too much boilerplate Signed-off-by: Nikita Strygin <[email protected]>
…r-iroha#3068: Add event filters for PermissionTokenSchemaUpdate, Configuration and Executor events Signed-off-by: Nikita Strygin <[email protected]>
…r-iroha#3068: Remove some more redundancy when constructing event filters Signed-off-by: Nikita Strygin <[email protected]>
…r-iroha#3068: Update event filter builder methods naming Follow the pattern of [event filter] for_X everywhere Signed-off-by: Nikita Strygin <[email protected]>
…r-iroha#3068: Remove NotificationEvent Instead of it, expose TriggerCompletedEvent as a top-level event, same as ExecuteTriggerEvent Signed-off-by: Nikita Strygin <[email protected]>
21f2f9d
to
60065fe
Compare
Description
Filter
trait toEventFilter
*EventSet
types with a derive macroAccountFilter
->AccountEventFilter
(filters the event as a whole: by origin id and event type)-
AccountEventFilter
->AccountEventSet
(matches over a type of event)Linked issue
Closes #1981, #4195
Benefits
Checklist