You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by rh...@apache.org on 2008/05/23 21:16:06 UTC

svn commit: r659631 - in /incubator/qpid/trunk/qpid/java: client/src/main/java/org/apache/qpid/client/ client/src/test/java/org/apache/qpid/test/unit/basic/ common/ common/src/main/java/org/apache/qpidity/transport/ common/src/main/java/org/apache/qpid...

Author: rhs
Date: Fri May 23 12:16:04 2008
New Revision: 659631

URL: http://svn.apache.org/viewvc?rev=659631&view=rev
Log:
QPID-901: Track and report session exceptions, modified generator validate values before trying to encode them. Also, moved createDurableSubscriber from AMQSession_0_10 -> AMQSession.

Added:
    incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/SessionException.java   (with props)
    incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/codec/Validator.java   (with props)
Modified:
    incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java
    incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java
    incubator/qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/test/unit/basic/PropertyValueTest.java
    incubator/qpid/trunk/qpid/java/common/Composite.tpl
    incubator/qpid/trunk/qpid/java/common/genutil.py
    incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/ChannelDelegate.java
    incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/Session.java
    incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/SessionDelegate.java
    incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/codec/AbstractEncoder.java

Modified: incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java
URL: http://svn.apache.org/viewvc/incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java?rev=659631&r1=659630&r2=659631&view=diff
==============================================================================
--- incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java (original)
+++ incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java Fri May 23 12:16:04 2008
@@ -919,28 +919,22 @@
 
     public abstract TopicSubscriber createDurableSubscriber(Topic topic, String name) throws JMSException;
 
-    /** Note, currently this does not handle reuse of the same name with different topics correctly. */
     public TopicSubscriber createDurableSubscriber(Topic topic, String name, String messageSelector, boolean noLocal)
-            throws JMSException
+        throws JMSException
     {
         checkNotClosed();
         checkValidTopic(topic);
-        AMQTopic dest = AMQTopic.createDurableTopic((AMQTopic) topic, name, _connection);
-        try
-        {
-            BasicMessageConsumer consumer = (BasicMessageConsumer) createConsumer(dest, messageSelector, noLocal);
-            TopicSubscriberAdaptor subscriber = new TopicSubscriberAdaptor(dest, consumer);
-            _subscriptions.put(name, subscriber);
-            _reverseSubscriptionMap.put(subscriber.getMessageConsumer(), name);
-
-            return subscriber;
-        }
-        catch (JMSException e)
+        if (_subscriptions.containsKey(name))
         {
-            deleteQueue(dest.getAMQQueueName());
-            throw e;
+            _subscriptions.get(name).close();
         }
-        
+        AMQTopic dest = AMQTopic.createDurableTopic((AMQTopic) topic, name, _connection);
+        BasicMessageConsumer consumer = (BasicMessageConsumer) createConsumer(dest, messageSelector, noLocal);
+        TopicSubscriberAdaptor subscriber = new TopicSubscriberAdaptor(dest, consumer);
+        _subscriptions.put(name, subscriber);
+        _reverseSubscriptionMap.put(subscriber.getMessageConsumer(), name);
+
+        return subscriber;
     }
 
     public MapMessage createMapMessage() throws JMSException

Modified: incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java
URL: http://svn.apache.org/viewvc/incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java?rev=659631&r1=659630&r2=659631&view=diff
==============================================================================
--- incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java (original)
+++ incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java Fri May 23 12:16:04 2008
@@ -125,33 +125,6 @@
 
     //------- overwritten methods of class AMQSession
 
-     public TopicSubscriber createDurableSubscriber(Topic topic, String name, String messageSelector, boolean noLocal)
-            throws JMSException
-    {
-        checkNotClosed();
-        checkValidTopic(topic);
-        if (_subscriptions.containsKey(name))
-        {
-            _subscriptions.get(name).close();
-        }
-        AMQTopic dest = AMQTopic.createDurableTopic((AMQTopic) topic, name, _connection);
-        try
-        {
-            BasicMessageConsumer consumer = (BasicMessageConsumer) createConsumer(dest, messageSelector, noLocal);
-            TopicSubscriberAdaptor subscriber = new TopicSubscriberAdaptor(dest, consumer);
-            _subscriptions.put(name, subscriber);
-            _reverseSubscriptionMap.put(subscriber.getMessageConsumer(), name);
-
-            return subscriber;
-        }
-        catch (JMSException e)
-        {
-            deleteQueue(dest.getAMQQueueName());
-            throw e;
-        }
-
-    }
-
     /**
      * Acknowledge one or many messages.
      *

Modified: incubator/qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/test/unit/basic/PropertyValueTest.java
URL: http://svn.apache.org/viewvc/incubator/qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/test/unit/basic/PropertyValueTest.java?rev=659631&r1=659630&r2=659631&view=diff
==============================================================================
--- incubator/qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/test/unit/basic/PropertyValueTest.java (original)
+++ incubator/qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/test/unit/basic/PropertyValueTest.java Fri May 23 12:16:04 2008
@@ -113,6 +113,7 @@
                 }
                 catch (Exception e)
                 {
+                    _logger.error("exception:", e);
                     fail("Unable to initialilse connection: " + e);
                 }
 

Modified: incubator/qpid/trunk/qpid/java/common/Composite.tpl
URL: http://svn.apache.org/viewvc/incubator/qpid/trunk/qpid/java/common/Composite.tpl?rev=659631&r1=659630&r2=659631&view=diff
==============================================================================
--- incubator/qpid/trunk/qpid/java/common/Composite.tpl (original)
+++ incubator/qpid/trunk/qpid/java/common/Composite.tpl Fri May 23 12:16:04 2008
@@ -8,6 +8,7 @@
 import org.apache.qpidity.transport.codec.Decoder;
 import org.apache.qpidity.transport.codec.Encodable;
 import org.apache.qpidity.transport.codec.Encoder;
+import org.apache.qpidity.transport.codec.Validator;
 
 import org.apache.qpidity.transport.network.Frame;
 
@@ -136,6 +137,7 @@
     }
 
     public final $name $(f.set)($(f.type) value) {
+        $(f.check)
         this.$(f.name) = value;
         this.has_$(f.name) = true;
         this.dirty = true;
@@ -143,6 +145,7 @@
     }
 
     public final $name $(f.name)($(f.type) value) {
+        $(f.check)
         this.$(f.name) = value;
         this.has_$(f.name) = true;
         this.dirty = true;

Modified: incubator/qpid/trunk/qpid/java/common/genutil.py
URL: http://svn.apache.org/viewvc/incubator/qpid/trunk/qpid/java/common/genutil.py?rev=659631&r1=659630&r2=659631&view=diff
==============================================================================
--- incubator/qpid/trunk/qpid/java/common/genutil.py (original)
+++ incubator/qpid/trunk/qpid/java/common/genutil.py Fri May 23 12:16:04 2008
@@ -152,15 +152,18 @@
     if self.type_node.name == "struct":
       self.read = "(%s) dec.readStruct(%s.TYPE)" % (tname, tname)
       self.write = "enc.writeStruct(%s.TYPE, check(struct).%s)" % (tname, self.name)
+      self.check = ""
       self.coder = "Struct"
     elif self.type_node.name == "domain":
       self.coder = camel(0, resolve_type(self.type_node)["@name"])
       self.read = "%s.get(dec.read%s())" % (tname, self.coder)
       self.write = "enc.write%s(check(struct).%s.getValue())" % (self.coder, self.name)
+      self.check = ""
     else:
       self.coder = camel(0, self.type_node["@name"])
       self.read = "dec.read%s()" % self.coder
       self.write = "enc.write%s(check(struct).%s)" % (self.coder, self.name)
+      self.check = "Validator.check%s(value);" % self.coder
     self.type = jtype(self.type_node)
     self.default = DEFAULTS.get(self.type, "null")
     self.has = camel(1, "has", self.name)

Modified: incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/ChannelDelegate.java
URL: http://svn.apache.org/viewvc/incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/ChannelDelegate.java?rev=659631&r1=659630&r2=659631&view=diff
==============================================================================
--- incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/ChannelDelegate.java (original)
+++ incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/ChannelDelegate.java Fri May 23 12:16:04 2008
@@ -47,4 +47,9 @@
         // weak hash map in connection.
     }
 
+    public @Override void sessionDetach(Channel channel, SessionDetach dtc)
+    {
+        channel.getSession().closed();
+    }
+
 }

Modified: incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/Session.java
URL: http://svn.apache.org/viewvc/incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/Session.java?rev=659631&r1=659630&r2=659631&view=diff
==============================================================================
--- incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/Session.java (original)
+++ incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/Session.java Fri May 23 12:16:04 2008
@@ -26,6 +26,7 @@
 import org.apache.qpidity.transport.util.Logger;
 
 import java.nio.ByteBuffer;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
@@ -310,7 +311,7 @@
             {
                 if (closed.get())
                 {
-                    throw new RuntimeException("session closed");
+                    throw new SessionException(getExceptions());
                 }
                 else
                 {
@@ -322,6 +323,8 @@
 
     private Map<Integer,ResultFuture<?>> results =
         new HashMap<Integer,ResultFuture<?>>();
+    private List<ExecutionException> exceptions =
+        new ArrayList<ExecutionException>();
 
     void result(int command, Struct result)
     {
@@ -333,6 +336,22 @@
         future.set(result);
     }
 
+    void addException(ExecutionException exc)
+    {
+        synchronized (exceptions)
+        {
+            exceptions.add(exc);
+        }
+    }
+
+    List<ExecutionException> getExceptions()
+    {
+        synchronized (exceptions)
+        {
+            return new ArrayList<ExecutionException>(exceptions);
+        }
+    }
+
     protected <T> Future<T> invoke(Method m, Class<T> klass)
     {
         synchronized (commands)
@@ -395,7 +414,7 @@
             }
             else if (closed.get())
             {
-                throw new RuntimeException("session closed");
+                throw new SessionException(getExceptions());
             }
             else
             {

Modified: incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/SessionDelegate.java
URL: http://svn.apache.org/viewvc/incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/SessionDelegate.java?rev=659631&r1=659630&r2=659631&view=diff
==============================================================================
--- incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/SessionDelegate.java (original)
+++ incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/SessionDelegate.java Fri May 23 12:16:04 2008
@@ -59,6 +59,11 @@
         ssn.result(result.getCommandId(), result.getValue());
     }
 
+    @Override public void executionException(Session ssn, ExecutionException exc)
+    {
+        ssn.addException(exc);
+    }
+
     @Override public void sessionCompleted(Session ssn, SessionCompleted cmp)
     {
         RangeSet ranges = cmp.getCommands();

Added: incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/SessionException.java
URL: http://svn.apache.org/viewvc/incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/SessionException.java?rev=659631&view=auto
==============================================================================
--- incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/SessionException.java (added)
+++ incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/SessionException.java Fri May 23 12:16:04 2008
@@ -0,0 +1,46 @@
+/*
+ *
+ * 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.qpidity.transport;
+
+import java.util.List;
+
+/**
+ * SessionException
+ *
+ */
+
+public class SessionException extends RuntimeException
+{
+
+    private List<ExecutionException> exceptions;
+
+    public SessionException(List<ExecutionException> exceptions)
+    {
+        super(exceptions.isEmpty() ? "" : exceptions.toString());
+        this.exceptions = exceptions;
+    }
+
+    public List<ExecutionException> getExceptions()
+    {
+        return exceptions;
+    }
+
+}

Propchange: incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/SessionException.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/codec/AbstractEncoder.java
URL: http://svn.apache.org/viewvc/incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/codec/AbstractEncoder.java?rev=659631&r1=659630&r2=659631&view=diff
==============================================================================
--- incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/codec/AbstractEncoder.java (original)
+++ incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/codec/AbstractEncoder.java Fri May 23 12:16:04 2008
@@ -340,7 +340,7 @@
         }
     }
 
-    private Type resolve(Class klass)
+    static final Type resolve(Class klass)
     {
         Type type = ENCODINGS.get(klass);
         if (type != null)

Added: incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/codec/Validator.java
URL: http://svn.apache.org/viewvc/incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/codec/Validator.java?rev=659631&view=auto
==============================================================================
--- incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/codec/Validator.java (added)
+++ incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/codec/Validator.java Fri May 23 12:16:04 2008
@@ -0,0 +1,177 @@
+/*
+ *
+ * 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.qpidity.transport.codec;
+
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+
+import org.apache.qpidity.transport.RangeSet;
+import org.apache.qpidity.transport.Struct;
+
+
+/**
+ * Validator
+ *
+ */
+
+public class Validator
+{
+
+    public static final void checkBit(boolean b)
+    {
+        // no illegal values
+    }
+
+    public static final void checkUint8(short s)
+    {
+        if (s > 0xFF || s < 0)
+        {
+            throw new IllegalArgumentException("" + s);
+        }
+    }
+
+    public static final void checkUint16(int i)
+    {
+        if (i > 0xFFFF || i < 0)
+        {
+            throw new IllegalArgumentException("" + i);
+        }
+    }
+
+    public static final void checkUint32(long l)
+    {
+        // XXX: we can't currently validate this because we do thinks
+        // like pass in -1 for 0xFFFFFFFF
+        // if (l > 0xFFFFFFFFL || l < 0)
+        // {
+        //     throw new IllegalArgumentException("" + l);
+        // }
+    }
+
+    public static final void checkSequenceNo(int s)
+    {
+        // no illegal values
+    }
+
+    public static final void checkUint64(long l)
+    {
+        // no illegal values
+    }
+
+    public static final void checkDatetime(long l)
+    {
+        // no illegal values
+    }
+
+    public static final void checkUuid(UUID u)
+    {
+        // no illegal values
+    }
+
+    public static final void checkStr8(String value)
+    {
+        if (value != null && value.length() > 255)
+        {
+            throw new IllegalArgumentException("" + value);
+        }
+    }
+
+    public static final void checkStr16(String value)
+    {
+        if (value != null && value.length() > 0xFFFF)
+        {
+            throw new IllegalArgumentException("" + value);
+        }
+    }
+
+    public static final void checkVbin8(byte[] value)
+    {
+        if (value != null && value.length > 255)
+        {
+            throw new IllegalArgumentException("" + value);
+        }
+    }
+
+    public static final void checkVbin16(byte[] value)
+    {
+        if (value != null && value.length > 0xFFFF)
+        {
+            throw new IllegalArgumentException("" + value);
+        }
+    }
+
+    public static final void checkByteRanges(RangeSet r)
+    {
+        // no illegal values
+    }
+
+    public static final void checkSequenceSet(RangeSet r)
+    {
+        // no illegal values
+    }
+
+    public static final void checkVbin32(byte[] value)
+    {
+        // no illegal values
+    }
+
+    public static final void checkStruct32(Struct s)
+    {
+        // no illegal values
+    }
+
+    public static final void checkArray(List<Object> array)
+    {
+        if (array == null)
+        {
+            return;
+        }
+
+        for (Object o : array)
+        {
+            checkObject(o);
+        }
+    }
+
+    public static final void checkMap(Map<String,Object> map)
+    {
+        if (map == null)
+        {
+            return;
+        }
+
+        for (Map.Entry<String,Object> entry : map.entrySet())
+        {
+            checkStr8(entry.getKey());
+            checkObject(entry.getValue());
+        }
+    }
+
+    public static final void checkObject(Object o)
+    {
+        if (o != null && AbstractEncoder.resolve(o.getClass()) == null)
+        {
+            throw new IllegalArgumentException("cannot encode " + o.getClass());
+        }
+    }
+
+}

Propchange: incubator/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpidity/transport/codec/Validator.java
------------------------------------------------------------------------------
    svn:eol-style = native