Hi all,

I refactored MINA API very heavily this weekend, and now it's time to show
the result to you.  Please feel free to browse my branch here:

http://svn.apache.org/viewcvs.cgi/d...ustin/mina-spi/

and here's the working examples:

http://svn.apache.org/viewcvs.cgi/d.../mina/examples/

Here's the list of notable changes:

* SocketAddress is replaced with IoAddress (org.apache.common)

* All bind/unbind/connect operation is performed via
org.apache.commin.IoService.  There's no need to instantiate an acceptor
implementation by yourself.  You never access
org.apache.mina.transport.*packages anymore.

* 'registry' package is removed.  Instead, IoService does the similar job.

* No more transport-type specific property getters and setters.  Now we use
IoAddress properties and the user-defined attributes in IoSession.  For
example:

ex1) IoService.bind(
"nio:socket:*:8080?reuseAddress=true&threadModel=normal&threadPoolSize=20&re
ceiveBufferSize=4096",
new MyHandler() );
ex2) session.setAttribute( "receiveBufferSize", new Integer( 2048 ) );  //
org.apache.mina.common.RuntimeIOException will be thrown if this operation
fails.

* Blacklist filter uses regular expressions to detect blacklisted sessions.

* Spring integration classes changed a lot due to these changes.

* Per-acceptor/connector IoFilterChainBuilder is removed.  Instead we could
implement IoService to support more attributes.  For example, we could do
like this:

IoService.connect( "nio:socket:
192.168.0.1:8080?reconnect=true&reconnectRetry=3&reconnectDelay=10", new
MyIoHandler() );

Please feel free to criticize these changes.  We're very open to your
feedback.  More feedback, better API! 

Thanks in advance,
Trustin
--
what we call human nature is actually human habit
--
http://gleamynode.net/
PGP Key ID: 0x854B996C

[ Post a follow-up to this message ]

2006/1/22, Trustin Lee <trustin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>
>
> * SocketAddress is replaced with IoAddress (org.apache.common )


org.apache.common -> org.apache.mina.common

Trustin
--
what we call human nature is actually human habit
--
http://gleamynode.net/
PGP Key ID: 0x854B996C

[ Post a follow-up to this message ]

[ Post a follow-up to this message ]

peter royal wrote:
...

> I don't like having this as a singleton with static methods. I need
> to be able to have multiple instances in a single JVM, and this
> breaks that. The new setup is a monolithic, which while great for new
> users, makes it harder to tweak the behavior. The old setup was a
> composition of multiple components, and I preferred that API style.

I see where you're coming from I agree.  In my last email I was
considering ease of API adoption for new comers over existing users.

>
> If the new IoService was merely a layer on top of the old API, I'd be
> more in favor of that.
>
+1
 
>
>
> I liked the transport-specific getters and setters, since it provided
> a level of type-safety when setting the properties, and you knew what
> you were setting (there was javadoc on the methods, no chance of mis-
> typing the attribute name, etc).
>
Again +1 here.  Man Peter I totally overlooked these points: thanks for
chiming in.
...

> The changes look like they help simplify usage of MINA in simple
> scenarios.. the examples are certainly much simpler, but for users
> that might have a more complex integration, they seem to be somewhat
> of a step backwards (my .02)

I'm starting to agree.  I'm a bit worried about the singleton with
static methods for using MINA in containers now.  This change makes life
easier for the new comer but becomes more restrictive for power users.

Alex

[ Post a follow-up to this message ]

SGksCgpPbiAxLzIyLzA2LCBUcnVzdGluIExlZSA8
dHJ1c3RpbkBnbWFpbC5jb20+IHdyb3RlOgo+
 IEhpIGFsbCwKPgo+IEkgcmVmYWN0b3JlZCBNSU5B
IEFQSSB2ZXJ5IGhlYXZpbHkgdGhpcyB3ZWVr
 ZW5kLCBhbmQgbm93IGl0J3MgdGltZSB0byBzaG93
Cj4gdGhlIHJlc3VsdCB0byB5b3UuICBQbGVh
 c2UgZmVlbCBmcmVlIHRvIGJyb3dzZSBteSBicmFu
Y2ggaGVyZToKPiAqIE5vIG1vcmUgdHJhbnNw
 b3J0LXR5cGUgc3BlY2lmaWMgcHJvcGVydHkgZ2V0
dGVycyBhbmQgc2V0dGVycy4gIE5vdyB3ZSB1
 c2UKPiBJb0FkZHJlc3MgcHJvcGVydGllcyBhbmQg
dGhlIHVzZXItZGVmaW5lZCBhdHRyaWJ1dGVz
 IGluIElvU2Vzc2lvbi4gIEZvcgo+IGV4YW1wbGU6
Cj4KPiBleDEpIElvU2VydmljZS5iaW5kKAo+
 ICJuaW86c29ja2V0Oio6ODA4MD9yZXVzZUFkZHJl
c3M9dHJ1ZSZ0aHJlYWRNb2RlbD1ub3JtYWwm
 dGhyZWFkUG9vbFNpemU9MjAmcmVjZWl2ZUJ1ZmZl
clNpemU9NDA5NiIsCj4gbmV3IE15SGFuZGxl
 cigpICk7Cj4gZXgyKSBzZXNzaW9uLnNldEF0dHJp
YnV0ZSggInJlY2VpdmVCdWZmZXJTaXplIiwg
 bmV3IEludGVnZXIoIDIwNDggKSApOyAgLy8KPiBv
cmcuYXBhY2hlLm1pbmEuY29tbW9uLlJ1bnRp
 bWVJT0V4Y2VwdGlvbiB3aWxsIGJlIHRocm93biBp
Zgo+IHRoaXMgb3BlcmF0aW9uIGZhaWxzLgoK
 SSByZWFsbHkgZmF2b3IgdHlwZXNhZmUgb3BlcmF0
aW9ucyB0byBzdHJpbmcgYmFzZWQgb25lcy4g
 SG93ZXZlciBhcyBJCnNlZSB0aGUgbmV3IEFQSSB1
c2VzIG1vcmUgc3RyaW5nIGJhc2VkIGNvbmZp
 Z3VyYXRpb24uCgo+IFBsZWFzZSBmZWVsIGZyZWUg
dG8gY3JpdGljaXplIHRoZXNlIGNoYW5nZXMu
 ICBXZSdyZSB2ZXJ5IG9wZW4gdG8geW91cgo+IGZl
ZWRiYWNrLiAgTW9yZSBmZWVkYmFjaywgYmV0
dGVyIEFQSSEgOikKCi0tCkVyc2luCg==

[ Post a follow-up to this message ]

Trustin Lee wrote:

> Hi all,
>
> I refactored MINA API very heavily this weekend, and now it's time to
> show the result to you.  Please feel free to browse my branch here:
>
> http://svn.apache.org/viewcvs.cgi/d...ustin/mina-spi/
>
> and here's the working examples:
>
> http://svn.apache.org/viewcvs.cgi/d.../mina/examples/
>
> Here's the list of notable changes:
>
> * SocketAddress is replaced with IoAddress (org.apache.common )

I see you probably meant o.a.mina.common.

>
> * All bind/unbind/connect operation is performed via
> org.apache.commin.IoService.  There's no need to instantiate an
> acceptor  implementation by yourself.  You never access
> org.apache.mina.transport.* packages anymore.

Nice I like this.

>
> * 'registry' package is removed.  Instead, IoService does the similar job.
>
Hmmm I liked the registry concept.  It made it clear that you could
register multiple services with MINA.  But if there is now a better
mechanism then I'm all for it.  Looks like this bind description string
does the job below referring to ex1.

> * No more transport-type specific property getters and setters.  Now
> we use IoAddress properties and the user-defined attributes in
> IoSession.  For example:
>
> ex1) IoService.bind(
> "nio:socket:*:8080?reuseAddress=true&threadModel=normal&threadPoolSize=20&
receiveBufferSize=4096",
> new MyHandler() );

This is cool.  Nice step in MINA evolution.  It's like a connector string.

> ex2) session.setAttribute( "receiveBufferSize", new Integer( 2048 )
> );  // org.apache.mina.common.RuntimeIOException will be thrown if
> this operation fails.
>
Hmmm why not throw an IllegalArgumentEx ... just curious why this is a
RtIOEx above.  Could you elaborate?

> * Blacklist filter uses regular expressions to detect blacklisted
> sessions.

That's a nice feature and it makes sense.

> * Spring integration classes changed a lot due to these changes.
>
What do you think the impact will be to all the protocol providers?

> * Per-acceptor/connector IoFilterChainBuilder is removed.  Instead we
> could implement IoService to support more attributes.  For example, we
> could do like this:
>
> IoService.connect( "nio:socket:
> 192.168.0.1:8080?reconnect=true&reconnectRetry=3&reconnectDelay=10
> <http://192.168.0.1:8080?reconnect=t...connectDelay=10
>",
> new MyIoHandler() );
>
Sorry I did not understand how this example explains the above.  Can you
elaborate on it?

> Please feel free to criticize these changes.  We're very open to your
> feedback.  More feedback, better API! 

Sounds like these changes further reduce the number of lines needed to
bind a MINA service.  Before we had the following for the echo example:

Service service = *new* Service( *"echo"*, TransportType.SOCKET, PORT );
registry.bind( service, *new* EchoProtocolHandler() );

And now we have just this one line:

IoService.bind( *"nio:socket:*:"* + PORT, *new* EchoProtocolHandler(), *new*
 DefaultIoFilterChainBuilder() );


I'm seeing the improvement.  I also think managing the filter chain has been
 simplified: it's
much cleaner now:

Before we had this for adding a logger to the chain:


*private* *static* *void* addLogger( ServiceRegistry registry ) *throws* Exc
eption
{
IoAcceptor acceptor = registry.getAcceptor( TransportType.SOCKET );
acceptor.getFilterChain().addLast( *"logger"*, *new* LoggingFilter() );
System.out.println( *"Logging ON"* );
}

Now we have this:

*private* *static* *void* addLogger( DefaultIoFilterChainBuilder chain ) *th
rows* Exception
{
chain.addLast( *"logger"*, *new* LoggingFilter() );
System.out.println( *"Logging ON"* );
}

It's more concise and clearer without exposing internal details.  A big fat 
+1 for this.

Overall I think this is a nice jump for the API.  I hope it simplifies the i
nternals as well.

Excuse me but I have to ask this  do we now have this following issue sort
ed out?

http://issues.apache.org/jira/browse/DIRMINA-166

Really great job T!  MINA keeps getting better.

Alex

P.S. I really recommend those evaluating this change look at the examples si
nce they
really show the impact of these changes best.  They clarified some points in
 this
email.

[ Post a follow-up to this message ]

Hi Peter,

2006/1/23, peter royal <proyal-1oDqGaOF3Lkdnm+yROfE0A@public.gmane.org>:
>
> On Jan 22, 2006, at 9:56 AM, Trustin Lee wrote: 
>
> What was wrong with using a SocketAddress?


Of course, we could create this address scheme
(<providerType>:<transportType>:<address> ) by extending SocketAddress, but
there was no point to do so because it only causes downcasting.  As you
know, you can't do anything with SocketAddress.  You have to downcast it to
something such as InetSocketAddress or VmPipeAddress.

> * All bind/unbind/connect operation is performed via 
>
> I don't like having this as a singleton with static methods. I need
> to be able to have multiple instances in a single JVM, and this
> breaks that. The new setup is a monolithic, which while great for new
> users, makes it harder to tweak the behavior. The old setup was a
> composition of multiple components, and I preferred that API style.
>
> If the new IoService was merely a layer on top of the old API, I'd be
> more in favor of that.


Actually, IoService does what you think except that it's a singleton.  It is
like a ServiceRegistry.  It creates acceptors and connectors for you and
encapsulated them.  Except that the number of acceptor/connector thread is
one, everything is identical with the previous API.  Yes, this can be a
problem, but we can make it non-singleton, of course.  If we change so,
there's no effective difference between the old and the new from the
viewpoint of the object composition.  It is just done by the IoService, not
by you.

> * No more transport-type specific property getters and setters. 
>
> I liked the transport-specific getters and setters, since it provided
> a level of type-safety when setting the properties, and you knew what
> you were setting (there was javadoc on the methods, no chance of mis-
> typing the attribute name, etc).


Right.  Now they are gone.  This approach has a trade-off you mentioned.
The advantage of user-defined attributes is that you don't need to downcast
and that it provides more flexibility.  For example, let's assume that you
need to switch to new AIO transport type implementation.  All you need to do
is change the providerType to 'aio' from 'nio' in IoAddress.  With the old
API, you'll have to add more 'if ( session instanceof ... ) { ...
setReceiveBufferSize(...); }' sentences as you change the provider.  But
with string key, it works only if the providers used the same keys.

Of course, this problem could be resolved if the authors of AIO provider and
NIO provider agreed on creating a common interface, but I think it's too
ideal.

Also, there are now 'magic' attribute name that affect the
> operation of a session that can no longer be used as attribute names
> in user code.


Yes, right.  If so, we could put a prefix such as 'provider.', but I'm not
sure we need to do this in a unstable branch.  Otherwise we could provide
separate attribute getters and setters, but I don't know it's advantage to
users.

The changes look like they help simplify usage of MINA in simple
> scenarios.. the examples are certainly much simpler, but for users
> that might have a more complex integration, they seem to be somewhat
> of a step backwards (my .02)


Please explain your complex integration scenario.  We'll make sure it works
with the new API.  

Thanks,
Trustin
--
what we call human nature is actually human habit
--
http://gleamynode.net/
PGP Key ID: 0x854B996C

[ Post a follow-up to this message ]

2006/1/23, Trustin Lee <trustin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
 
>
>
> Right.  Now they are gone.  This approach has a trade-off you mentioned.
> The advantage of user-defined attributes is that you don't need to downcas
t
> and that it provides more flexibility.  For example, let's assume that you
> need to switch to new AIO transport type implementation.  All you need tod
o
> is change the providerType to 'aio' from 'nio' in IoAddress.  With the old
> API, you'll have to add more 'if ( session instanceof ... ) { ...
> setReceiveBufferSize(...); }' sentences as you change the provider.  But
> with string key, it works only if the providers used the same keys.
>
> Of course, this problem could be resolved if the authors of AIO provider
> and NIO provider agreed on creating a common interface, but I think it's t
oo
> ideal.
>

I forgot to mention that we could provide a standardized set of attributes
as a documentation.

Thanks,
Trustin
--
what we call human nature is actually human habit
--
http://gleamynode.net/
PGP Key ID: 0x854B996C

[ Post a follow-up to this message ]

2006/1/23, Ersin Er <ersin.er-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>
> I really favor typesafe operations to string based ones. However as I
> see the new API uses more string based configuration.


We could change IoService throw an exception if any unknown attributes are
specified to make sure the safety.  For the property values, it checks the
value is correct in runtime, so it should be fine if you run the application
at least once in most cases or run the test case.

Trustin
--
what we call human nature is actually human habit
--
http://gleamynode.net/
PGP Key ID: 0x854B996C

[ Post a follow-up to this message ]

2006/1/23, Alex Karasulu <aok123-Bdlq13kUjeyLZ21kGMrzwg@public.gmane.org>:
> 
> Hmmm why not throw an IllegalArgumentEx ... just curious why this is a
> RtIOEx above.  Could you elaborate?


For example, Socket.getReceiveBufferSize() can throw a SocketException.  We
cannot throw an IllegalArgumentException because we didn't specify any
arguments.  That's why I created a RuntimeIOException.  Now we have
RuntimeIOException, so we also don't need to throw an
IllegalArgumentException from setAttribute() when a SocketException is
thrown.  Of course, the RuntimeExceptions are just passed through to the
caller, not being wrapped by RuntimeIOException.

> * Spring integration classes changed a lot due to these changes.
>
> What do you think the impact will be to all the protocol providers?


No impact at all.

> * Per-acceptor/connector IoFilterChainBuilder is removed.  Instead we 
> http://192.168.0.1:8080?reconnect=t...connectDelay=10 
>
> Sorry I did not understand how this example explains the above.  Can you
> elaborate on it?


Most filters that are added to the session manager level are very generic
ones such as thread pool filter, logging filter, and etc.  So we can provide
them as a parameter of the address.

Thanks,
Trustin
--
what we call human nature is actually human habit
--
http://gleamynode.net/
PGP Key ID: 0x854B996C

[ Post a follow-up to this message ]