Feature Request: Singleton management [173726702]

Fixed

Feature Request

Status Update

No update yet.

Description

ru...@gmail.com

created issue #1

Nov 21, 2020 05:17AM

DataStore Component used: All
DataStore Version used: 1.0.0-alpha-04

DataStoreFactory and PreferenceDataStoreFactory include the following warning:

The user is responsible for ensuring that there is never more than one instance of SingleProcessDataStore acting on a file at a time.

Could the factories be made responsible for returning the same instance if they've already created one, so the consuming app doesn't need to manage that themselves?

Comments

ap...@google.com <ap...@google.com> #2Feb 17, 2021 01:23AM

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

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

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

ro...@google.com <ro...@google.com> Feb 17, 2021 09:18PM

Marked as fixed.

ap...@google.com <ap...@google.com> #3Feb 23, 2021 05:22PM

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

commit 5fe0a262ac6af55f68777fd2e2ef58cbc0939536
Author: rohitsat13 <rohitsat@google.com>
Date: Fri Feb 19 09:28:47 2021

RxDataStore property delegates

Test: See new Delegate Tests
Bug: 173726702
Relnote: Added property delegates for RxDataStore users.
Change-Id: Ied7680e698bc98b385b4bb4f33b6481963e7589a

M datastore/datastore-preferences-rxjava2/api/current.txt
M datastore/datastore-preferences-rxjava2/api/public_plus_experimental_current.txt
M datastore/datastore-preferences-rxjava2/api/restricted_current.txt
A datastore/datastore-preferences-rxjava2/src/androidTest/java/androidx/datastore/preferences/rxjava2/RxPreferenceDataStoreDelegateTest.kt
A datastore/datastore-preferences-rxjava2/src/main/java/androidx/datastore/preferences/rxjava2/RxPreferenceDataStoreDelegate.kt
M datastore/datastore-preferences-rxjava3/api/current.txt
M datastore/datastore-preferences-rxjava3/api/public_plus_experimental_current.txt
M datastore/datastore-preferences-rxjava3/api/restricted_current.txt
A datastore/datastore-preferences-rxjava3/src/androidTest/java/androidx/datastore/preferences/rxjava3/RxPreferenceDataStoreDelegateTest.kt
A datastore/datastore-preferences-rxjava3/src/main/java/androidx/datastore/preferences/rxjava3/RxPreferenceDataStoreDelegate.kt
M datastore/datastore-rxjava2/api/current.txt
M datastore/datastore-rxjava2/api/public_plus_experimental_current.txt
M datastore/datastore-rxjava2/api/restricted_current.txt
A datastore/datastore-rxjava2/src/androidTest/java/androidx/datastore/rxjava2/RxDataStoreDelegateTest.kt
A datastore/datastore-rxjava2/src/main/java/androidx/datastore/rxjava2/RxDataStoreDelegate.kt
M datastore/datastore-rxjava3/api/current.txt
M datastore/datastore-rxjava3/api/public_plus_experimental_current.txt
M datastore/datastore-rxjava3/api/restricted_current.txt
A datastore/datastore-rxjava3/src/androidTest/java/androidx/datastore/rxjava3/RxDataStoreDelegateTest.kt
A datastore/datastore-rxjava3/src/main/java/androidx/datastore/rxjava3/RxDataStoreDelegate.kt

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

ru...@gmail.com <ru...@gmail.com> #4Feb 25, 2021 01:46AM

Thanks, but I don't understand why this was implemented in datastore rather than datastore-core (and related modules). I was hoping to be able to use it without needing a Context reference.

ro...@google.com <ro...@google.com> #5Feb 25, 2021 06:17AM

A few things led to the approach that we ended up with:

Since we need the result of context.filesDir, and context.filesDir can do file IO, we need to accept a callback for generating the file. Deduping on just the relative path (String) is easier than deduping on the callback () -> File.
You can't really depend on context at the top level of a kotlin file or in a static/companion object.
We wanted something that people could easily copy&paste into their code and get going without having to worry to much about it.
(Unrelated to your question but..) Different calls to createDataStore for the same file names could have different construction params. The various params probably don't implement value equality so you can't really throw errors in these cases (we'd probably just have to take the first set of params then silently ignore future ones).

That said, if you can just inject the DataStore using hilt with @Singleton you don't have to worry about any of this.

jd...@xooloo.com <jd...@xooloo.com> #6Feb 25, 2021 10:23AM

Release notes for that features are broken.

Not only they suggest to use = instead of by (which make the code snippet invalid), but they forget to mention how we are supposed to deal with the new Serializer argument.

jd...@xooloo.com <jd...@xooloo.com> #7Feb 25, 2021 10:38AM

How are we suppose to migrate code like this:

private val _store = context.createDataStore("store", migrations = listOf(
            SharedPreferencesMigration(context, "store")
    ))

There is not way to create SharedPreferencesMigration without context reference.

private val Context.store by preferencesDataStore("store", migrations = listOf(
            SharedPreferencesMigration(context, "store") // error context do not exists
    ))

ro...@google.com <ro...@google.com> #8Feb 26, 2021 04:08PM

Status: In Progress (Accepted) (reopened)

Thanks for reporting - we are working on a fix for the migrations for next release.

ap...@google.com <ap...@google.com> #9Feb 26, 2021 07:24PM

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

commit b64da89bd9caa4492b296a701e08d3d6fb7f2e52
Author: rohitsat13 <rohitsat@google.com>
Date: Fri Feb 26 09:51:28 2021

Change migrations param to migrationProducers param for datastore property delegate since you may need context to create a sp migration for example.

Test: See new tests
Relnote: You can now add a migration that depends on context to your `dataStore` and `preferencesDataStore` property delegate.
Bug: 173726702
Change-Id: I4ef69b0eed544cde4f05cdee9e885d36af0f4abb

M datastore/datastore-preferences-rxjava2/api/current.txt
M datastore/datastore-preferences-rxjava2/api/public_plus_experimental_current.txt
M datastore/datastore-preferences-rxjava2/api/restricted_current.txt
M datastore/datastore-preferences-rxjava2/src/androidTest/java/androidx/datastore/preferences/rxjava2/RxPreferenceDataStoreDelegateTest.kt
M datastore/datastore-preferences-rxjava2/src/main/java/androidx/datastore/preferences/rxjava2/RxPreferenceDataStoreDelegate.kt
M datastore/datastore-preferences-rxjava3/api/current.txt
M datastore/datastore-preferences-rxjava3/api/public_plus_experimental_current.txt
M datastore/datastore-preferences-rxjava3/api/restricted_current.txt
M datastore/datastore-preferences-rxjava3/src/androidTest/java/androidx/datastore/preferences/rxjava3/RxPreferenceDataStoreDelegateTest.kt
M datastore/datastore-preferences-rxjava3/src/main/java/androidx/datastore/preferences/rxjava3/RxPreferenceDataStoreDelegate.kt
M datastore/datastore-preferences/api/current.txt
M datastore/datastore-preferences/api/public_plus_experimental_current.txt
M datastore/datastore-preferences/api/restricted_current.txt
M datastore/datastore-preferences/src/androidTest/java/androidx/datastore/preferences/PreferenceDataStoreDelegateTest.kt
M datastore/datastore-preferences/src/androidTest/java/androidx/datastore/preferences/SharedPreferencesToPreferencesTest.kt
M datastore/datastore-preferences/src/main/java/androidx/datastore/preferences/PreferenceDataStoreDelegate.kt
M datastore/datastore-rxjava2/api/current.txt
M datastore/datastore-rxjava2/api/public_plus_experimental_current.txt
M datastore/datastore-rxjava2/api/restricted_current.txt
M datastore/datastore-rxjava2/src/androidTest/java/androidx/datastore/rxjava2/RxDataStoreDelegateTest.kt
M datastore/datastore-rxjava2/src/main/java/androidx/datastore/rxjava2/RxDataStoreDelegate.kt
M datastore/datastore-rxjava3/api/current.txt
M datastore/datastore-rxjava3/api/public_plus_experimental_current.txt
M datastore/datastore-rxjava3/api/restricted_current.txt
M datastore/datastore-rxjava3/src/androidTest/java/androidx/datastore/rxjava3/RxDataStoreDelegateTest.kt
M datastore/datastore-rxjava3/src/main/java/androidx/datastore/rxjava3/RxDataStoreDelegate.kt
M datastore/datastore/api/current.txt
M datastore/datastore/api/public_plus_experimental_current.txt
M datastore/datastore/api/restricted_current.txt
M datastore/datastore/src/androidTest/java/androidx/datastore/DataStoreDelegateTest.kt
M datastore/datastore/src/androidTest/java/androidx/datastore/migrations/SharedPreferencesMigrationTest.kt
M datastore/datastore/src/main/java/androidx/datastore/DataStoreDelegate.kt

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

ro...@google.com <ro...@google.com> Mar 5, 2021 07:19PM