[jboss-cvs] JBoss Messaging SVN: r1399 - in trunk: src/main/org/jboss/jms/server/endpoint src/main/org/jboss/messaging/core src/main/org/jboss/messaging/core/plugin/postoffice src/main/org/jboss/messaging/core/plugin/postoffice/cluster tests/src/org/jboss/test/messaging/jms

jboss-cvs-commits at lists.jboss.org jboss-cvs-commits at lists.jboss.org
Fri Sep 29 10:22:26 EDT 2006


Author: timfox
Date: 2006-09-29 10:22:17 -0400 (Fri, 29 Sep 2006)
New Revision: 1399

Modified:
   trunk/src/main/org/jboss/jms/server/endpoint/ServerConsumerEndpoint.java
   trunk/src/main/org/jboss/jms/server/endpoint/ServerSessionEndpoint.java
   trunk/src/main/org/jboss/messaging/core/ChannelSupport.java
   trunk/src/main/org/jboss/messaging/core/plugin/postoffice/DefaultPostOffice.java
   trunk/src/main/org/jboss/messaging/core/plugin/postoffice/cluster/DefaultClusteredPostOffice.java
   trunk/tests/src/org/jboss/test/messaging/jms/DurableSubscriberTest.java
   trunk/tests/src/org/jboss/test/messaging/jms/MessageConsumerTest.java
   trunk/tests/src/org/jboss/test/messaging/jms/SecurityTest.java
Log:
Add check and fix tests for unsubscribing with active consumers



Modified: trunk/src/main/org/jboss/jms/server/endpoint/ServerConsumerEndpoint.java
===================================================================
--- trunk/src/main/org/jboss/jms/server/endpoint/ServerConsumerEndpoint.java	2006-09-29 14:02:09 UTC (rev 1398)
+++ trunk/src/main/org/jboss/jms/server/endpoint/ServerConsumerEndpoint.java	2006-09-29 14:22:17 UTC (rev 1399)
@@ -370,14 +370,10 @@
                
                Binding binding = topicPostOffice.getBindingForQueueName(queueName);
 
-               // TODO - quick fix for the situation when the subscription was destroyed (using
-               //        Session.unsubscribe()) while the consumer is active. Not sure this is the
-               //        best solution, though. See http://jira.jboss.org/jira/browse/JBMESSAGING-564
-//               if (binding == null)
-//               {
-//                  throw new IllegalStateException("Cannot find queue with name " + queueName);
-//               }
-
+               //Note binding can be null since there can many competing subscribers for the subscription  - 
+               //in which case the first will have removed the subscription and subsequently
+               //ones won't find it
+               
                if (binding != null && !binding.getQueue().isRecoverable())
                {
                   topicPostOffice.unbindQueue(queueName);

Modified: trunk/src/main/org/jboss/jms/server/endpoint/ServerSessionEndpoint.java
===================================================================
--- trunk/src/main/org/jboss/jms/server/endpoint/ServerSessionEndpoint.java	2006-09-29 14:02:09 UTC (rev 1398)
+++ trunk/src/main/org/jboss/jms/server/endpoint/ServerSessionEndpoint.java	2006-09-29 14:22:17 UTC (rev 1399)
@@ -55,6 +55,7 @@
 import org.jboss.jms.util.ExceptionUtil;
 import org.jboss.jms.util.MessageQueueNameHelper;
 import org.jboss.logging.Logger;
+import org.jboss.messaging.core.Queue;
 import org.jboss.messaging.core.local.PagingFilteredQueue;
 import org.jboss.messaging.core.plugin.IdManager;
 import org.jboss.messaging.core.plugin.contract.ClusteredPostOffice;
@@ -687,6 +688,8 @@
    
    public void unsubscribe(String subscriptionName) throws JMSException
    {
+      log.info("unsubscribing: " + subscriptionName);
+      
       try
       {
          if (closed)
@@ -715,6 +718,20 @@
                                                   subscriptionName + " to unsubscribe");
          }
          
+         //Section 6.11. JMS 1.1. spec:
+         // "It is erroneous for a client to delete a
+         //durable subscription while it has an active TopicSubscriber for it or while a
+         //message received by it is part of a current transaction or has not been
+         //acknowledged in the session."
+         
+         Queue sub = binding.getQueue();
+         
+         if (sub.numberOfReceivers() != 0)
+         {
+            throw new IllegalStateException("Cannot unsubscribe durable subscription " +
+                                            subscriptionName + " since it has active subscribers");
+         }
+         
          //Unbind it
   
          topicPostOffice.unbindQueue(queueName);

Modified: trunk/src/main/org/jboss/messaging/core/ChannelSupport.java
===================================================================
--- trunk/src/main/org/jboss/messaging/core/ChannelSupport.java	2006-09-29 14:02:09 UTC (rev 1398)
+++ trunk/src/main/org/jboss/messaging/core/ChannelSupport.java	2006-09-29 14:22:17 UTC (rev 1399)
@@ -346,6 +346,8 @@
     */
    public void removeAllReferences() throws Throwable
    {
+      log.info(this + " remnoving all references");
+      
       synchronized (refLock)
       {
          synchronized (deliveryLock)

Modified: trunk/src/main/org/jboss/messaging/core/plugin/postoffice/DefaultPostOffice.java
===================================================================
--- trunk/src/main/org/jboss/messaging/core/plugin/postoffice/DefaultPostOffice.java	2006-09-29 14:02:09 UTC (rev 1398)
+++ trunk/src/main/org/jboss/messaging/core/plugin/postoffice/DefaultPostOffice.java	2006-09-29 14:22:17 UTC (rev 1399)
@@ -205,6 +205,8 @@
    {
       if (trace) { log.trace(this + " unbinding queue " + queueName); }
             
+      log.info("unbinding queue: " + queueName);
+      
       if (queueName == null)
       {
          throw new IllegalArgumentException("Queue name is null");
@@ -220,7 +222,9 @@
          {
             //Need to remove from db too
             
-            deleteBinding(binding.getQueue().getName());                        
+            deleteBinding(binding.getQueue().getName());    
+            
+            log.info("deleting binding from db");
          }
          
          binding.getQueue().removeAllReferences();         
@@ -530,6 +534,8 @@
 
          int rows = ps.executeUpdate();
          
+         log.info("deleted " + rows + " rows");
+         
          return rows == 1;
       }
       finally

Modified: trunk/src/main/org/jboss/messaging/core/plugin/postoffice/cluster/DefaultClusteredPostOffice.java
===================================================================
--- trunk/src/main/org/jboss/messaging/core/plugin/postoffice/cluster/DefaultClusteredPostOffice.java	2006-09-29 14:02:09 UTC (rev 1398)
+++ trunk/src/main/org/jboss/messaging/core/plugin/postoffice/cluster/DefaultClusteredPostOffice.java	2006-09-29 14:22:17 UTC (rev 1399)
@@ -562,7 +562,7 @@
          
          if (binding != null)
          {
-            throw new IllegalArgumentException(this.nodeId + "Binding already exists for node Id " + nodeId + " queue name " + queueName);
+            throw new IllegalArgumentException(this.nodeId + " Binding already exists for node Id " + nodeId + " queue name " + queueName);
          }
             
          binding = this.createBinding(nodeId, condition, queueName, channelID, filterString, durable);

Modified: trunk/tests/src/org/jboss/test/messaging/jms/DurableSubscriberTest.java
===================================================================
--- trunk/tests/src/org/jboss/test/messaging/jms/DurableSubscriberTest.java	2006-09-29 14:02:09 UTC (rev 1398)
+++ trunk/tests/src/org/jboss/test/messaging/jms/DurableSubscriberTest.java	2006-09-29 14:22:17 UTC (rev 1399)
@@ -24,6 +24,7 @@
 import javax.jms.Connection;
 import javax.jms.ConnectionFactory;
 import javax.jms.DeliveryMode;
+import javax.jms.IllegalStateException;
 import javax.jms.InvalidDestinationException;
 import javax.jms.InvalidSelectorException;
 import javax.jms.JMSException;
@@ -462,6 +463,15 @@
 
    /**
     * See http://jira.jboss.org/jira/browse/JBMESSAGING-564
+    * 
+    * Tim - this is an invalid test - see section 6.11 of jms spec:
+    *  "It is erroneous for a client to delete a
+    *  durable subscription while it has an active TopicSubscriber for it or while a
+    *  message received by it is part of a current transaction or has not been
+    *  acknowledged in the session."
+    *  
+    *  Need to remove it
+    * 
     */
    public void testUnsubscribe() throws Exception
    {
@@ -484,7 +494,35 @@
 
       conn.close();
    }
+   
+   //See JMS 1.1. spec sec 6.11
+   public void testUnsubscribeWithActiveConsumer() throws Exception
+   {
+      ConnectionFactory cf = (ConnectionFactory)ic.lookup("ConnectionFactory");
+      Topic topic = (Topic)ic.lookup("/topic/Topic");
 
+      Connection conn = cf.createConnection();
+      conn.setClientID("zeke");
+
+      Session s = conn.createSession(false, Session.AUTO_ACKNOWLEDGE);
+
+      TopicSubscriber dursub = s.createDurableSubscriber(topic, "dursub0");
+
+      try
+      {
+         s.unsubscribe("dursub0");
+         fail();
+      }
+      catch (IllegalStateException e)
+      {
+         //Ok - it is illegal to ubscribe a subscription if it has active consumers
+      }
+         
+      dursub.close();
+
+      conn.close();
+   }
+
    // Package protected ---------------------------------------------
 
    // Protected -----------------------------------------------------

Modified: trunk/tests/src/org/jboss/test/messaging/jms/MessageConsumerTest.java
===================================================================
--- trunk/tests/src/org/jboss/test/messaging/jms/MessageConsumerTest.java	2006-09-29 14:02:09 UTC (rev 1398)
+++ trunk/tests/src/org/jboss/test/messaging/jms/MessageConsumerTest.java	2006-09-29 14:22:17 UTC (rev 1399)
@@ -2394,13 +2394,10 @@
 
       try
       {
-
          conn1 = cf.createConnection();
 
-
          conn1.setClientID(CLIENT_ID1);
 
-
          Session sess1 = conn1.createSession(false, Session.AUTO_ACKNOWLEDGE);
          MessageProducer prod = sess1.createProducer(topic);
          prod.setDeliveryMode(DeliveryMode.PERSISTENT);
@@ -2429,6 +2426,8 @@
          }
 
          assertEquals(NUM_MESSAGES, count);
+         
+         durable.close();
 
          sess1.unsubscribe("mySubscription");
       }
@@ -2625,6 +2624,8 @@
          }
 
          log.debug("unsubscribing mySubscription");
+         
+         durable.close();
          sess5.unsubscribe("mySubscription");
          log.debug("unsubscribing done");
          conn5.close();
@@ -2638,6 +2639,8 @@
 
          TextMessage tm3 = (TextMessage)durable.receive(1000);
          assertNull(tm3);
+         
+         durable.close();
       }
       finally
       {
@@ -2662,7 +2665,7 @@
             conn5.close();
          }
          if (sess6 != null)
-         {
+         {            
             sess6.unsubscribe("mySubscription");
          }
          if (conn6 != null)
@@ -2683,16 +2686,13 @@
 
       try
       {
-
          conn1 = cf.createConnection();
          conn1.setClientID(CLIENT_ID1);
 
-
          Session sess1 = conn1.createSession(false, Session.AUTO_ACKNOWLEDGE);
          MessageProducer prod = sess1.createProducer(topic);
          prod.setDeliveryMode(DeliveryMode.PERSISTENT);
 
-
          log.debug("creating durable subscription");
          MessageConsumer durable = sess1.createDurableSubscriber(topic, "mySubscription");
          log.debug("durable subscription created");
@@ -2752,6 +2752,8 @@
          log.trace("Received " + count  + " messages");
 
          assertEquals(NUM_MESSAGES - NUM_TO_RECEIVE, count);
+         
+         durable2.close();
 
          sess2.unsubscribe("mySubscription");
       }
@@ -2780,13 +2782,10 @@
 
       try
       {
-
          conn1 = cf.createConnection();
 
-
          conn1.setClientID(CLIENT_ID1);
 
-
          Session sess1 = conn1.createSession(false, Session.AUTO_ACKNOWLEDGE);
          MessageProducer prod = sess1.createProducer(topic);
          prod.setDeliveryMode(DeliveryMode.PERSISTENT);
@@ -2842,6 +2841,8 @@
          }
 
          assertEquals(0, count);
+         
+         durable2.close();
 
          sess2.unsubscribe("mySubscription");
       }

Modified: trunk/tests/src/org/jboss/test/messaging/jms/SecurityTest.java
===================================================================
--- trunk/tests/src/org/jboss/test/messaging/jms/SecurityTest.java	2006-09-29 14:02:09 UTC (rev 1398)
+++ trunk/tests/src/org/jboss/test/messaging/jms/SecurityTest.java	2006-09-29 14:22:17 UTC (rev 1399)
@@ -27,6 +27,7 @@
 import javax.jms.IllegalStateException;
 import javax.jms.JMSSecurityException;
 import javax.jms.Message;
+import javax.jms.MessageConsumer;
 import javax.jms.MessageProducer;
 import javax.jms.Queue;
 import javax.jms.Session;
@@ -828,7 +829,8 @@
 
       try
       {
-         sess.createDurableSubscriber(topic, subName);
+         MessageConsumer cons = sess.createDurableSubscriber(topic, subName);
+         cons.close();
          sess.unsubscribe(subName);
          log.trace("Successfully created and unsubscribed subscription");
          return true;




More information about the jboss-cvs-commits mailing list