gnodet-bot commented on code in PR #26845:
URL: https://github.com/apache/camel/pull/26845#discussion_r4208759138
##########
components/camel-sql/src/main/java/org/apache/camel/component/sql/stored/SqlStoredComponent.java:
##########
@@ -112,4 +114,14 @@ public void setAllowTemplateFromHeader(boolean
allowTemplateFromHeader) {
this.allowTemplateFromHeader = allowTemplateFromHeader;
}
+ @Override
+ public void onSecretRotation(Object source) throws Exception {
Review Comment:
**Missing test for `onSecretRotation()`.**
`JdbcComponent` and `SqlComponent` each ship a `*SecretRotationAwareTest`
with three scenarios (component-owned DataSource, endpoint DataSource, no
DataSource). `SqlStoredComponent` now has the same implementation but no test
class. At a minimum, a `SqlStoredComponentSecretRotationAwareTest` mirroring
the JdbcComponent test structure would close this gap.
##########
core/camel-support/src/main/java/org/apache/camel/support/DataSourceHelper.java:
##########
@@ -0,0 +1,197 @@
+/*
+ * 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.support;
+
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.Collections;
+import java.util.IdentityHashMap;
+import java.util.Set;
+import java.util.function.Function;
+
+import javax.sql.DataSource;
+
+import org.apache.camel.Component;
+import org.apache.camel.Endpoint;
+import org.apache.camel.spi.InterceptSendToEndpoint;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Utility methods for working with JDBC {@link DataSource} instances.
+ */
+public final class DataSourceHelper {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(DataSourceHelper.class);
+
+ private DataSourceHelper() {
+ }
+
+ /**
+ * Collects and evicts stale connections from all DataSources used by a
component: the component's own
+ * {@code dataSource} field plus any DataSources held by the component's
active endpoints.
+ * <p/>
+ * Identity-based deduplication ensures each DataSource is evicted at most
once, even if the same instance is shared
+ * between the component and one or more endpoints.
+ * <p/>
+ * {@link InterceptSendToEndpoint} wrappers are unwrapped to the original
endpoint before the component check and
+ * DataSource extraction. This ensures that intercepted endpoints (created
by {@code interceptSendToEndpoint()}) are
+ * not silently skipped.
+ *
+ * @param componentDataSource the component-level DataSource (may be null)
+ * @param endpoints the component's active endpoints (from
{@code getCamelContext().getEndpoints()})
+ * @param componentInstance the component instance, used to filter
endpoints
+ * ({@code endpoint.getComponent() == this})
+ * @param endpointDsExtractor a function to extract the DataSource from an
endpoint (may return null)
+ * @param source an opaque label for the rotation event (used
in log messages only)
+ */
+ public static void evictComponentDataSources(
+ DataSource componentDataSource,
+ Iterable<Endpoint> endpoints,
+ Component componentInstance,
+ Function<Endpoint, DataSource> endpointDsExtractor,
+ Object source) {
+
+ Set<DataSource> dataSources = Collections.newSetFromMap(new
IdentityHashMap<>());
+ if (componentDataSource != null) {
+ dataSources.add(componentDataSource);
+ }
+ if (endpoints != null) {
+ for (Endpoint ep : endpoints) {
+ // unwrap InterceptSendToEndpoint proxies so that the
component check and extractor
+ // see the real endpoint type (e.g. JdbcEndpoint,
DefaultSqlEndpoint)
+ Endpoint target = ep instanceof InterceptSendToEndpoint ise ?
ise.getOriginalEndpoint() : ep;
+ if (target.getComponent() == componentInstance) {
+ DataSource ds = endpointDsExtractor.apply(target);
+ if (ds != null) {
+ dataSources.add(ds);
+ }
+ }
+ }
+ }
+ for (DataSource ds : dataSources) {
+ evictDataSourceConnections(ds, source);
+ }
+ }
+
+ /**
+ * Evicts stale connections from the given DataSource's connection pool.
+ * <p/>
+ * The following pool implementations are supported (detected via
reflection — no compile-time dependency required):
+ * <ul>
+ * <li><b>HikariCP</b>: {@code softEvictConnections()} is called via
{@code HikariPoolMXBean}. Idle connections are
+ * evicted immediately; borrowed connections are evicted when
returned.</li>
+ * <li><b>Agroal</b> (Quarkus default): {@code flush(GRACEFUL)} is called
on {@code AgroalDataSource}. Idle
+ * connections are closed immediately; active connections are closed when
returned to the pool.</li>
+ * </ul>
+ * Any DataSource not recognised as one of the above is left untouched — a
log message is emitted and the pool will
+ * naturally replace connections as they expire or are validated.
+ * <p/>
+ * <b>Important:</b> this method only evicts existing connections from the
pool. It does <em>not</em> update the
+ * pool's credentials. For pools configured with a static password (e.g.
Spring Boot
+ * {@code spring.datasource.password}), the pool will re-open connections
using the <em>old</em> credentials unless
+ * the credentials are resolved dynamically (e.g. {@code
HikariCredentialsProvider} (HikariCP 7.0 or later), the AWS
+ * JDBC wrapper secrets plugin, or a custom {@code DataSource} that
fetches credentials from a vault at connect
+ * time).
+ *
+ * @param ds the DataSource whose connections should be evicted
+ * @param source an opaque label for the rotation event (used in log
messages only)
+ */
+ public static void evictDataSourceConnections(DataSource ds, Object
source) {
+ // If the DataSource is a java.sql.Wrapper (e.g. Spring's
TransactionAwareDataSourceProxy,
+ // LazyConnectionDataSourceProxy, or a JDBC tracing proxy), try to
unwrap to the underlying
Review Comment:
**Comment overstates what this Wrapper path handles for Spring proxies.**
Spring's `DelegatingDataSource.unwrap(DataSource.class)` returns `this` (not
the underlying pool), because `iface.isInstance(this)` is `true` —
`DelegatingDataSource` itself implements `DataSource`. The `unwrapped != ds`
identity check on line 122 then prevents substitution, so
`TransactionAwareDataSourceProxy` and `LazyConnectionDataSourceProxy` are
**not** actually unwrapped to the pool.
The code is correct and safe for real delegation proxies (P6Spy, Flyway,
custom wrappers that properly return the target from `unwrap()`). The comment
should not name Spring's proxies as examples, or should clarify that Spring
proxies backed by HikariCP/Agroal are handled **only if** the underlying pool's
`isWrapperFor`/`unwrap` implementation traverses to it (which the standard
Spring wrappers do not do for `DataSource.class` directly).
Suggested fix:
```suggestion
// If the DataSource is a java.sql.Wrapper (e.g. a JDBC tracing
proxy like P6Spy,
// Flyway's ProxiedDataSource, or any delegation wrapper whose
unwrap(DataSource.class)
// returns the underlying pool), try to unwrap to the underlying
pool implementation
// so that pool-specific eviction can be applied.
// Note: Spring's DelegatingDataSource
(TransactionAwareDataSourceProxy, etc.) does NOT
// work here — its unwrap(DataSource.class) returns `this`, so the
identity check below
// leaves the proxy untouched.
```
--
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]
