[ https://issues.apache.org/jira/browse/FLINK-9386?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16487264#comment-16487264 ]
ASF GitHub Bot commented on FLINK-9386: --------------------------------------- Github user uce commented on a diff in the pull request: https://github.com/apache/flink/pull/6031#discussion_r190214988 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/rest/handler/router/RouterHandler.java --- @@ -0,0 +1,109 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.runtime.rest.handler.router; + +import org.apache.flink.runtime.rest.handler.AbstractRestHandler; +import org.apache.flink.runtime.rest.handler.util.HandlerUtils; +import org.apache.flink.runtime.rest.messages.ErrorResponseBody; + +import org.apache.flink.shaded.netty4.io.netty.channel.ChannelHandler; +import org.apache.flink.shaded.netty4.io.netty.channel.ChannelHandlerContext; +import org.apache.flink.shaded.netty4.io.netty.channel.ChannelInboundHandler; +import org.apache.flink.shaded.netty4.io.netty.channel.ChannelPipeline; +import org.apache.flink.shaded.netty4.io.netty.channel.SimpleChannelInboundHandler; +import org.apache.flink.shaded.netty4.io.netty.handler.codec.http.DefaultFullHttpResponse; +import org.apache.flink.shaded.netty4.io.netty.handler.codec.http.HttpHeaders; +import org.apache.flink.shaded.netty4.io.netty.handler.codec.http.HttpMethod; +import org.apache.flink.shaded.netty4.io.netty.handler.codec.http.HttpRequest; +import org.apache.flink.shaded.netty4.io.netty.handler.codec.http.HttpResponseStatus; +import org.apache.flink.shaded.netty4.io.netty.handler.codec.http.HttpVersion; +import org.apache.flink.shaded.netty4.io.netty.handler.codec.http.QueryStringDecoder; + +import java.util.Map; + +import static java.util.Objects.requireNonNull; + +/** + * This class replaces the standard error response to be identical with those sent by the {@link AbstractRestHandler}. + */ +public class RouterHandler extends SimpleChannelInboundHandler<HttpRequest> { + public static final String ROUTER_HANDLER_NAME = RouterHandler.class.getName() + "_ROUTER_HANDLER"; + public static final String ROUTED_HANDLER_NAME = RouterHandler.class.getName() + "_ROUTED_HANDLER"; + + private final Map<String, String> responseHeaders; + private final Router router; + + public RouterHandler(Router router, final Map<String, String> responseHeaders) { + this.router = requireNonNull(router); + this.responseHeaders = requireNonNull(responseHeaders); + } + + public String getName() { + return ROUTER_HANDLER_NAME; + } + + @Override + protected void channelRead0(ChannelHandlerContext channelHandlerContext, HttpRequest httpRequest) throws Exception { + if (HttpHeaders.is100ContinueExpected(httpRequest)) { + channelHandlerContext.writeAndFlush(new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.CONTINUE)); + return; + } + + // Route + HttpMethod method = httpRequest.getMethod(); + QueryStringDecoder qsd = new QueryStringDecoder(httpRequest.getUri()); + RouteResult routeResult = router.route(method, qsd.path(), qsd.parameters()); + + if (routeResult == null) { + respondNotFound(channelHandlerContext, httpRequest); + return; + } + + routed(channelHandlerContext, routeResult, httpRequest); + } + + private void routed( + ChannelHandlerContext channelHandlerContext, + RouteResult routeResult, + HttpRequest httpRequest) throws Exception { + ChannelInboundHandler handler = (ChannelInboundHandler) routeResult.target(); + + // The handler may have been added (keep alive) + ChannelPipeline pipeline = channelHandlerContext.pipeline(); + ChannelHandler addedHandler = pipeline.get(ROUTED_HANDLER_NAME); + if (handler != addedHandler) { + if (addedHandler == null) { + pipeline.addAfter(ROUTER_HANDLER_NAME, ROUTED_HANDLER_NAME, handler); + } else { + pipeline.replace(addedHandler, ROUTED_HANDLER_NAME, handler); + } + } + + channelHandlerContext.fireChannelRead(new RoutedRequest(routeResult, httpRequest).retain()); --- End diff -- Do you think it makes sense to move `new RoutedRequest(routeResult, httpRequest).retain()` to a separate line? I was wondering whether it is retained as in the original handler, because I missed the `retain()` here. I find the following clearer, but feel free to ignore: ```java RoutedRequest request = new RoutedRequest(routeResult, httpRequest); request.retain(); channelHandlerContext.fireChannelRead(request); ``` > Remove netty-router dependency > ------------------------------ > > Key: FLINK-9386 > URL: https://issues.apache.org/jira/browse/FLINK-9386 > Project: Flink > Issue Type: Sub-task > Reporter: Piotr Nowojski > Assignee: Piotr Nowojski > Priority: Major > Fix For: 1.6.0 > > > netty-router 1.10 blocks upgrade to 4.1, while netty-router 2.2.0 has broken > compatibility in a way that it's unusable by us (it doesn't allow to sort > router paths as in https://issues.apache.org/jira/browse/FLINK-8000 ). I > propose to copy & simplify & modify netty-router code to suite our needs. -- This message was sent by Atlassian JIRA (v7.6.3#76005)