Fix Null Pointer Exception in NSQConsumer

[kenshiro-o]

The code in the NSQConsumer.java does not check for a null Connection being returned from the call to

super.createConnection(address, port)

This PR fixes this issue which is manifested when we try to call a method on a null Connection (e.g. https://github.com/dustismo/TrendrrNSQClient/blob/master/src/main/java/com/trendrr/nsq/NSQConsumer.java#L39).

Moreover, some refactoring was done and tests added. In case you are interested, the stack trace looked as follows:

        at com.trendrr.nsq.NSQConsumer.createConnection(NSQConsumer.java:40)
        at com.trendrr.nsq.AbstractNSQClient.connect(AbstractNSQClient.java:183)
        at com.trendrr.nsq.AbstractNSQClient$1.run(AbstractNSQClient.java:75)
        at java.util.TimerThread.mainLoop(Timer.java:555)
        at java.util.TimerThread.run(Timer.java:505)

[kenshiro-o]

I will try to find some time in the next few months/weeks to add more tests and improve code. The library works really well otherwise 👍

[kenshiro-o]

@dustismo any thoughts?

[dustismo]

Why change this? I'm not against the change (log vs LOGGER) but you'll need to update it in the other classes for consistency.

[kenshiro-o]

I happened to have modified these files when I was investigating the issue. I'd like to do the refactoring across all files but wanted to keep this fix rather small.

[dustismo]

@kenshiro-o sorry it took so long for a response. I made a couple of comments otherwise looks good, thanks!

[kenshiro-o]

@dustismo I have removed the model package. With regards to changing the scope and name of the Logger across all source files, I'd rather do this in a separate PR to better contain the change.

Also the main reason why I modified your Logger instances is because they should be immutable (final) and also only relevant to the current class. You don't need to use protected as your subclasses define their own logger and hence do not inherit their parent's logger. Lastly the name was capitalized by convention since your Logger is a constant now.

Let me know if you are OK with the current PR 👍

[dustismo]

@kenshiro-o Looks good to me. Looks like there is some conflict with master, so if you fix that I will merge.

[kenshiro-o]

@dustismo I have finally found the time to merge the master changes into my branch. Take a look and let me know what you think!

Cheers,
Ed.

[mreiferson]

@kenshiro-o hi Ed 😄

I've taken over maintenance of this repo - what do we need to do here to get this in?

[kenshiro-o]

Hey @mreiferson 😸

I had completely forgotten about this PR...
I think we just need to check that I have not broken anything 😨 . But we've actually been running this version at Hailo for a long time so believe it's all good 😉 .