Skip to content
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.

Sign up for GitHub

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

feat: sqlserver driver supports setting schema #786

Merged
merged 3 commits into from
Dec 25, 2024
Merged

feat: sqlserver driver supports setting schema #786

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5dcc21d
feat: sqlserver driver supports setting schema
hwbrzzl Dec 24, 2024
7202c41
fix typo
hwbrzzl Dec 24, 2024
8b4943d
Merge branch 'master' into bowen/sqlserver-schema
hwbrzzl Dec 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion database/gorm/query_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3938,7 +3938,7 @@ func TestTablePrefixAndSingular(t *testing.T) {
}
}

func TestSchema(t *testing.T) {
func TestPostgresSchema(t *testing.T) {
if env.IsWindows() {
t.Skip("Skip test that using Docker")
}
Expand All @@ -3958,6 +3958,26 @@ func TestSchema(t *testing.T) {
assert.True(t, user1.ID > 0)
}

func TestSqlserverSchema(t *testing.T) {
if env.IsWindows() {
t.Skip("Skip test that using Docker")
}

sqlserverDocker := supportdocker.Sqlserver()
require.NoError(t, sqlserverDocker.Ready())

testQuery := NewTestQueryWithSchema(sqlserverDocker, "goravel")
testQuery.CreateTable(TestTableSchema)

schema := Schema{Name: "first_schema"}
assert.Nil(t, testQuery.Query().Create(&schema))
assert.True(t, schema.ID > 0)

var schema1 Schema
assert.Nil(t, testQuery.Query().Where("name", "first_schema").First(&schema1))
assert.True(t, schema1.ID > 0)
}

func paginator(page string, limit string) func(methods contractsorm.Query) contractsorm.Query {
return func(query contractsorm.Query) contractsorm.Query {
page, _ := strconv.Atoi(page)
Expand Down
9 changes: 9 additions & 0 deletions database/gorm/test_models.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,3 +420,12 @@ type Box struct {
func (p *Box) Connection() string {
return "postgres"
}

type Schema struct {
Model
Name string
}

func (r *Schema) TableName() string {
return "goravel.schemas"
}
90 changes: 79 additions & 11 deletions database/gorm/test_utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
TestTableRoleUser
TestTableUsers
TestTableGoravelUser
TestTableSchema
)

var testContext = context.Background()
Expand Down Expand Up @@ -215,39 +216,48 @@
// Create schema before build query with the schema
mockConfig := &mocksconfig.Config{}
mockDriver := getMockDriver(docker, mockConfig, docker.Driver().String())
mockDriver.WithPrefixAndSingular()
mockDriver.Common()
query, err := BuildQuery(testContext, mockConfig, docker.Driver().String(), utils.NewTestLog(), nil)
if err != nil {
panic(fmt.Sprintf("connect to %s failed: %v", docker.Driver().String(), err))
}

if _, err := query.Exec(fmt.Sprintf("CREATE SCHEMA %s", schema)); err != nil {
panic(fmt.Sprintf("create schema %s failed: %v", schema, err))
}

mockConfig = &mocksconfig.Config{}
mockDriver = getMockDriver(docker, mockConfig, docker.Driver().String())
testQuery := &TestQuery{
docker: docker,
mockConfig: mockConfig,
mockDriver: mockDriver,
query: query,
}

mockDriver.WithSchema(schema)
if _, err := query.Exec(fmt.Sprintf(`CREATE SCHEMA "%s"`, schema)); err != nil {
panic(fmt.Sprintf("create schema %s failed: %v", schema, err))

Check warning on line 233 in database/gorm/test_utils.go

View check run for this annotation

Codecov / codecov/patch

database/gorm/test_utils.go#L233 

Added line #L233 was not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Test coverage for schema operations is adequate, but error scenarios need attention

Based on the verification results, I found that:

  1. Schema creation and basic operations are well-tested for both PostgreSQL and SQL Server in:

    • database/schema/schema_test.go
    • database/gorm/query_test.go

  2. However, error scenarios are not adequately covered:

    • The panic in NewTestQueryWithSchema for unsupported drivers is tested implicitly
    • Schema creation error handling (line 233) lacks explicit test coverage
    • SQL Server schema configuration (lines 584-588) has basic tests but no error scenarios
    • Schema table definitions (lines 1266-1307) have functional tests but lack edge cases

Recommendations:

  • Add test cases for schema creation failures
  • Include error scenarios for SQL Server schema operations
  • Add edge cases for schema table definitions with invalid inputs
🔗 Analysis chain

Improve test coverage for schema operations.

Several critical code paths lack test coverage:

  1. Schema creation error handling
  2. SQL Server schema configuration
  3. Schema table definitions for different databases

Let's verify the existing test coverage:

Would you like me to help create additional test cases to cover these scenarios?

Also applies to: 584-588, 1266-1307

Let's gather more information about the test coverage for schema operations.

Let's check the actual test implementations to verify the coverage of error scenarios.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for existing schema-related tests
# Look for test files that might contain schema-related tests
rg -l "TestSchema|test.*Schema|schema.*Test" --type go

Length of output: 349

Script:

#!/bin/bash
# Check for test cases related to schema creation and error handling
rg "func.*Test.*Schema.*" --type go -A 5

# Check for specific schema creation error tests
rg "TestSchema.*Error|TestCreate.*Schema.*Error" --type go -A 5

# Look for SQL Server schema configuration tests
rg "TestWithSchema|TestSQLServer.*Schema" --type go -A 5

# Check schema table definition tests
rg "TestSchema.*Table|TestCreateTable" --type go -A 5

Length of output: 6846

Script:

#!/bin/bash
# Check the content of schema-related tests
rg "TestPostgresSchema|TestSqlserverSchema" --type go -B 2 -A 20

# Look for panic handling in tests
rg "panic.*schema" --type go -B 2 -A 2

# Check schema creation error handling in tests
rg "create.*schema.*error|schema.*create.*error" --type go -i -B 2 -A 2

Length of output: 7973

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 233-233: database/gorm/test_utils.go#L233
Added line #L233 was not covered by tests

}

if docker.Driver() == contractsdatabase.DriverSqlserver {
return testQuery
}

mockConfig = &mocksconfig.Config{}
mockDriver = getMockDriver(docker, mockConfig, docker.Driver().String())
mockDriver.WithSchema(schema)
query, err = BuildQuery(testContext, mockConfig, docker.Driver().String(), utils.NewTestLog(), nil)
if err != nil {
panic(fmt.Sprintf("connect to %s failed: %v", docker.Driver().String(), err))
}

testQuery.query = query
testQuery = &TestQuery{
docker: docker,
mockConfig: mockConfig,
mockDriver: mockDriver,
query: query,
}

return testQuery
}

func (r *TestQuery) CreateTable(testTables ...TestTable) {
for table, sql := range newTestTables(r.docker.Driver()).All() {
if len(testTables) == 0 || slices.Contains(testTables, table) {
if (len(testTables) == 0 && table != TestTableSchema) || slices.Contains(testTables, table) {
if _, err := r.query.Exec(sql()); err != nil {
panic(fmt.Sprintf("create table %v failed: %v", table, err))
}
Expand Down Expand Up @@ -540,6 +550,8 @@
func (r *MockSqlserver) Common() {
r.mockConfig.EXPECT().GetString(fmt.Sprintf("database.connections.%s.prefix", r.connection)).Return("")
r.mockConfig.EXPECT().GetBool(fmt.Sprintf("database.connections.%s.singular", r.connection)).Return(false)
r.mockConfig.EXPECT().GetString(fmt.Sprintf("database.connections.%s.schema", r.connection), "dbo").Return("dbo")

r.single()
r.basic()
}
Expand All @@ -554,18 +566,26 @@
r.mockConfig.EXPECT().GetString(fmt.Sprintf("database.connections.%s.dsn", r.connection)).Return("")
r.mockConfig.EXPECT().GetString(fmt.Sprintf("database.connections.%s.prefix", r.connection)).Return("")
r.mockConfig.EXPECT().GetBool(fmt.Sprintf("database.connections.%s.singular", r.connection)).Return(false)
r.mockConfig.EXPECT().GetString(fmt.Sprintf("database.connections.%s.schema", r.connection), "dbo").Return("dbo")

r.basic()
}

func (r *MockSqlserver) WithPrefixAndSingular() {
r.mockConfig.EXPECT().GetString(fmt.Sprintf("database.connections.%s.prefix", r.connection)).Return("goravel_")
r.mockConfig.EXPECT().GetBool(fmt.Sprintf("database.connections.%s.singular", r.connection)).Return(true)
r.mockConfig.EXPECT().GetString(fmt.Sprintf("database.connections.%s.schema", r.connection), "dbo").Return("dbo")

r.single()
r.basic()
}

func (r *MockSqlserver) WithSchema(schema string) {
panic("sqlserver does not support schema for now")
r.mockConfig.EXPECT().GetString(fmt.Sprintf("database.connections.%s.prefix", r.connection)).Return("")
r.mockConfig.EXPECT().GetBool(fmt.Sprintf("database.connections.%s.singular", r.connection)).Return(false)
r.mockConfig.EXPECT().GetString(fmt.Sprintf("database.connections.%s.schema", r.connection), "public").Return(schema)
r.single()
r.basic()

Check warning on line 588 in database/gorm/test_utils.go

View check run for this annotation

Codecov / codecov/patch

database/gorm/test_utils.go#L584-L588 

Added lines #L584 - L588 were not covered by tests
}

func (r *MockSqlserver) basic() {
Expand Down Expand Up @@ -611,6 +631,7 @@
TestTableRoleUser: r.roleUser,
TestTableUsers: r.users,
TestTableGoravelUser: r.goravelUser,
TestTableSchema: r.schema,
}
}

Expand Down Expand Up @@ -1240,6 +1261,53 @@
}
}

func (r *testTables) schema() string {
switch r.driver {
case contractsdatabase.DriverMysql:
return `
CREATE TABLE goravel.schemas (
id bigint(20) unsigned NOT NULL AUTO_INCREMENT,
name varchar(255) NOT NULL,
created_at datetime(3) NOT NULL,
updated_at datetime(3) NOT NULL,
PRIMARY KEY (id),
KEY idx_schemas_created_at (created_at),
KEY idx_schemas_updated_at (updated_at)
) ENGINE = InnoDB DEFAULT CHARSET = utf8mb4;

Check warning on line 1276 in database/gorm/test_utils.go

View check run for this annotation

Codecov / codecov/patch

database/gorm/test_utils.go#L1266-L1276 

Added lines #L1266 - L1276 were not covered by tests
`
case contractsdatabase.DriverPostgres:
return `
CREATE TABLE goravel.schemas (
id SERIAL PRIMARY KEY NOT NULL,
name varchar(255) NOT NULL,
created_at timestamp NOT NULL,
updated_at timestamp NOT NULL
);

Check warning on line 1285 in database/gorm/test_utils.go

View check run for this annotation

Codecov / codecov/patch

database/gorm/test_utils.go#L1278-L1285 

Added lines #L1278 - L1285 were not covered by tests
`
case contractsdatabase.DriverSqlite:
return `
CREATE TABLE goravel.schemas (
id integer PRIMARY KEY AUTOINCREMENT NOT NULL,
name varchar(255) NOT NULL,
created_at datetime NOT NULL,
updated_at datetime NOT NULL
);

Check warning on line 1294 in database/gorm/test_utils.go

View check run for this annotation

Codecov / codecov/patch

database/gorm/test_utils.go#L1287-L1294 

Added lines #L1287 - L1294 were not covered by tests
`
case contractsdatabase.DriverSqlserver:
return `
CREATE TABLE goravel.schemas (
id bigint NOT NULL IDENTITY(1,1),
name varchar(255) NOT NULL,
created_at datetime NOT NULL,
updated_at datetime NOT NULL,
PRIMARY KEY (id)
);
`
default:
return ""

Check warning on line 1307 in database/gorm/test_utils.go

View check run for this annotation

Codecov / codecov/patch

database/gorm/test_utils.go#L1306-L1307 

Added lines #L1306 - L1307 were not covered by tests
}
}

func mockPool(mockConfig *mocksconfig.Config) {
mockConfig.EXPECT().GetInt("database.pool.max_idle_conns", 10).Return(10)
mockConfig.EXPECT().GetInt("database.pool.max_open_conns", 100).Return(100)
Expand Down
64 changes: 56 additions & 8 deletions database/migration/default_migrator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func (s *DefaultMigratorWithDBSuite) TestStatus() {
}
}

func TestDefaultMigratorWithWithSchema(t *testing.T) {
func TestDefaultMigratorWithPostgresSchema(t *testing.T) {
if env.IsWindows() {
t.Skip("Skip test that using Docker")
}
Expand All @@ -182,6 +182,32 @@ func TestDefaultMigratorWithWithSchema(t *testing.T) {

assert.NoError(t, migrator.Run())
assert.True(t, schema.HasTable("users"))
assert.NoError(t, migrator.Rollback(1, 0))
assert.False(t, schema.HasTable("users"))
}

func TestDefaultMigratorWithSqlserverSchema(t *testing.T) {
if env.IsWindows() {
t.Skip("Skip test that using Docker")
}

sqlserverDocker := docker.Sqlserver()
require.NoError(t, sqlserverDocker.Ready())

sqlserverQuery := gorm.NewTestQueryWithSchema(sqlserverDocker, "goravel")
schema := databaseschema.GetTestSchema(sqlserverQuery, map[contractsdatabase.Driver]*gorm.TestQuery{
contractsdatabase.DriverSqlserver: sqlserverQuery,
})
testMigration := NewTestMigrationWithSqlserverSchema(schema)
schema.Register([]contractsschema.Migration{
testMigration,
})
migrator := NewDefaultMigrator(nil, schema, "migrations")

assert.NoError(t, migrator.Run())
assert.True(t, schema.HasTable("goravel.users"))
assert.NoError(t, migrator.Rollback(1, 0))
assert.False(t, schema.HasTable("goravel.users"))
}

type DefaultMigratorSuite struct {
Expand Down Expand Up @@ -861,9 +887,9 @@ func (s *DefaultMigratorSuite) TestStatus() {
}

func (s *DefaultMigratorSuite) mockRunDown(
mockOrm *mocksorm.Orm,
previousConnection, migrationSignature, table string,
err error,
mockOrm *mocksorm.Orm,
previousConnection, migrationSignature, table string,
err error,
) {
s.mockSchema.EXPECT().GetConnection().Return(previousConnection).Once()
s.mockSchema.EXPECT().Orm().Return(mockOrm).Times(5)
Expand All @@ -888,10 +914,10 @@ func (s *DefaultMigratorSuite) mockRunDown(
}

func (s *DefaultMigratorSuite) mockRunUp(
mockOrm *mocksorm.Orm,
previousConnection, migrationSignature, table string,
batch int,
err error,
mockOrm *mocksorm.Orm,
previousConnection, migrationSignature, table string,
batch int,
err error,
) {
s.mockSchema.EXPECT().GetConnection().Return(previousConnection).Once()
s.mockSchema.EXPECT().Orm().Return(mockOrm).Times(5)
Expand Down Expand Up @@ -937,6 +963,28 @@ func (r *TestMigration) Down() error {
return r.schema.DropIfExists("users")
}

type TestMigrationWithSqlserverSchema struct {
schema contractsschema.Schema
}

func NewTestMigrationWithSqlserverSchema(schema contractsschema.Schema) *TestMigrationWithSqlserverSchema {
return &TestMigrationWithSqlserverSchema{schema: schema}
}

func (r *TestMigrationWithSqlserverSchema) Signature() string {
return "20240817214501_create_users_table"
}

func (r *TestMigrationWithSqlserverSchema) Up() error {
return r.schema.Create("goravel.users", func(table contractsschema.Blueprint) {
table.String("name")
})
}

func (r *TestMigrationWithSqlserverSchema) Down() error {
return r.schema.DropIfExists("goravel.users")
}

type TestConnectionMigration struct {
schema contractsschema.Schema
}
Expand Down
3 changes: 1 addition & 2 deletions database/schema/grammars/wrap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,8 @@ func (r *Wrap) Table(table string) string {
}
if strings.Contains(table, ".") {
lastDotIndex := strings.LastIndex(table, ".")
newTable := table[:lastDotIndex] + "." + r.tablePrefix + table[lastDotIndex+1:]

return r.Value(newTable)
return r.Value(table[:lastDotIndex]) + "." + r.Value(r.tablePrefix+table[lastDotIndex+1:])
}

return r.Value(r.tablePrefix + table)
Expand Down
Loading
Loading