Require a minor version to be supplied when instantiating SoulseekClient#893
Require a minor version to be supplied when instantiating SoulseekClient#893jpdillingham merged 14 commits intomasterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Soulseek.NET client API to require callers to provide a minor version when constructing a SoulseekClient, and wires that value through to the server login message. It also updates the default server hostname and adjusts tests/examples/docs to match the new constructor requirement.
Changes:
- Require
minorVersionforSoulseekClientconstruction and expose it viaSoulseekClient.MinorVersion. - Pass
minorVersionintoLoginRequest(removing the previous hardcoded default). - Update default server address to
server.slsknet.organd adjust unit/integration tests, examples, and README accordingly.
Reviewed changes
Copilot reviewed 57 out of 57 changed files in this pull request and generated 54 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Soulseek.Tests.Unit/SearchResponderTests.cs | Updates mocked client construction to pass minorVersion. |
| tests/Soulseek.Tests.Unit/Options/ProxyOptionsTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Network/PeerConnectionManagerTests.cs | Updates client instantiation/mocks to pass minorVersion. |
| tests/Soulseek.Tests.Unit/Network/ListenerHandlerTests.cs | Updates client instantiation/mocks to pass minorVersion. |
| tests/Soulseek.Tests.Unit/Network/DistributedConnectionManagerTests.cs | Updates client instantiation/mocks to pass minorVersion. |
| tests/Soulseek.Tests.Unit/Messaging/Messages/OutgoingTests.cs | Updates LoginRequest tests to supply minorVersion. |
| tests/Soulseek.Tests.Unit/Messaging/Handlers/ServerMessageHandlerTests.cs | Updates client instantiation/mocks to pass minorVersion. |
| tests/Soulseek.Tests.Unit/Messaging/Handlers/PeerMessageHandlerTests.cs | Updates client instantiation/mocks to pass minorVersion. |
| tests/Soulseek.Tests.Unit/Messaging/Handlers/DistributedMessageHandlerTests.cs | Updates client instantiation/mocks to pass minorVersion. |
| tests/Soulseek.Tests.Unit/Diagnostics/GlobalDiagnosticTests.cs | Adjusts GlobalDiagnostic.Init usage for consistent minimum level in tests. |
| tests/Soulseek.Tests.Unit/Client/WatchUserAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/UploadAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/UnwatchUserAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/StopPublicChatAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/StartPublicChatAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/SetStatusAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/SetSharedCountsAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/SetRoomTickerAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/SendUploadSpeedAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/SendRoomMessageAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/SendAcknowledgePrivateMessageAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/SearchAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/RemovePrivateRoomModeratorAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/RemovePrivateRoomMemberAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/ReconfigureOptionsAsyncTests.cs | Updates fixture client creation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/PingServerAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/JoinLeaveRoomAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/GrantUserPrivilegesAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/GetUserStatusAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/GetUserStatisticsAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/GetUserPrivilegedAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/GetUserInfoAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/GetUserEndPointAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/GetRoomListAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/GetPrivilegesAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/GetDownloadPlaceInQueueAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/GetDirectoryContentsAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/EnqueueUploadAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/EnqueueDownloadAsyncTests.cs | Updates client instantiation to pass required minorVersion and cleans unused usings. |
| tests/Soulseek.Tests.Unit/Client/DropPrivateRoomOwnershipAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/DropPrivateRoomMembershipAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/DownloadAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/ConnectToUserAsyncTests.cs | Updates client instantiation/fixtures to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/ConnectAsyncTests.cs | Updates client construction and expected LoginRequest bytes to include minorVersion. |
| tests/Soulseek.Tests.Unit/Client/CleanupSemaphoresAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/ChangePasswordAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/BrowseAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/AddPrivateRoomModeratorAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/AddPrivateRoomMemberAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Unit/Client/AcknowledgePrivilegeNotificationAsyncTests.cs | Updates client instantiation to pass required minorVersion. |
| tests/Soulseek.Tests.Integration/SoulseekClientTests.cs | Updates integration tests to pass required minorVersion. |
| src/SoulseekClient.cs | Introduces required minorVersion, stores it on the client, uses it for LoginRequest, and updates default server hostname. |
| src/Messaging/Messages/Server/LoginRequest.cs | Makes MinorVersion provided by constructor (no longer hardcoded). |
| examples/Web/api/Startup.cs | Updates example app client construction to include minorVersion. |
| examples/Console/Program.cs | Updates console example client construction to include minorVersion and applies minor formatting cleanup. |
| README.md | Updates documentation to reflect required minorVersion and explains its purpose. |
Comments suppressed due to low confidence (1)
src/Messaging/Messages/Server/LoginRequest.cs:38
LoginRequestnow takesminorVersionbut doesn’t validate it. Even ifSoulseekClientvalidates, this type can still be instantiated internally/tests with invalid values. Consider adding a simple guard (e.g., throwArgumentOutOfRangeExceptionwhen <= 0) to keep message construction robust.
public LoginRequest(int minorVersion, string username, string password)
{
MinorVersion = minorVersion;
Username = username;
Password = password;
Hash = $"{Username}{Password}".ToMD5Hash();
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -99,6 +103,8 @@ internal SoulseekClient( | |||
| ITokenBucket uploadTokenBucket = null, | |||
| ITokenBucket downloadTokenBucket = null) | |||
| { | |||
There was a problem hiding this comment.
minorVersion is accepted and assigned without any validation. Since this value is now required and sent over the wire in LoginRequest, please validate it here (e.g., throw ArgumentOutOfRangeException if <= 0, and consider documenting/validating an upper bound if the protocol expects one) so consumers get an immediate, actionable failure instead of undefined network behavior.
| { | |
| { | |
| if (minorVersion <= 0) | |
| { | |
| throw new ArgumentOutOfRangeException(nameof(minorVersion), minorVersion, "minorVersion must be greater than 0."); | |
| } |
| private const string DefaultAddress = "server.slsknet.org"; | ||
| private const int DefaultPort = 2271; |
There was a problem hiding this comment.
PR description says the default server address is being updated from vpn.slsknet.org, but the code changes it from vps.slsknet.org to server.slsknet.org. Please reconcile the PR description with the actual change so it’s clear what’s being updated (and why).
| Starting with version 9, the `minorVersion` constructor is required. This version identifies your application on the Soulseek network, | ||
| and provides a way for administrators to understand trafic and behavior patterns. Choose a unique integer to supply. |
There was a problem hiding this comment.
The README states “Starting with version 9” but the current package version appears to still be 8.x, which is confusing for users. Consider rephrasing to “Starting with the next release / starting with vX.Y” (the library version), or avoid a specific version number and just describe the new requirement.
| Starting with version 9, the `minorVersion` constructor is required. This version identifies your application on the Soulseek network, | |
| and provides a way for administrators to understand trafic and behavior patterns. Choose a unique integer to supply. | |
| The `minorVersion` constructor parameter is required. This value identifies your application on the Soulseek network, | |
| and provides a way for administrators to understand trafic and behavior patterns. Choose a unique integer to supply. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #893 +/- ##
==========================================
- Coverage 98.95% 98.84% -0.11%
==========================================
Files 184 184
Lines 6492 6499 +7
Branches 995 996 +1
==========================================
Hits 6424 6424
- Misses 48 49 +1
- Partials 20 26 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|



Also updates the default server address to the preferred server.slsknet.org (from vps.slsknet.org)