allthingssecurity commented on code in PR #27250:
URL: https://github.com/apache/camel/pull/27250#discussion_r4163578588
##########
components/camel-dynamic-router/src/main/java/org/apache/camel/component/dynamicrouter/filter/DynamicRouterFilterService.java:
##########
@@ -170,17 +170,22 @@ public String addFilterForChannel(
* @return the ID of the added filter
*/
public String addFilterForChannel(final PrioritizedFilter filter, final
String channel, final boolean update) {
- boolean filterExists = !filterMap.isEmpty() &&
- filterMap.get(channel).stream().anyMatch(f ->
filter.id().equals(f.id()));
+ Set<PrioritizedFilter> filters = filterMap.computeIfAbsent(channel,
+ c -> new
ConcurrentSkipListSet<>(DynamicRouterConstants.FILTER_COMPARATOR));
+ List<PrioritizedFilterStatistics> filterStatistics =
filterStatisticsMap.computeIfAbsent(channel,
+ c -> Collections.synchronizedList(new ArrayList<>()));
+ boolean filterExists = filters.stream().anyMatch(f ->
filter.id().equals(f.id()));
boolean okToAdd = update == filterExists;
if (okToAdd) {
- Set<PrioritizedFilter> filters = filterMap.computeIfAbsent(channel,
- c -> new
ConcurrentSkipListSet<>(DynamicRouterConstants.FILTER_COMPARATOR));
+ if (filterExists) {
+ // the set is ordered by priority and id: adding the updated
filter would neither replace a filter with
+ // the same priority nor remove the one with the old priority,
so remove the existing filter first
+ // (its statistics stay, as when a filter is removed: they
represent actions that happened)
+ filters.removeIf(f -> filter.id().equals(f.id()));
Review Comment:
Done in 94083586c3b1. When the priority changes, the new filter is added
first and then the old instance is removed by identity (`f != filter && id
matches`). With the same priority it is still remove-then-add, because the
skip-list set has no atomic replace. One side effect: in that short window both
definitions are in the set, so an `allMatch` exchange routed at exactly that
moment could go to both. I think that is better than matching nothing.
`DynamicRouterFilterServiceUpdateTest.testUpdateWithOtherPriorityKeepsTheNewInstance`
updates to a lower and then a higher priority and checks that only the new
instance is left. All 138 unit tests pass.
_Claude Code on behalf of allthingssecurity_
--
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]
