New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ARTEMIS-607 Added interceptor support for MQTT protocol. #617
Conversation
@@ -380,7 +384,8 @@ public void testMQTTPathPatterns() throws Exception { | |||
Message msg = connection.receive(5, TimeUnit.SECONDS); | |||
do { | |||
assertNotNull("RETAINED null " + wildcard, msg); | |||
assertTrue("RETAINED prefix " + wildcard, new String(msg.getPayload()).startsWith(RETAINED)); | |||
String msgPayload = new String(msg.getPayload()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One worrisome thing. I think there's some interdependence in these tests. I originally added my interceptor checks in a new test method. When I did that, this test started failing. It was receiving the messages created previously. I suspect the server isn't getting cleaned up 100% correctly, or maybe not at all.
Could you squash these two commits together? |
These look good.. I can squash them at the time of the merge. I will run the whole testsuite just in case. (It takes 2 hours or so now) |
Up to you. I won't be able to squash until this evening. |
Also updated the maintainer's guide to clarify what is run in the PR builder.
Ok, I rebased from master and squashed. |
An interceptor must implement the `Interceptor interface`: | ||
All interceptors are protocol specific. | ||
|
||
An interceptor for the core protocol must implement the interface `Interceptor`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tiny thing. I'd myself change that to
implement the `Interceptor` interface
sounds more like standard Java-speak to me that way.
BTW, feel free to cry foul on things that don't look right. I was trying to follow the patterns in Stomp, but the structure just doesn't line up. I didn't want to duplicate the logic so I did move into a common base class. The result though is that the protocol handler knows about its parent protocol handler manager.