Fixed
Status Update
Comments
il...@google.com <il...@google.com> #2
Project: platform/frameworks/support
Branch: androidx-master-dev
commit b90079595f33f58fece04026a97faa0d243acdb1
Author: Yuichi Araki <yaraki@google.com>
Date: Wed Sep 18 16:55:49 2019
Change the way to detect mismatch between POJO and query
This fixes cursor mismatch warnings with expandProjection.
Bug: 140759491
Test: QueryMethodProcessorTest
Change-Id: I7659002e5e0d1ef60fc1af2a625c0c36da0664d8
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
M room/compiler/src/test/kotlin/androidx/room/processor/QueryMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/testing/TestProcessor.kt
https://android-review.googlesource.com/1123258
https://goto.google.com/android-sha1/b90079595f33f58fece04026a97faa0d243acdb1
Branch: androidx-master-dev
commit b90079595f33f58fece04026a97faa0d243acdb1
Author: Yuichi Araki <yaraki@google.com>
Date: Wed Sep 18 16:55:49 2019
Change the way to detect mismatch between POJO and query
This fixes cursor mismatch warnings with expandProjection.
Bug: 140759491
Test: QueryMethodProcessorTest
Change-Id: I7659002e5e0d1ef60fc1af2a625c0c36da0664d8
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
M room/compiler/src/test/kotlin/androidx/room/processor/QueryMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/testing/TestProcessor.kt
du...@gmail.com <du...@gmail.com> #3
il...@google.com <il...@google.com> #4
Project: platform/frameworks/support
Branch: androidx-master-dev
commit bdde5a1a970ddc9007b28de4aa29d60ffa588f08
Author: Yigit Boyar <yboyar@google.com>
Date: Thu Apr 16 16:47:05 2020
Re-factor how errors are dismissed when query is re-written
This CL changes how we handle errors/warnings if query is
re-written.
There was a bug in expandProjection where we would report warnings
for things that Room already fixes automatically ( b/140759491 ).
The solution to that problem (I7659002e5e0d1ef60fc1af2a625c0c36da0664d8)
solved it by deferring validating of columns until after re-write
decision is made. Unfortunately, this required changing PojoRowAdapter
to have a dummy mapping until it is validating, make it hard to use
as it does have a non-null mapping which is not useful.
This CL partially reverts that change and instead rely on the log
deferring logic we have in Context. This way, we don't need to break
the stability of PojoRowAdapter while still having the ability to
drop warnings that room fixes. This will also play nicer when we
have different query re-writing options that can use more information
about the query results.
Bug: 153387066
Bug: 140759491
Test: existing tests pass
Change-Id: I2ec967c763d33d7a3ff02c1a13c6953b460d1e5f
M room/compiler/src/main/kotlin/androidx/room/log/RLog.kt
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
https://android-review.googlesource.com/1288456
Branch: androidx-master-dev
commit bdde5a1a970ddc9007b28de4aa29d60ffa588f08
Author: Yigit Boyar <yboyar@google.com>
Date: Thu Apr 16 16:47:05 2020
Re-factor how errors are dismissed when query is re-written
This CL changes how we handle errors/warnings if query is
re-written.
There was a bug in expandProjection where we would report warnings
for things that Room already fixes automatically (
The solution to that problem (I7659002e5e0d1ef60fc1af2a625c0c36da0664d8)
solved it by deferring validating of columns until after re-write
decision is made. Unfortunately, this required changing PojoRowAdapter
to have a dummy mapping until it is validating, make it hard to use
as it does have a non-null mapping which is not useful.
This CL partially reverts that change and instead rely on the log
deferring logic we have in Context. This way, we don't need to break
the stability of PojoRowAdapter while still having the ability to
drop warnings that room fixes. This will also play nicer when we
have different query re-writing options that can use more information
about the query results.
Bug: 153387066
Bug: 140759491
Test: existing tests pass
Change-Id: I2ec967c763d33d7a3ff02c1a13c6953b460d1e5f
M room/compiler/src/main/kotlin/androidx/room/log/RLog.kt
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
il...@google.com <il...@google.com>
ap...@google.com <ap...@google.com> #5
Project: platform/frameworks/support
Branch: androidx-master-dev
commit bb456e84a524d4e5cd241a6a839a1062297b9cf4
Author: Ian Lake <ilake@google.com>
Date: Thu Jan 31 15:46:55 2019
Enforce "reference" types for reference default values
In order to ensure that inflation of a navigation
graph results in the same arguments as Safe Args,
we now enforce usage of the "reference" type for any
argument that has a defaultValue that references
another resource (i.e., @string/app_name, etc).
This makes it much more obvious that the default value
in Safe Args is in the form of R.string.app_name and
any provided override for that should also be in the
form of R.string.some_value.
In these cases, it is expected that developers either
1) Use the ids directly (for example, passing an
R.string to a method that takes a resource id)
2) Specifically call context.getString() with the
resource id they retrieve from their NavArgs instance.
Test: updated NavInflaterTest
Change-Id: I06b8c9a0d1420821b8c0064c22f176788da43d7a
Fixes: 123551990
M navigation/runtime/src/androidTest/java/androidx/navigation/NavInflaterTest.kt
M navigation/runtime/src/androidTest/res/navigation/nav_default_arguments.xml
A navigation/runtime/src/androidTest/res/navigation/nav_invalid_argument_arg_type.xml
A navigation/runtime/src/androidTest/res/navigation/nav_invalid_argument_default_value.xml
M navigation/runtime/src/main/java/androidx/navigation/NavInflater.java
https://android-review.googlesource.com/891680
https://goto.google.com/android-sha1/bb456e84a524d4e5cd241a6a839a1062297b9cf4
Branch: androidx-master-dev
commit bb456e84a524d4e5cd241a6a839a1062297b9cf4
Author: Ian Lake <ilake@google.com>
Date: Thu Jan 31 15:46:55 2019
Enforce "reference" types for reference default values
In order to ensure that inflation of a navigation
graph results in the same arguments as Safe Args,
we now enforce usage of the "reference" type for any
argument that has a defaultValue that references
another resource (i.e., @string/app_name, etc).
This makes it much more obvious that the default value
in Safe Args is in the form of R.string.app_name and
any provided override for that should also be in the
form of R.string.some_value.
In these cases, it is expected that developers either
1) Use the ids directly (for example, passing an
R.string to a method that takes a resource id)
2) Specifically call context.getString() with the
resource id they retrieve from their NavArgs instance.
Test: updated NavInflaterTest
Change-Id: I06b8c9a0d1420821b8c0064c22f176788da43d7a
Fixes: 123551990
M navigation/runtime/src/androidTest/java/androidx/navigation/NavInflaterTest.kt
M navigation/runtime/src/androidTest/res/navigation/nav_default_arguments.xml
A navigation/runtime/src/androidTest/res/navigation/nav_invalid_argument_arg_type.xml
A navigation/runtime/src/androidTest/res/navigation/nav_invalid_argument_default_value.xml
M navigation/runtime/src/main/java/androidx/navigation/NavInflater.java
il...@google.com <il...@google.com> #6
Navigation now enforces use of the "reference" type if you have a defaultValue referencing another resource. This ensures that Safe Args and Navigation's runtime behavior are always in sync.
Please starhttps://issuetracker.google.com/issues/36994900 to track allowing build time placeholders in XML files.
Please star
du...@gmail.com <du...@gmail.com> #7
Ok, great, now it is much more straightforward. Thanks!
ne...@gmail.com <ne...@gmail.com> #8
This "fix" broke our app and our navigation XML, the file looked like the one described in the first message by OP and that's exactly how we wanted it to work.
Let me describe the use case.
Our Fragments load content based on the argument "url" (coming from backend) and the default value is required to load the initial content when the user starts the app.
Previously we had this default value stored in string resources where all the app strings can be easily accessed and reused across the app.
After this "fix" in beta01, we have to hardcode the default path in the navigation XML file and we have lost the ability to reference the same default path in the code (now the string is duplicated in string resources and in the navigation XML file).
Another undesired side-effect is that another argument used the default value of @string/null_ and now this doesn't work anymore forcing us to use an ugly empty string with a space character and in the Fragment check for blank string.
Could you enable resources references again just for default XML arguments for other argTypes?
Let me describe the use case.
Our Fragments load content based on the argument "url" (coming from backend) and the default value is required to load the initial content when the user starts the app.
Previously we had this default value stored in string resources where all the app strings can be easily accessed and reused across the app.
After this "fix" in beta01, we have to hardcode the default path in the navigation XML file and we have lost the ability to reference the same default path in the code (now the string is duplicated in string resources and in the navigation XML file).
Another undesired side-effect is that another argument used the default value of @string/null_ and now this doesn't work anymore forcing us to use an ugly empty string with a space character and in the Fragment check for blank string.
Could you enable resources references again just for default XML arguments for other argTypes?
il...@google.com <il...@google.com> #9
Re #8:
- String, object, and array argument types support using android:defaultValue="@null" for null default values. This continues to be the case.https://issuetracker.google.com/issues/120627532 tracks adding documentation around that fact
- I've filedhttps://issuetracker.google.com/issues/124248602 for supporting an empty (i.e., 0) default value for reference types
Resources cannot be statically resolved at build time - they are inherently only resolvable at runtime, so there's no way for Safe Args to know what default value to add to the generate code. Given that, there's no plans on supporting resource references on anything other than "reference" types.
- String, object, and array argument types support using android:defaultValue="@null" for null default values. This continues to be the case.
- I've filed
Resources cannot be statically resolved at build time - they are inherently only resolvable at runtime, so there's no way for Safe Args to know what default value to add to the generate code. Given that, there's no plans on supporting resource references on anything other than "reference" types.
mo...@gmail.com <mo...@gmail.com> #10
you need to remove the default value from the nav_graph and figure out an alternative logic to set it .. I do not think that would be so hard to do.
[Deleted User] <[Deleted User]> #11 Restricted+
Restricted+
Comment has been deleted.
Description
Version used: android.arch.navigation:navigation-safe-args-gradle-plugin:1.0.0-alpha11
Devices/Android versions reproduced on: not related to device/api version, it is code generation problem
Bug description:
navigation file defined like this:
<?xml version="1.0" encoding="utf-8"?>
<navigation xmlns:android="
xmlns:app="
xmlns:tools="
android:id="@+id/nav_graph"
app:startDestination="@id/homeFragment">
<fragment
android:id="@+id/homeFragment"
android:name="no.amedia.newsapp.android.WebPageFragment"
android:label="fragment_home"
tools:layout="@layout/fragment_web_page" >
<argument
android:name="web_url"
app:argType="string"
android:defaultValue="@string/url" />
</fragment>
<action android:id="@+id/action_global_homeFragment"
app:destination="@id/homeFragment" />
....
</navigation>
where url (referenced as "@string/url") is defined as <string name="url" translatable="false">
generates code like this:
class NavGraphDirections private constructor() {
private data class ActionGlobalHomeFragment(val webUrl: String = "@string/url") :
NavDirections {
override fun getActionId(): Int = no.amedia.newsapp.android.R.id.action_global_homeFragment
override fun getArguments(): Bundle {
val result = Bundle()
result.putString("web_url", this.webUrl)
return result
}
}
companion object {
fun actionGlobalHomeFragment(webUrl: String = "@string/url"): NavDirections =
ActionGlobalHomeFragment(webUrl)
}
}
so the problem is webUrl: String = "@string/url" instead of webUrl: String = "
the same is with WebPageFragmentDirections generated class:
class WebPageFragmentDirections private constructor() {
companion object {
fun actionGlobalHomeFragment(webUrl: String = "@string/url"): NavDirections =
NavGraphDirections.actionGlobalHomeFragment(webUrl)
}
when I try to use code like this
navController.navigate(WebPageFragmentDirections.actionGlobalHomeFragment())
or
navController.navigate(NavGraphDirections.actionGlobalHomeFragment())
argument value in the fragment is "@string/url" instead of "
while if I navigate like this
navController.navigate(R.id.homeFragment)
default value from navigation xml is used correctly and I get "
I hope that I manage to describe the problem in a proper way, it is very simple to reproduce, but if you have any questions just ask