allthingssecurity opened a new pull request, #26993: URL: https://github.com/apache/camel/pull/26993
# Description [CAMEL-25092](https://issues.apache.org/jira/browse/CAMEL-25092) A normal, 0-based list of 11 or more nested beans configured with property binding silently loses elements: ``` camel.beans.cluster = #class:com.foo.Cluster camel.beans.cluster.servers[0].host = host0 camel.beans.cluster.servers[0].port = 1000 ... camel.beans.cluster.servers[11].host = host11 camel.beans.cluster.servers[11].port = 1011 main: 10 servers, host10 and host11 are gone, no error ``` Two things combine: - `bindProperties` sorts the keys with `PropertyBindingKeyComparator`, which ends with a plain `String.compareTo`. So `servers[10].host` is bound before `servers[1].host`. - For a key that goes through a list element, `getOrCreatePropertyOgnlPathViaReflection` and `getOrCreatePropertyOgnlPathViaConfigurer` look the element up with `list.size() > idx ? list.get(idx) : null`. If there is none, they create it and call `list.add(instance)`, which appends it at the end of the list whatever the index is. So the host of element 10 lands at index 1 and its port at index 2, and then the keys of elements 1 and 2 overwrite those two elements. The same append spreads the keys of one element over several elements whenever the index is not the next free position: `servers[1].host=a, servers[1].port=1` gives `[{a, 0}, {null, 1}]`. A single key such as `names[3]=x` (`ObjectHelper.addListByIndex`) and an array property already put the value at its index and pad with null, and so does a list whose elements are declared first with `#class:` (the CAMEL-15396 example). Only an element created for a nested key was appended. The list key `last` is documented in `property-binding.adoc` and in the class javadoc ("To refer to the last element, then use last as key"). It has been there since 3.0.0 but was never implemented: every index goes through `Integer.parseInt`, so `names[last]=z` fails with `NumberFormatException`. This change: - Both getOrCreate methods create a missing element with `ObjectHelper.addListByIndex(list, idx, instance)` when the key has an index. The empty key `[]` still appends. `addListByIndex` sets the element when the index is inside the list, so a null slot left by padding is filled in place and nothing shifts. With this the order of the keys no longer matters, so the comparator is left as it is. - A small `listIndex` helper maps `last` to the index of the last element (0 for an empty list) at the four places that parse a list index (the two single-key setters and the two getOrCreate methods). Arrays keep numeric indexes, as the docs only mention List. - `property-binding.adoc` and the class javadoc say that an index beyond the end of the list pads it with null. - Upgrade guide 4.23: a section on the null padding and on `last`. Compatibility: a configuration that numbers its elements from 1, or leaves a gap, with one key per element (`servers[1].host=a`, `servers[2].host=b`) works today by accident, because the elements are appended and the list has no gap (`[a, b]`). With this change the list is `[null, a, b]`, the same as for a single key or an array, so code that iterates the list can meet the null. The upgrade guide entry says so. If you would rather not implement `last`, I can drop that part and remove the sentence from the docs instead. Tests: - `PropertyBindingSupportListTest`, 7 new tests, all without declaring the elements: - `testPropertiesListNestedMoreThanTenElements`: `bar.works[0..11].id/name`, size 12 and every element right. - `testPropertiesListNestedWithGapsNoDeclaration`: `works[1].id` and `works[1].name` give `[null, {123, Acme}]`. - `testPropertiesListNestedSparse`: `works[0].name`, `works[5].id`, `works[5].name` give size 6 with null at 1 to 4. - `testPropertiesListNestedFromOne`: `works[1].name`, `works[2].name` give `[null, One, Two]` (the compatibility case). - `testPropertiesListNestedWithGapsViaConfigurer`: the configurer path, with a configurer that implements `getCollectionValueType` and reflection turned off. - `testPropertiesListLast` and `testPropertiesListLastEmpty`: `works[last].id/name` and `names[last]` on a list with two elements and on an empty list. - The existing tests (dense, gaps at the leaf, `#class:` declared elements, `[]`) are unchanged. - `MainBeansTest.testBindBeansNestedListMoreThanTenElements`: the same 12 servers through Camel Main `camel.beans.*`. Without the change in `PropertyBindingSupport`, the 7 new camel-core tests fail (for example `expected: <12> but was: <10>`, and `NumberFormatException` for `last`), and so does the camel-main test (`expected: <12> but was: <10>`). With it, `PropertyBindingSupport*,*PropertyBinding*,*Properties*` in camel-util, camel-core and camel-console pass: 198 tests, 0 failures, among them the 77 tests of the 11 `PropertyBindingSupport*Test` classes of camel-core. camel-main: `camel-main` has a test dependency on `camel-ai-observability-api`, whose own test dependencies (langchain4j) are not in my offline Maven repository, so I could not run the camel-main tests with Maven. I compiled camel-main and its `MainBeans*Test` and `PropertyBindingSupport*Test` classes against the built modules and ran them with the JUnit launcher: 39 of 44 pass. The 5 that fail (`PropertyBindingSupportOptionalValueTest`, `PropertyBindingSupportRootArray*Test`) check `getInvokedCounter()`, which needs the test configurers that the annotation processor generates in the Maven build, and they fail the same way without this change. I left the camel-main Maven run to CI. Found with a Lean model of the element lookup, which shows that for any list and any index beyond its end, two keys with the same index are bound on two different elements. A review then found the case with 11 or more elements, which comes from the key order and is not in the model, and I reproduced both against the real classes. # 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) filed for the change (usually before you start working on it). # 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 and tested the affected modules, including the formatter and import-sort plugins. I did not run the full root build.) # 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]
