feat: [#540] Remove SQL in show_command.go #825
Conversation
WalkthroughThis pull request introduces significant enhancements to the database configuration and ORM (Object-Relational Mapping) system in the Goravel framework. The changes focus on expanding the database configuration capabilities by adding more detailed connection parameters, removing hardcoded driver constants, and introducing new methods to retrieve database configuration and version information. The modifications span multiple files across the database and contracts packages, improving the flexibility and information accessibility of database connections. Changes
Sequence DiagramsequenceDiagram
participant Client
participant ORM
participant Database
Client->>ORM: Request Database Configuration
ORM->>Database: Retrieve Configuration
Database-->>ORM: Return Config
ORM-->>Client: Provide Database Config
Client->>ORM: Request Database Version
ORM->>Database: Get Version
Database-->>ORM: Return Version
ORM-->>Client: Provide Database Version
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #825 +/- ##
=======================================
Coverage 67.06% 67.06%
=======================================
Files 147 147
Lines 10396 10396
=======================================
Hits 6972 6972
Misses 3051 3051
Partials 373 373 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
contracts/database/config.go (1)
5-13: LGTM! Consider adding field documentation.The expanded Config struct with granular database connection fields is well-structured. Consider adding documentation comments for each field to describe their purpose and expected values.
type Config struct { Connection string - Database string - Driver string - Host string - Password string - Port int - Prefix string - Schema string - Username string - Version string + // Database name to connect to + Database string + // Database driver name (e.g., mysql, postgres) + Driver string + // Database server hostname + Host string + // Database user password + Password string + // Database server port number + Port int + // Table name prefix (if any) + Prefix string + // Database schema name + Schema string + // Database user name + Username string + // Database version string + Version string }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
contracts/database/config.go(1 hunks)contracts/database/orm/orm.go(2 hunks)database/console/show_command.go(4 hunks)database/console/show_command_test.go(5 hunks)database/gorm/query.go(2 hunks)database/orm/orm.go(8 hunks)mocks/database/orm/Orm.go(3 hunks)tests/query.go(1 hunks)
🔇 Additional comments (14)
tests/query.go (1)
77-77: LGTM! Properly handles the new return value.The change correctly adapts to the updated BuildQuery signature by using the blank identifier for the unused dbConfig return value.
database/console/show_command.go (4)
25-28: LGTM! Improved type safety with int fields.The change from string to int for Port and OpenConnections fields improves type safety and eliminates unnecessary type conversions.
76-81: LGTM! Simplified configuration retrieval.Direct access to database configuration through ORM's Config() method eliminates the need for SQL queries and improves code maintainability.
107-117: LGTM! Clean refactoring of database info retrieval.The refactored getDataBaseInfo method:
- Removes SQL queries in favor of ORM methods
- Properly handles database statistics
- Maintains clear separation of concerns
126-129: LGTM! Consistent string conversion using cast package.Proper use of cast.ToString for converting numeric values to strings in the display method.
database/orm/orm.go (5)
22-22: LGTM! Clean integration of database configuration.The dbConfig field and constructor updates properly integrate the database configuration into the Orm struct.
Also applies to: 35-35, 46-46
56-66: LGTM! Proper error handling in BuildOrm.The BuildOrm function correctly handles the new dbConfig return value and maintains proper error handling.
68-70: LGTM! Clean Config accessor method.Simple and effective accessor method for the database configuration.
77-89: LGTM! Consistent dbConfig handling in Connection method.The Connection method properly maintains dbConfig across different connection scenarios.
101-105: LGTM! Consistent database info retrieval.Database name, connection name, and version retrieval methods consistently use dbConfig.
Also applies to: 157-159
contracts/database/orm/orm.go (1)
11-12: LGTM! Clean interface additions.The new
Config()andVersion()methods are well-documented and follow good interface design principles.Also applies to: 33-34
mocks/database/orm/Orm.go (1)
29-72: LGTM! Correctly generated mock implementations.The mock implementations for
Config()andVersion()are properly generated with appropriate error handling and helper methods.Also applies to: 508-551
database/gorm/query.go (1)
66-66: LGTM! Clean function signature update.The
BuildQueryfunction has been properly updated to return the database configuration, with consistent error handling in the return statements.Also applies to: 69-69, 74-74, 79-79, 82-82
database/console/show_command_test.go (1)
4-4: LGTM! Comprehensive test updates.The test changes properly reflect the new ORM configuration approach:
- Mock expectations are correctly updated
- Test data values are reasonable
- Error cases are properly covered
Also applies to: 9-9, 34-34, 37-37, 39-39, 41-41, 43-43, 61-78, 90-103, 117-130, 139-142
|
Blocking my another PR, merge it first, please let me know if you have any concerns, I can fix them in the another PR. |
| } | ||
| } | ||
| func (r *ShowCommand) getDataBaseInfo() (name, version string, openConnections int, err error) { | ||
| name = r.schema.Orm().Name() |
There was a problem hiding this comment.
The database names should be PostgreSQL, MySQL, MariaDB, SQL Server, and SQLite, not lowercase driver name like postgres, mysql, sqlite, or sqlserver
📑 Description
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes
The updates provide more robust database configuration and connection management across the application's database-related components.
✅ Checks