Add regression test for recreating dropped encrypt rule

[ym0506]

Fixes #38657

Summary

• add regression coverage for deleting named rule configuration items in standalone metadata
• add an RDL E2E case that recreates an encrypt rule after dropping the same rule name
• add the expected DistSQL result for the recreated encrypt rule

Motivation

This covers the issue reported in #38657, where dropping an encrypt rule could remove it from persisted metadata while runtime metadata still behaved as if the rule existed.

Test Plan

• ./mvnw -pl mode/type/standalone/core -am -DskipITs -DskipIT -DskipCheck -Dspotless.check.skip=true -Dcheckstyle.skip=true -Dlicense.skip=true -Djacoco.skip=true -DskipTests=false -DfailIfNoTests=false -Dsurefire.failIfNoSpecifiedTests=false -Dtest=StandaloneMetaDataManagerPersistServiceTest test
• ./mvnw -pl features/encrypt/distsql/handler -am -DskipITs -DskipIT -DskipCheck -Dspotless.check.skip=true -Dcheckstyle.skip=true -Dlicense.skip=true -Djacoco.skip=true -DskipTests=false -DfailIfNoTests=false -Dsurefire.failIfNoSpecifiedTests=false -Dtest=DropEncryptRuleExecutorTest,CreateEncryptRuleExecutorTest test
• git diff --check
• xmllint --noout test/e2e/sql/src/test/resources/cases/rdl/e2e-rdl-create.xml test/e2e/sql/src/test/resources/cases/rdl/dataset/distsql_rdl/create_recreated_encrypt_rule.xml

[terrymanu]

Merge Verdict: Not Mergeable

Reviewed Scope: PR #38685 latest head 3905e54, 3 changed files: standalone metadata unit test and RDL E2E XML fixtures.

Not Reviewed Scope: I did not run Maven tests locally; I relied on PR CI plus diff/source inspection.

Need Expert Review: No special security/parser/concurrency review needed for this test-only PR.

Positive feedback: the added coverage is aimed at the right path. It covers named rule item deletion through DatabaseRuleItem("named", "foo_rule"), and the RDL case exercises CREATE -> DROP -> CREATE -> SHOW for the same encrypt rule name, matching issue #38657’s stale runtime metadata symptom.

Newly Introduced Issues

[P1] Diff check fails on trailing whitespace

Symptom: git diff --check 6d761e5...apache/pr/38685 exits 2.

Evidence: mode/type/standalone/core/src/test/java/org/apache/shardingsphere/mode/manager/standalone/persist/service/StandaloneMetaDataManagerPersistServiceTest.java:215: trailing whitespace.

Risk: this conflicts with the project submission gate in CODE_OF_CONDUCT.md (line 17), especially the requirements that coding standards and build/style steps complete successfully.

Recommended action: please remove the trailing whitespace and rerun git diff --check.

Next Steps

Remove the whitespace-only issue in StandaloneMetaDataManagerPersistServiceTest.java.

Rerun:

git diff --check

xmllint --noout test/e2e/sql/src/test/resources/cases/rdl/e2e-rdl-create.xml test/e2e/sql/src/test/resources/cases/rdl/dataset/distsql_rdl/create_recreated_encrypt_rule.xml

the two scoped Maven commands listed in the PR, preferably with -am.

[ym0506]

Merge Verdict: Not Mergeable

Reviewed Scope: PR #38685 latest head 3905e54, 3 changed files: standalone metadata unit test and RDL E2E XML fixtures.

Not Reviewed Scope: I did not run Maven tests locally; I relied on PR CI plus diff/source inspection.

Need Expert Review: No special security/parser/concurrency review needed for this test-only PR.

Positive feedback: the added coverage is aimed at the right path. It covers named rule item deletion through DatabaseRuleItem("named", "foo_rule"), and the RDL case exercises CREATE -> DROP -> CREATE -> SHOW for the same encrypt rule name, matching issue #38657’s stale runtime metadata symptom.

Newly Introduced Issues

[P1] Diff check fails on trailing whitespace

Symptom: git diff --check 6d761e5...apache/pr/38685 exits 2.

Evidence: mode/type/standalone/core/src/test/java/org/apache/shardingsphere/mode/manager/standalone/persist/service/StandaloneMetaDataManagerPersistServiceTest.java:215: trailing whitespace.

Risk: this conflicts with the project submission gate in CODE_OF_CONDUCT.md (line 17), especially the requirements that coding standards and build/style steps complete successfully.

Recommended action: please remove the trailing whitespace and rerun git diff --check.

Next Steps

Remove the whitespace-only issue in StandaloneMetaDataManagerPersistServiceTest.java.

Rerun:

git diff --check

xmllint --noout test/e2e/sql/src/test/resources/cases/rdl/e2e-rdl-create.xml test/e2e/sql/src/test/resources/cases/rdl/dataset/distsql_rdl/create_recreated_encrypt_rule.xml

the two scoped Maven commands listed in the PR, preferably with -am.

Adjusted StandaloneMetaDataManagerPersistServiceTest so the existing assertRemoveRuleConfigurationItem() test covers both unique and named database rule items. This avoids the added separator-line conflict while preserving the original coverage and keeping the DatabaseRuleItem("named", "foo_rule") regression check.

Verified locally:

• git diff --check 6d761e53884e37bff7af1d2047221088565bbbdc...HEAD
• ./mvnw spotless:check -Pcheck -T1C
• ./mvnw checkstyle:check -Pcheck -T1C
• ./mvnw -pl mode/type/standalone/core -am ... -Dtest=StandaloneMetaDataManagerPersistServiceTest test
• ./mvnw -pl features/encrypt/distsql/handler -am ... -Dtest=DropEncryptRuleExecutorTest,CreateEncryptRuleExecutorTest test

GitHub checks are green, including Check - Spotless, Check - License, Check - CheckStyle, CI, E2E - SQL (Smoke), and E2E - SQL (Stage 2).

[terrymanu]

Decision

• Merge Verdict: Not Mergeable
• Reviewed Scope: PR #38685 latest head c6f6466dee91979cecb46ff2ffea477764e91a3f; changed files under mode/type/standalone/core/src/test/java/.../StandaloneMetaDataManagerPersistServiceTest.java, test/e2e/sql/src/test/resources/cases/rdl/e2e-rdl-create.xml, and test/e2e/sql/src/test/resources/cases/rdl/dataset/distsql_rdl/create_recreated_encrypt_rule.xml.
• Not Reviewed Scope: I did not run the full Maven suite or manually reproduce the original Proxy PostgreSQL + MySQL repository topology from #38657.
• Need Expert Review: No special parser/security review is needed; this is a metadata/runtime refresh regression-coverage question.

Positive Feedback

• The added coverage is in the right direction.
• StandaloneMetaDataManagerPersistServiceTest.java:195 now verifies that both unique and named rule item deletion paths trigger DatabaseRuleItemManager.drop(...).
• test/e2e/sql/src/test/resources/cases/rdl/e2e-rdl-create.xml:41 adds the important same-name CREATE -> DROP -> CREATE -> SHOW regression path, which matches the duplicate-rule symptom in #38657.

Major Issues

• [P1] The original DML rewrite symptom is still not directly validated (test/e2e/sql/src/test/resources/cases/rdl/e2e-rdl-create.xml:41)

  • Symptom: The new E2E case validates same-name recreation after DROP ENCRYPT RULE, but #38657 also reports that after DROP ENCRYPT RULE test02, SHOW ENCRYPT RULES no longer shows the rule while subsequent INSERT is still encrypted.
  • Risk: The duplicate-name failure and the unexpected encryption both depend on stale runtime EncryptRule state, but they are observed through different high-frequency paths. The duplicate check uses rule table names, while DML rewrite enters through EncryptSQLRewriteContextDecorator and EncryptRule.findEncryptTable(...). Without a post-drop DML assertion, this PR can prevent the duplicate-create regression while still leaving the original rewrite symptom unproven.
  • Action: Please add regression coverage for CREATE ENCRYPT RULE -> INSERT encrypted -> DROP ENCRYPT RULE -> SHOW no rule -> INSERT should no longer be rewritten/encrypted. If the SQL E2E framework cannot express the repository/topology exactly, please add the closest production-path integration/unit coverage and document the topology gap in the PR.

• [P2] The LOAD SINGLE TABLE follow-up from #38657 is not covered (test/e2e/sql/src/test/resources/cases/rdl/e2e-rdl-create.xml:43)

  • Symptom: The issue reports an additional sequence after drop: LOAD SINGLE TABLE ..., then insert failure, then same-name recreate failure. The current added test only runs CREATE -> DROP in initial-sql, then recreates and shows the rule.
  • Risk: LOAD SINGLE TABLE touches metadata reload behavior, which is adjacent to the stale metadata root cause. If this path has a separate interaction with standalone persisted metadata, the current test does not prove it.
  • Action: Please either extend the regression scenario to include the LOAD SINGLE TABLE sequence from #38657, or add a short explanation with evidence that this path is already covered by another test and cannot affect the stale encrypt rule state.

Next Steps

• Add one regression that proves DML rewrite is disabled after dropping the encrypt rule.
• Add or justify coverage for the LOAD SINGLE TABLE follow-up path from #38657.
• Rerun the existing scoped checks with -am, plus the affected SQL E2E case.
• Keep the existing named-rule-item deletion test; it is useful and should stay.

Multi-Round Comparison

• Previous blocker fixed: the trailing whitespace issue from the earlier review no longer reproduces; git diff --check 6d761e53884e37bff7af1d2047221088565bbbdc...apache/pr/38685 is clean.
• Remaining blocker: current coverage still proves only part of #38657’s reported behavior, not the full original symptom chain.

Evidence Supplement

• git diff --check 6d761e53884e37bff7af1d2047221088565bbbdc...apache/pr/38685 passed.
• xmllint --noout passed for the two changed XML resources.
• GitHub checks on latest head show relevant Check - Spotless, Check - License, Check - CheckStyle, CI, and distsql_rdl E2E jobs passing.

[ym0506]

Addressed the latest review in commit 3678c275.

Changes:

• Added a core rewrite-context regression verifying that after an encrypt table rule item is dropped from EncryptRuleConfiguration, EncryptSQLRewriteContextDecorator no longer registers rewrite token generators for INSERT on that table.
• Added a distsql_rdl E2E case for DROP ENCRYPT RULE -> LOAD SINGLE TABLE *.* -> SHOW ENCRYPT TABLE RULE, asserting the dropped rule remains absent.
• Added a distsql_rdl E2E case for CREATE ENCRYPT RULE -> INSERT while encrypted -> DROP ENCRYPT RULE -> INSERT after drop -> LOAD SINGLE TABLE *.*. The assertion checks that the pre-drop row is stored through the cipher column and the post-drop row stays plaintext with a null cipher column.

Verified locally:

• xmllint --noout test/e2e/sql/src/test/resources/cases/rdl/e2e-rdl-create.xml test/e2e/sql/src/test/resources/cases/rdl/dataset/distsql_rdl/insert_after_drop_encrypt_rule.xml
• ./mvnw -pl features/encrypt/core -am -DskipITs -Djacoco.skip=true -Dspotless.apply.skip=true -Dcheckstyle.skip=true -Dlicense.skip=true -DskipNativeImageTest=true -Dsurefire.failIfNoSpecifiedTests=false -Dtest=EncryptSQLRewriteContextDecoratorTest test
• ./mvnw spotless:check -Pcheck -pl features/encrypt/core
• ./mvnw checkstyle:check -Pcheck -pl features/encrypt/core
• ./mvnw apache-rat:check -Pcheck -pl test/e2e/sql

GitHub Actions checks are green for the pushed commit, including CI, Check - Spotless, Check - License, Check - CheckStyle, E2E - SQL (Smoke), and the relevant E2E - SQL (Stage 2) distsql_rdl jobs for jdbc, Standalone, MySQL, proxy, Cluster, MySQL, and proxy, Cluster, PostgreSQL.

[terrymanu]

Decision

• Merge Verdict: Not Mergeable
• Reviewed Scope: PR #38685 latest head 3678c2751adfb7664106607cbf468c65a8f5eb00; changed tests and RDL fixtures under features/encrypt/core, mode/type/standalone/core, and test/e2e/sql. I also checked the related metadata-drop and encrypt rewrite paths.
• Not Reviewed Scope: I did not run Maven locally or manually reproduce the exact PostgreSQL-protocol Proxy + MySQL repository topology from #38657. No parser, dependency, or production source changes were present.
• Need Expert Review: No special parser/security/supply-chain review is needed for this test-only PR.

Positive Feedback

• The latest revision closes the previous functional coverage gaps. It now covers named rule item deletion, same-name encrypt rule recreation, LOAD SINGLE TABLE *.* after drop, and post-drop DML rewrite behavior.
• The added E2E cases in test/e2e/sql/src/test/resources/cases/rdl/e2e-rdl-create.xml:50 and test/e2e/sql/src/test/resources/cases/rdl/e2e-rdl-create.xml:62 are aligned with the stale runtime metadata symptoms from #38657.
• GitHub Actions on the latest head show the relevant CI, Check - Spotless, Check - License, Check - CheckStyle, and distsql_rdl E2E jobs passing.

Major Issues

• [P1] Latest head still fails git diff --check (features/encrypt/core/src/test/java/org/apache/shardingsphere/encrypt/rewrite/context/EncryptSQLRewriteContextDecoratorTest.java:88)
  • Symptom: git diff --check apache/master...apache/pr/38685 exits with code 2 and reports trailing whitespace at lines 88 and 101.
  • Risk: This conflicts with the project submission/style gate in CODE_OF_CONDUCT.md:18 and CODE_OF_CONDUCT.md:19, and it repeats the same class of formatting issue that was already requested in an earlier review round.
  • Action: Please remove the trailing spaces on the blank lines around the new test/helper, then rerun git diff --check apache/master...apache/pr/38685.

Next Steps

• Remove the trailing whitespace at EncryptSQLRewriteContextDecoratorTest.java:88 and EncryptSQLRewriteContextDecoratorTest.java:101.
• Rerun git diff --check apache/master...apache/pr/38685.
• Keep the current DML and LOAD SINGLE TABLE regression coverage; I did not find another functional blocker in this pass.

Multi-Round Comparison

• Fixed: the earlier whitespace issue in StandaloneMetaDataManagerPersistServiceTest.java is gone.
• Fixed: the previous DML rewrite coverage blocker is addressed by EncryptSQLRewriteContextDecoratorTest.java:89 and the new RDL DML dataset.
• Fixed: the previous LOAD SINGLE TABLE coverage blocker is addressed by the new RDL cases at e2e-rdl-create.xml:50 and e2e-rdl-create.xml:62.
• Newly introduced issue: the latest head adds trailing whitespace in EncryptSQLRewriteContextDecoratorTest.java, so the PR is still not mergeable until the diff check is clean.

[ym0506]

Addressed the latest whitespace review in commit febb648f.

The post-drop encrypt rewrite regression is now folded into the existing assertDecorateWithoutEncryptTable() test instead of adding a separate test/helper pair. This keeps the coverage while avoiding newly added blank separator lines that Spotless preserves but git diff --check reports as trailing whitespace.

Verified locally:

• git diff --check origin/master...HEAD
• ./mvnw spotless:check -Pcheck -pl features/encrypt/core
• ./mvnw checkstyle:check -Pcheck -pl features/encrypt/core
• ./mvnw -pl features/encrypt/core -am -DskipITs -Djacoco.skip=true -Dspotless.apply.skip=true -Dcheckstyle.skip=true -Dlicense.skip=true -DskipNativeImageTest=true -Dsurefire.failIfNoSpecifiedTests=false -Dtest=EncryptSQLRewriteContextDecoratorTest test

[ym0506]

CI update after the latest push:

• The main CI job, CheckStyle, Spotless, and License checks passed.
• The relevant distsql_rdl E2E jobs passed for MySQL and PostgreSQL.
• The only remaining red E2E job is E2E - SQL (Stage 2) (proxy, Cluster, MySQL, encrypt_shadow): https://github.com/apache/shardingsphere/actions/runs/26507917844/job/78068307991

That job failed during environment setup while downloading Maven from Maven Central:

wget: Failed to fetch https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.9.14/apache-maven-3.9.14-bin.zip

I tried to rerun the failed job, but GitHub requires repository admin permissions for this workflow rerun. Could a maintainer rerun the failed job when available?