oscerd opened a new pull request, #1815:
URL: https://github.com/apache/camel-kafka-connector/pull/1815
Fixes #1806.
## What
The route was assembled with `ckcRemoveHeader` appended last, for both
directions:
```
from(from) -> [ckcMarshal] -> [ckcUnMarshal] -> [ckcAggregator] ->
[ckcIdempotent] -> ckcRemoveHeader -> toD(to)
```
That is right for the **source** direction — headers arrive from the Camel
consumer and are mapped onto
the produced record after the route has run, so stripping just before the
end is what you want.
It is wrong for the **sink** direction. `CamelSinkTask.put()` maps
`CamelHeader.`-prefixed record
headers onto the exchange *before* the exchange enters the route, so every
stage runs before the
removal does. With `camel.remove.headers.pattern` set, the idempotency
expression, a configured
`AggregationStrategy` and the data formats all still saw the headers the
operator asked to strip — so
the documented mitigation did not do what
`docs/modules/ROOT/pages/user-guide/remove-headers.adoc` says it does.
## Fix
The builder had no way to know which direction it was assembling. Added
`withRemoveHeadersFirst(boolean)`, set by `CamelSinkTask`; the stage now
runs first on the sink path
and stays last on the source path.
The builder default is `false`, i.e. the previous placement, so anyone
driving
`CamelKafkaConnectMain.Builder` directly is unaffected. Only the sink task
opts in.
## Tests
`RemoveHeadersOrderTest` observes what the mid-route stages actually see, by
binding an
`AggregationStrategy` that records the header value it receives. Against the
old ordering, with
`camel.remove.headers.pattern=CamelExecCommandExecutable` explicitly
configured:
```
camel.remove.headers.pattern must strip the header before the aggregation
stage sees it on the
sink path, but the strategy saw: /bin/sh ==> expected: <null> but was:
</bin/sh>
```
The paired source-direction test asserts the strategy *does* still see the
header, pinning the
existing source behaviour so this change cannot silently alter it.
## Verification
- `core`: full suite passes.
- `./mvnw -Psourcecheck -Dcheckstyle.failOnViolation=true
checkstyle:checkstyle`: BUILD SUCCESS.
- Full reactor build from the repository root with the whole test suite
(`./mvnw clean install`):
BUILD SUCCESS, 24 test runs, no failures.
## Related
The issue also notes a gap this PR does not close: `removeHeaders` only acts
on message headers, while
`CamelSinkTask.put()` also maps `CamelProperty.`-prefixed record headers
into **exchange properties**
(`camel.map.properties`, default `true`), and there is no
`camel.remove.properties.pattern`
counterpart. That needs a new option rather than a reordering, so it seemed
worth deciding separately —
it is part of the discussion in #1807.
--
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]
