Repository navigation
fix(dubbo): resolve single name-mapped parameters instead of using the mapping JSON as a type name (#7736) - #7746
Conversation
Aias00
left a comment
There was a problem hiding this comment.
LGTM. I traced the whole method rather than just the guard, because the risk here is that the map path does not handle a single mapped POJO.
Evidence
- Before: a single-entry mapping fell into
buildSingleParameter(body, parameterTypes), which treats the raw{"arg":"..."}JSON as the type name and keeps theargwrapper in the value. Both halves of the bug are real. - After: the map path builds
paramNameListfrom the mapping keys andparamTypeListfrom the mapping values, then for each key takes the body value and converts aJsonObjecttoMap/ aJsonArraytoList. So a single mapped POJO now arrives asMapunder its declared type — which is the same shape the non-mapped single-bean path produces (testPlainSingleBeanKeepsWholeBodySemantics), i.e. the Dubbo generic-invocation convention is preserved rather than changed. - A single mapped base type never took the shortcut (
isBaseTypealready excluded it), so that path is provably untouched, and you pin it with a test. isNameMappingrequires the string to start with{and end with}, so a plain comma-separated type list can never be misclassified.buildSingleParameteritself is unchanged, so no other caller shifts behaviour, as the PR body claims.
Nits
- The shortcut condition is now a three-clause expression and
isNameMapping(parameterTypes)is evaluated twice per call (once for the branch above, once here). It is cheap, but a local boolean would read better and cost nothing. - Worth one line in the javadoc of
buildParameters: "name-mapped metadata never uses the whole-body shortcut" — this is the invariant the next person needs to keep.
CI: pr_build green. The red e2e jobs (http-sync, apache-dubbo, cluster-jdbc/zookeeper, rocketmq) all end with ##[error]The operation was canceled. and every case that actually executed reported Failures: 0 — the workflow was cancelled at ~5 min, so this is infrastructure, not the change. Please rerun e2e before merge.
|
Ping on state: this PR is approved on the code, but the required checks are red and it cannot be merged yet. The failing jobs are No code change is expected from you. Once the shared e2e problem is resolved, please rerun the failed jobs (or push a trivial rebase) so the required checks go green, and this can be merged. |
|
@konlue This PR is approved, but 9 checks are red: I inspected these lanes today and they fail for infrastructure reasons rather than on the diff — the Please rebase onto |
…e mapping JSON as a type name (apache#7736)
96ea0d6 to
ef3ecd7
Compare
|
Rebased onto the latest master to re-trigger the checks; the earlier failures were the known infra issues. |
Fixes #7736.
Summary
BodyParamUtils#buildParameterstakes the legacy whole-body single-argument path whenever the resolved type list has exactly one non-base entry — including a single-entry JSON name mapping.buildSingleParameterthen uses the original mapping JSON as the parameter type and keeps the outer parameter-name wrapper in the value, so generic type resolution fails or binds the wrong shape.Modifications
!isNameMapping(parameterTypes)); name-mapped parameters always use the resolved name/type/value path, regardless of entry count.BodyParamUtilsTestcovering: single-entry mapping →["java.util.Map"]/ bare map argument; multi-entry mapping ordering; plain single bean keeps whole-body semantics; single mapped base type unchanged.Verifying this change
Red-green: with the original shortcut restored,
testSingleNameMappedParameterUsesResolvedTypeAndValuefails (the type is the mapping JSON and the argument keeps theargwrapper); with the fix all four tests pass.Targeted tests pass:
./mvnw -B -ntp -pl shenyu-plugin/shenyu-plugin-api test -Dtest=BodyParamUtilsTest4/4 green.
Checkstyle passes:
clean.
Notes
buildSingleParameteritself is unchanged, so any direct caller keeps its behaviour; only the name-mapping branch insidebuildParameterschanges. Multi-parameter mappings and non-mapped plain-type calls are untouched.