gnodet-bot commented on code in PR #26845:
URL: https://github.com/apache/camel/pull/26845#discussion_r4094631232
##########
components/camel-jdbc/src/main/java/org/apache/camel/component/jdbc/JdbcComponent.java:
##########
@@ -114,6 +116,56 @@ public void setConnectionStrategy(ConnectionStrategy
connectionStrategy) {
this.connectionStrategy = connectionStrategy;
}
+ @Override
+ public void onSecretRotation(Object source) throws Exception {
+ // Collect all DataSources this component can reach: the one injected
directly (if any)
+ // plus all DataSource beans registered in the registry. The registry
beans are typically
+ // not recreated during a route reload, so their connection pools
still hold connections
+ // that were authenticated with the old credentials.
+ Set<DataSource> dataSources =
getCamelContext().getRegistry().findByType(DataSource.class);
+ if (this.dataSource != null) {
+ dataSources.add(this.dataSource);
Review Comment:
⚠️ **Potential double-eviction**: if `this.dataSource` was registered in the
Camel registry (the common Spring/Quarkus wiring), `findByType()` already
includes it, and `softEvictConnections()` will be called twice. `LinkedHashSet`
deduplicates by `equals`/`hashCode`, not identity — a `DataSource` wrapper that
delegates `equals` to the wrapped instance could slip through.
Use identity-based deduplication to make the intent explicit:
```suggestion
Set<DataSource> dataSources = Collections.newSetFromMap(new
IdentityHashMap<>());
dataSources.addAll(getCamelContext().getRegistry().findByType(DataSource.class));
if (this.dataSource != null) {
dataSources.add(this.dataSource);
}
```
(Same fix needed in `SqlComponent.onSecretRotation()`.)
##########
components/camel-sql/src/main/java/org/apache/camel/component/sql/SqlComponent.java:
##########
@@ -151,6 +158,56 @@ protected Endpoint createEndpoint(String uri, String
remaining, Map<String, Obje
return endpoint;
}
+ @Override
+ public void onSecretRotation(Object source) throws Exception {
+ // Collect all DataSources this component can reach: the one injected
directly (if any)
+ // plus all DataSource beans registered in the registry. The registry
beans are typically
+ // not recreated during a route reload, so their connection pools
still hold connections
+ // that were authenticated with the old credentials.
+ Set<DataSource> dataSources =
getCamelContext().getRegistry().findByType(DataSource.class);
+ if (this.dataSource != null) {
+ dataSources.add(this.dataSource);
+ }
+
+ for (DataSource ds : dataSources) {
+ evictDataSourceConnections(ds, source);
+ }
+ }
+
+ /**
+ * Evicts stale connections from the given DataSource so that the pool
rebuilds them with the rotated credentials.
+ * <p/>
+ * HikariCP is tried first via reflection (so camel-sql does not need a
compile-time dependency on it). Any
+ * DataSource that does not expose {@code getHikariPoolMXBean()} is left
untouched — the pool will pick up the new
+ * credentials on its own reconnect cycle when existing connections expire.
+ */
+ static void evictDataSourceConnections(DataSource ds, Object source) {
Review Comment:
⚠️ **Duplicate logic**: this method is identical to
`JdbcComponent.evictDataSourceConnections()` — same code, same Javadoc, same
log strings. Any future change (new pool vendor support, log level adjustment,
behaviour fix) must be applied in both places.
Extract to a shared `JdbcPoolEvictionSupport` utility class in
`camel-support` or in one of these two modules, and delegate from the other.
This is the kind of duplication that bites when the third pool vendor gets
added.
--
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]
