gnodet-bot commented on code in PR #29049:
URL: https://github.com/apache/camel/pull/29049#discussion_r4451576102
##########
components/camel-micrometer/src/test/java/org/apache/camel/component/micrometer/CounterRouteTest.java:
##########
@@ -136,4 +163,66 @@ public void testUsingScriptEvaluation() throws Exception {
assertEquals(Integer.toString(message.length()),
counter.getId().getTag("a"));
endpoint.assertIsSatisfied();
}
+
+ @Test
+ public void testIncrementFromBody() throws Exception {
+ endpoint.expectedMessageCount(1);
+ producer5.sendBody("42");
+ assertEquals(42.0D, registry.find("E").counter().count(), 0.01D);
+ endpoint.assertIsSatisfied();
+ }
+
+ @Test
+ public void testIncrementFromBodyNotNumber() throws Exception {
+ endpoint.expectedMessageCount(0);
+
+ assertIncrementFails(Map.of("count", 5));
+ assertIncrementFails(List.of(new Object()));
+ assertIncrementFails(new Object());
+ assertIncrementFails(null);
+
+ // the counter is not updated (it was incremented by 1 for each of
them)
+ assertEquals(0.0D, registry.find("E").counter().count(), 0.01D);
+ endpoint.assertIsSatisfied();
+ }
+
+ @Test
+ public void testIncrementOrDecrementFromHeader() throws Exception {
+ endpoint.expectedMessageCount(2);
+ producer6.sendBodyAndHeader(null, "inc", 10.0D);
+ producer6.sendBodyAndHeader(null, "dec", 3.0D);
+ assertEquals(7.0D, registry.find("F").counter().count(), 0.01D);
+ endpoint.assertIsSatisfied();
+ }
+
+ @Test
+ public void testIncrementOrDecrementFromHeaderMissing() throws Exception {
+ endpoint.expectedMessageCount(0);
+
+ CamelExecutionException e = assertThrows(CamelExecutionException.class,
+ () -> producer6.sendBody("Hello"));
+ assertInstanceOf(IllegalArgumentException.class, e.getCause());
+ assertEquals("The increment (${header.inc}) or decrement
(${header.dec}) of counter F did not evaluate to a number",
+ e.getCause().getMessage());
+
+ assertEquals(0.0D, registry.find("F").counter().count(), 0.01D);
+ endpoint.assertIsSatisfied();
+ }
+
+ @Test
+ public void testNoIncrementOrDecrement() throws Exception {
+ endpoint.expectedMessageCount(2);
+ producer7.sendBody(new Object());
+ producer7.sendBody(null);
+ assertEquals(2.0D, registry.find("G").counter().count(), 0.01D);
+ endpoint.assertIsSatisfied();
+ }
+
+ private void assertIncrementFails(Object body) {
+ CamelExecutionException e = assertThrows(CamelExecutionException.class,
+ () -> producer5.sendBody(body));
+ assertInstanceOf(IllegalArgumentException.class, e.getCause());
+ assertEquals("The increment (${body}) or decrement (null) of counter E
did not evaluate to a number",
+ e.getCause().getMessage());
Review Comment:
ℹ️ **Confusing error message** — when only `increment` is configured, the
error message reads `The increment (${body}) or decrement (null) of counter E
did not evaluate to a number`. The `decrement (null)` part is noise for users
who only set `increment`. Consider only including the non-null option in the
message, or phrasing it differently:
```suggestion
assertEquals("The increment (${body}) of counter E did not evaluate
to a number",
```
Note: this would require updating the error message in `CounterProducer` to
conditionally include only the configured expression. Minor UX improvement, not
a blocker.
##########
components/camel-micrometer/src/test/java/org/apache/camel/component/micrometer/CounterRouteTest.java:
##########
@@ -136,4 +163,66 @@ public void testUsingScriptEvaluation() throws Exception {
assertEquals(Integer.toString(message.length()),
counter.getId().getTag("a"));
endpoint.assertIsSatisfied();
}
+
+ @Test
+ public void testIncrementFromBody() throws Exception {
+ endpoint.expectedMessageCount(1);
+ producer5.sendBody("42");
+ assertEquals(42.0D, registry.find("E").counter().count(), 0.01D);
+ endpoint.assertIsSatisfied();
+ }
+
+ @Test
+ public void testIncrementFromBodyNotNumber() throws Exception {
+ endpoint.expectedMessageCount(0);
+
+ assertIncrementFails(Map.of("count", 5));
+ assertIncrementFails(List.of(new Object()));
+ assertIncrementFails(new Object());
+ assertIncrementFails(null);
+
+ // the counter is not updated (it was incremented by 1 for each of
them)
+ assertEquals(0.0D, registry.find("E").counter().count(), 0.01D);
Review Comment:
⚠️ **Misleading comment** — the parenthetical describes the OLD buggy
behavior, not the new one. After this fix, the counter is not updated because
the exchange fails with an `IllegalArgumentException` — not because it silently
incremented by 1 each time.
```suggestion
// the counter is not updated (exchange fails with
IllegalArgumentException)
```
--
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]
