fix: expose API of router (#3045) #3065
Conversation
|
we use https://github.com/dubbogo/tools?tab=readme-ov-file#imports-formatter to format import blocks, for u, you should download the tool and |
yes，sir |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3065 +/- ##
==========================================
+ Coverage 39.64% 40.22% +0.57%
==========================================
Files 457 458 +1
Lines 39075 32635 -6440
==========================================
- Hits 15493 13127 -2366
+ Misses 22318 18241 -4077
- Partials 1264 1267 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds router configuration support to the Dubbo-Go framework by introducing RouterConfig in the global package and integrating it into InstanceOptions. The feature was previously commented out and is now being enabled.
- Uncommented and activated the Router field in InstanceOptions
- Created new global/router_config.go with RouterConfig and Tag structs
- Added compatibility layer functions to convert between global and config package types
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| options.go | Uncommented Router field, added default initialization and CloneRouter method |
| global/router_config.go | New file defining RouterConfig and Tag structs with Clone method |
| compat.go | Added compatibility functions for bidirectional conversion between global and config RouterConfig types |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| copy(newConditions, c.Conditions) | ||
|
|
||
| newTags := make([]Tag, len(c.Tags)) | ||
| copy(newTags, c.Tags) |
There was a problem hiding this comment.
Shallow copy of Tags slice creates shared references to ParamMatch pointers. The Tag struct contains a Match field of type []*common.ParamMatch, which means the pointers will be shared between the original and cloned objects. This should perform a deep copy by iterating through each Tag and cloning its Match slice. Example fix:
newTags := make([]Tag, len(c.Tags))
for i, tag := range c.Tags {
newMatch := make([]*common.ParamMatch, len(tag.Match))
for j, match := range tag.Match {
if match != nil {
matchCopy := *match
newMatch[j] = &matchCopy
}
}
newAddresses := make([]string, len(tag.Addresses))
copy(newAddresses, tag.Addresses)
newTags[i] = Tag{
Name: tag.Name,
Match: newMatch,
Addresses: newAddresses,
}
}| copy(newTags, c.Tags) | |
| for i, tag := range c.Tags { | |
| // Deep copy Match slice | |
| newMatch := make([]*common.ParamMatch, len(tag.Match)) | |
| for j, match := range tag.Match { | |
| if match != nil { | |
| matchCopy := *match | |
| newMatch[j] = &matchCopy | |
| } | |
| } | |
| // Deep copy Addresses slice | |
| newAddresses := make([]string, len(tag.Addresses)) | |
| copy(newAddresses, tag.Addresses) | |
| newTags[i] = Tag{ | |
| Name: tag.Name, | |
| Match: newMatch, | |
| Addresses: newAddresses, | |
| } | |
| } |
| ScriptType: c.ScriptType, | ||
| Script: c.Script, | ||
| } | ||
|
|
There was a problem hiding this comment.
Extra blank line before closing brace. Remove the blank line at line 463 to maintain consistency with the codebase style (see compatShutdownConfig at line 443 and other similar functions).
I agree with this statement, and after you've added the imported API, I hope you'll add examples to dubbo-go-samples for verification.Additionally, submit to the develop branch |
Co-authored-by: Xuetao Li <[email protected]>
Co-authored-by: Xuetao Li <[email protected]>
|
|
4 participants
#3045 primarily for exposing the API of Router, a global router_config was added. Then, the initialization and cloning of instances were supplemented in options.go, and methods for mutual conversion between global and config were also added in compat.go