[type:fix] scope the meta_data path check on update by namespace - #7210
HY-love-sleep wants to merge 6 commits into
Conversation
Aias00
left a comment
There was a problem hiding this comment.
Summary
MetaDataMapper#pathExistedExclude checked path uniqueness globally. After namespaces were introduced, updating a meta-data row could be rejected because the same path existed in a different namespace. This PR adds namespace_id to the predicate so the uniqueness check is scoped to the row's own namespace.
Review
- The mapper signature gains
@Param("namespaceId") String namespaceIdand the sqlmap gainsAND namespace_id = #{namespaceId}in thepathExistedExcludeselect. Correct and minimal. - I checked the branch head for other call sites:
MetaDataServiceImpl#update(line ~301) is the only production caller, and it is updated.MetaDataServiceTest#testCreateOrUpdateForUpdateis updated too. No orphaned call sites remain, unlike the similar change in #7001. MetaDataServiceImpl#createis untouched — it usespathExisted/existrather thanpathExistedExclude, which is fine, since this PR only targets the update path.- The new
pathExistedExcludeIsScopedByNamespacetest is a real behavioural test against the mapper: it inserts a row innamespace-a, then asserts (a) the same path in the same namespace still collides, (b) the path innamespace-bdoes not, and (c) the row being updated is excluded from its own check. That covers all three branches of the predicate.
CI
Green. Note: the Analyze (java) (CodeQL) job reports failure, but its log shows only the CodeQL Action v3 deprecation notice and the Node 20 → Node 24 runner warning, with no findings against this change — infrastructure, not a code issue. The ci workflow on the same head commit is success.
Notes (non-blocking)
One thing to watch: if metaDataDTO.getNamespaceId() is ever null, namespace_id = NULL matches no rows under SQL three-valued logic, so the check silently passes and uniqueness stops being enforced — the failure mode degrades from "correctly rejected" to "silently allowed". Please confirm MetaDataDTO#getNamespaceId is always populated on the update path (it appears to be, since the DTO is built from an existing row). If it can be null, defaulting to Constants.SYS_DEFAULT_NAMESPACE_ID would keep the check meaningful.
LGTM.
What
meta_data.pathExistedExclude(used when updating a meta_data) filters only bypathand the excluded ids, while its siblingpathExisted(used when creating)filters by
path AND namespace_id. As a result an update was rejected withDATA_PATH_IS_EXISTwhenever the path existed in another namespace, even thoughthe path is unique inside the namespace being updated.
The query now scopes by namespace too:
<select id="pathExistedExclude" resultType="java.lang.Boolean"> SElECT true FROM meta_data WHERE path = #{path} + AND namespace_id = #{namespaceId} AND id NOT INMetaDataMapper#pathExistedExcludetakes thenamespaceIdright afterpath, mirroringpathExisted(path, namespaceId), andMetaDataServiceImpl#updatepassesmetaDataDTO.getNamespaceId()— the same value the create path already uses.Why
Closes #6689.
Verified
MetaDataMapperTest#pathExistedExcludeIsScopedByNamespace(new, runs on H2 throughAbstractSpringIntegrationTest) inserts/namespace-scoped-pathinnamespace-aand asserts:the same path in
namespace-bno longer counts as an existing path (the reported bug),a duplicate in the same namespace is still rejected,
the row being updated is still excluded from its own check.
./mvnw -pl shenyu-admin test -Dtest='MetaDataMapperTest,MetaDataServiceTest'→ Tests run: 32, Failures: 0, Errors: 0the new test fails on the unfixed query (checked by reverting the mapper change), so it guards the behaviour
checkstyle: 0 violations
Note
#6809 reports the same query from a performance angle (cross-namespace scan,
NOT IN,unindexed
path). The namespace filter removes the cross-namespace scan here; thesuggested
(namespace_id, path)index needs a schema change across all supporteddatabases, so I deliberately kept it out of this PR and left it as a follow-up.