Feature: PDU SubmitWindow with; MaxWindowSize option, ExpectedResponse handler, ExpiredPdus handler and NoRespPdu OnClose handler

[laduchesneau]

What:

• Add a submit window via a concurrent-map that tracks Requests (SubmitSM, EnquireLinks, ReplaceSM, etc..)
• Add functionality to return a expected response with the original sent PDU
• Add functionality to track PDUs with no response and a timer setting for when they expire
• Add a max window size setting, to limit the number of outbound request
• Add function call to get current bind window size on Tx and Trx
• Add function call to get a PDU stuck in the submit store when the bind closes.
• Add an example on how to use new settings
• Add an example on how to implemented a Custom PDU to add any fields to be tracked

Why: As requested in #126, #105 and #73, the user sometimes needs to track all requests sent to SMSC. Either to relate a response to a request, or to track a request that have received no response or even to limit the number of outgoing request without any response from the SMSC.

How:

• The main feature, the submit window, works by using a concurrent-map as a key/value store. The key is the PDU sequence number and the value is a new Request struct created for this feature. Concurrent-map is thread safe and has all the functionality needed for this use case. The map gets reset on every rebind and all PDUs stored in the map are can be retruned to the user via a func call when the session is closed.
• When the user Submits a PDU, it is stored in the new Request struct with the request is created
• When the library receives a PDU from the SMSC, it will verify if the PDU is a response type (SubmitSMResp, ReplaceSMResp, etc) and queries the key/value store with the sequence number. If the store contains a PDU request, the response is returned to the user with the PDU and the original request via OnExpectedPduResponse setting. The Request is removed from the store after the lookup.
• Receivable has been modified to add a new loopWithVerifyExpiredPdu, that verifies all PDU in the store and compare the time they were stored. If the time is great than the value entered by the setting PduExpireTimeOut, the PDU is removed from the store and return to the user via OnExpiredPduRequest.

The submit window will only contain PDU that return true on CanResponse, except Unbind and BindRequest:

• CancelSM
• DataSM
• DeliverSM
• EnquireLink
• QuerySM
• ReplaceSM
• SubmitMulti
• SubmitSM

This PR does not break current user experience, all old test pass and if user does not add the new settings, all will work as it previously did.

[laduchesneau]

Checks are failing because the test coverage is low. I'm working on increasing test coverage, especially for receivable.go and transmittable.go.

[laduchesneau]

The last commit added the last feature for this PR, with all test completed.

Again, this feature should not introduce any breaking changes or impact current user implementations. All previous test passed, except one. (transmittable_test.go) was modified to add the new concurrent-map variable as the test does not call newTransmittable.

Will update the PR status and will wait for code review and/or feedback.

[laduchesneau]

Ive pushed a first draft of the interface and its implementation:

• still need to add context support
• currently using settings to set WindowStore

[laduchesneau]

For the last three days, Ive tried to implement bigcache as the default store.

The main problem is the serialization of the data. When using bigcache, we loose the feature to support CustomPDU. One of the main feature of the PR is to allow users to add fields to their CustomPDU while implementing PDU, so they can track PDU Response to their Request. Like storing a internal message id or channel. If we use bigcache, the library needs to marshall /unmarshall the whole structure passed to the store. This means, the Request and the CustomPdu need a marshall/unmarshal functionality. This adds a lot of complexity to library and to the users that wants to use this feature. Also, not all field can be serialized, like a channel. This was my main use case. I'm building a Gateway, that convert HTTP to SMPP, when the Response is returned, my handlers returns something in the channel.

Also the code for bigcache will be harder to maintain vs concurrent-map, look at the bigcache implemantation here and look at the concurrrent-map here. The concurrent-map is easier to maintain. It should be noted, that to bypass the requirement to support users marshall/unmarshall implementation, I temporarily used gob, but this should not be used in production.... we also loose channel support with gob.

Here is what I am going to do...I'm going to implement default cache with concurrent-map and add the bigcache as a example.

[linxGnu]

@laduchesneau could you please fix the conflicts.

After that, I think we can have a final review round. Since we expose the interface for custom cache, we are all good 👍

[laduchesneau]

@laduchesneau could you please fix the conflicts.

After that, I think we can have a final review round. Since we expose the interface for custom cache, we are all good 👍

Yeah, Ill complete the context handling and change how we pass the cache, remove it from settings and use a with function option instead..

[laduchesneau]

Hi @linxGnu , Ive been using this branch in production for a couple of months, with 0 issues. Of course, there is no guarantee somebody else wont find a bug. My use case for it was purely for MT traffic, but that was the purpose of this PR. I honestly believe its good to be reviewed and merged.

If anybody is interested in running this code for testing, all you need to do is add the following line to go.mod

replace github.com/linxGnu/gosmpp v0.2.1 => github.com/laduchesneau/gosmpp add_window_setting

[linxGnu]

@laduchesneau I will have a final look tomorrow. In the meantime, could you please fix the conflicting (please also update all dependencies as you wish)

[linxGnu]

@laduchesneau
Most of code logic is LGTM. Could you please address my following comments

[laduchesneau]

I completed all recommended fixes.

I noticed , I did not handle ctx.Done() and some error werent handled correctly.

1st commit is the recommend changes
2nd/3rd commits is the changes done to the interface to allow error return

[linxGnu]

@laduchesneau thank you so much for great PR.

I just have 2 small comments. Please kindly address them. Then we can merge & release

[linxGnu]

@laduchesneau
LGTM. As always, thank you for awesome PR