Trustin Lee wrote:

> Hi Niklas,
>
> 2005/11/8, Niklas Therning <[EMAIL PROTECTED]
> <mailto:[EMAIL PROTECTED]>>:
>
>     For bind one idea is to have a Binding class which simply maps a
>     SocketAddress to an IoHandler. setBindings would take an array of
>     Binding objects:
>
>     public void setBindings(Binding[] bindings)
>
>     I don't think it would be necessary to provide something similar for
>     connect since connect is something you do programmatically at runtime.
>     Please correct me if I'm wrong.
>
>
> This additional method will make MINA more DI friendly definitely, but
> do we really need this just for DI?  Will this be useful also when we
> call this method from our code?  DI makes the configuration easier but
> in this case, it doesn't help any API design IMHO.  WDYT?

I think you are right about that. It is a lot more natural to call
bind() on an IoAcceptor then having to create an array or list of
Binding objects and set it using setBindings(). I agree that it's mostly
useless if you aren't configuring things using DI. In the Spring case
I'm perfectly fine with configuring IoAcceptors and IoConnectors using
Spring FactoryBeans. Of course that doesn't help users of other
DI-containers but perhaps those containers should be targeted seperately
just like we do now with Spring?


>
>     > For BlacklistFilter, I thought your Spring integration patch already
>     > contains it.  If it's not ready, I'll check in the fix.  Please
>     let me
>     > know.
>     >
>     The patch for BlacklistFilter isn't included in the Spring integration
>     patch I just uploaded to JIRA. The blacklist setter looks like this:
>
>     +    public void setBlacklist( InetAddress[] addresses )
>     +    {
>     +        blacklist.clear();
>     +        for( int i = 0; i < addresses.length; i++ )
>     +        {
>     +            blacklist.add( addresses[i] );
>     +        }
>     +    }
>
>     Please add it if you think it looks ok. 
>
>
> I think it's good, but we'll also need another setter which accepts a
> Collection of InetAddresses.  We're not using Java 5, so we'll have to
> check the type of all elements.

Ok, no problem. I'm attaching a new patch. I've also added checks to
block() and unblock() to prevent null addresses.

/Niklas

Index: src/java/org/apache/mina/filter/BlacklistFilter.java
===================================================================
--- src/java/org/apache/mina/filter/BlacklistFilter.java	(revision 332738)
+++ src/java/org/apache/mina/filter/BlacklistFilter.java	(working copy)
@@ -21,6 +21,7 @@
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
+import java.util.Collection;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -42,10 +43,51 @@
     private final Set blacklist = new HashSet();
 
     /**
+     * Sets the addresses to be blacklisted. NOTE: this call will remove any
+     * previously blacklisted addresses.
+     * 
+     * @param addresses an array of addresses to be blacklisted.
+     */
+    public void setBlacklist( InetAddress[] addresses )
+    {
+        if( addresses == null )
+            throw new NullPointerException( "addresses" );
+        blacklist.clear();
+        for( int i = 0; i < addresses.length; i++ )
+        {
+            blacklist.add( addresses[i] );
+        }
+    }
+    
+    /**
+     * Sets the addresses to be blacklisted. NOTE: this call will remove any
+     * previously blacklisted addresses.
+     * 
+     * @param addresses a collection of InetAddress objects representing the 
+     *        addresses to be blacklisted.
+     * @throws IllegalArgumentException if the specified collections contains 
+     *         non-InetAddress objects.
+     */
+    public void setBlacklist( Collection addresses )
+    {
+        if( addresses == null )
+            throw new NullPointerException( "addresses" );
+        InetAddress[] inetAddresses = new InetAddress[addresses.size()];
+        try {
+            setBlacklist( (InetAddress[]) addresses.toArray(inetAddresses) );
+        } catch (ArrayStoreException ase) {
+            throw new IllegalArgumentException( "Collection of addresses "
+                    + "must contain only InetAddress instances", ase );
+        }
+    }
+    
+    /**
      * Blocks the specified endpoint.
      */
     public synchronized void block( InetAddress address )
     {
+        if( address == null )
+            throw new NullPointerException( "address" );
         blacklist.add( address );
     }
 
@@ -54,6 +96,8 @@
      */
     public synchronized void unblock( InetAddress address )
     {
+        if( address == null )
+            throw new NullPointerException( "address" );
         blacklist.remove( address );
     }
     

Reply via email to