Repository: netty/netty
Issue: Proposal: add removeIfExists() method to ChannelPipeline

In some cases, I have to remove the handler from the pipeline that may exist or may not. Current Netty `remove()` implementation always throws `NoSuchElementException` that requires construction like this:

```
try {
    pipeline.remove(LetsEncryptHandler.class);
} catch (NoSuchElementException e) {
}
```

this could be simplified with:

`pipeline.removeIfExists(LetsEncryptHandler.class);`

WDYT?

Issue comments:
This would be very nice to have.
I've been using:
`if (pipeline.names.contains("someHandler")`

Fixed by https://github.com/netty/netty/pull/7663

Pull request review comments:
@doom369 I think this is not safe as in the meantime it could have been removed from another thread which would have this method throw a `NoSuchElementException`
@normanmaurer I don't see any place where NoSuchElementException could be thrown in above code.
ah never mind... this should work... 

Patch that resolved the issue:
diff --git a/transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java b/transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java
index e9d92a13970..0d3307a5351 100644
--- a/transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java
+++ b/transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java
@@ -457,6 +457,26 @@ public final <T extends ChannelHandler> T remove(Class<T> handlerType) {
         return (T) remove(getContextOrDie(handlerType)).handler();
     }
 
+    public final <T extends ChannelHandler> T removeIfExists(String name) {
+        return removeIfExists(context(name));
+    }
+
+    public final <T extends ChannelHandler> T removeIfExists(Class<T> handlerType) {
+        return removeIfExists(context(handlerType));
+    }
+
+    public final <T extends ChannelHandler> T removeIfExists(ChannelHandler handler) {
+        return removeIfExists(context(handler));
+    }
+
+    @SuppressWarnings("unchecked")
+    private <T extends ChannelHandler> T removeIfExists(ChannelHandlerContext ctx) {
+        if (ctx == null) {
+            return null;
+        }
+        return (T) remove((AbstractChannelHandlerContext) ctx).handler();
+    }
+
     private AbstractChannelHandlerContext remove(final AbstractChannelHandlerContext ctx) {
         assert ctx != head && ctx != tail;
 


Test patch:
diff --git a/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java b/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java
index 6071649b340..7cd51b2999f 100644
--- a/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java
+++ b/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java
@@ -48,6 +48,7 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.NoSuchElementException;
 import java.util.Queue;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.CountDownLatch;
@@ -184,6 +185,52 @@ public void testRemoveChannelHandler() {
         assertNull(pipeline.get("handler3"));
     }
 
+    @Test
+    public void testRemoveIfExists() {
+        DefaultChannelPipeline pipeline = new DefaultChannelPipeline(new LocalChannel());
+
+        ChannelHandler handler1 = newHandler();
+        ChannelHandler handler2 = newHandler();
+        ChannelHandler handler3 = newHandler();
+
+        pipeline.addLast("handler1", handler1);
+        pipeline.addLast("handler2", handler2);
+        pipeline.addLast("handler3", handler3);
+
+        assertNotNull(pipeline.removeIfExists(handler1));
+        assertNull(pipeline.get("handler1"));
+
+        assertNotNull(pipeline.removeIfExists("handler2"));
+        assertNull(pipeline.get("handler2"));
+
+        assertNotNull(pipeline.removeIfExists(TestHandler.class));
+        assertNull(pipeline.get("handler3"));
+    }
+
+    @Test
+    public void testRemoveIfExistsDoesNotThrowException() {
+        DefaultChannelPipeline pipeline = new DefaultChannelPipeline(new LocalChannel());
+
+        ChannelHandler handler1 = newHandler();
+        ChannelHandler handler2 = newHandler();
+        pipeline.addLast("handler1", handler1);
+
+        assertNull(pipeline.removeIfExists("handlerXXX"));
+        assertNull(pipeline.removeIfExists(handler2));
+        assertNull(pipeline.removeIfExists(ChannelOutboundHandlerAdapter.class));
+        assertNotNull(pipeline.get("handler1"));
+    }
+
+    @Test(expected = NoSuchElementException.class)
+    public void testRemoveThrowNoSuchElementException() {
+        DefaultChannelPipeline pipeline = new DefaultChannelPipeline(new LocalChannel());
+
+        ChannelHandler handler1 = newHandler();
+        pipeline.addLast("handler1", handler1);
+
+        pipeline.remove("handlerXXX");
+    }
+
     @Test
     public void testReplaceChannelHandler() {
         ChannelPipeline pipeline = new LocalChannel().pipeline();