This is an automated email from the ASF dual-hosted git repository.
Alanxtl pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/dubbo-go.git
The following commit(s) were added to refs/heads/develop by this push:
new 9757e7735 docs(router): complete script router comments and unit test
coverage (#3682)
9757e7735 is described below
commit 9757e7735d720e4f3bba348f8c4f94803b5fb61d
Author: MaoMeng <[email protected]>
AuthorDate: Wed Aug 19 14:25:54 2026 +0800
docs(router): complete script router comments and unit test coverage (#3682)
* docs(router): complete script router comments and unit test coverage
* test(router): fix CI failures in script router tests
---
cluster/router/script/instance/instances_pool.go | 10 +++
cluster/router/script/instance/js_instance.go | 16 ++++
cluster/router/script/instance/js_instance_test.go | 93 ++++++++++++++++++++++
cluster/router/script/router.go | 16 +++-
cluster/router/script/router_test.go | 35 ++++++++
5 files changed, 168 insertions(+), 2 deletions(-)
diff --git a/cluster/router/script/instance/instances_pool.go
b/cluster/router/script/instance/instances_pool.go
index 891668dc8..bf6216e3d 100644
--- a/cluster/router/script/instance/instances_pool.go
+++ b/cluster/router/script/instance/instances_pool.go
@@ -34,6 +34,14 @@ func init() {
setInstances(`javascript`, newJsInstances())
}
+// ScriptInstances is the engine bound to one script type. It owns the whole
+// lifecycle of scripts of that type: compile, run and destroy.
+// Compile compiles the script and increments its reference count; compiling
+// the same script again only increments the count.
+// Run executes a compiled script; Run returns the original invokers for a
+// script that was never compiled or has been destroyed.
+// Destroy decrements the reference count of the compiled script; the entry
+// is removed from the cache once the count drops to zero.
type ScriptInstances interface {
Run(rawScript string, invokers []base.Invoker, invocation
base.Invocation) ([]base.Invoker, error)
Compile(rawScript string) error
@@ -42,6 +50,7 @@ type ScriptInstances interface {
var factory map[string]ScriptInstances
+// GetInstances returns the engine instance for the given script type.
func GetInstances(scriptType string) (ScriptInstances, error) {
ins, ok := factory[strings.ToLower(scriptType)]
if !ok {
@@ -58,6 +67,7 @@ func RangeInstances(f func(instance ScriptInstances) bool) {
}
}
+// setInstances sets the engine instance for the given script type.
func setInstances(tpName string, instance ScriptInstances) {
factory[tpName] = instance
}
diff --git a/cluster/router/script/instance/js_instance.go
b/cluster/router/script/instance/js_instance.go
index 58e51693b..db319c32f 100644
--- a/cluster/router/script/instance/js_instance.go
+++ b/cluster/router/script/instance/js_instance.go
@@ -38,21 +38,25 @@ const (
jsScriptPrefix = "\n" + jsScriptResultName + ` = `
)
+// jsInstances is the JavaScript engine.
type jsInstances struct {
insPool *sync.Pool // store *goja.runtime
pgLock sync.RWMutex
program map[string]*program // rawScript to compiledProgram
}
+// jsInstance is one goja runtime.
type jsInstance struct {
rt *goja.Runtime
}
+// program is the compiled JavaScript script with its reference count.
type program struct {
pg *goja.Program
count int32
}
+// newProgram creates a new program.
func newProgram(pg *goja.Program) *program {
return &program{
pg: pg,
@@ -64,6 +68,7 @@ func (p *program) addCount(i int) int {
return int(atomic.AddInt32(&p.count, int32(i)))
}
+// newJsInstances creates a new jsInstances.
func newJsInstances() *jsInstances {
return &jsInstances{
program: map[string]*program{},
@@ -73,6 +78,9 @@ func newJsInstances() *jsInstances {
}
}
+// Run runs the JavaScript script. When the script is not compiled
+// or invokers is empty, it returns the original invokers directly.
+// When an error occurs, it returns the original invokers and the error.
func (i *jsInstances) Run(rawScript string, invokers []base.Invoker,
invocation base.Invocation) ([]base.Invoker, error) {
i.pgLock.RLock()
pg, ok := i.program[rawScript]
@@ -114,6 +122,10 @@ func (i *jsInstances) Run(rawScript string, invokers
[]base.Invoker, invocation
return result, nil
}
+// Compile compiles the JavaScript script. When the script is not
+// compiled, it compiles the script and increments the reference count.
+// When the script is already compiled, it only increments the reference
+// count, reusing the compiled script.
func (i *jsInstances) Compile(rawScript string) error {
var (
ok bool
@@ -145,6 +157,9 @@ func (i *jsInstances) Compile(rawScript string) error {
}
}
+// Destroy destroys the JavaScript script. When the script reference
+// count is greater than 0, it only decrements the reference count.
+// When the script reference count reaches 0, it deletes the script.
func (i *jsInstances) Destroy(rawScript string) {
i.pgLock.Lock()
if pg, ok := i.program[rawScript]; ok {
@@ -179,6 +194,7 @@ func (j jsInstance) initReplyVar() {
}
}
+// newJsInstance creates a new jsInstance.
func newJsInstance() *jsInstance {
return &jsInstance{
rt: goja.New(),
diff --git a/cluster/router/script/instance/js_instance_test.go
b/cluster/router/script/instance/js_instance_test.go
index 2ea812fa8..eccc80f5e 100644
--- a/cluster/router/script/instance/js_instance_test.go
+++ b/cluster/router/script/instance/js_instance_test.go
@@ -513,6 +513,38 @@ func TestFuncWithCompileConcurrent(t *testing.T) {
wg.Wait()
}
+func TestCompileConcurrentSingleProgram(t *testing.T) {
+ ins := newJsInstances()
+ const script = `(function route(i,v,c){ return [invokers[1],
invokers[2]]; }(invokers,invocation,context));`
+ const n = 20
+ var wg sync.WaitGroup
+ for range n {
+ wg.Go(func() { ; assert.NoError(t, ins.Compile(script)) })
+ }
+ wg.Wait()
+ assert.Len(t, ins.program, 1)
+ assert.EqualValues(t, n, ins.program[script].count)
+}
+
+func TestRunConcurrentWithRace(t *testing.T) {
+ globalIns, err := GetInstances("javascript")
+ assert.NoError(t, err)
+ const script = `(function route(i,v,c){ return [invokers[1],
invokers[2]]; }(invokers,invocation,context));`
+ assert.NoError(t, globalIns.Compile(script))
+ var wg sync.WaitGroup
+ for range 30 {
+ wg.Go(func() {
+ ins, err := GetInstances("javascript")
+ testify_require.NoError(t, err)
+ invokers, inv, _ := getRouteArgs()
+ got, err := ins.Run(script, invokers, inv)
+ testify_require.NoError(t, err)
+ assert.Len(t, got, 2)
+ })
+ }
+ wg.Wait()
+}
+
func TestFuncWithCompileAndRunRepeatedly(t *testing.T) {
pg, err := goja.Compile("routeJs", jsScriptPrefix+`(
function route(invokers,invocation,context) {
@@ -549,6 +581,44 @@ function route(invokers,invocation,context) {
}
}
+func TestJsInstancesRunFailurePaths(t *testing.T) {
+ const throwScript = `(function route(i,v,c){ throw new Error("boom");
}(invokers,invocation,context));`
+ const nonArrayScript = `(function route(i,v,c){ return {};
}(invokers,invocation,context));`
+ const badElemScript = `(function route(i,v,c){ var r=[]; r.push(1);
return r; }(invokers,invocation,context));`
+
+ t.Run("run without compile", func(t *testing.T) {
+ ins := newJsInstances()
+ invokers, inv, _ := getRouteArgs()
+ got, err := ins.Run(throwScript, invokers, inv)
+ testify_require.NoError(t, err)
+ assert.Equal(t, invokers, got)
+ })
+ t.Run("script throws", func(t *testing.T) {
+ ins := newJsInstances()
+ testify_require.NoError(t, ins.Compile(throwScript))
+ invokers, inv, _ := getRouteArgs()
+ got, err := ins.Run(throwScript, invokers, inv)
+ testify_require.Error(t, err)
+ assert.Equal(t, invokers, got)
+ })
+ t.Run("script returns non-array", func(t *testing.T) {
+ ins := newJsInstances()
+ testify_require.NoError(t, ins.Compile(nonArrayScript))
+ invokers, inv, _ := getRouteArgs()
+ got, err := ins.Run(nonArrayScript, invokers, inv)
+ testify_require.Error(t, err)
+ assert.Equal(t, invokers, got)
+ })
+ t.Run("script returns array with invalid element", func(t *testing.T) {
+ ins := newJsInstances()
+ testify_require.NoError(t, ins.Compile(badElemScript))
+ invokers, inv, _ := getRouteArgs()
+ got, err := ins.Run(badElemScript, invokers, inv)
+ testify_require.Error(t, err)
+ assert.Equal(t, invokers, got)
+ })
+}
+
func setRunScriptEnv() *goja.Runtime {
runtime := goja.New()
rt_link_external_libraries(runtime)
@@ -579,6 +649,29 @@ func setRunScriptEnv() *goja.Runtime {
return runtime
}
+func TestProgramRefcountLifecycle(t *testing.T) {
+ ins := newJsInstances()
+ const script = `(function route(i,v,c){ return [invokers[1],
invokers[2]]; }(invokers,invocation,context));`
+
+ assert.NoError(t, ins.Compile(script))
+ assert.NoError(t, ins.Compile(script))
+ assert.Len(t, ins.program, 1)
+ assert.EqualValues(t, 2, ins.program[script].count)
+
+ invokers, inv, _ := getRouteArgs()
+ ins.Destroy(script)
+ assert.Len(t, ins.program, 1)
+ got, err := ins.Run(script, invokers, inv)
+ testify_require.NoError(t, err)
+ assert.Len(t, got, 2)
+
+ ins.Destroy(script)
+ assert.Empty(t, ins.program)
+ got, err = ins.Run(script, invokers, inv)
+ testify_require.NoError(t, err)
+ assert.Len(t, got, 3)
+}
+
func TestRunScriptInPanic(t *testing.T) {
willPanic := func(errScript string) {
rt := setRunScriptEnv()
diff --git a/cluster/router/script/router.go b/cluster/router/script/router.go
index 44ef8531d..9e73de792 100644
--- a/cluster/router/script/router.go
+++ b/cluster/router/script/router.go
@@ -49,6 +49,7 @@ type ScriptRouter struct {
rawScript string
}
+// NewScriptRouter creates a new ScriptRouter.
func NewScriptRouter() *ScriptRouter {
return &ScriptRouter{
applicationName: "",
@@ -56,6 +57,7 @@ func NewScriptRouter() *ScriptRouter {
}
}
+// parseRoute decodes a YAML script rule into a RouterConfig.
func parseRoute(routeContent string) (*global.RouterConfig, error) {
routeDecoder := yaml.NewDecoder(strings.NewReader(routeContent))
routerConfig := &global.RouterConfig{}
@@ -66,6 +68,10 @@ func parseRoute(routeContent string) (*global.RouterConfig,
error) {
return routerConfig, nil
}
+// Process receives a script rule change event and mutates the router state
+// accordingly. On a Del event it resets the router to the disabled, empty
+// state. On an Add or Update event it destroys the old instance first, then
+// compiles the new instance.
func (s *ScriptRouter) Process(event *config_center.ConfigChangeEvent) {
s.mu.Lock()
defer s.mu.Unlock()
@@ -117,11 +123,11 @@ func (s *ScriptRouter) Process(event
*config_center.ConfigChangeEvent) {
logger.Error("[Router][Script] applicationName field
must be set in config")
return
}
- if !*cfg.Enabled {
+ if cfg.Enabled != nil && !*cfg.Enabled {
logger.Infof("[Router][Script] enabled field equals
false, this rule will be ignored, script=%s", cfg.Script)
}
// rewrite to ScriptRouter
- s.enabled = *cfg.Enabled
+ s.enabled = cfg.Enabled == nil || *cfg.Enabled
s.rawScript = cfg.Script
s.scriptType = cfg.ScriptType
@@ -144,6 +150,7 @@ func (s *ScriptRouter) Process(event
*config_center.ConfigChangeEvent) {
}
}
+// runScript executes rawScript through the engine registered for scriptType.
func (s *ScriptRouter) runScript(scriptType, rawScript string, invokers
[]base.Invoker, invocation base.Invocation) ([]base.Invoker, error) {
in, err := ins.GetInstances(scriptType)
if err != nil {
@@ -152,6 +159,7 @@ func (s *ScriptRouter) runScript(scriptType, rawScript
string, invokers []base.I
return in.Run(rawScript, invokers, invocation)
}
+// Route determines the target invokers by executing the enabled script.
func (s *ScriptRouter) Route(invokers []base.Invoker, _ *common.URL,
invocation base.Invocation) []base.Invoker {
if len(invokers) == 0 {
return []base.Invoker{}
@@ -173,14 +181,18 @@ func (s *ScriptRouter) Route(invokers []base.Invoker, _
*common.URL, invocation
return res
}
+// URL always returns nil.
func (s *ScriptRouter) URL() *common.URL {
return nil
}
+// Priority always returns 0.
func (s *ScriptRouter) Priority() int64 {
return 0
}
+// Notify subscribes this router to the script rule of the provider
+// application in the invoker list.
func (s *ScriptRouter) Notify(invokers []base.Invoker) {
if len(invokers) == 0 {
return
diff --git a/cluster/router/script/router_test.go
b/cluster/router/script/router_test.go
index e9b04c13e..75afb1471 100644
--- a/cluster/router/script/router_test.go
+++ b/cluster/router/script/router_test.go
@@ -242,6 +242,41 @@ func TestScriptRouterProcessSkipsNonStringConfig(t
*testing.T) {
assert.Equal(t, "old script", s.rawScript)
}
+func TestProcessInvalidRuleDisablesRouter(t *testing.T) {
+ cases := []struct{ name, cfg string }{
+ {"invalid yaml", ":\n\tnot yaml : ["},
+ {"missing key", "configVersion: v3.0\ntype:
javascript\nenabled: true\nscript: |-\n 1"},
+ {"missing type", "configVersion: v3.0\nkey: dubbo.io\nenabled:
true\nscript: |-\n 1"},
+ {"missing script", "configVersion: v3.0\nkey: dubbo.io\ntype:
javascript\nenabled: true"},
+ }
+ for _, c := range cases {
+ t.Run(c.name, func(t *testing.T) {
+ s := NewScriptRouter()
+ s.Process(&config_center.ConfigChangeEvent{Value:
c.cfg, ConfigType: remoting.EventTypeUpdate})
+ assert.False(t, s.enabled)
+ invokers, inv, _ := getRouteCheckArgs()
+ assert.True(t, checkInvokersSame(s.Route(invokers, nil,
inv), invokers))
+ })
+ }
+}
+
+func TestProcessMissingEnabledDefaultsTrue(t *testing.T) {
+ s := NewScriptRouter()
+ assert.NotPanics(t, func() {
+ s.Process(&config_center.ConfigChangeEvent{
+ Value: `configVersion: v3.0
+key: dubbo.io
+type: javascript
+script: |-
+ (function(){return invokers;}(invokers,invocation,context));`,
+ ConfigType: remoting.EventTypeUpdate,
+ })
+ })
+ assert.True(t, s.enabled)
+ invokers, inv, _ := getRouteCheckArgs()
+ assert.Len(t, s.Route(invokers, nil, inv), 3)
+}
+
func TestScriptRouterProcessDelSkipsConfigBody(t *testing.T) {
s := &ScriptRouter{
enabled: true,
