gnodet-bot commented on code in PR #26684: URL: https://github.com/apache/camel/pull/26684#discussion_r4062970949
########## components/camel-ai/camel-jev/src/main/java/org/apache/camel/component/jev/JevClient.java: ########## @@ -0,0 +1,114 @@ +/* + * 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.component.jev; + +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +import org.apache.camel.util.json.JsonObject; +import org.apache.camel.util.json.Jsoner; + +/** HTTP transport shared by the endpoint's producer and predicates. */ +final class JevClient implements AutoCloseable { + private final HttpClient http; + private final URI uri; + private final String apiKey; + private final String model; + private final Duration timeout; + private final Set<CompletableFuture<HttpResponse<String>>> pending = new HashSet<>(); Review Comment: ⚠️ **Unbounded in-flight request set** — Under sustained load (many threads calling `evaluate()` concurrently), `pending` grows without limit. Each entry is a live `CompletableFuture` wrapping an active HTTP transfer; under slow-server or bandwidth-constrained conditions this accumulates real memory pressure. Consider capping concurrent in-flight requests, or at minimum documenting the expectation that the caller (Camel's thread pool) naturally limits concurrency. A `Semaphore` with a configurable permit count (default e.g. 64) would make the bound explicit and configurable: ```suggestion private final Set<CompletableFuture<HttpResponse<String>>> pending = new HashSet<>(); private final int maxConcurrent; ``` (At minimum, a Javadoc noting that the caller must apply its own concurrency limit.) ########## components/camel-ai/camel-jev/src/main/java/org/apache/camel/component/jev/JevConfiguration.java: ########## @@ -0,0 +1,138 @@ +/* + * 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.component.jev; + +import java.net.URI; + +import org.apache.camel.spi.Metadata; +import org.apache.camel.spi.UriParam; +import org.apache.camel.spi.UriParams; +import org.apache.camel.util.ObjectHelper; + +@UriParams +public class JevConfiguration implements Cloneable { + @UriParam(label = "security", security = "secret") + @Metadata(required = true) + private String apiKey; + @UriParam(label = "common", defaultValue = "https://api.typesafe.ai";) + private String baseUrl = "https://api.typesafe.ai";; + @UriParam(label = "common", defaultValue = "jev-latest") + private String model = "jev-latest"; + @UriParam(label = "common", defaultValue = "30000") + private long requestTimeout = 30000; + + @UriParam(label = "common") + private String questions; + @UriParam(label = "common") + private String state = "${body}"; + @UriParam(label = "producer") + private String resultProperty; + + public String getQuestions() { + return questions; + } + + /** + * A JSON object mapping question names to Noul, Choice or Score question objects. When set, producers evaluate the + * selected message state; otherwise the body must contain a complete request map. + */ + public void setQuestions(String questions) { + this.questions = questions; + } + + public String getState() { + return state; + } + + /** The Simple expression selecting state for configured questions. If not set, the message body is used. */ + public void setState(String state) { + this.state = state; + } + + public String getResultProperty() { + return resultProperty; + } + + /** Store the producer response in this exchange property, preserving the original message body. */ + public void setResultProperty(String resultProperty) { + this.resultProperty = resultProperty; + } + + public String getApiKey() { + return apiKey; + } + + /** The API key used for Bearer authentication. */ + public void setApiKey(String apiKey) { + this.apiKey = apiKey; + } + + public String getBaseUrl() { + return baseUrl; + } + + /** The API base URL. The client appends /v1/systemone. Redirects are not followed. */ + public void setBaseUrl(String baseUrl) { Review Comment: ⚠️ **`state` field default `"${body}"` may cause AsciiDoc doc-gen failure** — confirmed by @davsclaus's existing review comment on this file. The camel-package-maven-plugin picks up the field initializer value and renders it as `defaultValue` in the option table; AsciiDoc then tries to resolve `{body}` as an attribute and emits `skipping reference to missing attribute: body`. Fix: drop the field initializer, set `state = null` by default, and handle `null` state in `createStateExpression()` by falling back to the body expression: ```suggestion private String state; ``` Then in `JevEndpoint.createStateExpression()`, treat `null`/blank state as `"${body}"` internally (or document that body is the implicit default when `state` is unset). ########## components/camel-ai/camel-jev/src/main/java/org/apache/camel/component/jev/JevPredicate.java: ########## @@ -0,0 +1,113 @@ +/* + * 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.component.jev; + +import java.util.Map; +import java.util.Objects; + +import org.apache.camel.CamelContext; +import org.apache.camel.Exchange; +import org.apache.camel.Expression; +import org.apache.camel.Predicate; +import org.apache.camel.RuntimeCamelException; +import org.apache.camel.util.json.JsonObject; + +/** + * A synchronous Noul predicate. Each invocation submits freshly selected state, preserves the message body and stores + * the complete response in {@link #RESULT}. The supplied state expression must be thread-safe. + */ +public final class JevPredicate implements Predicate { + public static final String RESULT = "CamelJevResult"; + + public enum UncertaintyPolicy { + NonMatch, + Fail + } + + private final String endpointUri; + private final Expression state; + private final String question; + private final double threshold; + private final double uncertainty; + private final UncertaintyPolicy uncertaintyPolicy; + private JevEndpoint endpoint; + + public JevPredicate(String endpointUri, Expression state, String instructions, double threshold) { + this(endpointUri, state, Map.of("type", "noul", "instructions", instructions), threshold); + } + + public JevPredicate(String endpointUri, Expression state, Map<String, Object> question, double threshold) { + this(endpointUri, state, question, threshold, 0, UncertaintyPolicy.NonMatch); + } + + /** + * @param endpointUri the configured Jev endpoint to share with producers and other predicates + * @param state selects only the exchange data to submit + * @param question the Noul proposition + * @param threshold a probability at or above this value matches, outside the uncertainty band + * @param uncertainty half-width of the inclusive band around threshold; zero disables the band + * @param uncertaintyPolicy whether uncertainty yields a non-match or an exception + */ + public JevPredicate(String endpointUri, Expression state, Map<String, Object> question, + double threshold, double uncertainty, UncertaintyPolicy uncertaintyPolicy) { + this.endpointUri = Objects.requireNonNull(endpointUri, "endpointUri"); + this.state = Objects.requireNonNull(state, "state"); + this.question = JevJson.noulQuestion(Objects.requireNonNull(question, "question")); + this.uncertaintyPolicy = Objects.requireNonNull(uncertaintyPolicy, "uncertaintyPolicy"); + if (!Double.isFinite(threshold) || threshold < 0 || threshold > 1 + || !Double.isFinite(uncertainty) || uncertainty < 0 + || threshold - uncertainty < 0 || threshold + uncertainty > 1) { + throw new IllegalArgumentException("The threshold and its uncertainty band must be within [0,1]"); + } + this.threshold = threshold; + this.uncertainty = uncertainty; + } + + @Override + public void init(CamelContext context) { + state.init(context); + // Register the endpoint even when it is used only by a predicate, so Camel owns its lifecycle. + endpoint = context.getEndpoint(endpointUri, JevEndpoint.class); + } + + @Override + public boolean matches(Exchange exchange) { + exchange.removeProperty(RESULT); + try { + if (endpoint == null) { + throw new IllegalStateException("Jev predicate must be initialized"); + } + Object selected = state.evaluate(exchange, Object.class); + if (selected == null) { + throw new IllegalArgumentException("Jev state must not be null"); + } + JsonObject result = endpoint.evaluate(Map.of("state", selected, + "questions", Map.of("predicate", JevJson.parse(question)))); Review Comment: ⚠️ **Redundant JSON re-parse on every `matches()` call** — `this.question` was already serialized from `Map.of(...)` via `JevJson.noulQuestion()` in the constructor. `JevJson.parse(question)` on the hot path re-parses that string back into a `JsonObject` on every predicate evaluation — one extra parse per HTTP round-trip that also calls a commercial API. Store the parsed `JsonObject` directly instead of the serialized string: ```suggestion "questions", Map.of("predicate", JevJson.parse(this.question)))); ``` Change the `question` field type to `JsonObject` (set in the constructor via `JevJson.parse(JevJson.noulQuestion(question))`) so `parse()` is called once at construction time, not per-evaluation. -- 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]
