-
Notifications
You must be signed in to change notification settings - Fork 53
fix: Fix page and block lock bug #238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis update modifies database indexing for the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CanvasServiceImpl
participant DB (Page/BlockMapper)
User->>CanvasServiceImpl: lockCanvas(id, state, type)
CanvasServiceImpl->>DB: Update occupier_by if not empty string
DB-->>CanvasServiceImpl: Update result
CanvasServiceImpl-->>User: Return updated CanvasDto with occupier info
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/src/main/resources/sql/h2/update_tables_ddl_v1.0.0_2025_0527.sql
(1 hunks)app/src/main/resources/sql/mysql/update_tables_ddl_v1.0.0_2025_0527.sql
(1 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/CanvasServiceImpl.java
(3 hunks)base/src/main/resources/mappers/BlockMapper.xml
(1 hunks)base/src/main/resources/mappers/PageMapper.xml
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
base/src/main/java/com/tinyengine/it/service/app/impl/CanvasServiceImpl.java (1)
base/src/main/java/com/tinyengine/it/common/enums/Enums.java (1)
Enums
(20-911)
🔇 Additional comments (7)
app/src/main/resources/sql/h2/update_tables_ddl_v1.0.0_2025_0527.sql (1)
1-2
: Verify the index strategy aligns with query patterns.The composite index on
tenant_id
,platform_id
,name
,app_id
suggests optimization for multi-tenant scenarios. Ensure this column order matches your most common query patterns for optimal performance.app/src/main/resources/sql/mysql/update_tables_ddl_v1.0.0_2025_0527.sql (1)
1-2
: Good consistency with H2 schema.The MySQL schema change matches the H2 version, maintaining cross-platform compatibility.
base/src/main/java/com/tinyengine/it/service/app/impl/CanvasServiceImpl.java (5)
17-17
: Good addition for enum-based state handling.Adding the
Enums
import enables type-safe state comparisons.
48-48
: Improve occupier initialization clarity.Explicitly initializing
occupier
to null improves code readability.
56-57
: Excellent refactoring for conditional logic.The
value
andoccupierValue
variables make the conditional assignment logic much clearer and eliminate code duplication.
60-62
: Essential null safety improvement.Adding the null check prevents
NullPointerException
when accessing nested occupier properties.
67-67
: Verify mapper compatibility with null values.The service logic correctly sets
value
to null for release operations and usesoccupierValue
consistently. However, this depends on the mappers correctly handling null values to clear theoccupier_by
field.The mapper changes in
BlockMapper.xml
andPageMapper.xml
currently prevent null values from being included in UPDATE statements, which will break the unlock functionality. Ensure the mapper fixes are applied before deploying this service change.Also applies to: 70-70, 80-80, 83-83, 88-88
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
Bug Fixes
Chores