grammar.go (1)

Line range hint 554-561: Check references to schema.ColumnType again.

At line 555, pipeline fails. Migrate this usage to contractsschema or define a helper method.

🧰 Tools

🪛 GitHub Check: test / ubuntu (1.23)

[failure] 555-555:
undefined: schema.ColumnType

🪛 GitHub Check: codecov / codecov

[failure] 555-555:
undefined: schema.ColumnType

🪛 GitHub Check: lint / lint

[failure] 555-555:
undefined: schema.ColumnType) (typecheck)

🪛 GitHub Check: test / ubuntu (1.22)

[failure] 555-555:
undefined: schema.ColumnType

gorm.go (1)

Line range hint 89-97: Add validation for required DNS fields.

The DNS generation doesn't validate required fields before formatting the string.

 func (r *Gorm) dns(config contracts.FullConfig) string {
     if config.Dsn != "" {
         return config.Dsn
     }
-    if config.Host == "" {
+    if config.Host == "" || config.Username == "" || config.Database == "" {
         return ""
     }
+    
+    port := config.Port
+    if port == 0 {
+        port = 5432 // Default PostgreSQL port
+    }

     return fmt.Sprintf("postgres://%s:%s@%s:%d/%s?sslmode=%s&timezone=%s&search_path=%s",
-        config.Username, config.Password, config.Host, config.Port, config.Database, config.Sslmode, config.Timezone, config.Schema)
+        config.Username, config.Password, config.Host, port, config.Database, 
+        defaultString(config.Sslmode, "disable"),
+        defaultString(config.Timezone, "UTC"),
+        defaultString(config.Schema, "public"))
 }
+
+func defaultString(value, defaultValue string) string {
+    if value == "" {
+        return defaultValue
+    }
+    return value
+}

postgres.go (2)

20-23: Use consistent naming for configuration references.

These newly added fields might cause confusion between configFacade and config. Consider renaming them for clarity, e.g., rawConfig vs. derivedConfig.

---

41-50: Ensure version retrieval is needed.

Storing version in database.Config might be optional. Verify if consumers rely on the version or if it's extraneous.

Would you like help writing a separate test to confirm this version is used properly?

docker_test.go (7)

4-4: Validate format import usage.

Line 4 imports "fmt". Confirm all references are used. Otherwise, remove it.

---

8-9: Revisit mocking strategy.

The mocks for config.Config and contracts might introduce tight coupling. Consider an interface-based approach for improved maintainability.

---

13-23: Add field documentation.

Add clarity to new suite fields: connection, database, username, password, etc. This helps maintainers understand the suite’s purpose.

---

37-48: Ensure table creation test covers rollback scenario.

Tests create and manipulate tables; consider verifying the environment resets or cleans up automatically in CI to avoid leftover test data.

---

79-79: Consider variable naming improvement.

databaseDriver is understandable but might conflict with internal usage. Possibly rename to freshDriver or similar.

---

86-101: Avoid mixing multiple tests in a single method.

TestDatabase verifies building, connecting, and shutting down. Consider splitting into smaller, more focused test methods.

---

103-103: Add comment for test logic.

Explain the reason behind TestImage beyond simply verifying the Docker image assignment. This helps maintain long-term code clarity.

grammar.go (19)

19-20: Document usage of prefix.

Briefly explain what prefix represents, e.g., table prefixes in multi-tenant schemas.

---

77-77: Rename command method?

CompileComment might conflict if there's a future naming in the framework. Evaluate prefixing all new methods with Compile or Generate.

---

105-125: Drop All Tables approach.

Check if you must also handle views or other object types. Also confirm if cascade suits all use cases or might cause unintended drops.

---

128-149: Refactor domain / type dropping logic.

The logic is correct but consider extracting into smaller functions for clarity. For instance, compileDropDomains() and compileDropTypes().

---

166-168: Consistent naming for array-based statements.

Check if CompileDropColumn aligns with the naming convention used in CompileDropAllTables or others. Consistency can aid maintainability.

---

190-190: Potential naming collision in constraints.

CompileDropPrimary might clash if the user previously named an index that ends with _pkey. Document assumptions or add a check to avoid collisions.

---

201-201: Add some basic constraints checks.

Appropriate for foreign keys. Consider also supporting DEFERRABLE constraints in the future if needed (like you did for unique).

---

243-245: Check default language.

Defaulting to "english" for CompileFullText might not suit all locales. Let’s keep it config-driven or documented.

---

270-277: Check table prefix injection.

Here you unify schema and table but add r.prefix to table. Guarantee no collisions occur if schema also uses a prefix.

---

292-292: Close string formatting with caution.

Ensure the final argument matches the placeholders. Some format strings can break if additional placeholders are introduced later.

---

Line range hint 303-307: Renaming indexes across schemas.

If a user tries renaming indexes in a different schema, ensure the statement includes the schema or prefix.

---

Line range hint 393-403: Nullability toggles on changes.

ModifyNullable is correct logically, but watch for conflicts with ModifyIncrement. Test carefully.

🧰 Tools

🪛 GitHub Check: test / ubuntu (1.23)

[failure] 382-382:
undefined: schema.ColumnDefaultValue

---

[failure] 387-387:
undefined: schema.ColumnDefaultValue

🪛 GitHub Check: codecov / codecov

[failure] 382-382:
undefined: schema.ColumnDefaultValue

---

[failure] 387-387:
undefined: schema.ColumnDefaultValue

🪛 GitHub Check: lint / lint

[failure] 382-382:
undefined: schema.ColumnDefaultValue

---

[failure] 387-387:
undefined: schema.ColumnDefaultValue

---

[failure] 382-382:
undefined: schema.ColumnDefaultValue

---

[failure] 387-387:
undefined: schema.ColumnDefaultValue

🪛 GitHub Check: test / ubuntu (1.22)

[failure] 382-382:
undefined: schema.ColumnDefaultValue

---

[failure] 387-387:
undefined: schema.ColumnDefaultValue

---

414-416: Prefer bigserial over biginteger?

Your logic is correct, but if almost always used with sequences, consider defaulting to bigserial.

---

422-424: Limit usage.

TypeChar is rarely used in Postgres. Confirm it matches the intended semantic vs. using varchar.

---

435-435: Use consistent naming.

TypeDateTime delegates to TypeTimestamp. The naming might confuse users expecting dedicated date-time logic.

---

451-453: Validate enumerations at creation.

This approach works, but consider partial or dynamic enumerations. Possibly store them in a domain instead.

---

464-466: Consider sequence approach.

For integer columns, serial is typical. If the user wants manual sequences, consider implementing a separate method.

---

484-488: Duplicate approach with medium text.

TypeMediumText also returns text. This is consistent but may cause confusion if users expect different size constraints.

---

Line range hint 545-552: Coordinate with blueprint columns.

The logic in getColumns fetches added columns, but ensure you handle indexes, foreign keys, or constraints consistently.

🧰 Tools

🪛 GitHub Check: test / ubuntu (1.23)

[failure] 523-523:
undefined: schema.Expression

---

[failure] 531-531:
undefined: schema.Expression

🪛 GitHub Check: codecov / codecov

[failure] 523-523:
undefined: schema.Expression

---

[failure] 531-531:
undefined: schema.Expression

🪛 GitHub Check: lint / lint

[failure] 523-523:
undefined: schema.Expression

---

[failure] 531-531:
undefined: schema.Expression

🪛 GitHub Check: test / ubuntu (1.22)

[failure] 523-523:
undefined: schema.Expression

---

[failure] 531-531:
undefined: schema.Expression

query.go (1)

5-22: Consider improving documentation and naming.

1. Add godoc comments to document the struct and methods.
2. Consider using a more descriptive receiver name than 'r' (e.g., 'q' for Query).

Example improvement:

+// Query provides PostgreSQL-specific query operations
 type Query struct {
 }

 func NewQuery() *Query {
 	return &Query{}
 }

+// LockForUpdate returns a FOR UPDATE lock expression
-func (r *Query) LockForUpdate() clause.Expression {
+func (q *Query) LockForUpdate() clause.Expression {
 	return clause.Locking{Strength: "UPDATE"}
 }

+// RandomOrder returns a random ordering expression
-func (r *Query) RandomOrder() string {
+func (q *Query) RandomOrder() string {
 	return "RANDOM()"
 }

+// SharedLock returns a FOR SHARE lock expression
-func (r *Query) SharedLock() clause.Expression {
+func (q *Query) SharedLock() clause.Expression {
 	return clause.Locking{Strength: "SHARE"}
 }

wrap.go (1)

103-103: Consider consolidating aliased table/value logic.

Both aliasedTable and aliasedValue methods have similar prefix handling logic. Consider extracting the common prefix handling into a helper method.

+func (r *Wrap) addPrefix(value string) string {
+    return r.prefix + value
+}

 func (r *Wrap) aliasedTable(table string) string {
     segments := strings.Split(table, " as ")
-    return r.Table(segments[0]) + " as " + r.Value(r.prefix+segments[1])
+    return r.Table(segments[0]) + " as " + r.Value(r.addPrefix(segments[1]))
 }

 func (r *Wrap) aliasedValue(value string) string {
     segments := strings.Split(value, " as ")
-    return r.Column(segments[0]) + " as " + r.Value(r.prefix+segments[1])
+    return r.Column(segments[0]) + " as " + r.Value(r.addPrefix(segments[1]))
 }

Also applies to: 109-109

postgres_test.go (1)

63-64: Strengthen version check assertion.

The version check only verifies the presence of "Debian". Consider adding more specific version checks.

-assert.Contains(t, version, "Debian")
+assert.Regexp(t, `PostgreSQL \d+\.\d+\.\d+ on .* \(Debian`, version)

config.go (1)

Line range hint 50-98: Reduce repetition in default config handling.

The default config handling has repetitive patterns for each field. Consider using reflection or a map-based approach.

+func (r *Config) getConfigString(key string, defaultValue ...string) string {
+    configKey := fmt.Sprintf("database.connections.%s.%s", r.connection, key)
+    if len(defaultValue) > 0 {
+        return r.config.GetString(configKey, defaultValue[0])
+    }
+    return r.config.GetString(configKey)
+}

 func (r *Config) fillDefault(configs []contracts.Config) []contracts.FullConfig {
     // ... existing code ...
     
     if fullConfig.Host == "" {
-        fullConfig.Host = r.config.GetString(fmt.Sprintf("database.connections.%s.host", r.connection))
+        fullConfig.Host = r.getConfigString("host")
     }
     // Apply similar pattern to other fields

gorm.go (1)

66-70: Extract pool configuration constants.

The pool configuration uses magic numbers for defaults. Consider extracting these as named constants.

+const (
+    DefaultMaxIdleConns    = 10
+    DefaultMaxOpenConns    = 100
+    DefaultConnMaxIdleTime = 3600
+    DefaultConnMaxLifetime = 3600
+)

 func (r *Gorm) configurePool(instance *gorm.DB) error {
     config := r.config.Config()
-    db.SetMaxIdleConns(config.GetInt("database.pool.max_idle_conns", 10))
-    db.SetMaxOpenConns(config.GetInt("database.pool.max_open_conns", 100))
-    db.SetConnMaxIdleTime(time.Duration(config.GetInt("database.pool.conn_max_idletime", 3600)) * time.Second)
-    db.SetConnMaxLifetime(time.Duration(config.GetInt("database.pool.conn_max_lifetime", 3600)) * time.Second)
+    db.SetMaxIdleConns(config.GetInt("database.pool.max_idle_conns", DefaultMaxIdleConns))
+    db.SetMaxOpenConns(config.GetInt("database.pool.max_open_conns", DefaultMaxOpenConns))
+    db.SetConnMaxIdleTime(time.Duration(config.GetInt("database.pool.conn_max_idletime", DefaultConnMaxIdleTime)) * time.Second)
+    db.SetConnMaxLifetime(time.Duration(config.GetInt("database.pool.conn_max_lifetime", DefaultConnMaxLifetime)) * time.Second)

processor_test.go (1)

103-134: Enhance test coverage with additional edge cases.

While the current test cases cover basic scenarios, consider adding tests for:

1. Indexes with multiple columns in different orders
2. Case sensitivity in index names
3. Invalid index types
4. Duplicate index names

 func (s *ProcessorTestSuite) TestProcessIndexes() {
     tests := []struct {
         name      string
         dbIndexes []schema.DBIndex
         expected  []schema.Index
     }{
+        {
+            name: "DuplicateIndexNames",
+            dbIndexes: []schema.DBIndex{
+                {Name: "idx_test", Columns: "col1", Type: "BTREE", Primary: false, Unique: true},
+                {Name: "idx_test", Columns: "col2", Type: "BTREE", Primary: false, Unique: true},
+            },
+            expected: []schema.Index{
+                {Name: "idx_test", Columns: []string{"col1"}, Type: "btree", Primary: false, Unique: true},
+                {Name: "idx_test_1", Columns: []string{"col2"}, Type: "btree", Primary: false, Unique: true},
+            },
+        },
+        {
+            name: "InvalidIndexType",
+            dbIndexes: []schema.DBIndex{
+                {Name: "test_index", Columns: "col1", Type: "INVALID", Primary: false, Unique: false},
+            },
+            expected: []schema.Index{
+                {Name: "test_index", Columns: []string{"col1"}, Type: "btree", Primary: false, Unique: false},
+            },
+        },
     }

gorm_test.go (2)

32-40: Extract test configuration to constants.

Hard-coded credentials and configuration values should be moved to constants for better maintainability.

+const (
+    testHost     = "localhost"
+    testDatabase = "goravel"
+    testUsername = "goravel"
+    testPassword = "Framework!123"
+    testSchema   = "public"
+    testTimezone = "UTC"
+)
+
 writes := []contracts.FullConfig{
     {
         Config: contracts.Config{
-            Host:     "localhost",
-            Database: "goravel",
-            Username: "goravel",
-            Password: "Framework!123",
-            Schema:   "public",
+            Host:     testHost,
+            Database: testDatabase,
+            Username: testUsername,
+            Password: testPassword,
+            Schema:   testSchema,
         },
-        Timezone: "UTC",
+        Timezone: testTimezone,
     },
 }

---

76-79: Extract pool configuration values to constants.

Magic numbers in pool configuration should be defined as constants with meaningful names.

+const (
+    defaultMaxIdleConns    = 10
+    defaultMaxOpenConns    = 100
+    defaultConnMaxIdleTime = 3600
+    defaultConnMaxLifetime = 3600
+)
+
-mockConfigFacade.EXPECT().GetInt("database.pool.max_idle_conns", 10).Return(10).Once()
-mockConfigFacade.EXPECT().GetInt("database.pool.max_open_conns", 100).Return(100).Once()
-mockConfigFacade.EXPECT().GetInt("database.pool.conn_max_idletime", 3600).Return(3600).Once()
-mockConfigFacade.EXPECT().GetInt("database.pool.conn_max_lifetime", 3600).Return(3600).Once()
+mockConfigFacade.EXPECT().GetInt("database.pool.max_idle_conns", defaultMaxIdleConns).Return(defaultMaxIdleConns).Once()
+mockConfigFacade.EXPECT().GetInt("database.pool.max_open_conns", defaultMaxOpenConns).Return(defaultMaxOpenConns).Once()
+mockConfigFacade.EXPECT().GetInt("database.pool.conn_max_idletime", defaultConnMaxIdleTime).Return(defaultConnMaxIdleTime).Once()
+mockConfigFacade.EXPECT().GetInt("database.pool.conn_max_lifetime", defaultConnMaxLifetime).Return(defaultConnMaxLifetime).Once()

.mockery.yaml (1)

1-11: Fix YAML formatting issues.

The file has two minor formatting issues:

1. Missing newline at end of file
2. Trailing spaces on line 11

Apply these changes:

 with-expecter: True
 disable-version-string: True
 all: True
 recursive: true
 packages:
   github.com/goravel/postgres/contracts:
     config:
       dir: mocks/{{replaceAll .InterfaceDirRelative "contracts" ""}}
       filename: "{{.InterfaceName}}.go"
-      mockname: "{{.InterfaceName}}"
-      
+      mockname: "{{.InterfaceName}}"

🧰 Tools

🪛 yamllint (1.35.1)

[error] 11-11: no new line character at the end of file

(new-line-at-end-of-file)

---

[error] 11-11: trailing spaces

(trailing-spaces)

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