Conversation
The project's metadata was updated as part of a rebranding effort, transforming the project from JamaaSMPP to Elyfe.Smpp, including updates in authorship, package URLs, descriptions and copyright. Additionally, the MSBuild process was adjusted to use the latest Visual Studio 2022 for the build process inside the cake and packaging scripts. These changes would reflect the new ownership and latest development tool requirements while ensuring a smooth transition for the users of the library. (cherry picked from commit 29952f0)
The commit modifies three files: Smpp.Net.Lib.csproj, common.props, and Smpp.Net.Client.csproj. The main changes were to adjust the TargetFramework to netstandard2.0 exclusively instead of net40, net48, and netstandard2.0. This was done to streamline the framework options and simplify deployment. A version number in common.props was also changed from 1.10.0 to 2023.10.0, likely corresponding with a large-scale update or release. Other minor updates include reordering of authors and owners, adjustments to url, and modification in the condition for the 'Release' configuration in Smpp.Net.Client.csproj file.
Added the properties 'AllowTransmit' and 'AllowReceive' to the 'SmppConnectionProperties' class, providing ability to control message transmission and reception. These changes allow better control over message flow direction based on specific use-case requirements. Additionally, property naming convention has been updated for consistency."
Removed Hungarian notation and implemented a switch-case structure to handle different connection states in OpenSession method in SmppClient.cs. This ensures better readability and ease of debugging, while maintaining existing functionality.
Refactored the code of the SMPP client to match C# 8.0 syntax style. This improvement includes the usage of null coalescing operators, conversion of properties into expressions-bodied form, the removal of unnecessary semicolons and redundant type specifications, and the consolidation of namespace declarations. These changes make the code cleaner, more readable, and adhere to modern C# coding standards.
Updated reference path from "JamaaTech.Smpp.Net.Lib" to "JamaaTech.SMPP.Net.Lib" in Smpp.Net.Client.csproj file. This change is important for maintaining naming consistency across our project.
Removed hardcoded version and updated information about the company, summary, and the inclusion of MinVer package. The changes were made to reflect the transfer of the project to a different company and to automate versioning. Adding the MinVer package allowed us to dynamically generate the version based on the Git history, removes the need to manually set it. In addition, updated project summary to more accurately represent the project's scope and purpose.
The package's license URL has been replaced by the MIT license tag. A README file has been included in the package for more comprehensive package information. These changes help to improve the package's usability and information accessibility. Meanwhile, MinVerTagPrefix was added for minimum version tagging convention.
A new file was created for GitHub Actions to automate the building, packing, and deployment of NuGet packages whenever push or pull request actions occur on either dev or master branches, or a v* tag is added. This CI/CD workflow will streamline the package release process and ensure all changes are properly packaged and published. This action only runs on tags and not on pull requests to prevent accidental deployments.
The source-url in the NuGet setup step of the GitHub workflow for our SMPP client library was removed. The project uses public NuGet feeds, thus the default "https://api.nuget.org/v3/index.json" is no longer needed in this step.
This commit updates the paths to the project in the nuget-deploy workflow file. Previously, these paths didn't include the relative directory (./) before the project name. Now, paths start with ./ to make it clear that they are relative to the current directory. This explicit relation ensures the correct file is located and
Removed the '--no-build' argument in the 'Pack' command of the 'nuget-deploy.yml' file. This is done to ensure that the project is built again during packaging, potentially catching any build-based errors that could be missed otherwise.
The name of the GitHub Actions workflow has been updated from "Build and Deploy with Docker Images" to "Build and Deploy Nuget Packages". This change better represents the workflow's purpose which is to build and deploy Nuget Packages, as Docker is no longer used.
The NuGet deployment workflow in the Elyfe.Smpp library has been updated to include a dependency on the build-package job. This change ensures that the package build is completed successfully before deployment begins. This will help avoid problems due to failed or incomplete builds.
As part of housekeeping in the project, the file codealike.json was added to the .gitignore file. This file is used for personal local development settings and should not be included in the repository. This change will prevent it from being accidentally committed in the future.
The update includes refactoring to clean up code in several files. Updates mostly involve changes in syntax, cleaning spaces and indentation, and making properties more concise by using lambda expressions. This refactoring improves the readability and maintainability of the code but does not alter any functionality.
This commit updated the versions of packages used in the `Smpp.Net.Lib.csproj`, `Winton.Extensions.Configuration.Consul.csproj`, and `Smpp.Net.Client.csproj` projects. It also added a new package reference to MinVer. This helps us to ensure we're using the latest and most secure versions of our dependencies, and to specify the minimum version of a package that is required.
Updated the MinVer package reference from version 5.0.0 to 6.0.0 in both the Smpp.Net.Client and Smpp.Net.Lib projects. This ensures compatibility with the latest features and improvements offered by MinVer.
Introduced `SmscConnectorGrain` to handle SMSC connections, message submission, and delivery receipts processing in Orleans. Includes necessary interfaces and models, implementing both SMPP and HTTP protocols. Also fixed casing on `SegmentID` to `SegmentId` in `ReceivedSmsProcessor`.
Updated GitHub Actions dependencies to their latest versions and upgraded the .NET target version from 7.0.x to 9.0.x in the NuGet deployment workflow. This ensures compatibility with newer features and improves the reliability of the pipeline.
Deleted `AnalyticsControllerTests` as it was no longer needed, streamlining the test suite. Added `Microsoft.AspNetCore.Mvc.Testing` to project dependencies and updated `JamaaTech.Smpp.Net.Client` with improved method documentation.
Introduced a new SMPP client library to manage SMS operations, including connection handling, message submission, and delivery receipt processing. Added unit tests to validate core functionality, ensuring reliability and maintaining expected behaviors.
There was a problem hiding this comment.
Pull Request Overview
This PR merges development features into the master branch, introducing SMPP client enhancements, build/script modernizations, and code style updates.
- Adds a new
QueryMessageStatusAPI toSmppClientand expands session binding options. - Modernizes packaging scripts and project metadata for .NET 9.0/.NET 8.0 support and MinVer usage.
- Refactors code to file-scoped namespaces, underscore-prefixed fields, and expression-bodied members.
Reviewed Changes
Copilot reviewed 127 out of 129 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| nupkg/pack.ps1 | Injects version parameter into NuGet pack commands |
| common.props | Updates project metadata (URL, authors, license, MinVer) |
| build.cake | Specifies MSBuild ToolVersion for VS2022 |
| README.md | Renames to Elyfe.Smpp, updates links, headings, and formatting |
| JamaaTech.Smpp.Net.Client/TextMessage.cs | Refactors private fields, uses expression-bodied members |
| JamaaTech.Smpp.Net.Client/SmppConnectionProperties.cs | Refactors fields to auto-properties and adds AllowReceive/AllowTransmit |
| JamaaTech.Smpp.Net.Client/SmppClient.cs | Adds QueryMessageStatus, refactors session logic |
Comments suppressed due to low confidence (1)
JamaaTech.Smpp.Net.Client/SmppClient.cs:180
- The new
QueryMessageStatusmethod lacks corresponding unit tests. Adding tests for both success and failure scenarios would improve coverage and reliability.
public ResponsePDU QueryMessageStatus(string messageId, string sourceAddress)
|
|
||
| ## SMPP Server Simulator | ||
|
|
||
| - Downlad from [http://www.seleniumsoftware.com/downloads.html](http://www.seleniumsoftware.com/downloads.html) |
There was a problem hiding this comment.
There's a typo in the README: "Downlad" should be "Download".
Suggested change
| - Downlad from [http://www.seleniumsoftware.com/downloads.html](http://www.seleniumsoftware.com/downloads.html) | |
| - Download from [http://www.seleniumsoftware.com/downloads.html](http://www.seleniumsoftware.com/downloads.html) |
| /// <summary> | ||
| /// <see cref="UseSeparateConnections"/> | ||
| /// </summary> | ||
| public bool CanSeparateConnections => UseSeparateConnections == true || InterfaceVersion == InterfaceVersion.v33; |
There was a problem hiding this comment.
The logic treats UseSeparateConnections=false as allowing separate connections when InterfaceVersion==v33. It should only default to true when UseSeparateConnections is null and the interface version is v33, e.g.: UseSeparateConnections == true || (UseSeparateConnections == null && InterfaceVersion == InterfaceVersion.v33).
Suggested change
| public bool CanSeparateConnections => UseSeparateConnections == true || InterfaceVersion == InterfaceVersion.v33; | |
| public bool CanSeparateConnections => UseSeparateConnections == true || (UseSeparateConnections == null && InterfaceVersion == InterfaceVersion.v33); |
| { | ||
| return queryResp; | ||
| } | ||
| throw new NotImplementedException("WIP"); |
There was a problem hiding this comment.
The exception message "WIP" is unclear. Consider using a descriptive message like "QueryMessageStatus: unexpected response type" or implement a proper fallback.
Suggested change
| throw new NotImplementedException("WIP"); | |
| throw new InvalidOperationException("QueryMessageStatus: unexpected response type received."); |
| // We check vText Length first | ||
| if (_vText.Length > _vMaxMessageLength && bytes.Length > _vMaxMessageLength) // Split into multiple! | ||
| { | ||
| var segId = new Random().Next(1000, 9999); // create random SegmentID |
There was a problem hiding this comment.
Instantiating new Random() inside a loop may produce the same seed and add overhead. Consider using a static Random instance to improve distribution and performance.
Suggested change
| var segId = new Random().Next(1000, 9999); // create random SegmentID | |
| var segId = _random.Next(1000, 9999); // create random SegmentID |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces several enhancements and new features across the codebase, focusing on enabling SMPP client functionality, improving test coverage, and modernizing existing code. Key changes include the implementation of an SMPP client with connection state management, the addition of unit tests, and updates to project files to support .NET 9.0.
SMPP Client Implementation:
Client/SmscClient.cs: Added theSmscClientclass to manage SMPP connections, handle state transitions, send SMS messages, and process received PDUs. Includes reconnect logic and event handling for connection state changes and incoming messages.Client/ISmscClient.cs: Defined theISmscClientinterface for SMPP client operations, including methods for connecting, disconnecting, and sending SMS messages, as well as events for state changes and message reception.Configuration and Dependency Injection:
Client/SmscOptions.cs: Introduced theSmscOptionsclass for configurable client settings such as host, port, and reconnect behavior.Client/ServiceCollectionExtensions.cs: Added extension methods to register the SMPP client and its configuration in the service collection for dependency injection.Unit Tests:
Client.Tests/SmscClientTests.cs: Implemented unit tests forSmscClient, covering scenarios such as connection state transitions, message sending, and event handling. Mocking was used to simulate SMPP session behavior.Client.Tests/Client.Tests.csproj: Created a test project targeting .NET 9.0 and added dependencies for testing frameworks like xUnit and Moq.Project and Workflow Updates:
.github/workflows/nuget-deploy.yml: Added a GitHub Actions workflow to build, test, and deploy NuGet packages for the SMPP client library. Supports branch pushes, pull requests, and tag-based deployments.Client/Client.csprojandClient.Tests/Client.Tests.csproj: Updated project files to target .NET 9.0, enable nullable reference types, and include necessary package references. [1] [2]Code Modernization:
JamaaTech.SMPP.Net.Lib/*: Modernized namespace declarations to use the file-scoped syntax, improving readability and alignment with C# 10 conventions. [1] [2] [3]