Skip to content

Commit

Permalink
[netty#5239] Allow adding handlers to pipeline with null name.
Browse files Browse the repository at this point in the history 
Motivation:

While doing 8fe3c83 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
  • Loading branch information
@normanmaurer
normanmaurer committed May 24, 2016
1 parent 26a175c commit 2618c2a
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 31 deletions.
22 changes: 11 additions & 11 deletions transport/src/main/java/io/netty/channel/ChannelPipeline.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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 extends ChannelHandler> T replace(Class<T> oldHandlerType, String newName,
Expand Down
35 changes: 15 additions & 20 deletions transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,12 +145,8 @@ public final ChannelPipeline addFirst(EventExecutorGroup group, String name, Cha
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);
Expand Down Expand Up @@ -197,14 +193,9 @@ public final ChannelPipeline addLast(EventExecutorGroup group, String name, Chan
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);
Expand Down Expand Up @@ -251,12 +242,8 @@ public final ChannelPipeline addBefore(
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);
Expand Down Expand Up @@ -292,22 +279,30 @@ private static void addBefore0(AbstractChannelHandlerContext ctx, AbstractChanne
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);
}

@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);
Expand Down Expand Up @@ -365,7 +360,7 @@ public final ChannelPipeline addFirst(EventExecutorGroup executor, ChannelHandle

for (int i = size - 1; i >= 0; i --) {
ChannelHandler h = handlers[i];
addFirst(executor, generateName(h), h);
addFirst(executor, null, h);
}

return this;
Expand All @@ -386,7 +381,7 @@ public final ChannelPipeline addLast(EventExecutorGroup executor, ChannelHandler
if (h == null) {
break;
}
addLast(executor, generateName(h), h);
addLast(executor, null, h);
}

return this;
Expand Down
14 changes: 14 additions & 0 deletions transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -879,6 +879,20 @@ public void testAddBefore() throws Throwable {
}
}

@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;
Expand Down

0 comments on commit 2618c2a

Please sign in to comment.