Skip to content

Commit

Permalink
#7281 improve isSpecial javadoc
Browse files Browse the repository at this point in the history 
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
  • Loading branch information
@lorban
lorban committed Jan 13, 2022
1 parent 5cad97e commit 0fb3079
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 62 deletions.
53 changes: 27 additions & 26 deletions jetty-server/src/main/java/org/eclipse/jetty/server/AsyncContentProducer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -324,33 +324,35 @@ private HttpInput.Content nextTransformedContent()

while (true)
{
if (_transformedContent != null && (_transformedContent.isSpecial() || !_transformedContent.isEmpty()))
if (_transformedContent != null)
{
if (_transformedContent.isSpecial() && !_error)
if (_transformedContent.isSpecial() || !_transformedContent.isEmpty())
{
// In case the _rawContent was set by consumeAll(), check the httpChannel
// to see if it has a more precise error. Otherwise, the exact same
// special content will be returned by the httpChannel; do not do that
// if the _error flag was set, meaning the current error is definitive.
HttpInput.Content refreshedRawContent = produceRawContent();
if (refreshedRawContent != null)
_rawContent = _transformedContent = refreshedRawContent;
_error = _rawContent.getError() != null;
if (_transformedContent.isSpecial() && !_error)
{
// In case the _rawContent was set by consumeAll(), check the httpChannel
// to see if it has a more precise error. Otherwise, the exact same
// special content will be returned by the httpChannel; do not do that
// if the _error flag was set, meaning the current error is definitive.
HttpInput.Content refreshedRawContent = produceRawContent();
if (refreshedRawContent != null)
_rawContent = _transformedContent = refreshedRawContent;
_error = _rawContent.getError() != null;
if (LOG.isDebugEnabled())
LOG.debug("refreshed raw content: {} {}", _rawContent, this);
}

if (LOG.isDebugEnabled())
LOG.debug("refreshed raw content: {} {}", _rawContent, this);
LOG.debug("transformed content not yet depleted, returning it {}", this);
return _transformedContent;
}
else
{
if (LOG.isDebugEnabled())
LOG.debug("current transformed content depleted {}", this);
_transformedContent.succeeded();
_transformedContent = null;
}

if (LOG.isDebugEnabled())
LOG.debug("transformed content not yet depleted, returning it {}", this);
return _transformedContent;
}

if (_transformedContent != null && _transformedContent.isEmpty() && !_transformedContent.isSpecial())
{
if (LOG.isDebugEnabled())
LOG.debug("current transformed content depleted {}", this);
_transformedContent.succeeded();
_transformedContent = null;
}

if (_rawContent == null)
Expand Down Expand Up @@ -390,7 +392,7 @@ private void transformRawContent()
return;
}

// If the interceptor generated a null content, recycle the raw content now if its empty.
// If the interceptor generated a null content, recycle the raw content now if it is empty.
if (_transformedContent == null && _rawContent.isEmpty() && !_rawContent.isSpecial())
{
if (LOG.isDebugEnabled())
Expand All @@ -400,14 +402,13 @@ private void transformRawContent()
return;
}

// If the interceptor returned the raw content, recycle the raw content now if its empty.
// If the interceptor returned the raw content, recycle the raw content now if it is empty.
if (_transformedContent == _rawContent && _rawContent.isEmpty() && !_rawContent.isSpecial())
{
if (LOG.isDebugEnabled())
LOG.debug("interceptor returned the raw content, recycle the empty raw content now {}", this);
_rawContent.succeeded();
_rawContent = _transformedContent = null;
return;
}
}
else
Expand Down
10 changes: 6 additions & 4 deletions jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ public String toString()
* When {@link Content} instances are generated, they are passed to the registered interceptor (if any)
* that is then responsible for providing the actual content that is consumed by {@link #read(byte[], int, int)} and its
* sibling methods.</p>
* <p>A minimal implementation could be as simple as:
* A minimal implementation could be as simple as:
* <pre>
* public HttpInput.Content readFrom(HttpInput.Content content)
* {
Expand Down Expand Up @@ -453,11 +453,11 @@ public String toString()
* occur only after the contained byte buffer is empty (see above) or at any time if the returned content was special.</li>
* <li>Once {@link #readFrom(Content)} returned a special content, subsequent calls to {@link #readFrom(Content)} must
* always return the same special content.</li>
* <li>When {@link #readFrom(Content)} gets passed a non-special content, it must either return the content it was
* passed or fully consume the contained byte buffer.</li>
* <li>Implementations implementing both this interface and {@link Destroyable} will have their
* {@link Destroyable#destroy()} method called when {@link #recycle()} is called.</li>
* </ul>
* </p>
* <p></p>
* @see org.eclipse.jetty.server.handler.gzip.GzipHttpInputInterceptor
*/
public interface Interceptor
Expand Down Expand Up @@ -612,7 +612,9 @@ public boolean isEmpty()
}

/**
* Check if the content is special.
* Check if the content is special. A content is deemed special
* if it does not hold bytes but rather conveys a special event,
* like when EOF has been reached or an error has occurred.
* @return true if the content is special, false otherwise.
*/
public boolean isSpecial()
Expand Down
64 changes: 32 additions & 32 deletions jetty-server/src/test/java/org/eclipse/jetty/server/BlockingContentProducerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,14 +136,14 @@ public void succeeded()
contentProducer.setInterceptor(content -> new HttpInput.ErrorContent(new Throwable("testBlockingContentProducerInterceptorGeneratesError interceptor error")));

HttpInput.Content content1 = contentProducer.nextContent();
assertThat(content1.isSpecial(), Matchers.is(true));
assertThat(content1.getError().getMessage(), Matchers.is("testBlockingContentProducerInterceptorGeneratesError interceptor error"));
assertThat(content1.isSpecial(), is(true));
assertThat(content1.getError().getMessage(), is("testBlockingContentProducerInterceptorGeneratesError interceptor error"));

HttpInput.Content content2 = contentProducer.nextContent();
assertThat(content2.isSpecial(), Matchers.is(true));
assertThat(content2.getError().getMessage(), Matchers.is("testBlockingContentProducerInterceptorGeneratesError interceptor error"));
assertThat(content2.isSpecial(), is(true));
assertThat(content2.getError().getMessage(), is("testBlockingContentProducerInterceptorGeneratesError interceptor error"));
}
assertThat(contentSucceededCount.get(), Matchers.is(1));
assertThat(contentSucceededCount.get(), is(1));
}

@Test
Expand All @@ -163,14 +163,14 @@ public void succeeded()
contentProducer.setInterceptor(content -> new HttpInput.EofContent());

HttpInput.Content content1 = contentProducer.nextContent();
assertThat(content1.isSpecial(), Matchers.is(true));
assertThat(content1.isEof(), Matchers.is(true));
assertThat(content1.isSpecial(), is(true));
assertThat(content1.isEof(), is(true));

HttpInput.Content content2 = contentProducer.nextContent();
assertThat(content2.isSpecial(), Matchers.is(true));
assertThat(content2.isEof(), Matchers.is(true));
assertThat(content2.isSpecial(), is(true));
assertThat(content2.isEof(), is(true));
}
assertThat(contentSucceededCount.get(), Matchers.is(1));
assertThat(contentSucceededCount.get(), is(1));
}

@Test
Expand All @@ -193,14 +193,14 @@ public void failed(Throwable x)
});

HttpInput.Content content1 = contentProducer.nextContent();
assertThat(content1.isSpecial(), Matchers.is(true));
assertThat(content1.getError().getCause().getMessage(), Matchers.is("testBlockingContentProducerInterceptorThrows error"));
assertThat(content1.isSpecial(), is(true));
assertThat(content1.getError().getCause().getMessage(), is("testBlockingContentProducerInterceptorThrows error"));

HttpInput.Content content2 = contentProducer.nextContent();
assertThat(content2.isSpecial(), Matchers.is(true));
assertThat(content1.getError().getCause().getMessage(), Matchers.is("testBlockingContentProducerInterceptorThrows error"));
assertThat(content2.isSpecial(), is(true));
assertThat(content1.getError().getCause().getMessage(), is("testBlockingContentProducerInterceptorThrows error"));
}
assertThat(contentFailedCount.get(), Matchers.is(1));
assertThat(contentFailedCount.get(), is(1));
}

@Test
Expand All @@ -226,16 +226,16 @@ public void testBlockingContentProducerEofContentIsPassedToInterceptor() throws
assertThat(error, nullValue());

HttpInput.Content lastContent = contentProducer.nextContent();
assertThat(lastContent.isSpecial(), Matchers.is(true));
assertThat(lastContent.isEof(), Matchers.is(true));
assertThat(lastContent.isSpecial(), is(true));
assertThat(lastContent.isEof(), is(true));
}

assertThat(interceptor.contents.size(), Matchers.is(4));
assertThat(interceptor.contents.get(0).isSpecial(), Matchers.is(false));
assertThat(interceptor.contents.get(1).isSpecial(), Matchers.is(false));
assertThat(interceptor.contents.get(2).isSpecial(), Matchers.is(false));
assertThat(interceptor.contents.get(3).isSpecial(), Matchers.is(true));
assertThat(interceptor.contents.get(3).isEof(), Matchers.is(true));
assertThat(interceptor.contents.size(), is(4));
assertThat(interceptor.contents.get(0).isSpecial(), is(false));
assertThat(interceptor.contents.get(1).isSpecial(), is(false));
assertThat(interceptor.contents.get(2).isSpecial(), is(false));
assertThat(interceptor.contents.get(3).isSpecial(), is(true));
assertThat(interceptor.contents.get(3).isEof(), is(true));
}

@Test
Expand All @@ -258,19 +258,19 @@ public void testBlockingContentProducerErrorContentIsPassedToInterceptor() throw
contentProducer.setInterceptor(interceptor);

Throwable error = readAndAssertContent(totalContentBytesCount, originalContentString, buffers.length + 1, contentProducer);
assertThat(error.getMessage(), Matchers.is("testBlockingContentProducerErrorContentIsPassedToInterceptor error"));
assertThat(error.getMessage(), is("testBlockingContentProducerErrorContentIsPassedToInterceptor error"));

HttpInput.Content lastContent = contentProducer.nextContent();
assertThat(lastContent.isSpecial(), Matchers.is(true));
assertThat(lastContent.getError().getMessage(), Matchers.is("testBlockingContentProducerErrorContentIsPassedToInterceptor error"));
assertThat(lastContent.isSpecial(), is(true));
assertThat(lastContent.getError().getMessage(), is("testBlockingContentProducerErrorContentIsPassedToInterceptor error"));
}

assertThat(interceptor.contents.size(), Matchers.is(4));
assertThat(interceptor.contents.get(0).isSpecial(), Matchers.is(false));
assertThat(interceptor.contents.get(1).isSpecial(), Matchers.is(false));
assertThat(interceptor.contents.get(2).isSpecial(), Matchers.is(false));
assertThat(interceptor.contents.get(3).isSpecial(), Matchers.is(true));
assertThat(interceptor.contents.get(3).getError().getMessage(), Matchers.is("testBlockingContentProducerErrorContentIsPassedToInterceptor error"));
assertThat(interceptor.contents.size(), is(4));
assertThat(interceptor.contents.get(0).isSpecial(), is(false));
assertThat(interceptor.contents.get(1).isSpecial(), is(false));
assertThat(interceptor.contents.get(2).isSpecial(), is(false));
assertThat(interceptor.contents.get(3).isSpecial(), is(true));
assertThat(interceptor.contents.get(3).getError().getMessage(), is("testBlockingContentProducerErrorContentIsPassedToInterceptor error"));
}

@Test
Expand Down

0 comments on commit 0fb3079

Please sign in to comment.