search

twitter-archive / cloudhopper-smpp

Efficient, scalable, and flexible Java implementation of the Short Messaging Peer to Peer Protocol (SMPP)
Other
382 stars 356 forks source link

Migrate to netty 4.0.X version #47

Closed ssserj closed 10 years ago

ssserj commented 10 years ago

I propose to consider and discuss the patch to use netty 4.0.x under the hood.

ssserj commented 10 years ago

Anybody GitHub?

xgp commented 10 years ago

Sorry for the delay, @ssserj. We've also got an internal branch for netty 4: https://github.com/twitter/cloudhopper-smpp/tree/netty4 To get this in a mergeable state, we need to check the two branches to make sure we made a reasonably similar approach (and reconcile it where we didn't), and get all of the unit tests passing. Currently your fork reports the following failures/errors:

Results :

Failed tests:   serverNotEnoughWorkerThreadsCausesBindTimerToCloseChannel(com.cloudhopper.smpp.impl.DefaultSmppServerTest): expected:<3> but was:<5>

Tests in error:
  serverSessionBindVersion31NormalizedTo33(com.cloudhopper.smpp.impl.DefaultSmppServerTest): Can't bind to port 9784 after 5000 milliseconds
  serverSessionTimesOutWithNoBindRequest(com.cloudhopper.smpp.impl.DefaultSmppServerTest): Can't bind to port 9784 after 50 milliseconds
  serverBindToUnavailablePortThrowsException(com.cloudhopper.smpp.impl.DefaultSmppServerTest): Can't bind to port 9784 after 5000 milliseconds
  serverSessionBindVersion35NormalizesTo34(com.cloudhopper.smpp.impl.DefaultSmppServerTest): Can't bind to port 9784 after 5000 milliseconds
  serverSessionBindRejectedWithInvalidPassword(com.cloudhopper.smpp.impl.DefaultSmppServerTest): Can't bind to port 9784 after 5000 milliseconds
  serverSessionBindRejectedWithInvalidSystemId(com.cloudhopper.smpp.impl.DefaultSmppServerTest): Can't bind to port 9784 after 5000 milliseconds
  serverSessionOK(com.cloudhopper.smpp.impl.DefaultSmppServerTest): Can't bind to port 9784 after 5000 milliseconds
  bindOverSSL(com.cloudhopper.smpp.ssl.SslSessionTest): Can't bind to port 9784 after 5000 milliseconds
  clientOverSSLButServerIsNotSSL(com.cloudhopper.smpp.ssl.SslSessionTest): Can't bind to port 9784 after 5000 milliseconds
  serverOverSSLButClientIsNotSSL(com.cloudhopper.smpp.ssl.SslSessionTest): Can't bind to port 9784 after 5000 milliseconds
  enquireLinkOverSSL(com.cloudhopper.smpp.ssl.SslSessionTest): Can't bind to port 9784 after 5000 milliseconds

If you're up for it, we're very happy to have the help. Thanks for the contribution.

ssserj commented 10 years ago

@garthpatil thanks for reply. Hmm, but I can't reproduce this errors and fails in my fork (git@github.com:ssserj/cloudhopper-smpp.git).

Results :

Tests run: 186, Failures: 0, Errors: 0, Skipped: 3

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 1:17.780s
[INFO] Finished at: Wed Mar 05 10:16:44 MSK 2014
[INFO] Final Memory: 24M/350M
[INFO] ------------------------------------------------------------------------

How you reproduce the errors/fails? The impression is that commits c96ffc0, 0fa9dc8, f397376 not been applied to the master branch.

ssserj commented 10 years ago

@garthpatil I couldn't wait. I merged my master with branch netty4. Tests was fixed. Please, check work ;)

ssserj commented 10 years ago

@jjlauer @garthpatil all work merged to netty4, see #53.