[Proxool-developer] CodeReview - building connections

[Bertrand R. <ber...@mo...>]

ProxyFactory.buildProxyConnection()

Lets consider the following code from ProxyFactory (after changes we =
have
been discussing today):

    protected static ProxyConnectionIF buildProxyConnection( long id,
ConnectionPool connectionPool, int status) throws SQLException {
        final String url =3D connectionPool.getDefinition().getUrl();

        // Build the *real* connection
        Properties info =3D
connectionPool.getDefinition().getDelegateProperties();
        Connection realConnection =3D DriverManager.getConnection(url, =
info);


        // Getting null would kill us!
        if (realConnection =3D=3D null) {
            throw new SQLException("Unable to create new connection - =
got
NULL from DriverManager! - using url "+url);
        }

        // Build a proxyConnection around it - release the *real* =
connection
if it fails
        ProxyConnection proxyConnection =3D null;
        try {
            proxyConnection =3D new ProxyConnection(realConnection, id, =
url,
connectionPool, status);
        }
        catch(RuntimeException re) {
            try {
                // Ooops - unable to build the proxy - close the
realConnection
                if(realConnection!=3Dnull) realConnection.close();
            }
            finally {
                throw re;
            }
        }
      =20
        Object delegate =3D Proxy.newProxyInstance(
                realConnection.getClass().getClassLoader(),
                realConnection.getClass().getInterfaces(),
                proxyConnection);

        return (ProxyConnection) Proxy.getInvocationHandler(delegate);
    }


My question is why do we have to return a Proxy around the =
ProxyConnectionIF
we just build ? And what extra value does it had besides extending the
ProxyConnectionIF instance so it implements the java.sql.Connection
interface...
=20
This method is called from Prototype.buildConnection() that expects =
nothing
more than a ProxyConnectionIF to be returned. Moreover, when it needs a
Connection, it does an extra call to
ProxyFactory.getConnection(ProxyConnectionIF) which adds an extra Proxy
around it :-o
=20
If I understand clearly, the ProxyConnectionIF is there to store
meta-information about a *real* connection - nothing more.
The ProxyConnection class implements this interface + the =
InvocationHandler
so it can be used as a target for a Proxy implementing the Connection
interface. This way the meta-info available through the =
ProxyConnectionIF is
made up-to-date.
=20
This latest proxy is build when a connection must be returned to the =
user
from the pool (using the ProxyFactory.getConnection(ProxyConnectionIF)
method).
=20
In short, internally Proxool manipulates only ProxyConnectionIF =
instances.
=20
=20
According to me, the latest above statements in bold, could be removed -
this would remove one level of indirection.
The end of the code would be as follows:
=20
        // Build a proxyConnection around it - release the *real* =
connection
if it fails
        ProxyConnection proxyConnection =3D null;
        try {
            return new ProxyConnection(realConnection, id, url,
connectionPool, status);
        }
        catch(RuntimeException re) {
            try {
                // Ooops - unable to build the proxy - close the
realConnection
                if(realConnection!=3Dnull) realConnection.close();
            }
            finally {
                throw re;
            }
        }
     }
=20
=20
Prototyper.buildConnection()
=20
This method calls the ProxyFactory when it has to build a new =
connection.
Somewhere in the code, you can see the following:
=20
            proxyConnection =3D ProxyFactory.buildProxyConnection(id,
connectionPool, status);
            connection =3D ProxyFactory.getConnection(proxyConnection);
=20
            try {
                connectionPool.onBirth(connection);=20
            }=20
            catch (Exception e) {
                log.error("Problem during onBirth (ignored)", e);
=20
=20
It gets a new ProxyConnection then request access to a =
java.sql.Connection
through it...
It does so because of the event notification mechanism.
=20
Since the event can be exported to the outside of Proxool (users can
register event listeners) - it is important note to export the *real*
connection in the event.
Therefore, writing something like the following is not acceptable:
=20
            proxyConnection =3D ProxyFactory.buildProxyConnection(id,
connectionPool, status);
=20
            try {
                connectionPool.onBirth(proxyConnection.getConnection()); =

            }=20
            catch (Exception e) {
                log.error("Problem during onBirth (ignored)", e);
=20
Note that in the current implementation, if we don't make the =
modifications
described in point 1 above, you will have created TWO proxies (the two =
first
lines above)...
=20
Unfortunatelly, this is not consistent with the removeProxyConnection()
implementation:
=20
            try {
                onDeath(proxyConnection.getConnection());
            }=20
            catch (SQLException e) {
                log.error("Problem during onDeath (ignored)", e);
            }
=20
=20
This event notification worries me a bit... In the life of a connection, =
at
least two eventswill be thrown: onBirth and onDeath. Unfortunatelly, the
connection *instance* passed as argument will be different although it
concerns the same *real* connection. See what I mean ?
=20
To make it short:
- we can't pass the *real* connection to the listeners (for safety - =
they
cannot have access to close() or whatever... this would confuse the =
pool);
- we should always pass the same instance...
=20
Conclusion: should we pass a Proxy or tell the user not to touch the
connection in the listeners ?
=20
=20
=20
(hope this is clear - quite hard to explain all these stuff)
=20