Fixed
Status Update
No update yet.
Comments
il...@google.com
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
ec...@gmail.com
ec...@gmail.com
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 (
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
Are you generating Java Safe Args code (vs generating Kotlin code)? If so, there's a known issue around default values: https://issuetracker.google.com/issues/140123727
ec...@gmail.com
Hi. Sorry for the delay. I'm using the Kotlin generator.
I took a look at the generator code. Shouldn't the null defaultValue be explicitly handled in this linehttps://android.googlesource.com/platform/frameworks/support/+/refs/heads/androidx-master-dev/navigation/navigation-safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/kotlin/KotlinNavWriter.kt#190 ? I mean, if defaultValue is not set in the XML, nothing is passed to the ParameterSpec defaultValue.
I took a look at the generator code. Shouldn't the null defaultValue be explicitly handled in this line
il...@google.com
Can you include a sample that reproduces your issue?
ec...@gmail.com
Ok. I'm confused. I'll write here, at least for my own investigation.
What I described in #4, that really happens. But that is solved by simply adding defaultValue="@null" in the XML, like people said somewhere else on the webs. By doing that, null is added to the corresponding field in the constructor of the Args class. This thus invalidates #6.
But, the deep link part doesn't like defaultValue="@null" when I try to navigate (through navigation component) with an URI that does not have that argument/parameter. As described in #1, the string parameter defined with defaultValue="@null" gets the string "@null". In short: navigating directly gives that argument with the default value null (from the Args class), but navigating with deepLink gives "@null".
I've also found that optional parameter in deepLinks has been introduced in the change linked herehttps://issuetracker.google.com/issues/133273839#comment3 , which includes the code I referenced in #1.
I can workaround all this by adjusting my code to use empty string as the marker for not present field, and setting defaultValue to "". The editor doesn't like it ("missing value"), but that's another issue.
What I described in #4, that really happens. But that is solved by simply adding defaultValue="@null" in the XML, like people said somewhere else on the webs. By doing that, null is added to the corresponding field in the constructor of the Args class. This thus invalidates #6.
But, the deep link part doesn't like defaultValue="@null" when I try to navigate (through navigation component) with an URI that does not have that argument/parameter. As described in #1, the string parameter defined with defaultValue="@null" gets the string "@null". In short: navigating directly gives that argument with the default value null (from the Args class), but navigating with deepLink gives "@null".
I've also found that optional parameter in deepLinks has been introduced in the change linked here
I can workaround all this by adjusting my code to use empty string as the marker for not present field, and setting defaultValue to "". The editor doesn't like it ("missing value"), but that's another issue.
jb...@google.com
We would like to investigate as well. Do you have a sample project that reproduces the issues that you can upload as a part of this bug?
ec...@gmail.com
Here it goes:
- in order to avoid dealing with initial boilerplate, clonedhttps://github.com/googlecodelabs/android-navigation ;
- uncomented TODO from the codelab steps;
- updated deps, plugins versions, fixed some compiling issue;
- removed code, left Home and DeepLinkFragment only;
- changed app:uri to
"www.example.com/query?param1={param1}¶m2={param2} ";
- added arguments in deepLinkFrangment in mobile_navigation.xml:
<argument
android:name="param1"
android:defaultValue="@null"
app:nullable="true"/>
<argument
android:name="param2"
android:defaultValue="@null"
app:nullable="true"/>
- added action in homeFragment targeting deepLinkFragment
<action
android:id="@+id/deeplink_action"
app:destination="@+id/deeplink_dest" />
- added a button and the following code to HomeFragment, using action generated code (notice that no argument is passed because they default to null):
view.findViewById<Button>(R.id.navigate_direct_button)?.setOnClickListener {
findNavController().navigate(HomeFragmentDirections.deeplinkAction(param1 = "HELLO"))
}
- added a button and the following code to HomeFragment, using Uri (notice that param2 is absent):
view.findViewById<Button>(R.id.navigate_deeplink_button)?.setOnClickListener {
findNavController().navigate(
Uri.parse("example://www.example.com/query?param1=HELLO "))
}
(had to change app:uri from "www.example.com " to "example://www.example.com/.. ." because the first wasn't matching (another issue i guess))
- adjusted DeepLinkFragment to show param1 and param2, from the arguments bundle (could have used by navArgs with same result)
textView.text = arguments?.getString("param1", "") + arguments?.getString("param2", "")
Running the app, when the "navigate with direct action" button is clicked, "HELLO" is shown in the next fragment, as expected. When "navigate with deeplink" is clicked, "HELLO@null" is shown, which is the issue.
- in order to avoid dealing with initial boilerplate, cloned
- uncomented TODO from the codelab steps;
- updated deps, plugins versions, fixed some compiling issue;
- removed code, left Home and DeepLinkFragment only;
- changed app:uri to
"
- added arguments in deepLinkFrangment in mobile_navigation.xml:
<argument
android:name="param1"
android:defaultValue="@null"
app:nullable="true"/>
<argument
android:name="param2"
android:defaultValue="@null"
app:nullable="true"/>
- added action in homeFragment targeting deepLinkFragment
<action
android:id="@+id/deeplink_action"
app:destination="@+id/deeplink_dest" />
- added a button and the following code to HomeFragment, using action generated code (notice that no argument is passed because they default to null):
view.findViewById<Button>(R.id.navigate_direct_button)?.setOnClickListener {
findNavController().navigate(HomeFragmentDirections.deeplinkAction(param1 = "HELLO"))
}
- added a button and the following code to HomeFragment, using Uri (notice that param2 is absent):
view.findViewById<Button>(R.id.navigate_deeplink_button)?.setOnClickListener {
findNavController().navigate(
Uri.parse("example://
}
(had to change app:uri from "
- adjusted DeepLinkFragment to show param1 and param2, from the arguments bundle (could have used by navArgs with same result)
textView.text = arguments?.getString("param1", "") + arguments?.getString("param2", "")
Running the app, when the "navigate with direct action" button is clicked, "HELLO" is shown in the next fragment, as expected. When "navigate with deeplink" is clicked, "HELLO@null" is shown, which is the issue.
b9...@gmail.com
I'm facing this issue as well, default null seems never be null.
The root cause seems in the androidx.navigation.NavDeepLink.java:
// Missing parameter so see if it has a default value or is Nullable
if (argument != null
&& (value == null || value.replaceAll("[{}]", "").equals(argName))) {
if (argument.getDefaultValue() != null) {
value = argument.getDefaultValue().toString();
} else if (argument.isNullable()) {
value = "@null";
}
}
The root cause seems in the androidx.navigation.NavDeepLink.java:
// Missing parameter so see if it has a default value or is Nullable
if (argument != null
&& (value == null || value.replaceAll("[{}]", "").equals(argName))) {
if (argument.getDefaultValue() != null) {
value = argument.getDefaultValue().toString();
} else if (argument.isNullable()) {
value = "@null";
}
}
ra...@gmail.com
+1
arguments?.run {
val args by navArgs<BFragmentArgs>()
//Should be null instead "@null"?
Log.d("ARGS", "Token ${args.token}")
}
arguments?.run {
val args by navArgs<BFragmentArgs>()
//Should be null instead "@null"?
Log.d("ARGS", "Token ${args.token}")
}
ra...@gmail.com
It would be helpful if defaultValue is really null, instead "@null" on deeplink cases like com.sample://fragmentB?token= (that we could receive by mistake from backend/external service)
<argument
android:name="token"
android:defaultValue="@null"
app:argType="string"
app:nullable="true" />
<deepLink
android:id="@+id/cta"
app:uri="com.sample://fragmentB?token={token}" />
<argument
android:name="token"
android:defaultValue="@null"
app:argType="string"
app:nullable="true" />
<deepLink
android:id="@+id/cta"
app:uri="com.sample://fragmentB?token={token}" />
ap...@google.com
Project: platform/frameworks/support
Branch: androidx-master-dev
commit 6e93d1d7ee8ac63451e8822422fb3b1da90fa321
Author: Jeremy Woods <jbwoods@google.com>
Date: Mon Nov 25 15:29:59 2019
Make nullable deep link params default to null
Currently a deep link param that cannot be matched but is marked as
nullable will be stored in the argument bundle with a value of "@null".
This causes the behavior for a missing param to be inconsistent between
a normal navigation and navigation from a deep link.
This change stores null an instance of null in the bundle for the value
instead to ensure we mimic the behavior of non-deep link nullable
params.
Test: adjusted NavDeepLinkTest
BUG: 141613546
Change-Id: I14625699fc9365088e07df9456a05257974a375b
M navigation/navigation-common/src/androidTest/java/androidx/navigation/NavDeepLinkTest.kt
M navigation/navigation-common/src/main/java/androidx/navigation/NavDeepLink.java
https://android-review.googlesource.com/1174705
Branch: androidx-master-dev
commit 6e93d1d7ee8ac63451e8822422fb3b1da90fa321
Author: Jeremy Woods <jbwoods@google.com>
Date: Mon Nov 25 15:29:59 2019
Make nullable deep link params default to null
Currently a deep link param that cannot be matched but is marked as
nullable will be stored in the argument bundle with a value of "@null".
This causes the behavior for a missing param to be inconsistent between
a normal navigation and navigation from a deep link.
This change stores null an instance of null in the bundle for the value
instead to ensure we mimic the behavior of non-deep link nullable
params.
Test: adjusted NavDeepLinkTest
BUG: 141613546
Change-Id: I14625699fc9365088e07df9456a05257974a375b
M navigation/navigation-common/src/androidTest/java/androidx/navigation/NavDeepLinkTest.kt
M navigation/navigation-common/src/main/java/androidx/navigation/NavDeepLink.java
jb...@google.com
This has been fixed internally and will be available in the Navigation 2.2.0-rc03 release.
Description
A parameter of argType string, nullable and defaultValue="@null", gives bundle with "@null" string. We would expect the bundle item to be null, not the string "@null".
From what I could track, there's something strange around NavDeepLink.java#153 and in NavType#parseValue for the string type.