Reduced object creation by caching the conversation response listeners.

This commit is contained in:
Simone Bordet 2013-02-20 11:18:00 +01:00
parent 9be9910cde
commit 29d779778e
7 changed files with 75 additions and 47 deletions

View File

@ -80,14 +80,12 @@ public class AuthenticationProtocolHandler implements ProtocolHandler
public void onComplete(Result result)
{
Request request = result.getRequest();
HttpConversation conversation = client.getConversation(request.getConversationID(), false);
List<Response.ResponseListener> listeners = conversation.getExchanges().peekFirst().getResponseListeners();
ContentResponse response = new HttpContentResponse(result.getResponse(), getContent(), getEncoding());
if (result.isFailed())
{
Throwable failure = result.getFailure();
LOG.debug("Authentication challenge failed {}", failure);
notifier.forwardFailureComplete(listeners, request, result.getRequestFailure(), response, result.getResponseFailure());
forwardFailureComplete(request, result.getRequestFailure(), response, result.getResponseFailure());
return;
}
@ -95,7 +93,7 @@ public class AuthenticationProtocolHandler implements ProtocolHandler
if (wwwAuthenticates.isEmpty())
{
LOG.debug("Authentication challenge without WWW-Authenticate header");
notifier.forwardFailureComplete(listeners, request, null, response, new HttpResponseException("HTTP protocol violation: 401 without WWW-Authenticate header", response));
forwardFailureComplete(request, null, response, new HttpResponseException("HTTP protocol violation: 401 without WWW-Authenticate header", response));
return;
}
@ -114,15 +112,16 @@ public class AuthenticationProtocolHandler implements ProtocolHandler
if (authentication == null)
{
LOG.debug("No authentication available for {}", request);
notifier.forwardSuccessComplete(listeners, request, response);
forwardSuccessComplete(request, response);
return;
}
HttpConversation conversation = client.getConversation(request.getConversationID(), false);
final Authentication.Result authnResult = authentication.authenticate(request, response, wwwAuthenticate.value, conversation);
LOG.debug("Authentication result {}", authnResult);
if (authnResult == null)
{
notifier.forwardSuccessComplete(listeners, request, response);
forwardSuccessComplete(request, response);
return;
}
@ -138,6 +137,20 @@ public class AuthenticationProtocolHandler implements ProtocolHandler
}).send(null);
}
private void forwardSuccessComplete(Request request, Response response)
{
HttpConversation conversation = client.getConversation(request.getConversationID(), false);
conversation.updateResponseListeners(null);
notifier.forwardSuccessComplete(conversation.getResponseListeners(), request, response);
}
private void forwardFailureComplete(Request request, Throwable requestFailure, Response response, Throwable responseFailure)
{
HttpConversation conversation = client.getConversation(request.getConversationID(), false);
conversation.updateResponseListeners(null);
notifier.forwardFailureComplete(conversation.getResponseListeners(), request, requestFailure, response, responseFailure);
}
private List<WWWAuthenticate> parseWWWAuthenticate(Response response)
{
// TODO: these should be ordered by strength

View File

@ -68,15 +68,16 @@ public class ContinueProtocolHandler implements ProtocolHandler
// Mark the 100 Continue response as handled
conversation.setAttribute(ATTRIBUTE, Boolean.TRUE);
// Reset the conversation listeners, since we are going to receive another response code
conversation.updateResponseListeners(null);
HttpExchange exchange = conversation.getExchanges().peekLast();
assert exchange.getResponse() == response;
List<Response.ResponseListener> listeners = exchange.getResponseListeners();
switch (response.getStatus())
{
case 100:
{
// All good, continue
conversation.setResponseListener(null);
exchange.resetResponse(true);
exchange.proceed(true);
break;
@ -86,7 +87,7 @@ public class ContinueProtocolHandler implements ProtocolHandler
// Server either does not support 100 Continue,
// or it does and wants to refuse the request content,
// or we got some other HTTP status code like a redirect.
conversation.setResponseListener(null);
List<Response.ResponseListener> listeners = exchange.getResponseListeners();
HttpContentResponse contentResponse = new HttpContentResponse(response, getContent(), getEncoding());
notifier.forwardSuccess(listeners, contentResponse);
exchange.proceed(false);
@ -101,6 +102,8 @@ public class ContinueProtocolHandler implements ProtocolHandler
HttpConversation conversation = client.getConversation(response.getConversationID(), false);
// Mark the 100 Continue response as handled
conversation.setAttribute(ATTRIBUTE, Boolean.TRUE);
// Reset the conversation listeners to allow the conversation to be completed
conversation.updateResponseListeners(null);
HttpExchange exchange = conversation.getExchanges().peekLast();
assert exchange.getResponse() == response;

View File

@ -19,7 +19,6 @@
package org.eclipse.jetty.client;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Deque;
import java.util.List;
import java.util.concurrent.ConcurrentLinkedDeque;
@ -32,7 +31,8 @@ public class HttpConversation extends AttributesMap
private final Deque<HttpExchange> exchanges = new ConcurrentLinkedDeque<>();
private final HttpClient client;
private final long id;
private volatile Response.ResponseListener listener;
private volatile boolean complete;
private volatile List<Response.ResponseListener> listeners;
public HttpConversation(HttpClient client, long id)
{
@ -55,42 +55,42 @@ public class HttpConversation extends AttributesMap
* This list changes as the conversation proceeds, as follows:
* <ol>
* <li>
* request R1 send => conversation.setResponseListener(null)
* request R1 send => conversation.updateResponseListeners(null)
* <ul>
* <li>exchanges in conversation: E1</li>
* <li>listeners to be notified: E1.listeners</li>
* </ul>
* </li>
* <li>
* response R1 arrived, 401 => conversation.setResponseListener(AuthenticationProtocolHandler.listener)
* response R1 arrived, 401 => conversation.updateResponseListeners(AuthenticationProtocolHandler.listener)
* <ul>
* <li>exchanges in conversation: E1</li>
* <li>listeners to be notified: AuthenticationProtocolHandler.listener</li>
* </ul>
* </li>
* <li>
* request R2 send => conversation.setResponseListener(null)
* request R2 send => conversation.updateResponseListeners(null)
* <ul>
* <li>exchanges in conversation: E1 + E2</li>
* <li>listeners to be notified: E2.listeners + E1.listeners</li>
* </ul>
* </li>
* <li>
* response R2 arrived, 302 => conversation.setResponseListener(RedirectProtocolHandler.listener)
* response R2 arrived, 302 => conversation.updateResponseListeners(RedirectProtocolHandler.listener)
* <ul>
* <li>exchanges in conversation: E1 + E2</li>
* <li>listeners to be notified: E2.listeners + RedirectProtocolHandler.listener</li>
* </ul>
* </li>
* <li>
* request R3 send => conversation.setResponseListener(null)
* request R3 send => conversation.updateResponseListeners(null)
* <ul>
* <li>exchanges in conversation: E1 + E2 + E3</li>
* <li>listeners to be notified: E3.listeners + E1.listeners</li>
* </ul>
* </li>
* <li>
* response R3 arrived, 200 => conversation.setResponseListener(null)
* response R3 arrived, 200 => conversation.updateResponseListeners(null)
* <ul>
* <li>exchanges in conversation: E1 + E2 + E3</li>
* <li>listeners to be notified: E3.listeners + E1.listeners</li>
@ -110,45 +110,52 @@ public class HttpConversation extends AttributesMap
*/
public List<Response.ResponseListener> getResponseListeners()
{
return listeners;
}
/**
* Requests to update the response listener, eventually using the given override response listener,
* that must be notified instead of the first exchange response listeners.
* This works in conjunction with {@link #getResponseListeners()}, returning the appropriate response
* listeners that needs to be notified of response events.
*
* @param overrideListener the override response listener
*/
public void updateResponseListeners(Response.ResponseListener overrideListener)
{
// If we have no override listener, then the
// conversation may be completed at a later time
complete = overrideListener == null;
// Create a new instance to avoid that iterating over the listeners
// will notify a listener that may send a new request and trigger
// another call to this method which will build different listeners
// which may be iterated over when the iteration continues.
listeners = new ArrayList<>();
HttpExchange firstExchange = exchanges.peekFirst();
HttpExchange lastExchange = exchanges.peekLast();
if (firstExchange == lastExchange)
{
if (listener != null)
return Arrays.asList(listener);
if (overrideListener != null)
listeners.add(overrideListener);
else
return firstExchange.getResponseListeners();
listeners.addAll(firstExchange.getResponseListeners());
}
else
{
// Order is important, we want to notify the last exchange first
List<Response.ResponseListener> result = new ArrayList<>(lastExchange.getResponseListeners());
if (listener != null)
result.add(listener);
listeners.addAll(lastExchange.getResponseListeners());
if (overrideListener != null)
listeners.add(overrideListener);
else
result.addAll(firstExchange.getResponseListeners());
return result;
listeners.addAll(firstExchange.getResponseListeners());
}
}
/**
* Sets an override response listener that must be notified instead of the first exchange response listeners.
* This works in conjunction with {@link #getResponseListeners()}, returning the appropriate response
* listeners that needs to be notified of response events.
*
* @param listener the override response listener
*/
public void setResponseListener(Response.ResponseListener listener)
{
this.listener = listener;
}
public void complete()
{
// The conversation is really terminated only
// when there is no conversation listener that
// may have continued the conversation.
if (listener == null)
if (complete)
client.removeConversation(this);
}

View File

@ -52,7 +52,7 @@ public class HttpExchange
this.listeners = listeners;
this.response = new HttpResponse(request, listeners);
conversation.getExchanges().offer(this);
conversation.setResponseListener(null);
conversation.updateResponseListeners(null);
}
public HttpConversation getConversation()

View File

@ -145,8 +145,13 @@ public class HttpReceiver implements HttpParser.ResponseHandler<ByteBuffer>
// Probe the protocol handlers
HttpClient client = connection.getHttpClient();
ProtocolHandler protocolHandler = client.findProtocolHandler(exchange.getRequest(), response);
Response.Listener handlerListener = protocolHandler == null ? null : protocolHandler.getResponseListener();
exchange.getConversation().setResponseListener(handlerListener);
Response.Listener handlerListener = null;
if (protocolHandler != null)
{
handlerListener = protocolHandler.getResponseListener();
LOG.debug("Found protocol handler {}", protocolHandler);
}
exchange.getConversation().updateResponseListeners(handlerListener);
LOG.debug("Receiving {}", response);
ResponseNotifier notifier = connection.getDestination().getResponseNotifier();

View File

@ -212,7 +212,8 @@ public class RedirectProtocolHandler extends Response.Listener.Empty implements
Request request = result.getRequest();
Response response = result.getResponse();
HttpConversation conversation = client.getConversation(request.getConversationID(), false);
List<Response.ResponseListener> listeners = conversation.getExchanges().peekFirst().getResponseListeners();
conversation.updateResponseListeners(null);
List<Response.ResponseListener> listeners = conversation.getResponseListeners();
notifier.notifyFailure(listeners, response, failure);
notifier.notifyComplete(listeners, new Result(request, response, failure));
}

View File

@ -49,7 +49,6 @@ import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.util.security.Constraint;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.junit.Assert;
import org.junit.Ignore;
import org.junit.Test;
public class HttpClientAuthenticationTest extends AbstractHttpClientServerTest
@ -104,7 +103,6 @@ public class HttpClientAuthenticationTest extends AbstractHttpClientServerTest
test_Authentication(new BasicAuthentication(uri, realm, "basic", "basic"));
}
@Ignore
@Test
public void test_DigestAuthentication() throws Exception
{
@ -135,6 +133,7 @@ public class HttpClientAuthenticationTest extends AbstractHttpClientServerTest
Assert.assertEquals(401, response.getStatus());
Assert.assertTrue(requests.get().await(5, TimeUnit.SECONDS));
client.getRequestListeners().remove(requestListener);
Assert.assertNull(client.getConversation(request.getConversationID(), false));
authenticationStore.addAuthentication(authentication);