Page MenuHomePhabricator

[native] refactor `migrate()` method to static member of `SQLiteQueryExecutor`
ClosedPublic

Authored by kamil on Dec 22 2022, 2:31 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Apr 23, 12:04 AM
Unknown Object (File)
Wed, Apr 3, 2:28 AM
Unknown Object (File)
Wed, Apr 3, 2:28 AM
Unknown Object (File)
Wed, Apr 3, 2:28 AM
Unknown Object (File)
Wed, Apr 3, 2:28 AM
Unknown Object (File)
Thu, Mar 28, 5:01 PM
Unknown Object (File)
Thu, Mar 28, 5:01 PM
Unknown Object (File)
Thu, Mar 28, 5:00 PM
Subscribers

Details

Summary

Motivation:

  1. Migration process is not connected to query executor instance, but to database (specific one, SQLite in this case) in general.
  2. In next diff from the stack we will need to make clearSensitiveData also static and we will not be able to use this->migrate() there.
Test Plan

Build the app and run migrations, there should behave the same way as previously

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kamil held this revision as a draft.
kamil published this revision for review.Dec 22 2022, 3:30 AM
marcin added a reviewer: tomek.

The fact that we can do that highlights an issue with design of this class. There's no state associated with instance of the class, so each method could become static. The only remnant of object oriented approach is the use of a constructor.

I don't think we have to address this soon, or even at all, but a better approach would be to have encryption key and path as instance variables. Because now, this object isn't a singleton, but behaves like one.

This revision is now accepted and ready to land.Dec 23 2022, 4:51 AM