From b5024804be2e0d1ae20493a306440cf4c78723a4 Mon Sep 17 00:00:00 2001 From: Rohan Singh Date: Fri, 14 Jan 2022 15:22:26 -0500 Subject: [PATCH] schema: Strongly type defaults and config values (#120) * schema: Strongly type defaults and config values Support config values of type other than string. Previously we just assumed all values were strings, which is kind of crappy for things like toggles where we were checking stuff like `config.get("X") == "true"`. Instead, we now check against the schema and provide a value of the correct type to Starlark. We still serialize everything as a string for storage, but we deserialize it to the appropriate type. * Updated documentation --- docs/schema.md | 35 +++++++-------- examples/schema_hello_world.star | 10 ++--- runtime/runtime.go | 76 +++++++++++++++++++++----------- schema/module.go | 4 +- schema/module_test.go | 2 +- schema/schema.go | 56 ++++++++++++----------- schema/schema_test.go | 54 +++++++++++------------ schema/toggle.go | 10 +++-- schema/toggle_test.go | 4 +- 9 files changed, 140 insertions(+), 111 deletions(-) diff --git a/docs/schema.md b/docs/schema.md index 11bde93009..a6ca4a4753 100644 --- a/docs/schema.md +++ b/docs/schema.md @@ -17,17 +17,16 @@ def main(): ``` This is a quick start, so let's start with the code and we'll break it down: + ```starlark load("render.star", "render") load("schema.star", "schema") -DEFAULT = "false" - def main(config): - small = config.get("small", DEFAULT) - msg = render.Text("Hello, World!") - if small == "false": + if config.get("small"): msg = render.Text("Hello, World!", font = "CG-pixel-3x5-mono") + else: + msg = render.Text("Hello, World!") return render.Root( child = msg, @@ -35,16 +34,16 @@ def main(config): def get_schema(): return schema.Schema( - version = "1", - fields = [ - schema.Toggle( - id = "small", - name = "Display small text", - desc = "A toggle to display smaller text.", - icon = "compress", - default = DEFAULT, - ), - ], + version = "1", + fields = [ + schema.Toggle( + id = "small", + name = "Display small text", + desc = "A toggle to display smaller text.", + icon = "compress", + default = False, + ), + ], ) ``` @@ -54,7 +53,7 @@ The `get_schema` method returns a `schema.Schema` object that contains _fields_. Next up should be more familiar. We're now passing `config` into `main()`. This is the same for current pixlet scripts that take `config` today. In [Community Apps](https://github.com/tidbyt/community), we will populate the config hashmap with values configured from the mobile app. More specifically, `config` is a key/value pair where the key is the `id` of the schema field and the value is determined by the user in the mobile app. -That's about it! One final note - we use the `DEFAULT` constant in two places here. That allows the `Toggle` to have a default value when we call `get_schema` and for the app to be run without `get_schema` being called. This could happen in a few different ways, such as using `pixlet render` or when we render previews for the mobile app listing. +That's about it! ## Icons Each schema field takes an `icon` value. We use the free icons from [Font Awesome](https://fontawesome.com/) with the names camel cased. For example [users-cog](https://fontawesome.com/v5.15/icons/users-cog?style=solid) should be `usersCog` in the `icon` value. @@ -63,7 +62,7 @@ Each schema field takes an `icon` value. We use the free icons from [Font Awesom These are the current fields we support through schema today. Note that any addition of a field will require changes in our mobile app before we can truly support them. ### Toggle -A toggle provides an on/off switch for your app. The values returned in `config` are either `"true"` or `"false"` strings. Note - you have to convert this to a boolean inside of your app. It's not ideal and we hope to fix it in a future version of schema. +A toggle provides an on/off switch for your app. The values returned in `config` are either `True` or `False`. The following field will display as follows in the mobile app: ```starlark @@ -72,7 +71,7 @@ schema.Toggle( name = "Display weather", desc = "A toggle to display weather or not.", icon = "cloud", - default = "true", + default = True, ) ``` ![toggle example](img/toggle.jpg) diff --git a/examples/schema_hello_world.star b/examples/schema_hello_world.star index c925354a14..07966b7b10 100644 --- a/examples/schema_hello_world.star +++ b/examples/schema_hello_world.star @@ -1,13 +1,11 @@ load("render.star", "render") load("schema.star", "schema") -DEFAULT = "false" - def main(config): - small = config.get("small", DEFAULT) - msg = render.Text("Hello, World!") - if small == "false": + if config.get("small"): msg = render.Text("Hello, World!", font = "CG-pixel-3x5-mono") + else: + msg = render.Text("Hello, World!") return render.Root( child = msg, @@ -22,7 +20,7 @@ def get_schema(): name = "Display small text", desc = "A toggle to display smaller text.", icon = "compress", - default = DEFAULT, + default = False, ), ], ) diff --git a/runtime/runtime.go b/runtime/runtime.go index feb26de467..aaeb921d6e 100644 --- a/runtime/runtime.go +++ b/runtime/runtime.go @@ -3,7 +3,9 @@ package runtime import ( "context" "crypto/md5" + "encoding/json" "fmt" + "strconv" "github.com/pkg/errors" starlibbase64 "github.com/qri-io/starlib/encoding/base64" @@ -38,15 +40,16 @@ func init() { } type Applet struct { - Filename string - Id string - Globals starlark.StringDict - src []byte - loader ModuleLoader - predeclared starlark.StringDict - main *starlark.Function - schema string - schemaHandler map[string]schema.SchemaHandler + Filename string + Id string + Globals starlark.StringDict + src []byte + loader ModuleLoader + predeclared starlark.StringDict + main *starlark.Function + + schema *schema.Schema + schemaJSON []byte } func (a *Applet) thread(initializers ...ThreadInitializer) *starlark.Thread { @@ -102,8 +105,6 @@ func (a *Applet) Load(filename string, src []byte, loader ModuleLoader) (err err } a.main = main - var s string - var handlers map[string]schema.SchemaHandler schemaFun, _ := a.Globals[schema.SchemaFunctionName].(*starlark.Function) if schemaFun != nil { schemaVal, err := a.Call(schemaFun, nil) @@ -111,14 +112,16 @@ func (a *Applet) Load(filename string, src []byte, loader ModuleLoader) (err err return errors.Wrap(err, "calling schema function") } - s, handlers, err = schema.EncodeSchema(schemaVal, a.Globals) + a.schema, err = schema.FromStarlark(schemaVal, a.Globals) if err != nil { - return errors.Wrap(err, "encode schema") + return errors.Wrap(err, "parsing Starlark schema") } - } - a.schema = s - a.schemaHandler = handlers + a.schemaJSON, err = json.Marshal(a.schema) + if err != nil { + return errors.Wrap(err, "serializing schema to JSON") + } + } return nil } @@ -130,10 +133,25 @@ func (a *Applet) Run(config map[string]string, initializers ...ThreadInitializer if a.main.NumParams() > 0 { starlarkConfig := starlark.NewDict(len(config)) for k, v := range config { - starlarkConfig.SetKey( - starlark.String(k), - starlark.String(v), - ) + var starlarkVal starlark.Value + starlarkVal = starlark.String(v) + + if a.schema != nil { + // app has a schema, so we can provide strongly typed config values + field := a.schema.Field(k) + + if field == nil { + // we have a value, but it's not part of the app's schema. + // drop it entirely. + starlarkVal = starlark.String(v) + continue + } else if field.Type == "onoff" { + b, _ := strconv.ParseBool(v) + starlarkVal = starlark.Bool(b) + } + } + + starlarkConfig.SetKey(starlark.String(k), starlarkVal) } args = starlark.Tuple{starlarkConfig} } @@ -173,7 +191,7 @@ func (a *Applet) Run(config map[string]string, initializers ...ThreadInitializer // CallSchemaHandler calls the schema handler for a field, passing it a single // string parameter and returning a single string value. func (app *Applet) CallSchemaHandler(ctx context.Context, fieldId, parameter string) (result string, err error) { - handler, found := app.schemaHandler[fieldId] + handler, found := app.schema.Handlers[fieldId] if !found { return "", fmt.Errorf("no handler exported for field id %s", fieldId) } @@ -196,11 +214,17 @@ func (app *Applet) CallSchemaHandler(ctx context.Context, fieldId, parameter str return options, nil case schema.ReturnSchema: - schema, _, err := schema.EncodeSchema(resultVal, app.Globals) + sch, err := schema.FromStarlark(resultVal, app.Globals) if err != nil { - return "", err + return "", errors.Wrap(err, "parsing Starlark schema") + } + + s, err := json.Marshal(sch) + if err != nil { + return "", errors.Wrap(err, "serializing schema to JSON") } - return schema, nil + + return string(s), nil case schema.ReturnString: str, ok := starlark.AsString(resultVal) @@ -216,9 +240,9 @@ func (app *Applet) CallSchemaHandler(ctx context.Context, fieldId, parameter str return "", fmt.Errorf("a very unexpected error happened for field \"%s\"", fieldId) } -// GetSchema returns the config for the applet. +// GetSchema returns the serialized schema for the applet. func (app *Applet) GetSchema() string { - return app.schema + return string(app.schemaJSON) } func attachContext(ctx context.Context) ThreadInitializer { diff --git a/schema/module.go b/schema/module.go index fb342da0bd..3a1d92d992 100644 --- a/schema/module.go +++ b/schema/module.go @@ -68,7 +68,7 @@ func newSchema(thread *starlark.Thread, _ *starlark.Builtin, args starlark.Tuple s := &StarlarkSchema{} s.Version = version.GoString() - s.Schema.Schema = []SchemaField{} + s.Schema.Fields = []SchemaField{} var fieldVal starlark.Value fieldIter := fields.Iterate() @@ -87,7 +87,7 @@ func newSchema(thread *starlark.Thread, _ *starlark.Builtin, args starlark.Tuple ) } - s.Schema.Schema = append(s.Schema.Schema, f.AsSchemaField()) + s.Schema.Fields = append(s.Schema.Fields, f.AsSchemaField()) } s.starlarkFields = fields diff --git a/schema/module_test.go b/schema/module_test.go index bd9b83cd8a..1040d63945 100644 --- a/schema/module_test.go +++ b/schema/module_test.go @@ -29,7 +29,7 @@ s = schema.Schema( name = "Display Weather", desc = "A toggle to determine if the weather should be displayed.", icon = "cloud", - default = "false", + default = False, ), ], ) diff --git a/schema/schema.go b/schema/schema.go index d1718c5094..5661c44c3b 100644 --- a/schema/schema.go +++ b/schema/schema.go @@ -26,8 +26,11 @@ const ( // Schema holds a configuration object for an applet. It holds a list of fileds // that are exported from an applet. type Schema struct { - Version string `json:"version" validate:"required"` - Schema []SchemaField `json:"schema" validate:"required,dive"` + Version string `json:"version" validate:"required"` + Fields []SchemaField `json:"schema" validate:"required,dive"` + Handlers map[string]SchemaHandler `json:"-"` + + fieldByID map[string]*SchemaField } // SchemaField represents an item in the config used to confgure an applet. @@ -78,38 +81,46 @@ type SchemaHandler struct { ReturnType HandlerReturnType } -// Encodes a starlark config schema into validated json and extracts -// all schema handlers. -func EncodeSchema( +// Field returns a pointer to the schema field with the given ID, or nil if it +// doesn't exist. +func (s *Schema) Field(id string) *SchemaField { + return s.fieldByID[id] +} + +// FromStarlark creates a new Schema from a Starlark schema object. +func FromStarlark( starlarkSchema starlark.Value, - globals starlark.StringDict) (string, map[string]SchemaHandler, error) { + globals starlark.StringDict) (*Schema, error) { schemaTree, err := unmarshalStarlark(starlarkSchema) if err != nil { - return "", nil, err + return nil, err } treeJSON, err := json.Marshal(schemaTree) if err != nil { - return "", nil, err + return nil, err } - schema := &Schema{Version: "1"} - if err := json.Unmarshal(treeJSON, &schema.Schema); err != nil { - return "", nil, err + schema := &Schema{Version: "1", + Handlers: make(map[string]SchemaHandler), + fieldByID: make(map[string]*SchemaField), + } + if err := json.Unmarshal(treeJSON, &schema.Fields); err != nil { + return nil, err } err = validateSchema(schema) if err != nil { - return "", nil, err + return nil, err } - handlers := map[string]SchemaHandler{} - for i, schemaField := range schema.Schema { + for i, schemaField := range schema.Fields { + schema.fieldByID[schemaField.ID] = &schemaField if schemaField.Handler != "" { handlerValue, found := globals[schemaField.Handler] if !found { - return "", nil, fmt.Errorf( + return nil, fmt.Errorf( "field %d references non-existent handler \"%s\"", i, schemaField.Handler) @@ -117,7 +128,7 @@ func EncodeSchema( handlerFun, ok := handlerValue.(*starlark.Function) if !ok { - return "", nil, fmt.Errorf( + return nil, fmt.Errorf( "field %d references \"%s\" which is not a function", i, schemaField.Handler) } @@ -135,21 +146,16 @@ func EncodeSchema( case "oauth1": handlerType = ReturnString default: - return "", nil, fmt.Errorf( + return nil, fmt.Errorf( "field %d of type \"%s\" can't have a handler function", i, schemaField.Type) } - handlers[schemaField.ID] = SchemaHandler{Function: handlerFun, ReturnType: handlerType} + schema.Handlers[schemaField.ID] = SchemaHandler{Function: handlerFun, ReturnType: handlerType} } } - schemaJSON, err := json.Marshal(schema) - if err != nil { - return "", nil, err - } - - return string(schemaJSON), handlers, nil + return schema, nil } // Encodes a list of schema options into validated json. @@ -188,7 +194,7 @@ func unmarshalStarlark(object starlark.Value) (interface{}, error) { switch v := object.(type) { case *StarlarkSchema: - return v.Schema.Schema, nil + return v.Schema.Fields, nil case starlark.String: return v.GoString(), nil diff --git a/schema/schema_test.go b/schema/schema_test.go index ec3812fcda..ce080ab7a9 100644 --- a/schema/schema_test.go +++ b/schema/schema_test.go @@ -6,7 +6,7 @@ import ( "testing" "github.com/pkg/errors" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "tidbyt.dev/pixlet/runtime" "tidbyt.dev/pixlet/schema" ) @@ -137,16 +137,16 @@ def main(): ` app, err := loadApp(code) - assert.NoError(t, err) + require.NoError(t, err) jsonSchema := app.GetSchema() var s schema.Schema json.Unmarshal([]byte(jsonSchema), &s) - assert.Equal(t, schema.Schema{ + require.Equal(t, schema.Schema{ Version: "1", - Schema: []schema.SchemaField{ + Fields: []schema.SchemaField{ { Type: "location", ID: "locationid", @@ -301,17 +301,17 @@ def main(): ` app, err := loadApp(code) - assert.NoError(t, err) + require.NoError(t, err) jsonSchema, err := app.CallSchemaHandler(context.Background(), "generatedid", "foobar") - assert.NoError(t, err) + require.NoError(t, err) var s schema.Schema json.Unmarshal([]byte(jsonSchema), &s) - assert.Equal(t, schema.Schema{ + require.Equal(t, schema.Schema{ Version: "1", - Schema: []schema.SchemaField{ + Fields: []schema.SchemaField{ { Type: "text", ID: "generatedid", @@ -354,10 +354,10 @@ def main(): ` app, err := loadApp(code) - assert.NoError(t, err) + require.NoError(t, err) _, err = app.CallSchemaHandler(context.Background(), "generatedid", "foobar") - assert.Error(t, err) + require.Error(t, err) } // Verifies that appruntime.Schemas returned by a generated field's handler is @@ -383,7 +383,7 @@ def main(): ` _, err := loadApp(code) - assert.Error(t, err) + require.Error(t, err) } func TestSchemaWithGeneratedIconNotAllowed(t *testing.T) { @@ -416,7 +416,7 @@ def main(): ` _, err := loadApp(code) - assert.Error(t, err) + require.Error(t, err) } func TestSchemaWithLocationBasedHandlerSuccess(t *testing.T) { @@ -440,11 +440,11 @@ def main(): ` app, err := loadApp(code) - assert.NoError(t, err) + require.NoError(t, err) stringValue, err := app.CallSchemaHandler(context.Background(), "locationbasedid", "fart") - assert.NoError(t, err) - assert.Equal(t, "[{\"text\":\"Your only option is\",\"value\":\"fart\"}]", stringValue) + require.NoError(t, err) + require.Equal(t, "[{\"text\":\"Your only option is\",\"value\":\"fart\"}]", stringValue) } func TestSchemaWithLocationBasedHandlerMalformed(t *testing.T) { @@ -468,10 +468,10 @@ def main(): ` app, err := loadApp(code) - assert.NoError(t, err) + require.NoError(t, err) _, err = app.CallSchemaHandler(context.Background(), "locationbasedid", "fart") - assert.Error(t, err) + require.Error(t, err) } func TestSchemaWithTypeaheadHandlerSuccess(t *testing.T) { @@ -495,11 +495,11 @@ def main(): ` app, err := loadApp(code) - assert.NoError(t, err) + require.NoError(t, err) stringValue, err := app.CallSchemaHandler(context.Background(), "typeaheadid", "farts") - assert.NoError(t, err) - assert.Equal(t, "[{\"text\":\"You searched for\",\"value\":\"farts\"}]", stringValue) + require.NoError(t, err) + require.Equal(t, "[{\"text\":\"You searched for\",\"value\":\"farts\"}]", stringValue) } func TestSchemaWithTypeaheadHandlerMalformed(t *testing.T) { @@ -523,10 +523,10 @@ def main(): ` app, err := loadApp(code) - assert.NoError(t, err) + require.NoError(t, err) _, err = app.CallSchemaHandler(context.Background(), "typeaheadid", "fart") - assert.Error(t, err) + require.Error(t, err) } func TestSchemaWithOAuth2HandlerSuccess(t *testing.T) { @@ -554,11 +554,11 @@ def main(): ` app, err := loadApp(code) - assert.NoError(t, err) + require.NoError(t, err) stringValue, err := app.CallSchemaHandler(context.Background(), "oauth2id", "farts") - assert.NoError(t, err) - assert.Equal(t, "a-refresh-token", stringValue) + require.NoError(t, err) + require.Equal(t, "a-refresh-token", stringValue) } func TestSchemaWithOAuth2HandlerMalformed(t *testing.T) { @@ -586,8 +586,8 @@ def main(): ` app, err := loadApp(code) - assert.NoError(t, err) + require.NoError(t, err) _, err = app.CallSchemaHandler(context.Background(), "oauth2id", "farts") - assert.Error(t, err) + require.Error(t, err) } diff --git a/schema/toggle.go b/schema/toggle.go index 0db883892b..5c2b340a26 100644 --- a/schema/toggle.go +++ b/schema/toggle.go @@ -2,6 +2,7 @@ package schema import ( "fmt" + "strconv" "github.com/mitchellh/hashstructure/v2" "go.starlark.net/starlark" @@ -22,7 +23,7 @@ func newToggle( name starlark.String desc starlark.String icon starlark.String - def starlark.String + def starlark.Bool ) if err := starlark.UnpackArgs( @@ -32,7 +33,7 @@ func newToggle( "name", &name, "desc", &desc, "icon", &icon, - "default", &def, + "default?", &def, ); err != nil { return nil, fmt.Errorf("unpacking arguments for Toggle: %s", err) } @@ -43,7 +44,7 @@ func newToggle( s.Name = name.GoString() s.Description = desc.GoString() s.Icon = icon.GoString() - s.Default = def.GoString() + s.Default = strconv.FormatBool(bool(def)) return s, nil } @@ -74,7 +75,8 @@ func (s *Toggle) Attr(name string) (starlark.Value, error) { return starlark.String(s.Icon), nil case "default": - return starlark.String(s.Default), nil + b, _ := strconv.ParseBool(s.Default) + return starlark.Bool(b), nil default: return nil, nil diff --git a/schema/toggle_test.go b/schema/toggle_test.go index 073c3ca8a9..932a77da30 100644 --- a/schema/toggle_test.go +++ b/schema/toggle_test.go @@ -19,14 +19,14 @@ t = schema.Toggle( name = "Display Weather", desc = "A toggle to determine if the weather should be displayed.", icon = "cloud", - default = "false", + default = True ) assert(t.id == "display_weather") assert(t.name == "Display Weather") assert(t.desc == "A toggle to determine if the weather should be displayed.") assert(t.icon == "cloud") -assert(t.default == "false") +assert(t.default) def main(): return []