feat: optimize refresh method #956
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR optimizes the refresh method to support refreshing specific instances or all refreshable modules (except the config binding) when needed. Key changes include updating the Refresh method signatures in the container, mocks, and contracts, adjusting test cases for the new behavior, and a minor behavioral change in the HTTP throttle middleware.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| foundation/container_test.go | Updated tests to verify loading of bindings using the new key "ok" |
| foundation/container.go | Modified Refresh method to accept variadic parameters and skip config |
| mocks/foundation/Application.go | Adjusted mock signatures and variadic argument handling for Refresh |
| contracts/foundation/application.go | Updated Refresh interface to match new signature |
| database/orm/orm.go | Updated refresh function signature for consistency |
| http/middleware/throttle.go | Changed loop break to early return in throttle middleware logic |
Comments suppressed due to low confidence (2)
foundation/container.go:324
- [nitpick] The use of the parameter name 'bindings' here might be confusing as it is used to specify keys for instance deletion. Consider renaming it to 'keys' or 'refreshKeys' for improved clarity.
func (c *Container) Refresh(bindings ...any) {
mocks/foundation/Application.go:2143
- [nitpick] Consider simplifying the conversion of 'args' into a []interface{} (if supported by the mock library) to make the code more concise and readable.
for i, a := range args[0:] {
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: cc927ef | Previous: ab139f2 | Ratio |
|---|---|---|---|
Benchmark_EncryptString |
6951 ns/op 2160 B/op 15 allocs/op |
2299 ns/op 2160 B/op 15 allocs/op |
3.02 |
Benchmark_EncryptString - ns/op |
6951 ns/op |
2299 ns/op |
3.02 |
Benchmark_DecryptString |
6674 ns/op 2040 B/op 17 allocs/op |
2100 ns/op 2040 B/op 17 allocs/op |
3.18 |
Benchmark_DecryptString - ns/op |
6674 ns/op |
2100 ns/op |
3.18 |
This comment was automatically generated by workflow using github-action-benchmark.
| // configInstance, ok := c.instances.Load(contracts.BindingConfig) | ||
|
|
||
| c.instances.Range(func(key, value any) bool { | ||
| if key != contracts.BindingConfig { |
There was a problem hiding this comment.
Shouldn't delete the base service provider.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v1.15.x #956 +/- ##
==========================================
Coverage ? 68.61%
==========================================
Files ? 219
Lines ? 18967
Branches ? 0
==========================================
Hits ? 13015
Misses ? 5285
Partials ? 667 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
📑 Description
We need to refresh all modules when testing in goravel/example with different drivers.
✅ Checks