Skip to content

fix: update lockCanvas,parameter validation#299

Open
msslulu wants to merge 2 commits intoopentiny:developfrom
msslulu:fix/lockcanvas
Open

fix: update lockCanvas,parameter validation#299
msslulu wants to merge 2 commits intoopentiny:developfrom
msslulu:fix/lockcanvas

Conversation

@msslulu
Copy link
Contributor

@msslulu msslulu commented Mar 10, 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
    • Enhanced input validation for canvas operations to detect and reject invalid or blank parameters early.
    • Operations now return a clear "failed" status when validation fails or when referenced resources (pages or blocks) are missing.
    • Improved error messages to explicitly indicate when requested resources cannot be found, preventing unintended processing.

@coderabbitai
Copy link

coderabbitai bot commented Mar 10, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 13d1a126-6565-4863-9e4c-66a7c2298aca

📥 Commits

Reviewing files that changed from the base of the PR and between 5767ff4 and e22da57.

📒 Files selected for processing (1)
  • base/src/main/java/com/tinyengine/it/service/app/impl/CanvasServiceImpl.java

Walkthrough

The lockCanvas method in CanvasServiceImpl now validates inputs (id, state, type), returns failures for missing/blank values, checks existence of referenced resources (page or block depending on type), and sets operate to "failed" on pre-check failures. A public static isNotBlank(String) helper was added.

Changes

Cohort / File(s) Summary
Canvas Service Validation
base/src/main/java/com/tinyengine/it/service/app/impl/CanvasServiceImpl.java
Added input validation in lockCanvas (null/blank checks for id, state, type) using new public static boolean isNotBlank(String) helper; added existence checks for page when type == "page" and for block otherwise; returns failure messages and a canvas DTO with operate = "failed" on pre-check failures; successful path behavior unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I twitch my whiskers, checking doors and tracks,
I hop through code, avoiding null-filled cracks.
Pages and blocks I sniff with care,
Locking the canvas with a tidy flair.
Hooray — validations dance in the air! 🚀

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding parameter validation to the lockCanvas method.

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

✨ Finishing Touches
🧪 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: 1

🧹 Nitpick comments (1)
base/src/main/java/com/tinyengine/it/service/app/impl/CanvasServiceImpl.java (1)

103-105: Consider making this method private or using existing utilities.

The helper is only used within this class. Making it public static exposes unnecessary API surface. Additionally, consider using existing library utilities like StringUtils.isBlank() from Apache Commons Lang or Spring's StringUtils.hasText().

♻️ Option 1: Make private
-public static boolean isNull(String str) {
+private static boolean isBlank(String str) {
     return str == null || str.trim().isEmpty();
 }
♻️ Option 2: Use Spring's StringUtils
+import org.springframework.util.StringUtils;
 ...
-if(isNull(state) || isNull(type)){
+if (!StringUtils.hasText(state) || !StringUtils.hasText(type)) {
     return Result.failed("invalid parameter");
 }

Then remove the custom isNull method entirely.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@base/src/main/java/com/tinyengine/it/service/app/impl/CanvasServiceImpl.java`
around lines 103 - 105, The isNull method currently declared public static
(isNull) is only used inside CanvasServiceImpl; change its visibility to private
static or remove it and replace its usages with a standard utility such as
org.springframework.util.StringUtils.hasText(str) or
org.apache.commons.lang3.StringUtils.isBlank(str) (negating appropriately) so
you don't expose unnecessary API surface—locate usages of isNull in
CanvasServiceImpl, replace calls with the chosen Spring/Apache utility, and
delete the now-unused isNull method.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@base/src/main/java/com/tinyengine/it/service/app/impl/CanvasServiceImpl.java`:
- Around line 103-105: The isNull method currently returns true for non-empty
strings; invert its logic so it returns true when the input is actually null or
blank: update CanvasServiceImpl.isNull(String str) to return true if str == null
or str.trim().isEmpty(), and false otherwise; keep the method name (isNull) so
existing callers such as the check in CanvasServiceImpl where it does
if(isNull(state) || isNull(type)) behave correctly.

---

Nitpick comments:
In
`@base/src/main/java/com/tinyengine/it/service/app/impl/CanvasServiceImpl.java`:
- Around line 103-105: The isNull method currently declared public static
(isNull) is only used inside CanvasServiceImpl; change its visibility to private
static or remove it and replace its usages with a standard utility such as
org.springframework.util.StringUtils.hasText(str) or
org.apache.commons.lang3.StringUtils.isBlank(str) (negating appropriately) so
you don't expose unnecessary API surface—locate usages of isNull in
CanvasServiceImpl, replace calls with the chosen Spring/Apache utility, and
delete the now-unused isNull method.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e89fa536-d416-4004-adb7-78aa0f8647a5

📥 Commits

Reviewing files that changed from the base of the PR and between 2b97f64 and 5767ff4.

📒 Files selected for processing (1)
  • base/src/main/java/com/tinyengine/it/service/app/impl/CanvasServiceImpl.java

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.

1 participant