feat: [#374] The postgres driver supports set schema #772
Conversation
WalkthroughThe pull request introduces support for setting a custom schema in PostgreSQL database configurations. A new Changes
Assessment against linked issues
Possibly related PRs
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 ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #772 +/- ##
==========================================
- Coverage 69.97% 69.94% -0.04%
==========================================
Files 213 213
Lines 18119 18164 +45
==========================================
+ Hits 12679 12705 +26
- Misses 4749 4762 +13
- Partials 691 697 +6 ☔ 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 (5)
database/gorm/test_utils.go (1)
169-187: Robust error handling and resource creation.The NewTestQuery function effectively constructs a test query, handles errors, and sets up relevant mocks. Consider adding clearer logs or additional checks for partial initialization failures.
contracts/database/config.go (1)
24-25: Introducing Schema field for Postgres.Adding this field extends configuration flexibility. Please ensure external references, environment variables, and docs mention the new field for clarity.
database/db/dsn_test.go (1)
18-18: Add more test cases for schema handling.While the basic test case is good, consider adding test cases for:
- Custom schema names
- Empty schema handling
- Invalid schema names (once validation is added)
Add these test cases:
func TestDsn(t *testing.T) { tests := []struct { name string config database.FullConfig expectDsn string }{ + { + name: "postgres with custom schema", + config: database.FullConfig{ + Config: database.Config{ + Host: testHost, + Port: testPort, + Database: testDatabase, + Username: testUsername, + Password: testPassword, + Schema: "custom_schema", + }, + Driver: database.DriverPostgres, + Sslmode: "disable", + Timezone: "UTC", + }, + expectDsn: fmt.Sprintf("postgres://%s:%s@%s:%d/%s?sslmode=%s&timezone=%s&search_path=%s", + testUsername, testPassword, testHost, testPort, testDatabase, "disable", "UTC", "custom_schema"), + }, + { + name: "postgres with empty schema", + config: database.FullConfig{ + Config: database.Config{ + Host: testHost, + Port: testPort, + Database: testDatabase, + Username: testUsername, + Password: testPassword, + Schema: "", + }, + Driver: database.DriverPostgres, + Sslmode: "disable", + Timezone: "UTC", + }, + expectDsn: fmt.Sprintf("postgres://%s:%s@%s:%d/%s?sslmode=%s&timezone=%s&search_path=%s", + testUsername, testPassword, testHost, testPort, testDatabase, "disable", "UTC", "public"), + },Also applies to: 27-27, 70-71
database/schema/schema.go (1)
47-47: LGTM! Consider adding documentationThe implementation correctly handles PostgreSQL schema configuration with a proper default value of "public".
Consider adding a code comment explaining the significance of the "public" schema in PostgreSQL and when users might want to configure a different schema.
foundation/application_test.go (1)
Line range hint
47-47: Consider documenting schema usage patternsThe implementation of PostgreSQL schema support is solid. Consider adding documentation in the README or configuration guide about:
- When and why users might want to use different schemas
- Best practices for schema management
- Migration considerations when using custom schemas
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
contracts/database/config.go(1 hunks)database/db/config_builder.go(1 hunks)database/db/dsn.go(1 hunks)database/db/dsn_test.go(3 hunks)database/gorm/query_test.go(1 hunks)database/gorm/test_utils.go(8 hunks)database/migration/default_migrator_test.go(3 hunks)database/migration/repository_test.go(1 hunks)database/schema/common_schema_test.go(1 hunks)database/schema/schema.go(1 hunks)database/schema/schema_test.go(4 hunks)foundation/application_test.go(1 hunks)
🔇 Additional comments (23)
database/gorm/test_utils.go (10)
38-38: New interface method for schema support.
The introduction of WithSchema(schema string) expands the interface for driver mocks, improving test coverage of schema-specific features.
149-155: Clear separation of prefix-and-singular code paths.
This conditional logic neatly segregates creation of queries with or without prefix-and-singular, promoting maintainability. No major issues found.
189-207: Method dedicated to prefix-and-singular initialization.
Adding NewTestQueryWithPrefixAndSingular clarifies test setup. The approach is consistent with the rest of the codebase.
344-346: MySQL schema support not implemented.
This function panics intentionally since MySQL doesn’t support user-defined schemas in the same way. Properly documented.
395-395: Default schema handling for Postgres.
Retrieving and defaulting to "public" is consistent, ensuring backward-compatibility for unspecified schemas.
410-410: Ensuring read/write handles default schema.
Again, the fallback to "public" is appropriate. No issues.
417-420: Prefix, singular, and schema usage.
Using WithPrefixAndSingular in tandem with a default schema maintains test uniformity.
422-425: Dynamic schema configuration in Postgres mock.
This newly added method properly sets up the schema override, verifying the "schema" key in the mock config.
493-495: SQLite schema usage panic.
SQLite doesn’t support distinct schemas. This panic clarifies unsupported usage.
559-561: SQL Server schema not supported yet.
Indicates a known limitation in the driver. Future expansions can remove this panic once schema support is ready.
database/schema/schema_test.go (4)
8-9: Additional testing libraries asserted.
The introduction of assert and require ensures more intuitive unit testing.
38-38: Migration to NewTestQueryWithPrefixAndSingular.
Replacing direct calls in SetupTest clarifies the test approach for prefix and singular.
Also applies to: 41-41, 46-46, 51-51
2334-2354: TestSpecificSchema ensures custom schema creation.
This thorough test confirms that tables can be created in a user-defined schema. Properly checks table name, schema name, and handles Docker readiness.
2154-2154: Mock configuration usage in teardown.
Checking for empty schema with a mock ensures graceful cleanup. This line highlights asynchronous interactions that might require caution if parallel tests are introduced.
✅ Verification successful
Let me gather more context about the test setup and teardown patterns.
Based on the context gathered, I can now generate the final response:
Mock configuration usage is appropriate and safe in this context
The mock configuration is used in a controlled test teardown sequence for dropping tables, and there's no evidence of parallel test execution in the codebase. The .Once() expectation ensures proper cleanup of the mock after use.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for references to testQuery.MockConfig() in teardown to ensure consistent usage
rg "testQuery.MockConfig()" -A 5
Length of output: 1263
Script:
#!/bin/bash
# Look for the test context around the MockConfig usage
rg -B 10 -A 10 "testQuery\.MockConfig\(\)\.EXPECT\(\)\.GetString\(\"database\.connections\.postgres\.schema\"\)"
# Also check for parallel test configurations
ast-grep --pattern 'func ($name *testing.T) {
$$$
$name.Parallel()
$$$
}'
Length of output: 1428
database/schema/common_schema_test.go (1)
31-31: Prefixed test query for Postgres.
This change aligns common_schema_test with the updated function usage. The test suite remains consistent across all DB drivers.
database/db/config_builder.go (1)
76-78: LGTM! Default schema handling looks correct.
The implementation properly handles the schema configuration for PostgreSQL with a sensible default of "public". This follows PostgreSQL conventions and maintains backward compatibility.
Let's verify the schema configuration handling:
✅ Verification successful
Let me gather more information about the schema handling implementation.
Schema configuration handling is consistent across the codebase
The verification confirms that the schema configuration with default "public" is consistently implemented:
- The config_builder correctly handles schema configuration for PostgreSQL
- Test utilities properly mock the schema configuration with the same default
- Schema package uses the same configuration pattern and default value
- All schema-related configurations follow the same path format: "database.connections.{connection}.schema"
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if schema configuration is consistently handled across the codebase
rg -g '*.go' -A 2 -B 2 'schema.*postgres|postgres.*schema'
Length of output: 2580
Script:
#!/bin/bash
# Check schema-related code in config_builder and related files
ast-grep --pattern 'Schema.*=.*public'
# Check for any other schema-related configuration handling
rg -g '*.go' 'GetString.*schema' -A 2 -B 2
Length of output: 3712
database/db/dsn.go (1)
23-24: 🛠️ Refactor suggestion
Consider adding schema name validation.
While the implementation correctly adds the schema to the search_path, we should ensure the schema name is properly validated to prevent potential SQL injection. PostgreSQL schema names could contain special characters that might need escaping.
Let's check for any existing schema name validation:
Consider adding schema name validation:
func Dsn(config database.FullConfig) string {
+ // Validate schema name to prevent SQL injection
+ if !isValidSchemaName(config.Schema) {
+ return ""
+ }
if config.Dsn != "" {
return config.Dsn
}database/migration/repository_test.go (1)
32-32: Verify test coverage with schema configurations
The change to use NewTestQueryWithPrefixAndSingular looks good, but let's ensure it properly handles schema configurations in tests.
✅ Verification successful
Let's examine the test coverage more deeply by looking at the actual test cases.
Let's check the implementation of NewTestQueryWithPrefixAndSingular to understand how it relates to schema handling.
Let's check one more thing to understand the relationship between schema handling and the test setup.
Let's check one final thing - the implementation of NewTestQueryWithPrefixAndSingular to understand how it handles schema configuration.
Based on the comprehensive analysis of the codebase, I can now generate the final response:
Schema handling is properly covered in test configurations
The change to NewTestQueryWithPrefixAndSingular is well-supported by the test infrastructure:
- The test suite includes dedicated schema test cases in
database/schema/schema_test.go - The repository tests properly initialize test queries with schema support through
schema.GetTestSchema - Schema configurations are properly handled in both PostgreSQL and SQLServer drivers
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify test coverage for schema configurations
# Check if there are corresponding test cases for schema handling
# Search for schema-related test cases
rg -l "schema.*test" database/
Length of output: 205
Script:
#!/bin/bash
# Check the actual test cases in repository_test.go and related files
rg "func.*Test.*Schema" database/ -A 5
# Also check for schema-related assertions in repository_test.go
rg "schema" database/migration/repository_test.go
Length of output: 4506
Script:
#!/bin/bash
# Check the implementation of NewTestQueryWithPrefixAndSingular
ast-grep --pattern 'func NewTestQueryWithPrefixAndSingular' -A 20
# Also check how it's used in repository_test.go
rg -B 2 -A 5 "NewTestQueryWithPrefixAndSingular" database/migration/repository_test.go
Length of output: 381
Script:
#!/bin/bash
# Check the test file content to understand the test setup and assertions
cat database/migration/repository_test.go
# Also check if there are any other tests using NewTestQueryWithPrefixAndSingular
rg "NewTestQueryWithPrefixAndSingular" database/
Length of output: 4917
Script:
#!/bin/bash
# Check the implementation of NewTestQueryWithPrefixAndSingular in test_utils.go
cat database/gorm/test_utils.go
Length of output: 38051
foundation/application_test.go (1)
334-334: LGTM! Test coverage looks good
The test properly verifies the schema configuration behavior with the default value.
database/migration/default_migrator_test.go (3)
12-12: LGTM: Good practice using require package
Using require package for assertions is a good practice as it provides better error messages and stops test execution on failure.
47-70: LGTM: Improved test setup with better error handling
The test setup has been enhanced with:
- Proper error handling using
Require().NoError - Clear separation of test query setup for different databases
- Consistent initialization pattern
120-136: LGTM: Well-structured schema support test
The test case properly validates schema support by:
- Setting up a Postgres docker instance
- Creating a test query with "goravel" schema
- Testing basic CRUD operations within the schema
- Using appropriate assertions
database/gorm/query_test.go (1)
3834-3848: LGTM: Comprehensive schema support test
The test case effectively validates schema support by:
- Setting up a Postgres docker instance
- Creating a test query with "goravel" schema
- Testing basic CRUD operations
- Using appropriate assertions
The test structure aligns well with the existing test patterns in the file.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.
| Benchmark suite | Current: 55f892c | Previous: 1a81ebb | Ratio |
|---|---|---|---|
BenchmarkFile_ReadWrite |
320408 ns/op 2072 B/op 27 allocs/op |
188050 ns/op 2072 B/op 27 allocs/op |
1.70 |
BenchmarkFile_ReadWrite - ns/op |
320408 ns/op |
188050 ns/op |
1.70 |
This comment was automatically generated by workflow using github-action-benchmark.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
database/gorm/query_test.go (1)
3834-3852: Test implementation looks good but could be more comprehensive.The basic test implementation for schema support is correct, but consider enhancing it with:
Additional test cases:
- Verify operations fail with incorrect schema
- Test schema switching
- Validate data isolation between schemas
Schema verification:
- Add assertions to confirm data is stored in the correct schema
- Test schema-specific queries
Consider expanding the test with these additional cases:
func TestSchema(t *testing.T) { if env.IsWindows() { t.Skip("Skip test that using Docker") } postgresDocker := supportdocker.Postgres() require.NoError(t, postgresDocker.Ready()) testQuery := NewTestQueryWithSchema(postgresDocker, "goravel") testQuery.CreateTable(TestTableUsers) user := User{Name: "first_user"} assert.Nil(t, testQuery.Query().Create(&user)) assert.True(t, user.ID > 0) var user1 User assert.Nil(t, testQuery.Query().Where("name", "first_user").First(&user1)) assert.True(t, user1.ID > 0) + + // Test schema isolation + defaultQuery := NewTestQuery(postgresDocker) + defaultQuery.CreateTable(TestTableUsers) + + var user2 User + assert.Nil(t, defaultQuery.Query().Where("name", "first_user").First(&user2)) + assert.Equal(t, uint(0), user2.ID, "User should not be found in default schema") + + // Verify schema switching + testQuery2 := NewTestQueryWithSchema(postgresDocker, "another_schema") + testQuery2.CreateTable(TestTableUsers) + + user3 := User{Name: "second_user"} + assert.Nil(t, testQuery2.Query().Create(&user3)) + assert.True(t, user3.ID > 0) + + var user4 User + assert.Nil(t, testQuery.Query().Where("name", "second_user").First(&user4)) + assert.Equal(t, uint(0), user4.ID, "User should not be found in different schema") + + // Cleanup + require.NoError(t, postgresDocker.Shutdown()) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
database/gorm/query_test.go(1 hunks)database/migration/default_migrator_test.go(4 hunks)database/schema/blueprint_test.go(1 hunks)database/schema/schema_test.go(4 hunks)
🔇 Additional comments (6)
database/schema/blueprint_test.go (1)
373-376: LGTM: Test case properly handles PostgreSQL column comments
The test case correctly verifies that PostgreSQL generates two SQL statements when creating a table with column comments - one for table creation and another for setting the column comment.
database/migration/default_migrator_test.go (2)
47-70: LGTM: Improved test setup with better error handling
The changes improve the test setup by:
- Using
Require().NoErrorfor critical setup steps which fails fast on errors - Properly initializing test queries for all supported database drivers
154-174: LGTM: Comprehensive test coverage for schema support
The new test function properly verifies PostgreSQL schema support by:
- Creating a custom schema
- Creating a table within that schema
- Verifying the operation's success
database/schema/schema_test.go (3)
8-9: LGTM: Added necessary test utility imports
Added required testify imports for improved test assertions.
2154-2154: LGTM: Added schema configuration test coverage
Added mock expectation to verify behavior when no schema is configured (default case).
2334-2358: LGTM: Added comprehensive schema-specific test
The new test function provides thorough verification of schema support:
- Creates a table in a specific schema
- Verifies table creation
- Validates schema name in table metadata
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
database/gorm/query_test.go (1)
3839-3841:⚠️ Potential issueAdd container cleanup to prevent resource leaks.
The PostgreSQL Docker container is not being cleaned up after the test, which could lead to resource leaks.
Add container cleanup in a
deferstatement right after container creation:postgresDocker := supportdocker.Postgres() require.NoError(t, postgresDocker.Ready()) +defer func() { + require.NoError(t, postgresDocker.Shutdown()) +}()
🧹 Nitpick comments (6)
database/migration/default_migrator_test.go (1)
72-74: Teardown step for SQLite
Calling s.driverToTestQuery[contractsdatabase.DriverSqlite].Docker().Shutdown() is good housekeeping to prevent leftover resources. For other drivers, consider applying a similar teardown routine whenever feasible, to ensure containers aren’t left running.database/gorm/test_utils.go (2)
189-207: Isolated logic for prefix + singular testing
NewTestQueryWithPrefixAndSingular clearly serves specialized test conditions. Ensuring each constructor has a single responsibility leads to a more flexible test setup.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 201-201: database/gorm/test_utils.go#L201
Added line #L201 was not covered by tests
559-560: SQL Server partial schema support
The panic message clarifies “sqlserver does not support schema for now.” If partial or future support is planned, consider referencing an issue or placeholder for future extension.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 559-560: database/gorm/test_utils.go#L559-L560
Added lines #L559 - L560 were not covered by testsdatabase/db/dsn_test.go (1)
18-18: Add test cases for different schema scenariosWhile the basic schema test is good, consider adding test cases for:
- Empty schema (should default to "public")
- Custom schema values
- Invalid schema names (after implementing validation)
Example test cases to add:
{ name: "postgres with empty schema", config: database.FullConfig{ Config: database.Config{ // ... other fields ... Schema: "", // Empty schema }, Driver: database.DriverPostgres, // ... other fields ... }, expectDsn: "postgres://...&search_path=public", // Should use default }, { name: "postgres with custom schema", config: database.FullConfig{ Config: database.Config{ // ... other fields ... Schema: "custom_schema", }, Driver: database.DriverPostgres, // ... other fields ... }, expectDsn: "postgres://...&search_path=custom_schema", },Also applies to: 27-27, 70-71
database/schema/schema.go (1)
47-47: LGTM! Consider adding documentationThe implementation correctly handles schema configuration with a sensible default of "public". This is a good practice for PostgreSQL databases.
Consider adding a comment explaining the schema configuration option in the code, for example:
+// Default schema is "public" for PostgreSQL if not explicitly configured schema = config.GetString(fmt.Sprintf("database.connections.%s.schema", orm.Name()), "public")database/schema/blueprint_test.go (1)
373-376: LGTM! Consider additional test casesThe test correctly verifies PostgreSQL-specific behavior for column comments. The implementation is clean and well-structured.
Consider adding test cases for:
- Multiple column comments in a single table
- Comments with special characters
- Schema-specific column comments
Example addition:
+// Test multiple column comments +s.blueprint.Create() +s.blueprint.String("name").Comment("name comment") +s.blueprint.String("description").Comment("desc comment") +if driver == database.DriverPostgres { + s.Len(s.blueprint.ToSql(grammar), 3) +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
contracts/database/config.go(1 hunks)database/db/config_builder.go(1 hunks)database/db/dsn.go(1 hunks)database/db/dsn_test.go(3 hunks)database/gorm/query_test.go(1 hunks)database/gorm/test_utils.go(8 hunks)database/migration/default_migrator_test.go(4 hunks)database/migration/repository_test.go(1 hunks)database/schema/blueprint.go(0 hunks)database/schema/blueprint_test.go(1 hunks)database/schema/common_schema_test.go(1 hunks)database/schema/schema.go(1 hunks)database/schema/schema_test.go(4 hunks)foundation/application_test.go(1 hunks)
💤 Files with no reviewable changes (1)
- database/schema/blueprint.go
🧰 Additional context used
🪛 GitHub Check: codecov/patch
database/db/config_builder.go
[warning] 77-78: database/db/config_builder.go#L77-L78
Added lines #L77 - L78 were not covered by tests
database/gorm/test_utils.go
[warning] 181-181: database/gorm/test_utils.go#L181
Added line #L181 was not covered by tests
[warning] 201-201: database/gorm/test_utils.go#L201
Added line #L201 was not covered by tests
[warning] 211-211: database/gorm/test_utils.go#L211
Added line #L211 was not covered by tests
[warning] 220-220: database/gorm/test_utils.go#L220
Added line #L220 was not covered by tests
[warning] 224-224: database/gorm/test_utils.go#L224
Added line #L224 was not covered by tests
[warning] 344-345: database/gorm/test_utils.go#L344-L345
Added lines #L344 - L345 were not covered by tests
[warning] 493-494: database/gorm/test_utils.go#L493-L494
Added lines #L493 - L494 were not covered by tests
[warning] 559-560: database/gorm/test_utils.go#L559-L560
Added lines #L559 - L560 were not covered by tests
🔇 Additional comments (21)
database/migration/default_migrator_test.go (6)
12-12: Use of "require" for immediate test failure is good practice
Switching from s.NoError to s.Require().NoError ensures that test execution stops immediately if this step fails, preventing subsequent assertions from running on invalid states.
47-48: Immediate readiness check ensures test stability
Calling s.Require().NoError(postgresDocker.Ready()) here helps confirm that the Docker container is fully ready before proceeding. This avoids hard-to-trace errors from partially initialized containers.
49-50: Consistent creation of Postgres test query
Creating a new test query object with prefix and singular settings is consistent with the updated approach. This helps ensure naming consistency across migrations and queries.
65-70: Streamlined driver assignment
Organizing the driver to test query mappings here makes the test suite easier to maintain and read. The approach is concise and follows a uniform pattern for each driver.
91-92: Explicit status check after migrations
Confirming the migration status helps validate that the migrations have actually run as expected.
154-175: Integration test for schema-specific migrations
Introducing TestDefaultMigratorWithWithSchema ensures that the new schema handling logic is correctly validated. This helps confirm that migrations target the intended schema rather than defaulting to "public".
database/gorm/test_utils.go (5)
149-155: Refactored query creation with prefix and singular
Using a separate function (NewTestQueryWithPrefixAndSingular) clarifies intent and avoids overloading the standard NewTestQuery. This separation helps maintain a clear interface for different testing scenarios.
169-187: Enhanced clarity for basic test query creation
NewTestQuery now reads more succinctly, focusing on minimal setup while delegating prefix/singular logic to other constructors. This design is clean and improves maintainability.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 181-181: database/gorm/test_utils.go#L181
Added line #L181 was not covered by tests
209-237: Schema creation for testing without teardown
You create a custom schema (“CREATE SCHEMA %s”) for tests but don’t drop it afterward. Over time or in concurrent runs, this can create clutter and potential resource constraints. Consider adding a cleanup step in test teardown.
Here’s an example approach:
- Add a mechanism to track schemas created in NewTestQueryWithSchema.
- Provide a teardown or cleanup method to drop these schemas at the end of the tests.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 211-211: database/gorm/test_utils.go#L211
Added line #L211 was not covered by tests
[warning] 220-220: database/gorm/test_utils.go#L220
Added line #L220 was not covered by tests
[warning] 224-224: database/gorm/test_utils.go#L224
Added line #L224 was not covered by tests
344-345: Confirmed MySQL schema usage raises an error
The panic is appropriate since MySQL doesn’t support separately named schemas. This makes the limitation explicit.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 344-345: database/gorm/test_utils.go#L344-L345
Added lines #L344 - L345 were not covered by tests
493-494: SQLite schema usage raises an error
Similarly for SQLite, the panic clarifies that custom schemas are not supported, making behavior predictable.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 493-494: database/gorm/test_utils.go#L493-L494
Added lines #L493 - L494 were not covered by tests
contracts/database/config.go (1)
24-25: Schema field addition
Adding Schema string broadens configuration for PostgreSQL while preserving backward compatibility for other drivers.
database/schema/common_schema_test.go (1)
31-31: Use of NewTestQueryWithPrefixAndSingular
Switching to the dedicated constructor clarifies that prefix and singular naming conventions are in effect for this test. This improves consistency with the rest of the codebase.
database/db/dsn.go (1)
23-24: LGTM! PostgreSQL DSN format is correct
The addition of search_path parameter to the PostgreSQL connection string is implemented correctly.
foundation/application_test.go (1)
334-334: LGTM! Test coverage added for postgres schema configuration.
The test properly verifies the new schema configuration functionality by expecting the schema config to be retrieved with a default value of "public".
database/schema/schema_test.go (4)
8-9: LGTM! Enhanced test assertions with better testing packages.
The addition of assert/require packages improves test assertions and error handling.
38-38: LGTM! Standardized test query initialization.
Test query initialization updated to use WithPrefixAndSingular consistently across test files.
Also applies to: 41-41, 46-46, 51-51
2154-2154: LGTM! Updated schema configuration path.
Schema configuration retrieval updated to use the new standardized path.
2334-2358: LGTM! Comprehensive test coverage for schema-specific functionality.
The new test validates:
- Table creation in a specific schema
- Schema name verification
- Table existence checks
Run the following script to verify schema-related test coverage:
✅ Verification successful
Schema-specific functionality is well-tested across the codebase
The verification shows comprehensive test coverage for schema functionality:
- Schema-specific operations in
TestSpecificSchema - Schema handling across different database drivers (Postgres, MySQL, SQLite, SQLServer)
- Schema operations including:
- Table creation and verification
- Column operations
- Index management
- Foreign key relationships
- Enum types
- Views
- Custom SQL execution
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify test coverage for schema functionality
# Test: Search for schema-related tests. Expect: Tests for creation, verification and table operations.
rg -A 5 'TestSpecific.*Schema|schema.*test'
Length of output: 14523
database/gorm/query_test.go (1)
3834-3852: LGTM! Added test coverage for schema-specific queries.
The test validates:
- Table creation in a specific schema
- Record creation and retrieval
database/migration/repository_test.go (1)
32-32: Verify test coverage with schema configurations
The change to use NewTestQueryWithPrefixAndSingular aligns with schema support. However, we should ensure test coverage for various schema configurations.
✅ Verification successful
Let me gather more information about the new method and its usage.
Let me check one more thing to understand the implementation of NewTestQueryWithPrefixAndSingular and its relationship with NewTestQuery.
Schema-related test coverage is comprehensive
The change to use NewTestQueryWithPrefixAndSingular is well-aligned with existing test coverage. The method is already being used across multiple test files including schema_test.go, common_schema_test.go, and default_migrator_test.go, covering all supported database drivers (Postgres, SQLite, MySQL, and SQL Server). The change in repository_test.go follows this established pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check test coverage for schema-related functionality
# Look for other test files that might need similar updates
# Find all test files that might need similar updates
fd -e go -t f "test" | xargs rg "NewTestQuery\("
Length of output: 1232
Script:
#!/bin/bash
# Check the implementation and usage of NewTestQueryWithPrefixAndSingular
ast-grep --pattern 'func NewTestQueryWithPrefixAndSingular'
# Also check for any existing tests with schema configurations
rg -l "schema|prefix|singular" database/
Length of output: 1976
Script:
#!/bin/bash
# Check the implementation of both methods and their relationship
rg -A 5 "func NewTestQuery|func NewTestQueryWithPrefixAndSingular" database/
# Also check for any existing tests using NewTestQueryWithPrefixAndSingular
rg "NewTestQueryWithPrefixAndSingular" database/
Length of output: 2771
|
@coderabbitai resolve |
✅ Actions performedComments resolved. |
📑 Description
Closes goravel/goravel#374
Tested locally, will implement sqlserver schema in next PR.
Summary by CodeRabbit
New Features
Schemafield in PostgreSQL database configurations.search_pathparameter.Bug Fixes
Tests
Chores
✅ Checks