-
Notifications
You must be signed in to change notification settings - Fork 30
Add missing apis and some small fixes #166
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
Conversation
RicardRC
commented
May 27, 2025
- Add default .clang-default for project.
- Add delete account api call.
- Add session logout api call.
- Expand session refresh to include vars (this can break existing code. Just pass an empty map as new vars parameter and that's it "{}")
- Fix Android crash on accessing CACertificates without calling loadLibrary.
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.
Pull Request Overview
This PR introduces several new API endpoints and updates along with a new .clang-format file to enforce coding style. Key changes include:
- Added delete account and session logout API calls and updated the session refresh API to include additional variables.
- Fixed an Android crash issue when accessing CA Certificates without calling loadLibrary.
- Added a new .clang-format configuration file to standardize code formatting.
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
test/src/test_authentication.cpp | Reformatted callback implementations and updated session refresh API call. |
satori-cpp/test/main.cpp | Updated API parameter handling and reordered code formatting. |
satori-cpp/src/InternalLowLevelSatoriAPI.cpp | Modified JSON error logging formatting and adjusted time-value parsing. |
impl/wsWslay/WslayIOCurl.h | Improved curl error handling and added a check for CA Certificates on Android. |
impl/httpCurl/NHttpClientLibCurl.cpp | Updated timeout handling and error messages; improved code style consistency. |
core/core-rest/RestClient.h | Added new API definitions for deleteAccount and sessionLogout, and updated existing endpoints. |
core/common/BaseClient.h | Revised function signatures to align with API changes. |
.clang-format | Added new file to enforce coding style standards. |
Files not reviewed (1)
- submodules/private: Language not supported
@RicardRC , could you split it reformat into own PR? and then leave only commits changing something substantial in this PR, otherwise it is very hard to see what is changing. |
Yeah, sorry. Opened a PR in #167 to merge the formatting first, then I'll rebase this one so it only has substantive changes. |
db15f0a
to
9bfb7af
Compare
@redbaron Meat's only thing left now 🙏. |
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.
Pull Request Overview
Adds and expands session-related functionality while fixing platform-specific crashes and updating tests.
- Extend
authenticateRefresh
to accept custom variables and introducesessionLogout
anddeleteAccount
(sync and async). - Update JSON parsing for timestamp fields to strings and default values.
- Add Android CA certificate null-checks to prevent crashes.
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
File | Description |
---|---|
test/src/test_authentication.cpp | Update tests to use new authenticateRefresh signature with vars |
submodules/private | Bump private submodule commit |
satori-cpp/test/main.cpp | Update server key, add message retrieval/update in main test |
satori-cpp/src/InternalLowLevelSatoriAPI.cpp | Parse timestamp fields as strings and set defaults on missing data |
interface/include/nakama-cpp/NClientInterface.h | Add sessionLogout , deleteAccount , and default vars parameter |
impl/wsWslay/WslayIOCurl.h | Add null-check for CA certificates on Android |
impl/httpCurl/NHttpClientLibCurl.cpp | Change timeout unit to seconds and add CA cert null-check |
core/core-rest/RestClient.h | Implement new methods and update authenticateRefresh signature |
core/core-rest/RestClient.cpp | Add sessionLogout , deleteAccount , and extend request bodies |
core/common/BaseClient.h | Add async overloads for new session methods |
core/common/BaseClient.cpp | Implement async sessionLogout and deleteAccount |
Comments suppressed due to low confidence (2)
test/src/test_authentication.cpp:89
- [nitpick] The variable 'vars' shadows another variable in the outer scope, which may lead to confusion; consider renaming one of them for clarity.
NStringMap vars;
satori-cpp/test/main.cpp:73
- The test retrieves messages but does not assert any expected conditions; consider adding assertions to verify that the behavior matches expectations.
Satori::SGetMessageListResponse messages =
* Fix message deserialization bug. * Fix Android crash on accessing CACertificates without calling loadLibrary. * Add missing functions (DeleteUser and Logout) (cherry picked from commit 1b0f4de) # Conflicts: # core/common/BaseClient.h # core/core-rest/RestClient.h # impl/httpCurl/NHttpClientLibCurl.cpp # impl/wsWslay/WslayIOCurl.h # interface/include/nakama-cpp/NClientInterface.h # satori-cpp/src/InternalLowLevelSatoriAPI.cpp # satori-cpp/test/main.cpp # submodules/private # test/src/test_authentication.cpp
* Fix message deserialization bug. * Fix Android crash on accessing CACertificates without calling loadLibrary. * Add missing functions (DeleteUser and Logout)