Aias00 commented on PR #6535:
URL: https://github.com/apache/shenyu/pull/6535#issuecomment-5156774509
Reviewed #6535 — fix for #6520. Tight and correct.
**Change scope (2 files, +9 lines):** `TagServiceImpl.createInner` now calls
`Assert.notNull(tagDO, "parent tag is not found")` right after
`tagMapper.selectByPrimaryKey(parentTagId)`, so a child-tag create with a
non-existent `parentTagId` throws `ValidFailException` instead of NPE-ing
inside `buildExtParamByParentTag(null)` (which dereferences
`parentTagDO.getId()` at line 203). The root-parent path (`else` branch) is
untouched, and the only other caller of `buildExtParamByParentTag` —
`recurseUpdateTag` — feeds it `allData.get(id)` where `id` always comes from
`allDataMap` keys, so it can't regress. `createRootTag`'s only production
caller (`PullSwaggerDocServiceImpl`) passes `TAG_ROOT_PARENT_ID`, so it takes
the root branch and is unaffected.
**Exception routing verified:** `ValidFailException` →
`ShenyuAdminException` → `ShenyuException`, caught by
`@ExceptionHandler(ShenyuException.class)` in `ExceptionHandlers`, returning
`ShenyuAdminResult.error("parent tag is not found")`. Replaces the previous
generic NPE response — strictly better for callers.
**Test:** `testCreateWithNonExistentParentTag` stubs
`selectByPrimaryKey("456")` → null and asserts `ValidFailException`. On master
(without the guard) this path throws NPE, so the test genuinely fails before
the fix and passes after — a real regression test, not a tautological one.
**Two nits (non-blocking):**
1. The new test asserts only the exception type. Consider also asserting
`getMessage().equals("parent tag is not found")` to lock the user-facing
contract the PR body describes.
2. Wording: the sibling guard at line 88 is `"the updated tag is not
found"`; the new one is `"parent tag is not found"` (missing leading article).
Trivial parallelism nit only.
No blockers. CI green, mergeState BLOCKED only pending review.
--
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]
