From 4969eafafc0a1046debd911d8e22da32f7cfd3a8 Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Fri, 27 Oct 2023 16:13:05 +0000 Subject: [PATCH] Less panics, more validation errors for invalid flow defs --- flows/definition/flow.go | 6 ++-- flows/definition/flow_test.go | 12 +++++++ flows/definition/node.go | 5 ++- .../testdata/broken_flows/null_action.json | 31 +++++++++++++++++++ .../testdata/broken_flows/null_exit.json | 24 ++++++++++++++ .../testdata/broken_flows/null_node.json | 24 ++++++++++++++ 6 files changed, 96 insertions(+), 6 deletions(-) create mode 100644 flows/definition/testdata/broken_flows/null_action.json create mode 100644 flows/definition/testdata/broken_flows/null_exit.json create mode 100644 flows/definition/testdata/broken_flows/null_node.json diff --git a/flows/definition/flow.go b/flows/definition/flow.go index 02a91f933..44eafa948 100644 --- a/flows/definition/flow.go +++ b/flows/definition/flow.go @@ -311,12 +311,12 @@ var _ flows.Flow = (*flow)(nil) type flowEnvelope struct { migrations.Header13 - Language i18n.Language `json:"language" validate:"required,language"` - Type flows.FlowType `json:"type" validate:"required,flow_type"` + Language i18n.Language `json:"language" validate:"required,language"` + Type flows.FlowType `json:"type" validate:"required,flow_type"` Revision int `json:"revision"` ExpireAfterMinutes int `json:"expire_after_minutes"` Localization localization `json:"localization"` - Nodes []*node `json:"nodes"` + Nodes []*node `json:"nodes" validate:"dive,required"` UI json.RawMessage `json:"_ui,omitempty"` } diff --git a/flows/definition/flow_test.go b/flows/definition/flow_test.go index 174350eb4..1dc2cf95c 100644 --- a/flows/definition/flow_test.go +++ b/flows/definition/flow_test.go @@ -39,6 +39,18 @@ func TestBrokenFlows(t *testing.T) { path string err string }{ + { + "null_node.json", + "field 'nodes[1]' is required", + }, + { + "null_exit.json", + "unable to read node: field 'exits[1]' is required", + }, + { + "null_action.json", + "unable to read action: field 'type' is required", + }, { "exitless_node.json", "unable to read node: field 'exits' must have a minimum of 1 items", diff --git a/flows/definition/node.go b/flows/definition/node.go index 2c7fe8ed2..6c6a82c96 100644 --- a/flows/definition/node.go +++ b/flows/definition/node.go @@ -12,7 +12,6 @@ import ( "github.com/nyaruka/goflow/flows/inspect" "github.com/nyaruka/goflow/flows/routers" "github.com/nyaruka/goflow/utils" - "github.com/pkg/errors" ) @@ -143,9 +142,9 @@ func (n *node) EnumerateLocalizables(include func(uuids.UUID, string, []string, type nodeEnvelope struct { UUID flows.NodeUUID `json:"uuid" validate:"required,uuid4"` - Actions []json.RawMessage `json:"actions,omitempty"` + Actions []json.RawMessage `json:"actions,omitempty" validate:"dive,required"` Router json.RawMessage `json:"router,omitempty"` - Exits []*exit `json:"exits" validate:"required,min=1"` + Exits []*exit `json:"exits" validate:"required,min=1,dive,required"` } // UnmarshalJSON unmarshals a flow node from the given JSON diff --git a/flows/definition/testdata/broken_flows/null_action.json b/flows/definition/testdata/broken_flows/null_action.json new file mode 100644 index 000000000..e2b76a0df --- /dev/null +++ b/flows/definition/testdata/broken_flows/null_action.json @@ -0,0 +1,31 @@ +{ + "flows": [ + { + "uuid": "76f0a02f-3b75-4b86-9064-e9195e1b3a02", + "name": "Test Flow", + "spec_version": "13.2.0", + "language": "eng", + "type": "messaging", + "nodes": [ + { + "uuid": "a58be63b-907d-4a1a-856b-0bb5579d7507", + "actions": [ + { + "uuid": "ad154980-7bf7-4ab8-8728-545fd6378912", + "type": "send_msg", + "text": "Hi there @contact.name" + }, + null + ], + "exits": [ + { + "uuid": "37d8813f-1402-4ad2-9cc2-e9054a96525b", + "name": "Default", + "destination_uuid": null + } + ] + } + ] + } + ] +} \ No newline at end of file diff --git a/flows/definition/testdata/broken_flows/null_exit.json b/flows/definition/testdata/broken_flows/null_exit.json new file mode 100644 index 000000000..a22204e41 --- /dev/null +++ b/flows/definition/testdata/broken_flows/null_exit.json @@ -0,0 +1,24 @@ +{ + "flows": [ + { + "uuid": "76f0a02f-3b75-4b86-9064-e9195e1b3a02", + "name": "Test Flow", + "spec_version": "13.2.0", + "language": "eng", + "type": "messaging", + "nodes": [ + { + "uuid": "a58be63b-907d-4a1a-856b-0bb5579d7507", + "exits": [ + { + "uuid": "37d8813f-1402-4ad2-9cc2-e9054a96525b", + "name": "Default", + "destination_uuid": null + }, + null + ] + } + ] + } + ] +} \ No newline at end of file diff --git a/flows/definition/testdata/broken_flows/null_node.json b/flows/definition/testdata/broken_flows/null_node.json new file mode 100644 index 000000000..ba9745dcf --- /dev/null +++ b/flows/definition/testdata/broken_flows/null_node.json @@ -0,0 +1,24 @@ +{ + "flows": [ + { + "uuid": "76f0a02f-3b75-4b86-9064-e9195e1b3a02", + "name": "Test Flow", + "spec_version": "13.2.0", + "language": "eng", + "type": "messaging", + "nodes": [ + { + "uuid": "a58be63b-907d-4a1a-856b-0bb5579d7507", + "exits": [ + { + "uuid": "37d8813f-1402-4ad2-9cc2-e9054a96525b", + "name": "Default", + "destination_uuid": null + } + ] + }, + null + ] + } + ] +} \ No newline at end of file