oscerd opened a new pull request, #27329: URL: https://github.com/apache/camel/pull/27329
`Endpoint.createExchange` states the rule outright: > **Important:** Consumers should use `Consumer#createExchange(boolean)` to create an exchange for which the consumer received a message. `MessageReceiverListenerImpl` followed it in one of its three inbound paths and not in the other two. | path | exchange from | released | |---|---|---| | `onAcceptAlertNotification` | `consumer.createExchange(false)` | yes | | `onAcceptDeliverSm` | `endpoint.createOnAcceptDeliverSmExchange(...)` | **no** | | `onAcceptDataSm` | `endpoint.createOnAcceptDataSm(...)` | **no** | Both endpoint methods end in `DefaultEndpoint.createExchange(pattern)`, which is `DefaultExchange.newFromEndpoint(this, pattern)` — a plain exchange that never reaches the configured `ExchangeFactory`. ### What it actually costs **Not a pool leak.** Nothing is taken from the pool, so nothing is lost from it. I want to be precise about that, because "no release" reads like a leak and it isn't one here. What *was* lost: - **Pooling never applied to received messages.** `camel.main.exchange-factory=pooled` had no effect on `deliverSm` — the path every received SMS takes — nor on `dataSm`. Only on the comparatively rare `alertNotification`. - **`fromRouteId` was unset on every received message.** `DefaultConsumer.createExchange` does `setFromRouteId(routeId)`; the endpoint path does not. So MDC logging, tracing and management saw null for inbound SMPP traffic. ### The change Both paths now build their exchange through the consumer, carrying over the endpoint's `ExchangePattern` and binding exactly as before, and all three release in a `finally`. The endpoint's `createOnAcceptDeliverSmExchange` and `createOnAcceptDataSm` are **public**, so they stay where they are rather than being removed. **One adjacent line, separable if you'd rather:** the alert-notification path already used the consumer factory correctly but released *outside* any `finally`, so a custom exception handler that threw would have skipped the release. Putting it in a `finally` makes the three paths symmetric, which is the substance of this issue — but say the word and I'll split it out. ### Testing Three tests in a new `MessageReceiverListenerImplTest`, asserting the contract directly: `verify(consumer).createExchange(false)` and `verify(consumer).releaseExchange(exchange, false)`, including the `ProcessRequestException` path so the SMSC-NACK case is covered too. They use plain `Mockito.mock()` rather than `@ExtendWith(MockitoExtension.class)` — camel-smpp carries `mockito-core` and not `mockito-junit-jupiter`, and this did not seem worth a new test dependency. **Revert-check, stated precisely:** putting either path back on the endpoint factory fails all three tests, but it fails them on an NPE rather than on the `verify` — with a mocked endpoint the listener simply gets a null exchange. So they are a genuine regression guard, and the `verify` calls are what pin the contract when the code is correct; I'd rather say that than imply a cleaner `Wanted but not invoked`. Module suite 140 tests green, 4 skipped. No `@UriParam` or doc change, so nothing regenerates — `git status` after the build shows only the two files here. Found while auditing camel-smpp, which had no behavioural Jira since 2023. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
