Fixed
Status Update
Comments
yb...@google.com <yb...@google.com> #2
*Room, not Realm haha
wo...@gmail.com <wo...@gmail.com> #3
i'm still skeptical about changing the default to crash because I'm worried that some apps won't handle migrating from older versions properly.
But creating a builder option totally makes sense so that more avare developers can turn it on.
Btw, another nice thing about drop by default is the development time. during development, you can easily increment it and pass the check and then just provide the migration from latest release to current release when shipping the app.
But creating a builder option totally makes sense so that more avare developers can turn it on.
Btw, another nice thing about drop by default is the development time. during development, you can easily increment it and pass the check and then just provide the migration from latest release to current release when shipping the app.
wo...@gmail.com <wo...@gmail.com> #4
If some apps won't handle migrating and it is convenient for them to drop user data, it should not become a problem for those who respect user data. Respecting user data is more important that convenience for *some* lazy devs who can't write JUST ONE LINE to drop tables.
mm...@commonsware.com <mm...@commonsware.com> #5
Dropping a user data is not always a bad thing, even for offline first apps.
Usually, local modifications are stored in a separate queue that is independent from the data in the db (mostly because data might be overridden).
So even if the data is nuked, local modifications can be restored. It is a fair compromise. In the common case scenario, the device probably had enough time to run any pending jobs before the app is updated so it is rarely a case where app is updated but it had local modifications. Even in that case, data is not lost since the modifications stay in a separate queue.
Worst case scenario of someone failing to write a migration and the app crash looping on the user's device is really bad. It is actually very easy to end up in that situation if the developer simply uninstalls the app to get around the migration because they were focused on solving another problem.
I think the best action for us is to add a "forceMigrations" method into the builder that will always enforce migrations. This way, more aware developers can turn it on (they are also more likely to read the docs).
Usually, local modifications are stored in a separate queue that is independent from the data in the db (mostly because data might be overridden).
So even if the data is nuked, local modifications can be restored. It is a fair compromise. In the common case scenario, the device probably had enough time to run any pending jobs before the app is updated so it is rarely a case where app is updated but it had local modifications. Even in that case, data is not lost since the modifications stay in a separate queue.
Worst case scenario of someone failing to write a migration and the app crash looping on the user's device is really bad. It is actually very easy to end up in that situation if the developer simply uninstalls the app to get around the migration because they were focused on solving another problem.
I think the best action for us is to add a "forceMigrations" method into the builder that will always enforce migrations. This way, more aware developers can turn it on (they are also more likely to read the docs).
wo...@gmail.com <wo...@gmail.com> #6
There are two offline scenarios IMHO: one is a pure cache mode and you basically do not care about local data, but the other one is where a user generates some content offline. In that scenario it's essential that the data is never deleted. Assuming that all developers respect their user data it is still possible to make a mistake and having user data dropped can be an expensive mistake in that case.
What is the issue with having a default migration that will fix up the schema to have the correct tables + columns? E.g. not dropping tables or columns, just adding them and updating indices etc. That could still fail when the data does not match the constraints set, but at least it prevents data to be lost and sends a signal during development as opposed to silently dropping the whole schema.
What is the issue with having a default migration that will fix up the schema to have the correct tables + columns? E.g. not dropping tables or columns, just adding them and updating indices etc. That could still fail when the data does not match the constraints set, but at least it prevents data to be lost and sends a signal during development as opposed to silently dropping the whole schema.
yb...@google.com <yb...@google.com> #7
Library deciding to *delete* something is a bad idea. At least by default.
How about the scenario when in versionCode=2 I forgot to add the migration, but oh god it rolled out overnight to 1000 users, so at morning I quickly create and upload a versionCode=3 to include the needed migration - only to find that the precious data has been deleted? hardly a good idea.
How about the scenario when in versionCode=2 I forgot to add the migration, but oh god it rolled out overnight to 1000 users, so at morning I quickly create and upload a versionCode=3 to include the needed migration - only to find that the precious data has been deleted? hardly a good idea.
wo...@gmail.com <wo...@gmail.com> #8
First of all, thanks a lot for all the feedback. This is why we label these as alpha and I'm glad it is working.
To be honest, I'm quite torn on this issue.
Let me first address the #6, the offline issue. You don't need to be persisting the user input in the database to make the app work offline. In fact, if you do that, you need to deal with synchronization between the local database and the job that uploads it to ensure that you always upload what is necessary etc.
An easier solution to that problem is to use a work queue for each user modification and de-couple it from the database. This also makes it very easy to architect the rest of the app (e.g. you can freely trim the database when it grows w/o caring about pending data).
That is actually what we did in my previous job and it worked very well. We initially had a queue on top of Square Tape, then wrote Android Priority JobQueue. Mind you it is 5-6 years old but here is something I wrote about it later on:http://www.birbit.com/a-recipe-for-writing-responsive-rest-clients-on-android . By the time, this solution was quite common among the very few offline ready apps.
About auto-migrations, it is something we wanted to do but not very trivial so we postponed to post 1.0. It is also likely to be a tool inside Studio to generate the migration instead of auto-magic migrations.
About #7, same scenario is as worse in the crash case where you'll get thousands of crash loops and uninstalls. It gets just worse if you didn't realize it next morning.
Auto-crashing is also annoying during development, though we can work around it by providing out of the box NukeMigration which will nuke the db and recreate.
Allowing a forceMigrations builder parameter seems like a proper sacrifice to me since it allows the more aware developer to put more strict rules for themselves while allowing the less aware developer to go with their flow. Also makes the common case easier.
Alternatively, we can default to crash but I feel like it will be a decision we'll regret as we receive more bugs that apps which use Room crash after update.
A third option might be to enforce force migration builder parameter, which avoids making a decision. It is an ugly API but will leave the decision to the developer in a more explicit way.
Please keep the comments coming, it is important to get this right for us and we really value your feedback.
To be honest, I'm quite torn on this issue.
Let me first address the #6, the offline issue. You don't need to be persisting the user input in the database to make the app work offline. In fact, if you do that, you need to deal with synchronization between the local database and the job that uploads it to ensure that you always upload what is necessary etc.
An easier solution to that problem is to use a work queue for each user modification and de-couple it from the database. This also makes it very easy to architect the rest of the app (e.g. you can freely trim the database when it grows w/o caring about pending data).
That is actually what we did in my previous job and it worked very well. We initially had a queue on top of Square Tape, then wrote Android Priority JobQueue. Mind you it is 5-6 years old but here is something I wrote about it later on:
About auto-migrations, it is something we wanted to do but not very trivial so we postponed to post 1.0. It is also likely to be a tool inside Studio to generate the migration instead of auto-magic migrations.
About #7, same scenario is as worse in the crash case where you'll get thousands of crash loops and uninstalls. It gets just worse if you didn't realize it next morning.
Auto-crashing is also annoying during development, though we can work around it by providing out of the box NukeMigration which will nuke the db and recreate.
Allowing a forceMigrations builder parameter seems like a proper sacrifice to me since it allows the more aware developer to put more strict rules for themselves while allowing the less aware developer to go with their flow. Also makes the common case easier.
Alternatively, we can default to crash but I feel like it will be a decision we'll regret as we receive more bugs that apps which use Room crash after update.
A third option might be to enforce force migration builder parameter, which avoids making a decision. It is an ugly API but will leave the decision to the developer in a more explicit way.
Please keep the comments coming, it is important to get this right for us and we really value your feedback.
yb...@google.com <yb...@google.com> #9
Re #6 If you want to show the content in the app while it's not synced to the network, it makes sense to store it in the database and additionally keep some sync data. I've built a couple of apps where this was the case: the user adds some content and doesn't really care if it is synced, just that it is synced eventually. By adding it to the database and marking it as "pending" somehow the content can now show up with the rest of the content which in fact simplifies the architecture.
One example is a photo app, where you just want to keep adding your photos to albums and once you are on wifi, everything is synced up to the cloud. Meanwhile you can review and work with your albums without change.
I recently ran into an app doing something similar (adding to the db + sync marker + scheduling a job to sync) so it's not just me doing that :D
It probably boils down to what you are describing, but without the need to consolidate the work queue with the data from the db, because that data in the db is already optimistically updated. Without debating which one is better, I think it's a valid approach.
Nuking the DB for development isn't great either. That same app did not have a way to fetch the previously synced data (for reasons...) and nuking the data during development would therefore not be something that would be very convenient.
Re #7: Of course crashing isn't great either, but if I have the choice of a crashing app for which I can stop roll out and provide an update vs telling users that I trashed their data, I know what I'd pick.
I'd prefer an automatic migration and I think that automatic migration should be at runtime (what if you forget to generate your migration in AS?). For the apps that I wrote, and for users using my library, it seems to match the 90% case for migrations where new entities are added and no data migration is required. If that's not possible then I guess an explicit choice would be best, because ultimately the dev has to indicate someway that it's OK for Room to nuke user data as a convenience or not.
One example is a photo app, where you just want to keep adding your photos to albums and once you are on wifi, everything is synced up to the cloud. Meanwhile you can review and work with your albums without change.
I recently ran into an app doing something similar (adding to the db + sync marker + scheduling a job to sync) so it's not just me doing that :D
It probably boils down to what you are describing, but without the need to consolidate the work queue with the data from the db, because that data in the db is already optimistically updated. Without debating which one is better, I think it's a valid approach.
Nuking the DB for development isn't great either. That same app did not have a way to fetch the previously synced data (for reasons...) and nuking the data during development would therefore not be something that would be very convenient.
Re #7: Of course crashing isn't great either, but if I have the choice of a crashing app for which I can stop roll out and provide an update vs telling users that I trashed their data, I know what I'd pick.
I'd prefer an automatic migration and I think that automatic migration should be at runtime (what if you forget to generate your migration in AS?). For the apps that I wrote, and for users using my library, it seems to match the 90% case for migrations where new entities are added and no data migration is required. If that's not possible then I guess an explicit choice would be best, because ultimately the dev has to indicate someway that it's OK for Room to nuke user data as a convenience or not.
yb...@google.com <yb...@google.com> #10
To clarify: I wrote "That same app", which is not true, I scrapped a second example to keep my reply shorter, but it was a different app :)
wo...@gmail.com <wo...@gmail.com> #11
Yea, it is always a trade-offs case :). We did the same, just double write into the queue and the database.
As far as I can see, the majority is in favor of crashing in which case we would ahead and change the API to crash by default and add a builder parameter to nuke if migration is missing.
As far as I can see, the majority is in favor of crashing in which case we would ahead and change the API to crash by default and add a builder parameter to nuke if migration is missing.
yb...@google.com <yb...@google.com> #12
In the next version, Room will throw by default.
This behavior can be changed by calling fallbackToDestructiveMigration in the builder.
Thanks for all the feedback.
This behavior can be changed by calling fallbackToDestructiveMigration in the builder.
Thanks for all the feedback.
wo...@gmail.com <wo...@gmail.com> #13
Thanks. It's partly fixed. I updated my sample project:
https://github.com/PaulWoitaschek/RoomMultiModuleBug
The property names on the data classes do now work. The dao interface parameter names don't work and still have to be called arg0 etc.:
https://github.com/PaulWoitaschek/RoomMultiModuleBug/blob/master/library/src/main/java/de/paul_woitaschek/roombug/library/LibraryDao.kt#L10
The property names on the data classes do now work. The dao interface parameter names don't work and still have to be called arg0 etc.:
wo...@gmail.com <wo...@gmail.com> #14
It looks like you can use the metadata annotation for interfaces too.
For example if I have this simple dao interface:
https://gist.github.com/PaulWoitaschek/e26d0e4f53460710c8caba9e995eb06d
It will producte the following Metadata annotation in byte code:
d2={"Lcom/yazio/android/misc/repo/MyDao;", "", "get", "Ljava/io/File;", "id", "", "production sources for module app"})
So all the information we need is there.
For example if I have this simple dao interface:
It will producte the following Metadata annotation in byte code:
d2={"Lcom/yazio/android/misc/repo/MyDao;", "", "get", "Ljava/io/File;", "id", "", "production sources for module app"})
So all the information we need is there.
yb...@google.com <yb...@google.com> #15
thanks. i'll take a look at it.
yb...@google.com <yb...@google.com> #16
ugh my bad, i totally missed the part about parsing dao methods :/ .
wo...@gmail.com <wo...@gmail.com> #18
Works, thank you!
be...@gmail.com <be...@gmail.com> #19
Hi, I have used "1.1.0-alpha3" but the problem still happened. When I builded the project, there showed some error like this:
错误: Each bind variable in the query must have a matching method parameter. Cannot find method parameters for :userIds.
错误: Unused parameter: arg0
错误: Each bind variable in the query must have a matching method parameter. Cannot find method parameters for :first, :last.
错误: Unused parameters: arg0,arg1
4 个错误
Here is my projecthttps://github.com/beyondbright/AndroidArchitectureSample
Did I miss something? Please help me. Thanks.
错误: Each bind variable in the query must have a matching method parameter. Cannot find method parameters for :userIds.
错误: Unused parameter: arg0
错误: Each bind variable in the query must have a matching method parameter. Cannot find method parameters for :first, :last.
错误: Unused parameters: arg0,arg1
4 个错误
Here is my project
Did I miss something? Please help me. Thanks.
wo...@gmail.com <wo...@gmail.com> #20
Yes, you need to use kotlin if you want that. The information gets not contained in the way it works right now for java.
Maybe reopen the issue till it's fixed for java too, Yigit?
Maybe reopen the issue till it's fixed for java too, Yigit?
be...@gmail.com <be...@gmail.com> #21
Everything was OK before I put the UserDao into the library module.
be...@gmail.com <be...@gmail.com> #22
Sorry. I know it now. But I use java not kotlin.
I can not use room in our project because of the issue. There are some modules in our project and I have to put the Dao into the library module.
Really hope it works for java.
I can not use room in our project because of the issue. There are some modules in our project and I have to put the Dao into the library module.
Really hope it works for java.
ka...@beehyv.com <ka...@beehyv.com> #23
I am using the java module 2.2.2 and still getting the same errors. However when I build again, it goes away, but having to build the module multiple times is really annoying.
yb...@google.com <yb...@google.com>
ra...@gmail.com <ra...@gmail.com> #24
Still the same problem in 2021. Always need to compile twice. This bug is not "fixed" (for Java)!
da...@google.com <da...@google.com> #25
Use javac -parameters
compiler flag so parameter names are compiled into the .class files and Room is able to read them when inspecting compiled code.
See
fa...@gmail.com <fa...@gmail.com> #26
I have the same problem again using Kotlin 1.7.10.
Does anyone know if this bug just appeared again?
Does anyone know if this bug just appeared again?
mo...@mindtickle.com <mo...@mindtickle.com> #27
yes, I'm facing this issue on kotlin version 1.9.20 as well. Has anyone found any solution for this other than naming arguments to arg0 etc?
du...@gmail.com <du...@gmail.com> #28
I am also facing this problem with Kotlin 1.9.23, please help to check. I don't want to rename to arg0, arg1,...
Description
Version used: 1.0.0-beta2
Devices/Android versions reproduced on:
Consider the following code, added in an android library module (AAR):
@Entity(tableName = "my_table")
public class MyTable {
@PrimaryKey
@NonNull
public String id;
}
@Dao
public interface MyTableDao {
@Query("SELECT * FROM my_table WHERE id = :id")
LiveData<List<MyTable>> sampleQuery(String id);
}
The actual database is instantiated in the app module. When compiling, I receive the following error:
Error:Each bind variable in the query must have a matching method parameter. Cannot find method parameters for :id.
Error:Unused parameter: arg0
Renaming ":id" to ":arg0" in the query fixes the issue. It seems that AAR packages lose the parameter names, and this causes Room to fail to bind the parameters correctly. A similar issue happens when an entity constructor has a parameter ("Error:Entities and Pojos must have a usable public constructor. You can have an empty constructor or a constructor whose parameters match the fields (by name and type).").