Refactored NPN code for the soon-to-arrive ALPN code.

Moved NPN tests to new module spdy-npn-tests, that now is the only module
requiring the -Xbootclasspath configuration for the Maven Surefire Plugin.

5 files changed, 216 insertions, 109 deletions

diff --git a/jetty-spdy/spdy-client/src/main/java/org/eclipse/jetty/spdy/client/NPNClientConnection.java b/jetty-spdy/spdy-client/src/main/java/org/eclipse/jetty/spdy/client/NPNClientConnection.java
index fc288894d6..c8e85f2c0e 100644
--- a/jetty-spdy/spdy-client/src/main/java/org/eclipse/jetty/spdy/client/NPNClientConnection.java
+++ b/jetty-spdy/spdy-client/src/main/java/org/eclipse/jetty/spdy/client/NPNClientConnection.java
@@ -18,87 +18,28 @@
 
 package org.eclipse.jetty.spdy.client;
 
-import java.io.IOException;
 import java.util.List;
 import java.util.Map;
-
+import java.util.concurrent.Executor;
 import javax.net.ssl.SSLEngine;
 
-import org.eclipse.jetty.io.AbstractConnection;
 import org.eclipse.jetty.io.ClientConnectionFactory;
-import org.eclipse.jetty.io.Connection;
 import org.eclipse.jetty.io.EndPoint;
-import org.eclipse.jetty.io.RuntimeIOException;
 import org.eclipse.jetty.npn.NextProtoNego;
-import org.eclipse.jetty.util.BufferUtil;
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.Logger;
 
-public class NPNClientConnection extends AbstractConnection implements NextProtoNego.ClientProvider
+public class NPNClientConnection extends NegotiatingClientConnection implements NextProtoNego.ClientProvider
 {
-    private final Logger LOG = Log.getLogger(getClass());
-    private final SPDYClient client;
-    private final ClientConnectionFactory connectionFactory;
-    private final SSLEngine engine;
-    private final Map<String, Object> context;
-    private volatile boolean completed;
+    private static final Logger LOG = Log.getLogger(NPNClientConnection.class);
 
-    public NPNClientConnection(EndPoint endPoint, SPDYClient client, ClientConnectionFactory connectionFactory, SSLEngine sslEngine, Map<String, Object> context)
-    {
-        super(endPoint, client.getFactory().getExecutor());
-        this.client = client;
-        this.connectionFactory = connectionFactory;
-        this.engine = sslEngine;
-        this.context = context;
-        NextProtoNego.put(engine, this);
-    }
+    private final String protocol;
 
-    @Override
-    public void onOpen()
+    public NPNClientConnection(EndPoint endPoint, Executor executor, ClientConnectionFactory connectionFactory, SSLEngine sslEngine, Map<String, Object> context, String protocol)
     {
-        super.onOpen();
-        try
-        {
-            getEndPoint().flush(BufferUtil.EMPTY_BUFFER);
-            if (completed)
-                replaceConnection();
-            else
-                fillInterested();
-        }
-        catch(IOException e)
-        {
-            throw new RuntimeIOException(e);
-        }
-    }
-
-    @Override
-    public void onFillable()
-    {
-        while (true)
-        {
-            int filled = fill();
-            if (filled == 0 && !completed)
-                fillInterested();
-            if (filled <= 0 || completed)
-                break;
-        }
-        if (completed)
-            replaceConnection();
-    }
-
-    private int fill()
-    {
-        try
-        {
-            return getEndPoint().fill(BufferUtil.EMPTY_BUFFER);
-        }
-        catch (IOException x)
-        {
-            LOG.debug(x);
-            NextProtoNego.remove(engine);
-            close();
-            return -1;
-        }
+        super(endPoint, executor, sslEngine, connectionFactory, context);
+        this.protocol = protocol;
+        NextProtoNego.put(sslEngine, this);
     }
 
     @Override
@@ -110,31 +51,31 @@ public class NPNClientConnection extends AbstractConnection implements NextProto
     @Override
     public void unsupported()
     {
-        NextProtoNego.remove(engine);
-        completed = true;
+        NextProtoNego.remove(getSSLEngine());
+        completed();
     }
 
     @Override
     public String selectProtocol(List<String> protocols)
     {
-        NextProtoNego.remove(engine);
-        completed = true;
-        return client.selectProtocol(protocols);
-    }
-
-    private void replaceConnection()
-    {
-        EndPoint endPoint = getEndPoint();
-        try
+        if (protocols.contains(protocol))
         {
-            Connection oldConnection = endPoint.getConnection();
-            Connection newConnection = connectionFactory.newConnection(endPoint, context);
-            ClientConnectionFactory.Helper.replaceConnection(oldConnection, newConnection);
+            NextProtoNego.remove(getSSLEngine());
+            completed();
+            return protocol;
         }
-        catch (Throwable x)
+        else
         {
-            LOG.debug(x);
+            LOG.info("Could not negotiate protocol: server {} - client {}", protocols, protocol);
             close();
+            return null;
         }
     }
+
+    @Override
+    public void close()
+    {
+        NextProtoNego.remove(getSSLEngine());
+        super.close();
+    }
 }
diff --git a/jetty-spdy/spdy-client/src/main/java/org/eclipse/jetty/spdy/client/NPNClientConnectionFactory.java b/jetty-spdy/spdy-client/src/main/java/org/eclipse/jetty/spdy/client/NPNClientConnectionFactory.java
index ec81cab156..d0458010c9 100644
--- a/jetty-spdy/spdy-client/src/main/java/org/eclipse/jetty/spdy/client/NPNClientConnectionFactory.java
+++ b/jetty-spdy/spdy-client/src/main/java/org/eclipse/jetty/spdy/client/NPNClientConnectionFactory.java
@@ -20,7 +20,7 @@ package org.eclipse.jetty.spdy.client;
 
 import java.io.IOException;
 import java.util.Map;
-
+import java.util.concurrent.Executor;
 import javax.net.ssl.SSLEngine;
 
 import org.eclipse.jetty.io.ClientConnectionFactory;
@@ -28,21 +28,22 @@ import org.eclipse.jetty.io.Connection;
 import org.eclipse.jetty.io.EndPoint;
 import org.eclipse.jetty.io.ssl.SslClientConnectionFactory;
 
-public class NPNClientConnectionFactory implements ClientConnectionFactory
+public class NPNClientConnectionFactory extends NegotiatingClientConnectionFactory
 {
-    private final SPDYClient client;
-    private final ClientConnectionFactory connectionFactory;
+    private final Executor executor;
+    private final String protocol;
 
-    public NPNClientConnectionFactory(SPDYClient client, ClientConnectionFactory connectionFactory)
+    public NPNClientConnectionFactory(Executor executor, ClientConnectionFactory connectionFactory, String protocol)
     {
-        this.client = client;
-        this.connectionFactory = connectionFactory;
+        super(connectionFactory);
+        this.executor = executor;
+        this.protocol = protocol;
     }
 
     @Override
     public Connection newConnection(EndPoint endPoint, Map<String, Object> context) throws IOException
     {
-        return new NPNClientConnection(endPoint, client, connectionFactory,
-                (SSLEngine)context.get(SslClientConnectionFactory.SSL_ENGINE_CONTEXT_KEY), context);
+        return new NPNClientConnection(endPoint, executor, getClientConnectionFactory(),
+                (SSLEngine)context.get(SslClientConnectionFactory.SSL_ENGINE_CONTEXT_KEY), context, protocol);
     }
 }
diff --git a/jetty-spdy/spdy-client/src/main/java/org/eclipse/jetty/spdy/client/NegotiatingClientConnection.java b/jetty-spdy/spdy-client/src/main/java/org/eclipse/jetty/spdy/client/NegotiatingClientConnection.java
new file mode 100644
index 0000000000..a2fc8511f4
--- /dev/null
+++ b/jetty-spdy/spdy-client/src/main/java/org/eclipse/jetty/spdy/client/NegotiatingClientConnection.java
@@ -0,0 +1,133 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2014 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.spdy.client;
+
+import java.io.IOException;
+import java.util.Map;
+import java.util.concurrent.Executor;
+import javax.net.ssl.SSLEngine;
+
+import org.eclipse.jetty.io.AbstractConnection;
+import org.eclipse.jetty.io.ClientConnectionFactory;
+import org.eclipse.jetty.io.Connection;
+import org.eclipse.jetty.io.EndPoint;
+import org.eclipse.jetty.io.RuntimeIOException;
+import org.eclipse.jetty.util.BufferUtil;
+import org.eclipse.jetty.util.log.Log;
+import org.eclipse.jetty.util.log.Logger;
+
+public abstract class NegotiatingClientConnection extends AbstractConnection
+{
+    private static final Logger LOG = Log.getLogger(NegotiatingClientConnection.class);
+
+    private final SSLEngine engine;
+    private final ClientConnectionFactory connectionFactory;
+    private final Map<String, Object> context;
+    private volatile boolean completed;
+
+    protected NegotiatingClientConnection(EndPoint endp, Executor executor, SSLEngine sslEngine, ClientConnectionFactory connectionFactory, Map<String, Object> context)
+    {
+        super(endp, executor);
+        this.engine = sslEngine;
+        this.connectionFactory = connectionFactory;
+        this.context = context;
+    }
+
+    protected SSLEngine getSSLEngine()
+    {
+        return engine;
+    }
+
+    protected void completed()
+    {
+        completed = true;
+    }
+
+    @Override
+    public void onOpen()
+    {
+        super.onOpen();
+        try
+        {
+            getEndPoint().flush(BufferUtil.EMPTY_BUFFER);
+            if (completed)
+                replaceConnection();
+            else
+                fillInterested();
+        }
+        catch (IOException x)
+        {
+            close();
+            throw new RuntimeIOException(x);
+        }
+    }
+
+    @Override
+    public void onFillable()
+    {
+        while (true)
+        {
+            int filled = fill();
+            if (filled == 0 && !completed)
+                fillInterested();
+            if (filled <= 0 || completed)
+                break;
+        }
+        if (completed)
+            replaceConnection();
+    }
+
+    private int fill()
+    {
+        try
+        {
+            return getEndPoint().fill(BufferUtil.EMPTY_BUFFER);
+        }
+        catch (IOException x)
+        {
+            LOG.debug(x);
+            close();
+            return -1;
+        }
+    }
+
+    private void replaceConnection()
+    {
+        EndPoint endPoint = getEndPoint();
+        try
+        {
+            Connection oldConnection = endPoint.getConnection();
+            Connection newConnection = connectionFactory.newConnection(endPoint, context);
+            ClientConnectionFactory.Helper.replaceConnection(oldConnection, newConnection);
+        }
+        catch (Throwable x)
+        {
+            LOG.debug(x);
+            close();
+        }
+    }
+
+    @Override
+    public void close()
+    {
+        // Gentler close for SSL.
+        getEndPoint().shutdownOutput();
+        super.close();
+    }
+}
diff --git a/jetty-spdy/spdy-client/src/main/java/org/eclipse/jetty/spdy/client/NegotiatingClientConnectionFactory.java b/jetty-spdy/spdy-client/src/main/java/org/eclipse/jetty/spdy/client/NegotiatingClientConnectionFactory.java
new file mode 100644
index 0000000000..50a1e5b223
--- /dev/null
+++ b/jetty-spdy/spdy-client/src/main/java/org/eclipse/jetty/spdy/client/NegotiatingClientConnectionFactory.java
@@ -0,0 +1,36 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2014 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.spdy.client;
+
+import org.eclipse.jetty.io.ClientConnectionFactory;
+
+public abstract class NegotiatingClientConnectionFactory implements ClientConnectionFactory
+{
+    private final ClientConnectionFactory connectionFactory;
+
+    protected NegotiatingClientConnectionFactory(ClientConnectionFactory connectionFactory)
+    {
+        this.connectionFactory = connectionFactory;
+    }
+
+    public ClientConnectionFactory getClientConnectionFactory()
+    {
+        return connectionFactory;
+    }
+}
diff --git a/jetty-spdy/spdy-client/src/main/java/org/eclipse/jetty/spdy/client/SPDYClient.java b/jetty-spdy/spdy-client/src/main/java/org/eclipse/jetty/spdy/client/SPDYClient.java
index 678aa2aac6..d30bc99f4e 100644
--- a/jetty-spdy/spdy-client/src/main/java/org/eclipse/jetty/spdy/client/SPDYClient.java
+++ b/jetty-spdy/spdy-client/src/main/java/org/eclipse/jetty/spdy/client/SPDYClient.java
@@ -26,7 +26,6 @@ import java.nio.channels.SocketChannel;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.Queue;
 import java.util.concurrent.ConcurrentLinkedQueue;
@@ -86,10 +85,6 @@ public class SPDYClient
         this.factory = factory;
         setInitialWindowSize(65536);
         setDispatchIO(true);
-        ClientConnectionFactory connectionFactory = new SPDYClientConnectionFactory();
-        if (factory.sslContextFactory != null)
-            connectionFactory = new SslClientConnectionFactory(factory.getSslContextFactory(), factory.getByteBufferPool(), factory.getExecutor(), new NPNClientConnectionFactory(this, connectionFactory));
-        setClientConnectionFactory(connectionFactory);
     }
 
     public short getVersion()
@@ -239,17 +234,6 @@ public class SPDYClient
         this.connectionFactory = connectionFactory;
     }
 
-    protected String selectProtocol(List<String> serverProtocols)
-    {
-        String protocol = "spdy/" + version;
-        for (String serverProtocol : serverProtocols)
-        {
-            if (serverProtocol.equals(protocol))
-                return protocol;
-        }
-        return null;
-    }
-
     protected FlowControlStrategy newFlowControlStrategy()
     {
         return FlowControlStrategyFactory.newFlowControlStrategy(version);
@@ -357,7 +341,19 @@ public class SPDYClient
 
         public SPDYClient newSPDYClient(short version)
         {
-            return new SPDYClient(version, this);
+            return newSPDYClient(version, new NPNClientConnectionFactory(getExecutor(), new SPDYClientConnectionFactory(), "spdy/" + version));
+        }
+
+        public SPDYClient newSPDYClient(short version, NegotiatingClientConnectionFactory negotiatingFactory)
+        {
+            SPDYClient client = new SPDYClient(version, this);
+
+            ClientConnectionFactory connectionFactory = negotiatingFactory.getClientConnectionFactory();
+            if (sslContextFactory != null)
+                connectionFactory = new SslClientConnectionFactory(getSslContextFactory(), getByteBufferPool(), getExecutor(), negotiatingFactory);
+
+            client.setClientConnectionFactory(connectionFactory);
+            return client;
         }
 
         @Override