From 1c933cfbaeae6518e2fcf55bcb58f3b714f04370 Mon Sep 17 00:00:00 2001
From: jbertram <jbertram@apache.org>
Date: Thu, 17 Sep 2015 15:46:18 -0500
Subject: [PATCH] ARTEMIS-225 validate clientID is set for durable sub

---
 .../ra/inflow/ActiveMQActivationSpec.java     |  5 ++
 .../ra/ActiveMQMessageHandlerTest.java        | 59 +++++++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/artemis-ra/src/main/java/org/apache/activemq/artemis/ra/inflow/ActiveMQActivationSpec.java b/artemis-ra/src/main/java/org/apache/activemq/artemis/ra/inflow/ActiveMQActivationSpec.java
index f80342b4f4..0b46b20da6 100644
--- a/artemis-ra/src/main/java/org/apache/activemq/artemis/ra/inflow/ActiveMQActivationSpec.java
+++ b/artemis-ra/src/main/java/org/apache/activemq/artemis/ra/inflow/ActiveMQActivationSpec.java
@@ -696,6 +696,11 @@ public class ActiveMQActivationSpec extends ConnectionFactoryProperties implemen
             propsNotSet.add(new PropertyDescriptor("subscriptionName", ActiveMQActivationSpec.class));
             errorMessages.add("If subscription is durable then subscription name must be specified.");
          }
+
+         if ((isSubscriptionDurable() && getClientID() == null) || (isSubscriptionDurable() && getClientID() != null && getClientID().length() == 0)) {
+            propsNotSet.add(new PropertyDescriptor("clientID", ActiveMQActivationSpec.class));
+            errorMessages.add("If subscription is durable then clientID must be specified.");
+         }
       }
       catch (IntrospectionException e) {
          e.printStackTrace();
diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/ra/ActiveMQMessageHandlerTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/ra/ActiveMQMessageHandlerTest.java
index a5636cfc80..7f94f81eea 100644
--- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/ra/ActiveMQMessageHandlerTest.java
+++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/ra/ActiveMQMessageHandlerTest.java
@@ -714,6 +714,65 @@ public class ActiveMQMessageHandlerTest extends ActiveMQRATestBase {
       }
    }
 
+   @Test
+   public void testNullClientID() throws Exception {
+      ActiveMQResourceAdapter qResourceAdapter = newResourceAdapter();
+      MyBootstrapContext ctx = new MyBootstrapContext();
+      qResourceAdapter.start(ctx);
+
+      ActiveMQActivationSpec spec = new ActiveMQActivationSpec();
+      spec.setResourceAdapter(qResourceAdapter);
+      spec.setUseJNDI(false);
+      spec.setDestination("mdbTopic");
+      spec.setSubscriptionDurability("Durable");
+      spec.setSubscriptionName("sub");
+      spec.setSetupAttempts(1);
+      spec.setShareSubscriptions(true);
+      spec.setMaxSession(1);
+
+      CountDownLatch latch = new CountDownLatch(5);
+      DummyMessageEndpoint endpoint = new DummyMessageEndpoint(latch);
+      DummyMessageEndpointFactory endpointFactory = new DummyMessageEndpointFactory(endpoint, false);
+      try {
+         qResourceAdapter.endpointActivation(endpointFactory, spec);
+         fail();
+      }
+      catch (Exception e) {
+         assertTrue(e instanceof InvalidPropertyException);
+         assertEquals("clientID", ((InvalidPropertyException) e).getInvalidPropertyDescriptors()[0].getName());
+      }
+   }
+
+   @Test
+   public void testEmptyClientID() throws Exception {
+      ActiveMQResourceAdapter qResourceAdapter = newResourceAdapter();
+      MyBootstrapContext ctx = new MyBootstrapContext();
+      qResourceAdapter.start(ctx);
+
+      ActiveMQActivationSpec spec = new ActiveMQActivationSpec();
+      spec.setResourceAdapter(qResourceAdapter);
+      spec.setUseJNDI(false);
+      spec.setDestination("mdbTopic");
+      spec.setSubscriptionDurability("Durable");
+      spec.setSubscriptionName("sub");
+      spec.setClientID("");
+      spec.setSetupAttempts(1);
+      spec.setShareSubscriptions(true);
+      spec.setMaxSession(1);
+
+      CountDownLatch latch = new CountDownLatch(5);
+      DummyMessageEndpoint endpoint = new DummyMessageEndpoint(latch);
+      DummyMessageEndpointFactory endpointFactory = new DummyMessageEndpointFactory(endpoint, false);
+      try {
+         qResourceAdapter.endpointActivation(endpointFactory, spec);
+         fail();
+      }
+      catch (Exception e) {
+         assertTrue(e instanceof InvalidPropertyException);
+         assertEquals("clientID", ((InvalidPropertyException) e).getInvalidPropertyDescriptors()[0].getName());
+      }
+   }
+
    @Test
    public void testBadDestinationType() throws Exception {
       ActiveMQResourceAdapter qResourceAdapter = newResourceAdapter();