Allow and skip null handlers when adding a vararg list of handlers (#10751)
Motivation: Allowing null handlers allows for more convenient idioms in conditionally adding handlers, e.g., ch.pipeline().addLast( new FooHandler(), condition ? new BarHandler() : null, new BazHandler() ); Modifications: * Change addFirst(..) and addLast(..) to skip null handlers, rather than break or short-circuit. * Add new unit tests. Result: * Makes addFirst(..) and addLast(..) behavior more consistent * Resolves https://github.com/netty/netty/issues/10728
This commit is contained in:
parent
7e1147ea4f
commit
3b90b536bb
@ -342,20 +342,12 @@ public class DefaultChannelPipeline implements ChannelPipeline {
|
|||||||
@Override
|
@Override
|
||||||
public final ChannelPipeline addFirst(EventExecutorGroup executor, ChannelHandler... handlers) {
|
public final ChannelPipeline addFirst(EventExecutorGroup executor, ChannelHandler... handlers) {
|
||||||
ObjectUtil.checkNotNull(handlers, "handlers");
|
ObjectUtil.checkNotNull(handlers, "handlers");
|
||||||
if (handlers.length == 0 || handlers[0] == null) {
|
|
||||||
return this;
|
|
||||||
}
|
|
||||||
|
|
||||||
int size;
|
for (int i = handlers.length - 1; i >= 0; i--) {
|
||||||
for (size = 1; size < handlers.length; size ++) {
|
|
||||||
if (handlers[size] == null) {
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
for (int i = size - 1; i >= 0; i --) {
|
|
||||||
ChannelHandler h = handlers[i];
|
ChannelHandler h = handlers[i];
|
||||||
addFirst(executor, null, h);
|
if (h != null) {
|
||||||
|
addFirst(executor, null, h);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return this;
|
return this;
|
||||||
@ -374,11 +366,10 @@ public class DefaultChannelPipeline implements ChannelPipeline {
|
|||||||
public final ChannelPipeline addLast(EventExecutorGroup executor, ChannelHandler... handlers) {
|
public final ChannelPipeline addLast(EventExecutorGroup executor, ChannelHandler... handlers) {
|
||||||
ObjectUtil.checkNotNull(handlers, "handlers");
|
ObjectUtil.checkNotNull(handlers, "handlers");
|
||||||
|
|
||||||
for (ChannelHandler h: handlers) {
|
for (ChannelHandler h : handlers) {
|
||||||
if (h == null) {
|
if (h != null) {
|
||||||
break;
|
addLast(executor, null, h);
|
||||||
}
|
}
|
||||||
addLast(executor, null, h);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
return this;
|
return this;
|
||||||
|
@ -171,6 +171,44 @@ public class DefaultChannelPipelineTest {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testAddLastVarArgsSkipsNull() {
|
||||||
|
ChannelPipeline pipeline = new LocalChannel().pipeline();
|
||||||
|
assertEquals(1, pipeline.names().size());
|
||||||
|
|
||||||
|
pipeline.addLast((ChannelHandler) null, newHandler(), null);
|
||||||
|
assertEquals(2, pipeline.names().size());
|
||||||
|
assertEquals("DefaultChannelPipelineTest$TestHandler#0", pipeline.names().get(0));
|
||||||
|
|
||||||
|
pipeline.addLast(newHandler(), null, newHandler());
|
||||||
|
assertEquals(4, pipeline.names().size());
|
||||||
|
assertEquals("DefaultChannelPipelineTest$TestHandler#0", pipeline.names().get(0));
|
||||||
|
assertEquals("DefaultChannelPipelineTest$TestHandler#1", pipeline.names().get(1));
|
||||||
|
assertEquals("DefaultChannelPipelineTest$TestHandler#2", pipeline.names().get(2));
|
||||||
|
|
||||||
|
pipeline.addLast((ChannelHandler) null);
|
||||||
|
assertEquals(4, pipeline.names().size());
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testAddFirstVarArgsSkipsNull() {
|
||||||
|
ChannelPipeline pipeline = new LocalChannel().pipeline();
|
||||||
|
assertEquals(1, pipeline.names().size());
|
||||||
|
|
||||||
|
pipeline.addFirst((ChannelHandler) null, newHandler(), null);
|
||||||
|
assertEquals(2, pipeline.names().size());
|
||||||
|
assertEquals("DefaultChannelPipelineTest$TestHandler#0", pipeline.names().get(0));
|
||||||
|
|
||||||
|
pipeline.addFirst(newHandler(), null, newHandler());
|
||||||
|
assertEquals(4, pipeline.names().size());
|
||||||
|
assertEquals("DefaultChannelPipelineTest$TestHandler#2", pipeline.names().get(0));
|
||||||
|
assertEquals("DefaultChannelPipelineTest$TestHandler#1", pipeline.names().get(1));
|
||||||
|
assertEquals("DefaultChannelPipelineTest$TestHandler#0", pipeline.names().get(2));
|
||||||
|
|
||||||
|
pipeline.addFirst((ChannelHandler) null);
|
||||||
|
assertEquals(4, pipeline.names().size());
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testRemoveChannelHandler() {
|
public void testRemoveChannelHandler() {
|
||||||
ChannelPipeline pipeline = new LocalChannel().pipeline();
|
ChannelPipeline pipeline = new LocalChannel().pipeline();
|
||||||
|
Loading…
Reference in New Issue
Block a user