Skip to content

[type:fix] scope the meta_data path check on update by namespace - #7210

Open
HY-love-sleep wants to merge 6 commits into
apache:masterfrom
HY-love-sleep:fix/meta-data-path-namespace
Open

HY-love-sleep wants to merge 6 commits into
apache:masterfrom
HY-love-sleep:fix/meta-data-path-namespace

Conversation

@HY-love-sleep

Copy link
Copy Markdown
Contributor

What

meta_data.pathExistedExclude (used when updating a meta_data) filters only by
path and the excluded ids, while its sibling pathExisted (used when creating)
filters by path AND namespace_id. As a result an update was rejected with
DATA_PATH_IS_EXIST whenever the path existed in another namespace, even though
the 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 IN

MetaDataMapper#pathExistedExclude takes the namespaceId right after path, mirroring
pathExisted(path, namespaceId), and MetaDataServiceImpl#update passes
metaDataDTO.getNamespaceId() — the same value the create path already uses.

Why

Closes #6689.

Verified

MetaDataMapperTest#pathExistedExcludeIsScopedByNamespace (new, runs on H2 through
AbstractSpringIntegrationTest) inserts /namespace-scoped-path in namespace-a and asserts:

  • the same path in namespace-b no 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: 0

  • the 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; the
suggested (namespace_id, path) index needs a schema change across all supported
databases, so I deliberately kept it out of this PR and left it as a follow-up.

@Aias00 Aias00 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 namespaceId and the sqlmap gains AND namespace_id = #{namespaceId} in the pathExistedExclude select. 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#testCreateOrUpdateForUpdate is updated too. No orphaned call sites remain, unlike the similar change in #7001.
  • MetaDataServiceImpl#create is untouched — it uses pathExisted/exist rather than pathExistedExclude, which is fine, since this PR only targets the update path.
  • The new pathExistedExcludeIsScopedByNamespace test is a real behavioural test against the mapper: it inserts a row in namespace-a, then asserts (a) the same path in the same namespace still collides, (b) the path in namespace-b does 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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] meta_data.pathExistedExclude ignores namespace_id → cross-namespace false-positive path collision

2 participants