From 28234e2607d6a14c21fe5af9a7845acc51e88ff6 Mon Sep 17 00:00:00 2001 From: Rohan Singh Date: Tue, 21 May 2024 17:03:32 -0400 Subject: [PATCH] Refactor the notifications schema definition (#1077) * applet: Expose Starlark globals and main file for embedders * Refactor the notifications schema definition Notifications are still a work in progress. With this change, each notification needs to specify a `builder` function. --- runtime/applet.go | 76 ++++++++++++++++++++++--------------- runtime/render_test.go | 6 +-- schema/module.go | 2 +- schema/notification.go | 19 +++++++--- schema/notification_test.go | 2 + schema/schema.go | 10 +++-- schema/schema_test.go | 29 ++++++++------ 7 files changed, 89 insertions(+), 55 deletions(-) diff --git a/runtime/applet.go b/runtime/applet.go index 507278117c..ff0406820f 100644 --- a/runtime/applet.go +++ b/runtime/applet.go @@ -54,15 +54,14 @@ type AppletOption func(*Applet) error type ThreadInitializer func(thread *starlark.Thread) *starlark.Thread type Applet struct { - ID string + ID string + Globals map[string]starlark.StringDict + MainFile string loader ModuleLoader initializers []ThreadInitializer loadedPaths map[string]bool - globals map[string]starlark.StringDict - - mainFile string mainFun *starlark.Function schemaFile string @@ -130,7 +129,7 @@ func NewApplet(id string, src []byte, opts ...AppletOption) (*Applet, error) { func NewAppletFromFS(id string, fsys fs.FS, opts ...AppletOption) (*Applet, error) { a := &Applet{ ID: id, - globals: make(map[string]starlark.StringDict), + Globals: make(map[string]starlark.StringDict), loadedPaths: make(map[string]bool), } @@ -153,23 +152,18 @@ func (a *Applet) Run(ctx context.Context) (roots []render.Root, err error) { return a.RunWithConfig(ctx, nil) } -// RunWithConfig exceutes the applet's main function, passing it configuration as a -// starlark dict. It returns the render roots that are returned by the applet. -func (a *Applet) RunWithConfig(ctx context.Context, config map[string]string) (roots []render.Root, err error) { - var args starlark.Tuple - if a.mainFun.NumParams() > 0 { - starlarkConfig := AppletConfig(config) - args = starlark.Tuple{starlarkConfig} - } - - returnValue, err := a.Call(ctx, a.mainFun, args...) - if err != nil { - return nil, err - } +// ExtractRoots extracts render roots from a Starlark value. It expects the value +// to be either a single render root or a list of render roots. +// +// It's used internally by RunWithConfig to extract the roots returned by the applet. +func ExtractRoots(val starlark.Value) ([]render.Root, error) { + var roots []render.Root - if returnRoot, ok := returnValue.(render_runtime.Rootable); ok { + if val == starlark.None { + // no roots returned + } else if returnRoot, ok := val.(render_runtime.Rootable); ok { roots = []render.Root{returnRoot.AsRenderRoot()} - } else if returnList, ok := returnValue.(*starlark.List); ok { + } else if returnList, ok := val.(*starlark.List); ok { roots = make([]render.Root, returnList.Len()) iter := returnList.Iterate() defer iter.Done() @@ -188,7 +182,29 @@ func (a *Applet) RunWithConfig(ctx context.Context, config map[string]string) (r i++ } } else { - return nil, fmt.Errorf("expected app implementation to return Root(s) but found: %s", returnValue.Type()) + return nil, fmt.Errorf("expected app implementation to return Root(s) but found: %s", val.Type()) + } + + return roots, nil +} + +// RunWithConfig exceutes the applet's main function, passing it configuration as a +// starlark dict. It returns the render roots that are returned by the applet. +func (a *Applet) RunWithConfig(ctx context.Context, config map[string]string) (roots []render.Root, err error) { + var args starlark.Tuple + if a.mainFun.NumParams() > 0 { + starlarkConfig := AppletConfig(config) + args = starlark.Tuple{starlarkConfig} + } + + returnValue, err := a.Call(ctx, a.mainFun, args...) + if err != nil { + return nil, err + } + + roots, err = ExtractRoots(returnValue) + if err != nil { + return nil, err } return roots, nil @@ -220,7 +236,7 @@ func (app *Applet) CallSchemaHandler(ctx context.Context, handlerName, parameter return options, nil case schema.ReturnSchema: - sch, err := schema.FromStarlark(resultVal, app.globals[app.schemaFile]) + sch, err := schema.FromStarlark(resultVal, app.Globals[app.schemaFile]) if err != nil { return "", err } @@ -253,7 +269,7 @@ func (app *Applet) RunTests(t *testing.T) { return thread }) - for file, globals := range app.globals { + for file, globals := range app.Globals { for name, global := range globals { if !strings.HasPrefix(name, "test_") { continue @@ -347,7 +363,7 @@ func (a *Applet) ensureLoaded(fsys fs.FS, pathToLoad string, currentlyLoading .. // normalize path so that it can be used as a key pathToLoad = path.Clean(pathToLoad) - if _, ok := a.globals[pathToLoad]; ok { + if _, ok := a.Globals[pathToLoad]; ok { // already loaded, good to go return nil } @@ -390,7 +406,7 @@ func (a *Applet) ensureLoaded(fsys fs.FS, pathToLoad string, currentlyLoading .. return nil, err } - if g, ok := a.globals[modulePath]; !ok { + if g, ok := a.Globals[modulePath]; !ok { return nil, fmt.Errorf("module %s not loaded", modulePath) } else { return g, nil @@ -416,17 +432,17 @@ func (a *Applet) ensureLoaded(fsys fs.FS, pathToLoad string, currentlyLoading .. if err != nil { return fmt.Errorf("starlark.ExecFile: %v", err) } - a.globals[pathToLoad] = globals + a.Globals[pathToLoad] = globals // if the file is in the root directory, check for the main function // and schema function mainFun, _ := globals["main"].(*starlark.Function) if mainFun != nil { - if a.mainFile != "" { - return fmt.Errorf("multiple files with a main() function:\n- %s\n- %s", pathToLoad, a.mainFile) + if a.MainFile != "" { + return fmt.Errorf("multiple files with a main() function:\n- %s\n- %s", pathToLoad, a.MainFile) } - a.mainFile = pathToLoad + a.MainFile = pathToLoad a.mainFun = mainFun } @@ -454,7 +470,7 @@ func (a *Applet) ensureLoaded(fsys fs.FS, pathToLoad string, currentlyLoading .. } default: - a.globals[pathToLoad] = starlark.StringDict{ + a.Globals[pathToLoad] = starlark.StringDict{ "file": &file.File{ FS: fsys, Path: pathToLoad, diff --git a/runtime/render_test.go b/runtime/render_test.go index 68117e8816..ac24290c35 100644 --- a/runtime/render_test.go +++ b/runtime/render_test.go @@ -161,7 +161,7 @@ def main(): app, err := NewApplet(filename, []byte(src)) assert.NoError(t, err) - b := app.globals["test_box.star"]["b"] + b := app.Globals["test_box.star"]["b"] assert.IsType(t, &render_runtime.Box{}, b) widget := b.(*render_runtime.Box).AsRenderWidget() @@ -196,7 +196,7 @@ def main(): app, err := NewApplet(filename, []byte(src)) assert.NoError(t, err) - txt := app.globals["test_text.star"]["t"] + txt := app.Globals["test_text.star"]["t"] assert.IsType(t, &render_runtime.Text{}, txt) widget := txt.(*render_runtime.Text).AsRenderWidget() @@ -240,7 +240,7 @@ def main(): app, err := NewApplet(filename, []byte(src)) assert.NoError(t, err) - starlarkP := app.globals["test_png.star"]["img"] + starlarkP := app.Globals["test_png.star"]["img"] require.IsType(t, &render_runtime.Image{}, starlarkP) actualIm := render.PaintWidget(starlarkP.(*render_runtime.Image).AsRenderWidget(), image.Rect(0, 0, 64, 32), 0) diff --git a/schema/module.go b/schema/module.go index 2b865dd0dd..4bdca1edb8 100644 --- a/schema/module.go +++ b/schema/module.go @@ -170,7 +170,7 @@ func newSchema(thread *starlark.Thread, _ *starlark.Builtin, args starlark.Tuple ) } - s.Schema.Notifications = append(s.Schema.Notifications, n.AsSchemaField()) + s.Schema.Notifications = append(s.Schema.Notifications, *n) } } diff --git a/schema/notification.go b/schema/notification.go index a68e16b7ed..59eb268da7 100644 --- a/schema/notification.go +++ b/schema/notification.go @@ -9,6 +9,7 @@ import ( type Notification struct { SchemaField + Builder *starlark.Function `json:"-"` starlarkSounds *starlark.List } @@ -19,11 +20,12 @@ func newNotification( kwargs []starlark.Tuple, ) (starlark.Value, error) { var ( - id starlark.String - name starlark.String - desc starlark.String - icon starlark.String - sounds *starlark.List + id starlark.String + name starlark.String + desc starlark.String + icon starlark.String + sounds *starlark.List + builder *starlark.Function ) if err := starlark.UnpackArgs( @@ -34,6 +36,7 @@ func newNotification( "desc", &desc, "icon", &icon, "sounds", &sounds, + "builder", &builder, ); err != nil { return nil, fmt.Errorf("unpacking arguments for Notification: %s", err) } @@ -44,6 +47,7 @@ func newNotification( s.Name = name.GoString() s.Description = desc.GoString() s.Icon = icon.GoString() + s.Builder = builder var soundVal starlark.Value soundIter := sounds.Iterate() @@ -75,7 +79,7 @@ func (s *Notification) AsSchemaField() SchemaField { func (s *Notification) AttrNames() []string { return []string{ - "id", "name", "desc", "icon", "sounds", + "id", "name", "desc", "icon", "sounds", "builder", } } @@ -97,6 +101,9 @@ func (s *Notification) Attr(name string) (starlark.Value, error) { case "sounds": return s.starlarkSounds, nil + case "builder": + return s.Builder, nil + default: return nil, nil } diff --git a/schema/notification_test.go b/schema/notification_test.go index aa9276112b..643ea81e33 100644 --- a/schema/notification_test.go +++ b/schema/notification_test.go @@ -6,6 +6,7 @@ import ( "testing/fstest" "github.com/stretchr/testify/assert" + "tidbyt.dev/pixlet/runtime" ) @@ -29,6 +30,7 @@ s = schema.Notification( desc = "A new message has arrived", icon = "message", sounds = sounds, + builder = lambda: None, ) assert.eq(s.id, "notification1") diff --git a/schema/schema.go b/schema/schema.go index c9fe1905b6..29760c9399 100644 --- a/schema/schema.go +++ b/schema/schema.go @@ -26,9 +26,9 @@ const ( // Schema holds a configuration object for an applet. It holds a list of fields // that are exported from an applet. type Schema struct { - Version string `json:"version" validate:"required"` - Fields []SchemaField `json:"schema" validate:"dive"` - Notifications []SchemaField `json:"notifications,omitempty" validate:"dive"` + Version string `json:"version" validate:"required"` + Fields []SchemaField `json:"schema" validate:"dive"` + Notifications []Notification `json:"notifications,omitempty" validate:"dive"` Handlers map[string]SchemaHandler `json:"-"` } @@ -107,7 +107,7 @@ func (s Schema) MarshalJSON() ([]byte, error) { a.Fields = make([]SchemaField, 0) } if a.Notifications == nil { - a.Notifications = make([]SchemaField, 0) + a.Notifications = make([]Notification, 0) } js, err := json.Marshal(a) @@ -165,6 +165,8 @@ func FromStarlark( if schemaField.StarlarkHandler != nil { handlerFun = schemaField.StarlarkHandler } else if schemaField.Handler != "" { + // legacy schema, where the handler was a string instead of + // a function reference handlerValue, ok := globals[schemaField.Handler] if !ok { return nil, fmt.Errorf( diff --git a/schema/schema_test.go b/schema/schema_test.go index 67db5d85a5..36168725fc 100644 --- a/schema/schema_test.go +++ b/schema/schema_test.go @@ -39,6 +39,9 @@ def typeaheadhandler(): def oauth2handler(): return "a-refresh-token" +def build_notification(): + return None + def get_schema(): return schema.Schema( version = "1", @@ -49,6 +52,7 @@ def get_schema(): name = "Notification", desc = "A Notification", icon = "notification", + builder = build_notification, sounds = [ schema.Sound( id = "ding", @@ -154,18 +158,20 @@ def main(): assert.Equal(t, schema.Schema{ Version: "1", - Notifications: []schema.SchemaField{ + Notifications: []schema.Notification{ { - Type: "notification", - ID: "notificationid", - Name: "Notification", - Description: "A Notification", - Icon: "notification", - Sounds: []schema.SchemaSound{ - { - ID: "ding", - Title: "Ding!", - Path: "ding.mp3", + SchemaField: schema.SchemaField{ + Type: "notification", + ID: "notificationid", + Name: "Notification", + Description: "A Notification", + Icon: "notification", + Sounds: []schema.SchemaSound{ + { + ID: "ding", + Title: "Ding!", + Path: "ding.mp3", + }, }, }, }, @@ -307,6 +313,7 @@ def get_schema(): name = "Notification", desc = "A Notification", icon = "notification", + builder = lambda: None, sounds = [ schema.Sound( id = "ding",