Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -958,13 +958,14 @@ public RawClient createRawClientFor(SalesforceEndpoint endpoint) throws Salesfor
return new DefaultRawClient(httpClient, "", session, loginConfig);
}

// ExecutorService lifecycle is managed by SalesforceHttpClient
@SuppressWarnings("java:S2095")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The // NOSONAR inline comment was the accepted approach from PR #22343 review. Replacing it with method-level @SuppressWarnings widens the suppression scope. Same question: is SonarCloud not picking up the // NOSONAR?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code on behalf of Guillaume Nodet

Confirmed — SonarCloud still flags SalesforceComponent.java:968 despite the // NOSONAR on line 967. The NOSONAR comment is on the constructor call line, but SonarCloud flags the next line (the argument). Method-level @SuppressWarnings is the fallback that SonarCloud actually respects for this pattern.

static SalesforceHttpClient createHttpClient(
Object source, final SslContextFactory.Client sslContextFactory, final CamelContext context, int workerPoolSize,
int workerPoolMaxSize) {
SecurityUtils.adaptToIBMCipherNames(sslContextFactory);

// ExecutorService lifecycle is managed by SalesforceHttpClient
final SalesforceHttpClient httpClient = new SalesforceHttpClient( // NOSONAR
final SalesforceHttpClient httpClient = new SalesforceHttpClient(
context, context.getExecutorServiceManager().newThreadPool(source, "SalesforceHttpClient", workerPoolSize,
workerPoolMaxSize),
sslContextFactory);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ public Processor createProcessor() throws Exception {
return createAggregator();
}

// ExecutorService lifecycle is managed by AggregateProcessor via shutdownThreadPool flag
@SuppressWarnings("java:S2095")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This @SuppressWarnings was intentionally placed at the local variable declaration level in PR #22343 after reviewer feedback from @squakez asked to narrow the scope to avoid masking future real leaks in the same method. Moving it back to method level reverses that decision.

If SonarCloud is not recognizing the statement-level annotation, that would justify this change — could you confirm?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code on behalf of Guillaume Nodet

Yes — SonarCloud is still flagging this file despite the statement-level @SuppressWarnings from PR #22343. The annotation sits on line 80 (the variable declaration), but SonarCloud tracks the resource flow to a different exit point and flags line 86 instead, bypassing the annotation.

Confirmed via API: https://sonarcloud.io/api/issues/search?componentKeys=apache_camel&branch=main&rules=java:S2095 still lists AggregateReifier.java:86 as an open issue.

Moving to method-level is the only approach SonarCloud recognizes for this pattern. We acknowledge it broadens the scope, but given that this method only creates one ExecutorService with well-documented lifecycle, the risk of masking future leaks is low.

protected AggregateProcessor createAggregator() throws Exception {
Processor childProcessor = this.createChildProcessor(true);

Expand All @@ -76,8 +78,6 @@ protected AggregateProcessor createAggregator() throws Exception {

boolean parallel = parseBoolean(definition.getParallelProcessing(), false);
boolean shutdownThreadPool = willCreateNewThreadPool(definition, parallel);
// ExecutorService lifecycle is managed by AggregateProcessor via shutdownThreadPool flag
@SuppressWarnings("java:S2095")
ExecutorService threadPool = getConfiguredExecutorService("Aggregator", definition, parallel);
if (threadPool == null && !parallel) {
// executor service is mandatory for the Aggregator
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ public ThreadsReifier(Route route, ProcessorDefinition<?> definition) {
super(route, (ThreadsDefinition) definition);
}

// ExecutorService lifecycle is managed by ThreadsProcessor via shutdownThreadPool flag
@SuppressWarnings("java:S2095")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as AggregateReifier — this was narrowed to statement level in #22343 after explicit review feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code on behalf of Guillaume Nodet

Same situation — SonarCloud still flags ThreadsReifier.java:75 despite the statement-level @SuppressWarnings on line 47. The annotation doesn't cover the line SonarCloud is tracking the resource flow to.

@Override
public Processor createProcessor() throws Exception {
// the threads name
Expand All @@ -43,8 +45,6 @@ public Processor createProcessor() throws Exception {
}
// prefer any explicit configured executor service
boolean shutdownThreadPool = willCreateNewThreadPool(definition, true);
// ExecutorService lifecycle is managed by ThreadsProcessor via shutdownThreadPool flag
@SuppressWarnings("java:S2095")
ExecutorService threadPool = getConfiguredExecutorService(name, definition, false);

// resolve what rejected policy to use
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,8 @@ public static CamelCatalog loadSpringBootCatalog(
return answer;
}

// ClassLoader is intentionally kept open; it becomes the version manager's classloader
@SuppressWarnings("java:S2095")
public static CamelCatalog loadQuarkusCatalog(MavenGav quarkusCamelBom, Function<MavenGav, MavenArtifact> downloader)
throws Exception {
String camelQuarkusVersion = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,8 @@ private static ResolveResult resolvePlugin(
return new ResolveResult(dr.plugin(), cacheWritten);
}

// URLClassLoader is intentionally kept open; it becomes the plugin's classloader via setClassLoader()
@SuppressWarnings("java:S2095")
private static Optional<Plugin> loadFromCache(JsonObject entry, String camelVersion, String gav, String repos) {
if (entry == null) {
return Optional.empty();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,8 @@ static Line convertRow(long[] buffer, int offset, int width) {

private String startError;

// Streams are owned by virtualTerminal, which is closed in stopShell() via virtualTerminal.close()
@SuppressWarnings("java:S2095")
private void startShell(int width, int height) {
try {
screenTerminal = new ScreenTerminal(width, height);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.apache.camel.test.infra.openai.mock;

import java.io.IOException;
import java.io.InputStream;
import java.util.List;

import com.fasterxml.jackson.databind.ObjectMapper;
Expand All @@ -43,7 +44,9 @@ public AudioTranscriptionRequestHandler(List<AudioTranscriptionExpectation> expe
public String handleRequest(HttpExchange exchange) throws IOException {
try {
// consume the request body
exchange.getRequestBody().readAllBytes();
try (InputStream requestBody = exchange.getRequestBody()) {
requestBody.readAllBytes();
}
LOG.debug("Processing audio transcription request (call #{})", callIndex);

if (expectations.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* 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.test.infra.openai.mock;

import java.net.http.HttpClient;

/**
* Wrapper that makes {@link HttpClient} usable in try-with-resources. On Java 21+ HttpClient implements AutoCloseable
* natively; the instanceof check future-proofs us for when the minimum JDK is raised.
*/
final class CloseableHttpClient implements AutoCloseable {
final HttpClient httpClient = HttpClient.newHttpClient();

@Override
public void close() throws Exception {
if (httpClient instanceof AutoCloseable closeable) {
closeable.close();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.apache.camel.test.infra.openai.mock;

import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -45,7 +46,10 @@ public EmbeddingRequestHandler(List<EmbeddingExpectation> expectations, ObjectMa

public String handleRequest(HttpExchange exchange) throws IOException {
try {
String requestBody = new String(exchange.getRequestBody().readAllBytes(), StandardCharsets.UTF_8);
String requestBody;
try (InputStream is = exchange.getRequestBody()) {
requestBody = new String(is.readAllBytes(), StandardCharsets.UTF_8);
}
LOG.debug("Processing embedding request: {}", requestBody);

JsonNode rootNode = objectMapper.readTree(requestBody);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package org.apache.camel.test.infra.openai.mock;

import java.net.URI;
import java.net.http.HttpClient;
import java.net.http.HttpRequest;
import java.net.http.HttpResponse;
import java.util.concurrent.atomic.AtomicBoolean;
Expand Down Expand Up @@ -48,35 +47,35 @@ public class OpenAIMockConversationHistoryTest {

@Test
public void testConversationHistoryAssertion() throws Exception {
HttpClient client = HttpClient.newHttpClient();
try (CloseableHttpClient hc = new CloseableHttpClient()) {
// Send request with conversation history
String requestBody = "{\"messages\": [" +
"{\"role\": \"user\", \"content\": \"Hi! Can you look up user 123 and tell me about our rental policies?\"},"
+
"{\"role\": \"assistant\", \"content\": \"Previous response\"}," +
"{\"role\": \"user\", \"content\": \"What's his preferred vehicle type?\"}" +
"]}";

// Send request with conversation history
String requestBody = "{\"messages\": [" +
"{\"role\": \"user\", \"content\": \"Hi! Can you look up user 123 and tell me about our rental policies?\"},"
+
"{\"role\": \"assistant\", \"content\": \"Previous response\"}," +
"{\"role\": \"user\", \"content\": \"What's his preferred vehicle type?\"}" +
"]}";
HttpRequest request = HttpRequest.newBuilder()
.uri(URI.create(openAIMock.getBaseUrl() + "/v1/chat/completions"))
.header("Content-Type", "application/json")
.POST(HttpRequest.BodyPublishers.ofString(requestBody))
.build();

HttpRequest request = HttpRequest.newBuilder()
.uri(URI.create(openAIMock.getBaseUrl() + "/v1/chat/completions"))
.header("Content-Type", "application/json")
.POST(HttpRequest.BodyPublishers.ofString(requestBody))
.build();
HttpResponse<String> response = hc.httpClient.send(request, HttpResponse.BodyHandlers.ofString());
String responseBody = response.body();

HttpResponse<String> response = client.send(request, HttpResponse.BodyHandlers.ofString());
String responseBody = response.body();
ObjectMapper objectMapper = new ObjectMapper();
JsonNode responseJson = objectMapper.readTree(responseBody);

ObjectMapper objectMapper = new ObjectMapper();
JsonNode responseJson = objectMapper.readTree(responseBody);
JsonNode choice = responseJson.path("choices").get(0);
JsonNode message = choice.path("message");

JsonNode choice = responseJson.path("choices").get(0);
JsonNode message = choice.path("message");
assertEquals("assistant", message.path("role").asText());
assertEquals("SUV", message.path("content").asText());

assertEquals("assistant", message.path("role").asText());
assertEquals("SUV", message.path("content").asText());

// Verify assertion was executed
assertTrue(secondAssertionExecuted.get(), "Second assertion should have been executed");
// Verify assertion was executed
assertTrue(secondAssertionExecuted.get(), "Second assertion should have been executed");
}
}
}
Loading