allthingssecurity opened a new pull request, #28809: URL: https://github.com/apache/camel/pull/28809
# Description [CAMEL-26452](https://issues.apache.org/jira/browse/CAMEL-26452) A custom `BeanIOErrorHandler` can turn an invalid record into a result with `handleErrorAndAddAsResult(Object)` (the `MyErrorHandler` of the component tests does this). The `beanio` data format adds that result to its list. `BeanIOSplitter` gives it to its `BeanIOIterator` with `setNext(result)`. The error handler runs inside `BeanReader.read()`. After it returns, BeanIO goes on reading, and the same `read()` returns the next valid record (BeanIO 3.2.1 `BeanReaderImpl.read()` loops until it has a record or reaches the end). `BeanIOIterator` kept the result in a single field and then put it in place of that record: ```java next = reader.read(); // the valid record after the invalid one if (forceNext != null) { next = forceNext; // ... is replaced by the result of the error handler forceNext = null; } ``` Two things went wrong: - The valid record after each handled record was not split. - When two records in a row were handled, only the last result was kept. Nothing was logged. Only a single handled record at the end of the data worked, which is the case the existing `testSplitFail` covers. This happens with any BeanIO stream format. | Records | `split(splitter).streaming()` on main | unmarshal with the data format | |---|---|---| | Joe, Jane (invalid salary), Jon | Joe, ErrorDto | Joe, ErrorDto, Jon | | Joe, Jane (invalid), Jim (invalid), Jon, Jack | Joe, ErrorDto, Jack | Joe, ErrorDto, ErrorDto, Jon, Jack | A second problem sits next to it in `BeanIOSplitter.evaluate`. The splitter read the first record (in the `BeanIOIterator` constructor) before it set the error handler on the reader. So an invalid, unexpected or unidentified first record failed the split even with the matching ignore option (`ignoreInvalidRecords` and so on) or a custom error handler. With `ignoreInvalidRecords=false` and no custom handler, the split fails with the same `InvalidRecordException` as before; the only difference there is the WARN line of `BeanIOErrorHandler`, as for any other invalid record. Also, when the error handler could not be created (for example a wrong `beanReaderErrorHandlerType`), the reader of the body was not closed. Changes: - `BeanIOIterator` queues the results that the error handler adds during a read. It returns them in the order they were added, followed by the record that the read returned. This is the order the data format's list has. `setNext` adds to the queue; a `null` is ignored. - Results of handled records in a row are kept until the reader returns the next record. This comes from BeanIO reading on inside one `read()`. The data format keeps them all anyway. - `BeanIOSplitter` creates the iterator without reading, sets the error handler, then reads the first record (a package-private constructor and `readFirst()`; the public constructor still reads the first record). The reader is closed when any of these steps fails. - The javadoc of `setNext` says so, and an entry in the 4.24 upgrade guide, since a route now gets records it did not get before. Tests: `BeanIOSplitterCustomBeanReaderErrorHandlerTest` gets 4 tests: - an invalid record in the middle; - two invalid records in a row followed by two valid records, with the order of the two results pinned (their messages name line 2 and line 3; a variant that returns the results in reverse order fails this test); - an invalid first record; - an error handler type that does not exist: the split fails (caused by `ClassNotFoundException`) and the reader of the body is closed (the route turns stream caching off, else stream caching reads and closes the reader first). All 4 fail on main and pass with the change; they were checked against main's `BeanIOIterator` and `BeanIOSplitter` compiled over the branch. The test class (6 tests, 4 of them new) ran 20 times in one JVM: 120 tests, 0 failures. All tests of camel-beanio pass: 30, 0 failures. The formatter and import-sort plugins ran, and the tree was clean afterwards. Backport: the same `forceNext` code is on the 4.14.x, 4.18.x and 4.22.x branches. It is silent data loss, so it is a candidate for a backport. The `BeanIOSplitter` part sits on the try blocks that CAMEL-25904 added on main only, so a backport has to adapt it (a cherry-pick conflicts in `BeanIOSplitter.java`, and on 4.14.x also in `BeanIOIterator.java`). Not changed: a `BeanIOErrorHandler` *instance* set with `beanReaderErrorHandler` (rather than a type) is shared by all exchanges, and `getOrCreateBeanReaderErrorHandler` sets the current exchange, list and iterator on it. So two exchanges split at the same time can get each other's results. I can open a JIRA for it if wanted. # Target - [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch) # Tracking - [x] If this is a large change, bug fix, or code improvement, I checked there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL-26452) filed for the change (usually before you start working on it): [CAMEL-26452](https://issues.apache.org/jira/browse/CAMEL-26452). # Apache Camel coding standards and style - [x] I checked that each commit in the pull request has a meaningful subject line and body. - [ ] I have run `mvn clean install -DskipTests` locally from root folder and I have committed all auto-generated changes. (I built camel-beanio and ran its tests, including the formatter and import-sort plugins. I did not run the full root build. No option or generated file changed.) # AI-assisted contributions - [x] If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., `Co-authored-by` trailers) and the PR description identifies the AI tool used. This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a `Co-Authored-By` trailer. _Claude Code on behalf of allthingssecurity_ 🤖 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]
