Make generated jwt attributes in a predictable order

[liga-oz]

Follow up PR for the #1338

[finkmanAtSap]

I don't really like the approach to change internals of JSONObject via reflection. Are there no clean alternatives?

What problem does this PR address that PR #1338 does not? Is it an alternative or are both PRs required?

[liga-oz]

I don't really like the approach to change internals of JSONObject via reflection. Are there no clean alternatives?

What problem does this PR address that PR #1338 does not? Is it an alternative or are both PRs required?

@finkmanAtSap Even thou it's a reflection I found it to be a cleaner approach than storing JSONObject under an ArrayList then sorting it and then doing some string manipulations. We could discuss here the performance gains too, but i dont thinks it plays a critical role here. PR #1338 is an alternative.

[finkmanAtSap]

I'm not sure if we are solving the right problem after thinking some about it:
JSONObject is order-agnostic because it claims this is a fundamental property of JSON.
Shouldn't then AuthenticationToken.equals also return true for two tokens who are equal except for the order of their JSON properties? I think the equals implementation of our Token interface returns false in this case which might be questionable. I don't see any other way why AuthenticationToken.equals would return false.

With reflection, we are introducing something that could cause more problems than this solves in my opinion.
The failing test itself has limited value IMO if it requires the exact same token strings for two instances to be equal. There are no sources of randomness inside the logic, so I am not sure why it would ever produce different results for the same token.

Yet, we are trying really hard to produce JWTs whose String representation is identical for the same properties. If we decide that we really do need that, I think we should use a LinkedHashMap/TreeMap to store the properties instead of a JSONObject and then use a JSON serializer that produces consistent results. JSONObject tells us it is not the right implementation to produce consistent JSONs.

I think performance is very negligible here because it is just unit tests whose runtimes are dominated by other parts of the code. The objects in question are tiny data structures that are only sorted for token creation.

But I understand we do not want to put too much effort on this and the reflection solution is giving us just that. I'm just scared of yet another consumer complaining because our test modules break their unit tests at one point.

[finkmanAtSap]

Approving since it seems like the smallest change to fix it and the catch clauses should prevent it from causing any issues