[GitHub] activemq-artemis pull request #1389: ARTEMIS-1272 Artemis incorrectly handle...

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis pull request #1389: ARTEMIS-1272 Artemis incorrectly handle...

franz1981
GitHub user Odyldzhon opened a pull request:

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

    ARTEMIS-1272 Artemis incorrectly handle MQTT acknowledgement

   

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

    $ git pull https://github.com/Odyldzhon/activemq-artemis ARTEMIS-1272_MQTT_acknowledgement_issue

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

    https://github.com/apache/activemq-artemis/pull/1389.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 #1389
   
----
commit 021fd9a8e03a563eceb0c7bed1aa52f85696f973
Author: Odyldzhon Toshbekov <[hidden email]>
Date:   2017-07-07T23:41:41Z

    fix mqtt acknowledgement issue

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1389: ARTEMIS-1272 Artemis incorrectly handle MQTT a...

franz1981
Github user tabish121 commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1389
 
    This fix lacks any tests so it is not verifiable and would not be protected into the future


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1389: ARTEMIS-1272 Artemis incorrectly handle MQTT a...

franz1981
In reply to this post by franz1981
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1389
 
    @mtaylor  if you received two messages on the client... if you ack the last one, what should be the outcome? both acked.. or just the one you acked?
   
    Just trying to figure out what the MQTT spec says...  I may find out on monday.. right now (saturday) it's easier to ask :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1389: ARTEMIS-1272 Artemis incorrectly handle MQTT a...

franz1981
In reply to this post by franz1981
Github user Odyldzhon commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1389
 
    I have added some tests for QOS 0,1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1389: ARTEMIS-1272 Artemis incorrectly handle MQTT a...

franz1981
In reply to this post by franz1981
Github user mtaylor commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1389
 
    @clebertsuconic For QoS1 and QoS2 **every message** must be ack'd.  There is no optimisation like we have in CORE to do batching of acks.  This includes duplicate messages, note: It is possible for clients to have overlapping subscriptions and receive duplicates:
   
    > "When Clients make subscriptions with Topic Filters that include wildcards, it is possible for a Client’s subscriptions to overlap so that a published message might match multiple filters. In this case the Server MUST deliver the message to the Client respecting the maximum QoS of all the matching subscriptions [MQTT-3.3.5-1]. In addition, the Server MAY deliver further copies of the message, one for each additional matching subscription and respecting the subscription’s QoS in each case. "
   
    >


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1389: ARTEMIS-1272 Artemis incorrectly handle MQTT a...

franz1981
In reply to this post by franz1981
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1389
 
    Ok. The patch here is really urgent then.  


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1389: ARTEMIS-1272 Artemis incorrectly handle MQTT a...

franz1981
In reply to this post by franz1981
Github user mtaylor commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1389
 
    @clebertsuconic Yes.  We should also verify for QoS2 and any MQTT management messages we use.  I can take a look Monday.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis pull request #1389: ARTEMIS-1272 Artemis incorrectly handle...

franz1981
In reply to this post by franz1981
Github user mtaylor commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1389#discussion_r126466676
 
    --- Diff: tests/integration-tests/pom.xml ---
    @@ -177,6 +178,11 @@
           <!-- END MQTT Deps -->
     
           <dependency>
    +         <groupId>org.awaitility</groupId>
    +         <artifactId>awaitility</artifactId>
    +         <version>2.0.0</version>
    +      </dependency>
    +      <dependency>
    --- End diff --
   
    Rather than introducing new deps, you can re-use the wait utility we already in the tests.  I'll add an example.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis pull request #1389: ARTEMIS-1272 Artemis incorrectly handle...

franz1981
In reply to this post by franz1981
Github user mtaylor commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1389#discussion_r126465351
 
    --- Diff: tests/integration-tests/pom.xml ---
    @@ -167,8 +167,9 @@
              <artifactId>mqtt-client</artifactId>
           </dependency>
           <dependency>
    -          <groupId>org.eclipse.paho</groupId>
    -          <artifactId>mqtt-client</artifactId>
    +         <groupId>org.eclipse.paho</groupId>
    +         <artifactId>org.eclipse.paho.client.mqttv3</artifactId>
    +         <version>1.1.0</version>
    --- End diff --
   
    Can you please put the version in the dependency management section of the pom.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis pull request #1389: ARTEMIS-1272 Artemis incorrectly handle...

franz1981
In reply to this post by franz1981
Github user mtaylor commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1389#discussion_r126467111
 
    --- Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/MqttAcknowledgementTest.java ---
    @@ -0,0 +1,127 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements. See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License. You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.activemq.artemis.tests.integration.mqtt;
    +
    +import java.util.LinkedList;
    +import java.util.concurrent.TimeUnit;
    +
    +import org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTestSupport;
    +import org.awaitility.Awaitility;
    +import org.awaitility.core.ConditionTimeoutException;
    +import org.eclipse.paho.client.mqttv3.IMqttDeliveryToken;
    +import org.eclipse.paho.client.mqttv3.MqttCallback;
    +import org.eclipse.paho.client.mqttv3.MqttClient;
    +import org.eclipse.paho.client.mqttv3.MqttConnectOptions;
    +import org.eclipse.paho.client.mqttv3.MqttException;
    +import org.eclipse.paho.client.mqttv3.MqttMessage;
    +import org.eclipse.paho.client.mqttv3.persist.MemoryPersistence;
    +import org.jgroups.util.UUID;
    +import org.junit.After;
    +import org.junit.Test;
    +
    +public class MqttAcknowledgementTest extends MQTTTestSupport {
    +
    +   private volatile LinkedList<Integer> messageIds = new LinkedList<>();
    +   private volatile boolean messageArrived = false;
    +
    +   private MqttClient subscriber;
    +   MqttClient sender;
    +
    +   @After
    +   public void clean() throws MqttException {
    +      messageArrived = false;
    +      messageIds.clear();
    +      if (subscriber.isConnected()) {
    +         subscriber.disconnect();
    +      }
    +      if (sender.isConnected()) {
    +         sender.disconnect();
    +      }
    +      subscriber.close();
    +      sender.close();
    +   }
    +
    +   @Test(timeout = 300000)
    +   public void testAcknowledgementQOS1() throws MqttException {
    +      test(1);
    +   }
    +
    +   @Test(timeout = 300000, expected = ConditionTimeoutException.class)
    +   public void testAcknowledgementQOS0() throws MqttException {
    +      test(0);
    +   }
    +
    +   private void test(int qos) throws MqttException {
    +      String subscriberId = UUID.randomUUID().toString();
    +      String senderId = UUID.randomUUID().toString();
    +      String topic = UUID.randomUUID().toString();
    +
    +      subscriber = createMqttClient(subscriberId);
    +      subscriber.subscribe(topic, qos);
    +
    +      sender = createMqttClient(senderId);
    +      sender.publish(topic, UUID.randomUUID().toString().getBytes(), qos, false);
    +      sender.publish(topic, UUID.randomUUID().toString().getBytes(), qos, false);
    +
    +      Awaitility.await().atMost(5_000, TimeUnit.MILLISECONDS).until(() -> messageIds.size() == 2);
    +
    +      subscriber.messageArrivedComplete(messageIds.getLast(), qos);
    +      subscriber.disconnect();
    +      subscriber.close();
    +      messageArrived = false;
    +
    +      Awaitility.await().atMost(60_000, TimeUnit.MILLISECONDS).until(() -> {
    +         try {
    +            subscriber = createMqttClient(subscriberId);
    +            return true;
    +         } catch (MqttException e) {
    +            return false;
    +         }
    +      });
    --- End diff --
   
    You can reuse the Wait.waitFor(Condition) method to do the same thing:
   
    ```java
          Wait.waitFor(new Wait.Condition() {
             @Override
             public boolean isSatisfied() throws Exception {
                try {
                   subscriber = createMqttClient(subscriberId);
                   return true;
                } catch (MqttException e) {
                   return false;
                }
             }
          }, 60000);
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis pull request #1389: ARTEMIS-1272 Artemis incorrectly handle...

franz1981
In reply to this post by franz1981
Github user mtaylor commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1389#discussion_r126467367
 
    --- Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/MqttAcknowledgementTest.java ---
    @@ -0,0 +1,127 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements. See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License. You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.activemq.artemis.tests.integration.mqtt;
    +
    +import java.util.LinkedList;
    +import java.util.concurrent.TimeUnit;
    +
    +import org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTestSupport;
    +import org.awaitility.Awaitility;
    +import org.awaitility.core.ConditionTimeoutException;
    +import org.eclipse.paho.client.mqttv3.IMqttDeliveryToken;
    +import org.eclipse.paho.client.mqttv3.MqttCallback;
    +import org.eclipse.paho.client.mqttv3.MqttClient;
    +import org.eclipse.paho.client.mqttv3.MqttConnectOptions;
    +import org.eclipse.paho.client.mqttv3.MqttException;
    +import org.eclipse.paho.client.mqttv3.MqttMessage;
    +import org.eclipse.paho.client.mqttv3.persist.MemoryPersistence;
    +import org.jgroups.util.UUID;
    +import org.junit.After;
    +import org.junit.Test;
    +
    +public class MqttAcknowledgementTest extends MQTTTestSupport {
    +
    +   private volatile LinkedList<Integer> messageIds = new LinkedList<>();
    +   private volatile boolean messageArrived = false;
    +
    +   private MqttClient subscriber;
    +   MqttClient sender;
    +
    +   @After
    +   public void clean() throws MqttException {
    +      messageArrived = false;
    +      messageIds.clear();
    +      if (subscriber.isConnected()) {
    +         subscriber.disconnect();
    +      }
    +      if (sender.isConnected()) {
    +         sender.disconnect();
    +      }
    +      subscriber.close();
    +      sender.close();
    +   }
    +
    +   @Test(timeout = 300000)
    +   public void testAcknowledgementQOS1() throws MqttException {
    +      test(1);
    +   }
    +
    +   @Test(timeout = 300000, expected = ConditionTimeoutException.class)
    +   public void testAcknowledgementQOS0() throws MqttException {
    +      test(0);
    +   }
    +
    +   private void test(int qos) throws MqttException {
    +      String subscriberId = UUID.randomUUID().toString();
    +      String senderId = UUID.randomUUID().toString();
    +      String topic = UUID.randomUUID().toString();
    +
    +      subscriber = createMqttClient(subscriberId);
    +      subscriber.subscribe(topic, qos);
    +
    +      sender = createMqttClient(senderId);
    +      sender.publish(topic, UUID.randomUUID().toString().getBytes(), qos, false);
    +      sender.publish(topic, UUID.randomUUID().toString().getBytes(), qos, false);
    +
    +      Awaitility.await().atMost(5_000, TimeUnit.MILLISECONDS).until(() -> messageIds.size() == 2);
    +
    +      subscriber.messageArrivedComplete(messageIds.getLast(), qos);
    +      subscriber.disconnect();
    +      subscriber.close();
    +      messageArrived = false;
    +
    +      Awaitility.await().atMost(60_000, TimeUnit.MILLISECONDS).until(() -> {
    +         try {
    +            subscriber = createMqttClient(subscriberId);
    +            return true;
    +         } catch (MqttException e) {
    +            return false;
    +         }
    +      });
    +
    +      Awaitility.await().atMost(5_000, TimeUnit.MILLISECONDS).until(() -> messageArrived == true);
    +   }
    +
    --- End diff --
   
    Same comment as above.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis pull request #1389: ARTEMIS-1272 Artemis incorrectly handle...

franz1981
In reply to this post by franz1981
Github user Odyldzhon commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1389#discussion_r126518160
 
    --- Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/MqttAcknowledgementTest.java ---
    @@ -0,0 +1,127 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements. See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License. You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.activemq.artemis.tests.integration.mqtt;
    +
    +import java.util.LinkedList;
    +import java.util.concurrent.TimeUnit;
    +
    +import org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTestSupport;
    +import org.awaitility.Awaitility;
    +import org.awaitility.core.ConditionTimeoutException;
    +import org.eclipse.paho.client.mqttv3.IMqttDeliveryToken;
    +import org.eclipse.paho.client.mqttv3.MqttCallback;
    +import org.eclipse.paho.client.mqttv3.MqttClient;
    +import org.eclipse.paho.client.mqttv3.MqttConnectOptions;
    +import org.eclipse.paho.client.mqttv3.MqttException;
    +import org.eclipse.paho.client.mqttv3.MqttMessage;
    +import org.eclipse.paho.client.mqttv3.persist.MemoryPersistence;
    +import org.jgroups.util.UUID;
    +import org.junit.After;
    +import org.junit.Test;
    +
    +public class MqttAcknowledgementTest extends MQTTTestSupport {
    +
    +   private volatile LinkedList<Integer> messageIds = new LinkedList<>();
    +   private volatile boolean messageArrived = false;
    +
    +   private MqttClient subscriber;
    +   MqttClient sender;
    +
    +   @After
    +   public void clean() throws MqttException {
    +      messageArrived = false;
    +      messageIds.clear();
    +      if (subscriber.isConnected()) {
    +         subscriber.disconnect();
    +      }
    +      if (sender.isConnected()) {
    +         sender.disconnect();
    +      }
    +      subscriber.close();
    +      sender.close();
    +   }
    +
    +   @Test(timeout = 300000)
    +   public void testAcknowledgementQOS1() throws MqttException {
    +      test(1);
    +   }
    +
    +   @Test(timeout = 300000, expected = ConditionTimeoutException.class)
    +   public void testAcknowledgementQOS0() throws MqttException {
    +      test(0);
    +   }
    +
    +   private void test(int qos) throws MqttException {
    +      String subscriberId = UUID.randomUUID().toString();
    +      String senderId = UUID.randomUUID().toString();
    +      String topic = UUID.randomUUID().toString();
    +
    +      subscriber = createMqttClient(subscriberId);
    +      subscriber.subscribe(topic, qos);
    +
    +      sender = createMqttClient(senderId);
    +      sender.publish(topic, UUID.randomUUID().toString().getBytes(), qos, false);
    +      sender.publish(topic, UUID.randomUUID().toString().getBytes(), qos, false);
    +
    +      Awaitility.await().atMost(5_000, TimeUnit.MILLISECONDS).until(() -> messageIds.size() == 2);
    +
    +      subscriber.messageArrivedComplete(messageIds.getLast(), qos);
    +      subscriber.disconnect();
    +      subscriber.close();
    +      messageArrived = false;
    +
    +      Awaitility.await().atMost(60_000, TimeUnit.MILLISECONDS).until(() -> {
    +         try {
    +            subscriber = createMqttClient(subscriberId);
    +            return true;
    +         } catch (MqttException e) {
    +            return false;
    +         }
    +      });
    --- End diff --
   
    done


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis pull request #1389: ARTEMIS-1272 Artemis incorrectly handle...

franz1981
In reply to this post by franz1981
Github user Odyldzhon commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1389#discussion_r126518183
 
    --- Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/MqttAcknowledgementTest.java ---
    @@ -0,0 +1,127 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements. See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License. You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.activemq.artemis.tests.integration.mqtt;
    +
    +import java.util.LinkedList;
    +import java.util.concurrent.TimeUnit;
    +
    +import org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTestSupport;
    +import org.awaitility.Awaitility;
    +import org.awaitility.core.ConditionTimeoutException;
    +import org.eclipse.paho.client.mqttv3.IMqttDeliveryToken;
    +import org.eclipse.paho.client.mqttv3.MqttCallback;
    +import org.eclipse.paho.client.mqttv3.MqttClient;
    +import org.eclipse.paho.client.mqttv3.MqttConnectOptions;
    +import org.eclipse.paho.client.mqttv3.MqttException;
    +import org.eclipse.paho.client.mqttv3.MqttMessage;
    +import org.eclipse.paho.client.mqttv3.persist.MemoryPersistence;
    +import org.jgroups.util.UUID;
    +import org.junit.After;
    +import org.junit.Test;
    +
    +public class MqttAcknowledgementTest extends MQTTTestSupport {
    +
    +   private volatile LinkedList<Integer> messageIds = new LinkedList<>();
    +   private volatile boolean messageArrived = false;
    +
    +   private MqttClient subscriber;
    +   MqttClient sender;
    +
    +   @After
    +   public void clean() throws MqttException {
    +      messageArrived = false;
    +      messageIds.clear();
    +      if (subscriber.isConnected()) {
    +         subscriber.disconnect();
    +      }
    +      if (sender.isConnected()) {
    +         sender.disconnect();
    +      }
    +      subscriber.close();
    +      sender.close();
    +   }
    +
    +   @Test(timeout = 300000)
    +   public void testAcknowledgementQOS1() throws MqttException {
    +      test(1);
    +   }
    +
    +   @Test(timeout = 300000, expected = ConditionTimeoutException.class)
    +   public void testAcknowledgementQOS0() throws MqttException {
    +      test(0);
    +   }
    +
    +   private void test(int qos) throws MqttException {
    +      String subscriberId = UUID.randomUUID().toString();
    +      String senderId = UUID.randomUUID().toString();
    +      String topic = UUID.randomUUID().toString();
    +
    +      subscriber = createMqttClient(subscriberId);
    +      subscriber.subscribe(topic, qos);
    +
    +      sender = createMqttClient(senderId);
    +      sender.publish(topic, UUID.randomUUID().toString().getBytes(), qos, false);
    +      sender.publish(topic, UUID.randomUUID().toString().getBytes(), qos, false);
    +
    +      Awaitility.await().atMost(5_000, TimeUnit.MILLISECONDS).until(() -> messageIds.size() == 2);
    +
    +      subscriber.messageArrivedComplete(messageIds.getLast(), qos);
    +      subscriber.disconnect();
    +      subscriber.close();
    +      messageArrived = false;
    +
    +      Awaitility.await().atMost(60_000, TimeUnit.MILLISECONDS).until(() -> {
    +         try {
    +            subscriber = createMqttClient(subscriberId);
    +            return true;
    +         } catch (MqttException e) {
    +            return false;
    +         }
    +      });
    +
    +      Awaitility.await().atMost(5_000, TimeUnit.MILLISECONDS).until(() -> messageArrived == true);
    +   }
    +
    --- End diff --
   
    done


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis pull request #1389: ARTEMIS-1272 Artemis incorrectly handle...

franz1981
In reply to this post by franz1981
Github user Odyldzhon commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1389#discussion_r126518213
 
    --- Diff: tests/integration-tests/pom.xml ---
    @@ -177,6 +178,11 @@
           <!-- END MQTT Deps -->
     
           <dependency>
    +         <groupId>org.awaitility</groupId>
    +         <artifactId>awaitility</artifactId>
    +         <version>2.0.0</version>
    +      </dependency>
    +      <dependency>
    --- End diff --
   
    removed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis pull request #1389: ARTEMIS-1272 Artemis incorrectly handle...

franz1981
In reply to this post by franz1981
Github user Odyldzhon commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1389#discussion_r126518567
 
    --- Diff: tests/integration-tests/pom.xml ---
    @@ -167,8 +167,9 @@
              <artifactId>mqtt-client</artifactId>
           </dependency>
           <dependency>
    -          <groupId>org.eclipse.paho</groupId>
    -          <artifactId>mqtt-client</artifactId>
    +         <groupId>org.eclipse.paho</groupId>
    +         <artifactId>org.eclipse.paho.client.mqttv3</artifactId>
    +         <version>1.1.0</version>
    --- End diff --
   
    I have put this at the top of file as property, hope you meant this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1389: ARTEMIS-1272 Artemis incorrectly handle MQTT a...

franz1981
In reply to this post by franz1981
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1389
 
    I will merge this by squashing all the commits into one with this description:
   
    ```ARTEMIS-1272 fix mqtt acknowledgement issue```
   
    I'm just running final tests now.. but if you want to squash it yourself...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis pull request #1389: ARTEMIS-1272 Artemis incorrectly handle...

franz1981
In reply to this post by franz1981
Github user asfgit closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Loading...