Code for native ops
Depends on D10776
Details
Details
- Reviewers
inka marcin - Commits
- rCOMM9885469a8000: [SQLite] implement C++ code to get keyservers
Call this code:
let data = await commCoreModule.getClientDBStore();
console.log(data.keyservers);
await commCoreModule.processKeyserverStoreOperations([
{
type: 'replace_keyserver',
payload: {
id: '1',
keyserverInfo: 'fdfd',
},
},
{
type: 'replace_keyserver',
payload: {
id: '2',
keyserverInfo: 'fdfd',
},
},
]);
data = await commCoreModule.getClientDBStore();
console.log(data.keyservers);
await commCoreModule.processKeyserverStoreOperations([
{
type: 'replace_keyserver',
payload: {
id: '2',
keyserverInfo: 'fdfd45454545',
},
},
]);
data = await commCoreModule.getClientDBStore();
console.log(data.keyservers);
await commCoreModule.processKeyserverStoreOperations([
{
type: 'remove_keyserver',
payload: {
id: '1',
},
},
]);
data = await commCoreModule.getClientDBStore();
console.log(data.keyservers);
await commCoreModule.processKeyserverStoreOperations([
{
type: 'remove_all_keyservers',
},
]);
data = await commCoreModule.getClientDBStore();
console.log(data.keyservers);Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| web/database/worker/process-operations.js | ||
|---|---|---|
| 161 ↗ | (On Diff #35956) | A bit confused about this function and its name. There appears to be another getClientStore in web/database/utils/store.js. What's the difference between the two? Should one be renamed? Also wondering why this function returns a ClientDBStore with threads, drafts, and reports populated, but the other fields are empty. |
Comment Actions
We discussed this offline and agreed that it would be good if someone else had a look as well - I don't feel confident reviewing this code.
| web/database/worker/process-operations.js | ||
|---|---|---|
| 161 ↗ | (On Diff #35956) |
One is executed on the main thread, and the second is executed on a worker. I could rename this but I think that directory path is enough to distinguish.
Other fields are empty because we don't support messages and users on the web (users are currently in the testing process).
That's correct |
| web/database/worker/process-operations.js | ||
|---|---|---|
| 161 ↗ | (On Diff #35956) |
If they weren't being exported I think it'd be fine, but since they're both exported I think it would be good to change both of their names to be different (and more specific) |
| web/database/worker/process-operations.js | ||
|---|---|---|
| 161 ↗ | (On Diff #35956) | makes sense, I'll do that in a separate diff |
| native/cpp/CommonCpp/NativeModules/PersistentStorageUtilities/DataStores/KeyserverStore.cpp | ||
|---|---|---|
| 26 ↗ | (On Diff #35956) | As far as I understand for all store types we parse JSON in C++ so that returned ClientDB<...>Info is a proper JS object. Here we just return stringified JSON to the JS and probably expect that parsing will be done on JS side. Is there a string reason not to treat keyserver store in the same manner as other store types? |
| native/cpp/CommonCpp/NativeModules/PersistentStorageUtilities/DataStores/KeyserverStore.cpp | ||
|---|---|---|
| 26 ↗ | (On Diff #35956) |
I don't think we do that. We always return data that align with the database structure and do parsing on the JS side: |
| web/database/worker/process-operations.js | ||
| 161 ↗ | (On Diff #35956) | |
| native/cpp/CommonCpp/NativeModules/PersistentStorageUtilities/DataStores/KeyserverStore.cpp | ||
|---|---|---|
| 26 ↗ | (On Diff #35956) | Parsing in C++ is done for MessageStore, DraftStore and ThreadStore. But on the other hand it is not done in C++ for ReportStore and UserStore so I was wrong telling that KeyserverStore breaks the pattern - it has been broken since ReportStore and UserStore (not sure which one was first). We might consider to unify approach in future but this is out of the scope of this diff. |
| native/cpp/CommonCpp/NativeModules/PersistentStorageUtilities/DataStores/KeyserverStore.cpp | ||
|---|---|---|
| 26 ↗ | (On Diff #35956) |
Could link where or what parsing you mean? For ThreadStore we map the database result for the JSI object (exactly the same as here), and if we have stringified JSON somewhere, like for avatar we pass it as string and parse in JS. I cannot find any JSON parsing for either of these stores in C++ (other than some utils notif function linked in comment above) - but maybe I am missing something obvious. |