Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Experiment/12/improve default servlet #10222

Merged
merged 14 commits into from
Aug 17, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -774,7 +774,7 @@ else if (response != null)
}

if (LOG.isDebugEnabled())
LOG.debug(_endOfContent.toString());
LOG.debug("endOfContent {} content-Length {}", _endOfContent.toString(), contentLength);

// Add transfer encoding if it is not chunking
if (transferEncoding != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +656,7 @@ public void close()
@Override
public long getLength()
{
// TODO: it is difficult to calculate the length because
// TODO: #10307 it is difficult to calculate the length because
// we need to allow for customization of the headers from
// subclasses, and then serialize all the headers to get
// their length (handling UTF-8 values) and we don't want
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,12 @@ public boolean isCommitted()
return true;
}

@Override
public boolean hasLastWrite()
{
return false;
}

@Override
public boolean isCompletedSuccessfully()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -768,13 +768,13 @@ public Request getWrapped()
}

@SuppressWarnings("unchecked")
static <T extends Request.Wrapper> T as(Request request, Class<T> type)
static <T extends Request> T as(Request request, Class<T> type)
{
while (request instanceof Request.Wrapper wrapper)
while (request != null)
{
if (type.isInstance(wrapper))
return (T)wrapper;
request = wrapper.getWrapped();
if (type.isInstance(request))
return (T)request;
request = request instanceof Request.Wrapper wrapper ? wrapper.getWrapped() : null;
}
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public List<String> getGzipEquivalentFileExtensions()
return _gzipEquivalentFileExtensions;
}

public void doGet(Request request, Response response, Callback callback, HttpContent content) throws Exception
public void doGet(Request request, Response response, Callback callback, HttpContent content)
{
String pathInContext = Request.getPathInContext(request);

Expand Down Expand Up @@ -523,7 +523,7 @@ protected void handleWelcomeAction(Request request, Response response, Callback
// TODO : check conditional headers.
serveWelcome(request, response, callback, welcomeAction.target);
case REHANDLE -> rehandleWelcome(request, response, callback, welcomeAction.target);
};
}
}

/**
Expand Down Expand Up @@ -683,14 +683,15 @@ private void sendData(Request request, Response response, Callback callback, Htt
}

// There are multiple non-overlapping ranges, send a multipart/byteranges 206 response.
putHeaders(response, content, NO_CONTENT_LENGTH);
response.setStatus(HttpStatus.PARTIAL_CONTENT_206);
String contentType = "multipart/byteranges; boundary=";
String boundary = MultiPart.generateBoundary(null, 24);
response.getHeaders().put(HttpHeader.CONTENT_TYPE, contentType + boundary);
MultiPartByteRanges.ContentSource byteRanges = new MultiPartByteRanges.ContentSource(boundary);
ranges.forEach(range -> byteRanges.addPart(new MultiPartByteRanges.Part(content.getContentTypeValue(), content.getResource().getPath(), range, contentLength)));
byteRanges.close();
long partsContentLength = byteRanges.getLength();
putHeaders(response, content, partsContentLength);
response.getHeaders().put(HttpHeader.CONTENT_TYPE, contentType + boundary);
Content.copy(byteRanges, response, callback);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,13 @@ public interface Response extends Content.Sink
*/
boolean isCommitted();

/**
* <p>Returns whether the last write has been initiated on the response.</p>
*
* @return {@code true} if {@code last==true} has been passed to {@link #write(boolean, ByteBuffer, Callback)}.
*/
boolean hasLastWrite();

/**
* <p>Returns whether the response completed successfully.</p>
* <p>The response HTTP status code, HTTP headers and content
Expand Down Expand Up @@ -207,13 +214,13 @@ static Content.Chunk.Processor newTrailersChunkProcessor(Response response)
* @see Wrapper
*/
@SuppressWarnings("unchecked")
static <T extends Response.Wrapper> T as(Response response, Class<T> type)
static <T extends Response> T as(Response response, Class<T> type)
{
while (response instanceof Response.Wrapper wrapper)
while (response != null)
{
if (type.isInstance(wrapper))
return (T)wrapper;
response = wrapper.getWrapped();
if (type.isInstance(response))
return (T)response;
response = response instanceof Response.Wrapper wrapper ? wrapper.getWrapped() : null;
}
return null;
}
Expand Down Expand Up @@ -580,6 +587,12 @@ public boolean isCommitted()
return getWrapped().isCommitted();
}

@Override
public boolean hasLastWrite()
{
return getWrapped().hasLastWrite();
}

@Override
public boolean isCompletedSuccessfully()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,7 @@ protected void generateResponse(Request request, Response response, int code, St
{
if (LOG.isDebugEnabled())
LOG.debug("Unable to process error {}", reRequest, e);
if (ExceptionUtil.areNotAssociated(cause, e))
cause.addSuppressed(e);
ExceptionUtil.addSuppressedIfNotAssociated(cause, e);
response.setStatus(code);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,13 @@ public void write(boolean last, ByteBuffer content, Callback callback)
case NOT_COMPRESSING -> super.write(last, content, callback);
case COMMITTING -> callback.failed(new WritePendingException());
case COMPRESSING -> gzip(last, callback, content);
default -> callback.failed(new IllegalStateException("state=" + _state.get()));
default ->
{
if (BufferUtil.isEmpty(content))
callback.succeeded();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the case where this happens?
If last is true do we not need to do anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lachlan-roberts This can happen is something does a flush when last has already been sent. This can be needed when checking there is no data buffered in a writer.

else
callback.failed(new IllegalStateException("state=" + _state.get()));
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -473,8 +473,7 @@ else if (ExceptionUtil.areNotAssociated(_failure.getFailure(), x) && _failure.ge
}
catch (Throwable throwable)
{
if (ExceptionUtil.areNotAssociated(x, throwable))
x.addSuppressed(throwable);
ExceptionUtil.addSuppressedIfNotAssociated(x, throwable);
}

// If the application has not been otherwise informed of the failure
Expand Down Expand Up @@ -1080,8 +1079,7 @@ public void addFailureListener(Consumer<Throwable> onFailure)
}
catch (Throwable t)
{
if (ExceptionUtil.areNotAssociated(throwable, t))
throwable.addSuppressed(t);
ExceptionUtil.addSuppressedIfNotAssociated(throwable, t);
}
finally
{
Expand Down Expand Up @@ -1354,6 +1352,18 @@ public boolean isCommitted()
return _httpFields.isCommitted();
}

@Override
public boolean hasLastWrite()
{
try (AutoLock ignored = _request._lock.lock())
{
if (_request._httpChannelState == null)
return true;

return _request._httpChannelState._streamSendState != StreamSendState.SENDING;
}
}

@Override
public boolean isCompletedSuccessfully()
{
Expand Down Expand Up @@ -1540,8 +1550,7 @@ public void failed(Throwable failure)

// Consume any input.
Throwable unconsumed = stream.consumeAvailable();
if (ExceptionUtil.areNotAssociated(unconsumed, failure))
failure.addSuppressed(unconsumed);
ExceptionUtil.addSuppressedIfNotAssociated(failure, unconsumed);

if (LOG.isDebugEnabled())
LOG.debug("failed stream.isCommitted={}, response.isCommitted={} {}", httpChannelState._stream.isCommitted(), httpChannelState._response.isCommitted(), this);
Expand Down Expand Up @@ -1689,8 +1698,7 @@ public void succeeded()
Callback.from(() -> httpChannelState._handlerInvoker.failed(failure),
x ->
{
if (ExceptionUtil.areNotAssociated(failure, x))
failure.addSuppressed(x);
ExceptionUtil.addSuppressedIfNotAssociated(failure, x);
httpChannelState._handlerInvoker.failed(failure);
}));
}
Expand Down Expand Up @@ -1758,8 +1766,7 @@ else if (error == null)
}
catch (Throwable t)
{
if (ExceptionUtil.areNotAssociated(failure, t))
failure.addSuppressed(t);
ExceptionUtil.addSuppressedIfNotAssociated(failure, t);
super.onError(task, failure);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -478,8 +478,7 @@ public void failed(Throwable x)
}
catch (Throwable t)
{
if (ExceptionUtil.areNotAssociated(x, t))
x.addSuppressed(t);
ExceptionUtil.addSuppressedIfNotAssociated(x, t);
}
finally
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
*/
public class ExceptionUtil
{

/**
* <p>Convert a {@link Throwable} to a specific type by casting or construction on a new instance.</p>
*
Expand Down Expand Up @@ -178,6 +179,18 @@ public static boolean areNotAssociated(Throwable t1, Throwable t2)
return true;
}

/**
* Add a suppressed exception if it is not associated.
* @see #areNotAssociated(Throwable, Throwable)
* @param throwable The main Throwable
* @param suppressed The Throwable to suppress if it is not associated.
*/
public static void addSuppressedIfNotAssociated(Throwable throwable, Throwable suppressed)
{
if (areNotAssociated(throwable, suppressed))
throwable.addSuppressed(suppressed);
}

/**
* Decorate a Throwable with the suppressed errors and return it.
* @param t the throwable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,6 @@ public void reset()
_state = null;
}

public ServletChannelState getServletChannelState()
{
return state();
}

public static class WrappedAsyncListener implements AsyncListener
{
private final AsyncListener _listener;
Expand Down
Loading