From 14bd537fe07cf669d54f5c732980b64b0a1997ea Mon Sep 17 00:00:00 2001 From: Cedric Date: Mon, 8 Apr 2024 18:00:04 +0100 Subject: [PATCH] [KS-136] Correctly handle numbers in YAML by converting them to floats or ints (#12739) --- core/services/workflows/delegate.go | 6 +- core/services/workflows/models_yaml.go | 84 ++++++++++++++++++- core/services/workflows/models_yaml_test.go | 35 +++++++- core/services/workflows/state.go | 14 ++++ .../fixtures/workflows/workflow_schema.json | 9 +- 5 files changed, 137 insertions(+), 11 deletions(-) diff --git a/core/services/workflows/delegate.go b/core/services/workflows/delegate.go index 2c95b478709..b81ec5407e2 100644 --- a/core/services/workflows/delegate.go +++ b/core/services/workflows/delegate.go @@ -35,13 +35,13 @@ consensus: aggregation_config: "0x1111111111111111111100000000000000000000000000000000000000000000": deviation: "0.001" - heartbeat: "30m" + heartbeat: 3600 "0x2222222222222222222200000000000000000000000000000000000000000000": deviation: "0.001" - heartbeat: "30m" + heartbeat: 3600 "0x3333333333333333333300000000000000000000000000000000000000000000": deviation: "0.001" - heartbeat: "30m" + heartbeat: 3600 encoder: "EVM" encoder_config: abi: "mercury_reports bytes[]" diff --git a/core/services/workflows/models_yaml.go b/core/services/workflows/models_yaml.go index aceabb44ec2..396811c3729 100644 --- a/core/services/workflows/models_yaml.go +++ b/core/services/workflows/models_yaml.go @@ -1,12 +1,14 @@ package workflows import ( + "bytes" "encoding/json" "fmt" "slices" "strings" "github.com/invopop/jsonschema" + "github.com/shopspring/decimal" "sigs.k8s.io/yaml" ) @@ -71,6 +73,84 @@ func (w workflowSpecYaml) toWorkflowSpec() workflowSpec { } } +type mapping map[string]any + +func (m *mapping) UnmarshalJSON(b []byte) error { + mp := map[string]any{} + + d := json.NewDecoder(bytes.NewReader(b)) + d.UseNumber() + + err := d.Decode(&mp) + if err != nil { + return err + } + + nm, err := convertNumbers(mp) + if err != nil { + return err + } + + *m = (mapping)(nm) + return err +} + +func convertNumber(el any) (any, error) { + switch elv := el.(type) { + case json.Number: + if strings.Contains(elv.String(), ".") { + f, err := elv.Float64() + if err == nil { + return decimal.NewFromFloat(f), nil + } + } + + return elv.Int64() + default: + return el, nil + } +} + +func convertNumbers(m map[string]any) (map[string]any, error) { + nm := map[string]any{} + for k, v := range m { + switch tv := v.(type) { + case map[string]any: + cm, err := convertNumbers(tv) + if err != nil { + return nil, err + } + + nm[k] = cm + case []any: + na := make([]any, len(tv)) + for i, v := range tv { + cv, err := convertNumber(v) + if err != nil { + return nil, err + } + + na[i] = cv + } + + nm[k] = na + default: + cv, err := convertNumber(v) + if err != nil { + return nil, err + } + + nm[k] = cv + } + } + + return nm, nil +} + +func (m mapping) MarshalJSON() ([]byte, error) { + return json.Marshal(map[string]any(m)) +} + // stepDefinitionYaml is the YAML representation of a step in a workflow. // // It allows for multiple ways of defining a step, which we later @@ -131,7 +211,7 @@ type stepDefinitionYaml struct { // - Input reference cannot be resolved. // - Input is defined on triggers // NOTE: Should introduce a custom validator to cover trigger case - Inputs map[string]any `json:"inputs,omitempty"` + Inputs mapping `json:"inputs,omitempty"` // The configuration of a Capability will be done using the “config” property. Each capability is responsible for defining an external interface used during setup. This interface may be unique or identical, meaning multiple Capabilities might use the same configuration properties. // @@ -149,7 +229,7 @@ type stepDefinitionYaml struct { // address: "0xaabbcc" // method: "updateFeedValues(report bytes, role uint8)" // params: [$(inputs.report), 1] - Config map[string]any `json:"config" jsonschema:"required"` + Config mapping `json:"config" jsonschema:"required"` } // toStepDefinition converts a stepDefinitionYaml to a stepDefinition. diff --git a/core/services/workflows/models_yaml_test.go b/core/services/workflows/models_yaml_test.go index 8f2461c49b5..411781a3782 100644 --- a/core/services/workflows/models_yaml_test.go +++ b/core/services/workflows/models_yaml_test.go @@ -8,6 +8,8 @@ import ( "github.com/google/go-cmp/cmp" "github.com/santhosh-tekuri/jsonschema/v5" + "github.com/shopspring/decimal" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "sigs.k8s.io/yaml" @@ -55,7 +57,7 @@ func TestWorkflowSpecMarshalling(t *testing.T) { t.Run("Type coercion", func(t *testing.T) { workflowBytes := fixtureReader("workflow_1") - spec := workflowSpec{} + spec := workflowSpecYaml{} err := yaml.Unmarshal(workflowBytes, &spec) require.NoError(t, err) @@ -108,8 +110,8 @@ func TestWorkflowSpecMarshalling(t *testing.T) { numbers, ok := booleanCoercions["numbers"] require.True(t, ok, "expected numbers to be present in boolean_coercions") for _, v := range numbers.([]interface{}) { - _, ok = v.(float64) - require.True(t, ok, "expected float64 but got %T", v) + _, ok = v.(int64) + require.True(t, ok, "expected int64 but got %T", v) } }) @@ -227,3 +229,30 @@ func TestJsonSchema(t *testing.T) { }) }) } + +func TestParsesIntsCorrectly(t *testing.T) { + wf, err := Parse(hardcodedWorkflow) + require.NoError(t, err) + + n, err := wf.Vertex("evm_median") + require.NoError(t, err) + + assert.Equal(t, int64(3600), n.Config["aggregation_config"].(map[string]any)["0x1111111111111111111100000000000000000000000000000000000000000000"].(map[string]any)["heartbeat"]) + +} + +func TestMappingCustomType(t *testing.T) { + m := mapping(map[string]any{}) + data := ` +{ + "foo": 100, + "bar": 100.00, + "baz": { "gnat": 11.10 } +}` + + err := m.UnmarshalJSON([]byte(data)) + require.NoError(t, err) + assert.Equal(t, int64(100), m["foo"], m) + assert.Equal(t, decimal.NewFromFloat(100.00), m["bar"], m) + assert.Equal(t, decimal.NewFromFloat(11.10), m["baz"].(map[string]any)["gnat"], m) +} diff --git a/core/services/workflows/state.go b/core/services/workflows/state.go index f70b4661897..c229b14e1dd 100644 --- a/core/services/workflows/state.go +++ b/core/services/workflows/state.go @@ -216,6 +216,20 @@ func deepMap(input any, transform func(el string) (any, error)) (any, error) { } return nv, nil + case mapping: + // coerce mapping to map[string]any + mp := map[string]any(tv) + + nm := map[string]any{} + for k, v := range mp { + nv, err := deepMap(v, transform) + if err != nil { + return nil, err + } + + nm[k] = nv + } + return nm, nil case map[string]any: nm := map[string]any{} for k, v := range tv { diff --git a/core/services/workflows/testdata/fixtures/workflows/workflow_schema.json b/core/services/workflows/testdata/fixtures/workflows/workflow_schema.json index 04400ce20fc..2cb02c7921d 100644 --- a/core/services/workflows/testdata/fixtures/workflows/workflow_schema.json +++ b/core/services/workflows/testdata/fixtures/workflows/workflow_schema.json @@ -3,6 +3,9 @@ "$id": "https://github.com/smartcontractkit/chainlink/v2/core/services/workflows/workflow-spec-yaml", "$ref": "#/$defs/workflowSpecYaml", "$defs": { + "mapping": { + "type": "object" + }, "stepDefinitionType": { "oneOf": [ { @@ -48,10 +51,10 @@ "pattern": "^[a-z0-9_]+$" }, "inputs": { - "type": "object" + "$ref": "#/$defs/mapping" }, "config": { - "type": "object" + "$ref": "#/$defs/mapping" } }, "additionalProperties": false, @@ -97,4 +100,4 @@ ] } } -} \ No newline at end of file +}