-
Notifications
You must be signed in to change notification settings - Fork 75
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
Port db package to Kotlin #532
Conversation
@@ -47,8 +47,10 @@ dependencies { | |||
|
|||
// Unit testing dependencies | |||
testImplementation(project(":test_shared")) | |||
testImplementation(libs.test.robolectric) |
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.
The tests couldn't build on my setup without adding this dependency
@@ -19,7 +19,7 @@ import org.json.JSONObject | |||
/** | |||
* This class is a utils class, mainly used to handle migration when encryption fails or encryption level is changed | |||
*/ | |||
object CryptUtils { | |||
internal object CryptUtils { |
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.
Made internal because I changed the DBAdapter to be internal instead of RestrictTo(Library) annotation and this class exposed DBAdapter. I think it should also probably be internal so I changed it as well.
private var dbAdapter: DBAdapter? = null | ||
|
||
@WorkerThread | ||
@Synchronized |
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.
Made synchronized to avoid double initialization of DBAdapter per DBManager. Could also be implemented with double-checked locking.
Can we also rename the method to something like getDBAdapter(Context), because currently it is used at many places with the semantics of "get" instead of "load"
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.
Does the annotation ensure that syncromisation is done or is it a marker so the implementer handles it?
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.
The annotation is used to mark the JVM generated method as synchronized
(the same as having the synchronized
keyword on a java method).
Update tests of DBManager and DBAdapter Rename QueueCursor to QueueData Port BaseDatabaseManager to Kotlin
Change usage of ArgumentCaptor with DBAdapter to use mockito-kotlin
Port all classes in db package to Kotlin with some minor improvements
Never close of the db connections once opened - up for consideration
Multi-threaded synchronization of the db usage is not modified