storage-ktx: add callbackFlow for upload/download progress #4139

thatfiredev · 2022-09-23T10:26:11Z

Part of go/kotlin-flows-firebase-android.

…-sdk into rpf-storage-flows

thatfiredev · 2022-09-23T10:30:21Z

firebase-storage/ktx/src/main/kotlin/com/google/firebase/storage/ktx/Storage.kt

+        addOnProgressListener(progressListener)
+        addOnPausedListener(pauseListener)


My initial idea was to pass the same executor to these listeners, but Storage's callback executor is marked as private:

firebase-android-sdk/firebase-storage/src/main/java/com/google/firebase/storage/StorageTaskScheduler.java

Lines 54 to 56 in 4f24be3

private static final ThreadPoolExecutor CALLBACK_QUEUE_EXECUTOR =

new ThreadPoolExecutor(

1, 1, 5, TimeUnit.SECONDS, mCallbackQueue, new StorageThreadFactory("Callbacks-"));

So instead I'm using the StorageTaskScheduler singleton to schedule all callbacks in that same executor:

firebase-android-sdk/firebase-storage/ktx/src/main/kotlin/com/google/firebase/storage/ktx/Storage.kt

Lines 153 to 155 in 27ed605

StorageTaskScheduler.getInstance().scheduleCallback {

trySendBlocking(StorageProgress.InProgress(snapshot))

}

thatfiredev · 2022-09-23T10:33:30Z

firebase-storage/ktx/src/main/kotlin/com/google/firebase/storage/ktx/StorageProgress.kt

+/**
+ * Used to emit events about the progress of storage tasks.
+ */
+abstract class StorageProgress<T> private constructor() {


Not using a sealed class here to avoid binary incompatibility in a future release if some day we potentially include a new listener/event as part of progress monitoring.

Binary incompatibility is well explained here: https://jakewharton.com/public-api-challenges-in-kotlin/#binary-compatibility

firebase-storage/ktx/src/main/kotlin/com/google/firebase/storage/ktx/Storage.kt

vkryachko · 2022-09-23T13:06:08Z

@thatfiredev Thanks for all the work on Kotlin improvements!

I was wondering if you had interest and cycles to drive the following changes: Flow<FirebaseUser> for auth's current user and fun trace(name:String) for perf 😄

thatfiredev · 2022-09-23T13:15:21Z

@vkryachko I was actually digging through auth-ktx on g3 to figure out how to implement that 🤣 Will send a CL soon.

Do you think the trace ktx will require an API Council review?

vkryachko · 2022-09-23T13:25:20Z

@vkryachko I was actually digging through auth-ktx on g3 to figure out how to implement that 🤣 Will send a CL soon.

Yeah that one is a different experience for sure, let me know if you need any pointers :)

Do you think the trace ktx will require an API Council review?

I think it's best to follow the process, I expect it to be a very light and quick review

thatfiredev · 2022-09-23T13:28:09Z

@vkryachko I just noticed that in my original API proposal, I called this StorageTask.taskState. Do you think we should keep it that way?

    var uploadTask = ref.putFile(...)
    uploadTask.taskState.collect { state ->
        when (state) {
            is TaskState.InProgress -> {

            }
            is TaskState.Paused -> {
                
            }
        }
    }

vkryachko · 2022-09-23T13:32:52Z

Yeah, let's stick to the api proposal's naming

firebase-storage/ktx/src/main/kotlin/com/google/firebase/storage/ktx/Storage.kt

vkryachko

Just realized that I forgot to "send" the comment, sending now

firebase-storage/ktx/src/main/kotlin/com/google/firebase/storage/ktx/Storage.kt

* Deflake firebase_common HeartBeat tests. (#4083) The tests relied on `TestOnCompleteListener` that was not safe to call more than once since it was based on a count down latch. So reusing it multiple times would cause await() to return immediately. This change makes it so that a new latch is created for every await() call, making all await() calls work. Fixes: http://b/245956774 * Add Javadoc support to the DackkaPlugin (#4082) * Add util method for copying directories * Add javadoc support to our dackka plugin * Remove the extension check on fromDirectory * Add a note about cache compliance and the javadoc task * Add reference to kotlin stdlib package list (#4093) * Add appcheck-ktx to bom config (#4081) * Removing getRunningAppProcesses since the process_name isn't used (#4057) * Fix Documentation classpath (#4099) b/241795594 * Add projectSpecificSources back to the DackkaPlugin (#4110) * Added extra method for TaskProviders * Added specificSources method back * Revert to dependsOn for docstubs dep * Revamp test harness for macrobenchmark tests (#4071) * Fix dependabot security alerts (#4123) * Make firesite transform cacheable (#4124) * add coroutines-play-services as a transitive dep to firebase-common-ktx (#4044) * add kotlinx-coroutines-play-services as a transitive dep to firebase-common-ktx * Update to Coroutines 1.6.4 * database-ktx: add callbackFlow for eventlisteners (#4012) * add callbackFlow for RTDB's ValueEventListener * add callbackFlow for RTDB's ChildEventListener * delegate trySendBlocking to DefaultRunLoop * add group to ktx.gradle * update api.txt file * Update released versions (#4135) * Upgrade dackkaConfig (#4141) * Add names to all Firebase components (#4117) * Add appcheck's ktx artifact back to package list file (#4142) * Add strict mode testing in firebase-messaging (#4095) * Add gralde property to instrument Fireperf E2E test (#4144) The perf gradle PR is #334 in the gradle repo. b/246802885 * Resolve StrictMode violation in App Check. (#4085) * Resolve StrictMode violation in App Check. * Attempt to fix some tests. * Fix unit tests. * Make `retrieveStoredAppCheckTokenInBackground` private instead of package-private. * Move listener invocations back to the main thread while keeping disk write on background thread. * Refactor to use lambda syntax. * Implemented exponential backoff and max retry with resumable uploads (#4087) * storage-ktx: add callbackFlow for upload/download progress (#4139) * add kotlin flows to storage * update api.txt file * add group to storage/ktx.gradle * Make a best effort attempt to flush reports at crash time (#4112) This should allow us to upload reports for start-up crashes. * Public Count (#4130) * Public Count * Disable prod testing * Long to long * Api.txt * Backfill changelog * Add PR * Fix assertEquals error * Re-write API javadocs for COUNT API (#4143) Co-authored-by: Denver Coneybeare <[email protected]> * [Fireperf][AASA] send `_experiment_app_start_ttid` trace, controlled by RC flag (#4114) * log _experiment_as_ttid * send event and RRC mitigation * add RC wip * modified save to cache when RC fetches * dev-app manifest override * unit test for RCc cache saving * better name and comments * better formatting remoteconfigmanagertest * better comments and added local RC lookup back * Specify unique ref tags in Dackka output (#4149) * Add util methods for gradle projects * Disable Javadoc generation on empty projects * Fix ref path generation in Dackka output * Add documentation for util methods * Update the DackkaPlugin docs * Reduced path to relative from tenant * Reduced ref head path even more * Fixed ref tag path to working solution * Disabled publishJavadoc by default for tests It should be enabled explicitly when being tested anyhow, and causes issues otherwise. Instead of disabling it for the tests that don't need it- this is much quicker and easier to manager. * Add strict mode tests to inappmessaging and inappmessaging-display (#4136) * Fix strict mode violations for appcheck (#4148) * Fix strict mode violations for appcheck * Formatting * Add copyright header * Populate SDKs changelog files (#4070) * first try seeding changelogs * Added unreleased section to CHANGELOG * Fix empty lines between sections * Add missing entries for abt * Update data to include latest releases * Update CHANGELOG.md * Add missing line in unreleased section for perf. * Enable CHANGELOG check globally (#4084) * Enable CHANGELOG check globally * Simplify conditional. * Enable COUNT integration tests, now that backend support has rolled out (#4163) * Remove separation of kotlin directories in dackka (#4166) * Deprecate App Check SafetyNet SDK (#4187) * Add `@Deprecated` annotations to Firebase App Check SafetyNet SDK public API. * Add `@deprecated` tag in the Javadoc as well. * Remove stale entries from Unreleased section. (#4185) * Assign ConfigContainer Builder return values. (#4194) * update bom (#4155) * update bom * update * update * feat(perf-ktx): add trace(name, block) extension function (#4180) * Remove smoke test for app indexing (#4219) App Indexing is deprecated starting BoM 31.0.0 . https://firebase.google.com/support/release-notes/android#bom_v31-0-0 * Bump Robolectric to 4.9 (#4161) * Add plexus-utils for firebase-database tests Looks like firebase-database tests use plexus-utils dependency of Robolectric directly. But this dependency was removed by Robolectric. So this CL adds plexus-utils explictly for firebase-database tests. Signed-off-by: utzcoz <[email protected]> * Bump Robolectric to 4.9 1. Use legacy LooperMode for tests explicitly, because recent Robolectric releases switch to use PAUSED mode default. Before these tests migrate to PAUSED mode, they use LEGACY mode to pass tests. 2. Migrate Assert.assertThat to Truth.assertThat to avoid using removed APIs. 3. All build.gradle use the same robolectricVersion except transport-backend-cct because Robolectric 4.8+ has compatibility problem for TelephonyManager with low compile/targetSdkversion. To keep httpclient compatibility, transport-backend-ccts continues to use Robolectric 4.3.1. 4. Remove unused exclude protobuf-java from Robolectric. 5. Add necessary protobuf-lite dependency on classpath for some ktx modules' tests. Signed-off-by: utzcoz <[email protected]> Signed-off-by: utzcoz <[email protected]> * return exception if modelname is empty (#4226) * Add "create release PR" github action (#4236) This implementation: - Creates the base branch (name is based in user input) - Creates the release branch (name is based in user input) - Creates the release.cfg file in the release branch without adding any SDK (module) to it. It can create the branches based on any existing branch of the repo. * Sync spec tests from web SDK to Android SDK (#4230) * Update versions (#4238) * Update versions * Exclude .github dir from `firebaseContinuousIntegration` paths (#4239) * Performing IN expansion (#4221) * WIP: `in` expansion. * Add composite filter in-expansion test. * Fix formatting. * Run in-expansion as part of DNF computation and add tests. * Add test with nested IN filters with CSI. * Add tests for other cases. * typo fix (#4237) * Firestore: Add test that verifies count query error message when missing index (#4232) * refactor(functions): update firebase-iid to 21.1.0 (#4225) * refactor(functions): update firebase-iid to 21.1.0 * Update CHANGELOG.md * Update CHANGELOG.md * bump firebase-iid-interop to 17.1.0 * exclude firebase-components from firebase-iid dependency Signed-off-by: utzcoz <[email protected]> Co-authored-by: Vladimir Kryachko <[email protected]> Co-authored-by: Daymon <[email protected]> Co-authored-by: Raymond Lam <[email protected]> Co-authored-by: Yifan Yang <[email protected]> Co-authored-by: Rosário Pereira Fernandes <[email protected]> Co-authored-by: emilypgoogle <[email protected]> Co-authored-by: Jeremy Jiang <[email protected]> Co-authored-by: Rosalyn Tan <[email protected]> Co-authored-by: Maneesh Tewani <[email protected]> Co-authored-by: Matthew Robertson <[email protected]> Co-authored-by: wu-hui <[email protected]> Co-authored-by: Denver Coneybeare <[email protected]> Co-authored-by: Leo Zhan <[email protected]> Co-authored-by: Rodrigo Lazo <[email protected]> Co-authored-by: Dana Silver <[email protected]> Co-authored-by: Vinay Guthal <[email protected]> Co-authored-by: utzcoz <[email protected]> Co-authored-by: argzdev <[email protected]> Co-authored-by: Mila <[email protected]> Co-authored-by: Ehsan <[email protected]> Co-authored-by: cherylEnkidu <[email protected]>

thatfiredev added 3 commits September 23, 2022 11:20

add kotlin flows to storage

535c184

Merge branch 'master' of https://github.com/firebase/firebase-android…

2319f36

…-sdk into rpf-storage-flows

use project-wide coroutinesVersion

27ed605

thatfiredev requested review from vkryachko and rlazo September 23, 2022 10:26

google-oss-bot added the size/L label Sep 23, 2022

thatfiredev commented Sep 23, 2022

View reviewed changes

thatfiredev added 2 commits September 23, 2022 11:34

update api.txt file

cf62e3f

add group to storage/ktx.gradle

471edc5

vkryachko reviewed Sep 23, 2022

View reviewed changes

firebase-storage/ktx/src/main/kotlin/com/google/firebase/storage/ktx/Storage.kt Outdated Show resolved Hide resolved

thatfiredev added 2 commits September 23, 2022 14:35

StorageProgress -> TaskState

8f0d7c7

update api.txt file

bf8507a

google-oss-bot added size/M and removed size/L labels Sep 23, 2022

maneesht reviewed Sep 23, 2022

View reviewed changes

firebase-storage/ktx/src/main/kotlin/com/google/firebase/storage/ktx/Storage.kt Show resolved Hide resolved

thatfiredev requested a review from vkryachko September 26, 2022 17:10

vkryachko reviewed Sep 26, 2022

View reviewed changes

firebase-storage/ktx/src/main/kotlin/com/google/firebase/storage/ktx/Storage.kt Outdated Show resolved Hide resolved

add OnCompleteListener to cancel/close the flow

7a27083

vkryachko approved these changes Sep 26, 2022

View reviewed changes

add OnCompleteListener to ref doc

3d7dc0f

thatfiredev merged commit 4131616 into firebase:master Sep 26, 2022

firebase locked and limited conversation to collaborators Oct 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

storage-ktx: add callbackFlow for upload/download progress #4139

storage-ktx: add callbackFlow for upload/download progress #4139

Uh oh!

thatfiredev commented Sep 23, 2022

Uh oh!

thatfiredev Sep 23, 2022

Uh oh!

thatfiredev Sep 23, 2022

Uh oh!

Uh oh!

vkryachko commented Sep 23, 2022

Uh oh!

thatfiredev commented Sep 23, 2022

Uh oh!

vkryachko commented Sep 23, 2022 •

edited

Loading

Uh oh!

thatfiredev commented Sep 23, 2022

Uh oh!

vkryachko commented Sep 23, 2022

Uh oh!

Uh oh!

vkryachko left a comment

Uh oh!

Uh oh!

Uh oh!

		addOnProgressListener(progressListener)
		addOnPausedListener(pauseListener)

	private static final ThreadPoolExecutor CALLBACK_QUEUE_EXECUTOR =
	new ThreadPoolExecutor(
	1, 1, 5, TimeUnit.SECONDS, mCallbackQueue, new StorageThreadFactory("Callbacks-"));

	StorageTaskScheduler.getInstance().scheduleCallback {
	trySendBlocking(StorageProgress.InProgress(snapshot))
	}

storage-ktx: add callbackFlow for upload/download progress #4139

storage-ktx: add callbackFlow for upload/download progress #4139

Uh oh!

Conversation

thatfiredev commented Sep 23, 2022

Uh oh!

thatfiredev Sep 23, 2022

Choose a reason for hiding this comment

Uh oh!

thatfiredev Sep 23, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vkryachko commented Sep 23, 2022

Uh oh!

thatfiredev commented Sep 23, 2022

Uh oh!

vkryachko commented Sep 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thatfiredev commented Sep 23, 2022

Uh oh!

vkryachko commented Sep 23, 2022

Uh oh!

Uh oh!

vkryachko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vkryachko commented Sep 23, 2022 •

edited

Loading