activemq-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rajdav...@apache.org
Subject svn commit: r926152 - in /activemq/trunk/activemq-core/src: main/java/org/apache/activemq/transport/tcp/ test/java/org/apache/activemq/transport/tcp/
Date Mon, 22 Mar 2010 16:09:44 GMT
Author: rajdavies
Date: Mon Mar 22 16:09:44 2010
New Revision: 926152

URL: http://svn.apache.org/viewvc?rev=926152&view=rev
Log:
Applied updated patch for https://issues.apache.org/activemq/browse/AMQ-2636

Modified:
    activemq/trunk/activemq-core/src/main/java/org/apache/activemq/transport/tcp/QualityOfServiceUtils.java
    activemq/trunk/activemq-core/src/main/java/org/apache/activemq/transport/tcp/TcpTransport.java
    activemq/trunk/activemq-core/src/test/java/org/apache/activemq/transport/tcp/QualityOfServiceUtilsTest.java
    activemq/trunk/activemq-core/src/test/java/org/apache/activemq/transport/tcp/TransportUriTest.java

Modified: activemq/trunk/activemq-core/src/main/java/org/apache/activemq/transport/tcp/QualityOfServiceUtils.java
URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-core/src/main/java/org/apache/activemq/transport/tcp/QualityOfServiceUtils.java?rev=926152&r1=926151&r2=926152&view=diff
==============================================================================
--- activemq/trunk/activemq-core/src/main/java/org/apache/activemq/transport/tcp/QualityOfServiceUtils.java
(original)
+++ activemq/trunk/activemq-core/src/main/java/org/apache/activemq/transport/tcp/QualityOfServiceUtils.java
Mon Mar 22 16:09:44 2010
@@ -28,29 +28,32 @@ import java.util.Map;
  */
 public class QualityOfServiceUtils {
 
-    private static final int MAX_DIFF_SERV = 64;
+    private static final int MAX_DIFF_SERV = 63;
     private static final int MIN_DIFF_SERV = 0;
     private static final Map<String, Integer> DIFF_SERV_NAMES
-	= new HashMap<String, Integer>();
+        = new HashMap<String, Integer>();
     // TODO: Find other names used for Differentiated Services values.
     static {
-	DIFF_SERV_NAMES.put("EF", 46);
-	DIFF_SERV_NAMES.put("AF11", 10);
-	DIFF_SERV_NAMES.put("AF12", 12);
-	DIFF_SERV_NAMES.put("AF13", 14);
-	DIFF_SERV_NAMES.put("AF21", 18);
-	DIFF_SERV_NAMES.put("AF22", 20);
-	DIFF_SERV_NAMES.put("AF23", 22);
-	DIFF_SERV_NAMES.put("AF31", 26);
-	DIFF_SERV_NAMES.put("AF32", 28);
-	DIFF_SERV_NAMES.put("AF33", 30);
- 	DIFF_SERV_NAMES.put("AF41", 34);
-	DIFF_SERV_NAMES.put("AF42", 36);
-	DIFF_SERV_NAMES.put("AF43", 38);
+        DIFF_SERV_NAMES.put("EF", 46);
+        DIFF_SERV_NAMES.put("AF11", 10);
+        DIFF_SERV_NAMES.put("AF12", 12);
+        DIFF_SERV_NAMES.put("AF13", 14);
+        DIFF_SERV_NAMES.put("AF21", 18);
+        DIFF_SERV_NAMES.put("AF22", 20);
+        DIFF_SERV_NAMES.put("AF23", 22);
+        DIFF_SERV_NAMES.put("AF31", 26);
+        DIFF_SERV_NAMES.put("AF32", 28);
+        DIFF_SERV_NAMES.put("AF33", 30);
+        DIFF_SERV_NAMES.put("AF41", 34);
+        DIFF_SERV_NAMES.put("AF42", 36);
+        DIFF_SERV_NAMES.put("AF43", 38);
     }
 
+    private static final int MAX_TOS = 255;
+    private static final int MIN_TOS = 0;
+
     /**
-     * @param The value to be used for Differentiated Services.
+     * @param value A potential value to be used for Differentiated Services.
      * @return The corresponding Differentiated Services Code Point (DSCP).
      * @throws IllegalArgumentException if the value does not correspond to a
      *         Differentiated Services Code Point or setting the DSCP is not
@@ -60,52 +63,67 @@ public class QualityOfServiceUtils {
         int intValue = -1;
 
         // Check the names first.
-	if (DIFF_SERV_NAMES.containsKey(value)) {
+        if (DIFF_SERV_NAMES.containsKey(value)) {
             intValue = DIFF_SERV_NAMES.get(value);
-	} else {
+        } else {
             try {
                 intValue = Integer.parseInt(value);
-                if (intValue >= MAX_DIFF_SERV || intValue < MIN_DIFF_SERV) {
-                    throw new IllegalArgumentException("Differentiated Services "
-                            + "value: " + intValue + " must be between "
-                            + MIN_DIFF_SERV + " and " + (MAX_DIFF_SERV - 1) + ".");
+                if (intValue > MAX_DIFF_SERV || intValue < MIN_DIFF_SERV) {
+                    throw new IllegalArgumentException("Differentiated Services"
+                        + " value: " + intValue + " not in legal range ["
+                        + MIN_DIFF_SERV + ", " + MAX_DIFF_SERV + "].");
                 }
             } catch (NumberFormatException e) {
                 // value must have been a malformed name.
-	        throw new IllegalArgumentException("No such Differentiated "
-                        + "Services name: " + value);
+                throw new IllegalArgumentException("No such Differentiated "
+                    + "Services name: " + value);
             }
         }
 
         return adjustDSCPForECN(intValue);
      }
 
+
+    /**
+     * @param value A potential value to be used for Type of Service.
+     * @return A valid value that can be used to set the Type of Service in the
+     *         packet headers.
+     * @throws IllegalArgumentException if the value is not a legal Type of
+     *         Service value.
+     */
+    public static int getToS(int value) throws IllegalArgumentException {
+        if (value > MAX_TOS || value < MIN_TOS) {
+            throw new IllegalArgumentException("Type of Service value: "
+                + value + " not in legal range [" + MIN_TOS + ", " + MAX_TOS
+                + ".");
+        }
+        return value;
+    }
+
     /**
      * The Differentiated Services values use only 6 of the 8 bits in the field
      * in the TCP/IP packet header. Make sure any values the system has set for
      * the other two bits (the ECN bits) are maintained.
      *
-     * @param The Differentiated Services Code Point.
+     * @param dscp The Differentiated Services Code Point.
      * @return A Differentiated Services Code Point that respects the ECN bits
      *         set on the system.
      * @throws IllegalArgumentException if setting Differentiated Services is
      *         not supported.
      */
-    private static int adjustDSCPForECN(int value)
+    private static int adjustDSCPForECN(int dscp)
             throws IllegalArgumentException {
-	// The only way to see if there are any values set for the ECN is to
+        // The only way to see if there are any values set for the ECN is to
         // read the traffic class automatically set by the system and isolate
         // the ECN bits.
-	Socket socket = new Socket();
+        Socket socket = new Socket();
         try {
             int systemTrafficClass = socket.getTrafficClass();
             // The 7th and 8th bits of the system traffic class are the ECN bits.
-            return value | (systemTrafficClass & 192);
+            return dscp | (systemTrafficClass & 192);
         } catch (SocketException e) {
-            throw new IllegalArgumentException("Setting Differentiated Services "
-                    + "not supported: " + e);
+            throw new IllegalArgumentException("Setting Differentiated Services"
+                + " not supported: " + e);
         }
     }
-
-    // TODO: Add getter methods for ToS values.
 }

Modified: activemq/trunk/activemq-core/src/main/java/org/apache/activemq/transport/tcp/TcpTransport.java
URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-core/src/main/java/org/apache/activemq/transport/tcp/TcpTransport.java?rev=926152&r1=926151&r2=926152&view=diff
==============================================================================
--- activemq/trunk/activemq-core/src/main/java/org/apache/activemq/transport/tcp/TcpTransport.java
(original)
+++ activemq/trunk/activemq-core/src/main/java/org/apache/activemq/transport/tcp/TcpTransport.java
Mon Mar 22 16:09:44 2010
@@ -69,15 +69,21 @@ public class TcpTransport extends Transp
     protected DataInputStream dataIn;
     protected TcpBufferedOutputStream buffOut = null;
     /**
-     * Differentiated Services Code Point. Determines the Traffic Class to be
-     * set on the socket.
+     * The Traffic Class to be set on the socket.
      */
-    protected int dscp = 0;
+    protected int trafficClass = 0;
     /**
-     * Keeps track of attempts to set the Traffic Class.
+     * Keeps track of attempts to set the Traffic Class on the socket.
      */
     private boolean trafficClassSet = false;
     /**
+     * Prevents setting both the Differentiated Services and Type of Service
+     * transport options at the same time, since they share the same spot in
+     * the TCP/IP packet headers.
+     */
+    protected boolean diffServChosen = false;
+    protected boolean typeOfServiceChosen = false;
+    /**
      * trace=true -> the Transport stack where this TcpTransport
      * object will be, will have a TransportLogger layer
      * trace=false -> the Transport stack where this TcpTransport
@@ -225,14 +231,25 @@ public class TcpTransport extends Transp
         // This is the value requested by the user by setting the Tcp Transport
         // options. If the socket hasn't been created, then this value may not
         // reflect the value returned by Socket.getTrafficClass().
-        return Integer.toString(dscp);
+        return Integer.toString(this.trafficClass);
     }
 
     public void setDiffServ(String diffServ) throws IllegalArgumentException {
-        this.dscp = QualityOfServiceUtils.getDSCP(diffServ);
+        this.trafficClass = QualityOfServiceUtils.getDSCP(diffServ);
+        this.diffServChosen = true;
     }
 
-    // TODO: Add methods for setting and getting a ToS value.
+    public int getTypeOfService() {
+        // This is the value requested by the user by setting the Tcp Transport
+        // options. If the socket hasn't been created, then this value may not
+        // reflect the value returned by Socket.getTrafficClass().
+        return this.trafficClass;
+    }
+  
+    public void setTypeOfService(int typeOfService) {
+        this.trafficClass = QualityOfServiceUtils.getToS(typeOfService);
+        this.typeOfServiceChosen = true;
+    }
 
     public boolean isTrace() {
         return trace;
@@ -394,9 +411,11 @@ public class TcpTransport extends Transp
      * Configures the socket for use
      * 
      * @param sock
-     * @throws SocketException
+     * @throws SocketException, IllegalArgumentException if setting the options
+     *         on the socket failed.
      */
-    protected void initialiseSocket(Socket sock) throws SocketException {
+    protected void initialiseSocket(Socket sock) throws SocketException,
+            IllegalArgumentException {
         if (socketOptions != null) {
             IntrospectionSupport.setProperties(socket, socketOptions);
         }
@@ -416,8 +435,8 @@ public class TcpTransport extends Transp
         if (tcpNoDelay != null) {
             sock.setTcpNoDelay(tcpNoDelay.booleanValue());
         }
-        if (!trafficClassSet) {
-            trafficClassSet = setTrafficClass(sock);
+        if (!this.trafficClassSet) {
+            this.trafficClassSet = setTrafficClass(sock);
         }
     }
 
@@ -448,7 +467,7 @@ public class TcpTransport extends Transp
         }
         // Set the traffic class before the socket is connected when possible so
         // that the connection packets are given the correct traffic class.
-        trafficClassSet = setTrafficClass(socket);
+        this.trafficClassSet = setTrafficClass(socket);
 
         if (socket != null) {
 
@@ -607,26 +626,33 @@ public class TcpTransport extends Transp
         return receiveCounter;
     }
     
+
     /**
+     * @param sock The socket on which to set the Traffic Class.
      * @return Whether or not the Traffic Class was set on the given socket.
-     */
-    private boolean setTrafficClass(Socket sock) {
-        // TODO: Add in ToS support.
-
-        if (sock == null)
+     * @throws SocketException if the system does not support setting the
+     *         Traffic Class.
+     * @throws IllegalArgumentException if both the Differentiated Services and
+     *         Type of Services transport options have been set on the same
+     *         connection.
+     */
+    private boolean setTrafficClass(Socket sock) throws SocketException,
+            IllegalArgumentException {
+        if (sock == null
+            || (!this.diffServChosen && !this.typeOfServiceChosen)) {
             return false;
-
-        boolean success = false;
-
-        try {
-            sock.setTrafficClass(this.dscp);
-            success = true;
-        } catch (SocketException e) {
-            // The system does not support setting the traffic class through
-            // setTrafficClass.
-            LOG.error("Unable to set the traffic class: " + e);
         }
-
-        return success;
+        if (this.diffServChosen && this.typeOfServiceChosen) {
+            throw new IllegalArgumentException("Cannot set both the "
+                + " Differentiated Services and Type of Services transport "
+                + " options on the same connection.");
+        }
+        sock.setTrafficClass(this.trafficClass);
+        // Reset the guards that prevent both the Differentiated Services
+        // option and the Type of Service option from being set on the same
+        // connection.
+        this.diffServChosen = false;
+        this.typeOfServiceChosen = false;
+        return true;
     }
 }

Modified: activemq/trunk/activemq-core/src/test/java/org/apache/activemq/transport/tcp/QualityOfServiceUtilsTest.java
URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-core/src/test/java/org/apache/activemq/transport/tcp/QualityOfServiceUtilsTest.java?rev=926152&r1=926151&r2=926152&view=diff
==============================================================================
--- activemq/trunk/activemq-core/src/test/java/org/apache/activemq/transport/tcp/QualityOfServiceUtilsTest.java
(original)
+++ activemq/trunk/activemq-core/src/test/java/org/apache/activemq/transport/tcp/QualityOfServiceUtilsTest.java
Mon Mar 22 16:09:44 2010
@@ -22,47 +22,52 @@ import java.util.Map;
 import junit.framework.TestCase;
 
 public class QualityOfServiceUtilsTest extends TestCase {
+    /**
+     * Keeps track of the value that the System has set for the ECN bits, which
+     * should not be overridden when Differentiated Services is set, but may be
+     * overridden when Type of Service is set.
+     */
     private int ECN;
 
     protected void setUp() throws Exception {
-	Socket socket = new Socket();
-	ECN = socket.getTrafficClass();
-	ECN = ECN & Integer.parseInt("11000000", 2);
+        Socket socket = new Socket();
+        ECN = socket.getTrafficClass();
+        ECN = ECN & Integer.parseInt("11000000", 2);
     }
 
     protected void tearDown() throws Exception {
-	super.tearDown();
+        super.tearDown();
     }
 
     public void testValidDiffServIntegerValues() {
-	int[] values = {0, 1, 32, 62, 63};
+        int[] values = {0, 1, 32, 62, 63};
         for (int val : values) {
-	    testValidDiffServIntegerValue(val);
-	}
+            testValidDiffServIntegerValue(val);
+        }
     }
 
     public void testInvalidDiffServIntegerValues() {
-	int[] values = {-2, -1, 64, 65};
+        int[] values = {-2, -1, 64, 65};
         for (int val : values) {
-	    testInvalidDiffServIntegerValue(val);
-	}
+            testInvalidDiffServIntegerValue(val);
+        }
     }
 
     public void testValidDiffServNames() {
         Map<String, Integer> namesToExpected = new HashMap<String, Integer>();
-	namesToExpected.put("EF", Integer.valueOf("101110", 2));
-	namesToExpected.put("AF11", Integer.valueOf("001010", 2));
-	namesToExpected.put("AF12", Integer.valueOf("001100", 2));
-	namesToExpected.put("AF13", Integer.valueOf("001110", 2));
-	namesToExpected.put("AF21", Integer.valueOf("010010", 2));
-	namesToExpected.put("AF22", Integer.valueOf("010100", 2));
-	namesToExpected.put("AF23", Integer.valueOf("010110", 2));
-	namesToExpected.put("AF31", Integer.valueOf("011010", 2));
-	namesToExpected.put("AF32", Integer.valueOf("011100", 2));
-	namesToExpected.put("AF33", Integer.valueOf("011110", 2));
- 	namesToExpected.put("AF41", Integer.valueOf("100010", 2));
-	namesToExpected.put("AF42", Integer.valueOf("100100", 2));
-	namesToExpected.put("AF43", Integer.valueOf("100110", 2));
+        namesToExpected.put("EF", Integer.valueOf("101110", 2));
+        namesToExpected.put("AF11", Integer.valueOf("001010", 2));
+        namesToExpected.put("AF12", Integer.valueOf("001100", 2));
+        namesToExpected.put("AF13", Integer.valueOf("001110", 2));
+        namesToExpected.put("AF21", Integer.valueOf("010010", 2));
+        namesToExpected.put("AF22", Integer.valueOf("010100", 2));
+        namesToExpected.put("AF23", Integer.valueOf("010110", 2));
+        namesToExpected.put("AF31", Integer.valueOf("011010", 2));
+        namesToExpected.put("AF32", Integer.valueOf("011100", 2));
+        namesToExpected.put("AF33", Integer.valueOf("011110", 2));
+        namesToExpected.put("AF41", Integer.valueOf("100010", 2));
+        namesToExpected.put("AF42", Integer.valueOf("100100", 2));
+        namesToExpected.put("AF43", Integer.valueOf("100110", 2));
         for (String name : namesToExpected.keySet()) {
             testValidDiffServName(name, namesToExpected.get(name));
         }
@@ -75,49 +80,80 @@ public class QualityOfServiceUtilsTest e
         }
     }
 
-    private void testValidDiffServIntegerValue(int val) {
+    private void testValidDiffServName(String name, int expected) {
         int dscp = -1;
-	try {
-	    dscp = QualityOfServiceUtils.getDSCP(Integer.toString(val));
-	} catch (IllegalArgumentException e) {
-	    fail("IllegalArgumentException thrown for valid Differentiated Services "
-                 + "value: " + val);
-	}
+        try {
+            dscp = QualityOfServiceUtils.getDSCP(name);
+        } catch (IllegalArgumentException e) {
+            fail("IllegalArgumentException thrown for valid Differentiated "
+                 + " Services name: " + name);
+        }
         // Make sure it adjusted for any system ECN values.
-	assertEquals("Incorrect Differentiated Services Code Point " 
-		     + dscp + " returned for value " + val + ".",
-                     ECN | val, dscp);
+        assertEquals("Incorrect Differentiated Services Code Point "  + dscp
+            + " returned for name " + name + ".", ECN | expected, dscp);
+    }
+
+    private void testInvalidDiffServName(String name) {
+        try {
+            int dscp = QualityOfServiceUtils.getDSCP(name);
+            fail("No IllegalArgumentException thrown for invalid Differentiated"
+                 + " Services value: " + name + ".");
+        } catch (IllegalArgumentException e) {
+        }
+    }
+    
+    private void testValidDiffServIntegerValue(int val) {
+        try {
+            int dscp = QualityOfServiceUtils.getDSCP(Integer.toString(val));
+            // Make sure it adjusted for any system ECN values.
+            assertEquals("Incorrect Differentiated Services Code Point "
+                + "returned for value " + val + ".", ECN | val, dscp);
+        } catch (IllegalArgumentException e) {
+            fail("IllegalArgumentException thrown for valid Differentiated "
+                 + "Services value " + val);
+        }
     }
 
     private void testInvalidDiffServIntegerValue(int val) {
-	try {
-	    int dscp = QualityOfServiceUtils.getDSCP(Integer.toString(val));
-	    fail("No IllegalArgumentException thrown for invalid Differentiated "
-                 + "Services value: " + val + ".");
-	} catch (IllegalArgumentException e) {
-	}
+        try {
+            int dscp = QualityOfServiceUtils.getDSCP(Integer.toString(val));
+            fail("No IllegalArgumentException thrown for invalid "
+                + "Differentiated Services value " + val + ".");
+        } catch (IllegalArgumentException expected) {
+        }
     }
 
-    private void testValidDiffServName(String name, int expected) {
-        int dscp = -1;
-	try {
-	    dscp = QualityOfServiceUtils.getDSCP(name);
-	} catch (IllegalArgumentException e) {
-	    fail("IllegalArgumentException thrown for valid Differentiated "
-                 + " Services name: " + name);
-	}
-        // Make sure it adjusted for any system ECN values.
-	assertEquals("Incorrect Differentiated Services Code Point " 
-		     + dscp + " returned for name " + name + ".",
-                     ECN | expected, dscp);
+    public void testValidTypeOfServiceValues() {
+        int[] values = {0, 1, 32, 100, 255};
+        for (int val : values) {
+            testValidTypeOfServiceValue(val);
+        }
     }
 
-    private void testInvalidDiffServName(String name) {
-	try {
-	    int dscp = QualityOfServiceUtils.getDSCP(name);
-	    fail("No IllegalArgumentException thrown for invalid Differentiated "
-                 + "Services value: " + name + ".");
-	} catch (IllegalArgumentException e) {
-	}
+    public void testInvalidTypeOfServiceValues() {
+        int[] values = {-2, -1, 256, 257};
+        for (int val : values) {
+            testInvalidTypeOfServiceValue(val);
+        }
+    }
+
+    private void testValidTypeOfServiceValue(int val) {
+        try {
+            int typeOfService = QualityOfServiceUtils.getToS(val);
+            assertEquals("Incorrect Type of Services value returned for " + val
+                + ".", val, typeOfService);
+        } catch (IllegalArgumentException e) {
+            fail("IllegalArgumentException thrown for valid Type of Service "
+                 + "value " + val + ".");
+        }
+    }
+
+    private void testInvalidTypeOfServiceValue(int val) {
+        try {
+            int typeOfService = QualityOfServiceUtils.getToS(val);
+            fail("No IllegalArgumentException thrown for invalid "
+                + "Type of Service value " + val + ".");
+        } catch (IllegalArgumentException expected) {
+        }
     }
 }

Modified: activemq/trunk/activemq-core/src/test/java/org/apache/activemq/transport/tcp/TransportUriTest.java
URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-core/src/test/java/org/apache/activemq/transport/tcp/TransportUriTest.java?rev=926152&r1=926151&r2=926152&view=diff
==============================================================================
--- activemq/trunk/activemq-core/src/test/java/org/apache/activemq/transport/tcp/TransportUriTest.java
(original)
+++ activemq/trunk/activemq-core/src/test/java/org/apache/activemq/transport/tcp/TransportUriTest.java
Mon Mar 22 16:09:44 2010
@@ -32,17 +32,19 @@ import org.apache.commons.logging.LogFac
  */
 public class TransportUriTest extends EmbeddedBrokerTestSupport {
 
-	private static final Log LOG = LogFactory.getLog(TransportUriTest.class);
-	
+    private static final Log LOG = LogFactory.getLog(TransportUriTest.class);
+    private static final String DIFF_SERV = "&diffServ=";
+    private static final String TOS = "&typeOfService=";
+
     protected Connection connection;
     
     public String prefix;
     public String postfix;
-    
+
+
     public void initCombosForTestUriOptionsWork() {
-		addCombinationValues("prefix", new Object[] {""});
-		addCombinationValues("postfix", new Object[] {"?tcpNoDelay=true&keepAlive=true"});
-	}
+        initSharedCombos();
+    }
 
     public void testUriOptionsWork() throws Exception {
         String uri = prefix + bindAddress + postfix;
@@ -53,99 +55,116 @@ public class TransportUriTest extends Em
     }
 
     public void initCombosForTestValidDiffServOptionsWork() {
-	addCombinationValues("prefix", new Object[] {""});
-	// TODO: Add more combinations so that we know it plays nice with other
-	// transport options.
-	addCombinationValues("postfix", new Object[] {"?tcpNoDelay=true&keepAlive=true"});
+        initSharedCombos();
     }
 
     public void testValidDiffServOptionsWork() throws Exception {
         String[] validIntegerOptions = {"0", "1", "32", "62", "63"};
         for (String opt : validIntegerOptions) {
-            testValidDiffServOption(opt);
+            testValidOptionsWork(DIFF_SERV + opt, "");
         }
         String[] validNameOptions = {"EF", "AF11", "AF12", "AF13", "AF21",
-                                     "AF22", "AF23", "AF31", "AF32", "AF33",
-                                     "AF41", "AF42", "AF43"};
+            "AF22", "AF23", "AF31", "AF32", "AF33", "AF41", "AF42", "AF43"};
         for (String opt : validNameOptions) {
-            testValidDiffServOption(opt);
-        }
-    }
-
-    private void testValidDiffServOption(String value) {
-        String uri = prefix + bindAddress + postfix + "&diffServ=" + value;
-        LOG.info("Connecting via: " + uri);
-
-        try {
-            connection = new ActiveMQConnectionFactory(uri).createConnection();
-            connection.start();
-        } catch (Exception e) {
-	    fail("Valid Differentiated Services option: diffServ=" + value
-		 + ", should not have thrown an exception: " + e);
+            testValidOptionsWork(DIFF_SERV + opt, "");
         }
     }
 
     public void initCombosForTestInvalidDiffServOptionDoesNotWork() {
-	addCombinationValues("prefix", new Object[] {""});
-	// TODO: Add more combinations so that we know it plays nice with other
-	// transport options.
-	addCombinationValues("postfix", new Object[] {"?tcpNoDelay=true&keepAlive=true"});
+        initSharedCombos();
     }
 
     public void testInvalidDiffServOptionsDoesNotWork() throws Exception {
         String[] invalidIntegerOptions = {"-2", "-1", "64", "65", "100", "255"};
         for (String opt : invalidIntegerOptions) {
-            testInvalidDiffServOption(opt);
+            testInvalidOptionsDoNotWork(DIFF_SERV + opt, "");
         }
         String[] invalidNameOptions = {"hi", "", "A", "AF", "-AF21"};
         for (String opt : invalidNameOptions) {
-            testInvalidDiffServOption(opt);
+            testInvalidOptionsDoNotWork(DIFF_SERV + opt, "");
         }
     }
 
-    private void testInvalidDiffServOption(String value) {
-        String uri = prefix + bindAddress + postfix + "&diffServ=" + value;
-        LOG.info("Connecting via: " + uri);
+    public void initCombosForTestValidTypeOfServiceOptionsWork() {
+        initSharedCombos();
+    }
 
-        try {
-            connection = new ActiveMQConnectionFactory(uri).createConnection();
-            connection.start();
-            fail("Invalid Differentiated Services option: diffServ=" + value
-                 + " should have thrown an exception!");
-        } catch (Exception expected) {
+    public void testValidTypeOfServiceOptionsWork() throws Exception {
+        int[] validOptions = {0, 1, 32, 100, 254, 255};
+        for (int opt : validOptions) {
+            testValidOptionsWork(TOS + opt, "");
         }
     }
 
-	public void initCombosForTestBadVersionNumberDoesNotWork() {
-		addCombinationValues("prefix", new Object[] {""});
-		addCombinationValues("postfix", new Object[] {"?tcpNoDelay=true&keepAlive=true"});
-	}
+    public void initCombosForTestInvalidTypeOfServiceOptionDoesNotWork() {
+        initSharedCombos();
+    }
+
+    public void testInvalidTypeOfServiceOptionDoesNotWork() throws Exception {
+        int[] invalidOptions = {-2, -1, 256, 257};
+        for (int opt : invalidOptions) {
+            testInvalidOptionsDoNotWork(TOS + opt, "");
+        }
+    }
+
+    public void initCombosForTestDiffServAndTypeOfServiceMutuallyExclusive() {
+        initSharedCombos();
+    }
+
+    public void testDiffServAndTypeServiceMutuallyExclusive() {
+        String msg = "It should not be possible to set both Differentiated "
+            + "Services and Type of Service options on the same connection "
+            + "URI.";
+        testInvalidOptionsDoNotWork(TOS + 32 + DIFF_SERV, msg);
+        testInvalidOptionsDoNotWork(DIFF_SERV + 32 + TOS + 32, msg);
+    }
+
+    public void initCombosForTestBadVersionNumberDoesNotWork() {
+        initSharedCombos();
+    }
 
     public void testBadVersionNumberDoesNotWork() throws Exception {
-        String uri = prefix + bindAddress + postfix + "&minmumWireFormatVersion=65535";
+        testInvalidOptionsDoNotWork("&minmumWireFormatVersion=65535", "");
+    }
+
+    public void initCombosForTestBadPropertyNameFails() {
+        initSharedCombos();
+    }
+        
+    public void testBadPropertyNameFails() throws Exception {
+        testInvalidOptionsDoNotWork("&cheese=abc", "");
+    }
+
+    private void initSharedCombos() {
+        addCombinationValues("prefix", new Object[] {""});
+        // TODO: Add more combinations.
+        addCombinationValues("postfix", new Object[]
+            {"?tcpNoDelay=true&keepAlive=true"});
+    }
+
+    private void testValidOptionsWork(String options, String msg) {
+        String uri = prefix + bindAddress + postfix + options;
         LOG.info("Connecting via: " + uri);
 
         try {
             connection = new ActiveMQConnectionFactory(uri).createConnection();
             connection.start();
-            fail("Should have thrown an exception!");
-        } catch (Exception expected) {
+        } catch (Exception unexpected) {
+            fail("Valid options '" + options + "' on URI '" + uri + "' should "
+                 + "not have caused an exception to be thrown. " + msg
+                 + " Exception: " + unexpected);
         }
     }
 
-	public void initCombosForTestBadPropertyNameFails() {
-		addCombinationValues("prefix", new Object[] {""});
-		addCombinationValues("postfix", new Object[] {"?tcpNoDelay=true&keepAlive=true"});
-	}
-	
-    public void testBadPropertyNameFails() throws Exception {
-        String uri = prefix + bindAddress + postfix + "&cheese=abc";
+    private void testInvalidOptionsDoNotWork(String options, String msg) {
+        String uri = prefix + bindAddress + postfix + options;
         LOG.info("Connecting via: " + uri);
 
         try {
             connection = new ActiveMQConnectionFactory(uri).createConnection();
             connection.start();
-            fail("Should have thrown an exception!");
+            fail("Invalid options '" + options + "' on URI '" + uri + "' should"
+                 + " have caused an exception to be thrown. " + msg);
         } catch (Exception expected) {
         }
     }
@@ -175,6 +194,6 @@ public class TransportUriTest extends Em
     }
     
     public static Test suite() {
-    	return suite(TransportUriTest.class);
+        return suite(TransportUriTest.class);
     }
 }



Mime
View raw message