oscerd commented on code in PR #27474:
URL: https://github.com/apache/camel/pull/27474#discussion_r4204308495
##########
docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc:
##########
@@ -2052,6 +2052,17 @@ now reports a top-level `from:` with the form to write,
the way it reports the o
form as well; a Kamelet's `template:` keeps its `from:`, which is the Kamelet
spec's shape. The classic schema and `camel run` are unchanged: the file keeps
working,
with the compact notation warning `camel run` already logged for it.
+=== camel-yaml-dsl - endpoint parameters keep the order they are written in
Review Comment:
**User-visible API output change without a Jira**
This changes what the public `EndpointUriFactory.buildUri` and
`CamelCatalog`/`RuntimeCamelCatalog.asEndpointUri` return for ordered maps and
adds a 4.23 upgrade-guide entry, but the PR is a `chore:` with no CAMEL-XXXX. A
Jira (or at least a reference to CAMEL-25381 / #27449, whose dump sorting this
reverts) would help release notes and downstream tooling (Kaoto, Karavan,
camel-quarkus). The ordered-vs-unordered rule is also only stated in the
upgrade guide and the protected `copyParameters` Javadoc; one sentence in the
Javadoc of `buildUri` / `asEndpointUri` would make it discoverable.
##########
core/camel-support/src/main/java/org/apache/camel/support/component/EndpointUriFactorySupport.java:
##########
@@ -93,10 +95,24 @@ protected String buildPathParameter(
return uri;
}
+ /**
+ * A copy of the parameters to build the uri from: in their order when
they have one (a {@link LinkedHashMap} such
+ * as the parameters of a route in the YAML DSL, which then keep the order
they are written in), otherwise sorted.
+ */
+ protected static Map<String, Object> copyParameters(Map<String, Object>
parameters) {
+ if (isOrdered(parameters)) {
Review Comment:
**Unordered input with a multi-value option is no longer sorted as before
(the SortedMap branch at line 130 is dead on the generated path)**
In the generated `buildUri`, a `HashMap` is copied by `copyParameters` into
a `TreeMap`; `buildQueryParameters` then calls `copyParameters(copy)` again,
and since a `TreeMap` counts as ordered it returns a `LinkedHashMap`. So `map
instanceof SortedMap` at line 130 is never true on this path, and the flattened
`prefix.key` options are spliced in at the position of the multi-value option
name instead of being sorted with the other keys. That changes the output
wherever a prefix sorts differently from its option name (bean/class
`parameters` → `bean.`, jdbc/sql `parameters` → `statement.`, mail
`additionalJavaMailProperties` → `mail.`, jpa `entityManagerProperties` →
`emf.`, jmx `objectProperties` → `key.`): e.g.
`bean:foo?bean.x=1&method=hello&scope=Request` on main becomes
`bean:foo?method=hello&bean.x=1&scope=Request`, while the upgrade guide says a
HashMap "has its options sorted as before". The resolved endpoint is unaffected
(normalizeUri sorts), only the string chang
es. The hand-written assemblers in the tests pass the raw HashMap straight to
`buildQueryParameters`, so they don't catch it. Keeping a sorted map sorted:
```suggestion
if (parameters instanceof SortedMap<String, Object> sorted) {
// stay sorted, so buildQueryParameters also sorts the flattened
multi-value options (as before)
return new TreeMap<>(sorted);
}
if (isOrdered(parameters)) {
```
plus a test that, like the generated code, calls `copyParameters` before
`buildQueryParameters` with a HashMap and a multi-value prefix that sorts
differently from its option name.
--
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]
