From a46c46657cdb5c914c34d421dc60455de32dd162 Mon Sep 17 00:00:00 2001 From: hyorigo <67533928+hyorigo@users.noreply.github.com> Date: Fri, 21 Jul 2023 16:50:39 +0800 Subject: [PATCH] [feat] Disable Input/Output Starlight Conversion (#38) * Strip * For input conversion * Hack for convert * For starlight 5 * For cmd update * For conv * Adjust conv name * For better perf * convert -> cast * For internal lib testing * Set conversion enabled * Set enabled * Test conv itself * X * one converter * For testing convert * For exported --- cmd/starlet/cgi.star | 2 +- cmd/starlet/go.mod | 2 +- cmd/starlet/go.sum | 8 +-- go.mod | 4 +- go.sum | 8 +-- internal_test.go | 100 ++++++++++++++++++++++++++++ machine.go | 60 ++++++++++++----- machine_test.go | 153 +++++++++++++++++++++++++++++++++++++++++++ run.go | 51 ++++++++++++++- 9 files changed, 355 insertions(+), 33 deletions(-) create mode 100644 internal_test.go diff --git a/cmd/starlet/cgi.star b/cmd/starlet/cgi.star index e4b9a2fc..fb1c8967 100644 --- a/cmd/starlet/cgi.star +++ b/cmd/starlet/cgi.star @@ -12,6 +12,6 @@ text = '''

This is a simple CGI script written in Starlark.

-'''.format(now, json.dumps(reader.Header, indent=2)) +'''.format(now, json.dumps(reader.Header, indent=2)).strip() writer.Write(text) diff --git a/cmd/starlet/go.mod b/cmd/starlet/go.mod index 635561fd..ba6ea45d 100644 --- a/cmd/starlet/go.mod +++ b/cmd/starlet/go.mod @@ -8,7 +8,7 @@ require ( github.com/1set/gut v0.0.0-20201117175203-a82363231997 github.com/1set/starlet v0.0.2 github.com/spf13/pflag v1.0.5 - go.starlark.net v0.0.0-20230612165344-9532f5667272 + go.starlark.net v0.0.0-20230718153141-1c3ac63bd217 golang.org/x/term v0.0.0-20220526004731-065cf7ba2467 ) diff --git a/cmd/starlet/go.sum b/cmd/starlet/go.sum index c9ae4a75..4d7a7b7a 100644 --- a/cmd/starlet/go.sum +++ b/cmd/starlet/go.sum @@ -5,8 +5,8 @@ bitbucket.org/neiku/winornot v0.0.4/go.mod h1:xzKlwxkpGIumHbxwUrB41kFIra8gz/ZZgJ cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= github.com/1set/gut v0.0.0-20201117175203-a82363231997 h1:za2jSkE1Rx56hTzBko3ZZ4gA/nq+rA/jVovWuAF4jyo= github.com/1set/gut v0.0.0-20201117175203-a82363231997/go.mod h1:DpCCAL0dgBMQdiqPUIIRpdU9zNcIZwJjW+L/8Mb30mw= -github.com/1set/starlight v0.0.4 h1:f0eYs+1ZrLRrqyD1mtER/9nRyNZFef+b/tXcGvG+tg8= -github.com/1set/starlight v0.0.4/go.mod h1:xwY+rrC/P9U2qj6ADrwKgqPaVlvcKl3PE3u1zJVYunE= +github.com/1set/starlight v0.0.5 h1:FfYaF1EIa+WixfrmYbUb3nolwbIcpbS4FamhYhQr+Oc= +github.com/1set/starlight v0.0.5/go.mod h1:Aih2Bxp+TTSa2DPSGfDW51pPVHLKoroAAO225ktOkgM= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/aymanbagabas/go-osc52 v1.0.3 h1:DTwqENW7X9arYimJrPeGZcV0ln14sGMt3pHZspWD+Mg= github.com/aymanbagabas/go-osc52 v1.0.3/go.mod h1:zT8H+Rk4VSabYN90pWyugflM3ZhpTZNC7cASDfUCdT4= @@ -54,8 +54,8 @@ github.com/rodolfoag/gow32 v0.0.0-20160917004320-d95ff468acf8 h1:p7tJTb+Rqvp8dS8 github.com/rodolfoag/gow32 v0.0.0-20160917004320-d95ff468acf8/go.mod h1:w/ebPUfAcyZMYjstwPIWTEGSahChHx5R3Y+xElrvxDc= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= -go.starlark.net v0.0.0-20230612165344-9532f5667272 h1:2/wtqS591wZyD2OsClsVBKRPEvBsQt/Js+fsCiYhwu8= -go.starlark.net v0.0.0-20230612165344-9532f5667272/go.mod h1:jxU+3+j+71eXOW14274+SmmuW82qJzl6iZSeqEtTGds= +go.starlark.net v0.0.0-20230718153141-1c3ac63bd217 h1:PZww6GnYa6qc38W5SJu0iPhSCmW+ca3MuNO1FVe6aPw= +go.starlark.net v0.0.0-20230718153141-1c3ac63bd217/go.mod h1:jxU+3+j+71eXOW14274+SmmuW82qJzl6iZSeqEtTGds= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= diff --git a/go.mod b/go.mod index 849b501c..a011eac8 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,6 @@ module github.com/1set/starlet go 1.16 require ( - github.com/1set/starlight v0.0.4 - go.starlark.net v0.0.0-20230612165344-9532f5667272 + github.com/1set/starlight v0.0.5 + go.starlark.net v0.0.0-20230718153141-1c3ac63bd217 ) diff --git a/go.sum b/go.sum index 9c3ce14c..1ac203d2 100644 --- a/go.sum +++ b/go.sum @@ -1,6 +1,6 @@ cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= -github.com/1set/starlight v0.0.4 h1:f0eYs+1ZrLRrqyD1mtER/9nRyNZFef+b/tXcGvG+tg8= -github.com/1set/starlight v0.0.4/go.mod h1:xwY+rrC/P9U2qj6ADrwKgqPaVlvcKl3PE3u1zJVYunE= +github.com/1set/starlight v0.0.5 h1:FfYaF1EIa+WixfrmYbUb3nolwbIcpbS4FamhYhQr+Oc= +github.com/1set/starlight v0.0.5/go.mod h1:Aih2Bxp+TTSa2DPSGfDW51pPVHLKoroAAO225ktOkgM= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= github.com/chzyer/logex v1.1.10 h1:Swpa1K6QvQznwJRcfTfQJmTE72DqScAa40E+fbHEXEE= @@ -30,8 +30,8 @@ github.com/google/go-cmp v0.5.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/ github.com/google/go-cmp v0.5.1 h1:JFrFEBb2xKufg6XkJsJr+WbKb4FQlURi5RUcBveYu9k= github.com/google/go-cmp v0.5.1/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= -go.starlark.net v0.0.0-20230612165344-9532f5667272 h1:2/wtqS591wZyD2OsClsVBKRPEvBsQt/Js+fsCiYhwu8= -go.starlark.net v0.0.0-20230612165344-9532f5667272/go.mod h1:jxU+3+j+71eXOW14274+SmmuW82qJzl6iZSeqEtTGds= +go.starlark.net v0.0.0-20230718153141-1c3ac63bd217 h1:PZww6GnYa6qc38W5SJu0iPhSCmW+ca3MuNO1FVe6aPw= +go.starlark.net v0.0.0-20230718153141-1c3ac63bd217/go.mod h1:jxU+3+j+71eXOW14274+SmmuW82qJzl6iZSeqEtTGds= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= diff --git a/internal_test.go b/internal_test.go new file mode 100644 index 00000000..7ecfcb34 --- /dev/null +++ b/internal_test.go @@ -0,0 +1,100 @@ +package starlet + +import ( + "reflect" + "testing" + + "go.starlark.net/starlark" +) + +func TestCastStringDictToAnyMap(t *testing.T) { + // Create a starlark.StringDict + m := starlark.StringDict{ + "key1": starlark.String("value1"), + "key2": starlark.String("value2"), + "key3": starlark.NewList([]starlark.Value{starlark.String("value3")}), + } + + // Convert to StringAnyMap + anyMap := castStringDictToAnyMap(m) + + // Check that the conversion was successful + if anyMap["key1"].(starlark.String) != "value1" { + t.Errorf("Expected 'value1', got '%s'", anyMap["key1"].(starlark.String)) + } + if anyMap["key2"].(starlark.String) != "value2" { + t.Errorf("Expected 'value2', got '%s'", anyMap["key2"].(starlark.String)) + } + if !reflect.DeepEqual(anyMap["key3"], m["key3"]) { + t.Errorf("Expected '%v', got '%v'", m["key3"], anyMap["key3"]) + } +} + +func TestCastStringAnyMapToStringDict(t *testing.T) { + // Create a StringAnyMap + m := StringAnyMap{ + "key1": starlark.String("value1"), + "key2": starlark.String("value2"), + "key3": starlark.NewList([]starlark.Value{starlark.String("value3")}), + } + + // Convert to starlark.StringDict + stringDict, err := castStringAnyMapToStringDict(m) + + // Check that the conversion was successful + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if stringDict["key1"].(starlark.String) != "value1" { + t.Errorf("Expected 'value1', got '%s'", stringDict["key1"].(starlark.String)) + } + if stringDict["key2"].(starlark.String) != "value2" { + t.Errorf("Expected 'value2', got '%s'", stringDict["key2"].(starlark.String)) + } + if !reflect.DeepEqual(stringDict["key3"], m["key3"]) { + t.Errorf("Expected '%v', got '%v'", m["key3"], stringDict["key3"]) + } + + // Test with a non-starlark.Value in the map + m["key4"] = "value4" + _, err = castStringAnyMapToStringDict(m) + + // Check that an error was returned + if err == nil { + t.Errorf("Expected error, got nil") + } +} + +func TestSetInputConversionEnabled(t *testing.T) { + m := NewDefault() + if m.enableInConv != true { + t.Errorf("Expected input conversion to be enabled by default, but it wasn't") + } + + m.SetInputConversionEnabled(false) + if m.enableInConv != false { + t.Errorf("Expected input conversion to be disabled, but it wasn't") + } + + m.SetInputConversionEnabled(true) + if m.enableInConv != true { + t.Errorf("Expected input conversion to be enabled, but it wasn't") + } +} + +func TestSetOutputConversionEnabled(t *testing.T) { + m := NewDefault() + if m.enableOutConv != true { + t.Errorf("Expected output conversion to be enabled by default, but it wasn't") + } + + m.SetOutputConversionEnabled(false) + if m.enableOutConv != false { + t.Errorf("Expected output conversion to be disabled, but it wasn't") + } + + m.SetOutputConversionEnabled(true) + if m.enableOutConv != true { + t.Errorf("Expected output conversion to be enabled, but it wasn't") + } +} diff --git a/machine.go b/machine.go index 13f5f6f8..4603a961 100644 --- a/machine.go +++ b/machine.go @@ -10,7 +10,6 @@ import ( "io/fs" "sync" - "github.com/1set/starlight/convert" "go.starlark.net/starlark" ) @@ -31,14 +30,15 @@ import ( // The result of each run is cached and written back to the environment, so that it can be used in the next run of the script. // // The environment can be reset, allowing the script to be run again with a fresh set of variables and modules. -// type Machine struct { mu sync.RWMutex // set variables - globals StringAnyMap - preloadMods ModuleLoaderList - lazyloadMods ModuleLoaderMap - printFunc PrintFunc + globals StringAnyMap + preloadMods ModuleLoaderList + lazyloadMods ModuleLoaderMap + printFunc PrintFunc + enableInConv bool + enableOutConv bool // source code scriptName string scriptContent []byte @@ -69,22 +69,26 @@ type LoadFunc func(thread *starlark.Thread, module string) (starlark.StringDict, // NewDefault creates a new Starlark runtime environment. func NewDefault() *Machine { - return &Machine{} + return &Machine{enableInConv: true, enableOutConv: true} } // NewWithGlobals creates a new Starlark runtime environment with given global variables. func NewWithGlobals(globals StringAnyMap) *Machine { return &Machine{ - globals: globals, + enableInConv: true, + enableOutConv: true, + globals: globals, } } // NewWithLoaders creates a new Starlark runtime environment with given global variables and preload & lazyload module loaders. func NewWithLoaders(globals StringAnyMap, preload ModuleLoaderList, lazyload ModuleLoaderMap) *Machine { return &Machine{ - globals: globals, - preloadMods: preload, - lazyloadMods: lazyload, + enableInConv: true, + enableOutConv: true, + globals: globals, + preloadMods: preload, + lazyloadMods: lazyload, } } @@ -94,9 +98,11 @@ func NewWithBuiltins(globals StringAnyMap, additionalPreload ModuleLoaderList, a lazy := GetBuiltinModuleMap() lazy.Merge(additionalLazyload) return &Machine{ - globals: globals, - preloadMods: pre, - lazyloadMods: lazy, + enableInConv: true, + enableOutConv: true, + globals: globals, + preloadMods: pre, + lazyloadMods: lazy, } } @@ -112,9 +118,11 @@ func NewWithNames(globals StringAnyMap, preloads []string, lazyloads []string) * panic(err) } return &Machine{ - globals: globals, - preloadMods: pre, - lazyloadMods: lazy, + enableInConv: true, + enableOutConv: true, + globals: globals, + preloadMods: pre, + lazyloadMods: lazy, } } @@ -220,12 +228,28 @@ func (m *Machine) SetScript(name string, content []byte, fileSys fs.FS) { m.scriptFS = fileSys } +// SetInputConversionEnabled controls the conversion of Starlark variables from input into Starlight wrappers. +func (m *Machine) SetInputConversionEnabled(enabled bool) { + m.mu.Lock() // Locking to avoid concurrent access + defer m.mu.Unlock() + + m.enableInConv = enabled +} + +// SetOutputConversionEnabled controls the conversion of Starlark variables from output into Starlight wrappers. +func (m *Machine) SetOutputConversionEnabled(enabled bool) { + m.mu.Lock() // Locking to avoid concurrent access + defer m.mu.Unlock() + + m.enableOutConv = enabled +} + // Export returns the current variables of the Starlark runtime environment. func (m *Machine) Export() StringAnyMap { m.mu.RLock() defer m.mu.RUnlock() - return convert.FromStringDict(m.predeclared) + return m.convertOutput(m.predeclared) } // StringAnyMap type is a map of string to interface{} and is used to store global variables like StringDict of Starlark, but not a Starlark type. diff --git a/machine_test.go b/machine_test.go index 25125902..52d285a2 100644 --- a/machine_test.go +++ b/machine_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/1set/starlet" + "go.starlark.net/starlark" ) func TestNewDefault(t *testing.T) { @@ -310,3 +311,155 @@ func TestMachine_Export_Run(t *testing.T) { } } } + +func TestMachine_DisableInputConversion(t *testing.T) { + getMac := func(g starlet.StringAnyMap, code string) *starlet.Machine { + m := starlet.NewWithGlobals(g) + m.SetInputConversionEnabled(false) + m.SetScript("test", []byte(code), nil) + return m + } + + // nil input + { + m := getMac(nil, `a = 1`) + _, err := m.Run() + if err != nil { + t.Errorf("expected no error for nil input, got %v", err) + } + } + + // empty input + { + m := getMac(starlet.StringAnyMap{}, `a = 1`) + _, err := m.Run() + if err != nil { + t.Errorf("expected no error for empty input, got %v", err) + } + } + + // converted + { + m := getMac(starlet.StringAnyMap{"a": starlark.MakeInt(100)}, `b = a + 1`) + res, err := m.Run() + if err != nil { + t.Errorf("expected no error for converted input, got %v", err) + } + if exp := starlet.StringAnyMap(map[string]interface{}{"b": int64(101)}); !expectEqualStringAnyMap(t, res, exp) { + t.Errorf("expected result of converted input %v, got %v", exp, res) + return + } + } + + // unconverted -- error + { + m := getMac(starlet.StringAnyMap{"a": 100}, `b = a + 1`) + _, err := m.Run() + if err == nil { + t.Errorf("expected error for unconverted input, got none") + } + } +} + +func TestMachine_DisableOutputConversion(t *testing.T) { + getMac := func(code string) *starlet.Machine { + g := starlet.StringAnyMap{"a": 100} + m := starlet.NewWithGlobals(g) + m.SetOutputConversionEnabled(false) + m.SetScript("test", []byte(code), nil) + return m + } + + // empty output + { + m := getMac(`a + 100`) + res, err := m.Run() + if err != nil { + t.Errorf("expected no error for empty output, got %v", err) + } + if len(res) != 0 { + t.Errorf("expected empty result for empty output, got %v", res) + } + } + + // has output + { + m := getMac(`b = a << 3`) + res, err := m.Run() + if err != nil { + t.Errorf("expected no error for output, got %v", err) + } + if exp := starlet.StringAnyMap(map[string]interface{}{"b": starlark.MakeInt(800)}); !expectEqualStringAnyMap(t, res, exp) { + t.Errorf("expected result for output %v, got %v", exp, res) + return + } + } + + // for export + { + m := getMac(`b = a << 2`) + res, err := m.Run() + if err != nil { + t.Errorf("expected no error for output, got %v", err) + } + if exp := starlet.StringAnyMap(map[string]interface{}{"b": starlark.MakeInt(400)}); !expectEqualStringAnyMap(t, res, exp) { + t.Errorf("expected result for output %v, got %v", exp, res) + return + } + ed := m.Export() + if exp := starlet.StringAnyMap(map[string]interface{}{"a": starlark.MakeInt(100), "b": starlark.MakeInt(400)}); !expectEqualStringAnyMap(t, ed, exp) { + t.Errorf("expected export for output %v, got %v", exp, ed) + return + } + } +} + +func TestMachine_DisableBothConversion(t *testing.T) { + getMac := func(g starlet.StringAnyMap, code string) *starlet.Machine { + m := starlet.NewWithGlobals(g) + m.SetInputConversionEnabled(false) + m.SetOutputConversionEnabled(false) + m.SetScript("test", []byte(code), nil) + return m + } + + // nil input + { + m := getMac(nil, `a = 1`) + _, err := m.Run() + if err != nil { + t.Errorf("expected no error for nil input, got %v", err) + } + } + + // empty input + { + m := getMac(starlet.StringAnyMap{}, `a = 1`) + _, err := m.Run() + if err != nil { + t.Errorf("expected no error for empty input, got %v", err) + } + } + + // converted + { + m := getMac(starlet.StringAnyMap{"a": starlark.MakeInt(100)}, `b = a * 2`) + res, err := m.Run() + if err != nil { + t.Errorf("expected no error for converted input, got %v", err) + } + if exp := starlet.StringAnyMap(map[string]interface{}{"b": starlark.MakeInt(200)}); !expectEqualStringAnyMap(t, res, exp) { + t.Errorf("expected result of converted input %v, got %v", exp, res) + return + } + } + + // unconverted -- error + { + m := getMac(starlet.StringAnyMap{"a": 100}, `b = a + 1`) + _, err := m.Run() + if err == nil { + t.Errorf("expected error for unconverted input, got none") + } + } +} diff --git a/run.go b/run.go index 6adf84d0..33f8fd5c 100644 --- a/run.go +++ b/run.go @@ -2,6 +2,7 @@ package starlet import ( "context" + "fmt" "io/fs" "sync" "time" @@ -175,7 +176,7 @@ func (m *Machine) runInternal(ctx context.Context, extras StringAnyMap) (out Str } // handle result and convert - out = convert.FromStringDict(res) + out = m.convertOutput(res) if err != nil { // for exit code if err.Error() == goidiomatic.ErrSystemExit.Error() { @@ -205,16 +206,19 @@ func (m *Machine) prepareThread(extras StringAnyMap) (err error) { if m.thread == nil { // -- for the first run // preset globals + preload modules + extras -> predeclared - if m.predeclared, err = convert.MakeStringDict(m.globals); err != nil { + if m.predeclared, err = m.convertInput(m.globals); err != nil { return errorStarlightConvert("globals", err) } if err = m.preloadMods.LoadAll(m.predeclared); err != nil { return errorStarletError("preload", err) } - esd, err := convert.MakeStringDict(extras) + + // convert extras or not + esd, err := m.convertInput(extras) if err != nil { return errorStarlightConvert("extras", err) } + // merge extras for k, v := range esd { m.predeclared[k] = v } @@ -254,3 +258,44 @@ func (m *Machine) Reset() { m.loadCache = nil m.predeclared = nil } + +// convertInput converts a StringAnyMap to a starlark.StringDict, usually for output variable. +func (m *Machine) convertInput(a StringAnyMap) (starlark.StringDict, error) { + if m.enableInConv { + return convert.MakeStringDict(a) + } else { + return castStringAnyMapToStringDict(a) + } +} + +// convertOutput converts a starlark.StringDict to a StringAnyMap, usually for output variable. +func (m *Machine) convertOutput(d starlark.StringDict) StringAnyMap { + if m.enableOutConv { + return convert.FromStringDict(d) + } else { + return castStringDictToAnyMap(d) + } +} + +// castStringDictToAnyMap converts a starlark.StringDict to a StringAnyMap without any Starlight conversion. +func castStringDictToAnyMap(m starlark.StringDict) StringAnyMap { + ret := make(StringAnyMap, len(m)) + for k, v := range m { + ret[k] = v + } + return ret +} + +// castStringAnyMapToStringDict converts a StringAnyMap to a starlark.StringDict without any Starlight conversion. +// It fails if any values are not starlark.Value. +func castStringAnyMapToStringDict(m StringAnyMap) (starlark.StringDict, error) { + ret := make(starlark.StringDict, len(m)) + for k, v := range m { + sv, ok := v.(starlark.Value) + if !ok { + return nil, fmt.Errorf("value of key %q is not a starlark.Value", k) + } + ret[k] = sv + } + return ret, nil +}