Sample/firestore database storage#2406
Conversation
…stUtils in paging test
…instance and add unit tests
Bumps com.gradle.develocity from 4.4.3 to 4.5.0. --- updated-dependencies: - dependency-name: com.gradle.develocity dependency-version: 4.5.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/stale](https://github.com/actions/stale) from 10.3.0 to 10.4.0. - [Release notes](https://github.com/actions/stale/releases) - [Changelog](https://github.com/actions/stale/blob/main/CHANGELOG.md) - [Commits](actions/stale@eb5cf3a...1e223db) --- updated-dependencies: - dependency-name: actions/stale dependency-version: 10.4.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps androidx.constraintlayout:constraintlayout from 2.0.4 to 2.2.1. --- updated-dependencies: - dependency-name: androidx.constraintlayout:constraintlayout dependency-version: 2.2.1 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps androidx.activity:activity-compose from 1.11.0 to 1.13.0. --- updated-dependencies: - dependency-name: androidx.activity:activity-compose dependency-version: 1.13.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [org.jetbrains.dokka](https://github.com/Kotlin/dokka) from 2.0.0 to 2.2.0. - [Release notes](https://github.com/Kotlin/dokka/releases) - [Commits](Kotlin/dokka@v2.0.0...v2.2.0) --- updated-dependencies: - dependency-name: org.jetbrains.dokka dependency-version: 2.2.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/setup-node](https://github.com/actions/setup-node) from 6.4.0 to 7.0.0. - [Release notes](https://github.com/actions/setup-node/releases) - [Commits](actions/setup-node@48b55a0...8207627) --- updated-dependencies: - dependency-name: actions/setup-node dependency-version: 7.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps androidx.test:runner from 1.5.0 to 1.7.0. --- updated-dependencies: - dependency-name: androidx.test:runner dependency-version: 1.7.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…2401) Bumps [org.mockito:mockito-android](https://github.com/mockito/mockito) from 2.21.0 to 5.23.0. - [Release notes](https://github.com/mockito/mockito/releases) - [Commits](mockito/mockito@v2.21.0...v5.23.0) --- updated-dependencies: - dependency-name: org.mockito:mockito-android dependency-version: 5.23.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…#2402) Bumps [com.github.bumptech.glide:glide](https://github.com/bumptech/glide) from 5.0.7 to 5.0.9. - [Release notes](https://github.com/bumptech/glide/releases) - [Commits](bumptech/glide@v5.0.7...v5.0.9) --- updated-dependencies: - dependency-name: com.github.bumptech.glide:glide dependency-version: 5.0.9 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
# Conflicts: # app/src/main/AndroidManifest.xml # app/src/main/java/com/firebaseui/android/demo/MainActivity.kt # app/src/main/java/com/firebaseui/android/demo/auth/HighLevelApiDemoActivity.kt # storage/build.gradle.kts
There was a problem hiding this comment.
Code Review
This pull request restructures the demo application by modularizing the Auth demos and introducing new demonstration activities for Firebase Realtime Database, Firestore, and Storage (using Glide). It also updates FirestorePagingSource to properly handle InterruptedException and avoid undeliverable RxJava exceptions. Feedback focuses on several key improvements: configuring and initializing the Realtime Database emulator, exposing and passing the emulator mode to the Auth chooser screen, resolving a bug in the Storage demo where Glide loads are triggered on every keystroke, programmatically converting dp to pixels for view holder padding to avoid raw pixel anti-patterns, and optimizing Firestore data seeding using a WriteBatch.
| // 10.0.2.2 is the Android emulator's alias for the host machine's localhost. | ||
| private const val USE_FIRESTORE_EMULATOR = true | ||
| private const val FIRESTORE_EMULATOR_HOST = "10.0.2.2" | ||
| private const val FIRESTORE_EMULATOR_PORT = 8080 | ||
| } |
There was a problem hiding this comment.
The Realtime Database emulator configuration is missing. If a developer runs the database demo while emulators are enabled, it will attempt to connect to production instead of the local emulator. Let's add the database emulator constants here.
// 10.0.2.2 is the Android emulator's alias for the host machine's localhost.
private const val USE_FIRESTORE_EMULATOR = true
private const val FIRESTORE_EMULATOR_HOST = "10.0.2.2"
private const val FIRESTORE_EMULATOR_PORT = 8080
private const val USE_DATABASE_EMULATOR = true
private const val DATABASE_EMULATOR_HOST = "10.0.2.2"
private const val DATABASE_EMULATOR_PORT = 8199
}| if (USE_FIRESTORE_EMULATOR) { | ||
| FirebaseFirestore.getInstance() | ||
| .useEmulator(FIRESTORE_EMULATOR_HOST, FIRESTORE_EMULATOR_PORT) | ||
| } |
There was a problem hiding this comment.
Initialize the Realtime Database emulator if USE_DATABASE_EMULATOR is enabled, ensuring consistent emulator usage across all modules.
| if (USE_FIRESTORE_EMULATOR) { | |
| FirebaseFirestore.getInstance() | |
| .useEmulator(FIRESTORE_EMULATOR_HOST, FIRESTORE_EMULATOR_PORT) | |
| } | |
| if (USE_FIRESTORE_EMULATOR) { | |
| FirebaseFirestore.getInstance() | |
| .useEmulator(FIRESTORE_EMULATOR_HOST, FIRESTORE_EMULATOR_PORT) | |
| } | |
| if (USE_DATABASE_EMULATOR) { | |
| com.google.firebase.database.FirebaseDatabase.getInstance() | |
| .useEmulator(DATABASE_EMULATOR_HOST, DATABASE_EMULATOR_PORT) | |
| } |
| var gsUrl by remember { mutableStateOf("") } | ||
| var stringStatus by remember { mutableStateOf("Not loaded") } | ||
| var stringLoadKey by remember { mutableIntStateOf(0) } | ||
| var refStatus by remember { mutableStateOf("Not loaded") } | ||
| var refLoadKey by remember { mutableIntStateOf(0) } |
There was a problem hiding this comment.
Using stringLoadKey and refLoadKey to trigger Glide loads in AndroidView's update block is problematic. Because update reads gsUrl directly, any keystroke typed in the text field after clicking "Load" once will immediately trigger a new Glide load with the partial URL. This causes severe flickering, unnecessary network requests, and incorrect status displays. Let's capture the actual URL to load when the button is clicked.
| var gsUrl by remember { mutableStateOf("") } | |
| var stringStatus by remember { mutableStateOf("Not loaded") } | |
| var stringLoadKey by remember { mutableIntStateOf(0) } | |
| var refStatus by remember { mutableStateOf("Not loaded") } | |
| var refLoadKey by remember { mutableIntStateOf(0) } | |
| var gsUrl by remember { mutableStateOf("") } | |
| var stringStatus by remember { mutableStateOf("Not loaded") } | |
| var stringUrlToLoad by remember { mutableStateOf("") } | |
| var refStatus by remember { mutableStateOf("Not loaded") } | |
| var refUrlToLoad by remember { mutableStateOf("") } |
| Button(onClick = { | ||
| stringStatus = "Loading..." | ||
| stringLoadKey++ | ||
| }) { Text("Load") } | ||
| StatusText(stringStatus) | ||
| AndroidView( | ||
| factory = { ImageView(it) }, | ||
| update = { view -> | ||
| if (stringLoadKey > 0) { | ||
| GlideApp.with(view) | ||
| .load(gsUrl) | ||
| .listener(glideListener { success, error -> | ||
| stringStatus = if (success) "Loaded" else "Error: $error" | ||
| }) | ||
| .into(view) | ||
| } | ||
| }, |
There was a problem hiding this comment.
Update Approach 1 to load the image only when stringUrlToLoad is updated, preventing loads on every keystroke.
Button(onClick = {
if (gsUrl.isNotEmpty()) {
stringStatus = "Loading..."
stringUrlToLoad = gsUrl
}
}) { Text("Load") }
StatusText(stringStatus)
AndroidView(
factory = { ImageView(it) },
update = { view ->
if (stringUrlToLoad.isNotEmpty()) {
GlideApp.with(view)
.load(stringUrlToLoad)
.listener(glideListener { success, error ->
stringStatus = if (success) "Loaded" else "Error: $error"
})
.into(view)
}
},| Button(onClick = { | ||
| refStatus = "Loading..." | ||
| refLoadKey++ | ||
| }) { Text("Load") } | ||
| StatusText(refStatus) | ||
| AndroidView( | ||
| factory = { ImageView(it) }, | ||
| update = { view -> | ||
| if (refLoadKey > 0) { | ||
| runCatching { | ||
| FirebaseStorage.getInstance().getReferenceFromUrl(gsUrl) | ||
| }.onSuccess { ref -> | ||
| GlideApp.with(view) | ||
| .load(ref) | ||
| .listener(glideListener { success, error -> | ||
| refStatus = if (success) "Loaded" else "Error: $error" | ||
| }) | ||
| .into(view) | ||
| }.onFailure { e -> | ||
| refStatus = "Invalid URL: ${e.message}" | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
Update Approach 2 to load the image only when refUrlToLoad is updated, preventing loads on every keystroke.
Button(onClick = {
if (gsUrl.isNotEmpty()) {
refStatus = "Loading..."
refUrlToLoad = gsUrl
}
}) { Text("Load") }
StatusText(refStatus)
AndroidView(
factory = { ImageView(it) },
update = { view ->
if (refUrlToLoad.isNotEmpty()) {
runCatching {
FirebaseStorage.getInstance().getReferenceFromUrl(refUrlToLoad)
}.onSuccess { ref ->
GlideApp.with(view)
.load(ref)
.listener(glideListener { success, error ->
refStatus = if (success) "Loaded" else "Error: $error"
})
.into(view)
}.onFailure { e ->
refStatus = "Invalid URL: ${e.message}"
}
}
},| class MainActivity : ComponentActivity() { | ||
| companion object { | ||
| private const val USE_AUTH_EMULATOR = false | ||
| private const val USE_AUTH_EMULATOR = true |
There was a problem hiding this comment.
The USE_AUTH_EMULATOR constant is currently private. Since the Auth chooser screen has been moved to AuthChooserActivity, we need to pass this value to AuthChooserScreen to display the emulator warning card correctly. Changing this visibility to internal allows AuthChooserActivity to access it.
| private const val USE_AUTH_EMULATOR = true | |
| internal const val USE_AUTH_EMULATOR = true |
| AuthChooserScreen( | ||
| onHighLevelApiClick = { | ||
| startActivity(Intent(this, HighLevelApiDemoActivity::class.java)) | ||
| }, | ||
| onLowLevelApiClick = { | ||
| startActivity(Intent(this, AuthFlowControllerDemoActivity::class.java)) | ||
| }, | ||
| onCustomSlotsClick = { | ||
| startActivity(Intent(this, CustomSlotsThemingDemoActivity::class.java)) | ||
| }, | ||
| onCredentialLinkingClick = { | ||
| startActivity(Intent(this, CredentialLinkingDemoActivity::class.java)) | ||
| } | ||
| ) |
There was a problem hiding this comment.
Pass isEmulatorMode = MainActivity.USE_AUTH_EMULATOR to AuthChooserScreen so that the emulator warning card is correctly displayed when running in emulator mode.
AuthChooserScreen(
onHighLevelApiClick = {
startActivity(Intent(this, HighLevelApiDemoActivity::class.java))
},
onLowLevelApiClick = {
startActivity(Intent(this, AuthFlowControllerDemoActivity::class.java))
},
onCustomSlotsClick = {
startActivity(Intent(this, CustomSlotsThemingDemoActivity::class.java))
},
onCredentialLinkingClick = {
startActivity(Intent(this, CredentialLinkingDemoActivity::class.java))
},
isEmulatorMode = MainActivity.USE_AUTH_EMULATOR
)| class ScoreViewHolder(parent: ViewGroup) : RecyclerView.ViewHolder( | ||
| TextView(parent.context).apply { | ||
| layoutParams = RecyclerView.LayoutParams( | ||
| RecyclerView.LayoutParams.MATCH_PARENT, | ||
| RecyclerView.LayoutParams.WRAP_CONTENT | ||
| ) | ||
| setPadding(48, 24, 48, 24) | ||
| textSize = 16f | ||
| } | ||
| ) |
There was a problem hiding this comment.
Setting padding in raw pixels (48 and 24) is an anti-pattern in Android development. It will scale poorly across devices with different screen densities. Let's convert density-independent pixels (dp) to pixels programmatically.
class ScoreViewHolder(parent: ViewGroup) : RecyclerView.ViewHolder(
TextView(parent.context).apply {
layoutParams = RecyclerView.LayoutParams(
RecyclerView.LayoutParams.MATCH_PARENT,
RecyclerView.LayoutParams.WRAP_CONTENT
)
val density = context.resources.displayMetrics.density
val hPadding = (16 * density).toInt()
val vPadding = (8 * density).toInt()
setPadding(hPadding, vPadding, hPadding, vPadding)
textSize = 16f
}
)| private fun seedData(collection: CollectionReference) { | ||
| for (i in 1..50) { | ||
| collection.add(ScoreItem("Item $i", (1..100).random())) | ||
| } | ||
| } |
There was a problem hiding this comment.
Seeding 50 items individually in a loop causes 50 separate network requests. Using a Firestore WriteBatch is much more efficient and idiomatic for bulk writes.
private fun seedData(collection: CollectionReference) {
val batch = collection.firestore.batch()
for (i in 1..50) {
val docRef = collection.document()
batch.set(docRef, ScoreItem("Item $i", (1..100).random()))
}
batch.commit().addOnFailureListener { e ->
android.util.Log.e("FirestoreDemoActivity", "Failed to seed data", e)
}
}| class ScoreViewHolder(parent: ViewGroup) : RecyclerView.ViewHolder( | ||
| TextView(parent.context).apply { | ||
| layoutParams = RecyclerView.LayoutParams( | ||
| RecyclerView.LayoutParams.MATCH_PARENT, | ||
| RecyclerView.LayoutParams.WRAP_CONTENT | ||
| ) | ||
| setPadding(48, 24, 48, 24) | ||
| textSize = 16f | ||
| } | ||
| ) |
There was a problem hiding this comment.
Setting padding in raw pixels (48 and 24) is an anti-pattern in Android development. It will scale poorly across devices with different screen densities. Let's convert density-independent pixels (dp) to pixels programmatically.
class ScoreViewHolder(parent: ViewGroup) : RecyclerView.ViewHolder(
TextView(parent.context).apply {
layoutParams = RecyclerView.LayoutParams(
RecyclerView.LayoutParams.MATCH_PARENT,
RecyclerView.LayoutParams.WRAP_CONTENT
)
val density = context.resources.displayMetrics.density
val hPadding = (16 * density).toInt()
val vPadding = (8 * density).toInt()
setPadding(hPadding, vPadding, hPadding, vPadding)
textSize = 16f
}
)
No description provided.