feat: implement model-driven data API with dynamic SQL operations #290
+1,247
−13
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughExpands MyBatis mapper scan and adds a dynamic-data subsystem: new REST controller, DTOs, MyBatis dynamic SQL provider and DAO, plus services to create/modify/drop runtime tables and perform validated, paginated CRUD operations. (47 words) Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Controller as ModelDataController
participant Service as DynamicService
participant DAO as ModelDataDao
participant DB as Database
Client->>Controller: POST /platform-center/api/queryApi (DynamicQuery)
activate Controller
Controller->>Service: queryWithPage(dto)
activate Service
Service->>Service: validate table & params
Service->>DAO: select(params)
activate DAO
DAO->>DB: Execute dynamic SELECT (SQL from DynamicSqlProvider)
DB-->>DAO: rows
deactivate DAO
Service->>DAO: select count(params)
activate DAO
DAO->>DB: Execute COUNT(*)
DB-->>DAO: total
deactivate DAO
Service-->>Controller: {list, total, page}
deactivate Service
Controller-->>Client: Result<Map<String,Object>>
deactivate Controller
sequenceDiagram
actor Client
participant Controller as ModelDataController
participant Service as DynamicService
participant DAO as ModelDataDao
participant DB as Database
Client->>Controller: POST /platform-center/api/insertApi (DynamicInsert)
activate Controller
Controller->>Service: insert(dto)
activate Service
Service->>Service: validate & inject created_by/updated_by
Service->>DAO: insert(params)
activate DAO
DAO->>DB: Execute dynamic INSERT (SQL from DynamicSqlProvider)
DB-->>DAO: generatedId
deactivate DAO
Service-->>Controller: {count, id}
deactivate Service
Controller-->>Client: Result<Map<String,Object>>
deactivate Controller
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 20
🤖 Fix all issues with AI agents
In
`@base/src/main/java/com/tinyengine/it/dynamic/controller/ModelDataController.java`:
- Around line 43-47: The controller currently returns raw exception messages
(e.getMessage()) to clients; instead, in ModelDataController wrap each
endpoint's try/catch (e.g., the method calling
dynamicService.queryWithPage(dto)) to log the full exception server-side (using
your logger) and return a sanitized, non-sensitive message via Result.failed
(e.g., "Internal server error" or a brief contextual message). Update the catch
blocks across all endpoint methods in ModelDataController to log the exception
(including stack trace) and replace e.getMessage() with a generic error string
while preserving any error codes or flags your Result type supports.
In `@base/src/main/java/com/tinyengine/it/dynamic/dao/DynamicSqlProvider.java`:
- Around line 69-90: The update method in DynamicSqlProvider currently
interpolates tableName and column names from the data and conditions maps
directly (variables: tableName, data, conditions in method update of class
DynamicSqlProvider), creating an SQL injection risk; fix by
validating/whitelisting the tableName and each column key before building the
SQL (e.g., check against an allow-list or strict identifier regex like
/^[A-Za-z_][A-Za-z0-9_]*$/) and throw an IllegalArgumentException for any
invalid identifier, or resolve identifiers via a known mapping of
logical->actual column names; keep using parameter placeholders (#{data.xxx},
#{conditions.xxx}) for values but do not concatenate unchecked identifiers into
sql.SET/sql.WHERE.
- Around line 92-106: The delete(Map<String, Object> params) method currently
issues DELETE_FROM(tableName) even when conditions is null/empty; add a guard
that checks the conditions Map (from params.get("conditions")) and if it is null
or empty throw an IllegalArgumentException (or another explicit runtime
exception) instead of building the SQL, so no SQL is generated without a WHERE;
update the method that builds SQL (uses SQL sql = new SQL();
sql.DELETE_FROM(tableName); sql.WHERE(...)) to only perform DELETE_FROM and add
WHERE clauses after the non-empty check (or return/throw before calling
sql.DELETE_FROM) to ensure you never produce a DELETE FROM without conditions.
- Around line 10-51: The select method in DynamicSqlProvider currently
concatenates untrusted inputs (tableName, fields, orderBy, orderType, and
condition keys) into SQL, enabling injection; update select to validate and
whitelist identifiers before using them: ensure tableName and each field in
fields match a safe identifier whitelist or strict regex (letters, digits,
underscore) or against allowed table/column lists, validate orderBy is a single
whitelisted column and orderType is only "ASC" or "DESC", and validate
pageNum/pageSize are non-negative integers; only after validation pass the
sanitized identifiers to SQL.SELECT, SQL.FROM and SQL.ORDER_BY, and keep
parameter binding (#{conditions.xxx}) for values rather than concatenation to
eliminate injection vectors in condition values and LIMIT.
In `@base/src/main/java/com/tinyengine/it/dynamic/dao/ModelDataDao.java`:
- Around line 17-19: The DAO insert method has the wrong return type: change
ModelDataDao.insert's signature from Long insert(Map<String,Object> params) to
int (or Integer) insert(Map<String,Object> params) because MyBatis returns the
number of affected rows while generated key is populated into params via
`@Options`; then update callers (e.g., DynamicService.insert) to treat the return
value as rowsAffected (rename insertRow -> rowsAffected) and continue to read
the generated id from params.get("id"); leave DynamicSqlProvider.insert and the
`@Options` annotation as-is.
In
`@base/src/main/java/com/tinyengine/it/dynamic/service/DynamicModelService.java`:
- Around line 459-470: The getEnumOptions method is wrapping enum values in
single quotes without escaping, which breaks SQL when a value contains a single
quote; update getEnumOptions so each extracted value from JSON (in method
getEnumOptions) has single quotes escaped (replace ' with '') before you wrap it
in surrounding single quotes and join them, ensuring the map/stream step uses
the escaped string to build the final comma-separated list.
- Around line 132-144: The loop in DynamicModelService is overwriting each
ParametersDto default with the hardcoded "1" (param.setDefaultValue("1")), which
must be removed; instead read the existing default with param.getDefaultValue()
and only throw IllegalArgumentException if that value is null for a required
parameter (Boolean.TRUE.equals(param.getRequired())). Update the for-loop that
iterates over parameters (ParametersDto param) to stop setting a hardcoded
default and use convertValueByType(value, fieldType, columnName) with the actual
configured default value.
- Around line 387-405: The Date type mapping is inconsistent between
mapJavaTypeToSQL and generateColumnDefinition causing spurious schema-change
detection; fix by unifying the mapping: either change mapJavaTypeToSQL to return
"DATE" for "Date" (instead of "TIMESTAMP") or refactor generateColumnDefinition
to call mapJavaTypeToSQL (and use its result) so both use the same canonical
mapping; update references in modifyTableStructure-related logic to rely on that
single mapping function (mapJavaTypeToSQL) to avoid mismatches.
- Around line 237-249: The getWhereCondition method in DynamicModelService
concatenates raw map keys into SQL (entry.getKey()), which allows SQL injection;
fix by validating/whitelisting keys before using them: create or use an
allowedColumns set (or table metadata) and only build clauses for keys present
in that set, mapping validated keys to safe column identifiers (or
pre-quoted/escaped identifiers) and throw/log on unknown keys; continue to use
named parameters for values and append to sql and whereClauses only after
validation so no raw keys are injected.
- Around line 574-607: The updateDateById method is vulnerable because
updateFields' keys are concatenated into SQL; validate each field name against
an allowlist or a safe identifier pattern before appending (e.g., only
[a-zA-Z0-9_]+) or map DTO keys to known column names, and reject or ignore
invalid keys; build the SET clause using only validated/whitelisted column names
(still using "column = ?" placeholders for values) and use
jdbcTemplate.update(sql, params.toArray()) as before; reference updateDateById,
updateFields, tableName, the sql StringBuilder, and jdbcTemplate.update when
making this change.
- Around line 171-211: The dynamicQuery method currently concatenates
user-controlled identifiers (tableName, fields, orderBy) into SQL; fix by
validating/whitelisting or safely quoting these identifiers before building the
SQL: ensure tableName matches an allowed table name pattern or a maintained
whitelist, ensure each entry in fields is an allowed column name or matches a
safe identifier regex (reject/escape anything else), and validate orderBy to
only allow "column [ASC|DESC]" patterns or map it from a safe whitelist;
continue to use parameter binding for values via
getWhereCondition/namedParameterJdbcTemplate but do not pass unvalidated
identifiers directly into SQL—apply these checks in dynamicQuery (and any helper
like getWhereCondition if it touches identifiers) before appending to the
StringBuilder.
- Around line 309-366: modifyTableStructure: when querying
INFORMATION_SCHEMA.COLUMNS (fetchColumnsSql) include the schema filter (e.g.,
AND TABLE_SCHEMA = DATABASE()) to avoid cross-schema matches; normalize type
comparison by comparing a canonical form of types — call
mapJavaTypeToSQL(param.getType()) and compare it against the existing DATA_TYPE
+ length (or use startsWith/regex on existingColumnMap values) instead of raw
equalsIgnoreCase so VARCHAR(255) vs VARCHAR matches; and avoid mutating the
caller's list by making a defensive copy of parameters before calling
addCommonFields (e.g., copy to a new List and operate on that) so the original
Model.parameters is not changed.
- Around line 502-546: createData currently builds column list from
record.keySet() and then iterates record.values() to set PreparedStatement
parameters, which can misalign columns because HashMap iteration order is not
stable; fix by producing a stable iteration order (e.g., convert record to a
LinkedHashMap or build a List<String> keys = new ArrayList<>(record.keySet()) /
List<Map.Entry<String,Object>> entries = new ArrayList<>(record.entrySet())) and
use that same keys/entries sequence both to construct the columns/placeholders
and to set ps.setObject(...) in the PreparedStatementCreator; update references
in createData where columns/placeholders and the parameter-setting loop are
built so they use the same ordered keys/entries (keeping jdbcTemplate.update,
keyHolder usage and adding generatedId to record unchanged).
- Around line 261-286: queryWithPage currently ignores pageNum so pagination has
no OFFSET; compute an offset when pageNum and pageSize are provided (e.g., int
offset = (pageNum - 1) * pageSize), set limit = pageSize, and pass the offset
into dynamicQuery instead of the current single-argument limit; update the
dynamicQuery method signature (and its callers/implementation) to accept and
apply an offset parameter to the generated SQL and leave count(...) unchanged.
- Around line 411-457: In generateColumnDefinition(ParametersDto field,String
type) ensure null-safety and escape SQL-injectable values: first guard
field.getType() for null (e.g., treat null as default type "Text" or route to
default case) before the switch to avoid NPE; when appending default value and
description escape single quotes (and any other DB-specific special chars) by
replacing ' with '' (or the DB-appropriate escape) instead of directly embedding
field.getDefaultValue() or field.getDescription(); keep the existing structure
in generateColumnDefinition but perform the null check (for field and
field.getType()) and string escaping for getDefaultValue() and getDescription()
before appending to sb.
In `@base/src/main/java/com/tinyengine/it/dynamic/service/DynamicService.java`:
- Around line 185-190: The null-check risk comes from
modelService.getAllModelName() returning null and causing validateTableExists
(method validateTableExists) to NPE when calling tables.contains(...); fix the
data source method instead by updating ModelServiceImpl.getAllModelName to never
return null—return Collections.emptyList() or a new ArrayList<>() when no models
are found (or when internal value is null)—so validateTableExists can safely
call modelService.getAllModelName().contains(tableName).
- Line 9: The class DynamicService currently imports
jakarta.transaction.Transactional; replace that import with Spring's
org.springframework.transaction.annotation.Transactional so Spring's transaction
management is used; update any annotations or fully-qualified references in
DynamicService to use org.springframework.transaction.annotation.Transactional
(and remove the jakarta import) to ensure proper Spring-managed transactions.
- Around line 51-59: The count method in DynamicService calls
dynamicDao.select(params) and then blindly accesses result.get(0), which can
throw NPE or IndexOutOfBounds if result is null or empty; update count(String
tableName, Map<String,Object> conditions) to check that the List<JSONObject>
returned by dynamicDao.select(params) is non-null and non-empty before accessing
index 0, returning 0 (or a safe default) when no rows are found, and defensively
parsing the "count" field (handle null or non-numeric values) to avoid
NumberFormatException.
In
`@base/src/main/java/com/tinyengine/it/service/material/impl/ModelServiceImpl.java`:
- Around line 128-130: The exception construction in ModelServiceImpl uses
ExceptionEnum.CM001.getResultCode() for both code and message; change the second
argument to ExceptionEnum.CM001.getResultMsg() so the thrown ServiceException
carries the proper human-readable message (locate the throw in ModelServiceImpl
where the if (result != 1) block constructs the ServiceException).
- Around line 253-261: The getAllModelName method in ModelServiceImpl should
return an empty list instead of null; modify getAllModelName so that when
modelList is empty or null it returns Collections.emptyList() (or new
ArrayList<>()) and otherwise returns the mapped list
(modelList.stream().map(Model::getNameEn)...); ensure the method never returns
null so callers like DynamicService.validateTableExists can safely call
contains() without NPE.
🧹 Nitpick comments (18)
base/src/main/java/com/tinyengine/it/dynamic/dto/DynamicUpdate.java (1)
1-11: Consider naming consistency with sibling DTOs.The field naming differs from
DynamicInsert:
DynamicInsertusesparamsfor insert dataDynamicUpdateusesdatafor update values andparamsfor WHERE conditionsThis inconsistency could cause confusion. Consider aligning the naming convention, e.g., rename
datatoupdateDataorvaluesto make the distinction clearer, or updateDynamicInsert.paramsto match.Additionally, consider adding JSR-303 validation annotations (e.g.,
@NotBlankonnameEn) for required fields to ensure input validation at the controller layer.base/src/main/java/com/tinyengine/it/dynamic/dto/DynamicDelete.java (1)
5-5: Remove unused import.
java.util.Mapis imported but not used in this class.Proposed fix
package com.tinyengine.it.dynamic.dto; import lombok.Data; -import java.util.Map; - `@Data` public class DynamicDelete {base/src/main/java/com/tinyengine/it/dynamic/dto/DynamicInsert.java (1)
9-9: Clarify the comment.The comment says "插入/更新数据" (insert/update data), but this DTO is specifically for insert operations.
DynamicUpdatehandles updates separately. Consider updating the comment to avoid confusion:Proposed fix
- private Map<String, Object> params; // 插入/更新数据 + private Map<String, Object> params; // 插入数据base/src/main/java/com/tinyengine/it/service/material/impl/ModelServiceImpl.java (3)
155-160: Inconsistent exception handling pattern.This block catches an exception and rethrows a
RuntimeException, but elsewhere in this class (e.g., lines 119, 129, 153) aServiceExceptionis thrown. Additionally,dropDynamicTablealready throwsRuntimeExceptionon failure per the summary, so catching and re-wrapping it is redundant. Consider usingServiceExceptionfor consistency or letting the original exception propagate if the transactional rollback is the desired behavior.Proposed fix
- try { - dynamicModelService.dropDynamicTable(model); - } catch (Exception e) { - log.error("刪除动态表失败", e); - throw new RuntimeException("刪除动态表失败: " + e.getMessage()); - } + dynamicModelService.dropDynamicTable(model);
131-135: Remove or address commented-out code.Line 134 contains commented-out code for
initializeDynamicTable. If this functionality is not yet needed, remove it. If it's planned for future implementation, track it via a TODO comment with a ticket reference or open an issue.
191-196: Same inconsistent exception handling as deleteModelById.Apply consistent exception handling here as suggested for the delete operation.
base/src/main/java/com/tinyengine/it/dynamic/dto/DynamicQuery.java (1)
8-19: Add input validation annotations.This DTO lacks validation constraints that are important for security and correctness:
nameEnshould be@NotBlanksince it's requiredorderTypeshould be validated to only allow "ASC" or "DESC" to prevent SQL injection via the order clausepageSizeshould have a@Maxconstraint to prevent DoS via excessive page sizesProposed enhancement
package com.tinyengine.it.dynamic.dto; import lombok.Data; +import jakarta.validation.constraints.Max; +import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Pattern; import java.util.List; import java.util.Map; `@Data` public class DynamicQuery { + `@NotBlank`(message = "nameEn is required") private String nameEn; // 表名 private String nameCh; // 表中文名 private List<String> fields; // 查询字段 private Map<String, Object> params; // 查询条件 private Integer currentPage = 1; // 页码 + `@Max`(value = 1000, message = "pageSize cannot exceed 1000") private Integer pageSize = 10; // 每页大小 private String orderBy; // 排序字段 + `@Pattern`(regexp = "^(ASC|DESC)$", message = "orderType must be ASC or DESC") private String orderType = "ASC"; // 排序方式 }base/src/main/java/com/tinyengine/it/dynamic/dao/ModelDataDao.java (1)
10-12: Redundant annotation.
@Repositoryis unnecessary when@Mapperis present. MyBatis-Spring automatically registers mapper interfaces as beans.Proposed fix
-@Repository `@Mapper` public interface ModelDataDao {base/src/main/java/com/tinyengine/it/dynamic/controller/ModelDataController.java (2)
40-48: Add@Validannotation to enable DTO validation.The controller is annotated with
@Validated, but without@Validon the@RequestBodyparameter, Bean Validation constraints onDynamicQuerywon't be enforced.Proposed fix
`@PostMapping`("/queryApi") - public Result<Map<String, Object>> query(`@RequestBody` DynamicQuery dto) { + public Result<Map<String, Object>> query(`@RequestBody` `@Valid` DynamicQuery dto) {Apply the same change to
insert,update, anddeletemethods.
107-108: Consider using HTTP DELETE method for delete operation.Using
@PostMappingfor delete deviates from RESTful conventions. Consider@DeleteMappingfor semantic clarity and proper HTTP method usage.base/src/main/java/com/tinyengine/it/dynamic/service/DynamicService.java (3)
4-4: Remove unused import.
com.google.gson.JsonObjectis imported but never used in this file.
88-94: System fields added after params map is built.Lines 93-94 add
created_byandupdated_bytodto.getParams(), butparams.put("data", dto.getParams())was already called on line 90. Sincedto.getParams()is a reference, this works, but the order is confusing and mutates the input DTO which is a side effect. Consider building the data map explicitly.Proposed fix
public Map<String, Object> insert(DynamicInsert dto) { validateTableExists(dto.getNameEn()); validateTableAndData(dto.getNameEn(), dto.getParams()); String tableName = getTableName(dto.getNameEn()); + + // Build data map with system fields + Map<String, Object> data = new HashMap<>(dto.getParams()); + String userId = loginUserContext.getLoginUserId(); + data.put("created_by", userId); + data.put("updated_by", userId); + Map<String, Object> params = new HashMap<>(); params.put("tableName", tableName); - params.put("data", dto.getParams()); - String userId = loginUserContext.getLoginUserId(); - // 添加系统字段 - dto.getParams().put("created_by", userId); - dto.getParams().put("updated_by", userId); + params.put("data", data); // ... }
161-180: Validation validates nameEn but actual table uses "dynamic_" prefix.
validateTableAndDatavalidates thattableNamematches^[a-zA-Z_][a-zA-Z0-9_]*$, but it's called withdto.getNameEn(), not the actual table name (dynamic_+ nameEn). This is fine for validation purposes, but the method name and parameter name are misleading. Consider renaming for clarity.base/src/main/java/com/tinyengine/it/dynamic/service/DynamicModelService.java (5)
561-572: ValidatemodelIdbefore building SQL.While
modelIdis used internally, it should be validated to prevent SQL injection if this method ever receives untrusted input.🔧 Proposed fix
public Map<String, Object> getDataById(String modelId, Long id) { + if (modelId == null || !modelId.matches("^[a-zA-Z_][a-zA-Z0-9_]*$")) { + throw new IllegalArgumentException("Invalid model ID: " + modelId); + } String tableName = getTableName(modelId);
609-625: Consider soft delete instead of hard delete.The table schema includes a
deleted_atcolumn (line 110), suggesting soft delete was intended. However,deleteDataByIdperforms a hardDELETE. Consider using soft delete for data recovery capability.♻️ Soft delete implementation
public Map<String,Object> deleteDataById(DynamicDelete dto) { // ... validation ... String tableName = getTableName(modelId); - String sql = "DELETE FROM " + tableName + " WHERE id = ?"; + String sql = "UPDATE " + tableName + " SET deleted_at = CURRENT_TIMESTAMP WHERE id = ? AND deleted_at IS NULL"; int update = jdbcTemplate.update(sql, id);Also update query methods to exclude soft-deleted records by adding
WHERE deleted_at IS NULL.
476-496: Validation method exists but is not consistently used.This validation method is well-implemented but isn't called consistently before SQL operations (e.g.,
dynamicQuery,updateDateById, etc.). Consider calling it at the entry points of all public methods that construct dynamic SQL.
254-257: Add defensive null check for count result.While
COUNT(*)typically returns a single row, consider adding a defensive check to avoid potentialIndexOutOfBoundsException.♻️ Defensive fix
public Long count(String tableName, Map<String, Object> conditions) { List<Map<String, Object>> result = dynamicCount(tableName, conditions); - return Long.parseLong(result.get(0).get("count").toString()); + if (result == null || result.isEmpty()) { + return 0L; + } + return Long.parseLong(result.get(0).get("count").toString()); }
287-308: Limited number type handling and fragile date parsing.
"Number"(line 293) always parses asInteger, but values could beLong,BigDecimal, etc.Date.valueOf(line 297) assumes strict ISO format (yyyy-MM-dd) without explicit validation or error messaging.♻️ Improved number handling
case "Number": - return value != null ? Integer.parseInt(value.toString()) : null; + if (value == null) return null; + String numStr = value.toString(); + if (numStr.contains(".")) { + return new BigDecimal(numStr); + } + return Long.parseLong(numStr);
base/src/main/java/com/tinyengine/it/dynamic/controller/ModelDataController.java
Outdated
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/dynamic/dao/DynamicSqlProvider.java
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/dynamic/dao/DynamicSqlProvider.java
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/dynamic/dao/DynamicSqlProvider.java
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/dynamic/service/DynamicService.java
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/dynamic/service/DynamicService.java
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/dynamic/service/DynamicService.java
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/service/material/impl/ModelServiceImpl.java
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/service/material/impl/ModelServiceImpl.java
Show resolved
Hide resolved
msslulu
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@base/src/main/java/com/tinyengine/it/dynamic/controller/ModelDataController.java`:
- Line 41: Add the jakarta.validation.Valid import and annotate each
`@RequestBody` DTO parameter in ModelDataController with `@Valid` (specifically on
the query, insert, update, and delete methods) so Spring's method-level
validation is triggered for the DynamicQuery and other request DTOs; update the
import list to include jakarta.validation.Valid and place `@Valid` immediately
before each `@RequestBody` parameter declaration in the query(), insert(),
update(), and delete() methods.
- Around line 21-27: The code currently allows unvalidated orderBy/orderType and
lacks table-level authorization and DTO validations; fix by (1) updating
DynamicSqlProvider.select(...) to validate the orderBy value against the regex
^[a-zA-Z_][a-zA-Z0-9_]*$ and enforce orderType only equals "ASC" or "DESC"
before concatenating into the ORDER BY clause (reject or default to safe values
if validation fails), (2) adding table-level authorization in the
validateTableExists(...) method (or inside DynamicService methods called by
ModelDataController) to check current user's roles/permissions against the
requested table/model and throw a 403 if unauthorized, using your existing auth
principal or security context, and (3) adding Jakarta Bean Validation
annotations (`@NotBlank`, `@Pattern`(regex="^[a-zA-Z_][a-zA-Z0-9_]*$") for names,
and `@Min/`@Max or `@Positive/`@PositiveOrZero for pagination limits) to the DTO
classes DynamicQuery, DynamicInsert, DynamicUpdate, and DynamicDelete and ensure
controllers use `@Valid` on method parameters so invalid inputs are rejected at
the API boundary.
🧹 Nitpick comments (2)
base/src/main/java/com/tinyengine/it/dynamic/controller/ModelDataController.java (2)
26-27: Prefer constructor injection over field injection.Constructor injection improves testability and makes dependencies explicit. It's the recommended approach in modern Spring applications.
♻️ Suggested refactor
- `@Autowired` - private DynamicService dynamicService; + private final DynamicService dynamicService; + + public ModelDataController(DynamicService dynamicService) { + this.dynamicService = dynamicService; + }
86-86: Consider using idiomatic HTTP methods for update and delete.REST conventions suggest using
PUT/PATCHfor updates andDELETEfor deletions. If the DTOs contain complex query criteria that necessitate a request body, POST is acceptable, but consider documenting this design decision.Also applies to: 108-108
base/src/main/java/com/tinyengine/it/dynamic/controller/ModelDataController.java
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/dynamic/controller/ModelDataController.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@base/src/main/java/com/tinyengine/it/dynamic/service/DynamicService.java`:
- Around line 35-47: The query method calls getTableName(dto.getNameEn())
without validating dto or nameEn, risking NPE in getTableName (which calls
toLowerCase()); update DynamicService.query to validate dto and dto.getNameEn()
(e.g., check dto != null and nameEn not null/blank) before calling getTableName,
and handle invalid input consistently with other methods (throw an
IllegalArgumentException with a clear message or return an empty list as your
codebase convention dictates) so getTableName and the rest of the method never
receive a null name; reference: method query, getTableName, dto.getNameEn().
- Around line 112-116: The code unsafely casts params.get("id") to BigInteger
and directly calls longValue(), which can cause ClassCastException or NPE;
update the insertion return handling in DynamicService (around
insertRow/dynamicDao.insert and id extraction) to retrieve Object idObj =
params.get("id"), check for null, then if (idObj instanceof Number) use
((Number) idObj).longValue() to put into result, otherwise handle non-number
types (e.g., convert String with Long.parseLong or log/throw a clear error) so
you avoid direct BigInteger casts and null dereference.
- Around line 130-139: The update flow is missing a null-check for dto.getData()
and doesn't set the `updated_by` audit field; before calling
validateTableAndData(dto.getNameEn(), dto.getData()) ensure dto.getData() is not
null (throw an IllegalArgumentException with a clear message like "更新操作必须指定数据"
if it is) and then populate dto.getData().put("updated_by", currentUserId)
(matching the insert behavior) before building the params map; references:
validateTableAndData, dto.getData(), dto.getParams(), getTableName, and the
params map in DynamicService.
- Around line 106-109: The code mutates the caller's DTO params and doesn't
handle a null login user id; instead, create a defensive copy of the params map
(e.g., new HashMap<>(dto.getParams() or Collections.emptyMap())) and set that
copy back on the dto (use dto.setParams(...) or replace usage with a local map)
before adding system fields, and guard use of loginUserContext.getLoginUserId()
with a null check — only put "created_by" and "updated_by" into the new params
map when userId is non-null (or use a safe fallback value) to avoid side effects
and NPEs; update code around DynamicService -> loginUserContext.getLoginUserId()
and dto.getParams() accordingly.
- Around line 80-82: The code in DynamicService.java risks
division-by-zero/unboxing on dto.getPageSize(); change the pages computation to
first read Integer pageSize = dto.getPageSize() and handle null/zero: if
pageSize == null || pageSize == 0 then set result.put("pages", 0) (or a sensible
default), otherwise compute pages using Math.ceil((double) total / pageSize) and
put that value; update any related usages of result.put("pageSize", ...) to
safely handle null if needed.
🧹 Nitpick comments (5)
base/src/main/java/com/tinyengine/it/dynamic/service/DynamicService.java (5)
16-21: Consider constructor injection for better testability.Field injection with
@Autowiredworks but makes unit testing harder. Constructor injection is the recommended approach in Spring.♻️ Suggested refactor
- `@Autowired` - private ModelDataDao dynamicDao; - `@Autowired` - private ModelService modelService; - `@Autowired` - private LoginUserContext loginUserContext; + private final ModelDataDao dynamicDao; + private final ModelService modelService; + private final LoginUserContext loginUserContext; + + public DynamicService(ModelDataDao dynamicDao, ModelService modelService, LoginUserContext loginUserContext) { + this.dynamicDao = dynamicDao; + this.modelService = modelService; + this.loginUserContext = loginUserContext; + }
24-28: Remove unused constants.These operation type constants are defined but never used anywhere in the class.
♻️ Proposed fix
- // 操作类型常量 - private static final String OPERATION_SELECT = "SELECT"; - private static final String OPERATION_INSERT = "INSERT"; - private static final String OPERATION_UPDATE = "UPDATE"; - private static final String OPERATION_DELETE = "DELETE"; -
160-164: Use consistent Map type for conditions.
Map<Object, Object>at line 161 should beMap<String, Object>for consistency with other methods.♻️ Proposed fix
Map<String, Object> params = new HashMap<>(); - Map<Object, Object> conditions = new HashMap<>(); + Map<String, Object> conditions = new HashMap<>(); conditions.put("id", dto.getId());
176-179: Missing SQL injection validation on tableName.
validateTableExists()only checks if the model exists in the allowed list, but unlikevalidateTableAndData(), it doesn't validate the table name format. IfmodelService.getAllModelName()were compromised or contained invalid names, this could be a risk.♻️ Proposed fix
public List<Map<String, Object>> getTableStructure(String tableName) { + if (tableName == null || !tableName.matches("^[a-zA-Z_][a-zA-Z0-9_]*$")) { + throw new IllegalArgumentException("表名格式不正确"); + } validateTableExists(tableName); return dynamicDao.getTableStructure(tableName); }
214-216: Potential NPE if modelId is null.
modelId.toLowerCase()will throw NPE if modelId is null. While callers should validate, adding a defensive check here prevents cascading failures.♻️ Proposed fix
private String getTableName(String modelId) { + if (modelId == null) { + throw new IllegalArgumentException("modelId 不能为空"); + } return "dynamic_" + modelId.toLowerCase(); }
base/src/main/java/com/tinyengine/it/dynamic/service/DynamicService.java
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/dynamic/service/DynamicService.java
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/dynamic/service/DynamicService.java
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/dynamic/service/DynamicService.java
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/dynamic/service/DynamicService.java
Show resolved
Hide resolved
hexqi
reviewed
Jan 29, 2026
base/src/main/java/com/tinyengine/it/dynamic/controller/ModelDataController.java
Outdated
Show resolved
Hide resolved
hexqi
reviewed
Jan 29, 2026
base/src/main/java/com/tinyengine/it/dynamic/dao/DynamicSqlProvider.java
Show resolved
Hide resolved
hexqi
reviewed
Jan 29, 2026
base/src/main/java/com/tinyengine/it/dynamic/controller/ModelDataController.java
Outdated
Show resolved
Hide resolved
hexqi
approved these changes
Jan 30, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
English | 简体中文
PR
模型驱动
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
New Features
Enhancements
Data
✏️ Tip: You can customize this high-level summary in your review settings.