You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by te...@apache.org on 2020/12/04 08:55:50 UTC
[shardingsphere-elasticjob] branch master updated: Revise Filter in
elasticjob-restful (#1762)
This is an automated email from the ASF dual-hosted git repository.
technoboy pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shardingsphere-elasticjob.git
The following commit(s) were added to refs/heads/master by this push:
new a416399 Revise Filter in elasticjob-restful (#1762)
a416399 is described below
commit a416399bbecf2775e81159eedbdc23b37a9388fe
Author: 吴伟杰 <wu...@apache.org>
AuthorDate: Fri Dec 4 16:55:41 2020 +0800
Revise Filter in elasticjob-restful (#1762)
---
.../shardingsphere/elasticjob/restful/Filter.java | 3 +-
.../restful/filter/DefaultFilterChain.java | 15 ++--
.../restful/filter/DefaultFilterChainTest.java | 79 ++++++++++++----------
3 files changed, 54 insertions(+), 43 deletions(-)
diff --git a/elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/Filter.java b/elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/Filter.java
index ed618ce..65fb24b 100644
--- a/elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/Filter.java
+++ b/elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/Filter.java
@@ -32,7 +32,6 @@ public interface Filter {
* @param httpRequest HTTP request
* @param httpResponse HTTP response
* @param filterChain filter chain
- * @return pass through the filter if true, else do response
*/
- boolean doFilter(FullHttpRequest httpRequest, FullHttpResponse httpResponse, FilterChain filterChain);
+ void doFilter(FullHttpRequest httpRequest, FullHttpResponse httpResponse, FilterChain filterChain);
}
diff --git a/elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/filter/DefaultFilterChain.java b/elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/filter/DefaultFilterChain.java
index 927ca4c..13f493a 100644
--- a/elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/filter/DefaultFilterChain.java
+++ b/elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/filter/DefaultFilterChain.java
@@ -39,7 +39,9 @@ public final class DefaultFilterChain implements FilterChain {
private int current;
- private boolean finished;
+ private boolean passedThrough;
+
+ private boolean replied;
public DefaultFilterChain(final List<Filter> filterInstances, final ChannelHandlerContext ctx, final HandleContext<?> handleContext) {
filters = filterInstances.toArray(new Filter[0]);
@@ -49,23 +51,22 @@ public final class DefaultFilterChain implements FilterChain {
@Override
public void next(final FullHttpRequest httpRequest) {
- Preconditions.checkState(!finished, "FilterChain has already finished.");
+ Preconditions.checkState(!passedThrough && !replied, "FilterChain has already finished.");
if (current < filters.length) {
- Filter currentFilter = filters[current++];
- boolean passThrough = currentFilter.doFilter(httpRequest, handleContext.getHttpResponse(), this);
- if (!passThrough) {
- finished = true;
+ filters[current++].doFilter(httpRequest, handleContext.getHttpResponse(), this);
+ if (!passedThrough && !replied) {
doResponse();
}
return;
}
- finished = true;
+ passedThrough = true;
ctx.fireChannelRead(handleContext);
}
private void doResponse() {
try {
ctx.writeAndFlush(handleContext.getHttpResponse());
+ replied = true;
} finally {
ReferenceCountUtil.release(handleContext.getHttpRequest());
}
diff --git a/elasticjob-infra/elasticjob-restful/src/test/java/org/apache/shardingsphere/elasticjob/restful/filter/DefaultFilterChainTest.java b/elasticjob-infra/elasticjob-restful/src/test/java/org/apache/shardingsphere/elasticjob/restful/filter/DefaultFilterChainTest.java
index 067fdd1..85a2586 100644
--- a/elasticjob-infra/elasticjob-restful/src/test/java/org/apache/shardingsphere/elasticjob/restful/filter/DefaultFilterChainTest.java
+++ b/elasticjob-infra/elasticjob-restful/src/test/java/org/apache/shardingsphere/elasticjob/restful/filter/DefaultFilterChainTest.java
@@ -37,9 +37,10 @@ import java.util.Collections;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
@RunWith(MockitoJUnitRunner.class)
public final class DefaultFilterChainTest {
@@ -53,15 +54,6 @@ public final class DefaultFilterChainTest {
@Mock
private FullHttpResponse httpResponse;
- @Mock
- private Filter firstFilter;
-
- @Mock
- private Filter secondFilter;
-
- @Mock
- private Filter thirdFilter;
-
private HandleContext<Handler> handleContext;
@Before
@@ -75,56 +67,61 @@ public final class DefaultFilterChainTest {
filterChain.next(httpRequest);
verify(ctx, never()).writeAndFlush(httpResponse);
verify(ctx).fireChannelRead(handleContext);
+ assertTrue(isPassedThrough(filterChain));
+ assertFalse(isReplied(filterChain));
}
@Test
public void assertWithSingleFilterPassed() {
- DefaultFilterChain filterChain = new DefaultFilterChain(Collections.singletonList(firstFilter), ctx, handleContext);
- when(firstFilter.doFilter(httpRequest, httpResponse, filterChain)).thenReturn(true);
- filterChain.next(httpRequest);
- verify(firstFilter).doFilter(httpRequest, httpResponse, filterChain);
+ Filter passableFilter = spy(new PassableFilter());
+ DefaultFilterChain filterChain = new DefaultFilterChain(Collections.singletonList(passableFilter), ctx, handleContext);
filterChain.next(httpRequest);
+ verify(passableFilter).doFilter(httpRequest, httpResponse, filterChain);
verify(ctx).fireChannelRead(handleContext);
verify(ctx, never()).writeAndFlush(httpResponse);
+ assertTrue(isPassedThrough(filterChain));
+ assertFalse(isReplied(filterChain));
}
@Test
public void assertWithSingleFilterDoResponse() {
- DefaultFilterChain filterChain = new DefaultFilterChain(Collections.singletonList(firstFilter), ctx, handleContext);
+ Filter impassableFilter = mock(Filter.class);
+ DefaultFilterChain filterChain = new DefaultFilterChain(Collections.singletonList(impassableFilter), ctx, handleContext);
filterChain.next(httpRequest);
- verify(firstFilter).doFilter(httpRequest, httpResponse, filterChain);
+ verify(impassableFilter).doFilter(httpRequest, httpResponse, filterChain);
verify(ctx, never()).fireChannelRead(any(HandleContext.class));
verify(ctx).writeAndFlush(httpResponse);
+ assertTrue(isReplied(filterChain));
+ assertFalse(isPassedThrough(filterChain));
}
@Test
public void assertWithThreeFiltersPassed() {
+ Filter firstFilter = spy(new PassableFilter());
+ Filter secondFilter = spy(new PassableFilter());
+ Filter thirdFilter = spy(new PassableFilter());
DefaultFilterChain filterChain = new DefaultFilterChain(Arrays.asList(firstFilter, secondFilter, thirdFilter), ctx, handleContext);
- when(firstFilter.doFilter(httpRequest, httpResponse, filterChain)).thenReturn(true);
filterChain.next(httpRequest);
verify(firstFilter).doFilter(httpRequest, httpResponse, filterChain);
- when(secondFilter.doFilter(httpRequest, httpResponse, filterChain)).thenReturn(true);
- filterChain.next(httpRequest);
verify(secondFilter).doFilter(httpRequest, httpResponse, filterChain);
- when(thirdFilter.doFilter(httpRequest, httpResponse, filterChain)).thenReturn(true);
- filterChain.next(httpRequest);
verify(thirdFilter).doFilter(httpRequest, httpResponse, filterChain);
- filterChain.next(httpRequest);
+ assertTrue(isPassedThrough(filterChain));
+ assertFalse(isReplied(filterChain));
verify(ctx).fireChannelRead(handleContext);
verify(ctx, never()).writeAndFlush(any(FullHttpResponse.class));
}
@Test
public void assertWithThreeFiltersDoResponseByTheSecond() {
+ Filter firstFilter = spy(new PassableFilter());
+ Filter secondFilter = mock(Filter.class);
+ Filter thirdFilter = spy(new PassableFilter());
DefaultFilterChain filterChain = new DefaultFilterChain(Arrays.asList(firstFilter, secondFilter, thirdFilter), ctx, handleContext);
- when(firstFilter.doFilter(httpRequest, httpResponse, filterChain)).thenReturn(true);
filterChain.next(httpRequest);
verify(firstFilter).doFilter(httpRequest, httpResponse, filterChain);
- when(secondFilter.doFilter(httpRequest, httpResponse, filterChain)).thenReturn(false);
- assertFalse(isFinished(filterChain));
- filterChain.next(httpRequest);
verify(secondFilter).doFilter(httpRequest, httpResponse, filterChain);
- assertTrue(isFinished(filterChain));
+ assertFalse(isPassedThrough(filterChain));
+ assertTrue(isReplied(filterChain));
verify(thirdFilter, never()).doFilter(httpRequest, httpResponse, filterChain);
verify(ctx, never()).fireChannelRead(any(HandleContext.class));
verify(ctx).writeAndFlush(httpResponse);
@@ -138,23 +135,37 @@ public final class DefaultFilterChainTest {
}
@Test(expected = IllegalStateException.class)
- public void assertInvokeFinishedFilterChainWithTwoFilters() {
+ public void assertInvokePassedThroughFilterChainWithTwoFilters() {
+ Filter firstFilter = spy(new PassableFilter());
+ Filter secondFilter = spy(new PassableFilter());
DefaultFilterChain filterChain = new DefaultFilterChain(Arrays.asList(firstFilter, secondFilter), ctx, handleContext);
- when(firstFilter.doFilter(httpRequest, httpResponse, filterChain)).thenReturn(true);
filterChain.next(httpRequest);
verify(firstFilter).doFilter(httpRequest, httpResponse, filterChain);
- when(secondFilter.doFilter(httpRequest, httpResponse, filterChain)).thenReturn(true);
- filterChain.next(httpRequest);
verify(secondFilter).doFilter(httpRequest, httpResponse, filterChain);
- filterChain.next(httpRequest);
verify(ctx).fireChannelRead(handleContext);
filterChain.next(httpRequest);
}
+ private boolean isPassedThrough(final DefaultFilterChain filterChain) {
+ return getBoolean(filterChain, "passedThrough");
+ }
+
+ private boolean isReplied(final DefaultFilterChain filterChain) {
+ return getBoolean(filterChain, "replied");
+ }
+
@SneakyThrows
- private boolean isFinished(final DefaultFilterChain filterChain) {
- Field field = DefaultFilterChain.class.getDeclaredField("finished");
+ private boolean getBoolean(final DefaultFilterChain filterChain, final String fieldName) {
+ Field field = DefaultFilterChain.class.getDeclaredField(fieldName);
field.setAccessible(true);
return (boolean) field.get(filterChain);
}
+
+ private static class PassableFilter implements Filter {
+
+ @Override
+ public void doFilter(final FullHttpRequest httpRequest, final FullHttpResponse httpResponse, final FilterChain filterChain) {
+ filterChain.next(httpRequest);
+ }
+ }
}