Skip to content

Commit 0bf9acf

Browse files
oscerdclaude
andauthored
CAMEL-23289: Fix empty catch blocks and resource leaks in DrillProducer (#22417)
Signed-off-by: Andrea Cosentino <ancosen@gmail.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent dad41a9 commit 0bf9acf

3 files changed

Lines changed: 145 additions & 20 deletions

File tree

components/camel-drill/pom.xml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,12 @@
7272
<optional>true</optional>
7373
<scope>test</scope>
7474
</dependency>
75+
<dependency>
76+
<groupId>org.mockito</groupId>
77+
<artifactId>mockito-core</artifactId>
78+
<version>${mockito-version}</version>
79+
<scope>test</scope>
80+
</dependency>
7581
</dependencies>
7682

7783
</project>

components/camel-drill/src/main/java/org/apache/camel/component/drill/DrillProducer.java

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -54,35 +54,24 @@ protected void doStart() throws Exception {
5454
protected void doStop() throws Exception {
5555
super.doStop();
5656

57-
try {
58-
connection.close();
59-
} catch (Exception e) {
57+
if (connection != null) {
58+
try {
59+
connection.close();
60+
} catch (Exception e) {
61+
LOG.debug("Error closing JDBC connection: {}", e.getMessage(), e);
62+
}
6063
}
6164
}
6265

6366
@Override
6467
public void process(final Exchange exchange) throws Exception {
6568
final String query = exchange.getIn().getHeader(DrillConstants.DRILL_QUERY, String.class);
6669

67-
// check query
68-
Statement st = null;
69-
ResultSet rs = null;
70-
try {
71-
st = connection.createStatement();
72-
rs = st.executeQuery(query);
73-
70+
try (Statement st = connection.createStatement();
71+
ResultSet rs = st.executeQuery(query)) {
7472
exchange.getIn().setBody(endpoint.queryForList(rs));
75-
} finally {
76-
try {
77-
rs.close();
78-
} catch (Exception e) {
79-
}
80-
try {
81-
st.close();
82-
} catch (Exception e) {
83-
}
8473
}
85-
} // end process
74+
}
8675

8776
private void createJDBCConnection() throws ClassNotFoundException, SQLException {
8877
Class.forName(DrillConstants.DRILL_DRIVER);
Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.camel.component.drill;
18+
19+
import java.sql.Connection;
20+
import java.sql.ResultSet;
21+
import java.sql.SQLException;
22+
import java.sql.Statement;
23+
import java.util.Collections;
24+
25+
import org.apache.camel.Exchange;
26+
import org.apache.camel.Message;
27+
import org.junit.jupiter.api.BeforeEach;
28+
import org.junit.jupiter.api.Test;
29+
30+
import static org.mockito.ArgumentMatchers.any;
31+
import static org.mockito.Mockito.*;
32+
33+
class DrillProducerTest {
34+
35+
private DrillEndpoint endpoint;
36+
private Connection connection;
37+
private Statement statement;
38+
private ResultSet resultSet;
39+
private Exchange exchange;
40+
private Message message;
41+
42+
@BeforeEach
43+
void setUp() {
44+
endpoint = mock(DrillEndpoint.class);
45+
when(endpoint.getCamelContext()).thenReturn(null);
46+
when(endpoint.getEndpointUri()).thenReturn("drill://localhost");
47+
48+
connection = mock(Connection.class);
49+
statement = mock(Statement.class);
50+
resultSet = mock(ResultSet.class);
51+
exchange = mock(Exchange.class);
52+
message = mock(Message.class);
53+
54+
when(exchange.getIn()).thenReturn(message);
55+
}
56+
57+
@Test
58+
void testProcessClosesStatementAndResultSet() throws Exception {
59+
String query = "SELECT * FROM test";
60+
when(message.getHeader(DrillConstants.DRILL_QUERY, String.class)).thenReturn(query);
61+
when(connection.createStatement()).thenReturn(statement);
62+
when(statement.executeQuery(query)).thenReturn(resultSet);
63+
when(endpoint.queryForList(any(ResultSet.class))).thenReturn(Collections.emptyList());
64+
65+
DrillProducer producer = new DrillProducer(endpoint);
66+
setConnection(producer, connection);
67+
68+
producer.process(exchange);
69+
70+
verify(resultSet).close();
71+
verify(statement).close();
72+
}
73+
74+
@Test
75+
void testProcessClosesResourcesOnQueryFailure() throws Exception {
76+
String query = "SELECT * FROM test";
77+
when(message.getHeader(DrillConstants.DRILL_QUERY, String.class)).thenReturn(query);
78+
when(connection.createStatement()).thenReturn(statement);
79+
when(statement.executeQuery(query)).thenThrow(new SQLException("query failed"));
80+
81+
DrillProducer producer = new DrillProducer(endpoint);
82+
setConnection(producer, connection);
83+
84+
try {
85+
producer.process(exchange);
86+
} catch (SQLException e) {
87+
// expected
88+
}
89+
90+
verify(statement).close();
91+
}
92+
93+
@Test
94+
void testDoStopClosesConnection() throws Exception {
95+
DrillProducer producer = new DrillProducer(endpoint);
96+
setConnection(producer, connection);
97+
98+
producer.doStop();
99+
100+
verify(connection).close();
101+
}
102+
103+
@Test
104+
void testDoStopHandlesCloseException() throws Exception {
105+
doThrow(new SQLException("close failed")).when(connection).close();
106+
107+
DrillProducer producer = new DrillProducer(endpoint);
108+
setConnection(producer, connection);
109+
110+
// should not throw
111+
producer.doStop();
112+
113+
verify(connection).close();
114+
}
115+
116+
@Test
117+
void testDoStopHandlesNullConnection() throws Exception {
118+
DrillProducer producer = new DrillProducer(endpoint);
119+
setConnection(producer, null);
120+
121+
// should not throw
122+
producer.doStop();
123+
}
124+
125+
private void setConnection(DrillProducer producer, Connection conn) throws Exception {
126+
java.lang.reflect.Field field = DrillProducer.class.getDeclaredField("connection");
127+
field.setAccessible(true);
128+
field.set(producer, conn);
129+
}
130+
}

0 commit comments

Comments
 (0)