Split library into modules; extract bindings for netty3, netty4, etc.

[jjlauer]

This library was originally created when Netty was a young project. Netty-4 introduced so many backwards compat issues, that its almost an entirely new library in some ways. While we even had Netty's main author (Trustin) help port ch-smpp from netty-3 to netty-4, Twitter's applications still use the netty-3 branch.

Maintaining this project with two branches is cumbersome. Plus, there are possibly good uses of ch-smpp with different pluggable network bindings -- e.g. blocking I/O, xnio from jboss, netty-3, and netty-4.

The idea would be to split up ch-smpp into modules. Something like core, netty3, and netty4 for now. There already are interfaces in the library with implementations. Users of the library would simply pull in ch-smpp-core and pick the network layer bindings they wanted. All unit tests should verify each different binding works. I'm guessing the easiest method would be to add a 4th module called "tests", and possibly use JUnit parameterized runners to verify each binding passes the same tests.

Anyone from the community looking to take on this pretty extensive project?

[JChrist]

I'm in for it.
So, as I understand, ch-smpp would stay as the core artifact and there will be another two (for now), ch-smpp-netty3 and ch-smpp-netty4. I assume these will go to different git repositories, as well?

[jjlauer]

Not separate git repos, but as a multi-module maven project. Another
project as an example of multi-module https://github.com/fizzed/rocker

pom.xml

• ch-smpp-core
• ch-smpp-netty3
• ch-smpp-netty4

I think to reuse unit tests, you'd also be looking at a possible 4th
module

• ch-smpp-tests

So a class like com.cloudhopper.smpp.SmppClient would remain in the
ch-smpp-core module, but then com.cloudhopper.smpp.impl.DefaultSmppClient
in the master branch would be renamed to:

com.cloudhopper.smpp.netty3.Netty3SmppClient and only available in the
ch-smpp-netty3 module. Then the netty4 branch
com.cloudhopper.smpp.impl.DefaultSmppClient would be put in the
ch-smpp-netty4 module and named something like
com.cloudhopper.smpp.netty4.Netty4SmppClient.

Does that help clarify?

On Fri, Mar 25, 2016 at 10:32 AM, Ioannis Christodoulou <
[email protected]> wrote:

I'm in for it.
So, as I understand, ch-smpp would stay as the core artifact and there
will be another two (for now), ch-smpp-netty3 and ch-smpp-netty4. I assume
these will go to different git repositories, as well?

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#7 (comment)

[JChrist]

Hm, I think I've got what you mean, let me delve into it and I'll get back if I need further help

[JChrist]

My hardest problem would be how to tackle the ChannelBuffer / ByteBuf / (any future buffer type) references, e.g. in Pdu's abstract public void readBody(ChannelBuffer buffer) throws UnrecoverablePduException, RecoverablePduException; which extends to the whole hierarchy.

Any suggestions for this?

[JChrist]

My thoughts are that I should encapsulate all differences under the transcoder instance - which is module specific. In particular:

• Change PduTranscoder's ChannelBuffer/ByteBuf to Object
• Change Pdu's methods that need a ChannelBuffer/ByteBuf to receive an Object buffer and the transcoder instance.
• Move all straight calls from Pdu to ChannelBuffer/ByteBuf to the transcoder
• Move all static method calls to ChannelBufferUtil/ByteBufUtil to the transcoder

[JChrist]

I've got it here: https://github.com/JChrist/cloudhopper-smpp/tree/multimodule
Most notable changes:

• Changed ChannelBuffer/ByteBuf to Object
• Passing a PduTranscoder instance to Pdu classes for reading/writing bytes to Object buffer
• Delegated all calls to read/write to buffers to the PduTranscoder instance at hand
• Renamed ChannelBufferUtil to BufferUtil, receiving a pdu transcoder instance as parameter
• Kept all class names the same for impl classes, such as DefaultSmppServer, DefaultSmppClient, etc in both netty3 and netty4 modules.
  This will keep ch-smpp-netty3 backwards compatible, the only change needed would be to add the maven ch-smpp-netty3 dependency and it would work. Same goes for users of netty4, only ch-smpp-netty4 dependency would be needed. Of course, this means that ch-smpp-netty3 and ch-smpp-netty4 cannot co-exist.
• I did not go with another module for tests, since class names collide. Instead I moved tests requiring netty-specific classes under each submodule (e.g. DeliveryReceiptTest.parseRealWorldReceipt0 is duplicate in ch-smpp-netty3 and ch-smpp-netty4)
• Version probably needs to change, I've left it to the default of 5.0.10-SNAPSHOT

I'm looking forward to any feedback you have!

[JChrist]

It's been quite some time without any updates, @jjlauer , have you ever found the time to have a look at it?