Status Update
Comments
po...@google.com <po...@google.com>
po...@google.com <po...@google.com> #2
ch...@google.com <ch...@google.com>
ch...@google.com <ch...@google.com> #3
Thanks. Let us know when you have some API design (not looking for a doc, just commenting here is also fine, whichever is easier for you).
One tricky bit about this one is the "transactionality" of it.
e.g. we need to handle the case where we called onPopuplated but application crashed before onPopulated
could succeed. We need to re-run it in the next run.
We might do this by keeping a record in room_master_table
(there might be a better option, this is the first thing that comes to my mind)
po...@google.com <po...@google.com> #4
My idea is to:
1. Add RoomDatabase.Callback#onPopulated method
2. SQLiteCopyOpenHelper already has access to a list of callbacks (List<RoomDatabase.Callback>) through mDatabaseConfiguration.callbacks
3. Modify getWritableDatabase() and getReadableDatabase() so that after the file is copied I can iterate over those callbacks and call onPopulated. I would pass mDelegate.getWritableDatabase() and mDelegate.getReadableDatabase() as a parameter to onPopulated.
4. Add tests in DatabaseCallbackTest
I asked Danny about it and he said that I might need to temporarily open the DB with a vanilla open helper just to invoke onPopulated close it and then let Room re-open it through the normal path. Not sure how would I do that yet so I'm still reading and trying to get better understanding of the code.
This probably also doesn't take this "transactionality" thing into account so I need to look into it. I think I'll try to create some tests that will help me verify if it works or not.
I'm also wondering if there is a way to build the lib locally so I can test it on a sample project. Can I somehow publish it to local maven or something?
ch...@google.com <ch...@google.com> #5
For transactionality, we do actually have a table called `room_master_table` that we can use. I made a mistake on the design of that table by not having proper keys :facepalm: instead it only has 1 id column. It is possible to create a migration for it within room but for this task, we can just add another magic number (we use 42 for identity hash) to check if we did successfully call onPopuplated (and also, we need to call on populated as we propbably don't want to call it if it is not created from assets).
For testing, TestApp is probably sufficient.
If you want to build a maven repo, you can execute:
```
cd room && DIST_DIR=$(pwd)/../repo ./gradlew createDiffArchiveForAndroidxRoom
```
This is a special task that checks the versions in
ap...@google.com <ap...@google.com> #6
When it comes to naming, I was considering onPopulatedFromAssets but there is also createFromFile method in Room database builder so I think we should use something more generic. What do you think?
ap...@google.com <ap...@google.com> #7
yea makes sense. Lets go with your first suggestion for now, maybe Dany will have a better idea.
ch...@google.com <ch...@google.com> #8
I think onPopulated
is a good starting point. Maybe a more specific name could be onOpenPrepackagedDatabase
, we mention 'pre-package databases' a few in the documentation so maybe we can use that same term.
Meanwhile, let me clarify what I meant by 'opening the DB with a vanilla helper'. In SQLiteCopyOpenHelper
once delegate.getWritableDatabase()
is invoked it will actually open the DB and go through all the onCreate
, migrations and onOpen
callbacks which means if you add logic to call onPopulated
it would have already been too late. My feeling is onPopulated
should be called around the same time as onCreate
, before migrations and before onOpen
so that users get a chance to modify the pre-package DB, possibly making schemas changes to comply with Room's validation. Therefore, I think we need to temporarily open the DB just to pass it so onPopulated
, we can do this with a new FrameworkSQLiteOpenHelper
, then close the DB and let the delegate open it, which would go through the rest of Room's flow. Hope this makes sense!
ch...@google.com <ch...@google.com> #9
do you mean doing it in the copying helper? Maybe even we can let createFromAsset
builder methods to receive a callback which includes onPopulated?
ch...@google.com <ch...@google.com> #10
Yeah, doing it on copying helper, it's the most accurate place where we know a pre-packaged DB was used / copied. The callback itself can also be receive in createFromAsset
/ createFromFile
, or just a new callback in RoomDatabase.Callback
, I still think the tricky part is giving a DB in the callback itself (onPopulated(SuppoertSqliteDatabase db)
) which requires opening the DB in the middle of the 'opening sequence'.
Description
Component used: androidx.compose.ui:ui Version used: 1.0.0-beta03 Devices/Android versions reproduced on:
The camera image appears stretched and only one half is shown. Note that the view occupies the expected amount of space, so maybe the bug lies within cameraX. Works fine in landscape orientation.
Sample project and screenshot are attached.