-
Notifications
You must be signed in to change notification settings - Fork 150
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
[bindgen] Adding a app_user_as_sync_user
helper
#7684
Conversation
41b53d3
to
41639c9
Compare
cppName: app::User | ||
sharedPtrWrapped: SharedUser | ||
properties: | ||
is_logged_in: bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved this to the SyncUser
as this is where it's actually implemented:
realm-core/src/realm/object-store/sync/sync_user.hpp
Lines 47 to 50 in a45922f
bool is_logged_in() const | |
{ | |
return state() == State::LoggedIn; | |
} |
|
||
User: | ||
base: SyncUser |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the declaration of this inheritance, because the SDK emits class User extends SyncUser
and I saw assertion failures from the User
being passed as a SyncUser
into the binding, which just interpreted the shared pointer without performing the downcast.
The line giving resulting in assertion failures (the bindingSyncConfig.user
is a JS wrapper of app::User
but it expects a wrapper of a SyncUser
): https://github.com/realm/realm-js/blob/bf7c0c87c460e4adb3cada31df73b74bfa7f1cfc/packages/realm/src/Realm.ts#L384
This happenes because internal.user
is User
which extends SyncUser
and is therefore a valid value for the user
here: https://github.com/realm/realm-js/blob/bf7c0c87c460e4adb3cada31df73b74bfa7f1cfc/packages/realm/src/app-services/SyncConfiguration.ts#L341
Pull Request Test Coverage Report for Build kraen.hansen_71Details
💛 - Coveralls |
Co-authored-by: Kenneth Geisshirt <kenneth.geisshirt@mongodb.com>
Merging before CI is green, as the previous commit was green and this last change was to the changelog 🤞 |
* Adding app_user_as_sync_user * Update CHANGELOG.md Co-authored-by: Kenneth Geisshirt <kenneth.geisshirt@mongodb.com> --------- Co-authored-by: Kenneth Geisshirt <kenneth.geisshirt@mongodb.com>
What, How & Why?
This introduce a
app_user_as_sync_user
helper, which can be used by the SDK to downcast anapp::User
to aSyncUser
.☑️ ToDos
bindgen/spec.yml
, if public C++ API changed