Uploaded image for project: 'ActiveMQ Artemis'
  1. ActiveMQ Artemis
  2. ARTEMIS-607

Implement support for Interceptors with the MQTT protocol

Details

    • New Feature
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 1.3.0
    • 1.4.0
    • Broker
    • None

    Description

      Intercepting MQTT messages is not yet supported, only core protocol messages can be intercepted. I would like to request a feature for MQTT interceptor support.

      This feature was recently requested on the users@ mailing list http://activemq.2283324.n4.nabble.com/Interceptor-for-MQTT-td4713408.html

      I encountered this issue when trying to answer http://stackoverflow.com/questions/38101899/intercepting-mqtt-messages-in-artemis

      Attachments

        Activity

          jbertram Justin Bertram added a comment -

          This is not a bug, per se. Interceptors have to be implemented for each protocol since interceptors intercept implementation-specific packets. The "interceptor" example shipped with Artemis contains an interceptor which intercepts org.apache.activemq.artemis.core.protocol.core.Packet objects. These are clearly part of the core protocol so you shouldn't expect to intercept them from clients using other protocols.

          I recommend you open a feature request for the protocols for which you need interceptor support.

          FWIW, this was recently discussed on the user-list as it relates to MQTT already.

          jbertram Justin Bertram added a comment - This is not a bug, per se. Interceptors have to be implemented for each protocol since interceptors intercept implementation-specific packets. The "interceptor" example shipped with Artemis contains an interceptor which intercepts org.apache.activemq.artemis.core.protocol.core.Packet objects. These are clearly part of the core protocol so you shouldn't expect to intercept them from clients using other protocols. I recommend you open a feature request for the protocols for which you need interceptor support. FWIW, this was recently discussed on the user-list as it relates to MQTT already.
          johndament John D. Ament added a comment -

          I can't assign to myself, but I plan to raise a PR for this shortly.

          johndament John D. Ament added a comment - I can't assign to myself, but I plan to raise a PR for this shortly.
          githubbot ASF GitHub Bot added a comment -

          GitHub user johnament opened a pull request:

          https://github.com/apache/activemq-artemis/pull/617

          ARTEMIS-607 Added interceptor support for MQTT protocol.

          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.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/johnament/activemq-artemis ARTEMIS-607

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/activemq-artemis/pull/617.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #617


          commit 2a2b1dcda2943345102b335080795d4e6f3d6524
          Author: John D. Ament <johndament@apache.org>
          Date: 2016-07-04T02:30:13Z

          ARTEMIS-607 Added interceptor support for MQTT protocol.


          githubbot ASF GitHub Bot added a comment - GitHub user johnament opened a pull request: https://github.com/apache/activemq-artemis/pull/617 ARTEMIS-607 Added interceptor support for MQTT protocol. 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. You can merge this pull request into a Git repository by running: $ git pull https://github.com/johnament/activemq-artemis ARTEMIS-607 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/activemq-artemis/pull/617.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #617 commit 2a2b1dcda2943345102b335080795d4e6f3d6524 Author: John D. Ament <johndament@apache.org> Date: 2016-07-04T02:30:13Z ARTEMIS-607 Added interceptor support for MQTT protocol.
          githubbot ASF GitHub Bot added a comment -

          Github user johnament commented on a diff in the pull request:

          https://github.com/apache/activemq-artemis/pull/617#discussion_r69403171

          — Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/imported/MQTTTest.java —
          @@ -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());
              • End diff –

          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.

          githubbot ASF GitHub Bot added a comment - Github user johnament commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/617#discussion_r69403171 — Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/imported/MQTTTest.java — @@ -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()); End diff – 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.
          githubbot ASF GitHub Bot added a comment -

          Github user jbertram commented on the issue:

          https://github.com/apache/activemq-artemis/pull/617

          Could you squash these two commits together?

          githubbot ASF GitHub Bot added a comment - Github user jbertram commented on the issue: https://github.com/apache/activemq-artemis/pull/617 Could you squash these two commits together?
          githubbot ASF GitHub Bot added a comment -

          Github user clebertsuconic commented on the issue:

          https://github.com/apache/activemq-artemis/pull/617

          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)

          githubbot ASF GitHub Bot added a comment - Github user clebertsuconic commented on the issue: https://github.com/apache/activemq-artemis/pull/617 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)
          githubbot ASF GitHub Bot added a comment -

          Github user johnament commented on the issue:

          https://github.com/apache/activemq-artemis/pull/617

          Up to you. I won't be able to squash until this evening.

          githubbot ASF GitHub Bot added a comment - Github user johnament commented on the issue: https://github.com/apache/activemq-artemis/pull/617 Up to you. I won't be able to squash until this evening.
          githubbot ASF GitHub Bot added a comment -

          Github user johnament commented on the issue:

          https://github.com/apache/activemq-artemis/pull/617

          Ok, I rebased from master and squashed.

          githubbot ASF GitHub Bot added a comment - Github user johnament commented on the issue: https://github.com/apache/activemq-artemis/pull/617 Ok, I rebased from master and squashed.

          Commit 7c746c719e5018a0560fef27062ecd888dcc75d4 in activemq-artemis's branch refs/heads/master from johndament
          [ https://git-wip-us.apache.org/repos/asf?p=activemq-artemis.git;h=7c746c7 ]

          ARTEMIS-607 Added interceptor support for MQTT protocol.

          Also updated the maintainer's guide to clarify what is run in the PR builder.

          jira-bot ASF subversion and git services added a comment - Commit 7c746c719e5018a0560fef27062ecd888dcc75d4 in activemq-artemis's branch refs/heads/master from johndament [ https://git-wip-us.apache.org/repos/asf?p=activemq-artemis.git;h=7c746c7 ] ARTEMIS-607 Added interceptor support for MQTT protocol. Also updated the maintainer's guide to clarify what is run in the PR builder.
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/activemq-artemis/pull/617

          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/activemq-artemis/pull/617
          jdanek Jiri Daněk added a comment -

          Thanks for clarification and for the mailing list reference. I changed this issue into a Feature Request.

          jdanek Jiri Daněk added a comment - Thanks for clarification and for the mailing list reference. I changed this issue into a Feature Request.
          githubbot ASF GitHub Bot added a comment -

          GitHub user orpiske opened a pull request:

          https://github.com/apache/activemq-artemis/pull/1130

          Add a MQTT interceptor example

          This a sample code we created in order to verify ARTEMIS-607. It adds a simple MQTT interceptor that is run whenever a MQTT message is published.
          I talked with the developers and they would like to evaluate this example to see if it's suitable for inclusion in the code base.

          The example was based on the two interceptor examples that already exist on the code base and was modified to work with MQTT.

          Please advise if you have any questions or suggestions for this PR.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/orpiske/activemq-artemis master

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/activemq-artemis/pull/1130.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #1130


          commit 75a93588f5ecf6e9f8c1ea4af0f3819e779f71fa
          Author: Otavio Rodolfo Piske <opiske@redhat.com>
          Date: 2017-03-20T15:55:55Z

          ARTEMIS-607 Added an example/verification for calling an interceptor when using MQTT protocol


          githubbot ASF GitHub Bot added a comment - GitHub user orpiske opened a pull request: https://github.com/apache/activemq-artemis/pull/1130 Add a MQTT interceptor example This a sample code we created in order to verify ARTEMIS-607 . It adds a simple MQTT interceptor that is run whenever a MQTT message is published. I talked with the developers and they would like to evaluate this example to see if it's suitable for inclusion in the code base. The example was based on the two interceptor examples that already exist on the code base and was modified to work with MQTT. Please advise if you have any questions or suggestions for this PR. You can merge this pull request into a Git repository by running: $ git pull https://github.com/orpiske/activemq-artemis master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/activemq-artemis/pull/1130.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1130 commit 75a93588f5ecf6e9f8c1ea4af0f3819e779f71fa Author: Otavio Rodolfo Piske <opiske@redhat.com> Date: 2017-03-20T15:55:55Z ARTEMIS-607 Added an example/verification for calling an interceptor when using MQTT protocol

          Commit 20c67baa5af1391e8b58257190e25f59a5d9717a in activemq-artemis's branch refs/heads/master from orpiske
          [ https://git-wip-us.apache.org/repos/asf?p=activemq-artemis.git;h=20c67ba ]

          ARTEMIS-607 Added an example/verification for calling an interceptor when using MQTT protocol

          jira-bot ASF subversion and git services added a comment - Commit 20c67baa5af1391e8b58257190e25f59a5d9717a in activemq-artemis's branch refs/heads/master from orpiske [ https://git-wip-us.apache.org/repos/asf?p=activemq-artemis.git;h=20c67ba ] ARTEMIS-607 Added an example/verification for calling an interceptor when using MQTT protocol
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/activemq-artemis/pull/1130

          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/activemq-artemis/pull/1130
          githubbot ASF GitHub Bot added a comment -

          Github user michaelandrepearce commented on a diff in the pull request:

          https://github.com/apache/activemq-artemis/pull/1355#discussion_r123182665

          — Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AmqpSendReceiveInterceptorTest.java —
          @@ -26,8 +26,12 @@
          import org.apache.activemq.transport.amqp.client.AmqpReceiver;
          import org.apache.activemq.transport.amqp.client.AmqpSender;
          import org.apache.activemq.transport.amqp.client.AmqpSession;
          +import org.apache.felix.resolver.util.ArrayMap;
          — End diff –

          31 commits wowsies you work fast

          I think you may have accidentally brought in a lot of other commits in from your local repo, when doing your update

          githubbot ASF GitHub Bot added a comment - Github user michaelandrepearce commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/1355#discussion_r123182665 — Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AmqpSendReceiveInterceptorTest.java — @@ -26,8 +26,12 @@ import org.apache.activemq.transport.amqp.client.AmqpReceiver; import org.apache.activemq.transport.amqp.client.AmqpSender; import org.apache.activemq.transport.amqp.client.AmqpSession; +import org.apache.felix.resolver.util.ArrayMap; — End diff – 31 commits wowsies you work fast I think you may have accidentally brought in a lot of other commits in from your local repo, when doing your update
          githubbot ASF GitHub Bot added a comment -

          Github user michalxo commented on the issue:

          https://github.com/apache/activemq-artemis/pull/1355

          Yeah, I will close this PR and create a new one with corrected things.
          First real PR did not worked out well.

          githubbot ASF GitHub Bot added a comment - Github user michalxo commented on the issue: https://github.com/apache/activemq-artemis/pull/1355 Yeah, I will close this PR and create a new one with corrected things. First real PR did not worked out well.
          githubbot ASF GitHub Bot added a comment -

          Github user michalxo closed the pull request at:

          https://github.com/apache/activemq-artemis/pull/1355

          githubbot ASF GitHub Bot added a comment - Github user michalxo closed the pull request at: https://github.com/apache/activemq-artemis/pull/1355
          githubbot ASF GitHub Bot added a comment -

          Github user michaelandrepearce commented on the issue:

          https://github.com/apache/activemq-artemis/pull/1357

          If that test fails instead of adding build noise, maybe make it ignored or just add it as part of 1244 fix?

          githubbot ASF GitHub Bot added a comment - Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1357 If that test fails instead of adding build noise, maybe make it ignored or just add it as part of 1244 fix?
          githubbot ASF GitHub Bot added a comment -

          Github user michalxo commented on the issue:

          https://github.com/apache/activemq-artemis/pull/1357

          I am not a developer, but a tester. So this one is not up to me to decide.
          Talked to @andytaylor about he, told me it's ok if the test fails and jira is linked.

          githubbot ASF GitHub Bot added a comment - Github user michalxo commented on the issue: https://github.com/apache/activemq-artemis/pull/1357 I am not a developer, but a tester. So this one is not up to me to decide. Talked to @andytaylor about he, told me it's ok if the test fails and jira is linked.
          githubbot ASF GitHub Bot added a comment -

          Github user andytaylor commented on the issue:

          https://github.com/apache/activemq-artemis/pull/1357

          Im fine with it failing, we already have noise in several failing integration tests. Id rather that then it remain commented out once someone fixes it.

          githubbot ASF GitHub Bot added a comment - Github user andytaylor commented on the issue: https://github.com/apache/activemq-artemis/pull/1357 Im fine with it failing, we already have noise in several failing integration tests. Id rather that then it remain commented out once someone fixes it.
          githubbot ASF GitHub Bot added a comment -

          Github user jdanekrh commented on a diff in the pull request:

          https://github.com/apache/activemq-artemis/pull/617#discussion_r123258336

          — Diff: docs/user-manual/en/intercepting-operations.md —
          @@ -9,7 +9,9 @@ makes interceptors powerful, but also potentially dangerous.

            1. Implementing The Interceptors

          -An interceptor must implement the `Interceptor interface`:
          +All interceptors are protocol specific.
          +
          +An interceptor for the core protocol must implement the interface `Interceptor`:
          — End diff –

          I'd change that to

          implement the `Interceptor` interface

          sounds more like standard Java-speak to me that way.

          githubbot ASF GitHub Bot added a comment - Github user jdanekrh commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/617#discussion_r123258336 — Diff: docs/user-manual/en/intercepting-operations.md — @@ -9,7 +9,9 @@ makes interceptors powerful, but also potentially dangerous. Implementing The Interceptors -An interceptor must implement the `Interceptor interface`: +All interceptors are protocol specific. + +An interceptor for the core protocol must implement the interface `Interceptor`: — End diff – I'd change that to implement the `Interceptor` interface sounds more like standard Java-speak to me that way.
          githubbot ASF GitHub Bot added a comment -

          Github user clebertsuconic commented on the issue:

          https://github.com/apache/activemq-artemis/pull/1357

          @andytaylor actually, I have been trying to decrease noise... when you increase tests noise.. people tend to ignore the testsuite even more.

          githubbot ASF GitHub Bot added a comment - Github user clebertsuconic commented on the issue: https://github.com/apache/activemq-artemis/pull/1357 @andytaylor actually, I have been trying to decrease noise... when you increase tests noise.. people tend to ignore the testsuite even more.
          githubbot ASF GitHub Bot added a comment -

          Github user michalxo commented on the issue:

          https://github.com/apache/activemq-artemis/pull/1357

          @jdanekrh has made a patch for ARTEMIS-1244 issue. So this can be solved if PR is correct & accepted.

          githubbot ASF GitHub Bot added a comment - Github user michalxo commented on the issue: https://github.com/apache/activemq-artemis/pull/1357 @jdanekrh has made a patch for ARTEMIS-1244 issue. So this can be solved if PR is correct & accepted.
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/activemq-artemis/pull/1357

          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/activemq-artemis/pull/1357

          People

            johndament John D. Ament
            jdanek Jiri Daněk
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: