FR: core nodes to permit input and output to properties other than the hard coded payload

[Steve-Mcl]

Discussed and triaged with @dceejay but again, i am failing to find the supporting information. Dave listed out all the nodes where this made sense. I will update this once I locate the preparations/supporting discussion.

The PR proposes that, where it makes sense, we permit the input and output property to be user configurable.

It was determined this would be strictly a msg.xyz only option as this simplifies implementation and promotes the use of msg properties (over context or other potentially mutating values)

The aim is to reduce friction when flows are made in a serial fashion whereby users are not aware they can move or copy the payload to an alternate msg property (using change nodes). The first instinct many users seem to apply is to store the payload in context and then pick this up at the end of the flow. This leads to subtle timing bugs when messages flow in quick succession and async operations are involved.

[Steve-Mcl]

adapted from google Doc shared by @dceejay for markdown

Adding configurable output field to core nodes

D-C-J, SMcL

This is a quick think-list about the core nodes that could possibly benefit from having a configurable output property - rather than always setting msg.payload. This is to allow more flexibility so as not to overwrite an existing msg property. Rather than “All core nodes” this is an attempt to try to simplify the task and see if it’s just a few problem nodes or something more generic is required.

Basic Assumption

It only really needs to apply to nodes that have an input and output, as
a, this about outputs which input only nodes don’t have, and
b, output only nodes are at the start of a flow so you can’t overwrite an existing msg property (as there isn’t an existing msg).

This reduces the list to 22 possible nodes out of 43 core nodes we install. So by no means all, so no need for some generic solution.

List of core nodes with input and output

Node	_
link call	not needed - handles complete msg already.
function	not needed - handles complete msg already.
switch	not needed - no modification of msg
change	not needed - already moves, set any required property
range	possible
template	already does allow setting
delay	not needed - no modification of msg
trigger	possible - would be really complex ui as it potentially has multiple outputs and would each require an optional property
exec	possible
filter/rbe	not needed - not sure it makes sense though as you are filtering a particular property - so wouldn't you want that back modified ?
http request	probable - I can see utility here when chaining
tcp request	possible
split	unlikely - if anything this needs the input handling instead - currently it on only splits the payload. (in which case should that property be passed as part of msg.parts so the auto join can rebuild correctly ?)
join	possible
sort	not needed ? - it sorts a specific property so you would that to be returned sorted.
batch	not needed - just batches up msgs, no modification of msg
csv	potentially - but possible v4 rewrite anyway
html	already can be set
xml	possible
yaml	possible
write file	not needed - no modification of msg
read file	possible ? - to stop overwriting of incoming payload but node doesn’t require payload so … ?

Conclusion

Out of 22 I/O nodes I think 1 definite candidate (http request), 8 would possibly make sense, 9 don’t need modification, 2 potentially, and 2 allow setting already. So 11 don’t need modification, Of the remaining 11

Of these I think the higher priority ones are http request, exec, join, and then xml, json and yaml (I.E. the parsers - with CSV being fixed as part of making it RFC compliant.

Finally we should possibly let the split node specify the input property to split on

Other Thoughts

• what should this property be called / labelled… Ideally it would be something like “Output Property” but that is too wide for the default margin… Can we set the default label margin FOR ALL NODES to be 120px rather than 100 - i.e. a global css change to .red-ui-editor .form-row label, .red-ui-editor-dialog .form-row label
• and I think it should be placed as far down the node as makes sense (after all other options)
• likewise rename any nodes which currently have “property” label as “Input Property”
• and place input property as far up as makes sense.
• And as part of general tidy up… we should probably move the “Name” field to top of ALL NODES.

So a general node would be like

Name	FooNode
Input Property	msg.payload
Option ip	127.0.0.1
More options	> on all messages
Output Property	msg.payload

Migration consideration

Currently several nodes already allow setting the input property… and if you do so they use that as the output property also… so we need to think about migration… I think two options

1. If not set then use the input one for output (ie allow output to be left blank)
2. If not set the Copy input to output and from then on use the output one (ie output now mandatory)

In both cases default for input would be payload - and thus so would output.
I think 2 would end up being more consistent with any “new” nodes that do decide to add in and out as they would be totally separate. But can also see reasons for 1.

[Steve-Mcl]

A Draft PR implementing the majority of Dave's suggestions was raised in #4656

Below are screenshots of before and after for evaluation and agreement before I proceed with tests and doc updates.

Notes:

• Split, Join, trigger, Exec, TCP/UDP nodes were not addressed in this initial offering as they have complicated relationships (streaming etc) that can be addressed at a later time (if at all)
• Naming the "Property": I went with "Property In" and "Property Out".
  • Other considerations made were:
    • "Input Data" and "Output Data"
    • "Payload In" and "Payload Out"
    • Please feel free to offer up more suitable suggestions

CSV

1. Move name to top
2. Add property in
3. Add property out

JSON

1. Move name to top
2. Add property in
3. Add property out

YAML

1. Move name to top
2. Add property in
3. Add property out

XML

1. Move name to top
2. Add property in
3. Add property out

RANGE

1. Move name to top
2. Add property in
3. Add property out

FILE

1. Move name to top
2. Add property out

HTTP REQUEST

1. Move name to top
2. Add property in
3. Add property out

TEMPLATE

1. Move Property Out to bottom of form
   2. Update icon to match other nodes
   3. Update label to clarify what it is for

[Steve-Mcl]

Added/updated unit test specific to these changes

  range Node
    ✔ should load some defaults
    ✔ uses configured property to get input value and propertyOut to set output value

  template node
    ✔ should be loaded (44ms)

  HTTP Request Node
    request
      ✔ should be loaded (186ms)
      ✔ should get using message properties specified for `property in` and `property out` (43ms)

  CSV node (Legacy Mode)
    ✔ should be loaded with defaults
    csv to json
      ✔ should use message properties specified for `property in` and `property out`
    json object to csv
      ✔ should use message properties specified for `property in` and `property out`

  CSV node (RFC Mode)
    ✔ should be loaded with defaults
    csv to json
      ✔ should use message properties specified for `property in` and `property out`
    json object to csv
      ✔ should use message properties specified for `property in` and `property out`

  JSON node
    ✔ should be loaded with defaults (183ms)
    ✔ should convert a valid json string to a javascript object using message properties specified for `property in` and `property out`
    ✔ should convert a javascript object to a json string using message properties specified for `property in` and `property out`
    ✔ should pass through if property specified for `property in` is missing

  XML node
    ✔ should load with defaults (63ms)
    ✔ should convert a valid xml string to a javascript object using message properties specified for `property in` and `property out`
    ✔ should convert a javascript object to an xml string using message properties specified for `property in` and `property out`

  YAML node
    ✔ should load with defaults
    ✔ should convert a valid yaml string to a javascript object using message properties specified for `property in` and `property out`
    ✔ should convert a javascript object to a yaml string using message properties specified for `property in` and `property out`

  file Nodes
    file in Node
      ✔ should load with defaults
      ✔ should read in a file and output to the message property specified in `propertyOut`

[dceejay]

Looking good so far ! Great work

[knolleary]

I'm not yet won over on this.

For some nodes, I can see the benefit of providing more flexibility of what message property they operate on - such as the parsers and range nodes. The template node - I see the benefit, at the cost of reducing the screen space of the main part of the editor dialog (always been wary of adding too many options for that node).

For others... it feels like we're adding more clutter to the node UI to cater for some edge cases. I can understand the desire to have more flexibility and consistency... just want to be careful we don't make it more complicated.

Some stick out to me more than others. For example, the HTTP Request node. In that instance, we already have a field labeled 'payload' (only if the method is GET) which configures what to do with the incoming message payload. Given it now may not be payload, that field makes less sense to be presented in that way. I personally think that's a good example of a node that should not have the property in field at the top - as its a choice you make after you've configured the method and URL. Likewise the 'return' field at the bottom - feels more logic to set what property you want to use and then the format to be used.

This is why I don't think we should have an absolutely rigid approach. It needs to be what makes sense for the individual node.

I'll have more of a play and try to come up with some more constructive suggestions for moving forward on it. We may want to reduce the initial scope rather than try to solve them all in one go.