Skip to content

Fix/mbm #217

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

Closed
wants to merge 113 commits into from
Closed

Fix/mbm #217

wants to merge 113 commits into from

Conversation

lu-yg
Copy link
Collaborator

@lu-yg lu-yg commented May 14, 2025

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

  • New Features

    • Introduced a post-save extension point for pages, allowing custom actions after a page is created or updated.
  • Refactor

    • Updated database schema to allow the tenant ID field to be optional across multiple tables.
  • Chores

    • Adjusted automated field population to no longer set tenant or renter IDs during data insertion.
    • Updated related tests to reflect changes in field handling.

lu-yg and others added 30 commits December 26, 2024 00:15
Copy link

coderabbitai bot commented May 14, 2025

Walkthrough

The changes update SQL schema and migration scripts to make the tenant_id column nullable across multiple tables for both H2 and MySQL databases. Java code is adjusted to remove automatic population of tenantId, renterId, and siteId fields. Additionally, a new PostSaveHook interface is introduced, and PageServiceImpl is extended to optionally invoke this hook after saving a Page.

Changes

File(s) Change Summary
app/src/main/resources/sql/h2/create_all_tables_ddl_v1.h2.sql
app/src/main/resources/sql/mysql/create_all_tables_ddl_v1.mysql.sql
Removed the NOT NULL constraint from the tenant_id column in all relevant tables.
app/src/main/resources/sql/h2/update_all_tables_ddl.sql
app/src/main/resources/sql/mysql/update_all_tables_ddl.sql
Modified migration scripts to alter tenant_id columns to allow NULL values in multiple tables.
base/src/main/java/com/tinyengine/it/common/handler/MyMetaObjectHandler.java Stopped setting tenantId, renterId, and siteId during insert fill operations.
base/src/main/java/com/tinyengine/it/service/app/impl/PageServiceImpl.java Added optional dependency on PostSaveHook and invoked it after saving or updating a Page.
base/src/main/java/com/tinyengine/it/service/extend/PostSaveHook.java Introduced new PostSaveHook interface with method afterSave(Page page).
base/src/test/java/com/tinyengine/it/common/handler/MyMetaObjectHandlerTest.java Updated test to expect 5 instead of 8 calls to hasSetter in testInsertFill.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant PageServiceImpl
    participant PostSaveHook (optional)
    Client->>PageServiceImpl: create/update Page
    PageServiceImpl->>PageServiceImpl: Save Page and PageHistory
    alt PostSaveHook present
        PageServiceImpl->>PostSaveHook (optional): afterSave(Page)
    end
    PageServiceImpl-->>Client: Return result
Loading

Poem

In fields where tenants once must dwell,
Now empty nests are just as well.
A hook appears, post-save delight—
Page saved, then magic out of sight.
Fewer fields to fill, less test to mock,
The schema’s lighter, rabbits hop! 🐇

Tip

⚡️ Faster reviews with caching
  • CodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure Review - Disable Cache at either the organization or repository level. If you prefer to disable all data retention across your organization, simply turn off the Data Retention setting under your Organization Settings.

Enjoy the performance boost—your workflow just got faster.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c6b8c14 and 0e44141.

📒 Files selected for processing (8)
  • app/src/main/resources/sql/h2/create_all_tables_ddl_v1.h2.sql (20 hunks)
  • app/src/main/resources/sql/h2/update_all_tables_ddl.sql (1 hunks)
  • app/src/main/resources/sql/mysql/create_all_tables_ddl_v1.mysql.sql (20 hunks)
  • app/src/main/resources/sql/mysql/update_all_tables_ddl.sql (1 hunks)
  • base/src/main/java/com/tinyengine/it/common/handler/MyMetaObjectHandler.java (0 hunks)
  • base/src/main/java/com/tinyengine/it/service/app/impl/PageServiceImpl.java (4 hunks)
  • base/src/main/java/com/tinyengine/it/service/extend/PostSaveHook.java (1 hunks)
  • base/src/test/java/com/tinyengine/it/common/handler/MyMetaObjectHandlerTest.java (1 hunks)
💤 Files with no reviewable changes (1)
  • base/src/main/java/com/tinyengine/it/common/handler/MyMetaObjectHandler.java
🔇 Additional comments (11)
base/src/main/java/com/tinyengine/it/service/extend/PostSaveHook.java (1)

1-29: Clean and well-structured extension interface

The PostSaveHook interface serves as a clear extension point for post-save operations on Page entities. The design follows good practices by:

  • Defining a focused, single-method interface
  • Using clear naming convention that indicates its purpose
  • Properly documenting the method and parameters

This approach enables flexible extension of the page saving process without modifying core service logic.

base/src/main/java/com/tinyengine/it/service/app/impl/PageServiceImpl.java (4)

47-47: Appropriate import for the new hook interface

Import statement for the new PostSaveHook interface is correctly placed with the other service imports.


140-144: Well-implemented optional dependency injection

Using @Autowired(required = false) is the correct approach for this optional extension point, making the PostSaveHook an optional component that won't break the application if not present.


284-286: Properly implemented hook invocation with null check

The hook invocation is correctly placed after successful page creation and history record creation, with an appropriate null check to prevent NullPointerExceptions.


378-380: Consistent hook implementation in updatePage method

The hook invocation follows the same pattern as in the createPage method, maintaining consistency across the codebase.

base/src/test/java/com/tinyengine/it/common/handler/MyMetaObjectHandlerTest.java (1)

39-39: Test updated to reflect reduced field-setting behavior

The verification has been updated from 8 to 5 calls to reflect the removal of automatic tenant-related field population in the insertFill method. This aligns with the database schema changes making tenant_id nullable.

app/src/main/resources/sql/mysql/update_all_tables_ddl.sql (1)

10-29:

✅ Verification successful

Consistent schema changes to make tenant_id nullable across all tables

These ALTER statements systematically modify the tenant_id column to allow NULL values across 20 tables, which is consistent with the code changes in MyMetaObjectHandler that no longer automatically populates tenant-related fields.

This change appears to be part of a deliberate architectural shift to support scenarios where entities are not necessarily tied to a tenant.


🏁 Script executed:

#!/bin/bash
# Verify that MyMetaObjectHandler class no longer sets tenant-related fields

# Check the MyMetaObjectHandler class implementation
rg -A 10 -B 5 "insertFill" base/src/main/java/com/tinyengine/it/common/handler/MyMetaObjectHandler.java

Length of output: 897


Schema update: tenant_id now nullable across all tables

  • Verified that MyMetaObjectHandler.insertFill no longer sets tenantId (it only sets createdTime, lastUpdatedTime, createdBy, lastUpdatedBy, and platformId).
  • The ALTER statements in app/src/main/resources/sql/mysql/update_all_tables_ddl.sql (lines 10–29) consistently modify the tenant_id column to allow NULL, matching the handler change.
  • This supports the intended architectural shift to decouple entities from mandatory tenant assignment.
app/src/main/resources/sql/h2/update_all_tables_ddl.sql (2)

8-8: Allow NULL for publish_url in H2 migration
The change relaxes the publish_url column to accept NULL. Please verify that any application logic, validations, or reporting layers that consume this column handle NULL values gracefully (e.g., avoid unexpected NPEs or malformed URLs).


10-29: Make tenant_id columns nullable across H2 tables
This bulk modification drops NOT NULL on tenant_id for all listed tables. Note that H2’s handling of multi-column unique indexes with nullable columns may allow multiple rows with the same non-tenant keys when tenant_id IS NULL. Confirm that:

  1. Your unique constraints still enforce the intended data integrity.
  2. Downstream multi-tenancy filters (e.g., WHERE tenant_id = ?) behave correctly when tenant_id is NULL.
  3. No existing metadata or code paths assume a non-null tenant_id.
app/src/main/resources/sql/h2/create_all_tables_ddl_v1.h2.sql (1)

25-669: Drop NOT NULL constraints on tenant-related columns (and related fields) in initial H2 DDL
This patch removes the NOT NULL attribute from tenant_id (and a few supporting columns like vscode_url) in every CREATE TABLE statement. Ensure that:

  1. ORM mappings or manual inserts can supply NULL without errors.
  2. The unique indexes involving tenant_id still enforce uniqueness as expected under H2’s NULL semantics.
  3. The update migrations you applied earlier remain in sync with this initial schema.
app/src/main/resources/sql/mysql/create_all_tables_ddl_v1.mysql.sql (1)

25-678: Remove NOT NULL on tenant-related columns in MySQL initial DDL
Mirroring the H2 changes, this update makes tenant_id (and related columns) nullable in all MySQL CREATE TABLE statements. Please verify:

  • Application code and JDBC/ORM layers handle nullable tenant IDs correctly.
  • MySQL’s treatment of NULL in multi-column unique indexes aligns with your data-integrity requirements.
  • Consistency between your initial schema and the MySQL migration scripts (update_all_tables_ddl.sql).
✨ Finishing Touches
  • 📝 Generate Docstrings

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@lu-yg lu-yg closed this May 14, 2025
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