Don't remove remote message ids for SMPP delivery reports

[justinvdm]

Since #977 and #978, we remove message ids for failure and success delivery reports. It is common for us to get multiple delivery reports for the same message though, so we can't really do this.

[hodgestar]

I agree with this in general but I think there are two very common cases in which we can remove message ids:

• Successful delivery
• Permanent failed delivery

Delivery can't un-succeed or un-permanently fail, so these should be the last delivery report messages we receive. Un-succeeded or un-permanently failing are definitely errors and we might as well save space in Redis and log the failures rather than handing them over to the application which can only look at these cases with wide eyes and shrug.

[jerith]

In practice, delivery reports are arbitrary and often full of lies (and even when they're not, the mapping from DR content to actual status isn't well defined). Also, out-of-order DR delivery is incredibly common and we need to handle the case where we receive "delivered" followed immediately by "pending" instead of the other way around.

[jerith]

On the other hand, I think it's entirely reasonable to reset the TTL to something much shorter when we receive a "final" DR status.

[hodgestar]

Plus one on setting a much shorter TTL.

[justinvdm]

Ready for review.

[rudigiesler]

:+1: from me, assuming that people are happy with the 1 hour timeout

[jerith]

Looks good, except for the assertion comment. You can look at other SMPP tests that make assertions about Redis TTLs to see how they handle the problem.

[justinvdm]

Ready for re-review.

[hodgestar]

One minor comment about assertion message text, otherwise looks good to me.

[justinvdm]

Assertion messages fixed.

[hodgestar]

:+1: