This differential uses C++ functionality implemented in previous diffs to update SQLite directly in response to notifications. SQLite database is update if and only if database has been already created by the app (SQLite file exists)
Details
Details
In XCode debugger examine Message and Media database entities created from raw notification JSON string by translation functionality written in C++. Ensure also that SQLiteQueryExecutor::replaceMessage() method gets called. execute this process for all message types we define in Comm that hae generate_notifs flag set.
Comment out set_encryption_key method in SQLiteQueryExecutor (to disable encryption) code build app on an emulator, launch it and immediately kill. Then send a message from another client. Download comm.sqlite database from XCode. Examine its content in some sqlite explorer (vscode has a pretty good one) to ensure that messages are there even though app was not launched after sending message.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| native/ios/NotificationService/NotificationService.mm | ||
|---|---|---|
| 32 ↗ | (On Diff #12885) | comm::SQLiteQueryExecutor::initialize(sqliteFilePathStdStr) is implemented with std::call_once semantics, so multiple call to initialization method do not perform unnecessary operations. On the other hand I understand it might be confusing. Probably we could extend SQLiteQueryExecutor API to check if it is already initialized or add a flag in NotificationService. What do you think? |
| 36–38 ↗ | (On Diff #12885) | Ok, I will remove this log. |
| native/ios/NotificationService/NotificationService.mm | ||
|---|---|---|
| 34–36 ↗ | (On Diff #12942) | We're checking if the notification contains message infos, and not the existence of the database. |
| 32 ↗ | (On Diff #12885) | I wouldn't change the API - it is really convenient. But what do you think about changing the name to e.g. initializeIfNecessary? |
| native/ios/NotificationService/NotificationService.mm | ||
|---|---|---|
| 34–36 ↗ | (On Diff #12942) | Actually this if statement checks whether database exists. The first if statement checks whether notification contains message info and database exists. If the condition fails in the first if then it means either message does not contain messageInfos or the database does not exists or both. Then the second if is executed and if the message contains messageInfos key the it means previous if statement failed because database does not exists. I understand it is not that obvious at the first sight but it optimizes indenation. |
| native/ios/NotificationService/NotificationService.mm | ||
|---|---|---|
| 34–36 ↗ | (On Diff #12942) | Perhaps I should think about implementing it in a way that is more self-explanatory. @palys-swm let me know what do you think. |
| native/ios/NotificationService/NotificationService.mm | ||
|---|---|---|
| 34–36 ↗ | (On Diff #12942) | Ah, that makes sense. For me decreasing the indentation is a really useful tool when improving the readability. So the goal is readability and when deeper indentation produces more readable code, it should be preferred. In this case nested ifs would be more readable for me, but we have a strong preference for decreasing the indentation, so let's leave it. |