AlbumenJ commented on code in PR #14356:
URL: https://github.com/apache/dubbo/pull/14356#discussion_r1650705153


##########
dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/state/AbstractStateRouter.java:
##########
@@ -125,7 +125,7 @@ public final BitList<Invoker<T>> route(
 
         routeResult = doRoute(invokers, url, invocation, needToPrintMessage, 
nodeHolder, messageHolder);
         if (routeResult != invokers) {
-            routeResult = invokers.and(routeResult);
+            routeResult = routeResult.and(invokers);

Review Comment:
   Do not change it.



##########
dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/condition/MultiDestConditionRouter.java:
##########
@@ -0,0 +1,409 @@
+/*
+ * 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.dubbo.rpc.cluster.router.condition;
+
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.common.logger.ErrorTypeAwareLogger;
+import org.apache.dubbo.common.logger.LoggerFactory;
+import org.apache.dubbo.common.utils.CollectionUtils;
+import org.apache.dubbo.common.utils.Holder;
+import org.apache.dubbo.common.utils.NetUtils;
+import org.apache.dubbo.common.utils.StringUtils;
+import org.apache.dubbo.rpc.Invocation;
+import org.apache.dubbo.rpc.Invoker;
+import org.apache.dubbo.rpc.RpcException;
+import org.apache.dubbo.rpc.cluster.router.RouterSnapshotNode;
+import 
org.apache.dubbo.rpc.cluster.router.condition.config.model.ConditionSubSet;
+import 
org.apache.dubbo.rpc.cluster.router.condition.config.model.DestinationSet;
+import 
org.apache.dubbo.rpc.cluster.router.condition.config.model.MultiDestCondition;
+import org.apache.dubbo.rpc.cluster.router.condition.matcher.ConditionMatcher;
+import 
org.apache.dubbo.rpc.cluster.router.condition.matcher.ConditionMatcherFactory;
+import org.apache.dubbo.rpc.cluster.router.state.AbstractStateRouter;
+import org.apache.dubbo.rpc.cluster.router.state.BitList;
+
+import java.text.ParseException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import static 
org.apache.dubbo.common.constants.LoggerCodeConstants.CLUSTER_CONDITIONAL_ROUTE_LIST_EMPTY;
+import static 
org.apache.dubbo.common.constants.LoggerCodeConstants.CLUSTER_FAILED_EXEC_CONDITION_ROUTER;
+import static 
org.apache.dubbo.rpc.cluster.Constants.DefaultRouteConditionSubSetWeight;
+import static org.apache.dubbo.rpc.cluster.Constants.RULE_KEY;
+
+public class MultiDestConditionRouter<T> extends AbstractStateRouter<T> {
+    public static final String NAME = "multi_condition";
+
+    private static final ErrorTypeAwareLogger logger = 
LoggerFactory.getErrorTypeAwareLogger(AbstractStateRouter.class);
+    protected static final Pattern ROUTE_PATTERN = 
Pattern.compile("([&!=,]*)\\s*([^&!=,\\s]+)");
+    private Map<String, ConditionMatcher> whenCondition;
+    private boolean trafficDisable;
+    private List<ConditionSubSet> thenCondition;
+    private int ratio;
+    private int priority;
+    private boolean force;
+    protected List<ConditionMatcherFactory> matcherFactories;
+    private boolean enabled;
+
+    public MultiDestConditionRouter(URL url, MultiDestCondition 
multiDestCondition, boolean enabled) {
+        super(url);
+        this.enabled = enabled;
+        matcherFactories =
+                
moduleModel.getExtensionLoader(ConditionMatcherFactory.class).getActivateExtensions();
+        this.covert(multiDestCondition, this);
+        this.init(multiDestCondition.getFrom(), multiDestCondition.getTo());
+    }
+
+    public void init(Map<String, String> from, List<Map<String, String>> to) {
+        try {
+            if (from == null || to == null) {
+                throw new IllegalArgumentException("Illegal route rule!");
+            }
+            String whenRule = from.get("match");
+            Map<String, ConditionMatcher> when =
+                    StringUtils.isBlank(whenRule) || "true".equals(whenRule) ? 
new HashMap<>() : parseRule(whenRule);
+            this.whenCondition = when;
+
+            List<ConditionSubSet> thenConditions = new ArrayList<>();
+            for (Map<String, String> toMap : to) {
+                String thenRule = toMap.get("match");
+                Map<String, ConditionMatcher> then = 
StringUtils.isBlank(thenRule) || "false".equals(thenRule)
+                        ? new HashMap<>()
+                        : parseRule(thenRule);
+                // NOTE: It should be determined on the business level whether 
the `When condition` can be empty or not.
+
+                thenConditions.add(new ConditionSubSet(
+                        then,
+                        Integer.valueOf(
+                                toMap.getOrDefault("weight", 
String.valueOf(DefaultRouteConditionSubSetWeight)))));
+            }
+            this.thenCondition = thenConditions;
+        } catch (ParseException e) {
+            throw new IllegalStateException(e.getMessage(), e);
+        }
+    }
+
+    private Map<String, ConditionMatcher> parseRule(String rule) throws 
ParseException {
+        Map<String, ConditionMatcher> condition = new HashMap<>();
+        if (StringUtils.isBlank(rule)) {
+            return condition;
+        }
+        // Key-Value pair, stores both match and mismatch conditions
+        ConditionMatcher matcherPair = null;
+        // Multiple values
+        Set<String> values = null;
+        final Matcher matcher = ROUTE_PATTERN.matcher(rule);
+        while (matcher.find()) { // Try to match one by one
+            String separator = matcher.group(1);
+            String content = matcher.group(2);
+            // Start part of the condition expression.
+            if (StringUtils.isEmpty(separator)) {
+                matcherPair = this.getMatcher(content);
+                condition.put(content, matcherPair);
+            }
+            // The KV part of the condition expression
+            else if ("&".equals(separator)) {
+                if (condition.get(content) == null) {
+                    matcherPair = this.getMatcher(content);
+                    condition.put(content, matcherPair);
+                } else {
+                    matcherPair = condition.get(content);
+                }
+            }
+            // The Value in the KV part.
+            else if ("=".equals(separator)) {
+                if (matcherPair == null) {
+                    throw new ParseException(
+                            "Illegal route rule \"" + rule + "\", The error 
char '" + separator + "' at index "
+                                    + matcher.start() + " before \"" + content 
+ "\".",
+                            matcher.start());
+                }
+
+                values = matcherPair.getMatches();
+                values.add(content);
+            }
+            // The Value in the KV part.
+            else if ("!=".equals(separator)) {
+                if (matcherPair == null) {
+                    throw new ParseException(
+                            "Illegal route rule \"" + rule + "\", The error 
char '" + separator + "' at index "
+                                    + matcher.start() + " before \"" + content 
+ "\".",
+                            matcher.start());
+                }
+
+                values = matcherPair.getMismatches();
+                values.add(content);
+            }
+            // The Value in the KV part, if Value have more than one items.
+            else if (",".equals(separator)) { // Should be separated by ','
+                if (values == null || values.isEmpty()) {
+                    throw new ParseException(
+                            "Illegal route rule \"" + rule + "\", The error 
char '" + separator + "' at index "
+                                    + matcher.start() + " before \"" + content 
+ "\".",
+                            matcher.start());
+                }
+                values.add(content);
+            } else {
+                throw new ParseException(
+                        "Illegal route rule \"" + rule + "\", The error char 
'" + separator + "' at index "
+                                + matcher.start() + " before \"" + content + 
"\".",
+                        matcher.start());
+            }
+        }
+        return condition;
+    }
+
+    private ConditionMatcher getMatcher(String key) {
+        for (ConditionMatcherFactory factory : matcherFactories) {
+            if (factory.shouldMatch(key)) {
+                return factory.createMatcher(key, moduleModel);
+            }
+        }
+        return moduleModel
+                .getExtensionLoader(ConditionMatcherFactory.class)
+                .getExtension("param")

Review Comment:
   Hard code here. If there only exist **one** implementation, use it directly



##########
dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/condition/config/ListenableStateRouter.java:
##########
@@ -112,18 +122,45 @@ public BitList<Invoker<T>> doRoute(
         if (needToPrintMessage) {
             resultMessage = new StringBuilder();
         }
-        for (AbstractStateRouter<T> router : conditionRouters) {
-            invokers = router.route(invokers, url, invocation, 
needToPrintMessage, nodeHolder);
-            if (needToPrintMessage) {
-                resultMessage.append(messageHolder.get());
+
+        BitList<Invoker<T>> routeResult = invokers;
+        if (routerRule instanceof MultiDestConditionRouterRule
+                || routerRule.getVersion() != null && 
routerRule.getVersion().startsWith(RULE_VERSION_V31)) {
+            boolean trafficDisable = false;
+            for (MultiDestConditionRouter<T> multiDestConditionRouter : 
multiDestConditionRouters) {
+                routeResult = multiDestConditionRouter.route(invokers, url, 
invocation, needToPrintMessage, nodeHolder);
+                if (invokers == routeResult) {
+                    //                    not match or disable to continue 
next multiDestConditionRouter
+                    continue;
+                } else if (routeResult.size() == 0
+                        && !multiDestConditionRouter.isTrafficDisable()
+                        && !multiDestConditionRouter.isForce()) {
+                    //                    empty but can continue to next 
multiDestConditionRouter
+                    continue;
+                } else {
+                    trafficDisable = 
multiDestConditionRouter.isTrafficDisable();
+                    break;
+                }
+            }
+            //            if trafficDisable ignore root.force
+            if (routeResult.size() == 0 && !routerRule.isForce() && 
!trafficDisable) {
+                routeResult = invokers;
+            }
+        } else {
+            for (AbstractStateRouter<T> router : conditionRouters) {
+                routeResult = router.route(routeResult, url, invocation, 
needToPrintMessage, nodeHolder);
             }
         }
 
+        if (needToPrintMessage) {
+            resultMessage.append(messageHolder.get());
+        }
+
         if (needToPrintMessage) {
             messageHolder.set(resultMessage.toString());
         }

Review Comment:
   Is it correct?



##########
dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/condition/config/ListenableStateRouter.java:
##########
@@ -112,18 +122,45 @@ public BitList<Invoker<T>> doRoute(
         if (needToPrintMessage) {
             resultMessage = new StringBuilder();
         }
-        for (AbstractStateRouter<T> router : conditionRouters) {
-            invokers = router.route(invokers, url, invocation, 
needToPrintMessage, nodeHolder);
-            if (needToPrintMessage) {
-                resultMessage.append(messageHolder.get());
+
+        BitList<Invoker<T>> routeResult = invokers;
+        if (routerRule instanceof MultiDestConditionRouterRule
+                || routerRule.getVersion() != null && 
routerRule.getVersion().startsWith(RULE_VERSION_V31)) {
+            boolean trafficDisable = false;
+            for (MultiDestConditionRouter<T> multiDestConditionRouter : 
multiDestConditionRouters) {
+                routeResult = multiDestConditionRouter.route(invokers, url, 
invocation, needToPrintMessage, nodeHolder);

Review Comment:
   Should clone `invokers` here first



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to