jetty-9 - HTTP client: Clarified semantic of Response.Listener.onComplete(), introducing a Result class to hold the result of the exchange.
This commit is contained in:
parent
6f8a1c697f
commit
4ddc55ae11
|
@ -224,9 +224,9 @@ public class HttpClient extends AggregateLifeCycle
|
|||
return new HttpRequest(this, uri);
|
||||
}
|
||||
|
||||
protected Request newRequest(long id, URI uri)
|
||||
protected Request newRequest(long id, String uri)
|
||||
{
|
||||
return new HttpRequest(this, id, uri);
|
||||
return new HttpRequest(this, id, URI.create(uri));
|
||||
}
|
||||
|
||||
private String address(String scheme, String host, int port)
|
||||
|
|
|
@ -19,7 +19,6 @@
|
|||
package org.eclipse.jetty.client;
|
||||
|
||||
import org.eclipse.jetty.client.api.ContentResponse;
|
||||
import org.eclipse.jetty.client.api.Request;
|
||||
import org.eclipse.jetty.client.api.Response;
|
||||
import org.eclipse.jetty.http.HttpFields;
|
||||
import org.eclipse.jetty.http.HttpVersion;
|
||||
|
@ -35,12 +34,6 @@ public class HttpContentResponse implements ContentResponse
|
|||
this.content = content;
|
||||
}
|
||||
|
||||
@Override
|
||||
public Request request()
|
||||
{
|
||||
return response.request();
|
||||
}
|
||||
|
||||
@Override
|
||||
public Listener listener()
|
||||
{
|
||||
|
|
|
@ -42,7 +42,7 @@ public class HttpExchange
|
|||
this.connection = connection;
|
||||
this.request = request;
|
||||
this.listener = listener;
|
||||
this.response = new HttpResponse(request, listener);
|
||||
this.response = new HttpResponse(listener);
|
||||
}
|
||||
|
||||
public HttpConversation conversation()
|
||||
|
|
|
@ -26,6 +26,7 @@ import java.util.concurrent.TimeoutException;
|
|||
|
||||
import org.eclipse.jetty.client.api.CookieStore;
|
||||
import org.eclipse.jetty.client.api.Response;
|
||||
import org.eclipse.jetty.client.api.Result;
|
||||
import org.eclipse.jetty.http.HttpCookie;
|
||||
import org.eclipse.jetty.http.HttpHeader;
|
||||
import org.eclipse.jetty.http.HttpParser;
|
||||
|
@ -186,7 +187,10 @@ public class HttpReceiver implements HttpParser.ResponseHandler<ByteBuffer>
|
|||
HttpConversation conversation = exchange.conversation();
|
||||
notifySuccess(conversation.listener(), response);
|
||||
if (exchangeComplete)
|
||||
notifyComplete(conversation.listener(), exchange.response(), null);
|
||||
{
|
||||
Result result = new Result(exchange.request(), exchange.response());
|
||||
notifyComplete(conversation.listener(), result);
|
||||
}
|
||||
}
|
||||
|
||||
protected void fail(Throwable failure)
|
||||
|
@ -210,7 +214,10 @@ public class HttpReceiver implements HttpParser.ResponseHandler<ByteBuffer>
|
|||
HttpConversation conversation = exchange.conversation();
|
||||
notifyFailure(conversation.listener(), response, failure);
|
||||
if (exchangeComplete)
|
||||
notifyComplete(conversation.listener(), exchange.response(), failure);
|
||||
{
|
||||
Result result = new Result(exchange.request(), response, failure);
|
||||
notifyComplete(conversation.listener(), result);
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
|
@ -293,12 +300,12 @@ public class HttpReceiver implements HttpParser.ResponseHandler<ByteBuffer>
|
|||
}
|
||||
}
|
||||
|
||||
private void notifyComplete(Response.Listener listener, Response response, Throwable failure)
|
||||
private void notifyComplete(Response.Listener listener, Result result)
|
||||
{
|
||||
try
|
||||
{
|
||||
if (listener != null)
|
||||
listener.onComplete(response, failure);
|
||||
listener.onComplete(result);
|
||||
}
|
||||
catch (Exception x)
|
||||
{
|
||||
|
|
|
@ -32,6 +32,7 @@ import org.eclipse.jetty.client.api.ContentProvider;
|
|||
import org.eclipse.jetty.client.api.ContentResponse;
|
||||
import org.eclipse.jetty.client.api.Request;
|
||||
import org.eclipse.jetty.client.api.Response;
|
||||
import org.eclipse.jetty.client.api.Result;
|
||||
import org.eclipse.jetty.client.util.BufferingResponseListener;
|
||||
import org.eclipse.jetty.client.util.PathContentProvider;
|
||||
import org.eclipse.jetty.http.HttpFields;
|
||||
|
@ -283,22 +284,22 @@ public class HttpRequest implements Request
|
|||
@Override
|
||||
public Future<ContentResponse> send()
|
||||
{
|
||||
final FutureCallback<ContentResponse> result = new FutureCallback<>();
|
||||
final FutureCallback<ContentResponse> callback = new FutureCallback<>();
|
||||
BufferingResponseListener listener = new BufferingResponseListener()
|
||||
{
|
||||
@Override
|
||||
public void onComplete(Response response, Throwable failure)
|
||||
public void onComplete(Result result)
|
||||
{
|
||||
super.onComplete(response, failure);
|
||||
HttpContentResponse contentResponse = new HttpContentResponse(response, content());
|
||||
if (failure == null)
|
||||
result.completed(contentResponse);
|
||||
super.onComplete(result);
|
||||
HttpContentResponse contentResponse = new HttpContentResponse(result.getResponse(), content());
|
||||
if (!result.isFailed())
|
||||
callback.completed(contentResponse);
|
||||
else
|
||||
result.failed(contentResponse, failure);
|
||||
callback.failed(contentResponse, result.getFailure());
|
||||
}
|
||||
};
|
||||
send(listener);
|
||||
return result;
|
||||
return callback;
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
|
@ -18,7 +18,6 @@
|
|||
|
||||
package org.eclipse.jetty.client;
|
||||
|
||||
import org.eclipse.jetty.client.api.Request;
|
||||
import org.eclipse.jetty.client.api.Response;
|
||||
import org.eclipse.jetty.http.HttpFields;
|
||||
import org.eclipse.jetty.http.HttpVersion;
|
||||
|
@ -26,15 +25,13 @@ import org.eclipse.jetty.http.HttpVersion;
|
|||
public class HttpResponse implements Response
|
||||
{
|
||||
private final HttpFields headers = new HttpFields();
|
||||
private final Request request;
|
||||
private final Listener listener;
|
||||
private HttpVersion version;
|
||||
private int status;
|
||||
private String reason;
|
||||
|
||||
public HttpResponse(Request request, Response.Listener listener)
|
||||
public HttpResponse(Response.Listener listener)
|
||||
{
|
||||
this.request = request;
|
||||
this.listener = listener;
|
||||
}
|
||||
|
||||
|
@ -78,12 +75,6 @@ public class HttpResponse implements Response
|
|||
return headers;
|
||||
}
|
||||
|
||||
@Override
|
||||
public Request request()
|
||||
{
|
||||
return request;
|
||||
}
|
||||
|
||||
@Override
|
||||
public Listener listener()
|
||||
{
|
||||
|
|
|
@ -28,6 +28,7 @@ import java.util.concurrent.atomic.AtomicReference;
|
|||
import org.eclipse.jetty.client.api.ContentProvider;
|
||||
import org.eclipse.jetty.client.api.Request;
|
||||
import org.eclipse.jetty.client.api.Response;
|
||||
import org.eclipse.jetty.client.api.Result;
|
||||
import org.eclipse.jetty.http.HttpGenerator;
|
||||
import org.eclipse.jetty.io.ByteBufferPool;
|
||||
import org.eclipse.jetty.io.EndPoint;
|
||||
|
@ -202,14 +203,19 @@ public class HttpSender
|
|||
|
||||
// Notify after
|
||||
HttpExchange exchange = connection.getExchange();
|
||||
LOG.debug("Sent {}", exchange.request());
|
||||
Request request = exchange.request();
|
||||
LOG.debug("Sent {}", request);
|
||||
boolean exchangeCompleted = exchange.requestComplete(true);
|
||||
|
||||
// It is important to notify *after* we reset because
|
||||
// the notification may trigger another request/response
|
||||
notifyRequestSuccess(exchange.request());
|
||||
notifyRequestSuccess(request);
|
||||
if (exchangeCompleted)
|
||||
notifyComplete(exchange.conversation().listener(), exchange.response(), null);
|
||||
{
|
||||
HttpConversation conversation = exchange.conversation();
|
||||
Result result = new Result(request, exchange.response());
|
||||
notifyComplete(conversation.listener(), result);
|
||||
}
|
||||
}
|
||||
|
||||
protected void fail(Throwable failure)
|
||||
|
@ -224,16 +230,19 @@ public class HttpSender
|
|||
|
||||
// Notify after
|
||||
HttpExchange exchange = connection.getExchange();
|
||||
LOG.debug("Failed {}", exchange.request());
|
||||
Request request = exchange.request();
|
||||
LOG.debug("Failed {}", request);
|
||||
boolean exchangeCompleted = exchange.requestComplete(false);
|
||||
if (!exchangeCompleted && !committed)
|
||||
exchangeCompleted = exchange.responseComplete(false);
|
||||
|
||||
notifyRequestFailure(exchange.request(), failure);
|
||||
Response.Listener listener = exchange.conversation().listener();
|
||||
notifyResponseFailure(listener, failure);
|
||||
notifyRequestFailure(request, failure);
|
||||
if (exchangeCompleted)
|
||||
notifyComplete(listener, exchange.response(), failure);
|
||||
{
|
||||
HttpConversation conversation = exchange.conversation();
|
||||
Result result = new Result(request, failure, exchange.response());
|
||||
notifyComplete(conversation.listener(), result);
|
||||
}
|
||||
}
|
||||
|
||||
private void releaseBuffers()
|
||||
|
@ -307,25 +316,12 @@ public class HttpSender
|
|||
}
|
||||
}
|
||||
|
||||
private void notifyResponseFailure(Response.Listener listener, Throwable failure)
|
||||
private void notifyComplete(Response.Listener listener, Result result)
|
||||
{
|
||||
try
|
||||
{
|
||||
if (listener != null)
|
||||
listener.onFailure(null, failure);
|
||||
}
|
||||
catch (Exception x)
|
||||
{
|
||||
LOG.info("Exception while notifying listener " + listener, x);
|
||||
}
|
||||
}
|
||||
|
||||
private void notifyComplete(Response.Listener listener, Response response, Throwable failure)
|
||||
{
|
||||
try
|
||||
{
|
||||
if (listener != null)
|
||||
listener.onComplete(response, failure);
|
||||
listener.onComplete(result);
|
||||
}
|
||||
catch (Exception x)
|
||||
{
|
||||
|
|
|
@ -18,10 +18,9 @@
|
|||
|
||||
package org.eclipse.jetty.client;
|
||||
|
||||
import java.net.URI;
|
||||
|
||||
import org.eclipse.jetty.client.api.Request;
|
||||
import org.eclipse.jetty.client.api.Response;
|
||||
import org.eclipse.jetty.client.api.Result;
|
||||
|
||||
public class RedirectionProtocolListener extends Response.Listener.Adapter
|
||||
{
|
||||
|
@ -33,10 +32,11 @@ public class RedirectionProtocolListener extends Response.Listener.Adapter
|
|||
}
|
||||
|
||||
@Override
|
||||
public void onComplete(Response response, Throwable failure)
|
||||
public void onComplete(Result result)
|
||||
{
|
||||
if (failure == null)
|
||||
if (!result.isFailed())
|
||||
{
|
||||
Response response = result.getResponse();
|
||||
switch (response.status())
|
||||
{
|
||||
case 301: // GET or HEAD only allowed, keep the method
|
||||
|
@ -47,15 +47,16 @@ public class RedirectionProtocolListener extends Response.Listener.Adapter
|
|||
case 303: // use GET for next request
|
||||
{
|
||||
String location = response.headers().get("location");
|
||||
Request redirect = client.newRequest(response.request().id(), URI.create(location));
|
||||
Request redirect = client.newRequest(result.getRequest().id(), location);
|
||||
redirect.send(this);
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
// TODO: here I should call on conversation.first().listener() both onFailure() and onComplete()
|
||||
HttpConversation conversation = client.getConversation(response.request());
|
||||
HttpConversation conversation = client.getConversation(result.getRequest());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -25,8 +25,6 @@ import org.eclipse.jetty.http.HttpVersion;
|
|||
|
||||
public interface Response
|
||||
{
|
||||
Request request();
|
||||
|
||||
Listener listener();
|
||||
|
||||
HttpVersion version();
|
||||
|
@ -51,7 +49,7 @@ public interface Response
|
|||
|
||||
public void onFailure(Response response, Throwable failure);
|
||||
|
||||
public void onComplete(Response response, Throwable failure);
|
||||
public void onComplete(Result result);
|
||||
|
||||
public static class Adapter implements Listener
|
||||
{
|
||||
|
@ -81,7 +79,7 @@ public interface Response
|
|||
}
|
||||
|
||||
@Override
|
||||
public void onComplete(Response response, Throwable failure)
|
||||
public void onComplete(Result result)
|
||||
{
|
||||
}
|
||||
}
|
||||
|
|
|
@ -0,0 +1,70 @@
|
|||
//
|
||||
// ========================================================================
|
||||
// Copyright (c) 1995-2012 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.client.api;
|
||||
|
||||
public class Result
|
||||
{
|
||||
private final Request request;
|
||||
private final Throwable requestFailure;
|
||||
private final Response response;
|
||||
private final Throwable responseFailure;
|
||||
|
||||
public Result(Request request, Response response)
|
||||
{
|
||||
this(request, null, response, null);
|
||||
}
|
||||
|
||||
public Result(Request request, Response response, Throwable responseFailure)
|
||||
{
|
||||
this(request, null, response, responseFailure);
|
||||
}
|
||||
|
||||
public Result(Request request, Throwable requestFailure, Response response)
|
||||
{
|
||||
this(request, requestFailure, response, null);
|
||||
}
|
||||
|
||||
private Result(Request request, Throwable requestFailure, Response response, Throwable responseFailure)
|
||||
{
|
||||
this.request = request;
|
||||
this.requestFailure = requestFailure;
|
||||
this.response = response;
|
||||
this.responseFailure = responseFailure;
|
||||
}
|
||||
|
||||
public Request getRequest()
|
||||
{
|
||||
return request;
|
||||
}
|
||||
|
||||
public Response getResponse()
|
||||
{
|
||||
return response;
|
||||
}
|
||||
|
||||
public boolean isFailed()
|
||||
{
|
||||
return getFailure() != null;
|
||||
}
|
||||
|
||||
public Throwable getFailure()
|
||||
{
|
||||
return responseFailure != null ? responseFailure : requestFailure;
|
||||
}
|
||||
}
|
|
@ -41,6 +41,7 @@ import org.eclipse.jetty.client.api.ContentProvider;
|
|||
import org.eclipse.jetty.client.api.ContentResponse;
|
||||
import org.eclipse.jetty.client.api.Destination;
|
||||
import org.eclipse.jetty.client.api.Response;
|
||||
import org.eclipse.jetty.client.api.Result;
|
||||
import org.eclipse.jetty.http.HttpCookie;
|
||||
import org.eclipse.jetty.server.Request;
|
||||
import org.eclipse.jetty.server.handler.AbstractHandler;
|
||||
|
@ -363,7 +364,7 @@ public class HttpClientTest extends AbstractHttpClientServerTest
|
|||
}
|
||||
|
||||
@Override
|
||||
public void onComplete(Response response, Throwable failure)
|
||||
public void onComplete(Result result)
|
||||
{
|
||||
exchangeTime.set(System.nanoTime());
|
||||
latch.countDown();
|
||||
|
@ -421,7 +422,7 @@ public class HttpClientTest extends AbstractHttpClientServerTest
|
|||
.send(new Response.Listener.Adapter()
|
||||
{
|
||||
@Override
|
||||
public void onComplete(Response response, Throwable failure)
|
||||
public void onComplete(Result result)
|
||||
{
|
||||
latch.countDown();
|
||||
}
|
||||
|
@ -451,7 +452,7 @@ public class HttpClientTest extends AbstractHttpClientServerTest
|
|||
.send(new Response.Listener.Adapter()
|
||||
{
|
||||
@Override
|
||||
public void onComplete(Response response, Throwable failure)
|
||||
public void onComplete(Result result)
|
||||
{
|
||||
latch.countDown();
|
||||
}
|
||||
|
|
|
@ -25,6 +25,7 @@ import java.util.concurrent.TimeUnit;
|
|||
import org.eclipse.jetty.client.api.Connection;
|
||||
import org.eclipse.jetty.client.api.Request;
|
||||
import org.eclipse.jetty.client.api.Response;
|
||||
import org.eclipse.jetty.client.api.Result;
|
||||
import org.eclipse.jetty.http.HttpHeader;
|
||||
import org.junit.Assert;
|
||||
import org.junit.Test;
|
||||
|
@ -99,7 +100,7 @@ public class HttpConnectionLifecycleTest extends AbstractHttpClientServerTest
|
|||
Assert.assertEquals(0, activeConnections.size());
|
||||
|
||||
final CountDownLatch headersLatch = new CountDownLatch(1);
|
||||
final CountDownLatch failureLatch = new CountDownLatch(3);
|
||||
final CountDownLatch failureLatch = new CountDownLatch(2);
|
||||
client.newRequest(host, port).listener(new Request.Listener.Adapter()
|
||||
{
|
||||
@Override
|
||||
|
@ -117,14 +118,9 @@ public class HttpConnectionLifecycleTest extends AbstractHttpClientServerTest
|
|||
}).send(new Response.Listener.Adapter()
|
||||
{
|
||||
@Override
|
||||
public void onFailure(Response response, Throwable failure)
|
||||
{
|
||||
failureLatch.countDown();
|
||||
}
|
||||
|
||||
@Override
|
||||
public void onComplete(Response response, Throwable failure)
|
||||
public void onComplete(Result result)
|
||||
{
|
||||
Assert.assertTrue(result.isFailed());
|
||||
Assert.assertEquals(0, idleConnections.size());
|
||||
Assert.assertEquals(0, activeConnections.size());
|
||||
failureLatch.countDown();
|
||||
|
|
|
@ -25,6 +25,7 @@ import java.util.concurrent.TimeUnit;
|
|||
|
||||
import org.eclipse.jetty.client.api.Request;
|
||||
import org.eclipse.jetty.client.api.Response;
|
||||
import org.eclipse.jetty.client.api.Result;
|
||||
import org.eclipse.jetty.client.util.ByteBufferContentProvider;
|
||||
import org.eclipse.jetty.io.ByteArrayEndPoint;
|
||||
import org.eclipse.jetty.toolchain.test.annotation.Slow;
|
||||
|
@ -131,8 +132,9 @@ public class HttpSenderTest
|
|||
connection.send(request, new Response.Listener.Adapter()
|
||||
{
|
||||
@Override
|
||||
public void onFailure(Response response, Throwable failure)
|
||||
public void onComplete(Result result)
|
||||
{
|
||||
Assert.assertTrue(result.isFailed());
|
||||
failureLatch.countDown();
|
||||
}
|
||||
});
|
||||
|
@ -159,8 +161,9 @@ public class HttpSenderTest
|
|||
connection.send(request, new Response.Listener.Adapter()
|
||||
{
|
||||
@Override
|
||||
public void onFailure(Response response, Throwable failure)
|
||||
public void onComplete(Result result)
|
||||
{
|
||||
Assert.assertTrue(result.isFailed());
|
||||
failureLatch.countDown();
|
||||
}
|
||||
});
|
||||
|
|
Loading…
Reference in New Issue