CAMEL-24375: parseDuration should handle plain millis value - #25412
Merged
Conversation
…elying on type converter Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
Contributor
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 564 tested, 29 compile-only — current: 564 all testedMaveniverse Scalpel detected 593 affected modules (current approach: 564).
|
gnodet
approved these changes
Aug 9, 2026
gnodet
left a comment
gnodet
left a comment
Contributor
There was a problem hiding this comment.
Clean, well-focused fix that eliminates the timing dependency on the type converter registry by inlining the ISO-8601/human-readable/millis parsing logic directly in parseDuration().
Highlights:
- The new code mirrors
DurationConverter.toDuration(String)exactly, which is the right approach given the goal of avoiding converter registry dependency. - Tests cover the main input formats (plain millis, ISO-8601, human-readable) with proper AssertJ assertions and package-private visibility.
- CI is green.
Minor observations (non-blocking):
- The parsing logic at lines 472-477 duplicates
DurationConverter.toDuration(). SinceTimeUtils.toDuration(String)already exists and does exactly this, a possible follow-up would be to callTimeUtils.toDuration(s)instead — keeping both sites in sync automatically. - Minor behavioral change: the old path wrapped conversion failures in
IllegalArgumentExceptionwith a contextual message; the new code propagates a rawDateTimeParseException. No callers appear to catchIllegalArgumentExceptionspecifically, so this is unlikely to matter.
This review does not replace specialized AI review tools (CodeRabbit, Sourcery) or static analysis (SonarCloud).
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of @gnodet
Consolidate duration parsing logic into TimeUtils.toDuration() so both CamelContextHelper.parseDuration() and DurationConverter.toDuration() delegate to a single implementation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
Contributor
Author
|
Thanks for the review @gnodet. Addressed the suggestion to consolidate the duration parsing logic — moved the ISO-8601 handling into |
Contributor
Author
|
Need to be backported to 4.22.x |
davsclaus
added a commit
that referenced
this pull request
Aug 11, 2026
…n millis value (#25447) CAMEL-24375: parseDuration should handle plain millis value Consolidate duration parsing into TimeUtils.toDuration() so both CamelContextHelper.parseDuration() and DurationConverter.toDuration() delegate to a single implementation that handles plain millis, human- readable (20s, 1m30s), and ISO-8601 (PT20S) formats. This fixes NoTypeConversionAvailableException when parsing duration values during early route initialization before the type converter registry is loaded. Closes #25412 Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Claude Code on behalf of davsclaus
CamelContextHelper.parseDurationnow handles the String-to-Duration conversion directly usingTimeUtils.toMilliSecondsinstead of delegating to the type converter registry.NoTypeConversionAvailableExceptionwhen parsing plain millisecond values like"20000"during early route initialization (e.g. Resilience4j circuit breaker configuration) when theDurationConvertertype converter is not yet loaded.Supported formats: plain millis (
20000), human-readable (20s,1m30s,500ms), ISO-8601 (PT20S).Test plan
CamelContextHelperParseDurationTestcovering plain millis, human-readable, ISO-8601, and null inputmvn test -Dtest=CamelContextHelperParseDurationTestpasses🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com