gnodet-bot commented on code in PR #29521:
URL: https://github.com/apache/camel/pull/29521#discussion_r4518814068


##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/ChaosHttpFeed.java:
##########
@@ -0,0 +1,332 @@
+/*
+ * 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.camel.dsl.jbang.core.common;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.TreeMap;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.camel.cli.connector.RepeatedSend;
+import org.apache.camel.dsl.jbang.core.commands.ai.HttpEndpoints;
+import org.apache.camel.util.json.JsonObject;
+
+/**
+ * The feed of a chaos experiment for a route that serves HTTP (CAMEL-26793): 
such a route starts from platform-http or
+ * the REST DSL, which has no producer, so it is fed with HTTP requests to the 
integration's own server (localhost and
+ * the port of its HTTP server, as the status has them). The experiment sends 
them while it waits for the verdict, and
+ * its traffic counts the status codes the clients got.
+ */
+public final class ChaosHttpFeed {
+
+    /** Where the requests go: the method and the URL on the integration's own 
server. */
+    public record Target(String method, String url) {
+
+        @Override
+        public String toString() {
+            return method + " " + url;
+        }
+    }
+
+    // a path to call, with an optional method: /orders/42, PUT /orders/42
+    private static final Pattern PATH
+            = 
Pattern.compile("(?:(GET|POST|PUT|PATCH|DELETE|HEAD|OPTIONS)\\s+)?(/\\S*)", 
Pattern.CASE_INSENSITIVE);
+
+    private ChaosHttpFeed() {
+    }
+
+    /** Whether the feed is a path to call, such as /orders/42 or PUT 
/orders/42, rather than a route or endpoint. */
+    public static boolean isPath(String feed) {
+        return feed != null && PATH.matcher(feed.strip()).matches();
+    }
+
+    /** Whether the route starts from an input that is fed over the HTTP 
server of Camel: platform-http or rest. */
+    static boolean servedByCamel(String from) {
+        return from != null && (from.startsWith("platform-http:") || 
from.startsWith("rest:"));
+    }
+
+    /**
+     * Where the requests of the feed go: for a path to call, or for a route 
that starts from platform-http or rest (by
+     * its id or endpoint); null when the feed is for another route.
+     *
+     * @throws IllegalArgumentException when the feed is for HTTP but cannot 
be sent, with the reason
+     */
+    public static Target target(JsonObject status, JsonObject route, String 
feed) {
+        Matcher m = PATH.matcher(feed.strip());
+        String method;
+        String path;
+        String routeId = null;
+        if (m.matches()) {
+            method = m.group(1);
+            path = m.group(2);
+        } else if (route != null && servedByCamel(route.getString("from"))) {
+            routeId = route.getString("routeId");
+            String from = route.getString("from");
+            method = method(from);
+            path = path(from);
+        } else {
+            return null;
+        }
+        HttpEndpoints.Served served = HttpEndpoints.fromStatus(status);
+        int port = served != null ? served.port() : 0;
+        if (port <= 0) {
+            throw new IllegalArgumentException(
+                    "The feed " + feed + " is sent over HTTP, but the port of 
the HTTP server of the integration is not"
+                                               + " known from its status");
+        }
+        // the path as served, with the context path of the server or the REST 
DSL
+        HttpEndpoints.Endpoint served0 = served(served, routeId, method, path);
+        if (served0 != null) {
+            path = served0.path();
+            if (method == null && !"ANY".equals(served0.method())) {
+                method = served0.method();
+            }
+        }
+        if (path.contains("{")) {
+            throw new IllegalArgumentException(
+                    "Route " + (routeId != null ? routeId : feed) + " serves " 
+ path + ", with a parameter: give the"
+                                               + " feed as the path to call, 
such as POST "
+                                               + path.replaceAll("\\{[^}]*}", 
"1"));
+        }
+        String m0 = method != null ? method.toUpperCase(Locale.ROOT) : "POST";
+        return new Target(m0, "http://localhost:"; + port + path);
+    }
+
+    private static HttpEndpoints.Endpoint served(
+            HttpEndpoints.Served served, String routeId, String method, String 
path) {
+        if (served == null) {
+            return null;
+        }
+        HttpEndpoints.Endpoint answer = null;
+        for (HttpEndpoints.Endpoint e : served.endpoints()) {
+            if (routeId != null && routeId.equals(e.routeId())) {
+                return e;
+            }
+            boolean samePath = e.path().equals(path) || 
e.path().endsWith(path) && !"/".equals(path);

Review Comment:
   💡 Operator precedence is correct (`&&` binds tighter than `||`) but 
non-obvious at a glance. Parentheses make the grouping explicit:
   
   ```suggestion
               boolean samePath = e.path().equals(path) || 
(e.path().endsWith(path) && !"/".equals(path));
   ```



##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/ChaosExperimentRun.java:
##########
@@ -192,6 +231,12 @@ static JsonObject run(
             }
             waited = clock.now() - start;
         } finally {
+            if (sender != null) {
+                sender.stop();
+                sending.interrupt();
+                sending.join(5000);

Review Comment:
   If the virtual thread is mid-request (HttpClient 30s timeout) when the 
experiment ends, `join(5000)` will time out before the thread exits. 
`sender.toJson()` is safe (the `synchronized` gives a consistent snapshot), but 
the virtual thread lingers until its HTTP request finishes or the JVM exits. 
For a CLI tool that's acceptable — the process exits shortly after. Worth a 
brief comment so a future reader doesn't mistake this for a resource leak to 
fix:
   
   ```suggestion
                   sending.join(5000); // best-effort: virtual thread exits on 
interrupt; JVM exit cleans up any straggler
   ```



-- 
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]

Reply via email to