keep nlohmann::json private #371
Conversation
|
@glhewett Could you make a trivial push here so that the full CI runs, now that it's not marked as draft? |
|
@bifurcation - It is not working, yet. I would rather close the PR. You have my permission to close it once you read this. I will make sure we have an issue to track it. |
glhewett
force-pushed
the
glh-private-json
branch
from
f708bc2
to
5c16323
Compare
| address_opt = UserInfoClaimsAddress::from_json( | ||
| cred_subject_json.at(address_attr).dump()); |
There was a problem hiding this comment.
I don't love this back and forth between strings and JSON. Maybe there's some way to have static functions that take json? So that they're hidden from callers but we can still use them.
There was a problem hiding this comment.
This is the difficult thing about this library. If we have any use of our json library in the public api's then the consumer will have to build and link to our json library. If they use another library then they would have to convert. I think you understand that. I will think about the static function that might do the conversion. Out only options in the signature of a function is string or bytes. what do you have in mind?
There was a problem hiding this comment.
What I had in mind was something like:
// Standard from_json method so that `json::get` works
static void from_json(const json& j, UserInfoClaimsAddress& addr) {
// current body of UserInfoClaimsAddress::from_json
}
static void from_json(const json& j, UserInfoClaims& addr) {
// current body of UserInfoClaims::from_json
// including using get_optional<UserInfoClaimsAddress>(...)
}
// String methods just call through to JSON methods
UserInfoClaims
UserInfoClaims::from_json(const std::string& cred_subject) {
return json::parse(cred_subject).get<UserInfoClaims>();
}
UserInfoClaimsAddress
UserInfoClaimsAddress::from_json(const std::string& address) {
return json::parse(address).get<UserInfoClaimsAddress>();
}
Co-authored-by: Richard Barnes <rlb@ipv.sx>
| const auto userinfo_claims = | ||
| UserInfoClaims::from_json(credentialSubject.dump()); |
There was a problem hiding this comment.
Same raw string trick here. (Raw strings can span multiple lines.)
There was a problem hiding this comment.
I am using the original json as the known answer for the tests. The tests in that section seem to only test that the strings get shuffled around correctly.
|
created #393 to optimize implementation |
We want libraries that have dependencies on mlspp to not be forced to download and install nlohmann::json. This gets messy when other json libraries are involved. This PR will remove json libraries from public headers and remove the link dependency for the library.