Paths

Table of Contentst

Differential D14748

[web][native] create two `SQLiteQueryExecutor` instances
ClosedPublic
Actions

Authored by kamil on May 22 2025, 10:20 AM.

Tags

None

Referenced Files

None

Subscribers

Details

Reviewers

bartek
tomek
angelika

Commits

rCOMM9250ba026216: [web][native] create two `SQLiteQueryExecutor` instances

Summary

Creating two instances and updating all the code around.

I was having some issues with undefined symbol on native when using default arguments, so I decided to overload the method.

Created ENG-10786 to track the fact that database is not encrypted

Depends on D14746

Test Plan

Tested in D14752

Diff Detail

Repository

Branch

land

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

kamil created this revision.May 22 2025, 10:20 AM

kamil held this revision as a draft.

Herald added a subscriber: ashoat. · View Herald TranscriptMay 22 2025, 10:20 AM

kamil added a child revision: D14749: [web] bind `SQLiteQueryExecutor::migrate`.May 22 2025, 10:23 AM

Harbormaster failed remote builds in B34230: Diff 48165!May 22 2025, 10:47 AM

rebase

Harbormaster completed remote builds in B34237: Diff 48173.May 22 2025, 1:19 PM

kamil published this revision for review.May 22 2025, 2:56 PM

kamil edited the summary of this revision. (Show Details)

kamil added inline comments.

native/cpp/CommonCpp/DatabaseManagers/DatabaseManager.h
22–25 ↗	(On Diff #48173)	We could either implement ENG-10786 or do what I suggested in the Summary of D14734 (about renaming).

bartek accepted this revision.May 27 2025, 8:49 AM

bartek added inline comments.

native/cpp/CommonCpp/DatabaseManagers/DatabaseManager.h
22–25 ↗	(On Diff #48173)	I can't find anything about renaming in D14734. I guess you mean D14739. In this case, renaming to "unencrypted" makes sense. I'm a little worried that implementing ENG-10786 would make backup key management more complex. But the backup database is only here for restoration so perhaps it's as hard as I think
web/shared-worker/types/sqlite-query-executor.js
54 ↗	(On Diff #48173)	nit: this needs a rebase

This revision is now accepted and ready to land.May 27 2025, 8:49 AM

kamil edited the test plan for this revision. (Show Details)May 27 2025, 8:57 AM

kamil added inline comments.May 28 2025, 8:26 AM

native/cpp/CommonCpp/DatabaseManagers/DatabaseManager.h
22–25 ↗	(On Diff #48173)	I can't find anything about renaming in D14734. I guess you mean D14739. Yes, you're correct, thanks In this case, renaming to "unencrypted" makes sense. I'm a little worried that implementing ENG-10786 would make backup key management more complex. But the backup database is only here for restoration so perhaps it's as hard as I think I keep it as it is for now, and consider this again after ENG-10786 is done

rebase

Harbormaster completed remote builds in B34287: Diff 48283.May 28 2025, 8:46 AM

Closed by commit rCOMM9250ba026216: [web][native] create two `SQLiteQueryExecutor` instances. · Explain WhyMay 28 2025, 9:49 AM

This revision was automatically updated to reflect the committed changes.

kamil added a commit: rCOMM9250ba026216: [web][native] create two `SQLiteQueryExecutor` instances.

Revision Contents
Changeset List

Path

Size

native/

cpp/

CommonCpp/

DatabaseManagers/

DatabaseManager.h

8 lines

DatabaseManager.cpp

22 lines

NativeModules/

PersistentStorageUtilities/

BackupOperationsUtilities/

BackupOperationsExecutor.cpp

3 lines

web/

shared-worker/

worker/

19 lines

worker-database.js

27 lines

Diff 48283

native/cpp/CommonCpp/DatabaseManagers/DatabaseManager.h

Loading...

native/cpp/CommonCpp/DatabaseManagers/DatabaseManager.cpp

Loading...

native/cpp/CommonCpp/NativeModules/PersistentStorageUtilities/BackupOperationsUtilities/BackupOperationsExecutor.cpp

Loading...

web/shared-worker/worker/backup.js

Loading...

web/shared-worker/worker/worker-database.js

Loading...