Correctly send empty array to astarte #247
Open
+32
−21
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
eddbbt
changed the title
eddbbt
force-pushed
the
fix_empty_array_parser
branch
2 times, most recently
from
a6bd1a6
to
f03db4a
Compare
eddbbt
force-pushed
the
fix_empty_array_parser
branch
from
f03db4a
to
5e717c4
Compare
Add exception for enabling empty array sending (that differs from arrays with one null element) An empty array with corrrect type is now sent to astarte Signed-off-by: Eddy Babetto <eddy.babetto@secomind.com>
eddbbt
force-pushed
the
fix_empty_array_parser
branch
from
5e717c4
to
912a37d
Compare
Annopaolo
requested changes
May 6, 2024
Comment on lines
+1640
to
+1649
| if payload == "[]" { | ||
| ret = make([]interface{}, 0) | ||
|
|
||
| jsonOut := make([]interface{}, len(payload_parsed)) | ||
| for i, v := range payload_parsed { | ||
| jsonOut[i] = v | ||
| } | ||
| } else { | ||
|
|
||
| retArray := []interface{}{} | ||
| // Do a smarter conversion here. | ||
| for _, v := range jsonOut { | ||
| switch val := v.(type) { | ||
| case string: | ||
| p, err := parseSendDataPayload(strings.TrimSpace(val), interfaces.AstarteMappingType(strings.TrimSuffix(string(mappingType), "array"))) | ||
| if err != nil { | ||
| return nil, err | ||
| //wait it's all string? | ||
| payload = strings.Replace(payload, "[", "", -1) | ||
| payload = strings.Replace(payload, "]", "", -1) | ||
| payload_parsed := strings.Split(payload, ",") | ||
| //always has been |
There was a problem hiding this comment.
I would suggest using a different approach for parsing the array data. We know that:
- The input payload has the form
[<comma separated data>] - A
[<comma separated data>]value is a valid JSON object - Go's
json.Unmarshalis able to handle these objects (see https://go.dev/play/p/ixaicx2S7cj).
Therefore, my suggestion is to let Go handle characters such as [ and just do the conversion of payloads, something like:
var jsonPayload []interface{}
if err := json.Unmarshal([]byte(payload), &jsonPayload); err != nil {
fmt.Println(err) //Maybe also warn the user of the bad input payload
System.exit(1)
}
retArray := []interface{}{}
// Do a smarter conversion here.
for _, v := range jsonPayload {
// etc etc, the rest is the sameThis would mean that the user will have to write a valid JSON payload, e.g. strings inside arrays should be put in double quotes, like astartectl appengine devices publish-datastream <device_id> <interface> <path> '["a", "b", "c"]'
There was a problem hiding this comment.
sounds good, but we need to check that requiring an array of explicit strings ('["a", "b", "c"]') will not break compatibility with already existent implementations (if any).
Current string interpolation was added in #235
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix send of empty array to astarte, that defaulted to empty string that was obviously not the correct payload.
Now it is sent as an empty array.
Please note that an empty array differs from an array with one element of value "empty" or zero, that's why i did not edit the existent parser solution but added another one instead