Skip to content

Feat/ren/72 custom name ref repository #75

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

Merged
ren0503 merged 6 commits into from
Aug 31, 2025
Merged

Feat/ren/72 custom name ref repository #75

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
483c438
feat: add custom function name to ref repository
ren0503 Aug 30, 2025
9ac6d93
feat: update test case
ren0503 Aug 30, 2025
5d1cc6d
feat: update inject repo
ren0503 Aug 30, 2025
c32a6e7
feat: update reflect
ren0503 Aug 30, 2025
219a36b
ci: update
ren0503 Aug 30, 2025
7195bdf
feat: update test case
ren0503 Aug 31, 2025
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
16 changes: 0 additions & 16 deletions .github/workflows/go.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,27 +33,11 @@ jobs:
- name: Check out repository code
uses: actions/checkout@v4

- name: Decide if tests should run
id: set_run_tests
run: |
if [[ ("$GITHUB_EVENT_NAME" == "push" && ( "$GITHUB_HEAD_COMMIT_MESSAGE" == fix:* || "$GITHUB_HEAD_COMMIT_MESSAGE" == feat:* || "$GITHUB_HEAD_COMMIT_MESSAGE" == refactor:* || "$GITHUB_HEAD_COMMIT_MESSAGE" == perf:* )) || \
("$GITHUB_EVENT_NAME" == "pull_request" && ( "$GITHUB_PULL_REQUEST_TITLE" == fix:* || "$GITHUB_PULL_REQUEST_TITLE" == feat:* || "$GITHUB_PULL_REQUEST_TITLE" == refactor:* || "$GITHUB_PULL_REQUEST_TITLE" == perf:* )) ]]; then
echo "should_run_tests=true" >> $GITHUB_OUTPUT
else
echo "should_run_tests=false" >> $GITHUB_OUTPUT
fi
env:
GITHUB_HEAD_COMMIT_MESSAGE: ${{ github.event.head_commit.message }}
GITHUB_PULL_REQUEST_TITLE: ${{ github.event.pull_request.title }}
GITHUB_EVENT_NAME: ${{ github.event_name }}

- name: Test with coverage
if: steps.set_run_tests.outputs.should_run_tests == 'true'
run: |
go test -cover -coverprofile=coverage.txt ./...

- name: Upload coverage reports to Codecov
if: steps.set_run_tests.outputs.should_run_tests == 'true'
uses: codecov/codecov-action@v5
with:
token: ${{ secrets.CODECOV_TOKEN }}
Expand Down
12 changes: 11 additions & 1 deletion module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package sqlorm
import (
"fmt"
"time"
"reflect"

"github.com/tinh-tinh/tinhtinh/v2/common"
"github.com/tinh-tinh/tinhtinh/v2/core"
Expand Down Expand Up @@ -68,7 +69,16 @@ func Inject(ref core.RefProvider) *gorm.DB {

func InjectRepository[M any](ref core.RefProvider) *Repository[M] {
var model M
modelName := core.Provide(fmt.Sprintf("%sRepo", common.GetStructName(model)))

ctModel := reflect.ValueOf(&model).Elem()
fnc := ctModel.MethodByName("RepositoryName")
var name string
if fnc.IsValid() {
name = fnc.Call(nil)[0].String()
} else {
name = common.GetStructName(model)
}
modelName := core.Provide(GetRepoName(name))
Comment on lines +72 to +81
Copy link

@coderabbitai coderabbitai bot Aug 31, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Support pointer receivers and make reflection safe to avoid panics.

RepositoryName implemented with a pointer receiver (e.g., func (T* ) RepositoryName() string)) won’t be discovered, and fnc.Call(nil)[0] can panic if the signature differs. Handle both value/pointer receivers and validate the method signature before calling.

Apply:

-	
-	ctModel := reflect.ValueOf(&model).Elem()
-	fnc := ctModel.MethodByName("RepositoryName")
-	var name string
-	if fnc.IsValid() {
-		name = fnc.Call(nil)[0].String()
-	} else {
-		name = common.GetStructName(model)
-	}
-	modelName := core.Provide(GetRepoName(name))
+	var name string
+	ctModel := reflect.ValueOf(&model).Elem()
+	// Try value receiver first
+	if m := ctModel.MethodByName("RepositoryName"); m.IsValid() &&
+		m.Type().NumIn() == 0 && m.Type().NumOut() >= 1 && m.Type().Out(0).Kind() == reflect.String {
+		name = m.Call(nil)[0].String()
+	} else if m := ctModel.Addr().MethodByName("RepositoryName"); m.IsValid() &&
+		m.Type().NumIn() == 0 && m.Type().NumOut() >= 1 && m.Type().Out(0).Kind() == reflect.String {
+		name = m.Call(nil)[0].String()
+	} else {
+		name = common.GetStructName(model)
+	}
+	modelName := GetRepoName(name)

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In module.go around lines 72 to 81, the reflection logic currently misses
methods with pointer receivers and can panic when calling methods with
unexpected signatures; update it to first obtain both the value and pointer
reflect.Values (ensure the value is addressable: v := reflect.ValueOf(model); if
v.Kind()==ptr use v.Elem(); pv := v; if !pv.CanAddr() {pv =
reflect.New(v.Type()) ; pv.Elem().Set(v)} then check for RepositoryName on pv
(pointer) and v (value) in that order), verify the method IsValid and that
method.Type().NumIn() == 0 and method.Type().NumOut() == 1 and
method.Type().Out(0).Kind() == reflect.String before calling, and only call when
those conditions hold; otherwise fall back to common.GetStructName(model) to
avoid panics.

data, ok := ref.Ref(modelName).(*Repository[M])
if !ok {
return nil
Expand Down
23 changes: 23 additions & 0 deletions module_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,29 @@ func Test_Nil(t *testing.T) {
require.Nil(t, sqlorm.InjectRepository[User](core.NewModule(core.NewModuleOptions{})))
}

func Test_RefName(t *testing.T) {
require.NotPanics(t, func() {
createDatabaseForTest("test")
})
dsn := "host=localhost user=postgres password=postgres dbname=test port=5432 sslmode=disable TimeZone=Asia/Shanghai"

appModule := func() core.Module {
module := core.NewModule(core.NewModuleOptions{
Imports: []core.Modules{
sqlorm.ForRoot(sqlorm.Config{
Dialect: postgres.Open(dsn),
Models: []interface{}{&Abc{}},
}),
sqlorm.ForFeature(sqlorm.NewRepo(Abc{})),
},
})

return module
}
abcRepo := sqlorm.InjectRepository[Abc](appModule())
require.NotNil(t, abcRepo)
}

func createDatabaseForTest(dbName string) {
connStr := "host=localhost user=postgres password=postgres port=5432 dbname=postgres sslmode=disable TimeZone=Asia/Shanghai"

Expand Down
11 changes: 10 additions & 1 deletion repository.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,16 @@ type Repository[M any] struct {

func (r *Repository[M]) GetName() string {
var model M
return common.GetStructName(model)

ctModel := reflect.ValueOf(&model).Elem()
fnc := ctModel.MethodByName("RepositoryName")
var name string
if fnc.IsValid() {
name = fnc.Call(nil)[0].String()
} else {
name = common.GetStructName(model)
}
return name
Comment on lines +26 to +35
Copy link

@coderabbitai coderabbitai bot Aug 30, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Support pointer receivers and remove panic risks by replacing reflection with a simple interface check

Current code misses methods with pointer receivers and can panic if the method’s return type isn’t string. Prefer a type assertion that handles both value and pointer receivers and falls back cleanly.

Apply this diff:

 func (r *Repository[M]) GetName() string {
   var model M
-  
-  ctModel := reflect.ValueOf(&model).Elem()
-  fnc := ctModel.MethodByName("RepositoryName")
-  var name string
-  if fnc.IsValid() {
-    name = fnc.Call(nil)[0].String()
-  } else {
-    name = common.GetStructName(model)
-  }
-  return name
+  // Prefer interface assertion over reflection; supports both value and pointer receivers.
+  type repositoryNamer interface{ RepositoryName() string }
+  if rn, ok := any(model).(repositoryNamer); ok {
+    if n := rn.RepositoryName(); n != "" {
+      return n
+    }
+  }
+  if rn, ok := any(&model).(repositoryNamer); ok {
+    if n := rn.RepositoryName(); n != "" {
+      return n
+    }
+  }
+  return common.GetStructName(model)
 }

If you prefer to keep reflection, minimally switch to pointer first and validate signature:

- ctModel := reflect.ValueOf(&model).Elem()
- fnc := ctModel.MethodByName("RepositoryName")
+ ptr := reflect.ValueOf(&model)
+ fnc := ptr.MethodByName("RepositoryName")
+ if !fnc.IsValid() {
+   fnc = ptr.Elem().MethodByName("RepositoryName")
+ }
- if fnc.IsValid() {
-   name = fnc.Call(nil)[0].String()
+ if fnc.IsValid() && fnc.Type().NumIn() == 0 && fnc.Type().NumOut() >= 1 && fnc.Type().Out(0).Kind() == reflect.String {
+   out := fnc.Call(nil)
+   if len(out) > 0 {
+     s := out[0].String()
+     if s != "" {
+       return s
+     }
+   }
   } else {
-   name = common.GetStructName(model)
+   return common.GetStructName(model)
   }
- return name
+ return common.GetStructName(model)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ctModel := reflect.ValueOf(&model).Elem()
fnc := ctModel.MethodByName("RepositoryName")
var name string
if fnc.IsValid() {
name = fnc.Call(nil)[0].String()
} else {
name = common.GetStructName(model)
}
return name
func (r *Repository[M]) GetName() string {
var model M
// Prefer interface assertion over reflection; supports both value and pointer receivers.
type repositoryNamer interface{ RepositoryName() string }
if rn, ok := any(model).(repositoryNamer); ok {
if n := rn.RepositoryName(); n != "" {
return n
}
}
if rn, ok := any(&model).(repositoryNamer); ok {
if n := rn.RepositoryName(); n != "" {
return n
}
}
return common.GetStructName(model)
}
🤖 Prompt for AI Agents 
In repository.go around lines 26 to 35, the current reflection-based logic
misses methods with pointer receivers and may panic if the method signature
isn't string; replace it by defining a small interface type (e.g.
RepositoryNamer with RepositoryName() string) and perform a type assertion:
first try asserting the concrete model value, then try asserting its pointer
(address) if available; if either assertion succeeds, call RepositoryName() and
use the returned string, otherwise fall back to common.GetStructName(model).
This avoids reflection, supports pointer receivers, and removes the panic risk
by only calling the method after a successful interface assertion.

}

func (r *Repository[M]) SetDB(db *gorm.DB) {
Expand Down
14 changes: 14 additions & 0 deletions repository_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,17 @@ func Test_Map(t *testing.T) {
list4 := sqlorm.MapMany[Todo](nil)
require.Equal(t, 0, len(list4))
}

type Abc struct {
sqlorm.Model `gorm:"embedded"`
Name string `gorm:"type:varchar(255);not null"`
}

func (Abc) RepositoryName() string {
return "service"
}

func TestNameRepo(t *testing.T) {
repo := sqlorm.NewRepo(Abc{})
require.Equal(t, "service", repo.GetName())
}