fix:update sql file,fix bug #284
Conversation
WalkthroughThis pull request updates database migration scripts across H2, MySQL, and Docker deployment configurations. Key changes include reformatting SQL files, schema modifications to the t_tenant table (index restructuring and column constraints), addition of a hash column to t_resource, updates to tenant initialization data, and adjustments to Java service logic for uniqueness checks and HTTP exception handling. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
base/src/main/java/com/tinyengine/it/common/exception/GlobalExceptionAdvice.java (3)
43-49: Critical: HTTP 200 OK for exceptions violates HTTP semantics.Returning
HttpStatus.OKfor generic exceptions breaks HTTP standards and REST API conventions. Clients, monitoring tools, load balancers, and API gateways rely on status codes to detect failures.Impact:
- Clients cannot distinguish success from failure
- Breaks automated error handling, retries, and circuit breakers
- Monitoring/alerting systems won't detect failures
- Makes production debugging difficult
🔎 Recommended fix
- @ResponseStatus(HttpStatus.OK) + @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR) @ExceptionHandler(Exception.class) public Result<Map<String, String>> handleException(Exception e) { // 修改为 log.error，传递异常对象以打印堆栈信息 log.error("Exception occurred: ", e); return Result.failed(ExceptionEnum.CM001); }
72-78: Critical: HTTP 200 OK for business exceptions violates HTTP semantics.
ServiceExceptionshould return an appropriate 4xx or 5xx status code based on the error type, not200 OK. This makes it impossible for clients and infrastructure to detect failures.Consider:
400 Bad Requestfor client errors422 Unprocessable Entityfor validation failures500 Internal Server Errorfor server-side issues🔎 Recommended fix (example with 400)
- @ResponseStatus(HttpStatus.OK) + @ResponseStatus(HttpStatus.BAD_REQUEST) @ExceptionHandler(ServiceException.class) public Result<Map<String, String>> handleServiceException(ServiceException e) { // 修改为 log.error，传递异常对象以打印堆栈信息 log.error("Business exception occurred: ", e); return Result.failed(e.getCode(), e.getMessage()); }Alternatively, map
ServiceExceptionerror codes to appropriate HTTP status codes dynamically.
58-64: Existing handlers also return HTTP 200 for errors.The handlers for
NullPointerException(line 58) andMethodArgumentNotValidException(line 86) already useHttpStatus.OK, which has the same issues described above. Consider addressing these as well:
NullPointerException→500 Internal Server ErrorMethodArgumentNotValidException→400 Bad RequestAlso applies to: 86-95
♻️ Duplicate comments (1)
docker-deploy-data/mysql/init/07_update_table_ddl_2025_1014.sql (1)
33-33: Critical: Same data loss risk as H2 migrationThis MySQL migration has the identical DROP COLUMN issue as the H2 version (app/src/main/resources/sql/h2/07_update_table_ddl_2025_1014.sql line 33). The data loss affects:
tenant_id,site_id,renter_id,created_by,last_updated_bydata (permanently lost)Since this affects both database platforms, verify that:
- This is part of a coordinated deployment that includes data migration
- Production databases have proper backups before applying
- The deployment process is documented and tested
🧹 Nitpick comments (3)
app/src/main/resources/sql/mysql/08_update_table_ddl_2025_1217.sql (1)
1-2: Consider nullability constraint and indexing for the hash column.The new
hashcolumn lacks an explicitNULL/NOT NULLconstraint (will default to nullable) and has no index. Consider:
- Nullability: If
hashis required for new resources, addNOT NULLand plan a backfill for existing rows.- Indexing: If the hash is used for lookups or uniqueness checks, add an index to prevent full table scans.
🔎 Suggested improvements
If the hash should be required and indexed:
ALTER TABLE t_resource - ADD COLUMN hash VARCHAR(100) AFTER thumbnail_url; + ADD COLUMN hash VARCHAR(100) NOT NULL AFTER thumbnail_url, + ADD INDEX idx_resource_hash (hash);If it should be nullable but still indexed for performance:
ALTER TABLE t_resource - ADD COLUMN hash VARCHAR(100) AFTER thumbnail_url; + ADD COLUMN hash VARCHAR(100) AFTER thumbnail_url, + ADD INDEX idx_resource_hash (hash);docker-deploy-data/mysql/init/08_update_table_ddl_2025_1217.sql (1)
1-2: Schema change addshashcolumn tot_resource.The column addition is consistent across MySQL and H2 environments. Consider adding a column comment to document the purpose of the
hashfield (e.g., for content deduplication or integrity verification).🔎 Optional: Add column comment
ALTER TABLE t_resource - ADD COLUMN hash VARCHAR(100) AFTER thumbnail_url; + ADD COLUMN hash VARCHAR(100) COMMENT '资源内容哈希值' AFTER thumbnail_url;docker-deploy-data/mysql/init/09_update_table_ddl_2025_1229.sql (1)
1-5: Data migration validation recommended but existing controls mitigate risksThe schema changes shift the unique constraint from
name_cntoname_en, makingname_enNOT NULL. The original table definition showsname_enwas previously nullable andname_cnheld the unique index.Risk assessment:
- Test data contains no NULL or empty
name_envalues (verified: public tenant hasname_en='public')- Application code in
TenantServiceImpl.createTenant()already enforcesname_enuniqueness via runtime check before insertion (lines 116-121)- Existing controls effectively mitigate the stated risks
Recommendation: While the migration appears safe given existing application constraints, consider adding a pre-migration validation step to explicitly verify there are no NULL/duplicate
name_envalues before running this DDL. This follows defensive migration best practices.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
app/src/main/resources/sql/h2/01_create_all_tables_ddl_v1.0.0.h2.sqlapp/src/main/resources/sql/h2/02_create_model_ddl_2025_0717.sqlapp/src/main/resources/sql/h2/03_create_resources_ddl_2025_0902.sqlapp/src/main/resources/sql/h2/04_create_permission_table_ddl_2025_1029.sqlapp/src/main/resources/sql/h2/05_update_all_tables_ddl.sqlapp/src/main/resources/sql/h2/06_update_tables_ddl_v1.0.0_2025_0527.sqlapp/src/main/resources/sql/h2/07_update_table_ddl_2025_1014.sqlapp/src/main/resources/sql/h2/08_update_table_ddl_2025_1217.sqlapp/src/main/resources/sql/h2/09_update_table_ddl_2025_1229.sqlapp/src/main/resources/sql/h2/init_data_for_test_v1.0.0.sqlapp/src/main/resources/sql/mysql/01_create_all_tables_ddl_v1.0.0.mysql.sqlapp/src/main/resources/sql/mysql/02_create_model_ddl_2025_0717.sqlapp/src/main/resources/sql/mysql/03_create_resources_ddl_2025_0902.sqlapp/src/main/resources/sql/mysql/04_create_permission_table_ddl_2025_1029.sqlapp/src/main/resources/sql/mysql/05_update_all_tables_ddl.sqlapp/src/main/resources/sql/mysql/06_update_tables_ddl_v1.0.0_2025_0527.sqlapp/src/main/resources/sql/mysql/07_update_table_ddl_2025_1014.sqlapp/src/main/resources/sql/mysql/08_update_table_ddl_2025_1217.sqlapp/src/main/resources/sql/mysql/09_update_table_ddl_2025_1229.sqlapp/src/main/resources/sql/mysql/init_data_for_test_v1.0.0.sqlbase/src/main/java/com/tinyengine/it/common/exception/GlobalExceptionAdvice.javabase/src/main/java/com/tinyengine/it/service/platform/impl/TenantServiceImpl.javadocker-deploy-data/mysql/init/01_create_all_tables_ddl_v1.0.0.mysql.sqldocker-deploy-data/mysql/init/02_create_model_ddl_2025_0717.sqldocker-deploy-data/mysql/init/03_create_resources_ddl_2025_0902.sqldocker-deploy-data/mysql/init/04_create_permission_table_ddl_2025_1029.sqldocker-deploy-data/mysql/init/05_update_all_tables_ddl.sqldocker-deploy-data/mysql/init/06_update_tables_ddl_v1.0.0_2025_0527.sqldocker-deploy-data/mysql/init/07_update_table_ddl_2025_1014.sqldocker-deploy-data/mysql/init/08_update_table_ddl_2025_1217.sqldocker-deploy-data/mysql/init/09_update_table_ddl_2025_1229.sqldocker-deploy-data/mysql/init/init_data_for_test_v1.0.0.sql
🔇 Additional comments (18)
docker-deploy-data/mysql/init/05_update_all_tables_ddl.sql (1)
1-30: LGTM - Formatting-only change.This file contains no semantic changes to the DDL statements. All ALTER TABLE commands remain functionally identical.
app/src/main/resources/sql/mysql/02_create_model_ddl_2025_0717.sql (1)
1-22: LGTM - Formatting adjustments only.The changes are limited to whitespace and line formatting. The table schema, columns, constraints, and indexes remain semantically unchanged.
app/src/main/resources/sql/h2/06_update_tables_ddl_v1.0.0_2025_0527.sql (1)
1-2: LGTM - No functional change.The diff shows a no-op replacement where the identical SQL statement is rewritten. No semantic impact.
app/src/main/resources/sql/mysql/06_update_tables_ddl_v1.0.0_2025_0527.sql (1)
1-2: LGTM - No functional change.This is a no-op change where the statement is replaced with an identical copy. No semantic modifications.
app/src/main/resources/sql/mysql/05_update_all_tables_ddl.sql (1)
1-31: LGTM - Formatting-only change.The only change is the addition of trailing newlines. All DDL statements remain functionally identical.
app/src/main/resources/sql/h2/02_create_model_ddl_2025_0717.sql (1)
1-22: LGTM - Formatting adjustments only.The changes are limited to whitespace formatting. The table schema, including columns, constraints, and indexes, remains semantically unchanged.
app/src/main/resources/sql/h2/05_update_all_tables_ddl.sql (1)
1-29: LGTM - No semantic changes.The changes only update line markers while preserving identical DDL content. No functional impact.
docker-deploy-data/mysql/init/06_update_tables_ddl_v1.0.0_2025_0527.sql (1)
1-2: Formatting change acknowledged.No semantic changes to the DDL statements. The index modification correctly adds a composite index on
(tenant_id, platform_id, name, app_id)fort_block_group.app/src/main/resources/sql/mysql/04_create_permission_table_ddl_2025_1029.sql (1)
1-34: Formatting restructure with no semantic changes.Table definitions for
t_permission_roleandr_auth_users_units_rolesretain the same columns, constraints, and indexes. The reformatting improves readability.app/src/main/resources/sql/h2/04_create_permission_table_ddl_2025_1029.sql (1)
1-34: Formatting restructure mirrors MySQL version.Table definitions are consistent with the MySQL counterpart. The same H2 compatibility note applies regarding MySQL-specific syntax (
engine = innodb).app/src/main/resources/sql/mysql/03_create_resources_ddl_2025_0902.sql (1)
1-58: Formatting restructure with no semantic changes.Table definitions for resource-related tables retain identical columns, constraints, and indexes. The reformatting improves consistency across the codebase.
base/src/main/java/com/tinyengine/it/service/platform/impl/TenantServiceImpl.java (1)
114-133: [rewritten review comment]
[classification tag]app/src/main/resources/sql/h2/03_create_resources_ddl_2025_0902.sql (1)
27-27: Remove or replace MySQL-specific syntax in H2 DDL files.The
engine = innodbclause is MySQL-specific and unsupported by H2. This is a systemic issue found in 36+ instances across multiple H2 DDL files (01_create_all_tables_ddl_v1.0.0.h2.sql, 02_create_model_ddl_2025_0717.sql, 03_create_resources_ddl_2025_0902.sql, 04_create_permission_table_ddl_2025_1029.sql).Additionally, other MySQL-specific syntax is present (e.g.,
USING BTREEin index definitions,auto_increment). H2 may silently ignore these clauses in compatibility mode, but this could fail in strict mode or cause issues during test execution. Either standardize all H2 DDL files to H2-compatible syntax, or verify that H2 is configured to tolerate MySQL syntax without errors.app/src/main/resources/sql/h2/08_update_table_ddl_2025_1217.sql (1)
1-2: The AFTER clause is supported by H2 database in ALTER TABLE ADD COLUMN statements. This syntax has been available since H2 v1.3.171 and is documented in the official H2 command reference. The migration file requires no changes.Likely an incorrect or invalid review comment.
docker-deploy-data/mysql/init/init_data_for_test_v1.0.0.sql (1)
4-4: Data migration aligns with schema changes - verify execution orderLine 4 swaps the
name_cnandname_envalues, changing from:
- Old:
(1, 'public', '公共租户', ...)- New:
(1, '公共租户', 'public', ...)This aligns with the schema changes in
09_update_table_ddl_2025_1229.sqlwhich:
- Makes
name_enthe unique identifier (NOT NULL + unique index)- Makes
name_cnoptional (NULL allowed)Critical: Ensure migration scripts execute in this order:
- First: Schema changes (09_update_table_ddl_2025_1229.sql) OR data updates
- The order depends on whether this is a fresh init or updating existing data
For fresh deployments (init scripts), this is correct. For existing databases, you'll need an UPDATE script instead:
-- For migrating existing data UPDATE t_tenant SET name_en_temp = name_en, name_en = name_cn, name_cn = name_en_temp;Verify: Is this init script only for new deployments, or does it need to handle existing data migration?
app/src/main/resources/sql/mysql/init_data_for_test_v1.0.0.sql (1)
4-4: Column order and semantic values in t_tenant INSERT are correct.The test data correctly has Chinese text (
'公共租户') inname_cnand English text ('public') inname_en, matching the column semantics. Column order in the INSERT statement matches the table schema definition. All three test environments (MySQL, H2, and Docker) have identical t_tenant data, ensuring consistency.Note: The DDL changes in
09_update_table_ddl_2025_1229.sqlswap which column is required (makingname_enNOT NULL andname_cnnullable) and move the unique constraint fromname_cntoname_en. Verify that any existing production data handles these schema constraint changes correctly.app/src/main/resources/sql/h2/init_data_for_test_v1.0.0.sql (1)
4-4: LGTM - Test data correctly updatedThe swap of
name_enandname_cnvalues correctly aligns with the schema changes in the DDL files wherename_enbecomes the NOT NULL unique identifier.docker-deploy-data/mysql/init/02_create_model_ddl_2025_0717.sql (1)
1-22: No migration script needed—t_model is a newly introduced tableThe structural design with
name_cnandname_enis correct for this fresh table introduction. Since t_model does not exist in the base schema (file 01), there is no existing data to migrate. File 02 creates the table with the bilingual columns from the start, and file 07 correctly extends the schema by addingapp_idandplatform_idcolumns.The
DROP TABLE IF EXISTSpattern in file 02 ensures idempotency for fresh installations. No migration of a singlenamecolumn is required because the table is new to the codebase.
2 participants
English | 简体中文
PR
1.update : app/src/main/resources/sql/h2/init_data_for_test_v1.0.0.sql
app/src/main/resources/sql/mysql/init_data_for_test_v1.0.0.sql
docker-deploy-data/mysql/init/init_data_for_test_v1.0.0.sql
t_tenant 表数据
2.add : app/src/main/resources/sql/mysql/09_update_table_ddl_2025_1229.sql ,
app/src/main/resources/sql/h2/09_update_table_ddl_2025_1229.sql
docker-deploy-data/mysql/init/09_update_table_ddl_2025_1229.sql
(update t_tenant INDEX，MODIFY COLUMN name_en，MODIFY COLUMN name_cn NULL)
3.update: rename app/src/main/resources/sql/h2,docker-deploy-data/mysql/init,app/src/main/resources/sql/mysql 目录下sql 文件名字,确保部署脚本顺序执行
4.修改TenantServiceImpl createTenant方法
5.修改GlobalExceptionAdvice 异常返回200（前端要求返回200）
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.