Skip to content

Conversation

@msslulu
Copy link
Contributor

@msslulu msslulu commented Jan 30, 2026

English | 简体中文

PR

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our Commit Message Guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • Built its own designer, fully self-validated

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

Background and solution

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Summary by CodeRabbit

  • Bug Fixes
    • Made table-name lowercasing locale-insensitive to ensure consistent behavior across regional settings.
    • Standardized error handling in authentication and model operations using uniform error codes and clearer diagnostics.
    • Added validation for dynamic table names to prevent invalid or empty names and improve robustness.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 30, 2026

Walkthrough

Four Java classes updated: locale-aware lowercasing added, exception handling standardized to throw ServiceException with specific codes, and input validation added for dynamic table drop SQL generation.

Changes

Cohort / File(s) Summary
Locale-Aware Lowercasing
base/src/main/java/com/tinyengine/it/dynamic/service/DynamicService.java
Replaced toLowerCase() with toLowerCase(Locale.ROOT) in getTableName to ensure locale-insensitive lowercasing.
Token Validation / SSO
base/src/main/java/com/tinyengine/it/login/config/SSOInterceptor.java
Replaced rethrowing a generic Exception with throwing ServiceException using error code CM336 in token validation error path.
Model Service Error Handling
base/src/main/java/com/tinyengine/it/service/material/impl/ModelServiceImpl.java
Replaced RuntimeException throws with ServiceException using code CM001; updated logging tags in deleteModelById and updateModelById.
Dynamic Model Service: Errors & Validation
base/src/main/java/com/tinyengine/it/dynamic/service/DynamicModelService.java
Now throws ServiceException (CM001) on create/drop failures; added input validation for generateDropTableSQL to reject null/empty or disallowed table names (throws IllegalArgumentException).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through code with ears held high,

Locale.ROOT made table names comply,
Exceptions now rounded with tidy CM signs,
Validation checks guard the burrows and lines,
A soft thump for changes—neat, small, and spry.

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'fix: code check' is vague and generic, using non-descriptive terms that fail to convey meaningful information about the actual changes made (exception handling improvements, input validation, and locale-insensitive operations). Revise the title to be more specific and descriptive, such as 'refactor: standardize exception handling and add input validation' or 'fix: improve error handling with ServiceException and locale-insensitive operations'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In `@base/src/main/java/com/tinyengine/it/login/config/SSOInterceptor.java`:
- Around line 115-119: The current catch in SSOInterceptor is catching all
Exceptions and converting ServiceException instances (CM339/CM340/CM341/CM342)
into CM336; update the error handling so ServiceException is preserved: add a
specific catch (ServiceException se) in the same method in SSOInterceptor that
calls DefaultLoginUserContext.clear(), logs the exception, and rethrows se, then
keep the existing catch (Exception e) to handle unexpected errors (log, clear
context, and throw CM336); ensure logging messages reference the thrown
exception in both catches.

In
`@base/src/main/java/com/tinyengine/it/service/material/impl/ModelServiceImpl.java`:
- Around line 192-193: The error handling in updateModelById is wrong: the log
call uses a generic message and the ServiceException uses getResultCode() twice;
update the log inside updateModelById to a descriptive message like "failed to
update model by id" and pass the exception (e) to log.error, and change the
thrown ServiceException to use ExceptionEnum.CM001.getResultCode() as the first
arg and ExceptionEnum.CM001.getResultMsg() as the second arg so the message is
returned correctly.
- Around line 155-157: Fix the error handling in
ModelServiceImpl.deleteModelById: change the ServiceException construction to
pass ExceptionEnum.CM001.getResultMsg() as the second argument (message) instead
of getResultCode(), and improve the log.error call to include a descriptive
message and the caught exception and model id (e.g., log.error("Failed to delete
model by id {}:", modelId, e)) so the log provides context and the exception is
recorded; update references to ServiceException, ExceptionEnum.CM001,
deleteModelById, and the log.error invocation accordingly.
🧹 Nitpick comments (1)
base/src/main/java/com/tinyengine/it/service/material/impl/ModelServiceImpl.java (1)

148-150: Pre-existing ServiceException bug at line 150.

This line has the same getResultCode() / getResultCode() issue but wasn't changed in this PR. Consider fixing it for consistency.

♻️ Suggested fix for consistency
         if (result != 1) {
-            throw new ServiceException(ExceptionEnum.CM001.getResultCode(), ExceptionEnum.CM001.getResultCode());
+            throw new ServiceException(ExceptionEnum.CM001.getResultCode(), ExceptionEnum.CM001.getResultMsg());
         }

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
base/src/main/java/com/tinyengine/it/dynamic/service/DynamicModelService.java (1)

62-70: ⚠️ Potential issue | 🟡 Minor

Align table-name regex with validateTableAndData for consistency.
generateDropTableSQL uses ^[a-zA-Z0-9_]+$ while validateTableAndData uses ^[a-zA-Z_][a-zA-Z0-9_]*$. The current pattern also allows all-digit identifiers, which are invalid in MySQL. Aligning to the stricter pattern ensures consistency across the codebase.

♻️ Suggested fix
- if (!tableName.matches("^[a-zA-Z0-9_]+$")) {
+ if (!tableName.matches("^[a-zA-Z_][a-zA-Z0-9_]*$")) {
🤖 Fix all issues with AI agents
In
`@base/src/main/java/com/tinyengine/it/dynamic/service/DynamicModelService.java`:
- Around line 56-58: In the catch blocks inside DynamicModelService (e.g., the
one in createDynamicTable and the other at lines ~86-88), the ServiceException
is being constructed with ExceptionEnum.CM001.getResultCode() passed twice;
change the second argument to ExceptionEnum.CM001.getResultMsg() so the
human-readable message is preserved when throwing new ServiceException; update
both occurrences where getResultCode() is duplicated to use getResultMsg() as
the message parameter.

@hexqi hexqi merged commit 0223b20 into opentiny:develop Jan 30, 2026
1 check passed
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.

2 participants