fix/refactor(broker): Fix JSON generation of returned authentication data (userinfo and message) #43
Conversation
denisonbarbosa
force-pushed
the
fix-json-messaging
branch
from
d65edd6
to
dce370a
Compare
denisonbarbosa
force-pushed
the
fix-json-messaging
branch
from
c0a476b
to
7c832eb
Compare
There was a problem hiding this comment.
Nice work!
I think this is the right approach with the existing code. Thanks for tackling this!
As you saw, I have some improvements requests in the PR, but nothing changing the structure of the overall PR.
Also, shouldn’t we add a test where the error message was not producing a valid json previously on purpose? Because this is what is supposively fixed in the PR :D
...ta/TestFetchUserInfo/golden/successfully_fetch_user_info_with_default_home_when_not_provided
Show resolved
Hide resolved
...icating_with_password_still_allowed_if_token_is_expired_and_server_is_unreachable/first_call
Outdated
Show resolved
Hide resolved
|
I think it would be worth adding a test to ensure we properly do the encoding (something maybe inspired to this, but done with the proper abstraction which at the time I didn't do since it was mostly a PoC). Indeed Maybe also making the tests to-remarshall the output to a native go map, just to ensure that's valid? |
...a/TestIsAuthenticated/golden/authenticating_with_password_refreshes_expired_token/first_call
Outdated
Show resolved
Hide resolved
denisonbarbosa
force-pushed
the
fix-json-messaging
branch
from
18f7cad
to
d2f618e
Compare
Yeah, it wouldn't break anything (as we don't care about the message in the cases that the validation was failing, and unmarshaling an empty string is also fine). However, it's still good to keep things as standardized as possible and ensure we always have a valid JSON-encoded message in non-error cases. |
To prepare for the json marshaling changes, we need to turn the userInfo into its own type and add json annotations to the struct so that it can be marshaled correctly through json.Marshal.
We had some problems when error messages had double quotes in them, as it would break the JSON unmarshaling. To avoid this kind of errors again, we now use separate types for the returned data and marshal them through json.Marshal, which prevents mistakes that could happen when "manually" generating the message.
|
All good here after the final commit :) |
denisonbarbosa
force-pushed
the
fix-json-messaging
branch
from
e3e8306
to
0a6045e
Compare
Some errors were causing our previous strategy of generating the JSON on the go to fail, as some error messages could have double quotes. To avoid this, it's better to use the native marshaling of strings that the
encode/jsonpackage provides.UDENG-3428