strongduanmu commented on PR #38677:
URL: https://github.com/apache/shardingsphere/pull/38677#issuecomment-4410849963
Merge Verdict: Mergeable
Reviewed Scope:
- features/encrypt/core: AESEncryptAlgorithm, MD5AssistedEncryptAlgorithm,
encrypt column/rewrite call paths
- features/mask/core: MD5MaskAlgorithm
- infra/algorithm/type/cryptographic/type/aes: AESCryptographicAlgorithm
- infra/algorithm/type/message-digest/spi: MessageDigestAlgorithm
- Related AES/MD5 unit tests and GitHub CI/E2E status
Not Reviewed Scope:
- Commercial edition implementation details
- Full downstream custom SPI implementation matrix
Need Expert Review:
- No, based on the confirmed compatibility requirement for commercial
edition.
Basis:
- The change aligns AES and MD5-related encrypt/digest return types with
the existing Object-based algorithm contract.
- EncryptAlgorithm.encrypt and CryptographicAlgorithm.encrypt already
return Object, so AESEncryptAlgorithm and AESCryptographicAlgorithm are now
consistent with the public contract.
- The MessageDigestAlgorithm return type change is intentional for
commercial edition compatibility.
- Built-in AES/MD5 behavior remains unchanged for existing String and null
cases.
- No unrelated changes were found in the PR scope.
Risk Assessment:
- Design: acceptable; the change keeps responsibility within algorithm and
feature integration layers.
- Performance: no new loops, blocking I/O, remote calls, or extra hot-path
allocations were introduced.
- Compatibility: the SPI contract change is treated as intentional and
required for downstream compatibility.
- Regression surface: existing AES, MD5 assisted encrypt, MD5 mask, and
cryptographic algorithm tests pass.
- Documentation: no user-facing behavior change for built-in algorithms
was identified.
Pre-merge Checks:
- GitHub CI passed: Required - Check, CI, E2E - SQL, E2E - Agent, E2E -
Operation.
- Local scoped verification passed with `-am`:
`./mvnw -pl
infra/algorithm/type/cryptographic/type/aes,infra/algorithm/type/message-digest/type/md5,features/encrypt/core,features/mask/core
-am -DskipITs -Dspotless.skip=true
-Dtest=AESCryptographicAlgorithmTest,MD5MessageDigestAlgorithmTest,AESEncryptAlgorithmTest,MD5AssistedEncryptAlgorithmTest,MD5MaskAlgorithmTest
-Dsurefire.failIfNoSpecifiedTests=false test`
Result: exit code 0.
No blocking issue from my side.
--
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]
