scope file observer to active Datastore.data observers [267792241]

Fixed

Internal Cleanup

Status Update

No update yet.

Description

yb...@google.com

created issue #1

Feb 3, 2023 08:32PM

ideally, we should limit file observer to the time frame where someone is observing the `datastore.data` so it is better scoped and doesn't do work unless necessary.

we already have versioning check to avoid issues there.

Comments

zh...@google.com <zh...@google.com> Aug 31, 2023 08:12PM

Reassigned to zh...@google.com.

zh...@google.com <zh...@google.com> #2Sep 5, 2023 05:46PM

Accepted by zh...@google.com.

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 6f59e3d74d0da5a4bf79c8480f56964e3af47f1c
Author: Wojtek Kaliciński <wkal@google.com>
Date: Mon Jul 23 20:17:19 2018

Support more types for args

Provides support for Serializable, enums,
as well as arrays of primitive types and Parcelables
in Navigation arguments.

Only enums support default values other than @null,
in the form of the enum literal (without class name),
e.g. app:defaultValue="READ"

Test: unit tests updated
BUG: 111487504
BUG: 111316353
Change-Id: I7e5abbc8ed0950bfdef292342dbe1fb34a5c4b17

M navigation/runtime/src/androidTest/java/androidx/navigation/NavInflaterTest.kt
A navigation/runtime/src/androidTest/java/androidx/navigation/test/TestEnum.java
M navigation/runtime/src/androidTest/res/navigation/nav_default_arguments.xml
M navigation/runtime/src/main/java/androidx/navigation/NavInflater.java
M navigation/safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/NavParser.kt
M navigation/safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/NavParserErrors.kt
M navigation/safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/NavWriter.kt
M navigation/safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/Types.kt
M navigation/safe-args-generator/src/tests/kotlin/androidx/navigation/safe/args/generator/NavParserTest.kt
M navigation/safe-args-generator/src/tests/kotlin/androidx/navigation/safe/args/generator/NavWriterTest.kt
M navigation/safe-args-generator/src/tests/test-data/expected/nav_writer_test/MainFragmentArgs.java
M navigation/safe-args-generator/src/tests/test-data/expected/nav_writer_test/Next.java
M navigation/safe-args-generator/src/tests/test-data/naive_test.xml

https://android-review.googlesource.com/720246

https://goto.google.com/android-sha1/6f59e3d74d0da5a4bf79c8480f56964e3af47f1c

yb...@google.com <yb...@google.com> #3Sep 6, 2023 11:47PM

This is fixed internally and will be available in 1.0.0-alpha08

zh...@google.com <zh...@google.com> #4Sep 7, 2023 12:07AM

Thanks!

yb...@google.com <yb...@google.com> #5Sep 7, 2023 03:51PM

moved.

yb...@google.com <yb...@google.com> Feb 20, 2024 09:55PM

Assigned to yb...@google.com.

ap...@google.com <ap...@google.com> #6Feb 20, 2024 10:10PM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-main

commit 777ba8f9a1cdb4ca88263c6785c2708846b16d3f
Author: Yigit Boyar <yboyar@google.com>
Date: Tue Feb 13 09:46:31 2024

Collect update notifications only when datastore is observed

Previously, DataStore started collecting updates from IPC right after
it is initialized and kept this collection alive as long as the scope
is alive.

This change updates it to only observe the file if there is an active
observer on the DataStore. We achieve this by updating `data` to a
channel flow that will collect on the shared flow when it is used.

Some tests needed to be updated because they made an assumption on the
order of execution which slightly changes with this channel flow but
does not effect correctness.

Fixes: 267792241
Test: SingleProcessDataStoreTest#observeFileOnlyWhenDatastoreIsObserved
Change-Id: Id8221718f75b9755119d5e9f38c4827fa907b867

M datastore/datastore-core/src/androidInstrumentedTest/kotlin/androidx/datastore/core/MultiProcessDataStoreSingleProcessTest.kt
M datastore/datastore-core/src/commonMain/kotlin/androidx/datastore/core/DataStoreImpl.kt
M datastore/datastore-core/src/commonTest/kotlin/androidx/datastore/core/CloseDownstreamOnCloseTest.kt
M datastore/datastore-core/src/commonTest/kotlin/androidx/datastore/core/SingleProcessDataStoreTest.kt

https://android-review.googlesource.com/2961704

th...@shareforce.eu <th...@shareforce.eu> #7Feb 21, 2024 01:08PM

In my code I keep a reference to the supervisorJob used in the datastore scope:

val job = SupervisorJob()
val Context.dataStore: DataStore<Preferences> by preferencesDataStore(name = "dynamic name", CoroutineScope(Dispatchers.IO + job)

When the datastore is no longer in use, I call job.cancelAndJoin(). Does this change imply cancelling is no longer needed?

Message last modified on Feb 21, 2024 01:11PM

yb...@google.com <yb...@google.com> #8Feb 21, 2024 04:50PM

you still need to explicitly cancel if you want to re-open the same file via another instance. This is because StorageConnection objects still need to be explicitly closed and even when we are not observing the file, DataStore instance claims ownership of it.

There is a relevant FR about that here: b/221790279 . We do have plans to change it but we are not yet sure on the implementation. Ideally, we do want people keep these as singletons (since the file is a singleton on disk) but it does complicate certain use cases so we might allow multiple instances in the future.

th...@shareforce.eu <th...@shareforce.eu> #9Feb 22, 2024 01:42PM

Thank you for your quick answer and the relevant information, much appreciated.

na...@google.com <na...@google.com> #10Mar 6, 2024 07:49PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.datastore:datastore-core:1.1.0-beta02
androidx.datastore:datastore-core-android:1.1.0-beta02
androidx.datastore:datastore-core-iosarm64:1.1.0-beta02
androidx.datastore:datastore-core-iossimulatorarm64:1.1.0-beta02
androidx.datastore:datastore-core-iosx64:1.1.0-beta02
androidx.datastore:datastore-core-jvm:1.1.0-beta02
androidx.datastore:datastore-core-linuxx64:1.1.0-beta02
androidx.datastore:datastore-core-macosarm64:1.1.0-beta02
androidx.datastore:datastore-core-macosx64:1.1.0-beta02