From 2618c2a649772624bb6011e5e63455c3c81be0b0 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Mon, 23 May 2016 19:01:15 +0200 Subject: [PATCH] [#5239] Allow adding handlers to pipeline with null name. Motivation: While doing 8fe3c83e4ca9a64c03f5adcb9f056d9e9440a389 I made a change which disallowed using null as name for handlers in the pipeline (this generated a new name before). Modifications: Revert to old behaviour and adding test case. Result: Allow null name again --- .../io/netty/channel/ChannelPipeline.java | 22 ++++++------ .../netty/channel/DefaultChannelPipeline.java | 35 ++++++++----------- .../channel/DefaultChannelPipelineTest.java | 14 ++++++++ 3 files changed, 40 insertions(+), 31 deletions(-) diff --git a/transport/src/main/java/io/netty/channel/ChannelPipeline.java b/transport/src/main/java/io/netty/channel/ChannelPipeline.java index ae010edf2a..2752216b55 100644 --- a/transport/src/main/java/io/netty/channel/ChannelPipeline.java +++ b/transport/src/main/java/io/netty/channel/ChannelPipeline.java @@ -225,7 +225,7 @@ public interface ChannelPipeline * @throws IllegalArgumentException * if there's an entry with the same name already in the pipeline * @throws NullPointerException - * if the specified name or handler is {@code null} + * if the specified handler is {@code null} */ ChannelPipeline addFirst(String name, ChannelHandler handler); @@ -240,7 +240,7 @@ public interface ChannelPipeline * @throws IllegalArgumentException * if there's an entry with the same name already in the pipeline * @throws NullPointerException - * if the specified name or handler is {@code null} + * if the specified handler is {@code null} */ ChannelPipeline addFirst(EventExecutorGroup group, String name, ChannelHandler handler); @@ -253,7 +253,7 @@ public interface ChannelPipeline * @throws IllegalArgumentException * if there's an entry with the same name already in the pipeline * @throws NullPointerException - * if the specified name or handler is {@code null} + * if the specified handler is {@code null} */ ChannelPipeline addLast(String name, ChannelHandler handler); @@ -268,7 +268,7 @@ public interface ChannelPipeline * @throws IllegalArgumentException * if there's an entry with the same name already in the pipeline * @throws NullPointerException - * if the specified name or handler is {@code null} + * if the specified handler is {@code null} */ ChannelPipeline addLast(EventExecutorGroup group, String name, ChannelHandler handler); @@ -285,7 +285,7 @@ public interface ChannelPipeline * @throws IllegalArgumentException * if there's an entry with the same name already in the pipeline * @throws NullPointerException - * if the specified baseName, name, or handler is {@code null} + * if the specified baseName or handler is {@code null} */ ChannelPipeline addBefore(String baseName, String name, ChannelHandler handler); @@ -304,7 +304,7 @@ public interface ChannelPipeline * @throws IllegalArgumentException * if there's an entry with the same name already in the pipeline * @throws NullPointerException - * if the specified baseName, name, or handler is {@code null} + * if the specified baseName or handler is {@code null} */ ChannelPipeline addBefore(EventExecutorGroup group, String baseName, String name, ChannelHandler handler); @@ -321,7 +321,7 @@ public interface ChannelPipeline * @throws IllegalArgumentException * if there's an entry with the same name already in the pipeline * @throws NullPointerException - * if the specified baseName, name, or handler is {@code null} + * if the specified baseName or handler is {@code null} */ ChannelPipeline addAfter(String baseName, String name, ChannelHandler handler); @@ -340,7 +340,7 @@ public interface ChannelPipeline * @throws IllegalArgumentException * if there's an entry with the same name already in the pipeline * @throws NullPointerException - * if the specified baseName, name, or handler is {@code null} + * if the specified baseName or handler is {@code null} */ ChannelPipeline addAfter(EventExecutorGroup group, String baseName, String name, ChannelHandler handler); @@ -456,7 +456,7 @@ public interface ChannelPipeline * if a handler with the specified new name already exists in this * pipeline, except for the handler to be replaced * @throws NullPointerException - * if the specified old handler, new name, or new handler is + * if the specified old handler or new handler is * {@code null} */ ChannelPipeline replace(ChannelHandler oldHandler, String newName, ChannelHandler newHandler); @@ -476,7 +476,7 @@ public interface ChannelPipeline * if a handler with the specified new name already exists in this * pipeline, except for the handler to be replaced * @throws NullPointerException - * if the specified old handler, new name, or new handler is + * if the specified old handler or new handler is * {@code null} */ ChannelHandler replace(String oldName, String newName, ChannelHandler newHandler); @@ -497,7 +497,7 @@ public interface ChannelPipeline * if a handler with the specified new name already exists in this * pipeline, except for the handler to be replaced * @throws NullPointerException - * if the specified old handler, new name, or new handler is + * if the specified old handler or new handler is * {@code null} */ T replace(Class oldHandlerType, String newName, diff --git a/transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java b/transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java index 69e54a2c83..54e0b28b15 100644 --- a/transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java +++ b/transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java @@ -145,12 +145,8 @@ public class DefaultChannelPipeline implements ChannelPipeline { final AbstractChannelHandlerContext newCtx; final EventExecutor executor; synchronized (this) { - if (name == null) { - name = generateName(handler); - } else { - checkDuplicateName(name); - } checkMultiplicity(handler); + name = filterName(name, handler); newCtx = newContext(group, name, handler); executor = executorSafe(newCtx.executor); @@ -197,14 +193,9 @@ public class DefaultChannelPipeline implements ChannelPipeline { final EventExecutor executor; final AbstractChannelHandlerContext newCtx; synchronized (this) { - if (name == null) { - name = generateName(handler); - } else { - checkDuplicateName(name); - } checkMultiplicity(handler); - newCtx = newContext(group, name, handler); + newCtx = newContext(group, filterName(name, handler), handler); executor = executorSafe(newCtx.executor); addLast0(newCtx); @@ -251,12 +242,8 @@ public class DefaultChannelPipeline implements ChannelPipeline { final AbstractChannelHandlerContext ctx; synchronized (this) { checkMultiplicity(handler); + name = filterName(name, handler); ctx = getContextOrDie(baseName); - if (name == null) { - name = generateName(handler); - } else { - checkDuplicateName(name); - } newCtx = newContext(group, name, handler); executor = executorSafe(newCtx.executor); @@ -292,6 +279,14 @@ public class DefaultChannelPipeline implements ChannelPipeline { ctx.prev = newCtx; } + private String filterName(String name, ChannelHandler handler) { + if (name == null) { + return generateName(handler); + } + checkDuplicateName(name); + return name; + } + @Override public final ChannelPipeline addAfter(String baseName, String name, ChannelHandler handler) { return addAfter(null, baseName, name, handler); @@ -299,15 +294,15 @@ public class DefaultChannelPipeline implements ChannelPipeline { @Override public final ChannelPipeline addAfter( - EventExecutorGroup group, String baseName, final String name, ChannelHandler handler) { + EventExecutorGroup group, String baseName, String name, ChannelHandler handler) { final EventExecutor executor; final AbstractChannelHandlerContext newCtx; final AbstractChannelHandlerContext ctx; synchronized (this) { checkMultiplicity(handler); + name = filterName(name, handler); ctx = getContextOrDie(baseName); - checkDuplicateName(name); newCtx = newContext(group, name, handler); executor = executorSafe(newCtx.executor); @@ -365,7 +360,7 @@ public class DefaultChannelPipeline implements ChannelPipeline { for (int i = size - 1; i >= 0; i --) { ChannelHandler h = handlers[i]; - addFirst(executor, generateName(h), h); + addFirst(executor, null, h); } return this; @@ -386,7 +381,7 @@ public class DefaultChannelPipeline implements ChannelPipeline { if (h == null) { break; } - addLast(executor, generateName(h), h); + addLast(executor, null, h); } return this; diff --git a/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java b/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java index 444fad5e99..a6466709a1 100644 --- a/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java +++ b/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java @@ -879,6 +879,20 @@ public class DefaultChannelPipelineTest { } } + @Test + public void testNullName() { + ChannelPipeline pipeline = new LocalChannel().pipeline(); + pipeline.addLast(newHandler()); + pipeline.addLast(null, newHandler()); + pipeline.addFirst(newHandler()); + pipeline.addFirst(null, newHandler()); + + pipeline.addLast("test", newHandler()); + pipeline.addAfter("test", null, newHandler()); + + pipeline.addBefore("test", null, newHandler()); + } + private static final class TestTask implements Runnable { private final ChannelPipeline pipeline;