Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove usage of json.RawMessage via go-runtime/encoding package #1439

Closed
deniseli opened this issue May 8, 2024 · 0 comments · Fixed by #1454
Closed

Remove usage of json.RawMessage via go-runtime/encoding package #1439

deniseli opened this issue May 8, 2024 · 0 comments · Fixed by #1454
Labels
good first issue Good for newcomers

Comments

@deniseli
Copy link
Contributor

deniseli commented May 8, 2024

json.RawMessage is not a type that FTL supports. The only standard lib type we currently support is time.Time.

However, currently, ingress requests and responses have json.RawMessage fields in their structs. They are configured using the json struct tag to be omitted from the output JSON when undefined.

Now that we have support for omitempty (#1262), we no longer need to type these fields as json.RawMessage. They are encoded as []byte underlying, but byte arrays are serialized to base64 strings by our custom encoder, rather than a human readable value.

I haven't explored this fully to prove this works, but a reasonable option could be to convert the json.RawMessage fields to string, which would be human-readable.

Make sure to remove the special case for json.RawMessage in go-runtime/encoding/encoding.go as a part of this: link

@deniseli deniseli added the good first issue Good for newcomers label May 8, 2024
@github-actions github-actions bot added the triage Issue needs triaging label May 8, 2024
@alecthomas alecthomas mentioned this issue May 8, 2024
@matt2e matt2e removed the triage Issue needs triaging label May 8, 2024
@matt2e matt2e changed the title Remove usage of json.RawMessage Remove usage of json.RawMessage via go-runtime/encoding package May 8, 2024
deniseli added a commit that referenced this issue May 9, 2024
…esponse (#1454)

Fixes #1439

Alec was right - the `[]byte` array was the right solution here! The
response does come back encoded the same way it was when it was typed
`json.RawMessage`. I confirmed in
`backend/controller/ingress/handler_test.go` (which would fail if we
made the `encoding.go` change without the `response.go` change) and also
cleaned up the test a bit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants