Reliable Topic [API-1958]

[emreyigit]

The reliable topic is implemented and documented. Also, ReadManyWithResultSetAsync is implemented on HRingBuffer since it is required by listeners of the reliable topic.

[netlify]

👷 Deploy request for silly-valkyrie-e996d9 pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	8c5b3cb

[zpqrtbnk]

First batch of comments

[zpqrtbnk]

next batch of comments

[codecov-commenter]

Codecov Report

Merging #825 (8c5b3cb) into master (da31761) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #825      +/-   ##
==========================================
- Coverage   84.01%   84.01%   -0.01%     
==========================================
  Files         896      896              
  Lines       21295    21306      +11     
==========================================
+ Hits        17892    17901       +9     
- Misses       3403     3405       +2     

Impacted Files	Coverage Δ
src/Hazelcast.Net/HazelcastClientFactory.cs	83.05% <ø> (ø)
src/Hazelcast.Net/Networking/NetworkAddress.cs	90.24% <ø> (ø)
....Net/Serialization/Collections/ReadOnlyLazyList.cs	87.50% <ø> (ø)
...zelcast.Net/DistributedObjects/Impl/HRingBuffer.cs	100.00% <100.00%> (ø)
src/Hazelcast.Net/HazelcastClient.Objects.cs	96.29% <100.00%> (+0.55%)	⬆️
src/Hazelcast.Net/HazelcastOptions.Misc.cs	100.00% <100.00%> (ø)
src/Hazelcast.Net/HazelcastOptions.cs	100.00% <100.00%> (ø)

... and 9 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[zpqrtbnk]

commented

[zpqrtbnk]

commented

[zpqrtbnk]

Uh. Event arguments do not belong to the event handler. I know that in this special case the handler will only be invoked once, but it's... confusing. The args should be created outside of the handler, and passed to the HandleAsync method.

And then, you seem to imply that there can only be one unique handler for the Exception but ... that does not seem to be enforced (nothing prevents me from adding more than one handler, right?) and why should we limit ourselves?

[emreyigit]

There can be only one Exception handler (no method chaining). I terminated the fluent method calls at Exception since it changes the args.Cancel.

If we create the ExceptionArgs and pass to HandleAsync then we have to change the IReliableTopicEventHandler with an overload which accepts an EventArgs but each type event has different type of argument. So, each event becomes different type because IReliableTopicEventHandler should be generic to type of arguments. In this case, the polymorphism in here disappears. Or, all event arguments share same base class. Also, handler class is internal, so EventArg is too. Do you still want me to change the implementation?

[zpqrtbnk]

OK, I see what you have done here. But, the fact that there is no method chaining does not prevent me from doing:

await reliableTopic.SubscribeAsync(on =>
{
  on.Exception((sender, args) => DoSomething(sender, args));
  on.Exception((sender, args) => DoSomethingElse(sender, args));
});

Thus registering two handlers for the event. So I would think... we should align the Exception event with the others, just let ppl chain methods and register as many handlers as they want. Each handler will get the same event args, each handler can flip Cancel, what's important is the value of Cancel after all handlers have executed. That is how other .NET events that support cancellation work.

Then I realize... there are events handlers (plural) and event handler (singular). I believe ReliableTopicEventHandler<T> should be ReliableTopicEventHandlers<T> - see how it works for IMap. Helps make things less confusing.

Next... see maps, ReliableTopicEventHandlers<T> inherits EventHandlersBase<IReliableTopicEventHandlerBase> see it's IReliableTopicEventHandlerBase here not IReliableTopicEventHandler so ... each particular event handler can have it's own different event args = you can have one for exceptions. And create the args outside the handler, and run the handler that can have many methods, on the same args instance.

[emreyigit]

I see your concern. So, I've separated all events (message, exception and terminated) into different type of handlers and event arguments. I think current state af59734 allays your concerns.

[zpqrtbnk]

all good