Skip to content

Introduce protocol abstraction interfaces #22

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

Merged
merged 8 commits into from
May 10, 2025
Merged

Conversation

smdn
Copy link
Owner

@smdn smdn commented May 5, 2025

Description

Introduce an interface IMuninProtocolHandler to abstract the implementation of the data exchange protocol between munin master and munin node.

This PR creates a new namespace Smdn.Net.MuninNode.Protocol and introduces the interfaces IMuninProtocolHandlerFactory and IMuninProtocolHandler.
The munin protocol handling that has been implemented in NodeBase will be moved and separated to a new class MuninProtocolHandler that implements IMuninProtocolHandler.

In addition, an interface IMuninNodeProfile will be introduced to reduce the consolidation between NodeBase and IMuninProtocolHandler.

The outlines of the APIs to be introduced by this change are as follows:

+namespace Smdn.Net.MuninNode.Protocol {
+  public interface IMuninProtocolHandler {
+    ValueTask HandleCommandAsync(IMuninNodeClient client, ReadOnlySequence<byte> commandLine, CancellationToken cancellationToken);
+    ValueTask HandleTransactionEndAsync(IMuninNodeClient client, CancellationToken cancellationToken);
+    ValueTask HandleTransactionStartAsync(IMuninNodeClient client, CancellationToken cancellationToken);
+  }
+
+  public interface IMuninProtocolHandlerFactory {
+    ValueTask<IMuninProtocolHandler> CreateAsync(IMuninNodeProfile profile, CancellationToken cancellationToken);
+  }
+
+  public static class MuninProtocolHandlerFactory {
+    public static IMuninProtocolHandlerFactory Default { get; }
+  }
+
+  public interface IMuninNodeProfile {
+    Encoding Encoding { get; }
+    string HostName { get; }
+    IPluginProvider PluginProvider { get; }
+    string Version { get; }
+  }
+}

This change will improve the independence, extensibility, testability, and maintainability of the protocol implementation.
At the same time, reorganize the code base to improve protocol handling in the future.

This PR also fixes race condition due to use of single IBufferWriter while sending response.

Considerations

In this PR, the term transaction is used instead of the term session, following the example in the Munin's official document.
In the future, the terminology used with names such as INodeSessionCallback needs to be consistent with transaction.

@smdn smdn merged commit ba51e73 into main May 10, 2025
13 checks passed
@smdn smdn deleted the introduce-protocolhandler branch May 10, 2025 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant