General code cleanup and also fixed a IndexOutOfBoundsException (JM-271).

git-svn-id: http://svn.igniterealtime.org/svn/repos/messenger/trunk@1295 b35dd754-fafc-0310-a699-88a17e54d16e

src/java/org/jivesoftware/messenger/interceptor/InterceptorManager.java

@@ -3,7 +3,7 @@
  * $Revision$
  * $Date$
  *
- * Copyright (C) 2004 Jive Software. All rights reserved.
+ * Copyright (C) 2005 Jive Software. All rights reserved.
  *
  * This software is published under the terms of the GNU Public License (GPL),
  * a copy of which is included in this distribution.
@@ -20,15 +20,16 @@ import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.CopyOnWriteArrayList;
 
 /**
- * An InterceptorManager holds the list of global interceptors and interceptors per-user that will
- * be invoked before and after a packet was read in SocketReadThead and also when the packet is
- * about to be sent in SocketConnection. If the interceptor is related to a user then it will
- * get all the packets sent or received for <b>any</b> connection of the user.<p>
+ * An InterceptorManager manages the list of global interceptors and per-user
+ * interceptors that are invoked before and after packets are read and sent.
+ * If an interceptor is installed for a user then it will receive all packets
+ * sent or received for <b>any</b> connection of that user.<p>
  *
- * PacketInterceptors that are invoked before the packet is sent or processed (when read) may
- * change the original packet or may even reject the packet by throwing an exception. If the
- * interceptor rejects a received packet then the sender of the packet will get a not_allowed
- * answer.<p>
+ * PacketInterceptors that are invoked before the packet is sent or processed
+ * (when read) may change the original packet or reject the packet by throwing
+ * a {@link PacketRejectedException}. If the interceptor rejects a received packet
+ * then the sender of the packet receive a
+ * {@link org.xmpp.packet.PacketError.Condition#not_allowed not_allowed} error.
  *
  * @see PacketInterceptor
  * @author Gaston Dombiak
@@ -37,21 +38,28 @@ public class InterceptorManager {
 
     private static InterceptorManager instance = new InterceptorManager();
 
-    private List<PacketInterceptor> globalInterceptors = new CopyOnWriteArrayList<PacketInterceptor>();
-    private Map<String, List<PacketInterceptor>> usersInterceptors = new ConcurrentHashMap<String, List<PacketInterceptor>>();
+    private List<PacketInterceptor> globalInterceptors =
+            new CopyOnWriteArrayList<PacketInterceptor>();
+    private Map<String, List<PacketInterceptor>> usersInterceptors =
+            new ConcurrentHashMap<String, List<PacketInterceptor>>();
 
+    /**
+     * Returns a singleton instance of InterceptorManager.
+     *
+     * @return an instance of InterceptorManager.
+     */
     public static InterceptorManager getInstance() {
         return instance;
     }
 
     /**
-     * Returns the list of global packet interceptors. These are the interceptors that will be
-     * used for all the read and sent packets.
+     * Returns an unmodifiable list of global packet interceptors. Global
+     * interceptors are applied to all packets read and sent by the server.
      *
-     * @return the list of global packet interceptors.
+     * @return an unmodifiable list of the global packet interceptors.
      */
-    public Collection<PacketInterceptor> getInterceptors() {
-        return Collections.unmodifiableCollection(globalInterceptors);
+    public List<PacketInterceptor> getInterceptors() {
+        return Collections.unmodifiableList(globalInterceptors);
     }
 
     /**
@@ -61,7 +69,14 @@ public class InterceptorManager {
      * @param interceptor the interceptor to add.
      */
     public void addInterceptor(PacketInterceptor interceptor) {
-        addInterceptor(globalInterceptors.size(), interceptor);
+        if (interceptor == null) {
+            throw new NullPointerException("Parameter interceptor was null.");
+        }
+        // Remove the interceptor from the list since the position might have changed
+        if (globalInterceptors.contains(interceptor)) {
+            globalInterceptors.remove(interceptor);
+        }
+        globalInterceptors.add(interceptor);
     }
 
     /**
@@ -72,10 +87,21 @@ public class InterceptorManager {
      * @param interceptor the interceptor to add.
      */
     public void addInterceptor(int index, PacketInterceptor interceptor) {
+        if (index < 0 || (index > globalInterceptors.size())) {
+            throw new IndexOutOfBoundsException("Index " + index + " invalid.");
+        }
+        if (interceptor == null) {
+            throw new NullPointerException("Parameter interceptor was null.");
+        }
         // Remove the interceptor from the list since the position might have changed
         if (globalInterceptors.contains(interceptor)) {
+            int oldIndex = globalInterceptors.indexOf(interceptor);
+            if (oldIndex < index) {
+                index -= 1;
+            }
             globalInterceptors.remove(interceptor);
         }
+
         globalInterceptors.add(index, interceptor);
     }
 
@@ -90,20 +116,20 @@ public class InterceptorManager {
     }
 
     /**
-     * Returns the list of packet interceptors that are related to the specified username. These
-     * are the interceptors that will be used only when a packet was sent or received by the
+     * Returns an unmodifable list of packet interceptors that are related to the
      * specified username.
      *
      * @param username the name of the user.
-     * @return the list of packet interceptors that are related to the specified username.
+     * @return an unmodifiable list of packet interceptors that are related to
+     *      the specified username.
      */
-    public Collection<PacketInterceptor> getUserInterceptors(String username) {
+    public List<PacketInterceptor> getUserInterceptors(String username) {
         List<PacketInterceptor> userInterceptors = usersInterceptors.get(username);
         if (userInterceptors == null) {
-            return new ArrayList<PacketInterceptor>();
+            return Collections.emptyList();
         }
         else {
-            return Collections.unmodifiableCollection(userInterceptors);
+            return Collections.unmodifiableList(userInterceptors);
         }
     }
 
@@ -123,8 +149,19 @@ public class InterceptorManager {
             usersInterceptors.put(username, userInterceptors);
         }
         else {
+            if (index < 0 || (index > userInterceptors.size())) {
+                throw new IndexOutOfBoundsException("Index " + index + " invalid.");
+            }
+            if (interceptor == null) {
+                throw new NullPointerException("Parameter interceptor was null.");
+            }
+
             // Remove the interceptor from the list since the position might have changed
             if (userInterceptors.contains(interceptor)) {
+                int oldIndex = userInterceptors.indexOf(interceptor);
+                if (oldIndex < index) {
+                    index -= 1;
+                }
                 userInterceptors.remove(interceptor);
             }
         }
@@ -153,32 +190,43 @@ public class InterceptorManager {
     }
 
     /**
-     * Invokes all currently-installed interceptors on the specified packet. All global
-     * interceptors will be invoked as well as interceptors that are related to the address of the
-     * session that received or is sending the packet.<p>
+     * Invokes all currently-installed interceptors on the specified packet.
+     * All global interceptors will be invoked as well as interceptors that
+     * are related to the address of the session that received or is sending
+     * the packet.<p>
      *
-     * Interceptors may be executed before processing a read packet or sending a packet to a user.
-     * This means that interceptors are able to alter the read or packet to send. If possible
-     * interceptors should perform their work in a short time so the overall performance is not
+     * Interceptors are executed before and after processing an incoming packet
+     * and sending a packet to a user. This means that interceptors are able to alter or
+     * reject packets before they are processed further. If possible, interceptors
+     * should perform their work in a short time so that overall performance is not
      * compromised.
      *
-     * @param packet    the packet that has been read or sent.
-     * @param session   the session that received the packet or the packet was sent to.
-     * @param read      flag that indicates if the packet was read or sent.
-     * @param processed flag that indicates if the action (read/send) was performed. (PRE vs. POST).
+     * @param packet the packet that has been read or is about to be sent.
+     * @param session the session that received the packet or that the packet
+     *      will be sent to.
+     * @param read true indicates that the packet was read. When false, the packet
+     *      is being sent to a user.
+     * @param processed true if the packet has already processed (incoming or outgoing).
+     *      If the packet hasn't already been processed, this flag will be false.
      * @throws PacketRejectedException if the packet should be prevented from being processed.
      */
     public void invokeInterceptors(Packet packet, Session session, boolean read, boolean processed)
-            throws PacketRejectedException {
+            throws PacketRejectedException
+    {
         // Invoke the global interceptors for this packet
         for (PacketInterceptor interceptor : globalInterceptors) {
             try {
                 interceptor.interceptPacket(packet, session, read, processed);
             }
             catch (PacketRejectedException e) {
+                if (processed) {
+                    Log.error("Post interceptor cannot reject packet.", e);
+                }
+                else {
                     // Throw this exception since we don't really want to catch it
                     throw e;
                 }
+            }
             catch (Exception e) {
                 Log.error("Error in interceptor", e);
             }
@@ -193,9 +241,14 @@ public class InterceptorManager {
                         interceptor.interceptPacket(packet, session, read, processed);
                     }
                     catch (PacketRejectedException e) {
+                        if (processed) {
+                            Log.error("Post interceptor cannot reject packet.", e);
+                        }
+                        else {
                             // Throw this exception since we don't really want to catch it
                             throw e;
                         }
+                    }
                     catch (Exception e) {
                         Log.error("Error in interceptor", e);
                     }