From 14e9df3196349d85601d0e468ee9f846b7e3c3cb Mon Sep 17 00:00:00 2001 From: Oskar Codes Date: Thu, 10 Oct 2024 17:32:59 +0200 Subject: [PATCH 1/6] chore(firestore): Change signature of FirestoreReferenceList for less verbose code. This commit addresses the issue mentioned [here](https://github.com/SwEnt-Group13/Unio/pull/32#issuecomment-2402914659). --- .../model/firestore/FirestoreReferenceList.kt | 46 +++++++++---------- .../unio/model/firestore/ReferenceList.kt | 25 +++++++++- 2 files changed, 46 insertions(+), 25 deletions(-) diff --git a/app/src/main/java/com/android/unio/model/firestore/FirestoreReferenceList.kt b/app/src/main/java/com/android/unio/model/firestore/FirestoreReferenceList.kt index f24e22466..1d9f224a6 100644 --- a/app/src/main/java/com/android/unio/model/firestore/FirestoreReferenceList.kt +++ b/app/src/main/java/com/android/unio/model/firestore/FirestoreReferenceList.kt @@ -1,9 +1,9 @@ package com.android.unio.model.firestore +import android.util.Log +import com.google.firebase.firestore.CollectionReference import com.google.firebase.firestore.DocumentSnapshot -import com.google.firebase.firestore.FirebaseFirestore -import com.google.firebase.firestore.ktx.firestore -import com.google.firebase.ktx.Firebase +import com.google.firebase.firestore.FieldPath import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -17,23 +17,21 @@ import kotlinx.coroutines.flow.StateFlow * ``` * * @param T The type of the objects in the list. - * @property db The [FirebaseFirestore] instance to use. - * @property collectionPath The path to the Firestore collection that contains the objects. + * @property collection The reference to the Firestore collection. * @property hydrate A function that converts a [DocumentSnapshot] to a [T]. */ class FirestoreReferenceList( - private val db: FirebaseFirestore, - private val collectionPath: String, + private val collection: CollectionReference, private val hydrate: (DocumentSnapshot) -> T -) : ReferenceList { +) : ReferenceList { // The internal list of UIDs. - private var _uids = mutableListOf() + private val _uids = mutableListOf() // The internal list of objects. private val _list = MutableStateFlow>(emptyList()) // The public list of objects. - val list: StateFlow> = _list + override val list: StateFlow> = _list /** * Adds a UID to the list. @@ -55,37 +53,37 @@ class FirestoreReferenceList( /** Requests all documents from Firestore and updates the list. */ override fun requestAll() { - println("Requesting all") _list.value = emptyList() - _uids.forEach { uid -> - db.collection(collectionPath).document(uid).get().addOnSuccessListener { result -> - val item = hydrate(result) - _list.value += item - println("Added $item") - } - } + collection + .whereIn(FieldPath.documentId(), _uids) + .get() + .addOnSuccessListener { result -> + val items = result.documents.map { hydrate(it) } + _list.value = items + } + .addOnFailureListener { exception -> + Log.e("FirestoreReferenceList", "Failed to get documents", exception) + } } companion object { /** Creates a [FirestoreReferenceList] from a list of UIDs. */ fun fromList( list: List, - db: FirebaseFirestore = Firebase.firestore, - collectionPath: String, + collection: CollectionReference, hydrate: (DocumentSnapshot) -> T ): FirestoreReferenceList { - val result = FirestoreReferenceList(db, collectionPath, hydrate) + val result = FirestoreReferenceList(collection, hydrate) result.addAll(list) return result } /** Creates an empty [FirestoreReferenceList]. */ fun empty( - db: FirebaseFirestore = Firebase.firestore, - collectionPath: String, + collection: CollectionReference, hydrate: (DocumentSnapshot) -> T ): FirestoreReferenceList { - return FirestoreReferenceList(db, collectionPath, hydrate) + return FirestoreReferenceList(collection, hydrate) } } } diff --git a/app/src/main/java/com/android/unio/model/firestore/ReferenceList.kt b/app/src/main/java/com/android/unio/model/firestore/ReferenceList.kt index e4c3ccea6..68c5acdc3 100644 --- a/app/src/main/java/com/android/unio/model/firestore/ReferenceList.kt +++ b/app/src/main/java/com/android/unio/model/firestore/ReferenceList.kt @@ -1,9 +1,32 @@ package com.android.unio.model.firestore -interface ReferenceList { +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow + +interface ReferenceList { + val list: StateFlow> + fun add(uid: String) fun addAll(uids: List) fun requestAll() } + +class MockReferenceList : ReferenceList { + private val _uids = mutableListOf() + private val _list = MutableStateFlow>(emptyList()) + override val list: StateFlow> = _list + + override fun add(uid: String) { + _uids.add(uid) + } + + override fun addAll(uids: List) { + _uids.addAll(uids) + } + + override fun requestAll() { + _list.value = emptyList() + } +} From c556675644d0c5fe83876806d5a47caa7cdd2ffb Mon Sep 17 00:00:00 2001 From: Oskar Codes Date: Thu, 10 Oct 2024 17:34:42 +0200 Subject: [PATCH 2/6] chore(firestore): Adapt the existing code to use the updated ReferenceList interface and FirestoreReferenceList implementation. --- .../unio/model/association/Association.kt | 4 ++-- .../AssociationRepositoryFirestore.kt | 4 +++- .../com/android/unio/model/event/Event.kt | 8 ++++--- .../java/com/android/unio/model/user/User.kt | 4 ++-- .../model/user/UserRepositoryFirestore.kt | 3 +-- .../AssociationRepositoryFirestoreTest.kt | 21 ++++++++++------ .../model/association/ExploreViewModelTest.kt | 12 ++++++++-- .../event/EventRepositoryFirestoreTest.kt | 10 +++++--- .../model/user/UserRepositoryFirestoreTest.kt | 24 ++++++++++++------- .../com/android/unio/model/user/UserTest.kt | 9 ++++++- 10 files changed, 68 insertions(+), 31 deletions(-) diff --git a/app/src/main/java/com/android/unio/model/association/Association.kt b/app/src/main/java/com/android/unio/model/association/Association.kt index 364f12348..0e170d84e 100644 --- a/app/src/main/java/com/android/unio/model/association/Association.kt +++ b/app/src/main/java/com/android/unio/model/association/Association.kt @@ -1,6 +1,6 @@ package com.android.unio.model.association -import com.android.unio.model.firestore.FirestoreReferenceList +import com.android.unio.model.firestore.ReferenceList import com.android.unio.model.user.User data class Association( @@ -9,5 +9,5 @@ data class Association( val acronym: String = "", val fullName: String = "", val description: String = "", - val members: FirestoreReferenceList + val members: ReferenceList ) diff --git a/app/src/main/java/com/android/unio/model/association/AssociationRepositoryFirestore.kt b/app/src/main/java/com/android/unio/model/association/AssociationRepositoryFirestore.kt index bf42d9d33..57490fb61 100644 --- a/app/src/main/java/com/android/unio/model/association/AssociationRepositoryFirestore.kt +++ b/app/src/main/java/com/android/unio/model/association/AssociationRepositoryFirestore.kt @@ -4,10 +4,12 @@ import com.android.unio.model.firestore.FirestorePaths.ASSOCIATION_PATH import com.android.unio.model.firestore.FirestorePaths.USER_PATH import com.android.unio.model.firestore.FirestoreReferenceList import com.android.unio.model.user.UserRepositoryFirestore +import com.google.android.play.core.assetpacks.db import com.google.firebase.Firebase import com.google.firebase.auth.auth import com.google.firebase.firestore.DocumentSnapshot import com.google.firebase.firestore.FirebaseFirestore +import com.google.firebase.firestore.firestore class AssociationRepositoryFirestore(private val db: FirebaseFirestore) : AssociationRepository { @@ -58,7 +60,7 @@ class AssociationRepositoryFirestore(private val db: FirebaseFirestore) : Associ val members = FirestoreReferenceList.fromList( list = memberUids, - collectionPath = USER_PATH, + collection = Firebase.firestore.collection(USER_PATH), hydrate = UserRepositoryFirestore::hydrate) return Association( diff --git a/app/src/main/java/com/android/unio/model/event/Event.kt b/app/src/main/java/com/android/unio/model/event/Event.kt index a7f611f4b..26506d9d2 100644 --- a/app/src/main/java/com/android/unio/model/event/Event.kt +++ b/app/src/main/java/com/android/unio/model/event/Event.kt @@ -1,5 +1,7 @@ package com.android.unio.model.event +import com.android.unio.model.association.Association +import com.android.unio.model.firestore.ReferenceList import com.android.unio.model.map.Location import com.google.firebase.Timestamp import java.util.Date @@ -7,13 +9,13 @@ import java.util.Date data class Event( val uid: String = "", val title: String = "", - val organisers: List = mutableListOf(), - val taggedAssociations: List = mutableListOf(), + val organisers: ReferenceList, + val taggedAssociations: ReferenceList, val image: String = "", val description: String = "", val catchyDescription: String = "", val price: Double = 0.0, val date: Timestamp = Timestamp(Date()), val location: Location = Location(), - val types: List = mutableListOf() + val types: List = mutableListOf() ) diff --git a/app/src/main/java/com/android/unio/model/user/User.kt b/app/src/main/java/com/android/unio/model/user/User.kt index 666ccebb1..561b79d6a 100644 --- a/app/src/main/java/com/android/unio/model/user/User.kt +++ b/app/src/main/java/com/android/unio/model/user/User.kt @@ -1,11 +1,11 @@ package com.android.unio.model.user import com.android.unio.model.association.Association -import com.android.unio.model.firestore.FirestoreReferenceList +import com.android.unio.model.firestore.ReferenceList data class User( val uid: String, val name: String, val email: String, - val followingAssociations: FirestoreReferenceList + val followingAssociations: ReferenceList ) diff --git a/app/src/main/java/com/android/unio/model/user/UserRepositoryFirestore.kt b/app/src/main/java/com/android/unio/model/user/UserRepositoryFirestore.kt index 7ca200251..90b558188 100644 --- a/app/src/main/java/com/android/unio/model/user/UserRepositoryFirestore.kt +++ b/app/src/main/java/com/android/unio/model/user/UserRepositoryFirestore.kt @@ -53,8 +53,7 @@ class UserRepositoryFirestore(private val db: FirebaseFirestore) : UserRepositor val followingAssociations = FirestoreReferenceList.fromList( followingAssociationsUids, - db, - ASSOCIATION_PATH, + db.collection(ASSOCIATION_PATH), AssociationRepositoryFirestore::hydrate) return User( diff --git a/app/src/test/java/com/android/unio/model/association/AssociationRepositoryFirestoreTest.kt b/app/src/test/java/com/android/unio/model/association/AssociationRepositoryFirestoreTest.kt index 74cd80bf0..a179c2167 100644 --- a/app/src/test/java/com/android/unio/model/association/AssociationRepositoryFirestoreTest.kt +++ b/app/src/test/java/com/android/unio/model/association/AssociationRepositoryFirestoreTest.kt @@ -1,6 +1,7 @@ package com.android.unio.model.association import androidx.test.core.app.ApplicationProvider +import com.android.unio.model.firestore.FirestorePaths.ASSOCIATION_PATH import com.android.unio.model.firestore.FirestorePaths.USER_PATH import com.android.unio.model.firestore.FirestoreReferenceList import com.android.unio.model.user.UserRepositoryFirestore @@ -27,7 +28,8 @@ import org.robolectric.RobolectricTestRunner @RunWith(RobolectricTestRunner::class) class AssociationRepositoryFirestoreTest { @Mock private lateinit var db: FirebaseFirestore - @Mock private lateinit var collectionReference: CollectionReference + @Mock private lateinit var associationCollectionReference: CollectionReference + @Mock private lateinit var userCollectionReference: CollectionReference @Mock private lateinit var querySnapshot: QuerySnapshot @Mock private lateinit var queryDocumentSnapshot1: QueryDocumentSnapshot @Mock private lateinit var queryDocumentSnapshot2: QueryDocumentSnapshot @@ -49,6 +51,9 @@ class AssociationRepositoryFirestoreTest { FirebaseApp.initializeApp(ApplicationProvider.getApplicationContext()) } + `when`(db.collection(eq(ASSOCIATION_PATH))).thenReturn(associationCollectionReference) + `when`(db.collection(eq(USER_PATH))).thenReturn(userCollectionReference) + association1 = Association( uid = "1", @@ -58,7 +63,7 @@ class AssociationRepositoryFirestoreTest { "ACM is the world's largest educational and scientific computing society.", members = FirestoreReferenceList.fromList( - listOf("1", "2"), db, USER_PATH, UserRepositoryFirestore::hydrate)) + listOf("1", "2"), db.collection(USER_PATH), UserRepositoryFirestore::hydrate)) association2 = Association( @@ -69,12 +74,12 @@ class AssociationRepositoryFirestoreTest { "IEEE is the world's largest technical professional organization dedicated to advancing technology for the benefit of humanity.", members = FirestoreReferenceList.fromList( - listOf("3", "4"), db, USER_PATH, UserRepositoryFirestore::hydrate)) + listOf("3", "4"), db.collection(USER_PATH), UserRepositoryFirestore::hydrate)) // When getting the collection, return the task - `when`(db.collection(eq("associations"))).thenReturn(collectionReference) - `when`(collectionReference.get()).thenReturn(querySnapshotTask) - `when`(collectionReference.document(eq(association1.uid))).thenReturn(documentReference) + `when`(associationCollectionReference.get()).thenReturn(querySnapshotTask) + `when`(associationCollectionReference.document(eq(association1.uid))) + .thenReturn(documentReference) `when`(documentReference.get()).thenReturn(documentSnapshotTask) // When the query snapshot is iterated, return the two query document snapshots @@ -139,7 +144,9 @@ class AssociationRepositoryFirestoreTest { val emptyAssociation = Association( uid = association2.uid, - members = FirestoreReferenceList.empty(db, "", UserRepositoryFirestore::hydrate)) + members = + FirestoreReferenceList.empty( + db.collection(USER_PATH), UserRepositoryFirestore::hydrate)) assertEquals(2, associations.size) diff --git a/app/src/test/java/com/android/unio/model/association/ExploreViewModelTest.kt b/app/src/test/java/com/android/unio/model/association/ExploreViewModelTest.kt index 21aded2ac..ae20187f8 100644 --- a/app/src/test/java/com/android/unio/model/association/ExploreViewModelTest.kt +++ b/app/src/test/java/com/android/unio/model/association/ExploreViewModelTest.kt @@ -3,6 +3,7 @@ package com.android.unio.model.association import com.android.unio.model.firestore.FirestorePaths.USER_PATH import com.android.unio.model.firestore.FirestoreReferenceList import com.android.unio.model.user.UserRepositoryFirestore +import com.google.firebase.firestore.CollectionReference import com.google.firebase.firestore.FirebaseFirestore import junit.framework.TestCase.assertEquals import kotlinx.coroutines.Dispatchers @@ -24,6 +25,7 @@ import org.mockito.kotlin.any class ExploreViewModelTest { @Mock private lateinit var repository: AssociationRepositoryFirestore @Mock private lateinit var db: FirebaseFirestore + @Mock private lateinit var collectionReference: CollectionReference private lateinit var viewModel: ExploreViewModel @@ -36,6 +38,8 @@ class ExploreViewModelTest { MockitoAnnotations.openMocks(this) Dispatchers.setMain(testDispatcher) + `when`(db.collection(any())).thenReturn(collectionReference) + testAssociations = listOf( Association( @@ -46,7 +50,9 @@ class ExploreViewModelTest { "ACM is the world's largest educational and scientific computing society.", members = FirestoreReferenceList.fromList( - listOf("1", "2"), db, USER_PATH, UserRepositoryFirestore::hydrate)), + listOf("1", "2"), + db.collection(USER_PATH), + UserRepositoryFirestore::hydrate)), Association( uid = "2", acronym = "IEEE", @@ -54,7 +60,9 @@ class ExploreViewModelTest { description = "IEEE is the world's largest technical professional organization.", members = FirestoreReferenceList.fromList( - listOf("3", "4"), db, USER_PATH, UserRepositoryFirestore::hydrate))) + listOf("3", "4"), + db.collection(USER_PATH), + UserRepositoryFirestore::hydrate))) viewModel = ExploreViewModel(repository) } diff --git a/app/src/test/java/com/android/unio/model/event/EventRepositoryFirestoreTest.kt b/app/src/test/java/com/android/unio/model/event/EventRepositoryFirestoreTest.kt index da8fd1d16..5d058b38c 100644 --- a/app/src/test/java/com/android/unio/model/event/EventRepositoryFirestoreTest.kt +++ b/app/src/test/java/com/android/unio/model/event/EventRepositoryFirestoreTest.kt @@ -1,5 +1,6 @@ package com.android.unio.model.event +import com.android.unio.model.firestore.MockReferenceList import com.android.unio.model.map.Location import com.google.android.gms.tasks.OnSuccessListener import com.google.android.gms.tasks.Task @@ -43,12 +44,14 @@ class EventRepositoryFirestoreTest { @Mock private lateinit var voidTask: Task private lateinit var repository: EventRepositoryFirestore - private val defaultEvent = Event() + private val defaultEvent = + Event(organisers = MockReferenceList(), taggedAssociations = MockReferenceList()) private val event1 = Event( uid = "1", title = "Balelec", - organisers = mutableListOf("Balelec, EPFL"), + organisers = MockReferenceList(), + taggedAssociations = MockReferenceList(), image = "", description = "Plus grand festival du monde (non contractuel)", price = 40.5, @@ -58,7 +61,8 @@ class EventRepositoryFirestoreTest { Event( uid = "3", title = "Tremplin Sysmic", - organisers = mutableListOf("Sysmic, EPFL"), + organisers = MockReferenceList(), + taggedAssociations = MockReferenceList(), image = "", description = "Plus grand festival du monde (non contractuel)", price = 40.5, diff --git a/app/src/test/java/com/android/unio/model/user/UserRepositoryFirestoreTest.kt b/app/src/test/java/com/android/unio/model/user/UserRepositoryFirestoreTest.kt index 4e83dac30..580d76a39 100644 --- a/app/src/test/java/com/android/unio/model/user/UserRepositoryFirestoreTest.kt +++ b/app/src/test/java/com/android/unio/model/user/UserRepositoryFirestoreTest.kt @@ -2,6 +2,8 @@ package com.android.unio.model.user import androidx.test.core.app.ApplicationProvider import com.android.unio.model.association.AssociationRepositoryFirestore +import com.android.unio.model.firestore.FirestorePaths.ASSOCIATION_PATH +import com.android.unio.model.firestore.FirestorePaths.USER_PATH import com.android.unio.model.firestore.FirestoreReferenceList import com.google.android.gms.tasks.OnSuccessListener import com.google.android.gms.tasks.Task @@ -26,7 +28,8 @@ import org.robolectric.RobolectricTestRunner @RunWith(RobolectricTestRunner::class) class UserRepositoryFirestoreTest { @Mock private lateinit var db: FirebaseFirestore - @Mock private lateinit var collectionReference: CollectionReference + @Mock private lateinit var userCollectionReference: CollectionReference + @Mock private lateinit var associationCollectionReference: CollectionReference @Mock private lateinit var querySnapshot: QuerySnapshot @Mock private lateinit var queryDocumentSnapshot1: QueryDocumentSnapshot @Mock private lateinit var queryDocumentSnapshot2: QueryDocumentSnapshot @@ -48,13 +51,18 @@ class UserRepositoryFirestoreTest { FirebaseApp.initializeApp(ApplicationProvider.getApplicationContext()) } + // When getting the collection, return the task + `when`(db.collection(eq(USER_PATH))).thenReturn(userCollectionReference) + `when`(db.collection(eq(ASSOCIATION_PATH))).thenReturn(associationCollectionReference) + user1 = User( uid = "1", email = "example1@abcd.com", name = "Example 1", followingAssociations = - FirestoreReferenceList.empty(db, "", AssociationRepositoryFirestore::hydrate)) + FirestoreReferenceList.empty( + db.collection(ASSOCIATION_PATH), AssociationRepositoryFirestore::hydrate)) user2 = User( @@ -62,12 +70,11 @@ class UserRepositoryFirestoreTest { email = "example2@abcd.com", name = "Example 2", followingAssociations = - FirestoreReferenceList.empty(db, "", AssociationRepositoryFirestore::hydrate)) + FirestoreReferenceList.empty( + db.collection(ASSOCIATION_PATH), AssociationRepositoryFirestore::hydrate)) - // When getting the collection, return the task - `when`(db.collection(eq("users"))).thenReturn(collectionReference) - `when`(collectionReference.get()).thenReturn(querySnapshotTask) - `when`(collectionReference.document(eq(user1.uid))).thenReturn(documentReference) + `when`(userCollectionReference.get()).thenReturn(querySnapshotTask) + `when`(userCollectionReference.document(eq(user1.uid))).thenReturn(documentReference) `when`(documentReference.get()).thenReturn(documentSnapshotTask) // When the query snapshot is iterated, return the two query document snapshots @@ -134,7 +141,8 @@ class UserRepositoryFirestoreTest { email = "", name = "", followingAssociations = - FirestoreReferenceList.empty(db, "", AssociationRepositoryFirestore::hydrate)) + FirestoreReferenceList.empty( + db.collection(ASSOCIATION_PATH), AssociationRepositoryFirestore::hydrate)) assertEquals(2, users.size) assertEquals(user1.uid, users[0].uid) diff --git a/app/src/test/java/com/android/unio/model/user/UserTest.kt b/app/src/test/java/com/android/unio/model/user/UserTest.kt index 9d75bb35b..a8cbeb266 100644 --- a/app/src/test/java/com/android/unio/model/user/UserTest.kt +++ b/app/src/test/java/com/android/unio/model/user/UserTest.kt @@ -1,20 +1,27 @@ package com.android.unio.model.user import com.android.unio.model.association.AssociationRepositoryFirestore +import com.android.unio.model.firestore.FirestorePaths.ASSOCIATION_PATH import com.android.unio.model.firestore.FirestoreReferenceList +import com.google.firebase.firestore.CollectionReference import com.google.firebase.firestore.FirebaseFirestore import junit.framework.TestCase.assertEquals import org.junit.Before import org.junit.Test import org.mockito.Mock +import org.mockito.Mockito.`when` import org.mockito.MockitoAnnotations +import org.mockito.kotlin.any class UserTest { @Mock private lateinit var db: FirebaseFirestore + @Mock private lateinit var collectionReference: CollectionReference @Before fun setUp() { MockitoAnnotations.openMocks(this) + + `when`(db.collection(any())).thenReturn(collectionReference) } @Test @@ -25,7 +32,7 @@ class UserTest { "John", "john@example.com", FirestoreReferenceList.empty( - db, "associations", AssociationRepositoryFirestore::hydrate)) + db.collection(ASSOCIATION_PATH), AssociationRepositoryFirestore::hydrate)) assertEquals("1", user.uid) assertEquals("John", user.name) assertEquals("john@example.com", user.email) From 68fdf06236156a473041fd89a1bad17007226d4c Mon Sep 17 00:00:00 2001 From: Oskar Codes Date: Thu, 10 Oct 2024 17:35:36 +0200 Subject: [PATCH 3/6] chore(firestore): Adapt existing code to use the newly created MockReferenceList class --- .../model/association/MockAssociations.kt | 133 +++++++++--------- .../com/android/unio/ui/explore/Explore.kt | 2 +- 2 files changed, 66 insertions(+), 69 deletions(-) diff --git a/app/src/main/java/com/android/unio/model/association/MockAssociations.kt b/app/src/main/java/com/android/unio/model/association/MockAssociations.kt index 7573fbb8c..e683df09c 100644 --- a/app/src/main/java/com/android/unio/model/association/MockAssociations.kt +++ b/app/src/main/java/com/android/unio/model/association/MockAssociations.kt @@ -1,8 +1,7 @@ package com.android.unio.model.association -import com.android.unio.model.firestore.FirestorePaths.USER_PATH -import com.android.unio.model.firestore.FirestoreReferenceList -import com.android.unio.model.user.UserRepositoryFirestore +import com.android.unio.model.firestore.MockReferenceList +import com.android.unio.model.user.User enum class AssociationType { MUSIC, @@ -16,69 +15,67 @@ enum class AssociationType { data class MockAssociation(val association: Association, val type: AssociationType) -val emptyMembers = { - FirestoreReferenceList.empty( - collectionPath = USER_PATH, hydrate = UserRepositoryFirestore::hydrate) -} +val emptyMembers = { MockReferenceList() } -val mockAssociations = - listOf( - MockAssociation( - Association( - uid = "1", - acronym = "Musical", - fullName = "Musical Association", - description = - "AGEPoly Commission – stimulation of the practice of music on the campus", - members = emptyMembers()), - AssociationType.MUSIC), - MockAssociation( - Association( - uid = "2", - acronym = "Nuit De la Magistrale", - fullName = "Nuit De la Magistrale Association", - description = - "AGEPoly Commission – party following the formal Magistrale Graduation Ceremony", - members = emptyMembers()), - AssociationType.FESTIVALS), - MockAssociation( - Association( - uid = "3", - acronym = "Balélec", - fullName = "Festival Balélec", - description = "Open-air unique en Suisse, organisée par des bénévoles étudiants.", - members = emptyMembers()), - AssociationType.FESTIVALS), - MockAssociation( - Association( - uid = "4", - acronym = "Artiphys", - fullName = "Festival Artiphys", - description = "Festival à l'EPFL", - members = emptyMembers()), - AssociationType.FESTIVALS), - MockAssociation( - Association( - uid = "5", - acronym = "Sysmic", - fullName = "Festival Sysmic", - description = "Festival à l'EPFL", - members = emptyMembers()), - AssociationType.FESTIVALS), - MockAssociation( - Association( - uid = "6", - acronym = "IFL", - fullName = "Innovation Forum Lausanne", - description = "Innovation Forum Lausanne", - members = emptyMembers()), - AssociationType.INNOVATION), - MockAssociation( - Association( - uid = "7", - acronym = "Clic", - fullName = "Clic Association", - description = "Association of EPFL Students of IC Faculty", - members = emptyMembers()), - AssociationType.FACULTIES), - ) +fun mockAssociations(): List { + return listOf( + MockAssociation( + Association( + uid = "1", + acronym = "Musical", + fullName = "Musical Association", + description = + "AGEPoly Commission – stimulation of the practice of music on the campus", + members = emptyMembers()), + AssociationType.MUSIC), + MockAssociation( + Association( + uid = "2", + acronym = "Nuit De la Magistrale", + fullName = "Nuit De la Magistrale Association", + description = + "AGEPoly Commission – party following the formal Magistrale Graduation Ceremony", + members = emptyMembers()), + AssociationType.FESTIVALS), + MockAssociation( + Association( + uid = "3", + acronym = "Balélec", + fullName = "Festival Balélec", + description = "Open-air unique en Suisse, organisée par des bénévoles étudiants.", + members = emptyMembers()), + AssociationType.FESTIVALS), + MockAssociation( + Association( + uid = "4", + acronym = "Artiphys", + fullName = "Festival Artiphys", + description = "Festival à l'EPFL", + members = emptyMembers()), + AssociationType.FESTIVALS), + MockAssociation( + Association( + uid = "5", + acronym = "Sysmic", + fullName = "Festival Sysmic", + description = "Festival à l'EPFL", + members = emptyMembers()), + AssociationType.FESTIVALS), + MockAssociation( + Association( + uid = "6", + acronym = "IFL", + fullName = "Innovation Forum Lausanne", + description = "Innovation Forum Lausanne", + members = emptyMembers()), + AssociationType.INNOVATION), + MockAssociation( + Association( + uid = "7", + acronym = "Clic", + fullName = "Clic Association", + description = "Association of EPFL Students of IC Faculty", + members = emptyMembers()), + AssociationType.FACULTIES), + ) +} diff --git a/app/src/main/java/com/android/unio/ui/explore/Explore.kt b/app/src/main/java/com/android/unio/ui/explore/Explore.kt index b980de40c..f41927899 100644 --- a/app/src/main/java/com/android/unio/ui/explore/Explore.kt +++ b/app/src/main/java/com/android/unio/ui/explore/Explore.kt @@ -116,7 +116,7 @@ fun AssociationItem(association: Association) { fun getFilteredAssociationsByCategoryAndAlphabeticalOrder( category: AssociationType ): List { - return mockAssociations.filter { it.type == category }.sortedBy { it.association.acronym } + return mockAssociations().filter { it.type == category }.sortedBy { it.association.acronym } } /** Returns the name of the category with the first letter in uppercase. */ From 3dcab0912c87ed200f6b47c519a87cd37b555324 Mon Sep 17 00:00:00 2001 From: Oskar Codes Date: Thu, 10 Oct 2024 17:36:04 +0200 Subject: [PATCH 4/6] test(FirestoreReferenceList): Create tests for the FirestoreReferenceList class. --- .../com/android/unio/ui/ExploreScreenTest.kt | 2 +- .../firestore/FirestoreReferenceListTest.kt | 99 +++++++++++++------ 2 files changed, 68 insertions(+), 33 deletions(-) diff --git a/app/src/androidTest/java/com/android/unio/ui/ExploreScreenTest.kt b/app/src/androidTest/java/com/android/unio/ui/ExploreScreenTest.kt index 5513a528d..f8b21db78 100644 --- a/app/src/androidTest/java/com/android/unio/ui/ExploreScreenTest.kt +++ b/app/src/androidTest/java/com/android/unio/ui/ExploreScreenTest.kt @@ -8,7 +8,7 @@ import org.junit.Assert.assertEquals import org.junit.Rule import org.junit.Test -class ExploreScreen { +class ExploreScreenTest { @get:Rule val composeTestRule = createComposeRule() @Test diff --git a/app/src/test/java/com/android/unio/model/firestore/FirestoreReferenceListTest.kt b/app/src/test/java/com/android/unio/model/firestore/FirestoreReferenceListTest.kt index 0c4673bc9..37fcd8c85 100644 --- a/app/src/test/java/com/android/unio/model/firestore/FirestoreReferenceListTest.kt +++ b/app/src/test/java/com/android/unio/model/firestore/FirestoreReferenceListTest.kt @@ -3,74 +3,108 @@ package com.android.unio.model.firestore import com.google.android.gms.tasks.OnSuccessListener import com.google.android.gms.tasks.Task import com.google.firebase.firestore.CollectionReference -import com.google.firebase.firestore.DocumentReference import com.google.firebase.firestore.DocumentSnapshot -import com.google.firebase.firestore.FirebaseFirestore +import com.google.firebase.firestore.FieldPath +import com.google.firebase.firestore.Query +import com.google.firebase.firestore.QuerySnapshot +import junit.framework.TestCase.assertEquals import kotlinx.coroutines.flow.first import kotlinx.coroutines.test.runTest -import org.junit.Assert.assertEquals import org.junit.Before import org.junit.Test import org.mockito.Mock +import org.mockito.Mockito.`when` import org.mockito.MockitoAnnotations import org.mockito.kotlin.any -import org.mockito.kotlin.timeout +import org.mockito.kotlin.eq import org.mockito.kotlin.verify import org.mockito.kotlin.whenever class FirestoreReferenceListTest { - @Mock private lateinit var mockFirestore: FirebaseFirestore - @Mock private lateinit var mockCollectionRef: CollectionReference - @Mock private lateinit var mockDocumentRef: DocumentReference + @Mock private lateinit var mockCollection: CollectionReference @Mock private lateinit var mockSnapshot: DocumentSnapshot - @Mock private lateinit var mockTask: Task + @Mock private lateinit var mockQuerySnapshot: QuerySnapshot + @Mock private lateinit var mockTask: Task + @Mock private lateinit var mockQuery: Query @Mock private lateinit var firestoreReferenceList: FirestoreReferenceList @Before fun setup() { MockitoAnnotations.openMocks(this) - whenever(mockFirestore.collection(any())).thenReturn(mockCollectionRef) - whenever(mockCollectionRef.document(any())).thenReturn(mockDocumentRef) - whenever(mockDocumentRef.get()).thenReturn(mockTask) - whenever(mockTask.addOnSuccessListener(any())).thenAnswer { invocation -> - val thread = Thread { - Thread.sleep(100) - val callback = invocation.arguments[0] as OnSuccessListener - callback.onSuccess(mockSnapshot) - } - thread.start() + `when`(mockCollection.whereIn(eq(FieldPath.documentId()), any())).thenReturn(mockQuery) + `when`(mockQuery.get()).thenReturn(mockTask) + + firestoreReferenceList = + FirestoreReferenceList(mockCollection) { snapshot -> snapshot.getString("data") ?: "" } + } + + @Test + fun `test add does not request immediately`() { + `when`(mockTask.addOnSuccessListener(any())).thenAnswer { invocation -> + val callback = invocation.arguments[0] as OnSuccessListener + callback.onSuccess(mockQuerySnapshot) mockTask } - firestoreReferenceList = - FirestoreReferenceList(mockFirestore, "testPath") { snapshot -> - snapshot.getString("data") ?: "" - } + firestoreReferenceList.add("uid1") + firestoreReferenceList.add("uid2") + + // Internal list of UIDs should now contain "uid1" and "uid2" + assertEquals(0, firestoreReferenceList.list.value.size) // initial list should still be empty + } + + @Test + fun `test addAll does not request immediately`() { + `when`(mockTask.addOnSuccessListener(any())).thenAnswer { invocation -> + val callback = invocation.arguments[0] as OnSuccessListener + callback.onSuccess(mockQuerySnapshot) + mockTask + } + + val uids = listOf("uid1", "uid2", "uid3") + firestoreReferenceList.addAll(uids) + + assertEquals(0, firestoreReferenceList.list.value.size) // initial list should still be empty } @Test fun `test requestAll fetches documents and updates list`() = runTest { + // Prepare mocks + `when`(mockTask.addOnSuccessListener(any())).thenAnswer { invocation -> + val callback = invocation.arguments[0] as OnSuccessListener + callback.onSuccess(mockQuerySnapshot) + mockTask + } + + whenever(mockQuerySnapshot.documents).thenReturn(listOf(mockSnapshot, mockSnapshot)) whenever(mockSnapshot.getString("data")).thenReturn("Item1", "Item2") // Add UIDs and call requestAll firestoreReferenceList.addAll(listOf("uid1", "uid2")) firestoreReferenceList.requestAll() - // Verify firestore calls after 200ms - verify(mockFirestore, timeout(200).times(2)).collection("testPath") - verify(mockCollectionRef, timeout(200).times(2)).document(any()) - verify(mockDocumentRef, timeout(200).times(2)).get() + // Assert that the list was updated correctly + assertEquals(listOf("Item1", "Item2"), firestoreReferenceList.list.first()) + verify(mockQuery).get() } @Test fun `test requestAll clears list before updating`() = runTest { - // Set initial state + `when`(mockTask.addOnSuccessListener(any())).thenAnswer { invocation -> + val callback = invocation.arguments[0] as OnSuccessListener + callback.onSuccess(mockQuerySnapshot) + mockTask + } + + // Add initial UIDs firestoreReferenceList.addAll(listOf("uid1", "uid2")) + + // Request documents firestoreReferenceList.requestAll() - // Verify the list is cleared + // Assert that the list was cleared before updating assertEquals(0, firestoreReferenceList.list.value.size) } @@ -78,20 +112,21 @@ class FirestoreReferenceListTest { fun `test fromList creates FirestoreReferenceList with UIDs`() = runTest { val list = listOf("uid1", "uid2") val fromList = - FirestoreReferenceList.fromList(list, mockFirestore, "testPath") { snapshot -> + FirestoreReferenceList.fromList(list, mockCollection) { snapshot -> snapshot.getString("data") ?: "" } - assertEquals(emptyList(), fromList.list.first()) + assertEquals(0, fromList.list.value.size) } @Test fun `test empty creates FirestoreReferenceList without UIDs`() = runTest { val emptyList = - FirestoreReferenceList.empty(mockFirestore, "testPath") { snapshot -> + FirestoreReferenceList.empty(mockCollection) { snapshot -> snapshot.getString("data") ?: "" } - assertEquals(emptyList(), emptyList.list.first()) + // Initial list should be empty + assertEquals(0, emptyList.list.value.size) } } From 2eddd766245c6f285468e065701f0b8b0693b22c Mon Sep 17 00:00:00 2001 From: Oskar Codes Date: Thu, 10 Oct 2024 17:36:59 +0200 Subject: [PATCH 5/6] choreEventRepositoryFirestore): Adapt the EventRepositoryFirestore to use global path variables and expose its hydration function --- .../model/event/EventRepositoryFirestore.kt | 18 ++++++++---------- .../unio/model/firestore/FirestorePaths.kt | 1 + 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/app/src/main/java/com/android/unio/model/event/EventRepositoryFirestore.kt b/app/src/main/java/com/android/unio/model/event/EventRepositoryFirestore.kt index 291a85e86..7011fee0e 100644 --- a/app/src/main/java/com/android/unio/model/event/EventRepositoryFirestore.kt +++ b/app/src/main/java/com/android/unio/model/event/EventRepositoryFirestore.kt @@ -1,6 +1,7 @@ package com.android.unio.model.event import android.util.Log +import com.android.unio.model.firestore.FirestorePaths.EVENT_PATH import com.google.firebase.Timestamp import com.google.firebase.firestore.DocumentSnapshot import com.google.firebase.firestore.FirebaseFirestore @@ -77,16 +78,13 @@ class EventRepositoryFirestore(private val db: FirebaseFirestore) : EventReposit } } - private fun hydrate(doc: DocumentSnapshot): Event? { - - val event = doc.toObject(Event::class.java) - if (event == null) { - Log.e("EventRepositoryFirestore", "Error while converting db document to Event object") - } - return event - } - companion object { - private const val EVENT_PATH = "events" + fun hydrate(doc: DocumentSnapshot): Event? { + val event = doc.toObject(Event::class.java) + if (event == null) { + Log.e("EventRepositoryFirestore", "Error while converting db document to Event object") + } + return event + } } } diff --git a/app/src/main/java/com/android/unio/model/firestore/FirestorePaths.kt b/app/src/main/java/com/android/unio/model/firestore/FirestorePaths.kt index 549fae7e7..f733804fe 100644 --- a/app/src/main/java/com/android/unio/model/firestore/FirestorePaths.kt +++ b/app/src/main/java/com/android/unio/model/firestore/FirestorePaths.kt @@ -3,4 +3,5 @@ package com.android.unio.model.firestore object FirestorePaths { const val ASSOCIATION_PATH = "associations" const val USER_PATH = "users" + const val EVENT_PATH = "events" } From 919166ebb4ccd5648b0a31dea037713b384c83eb Mon Sep 17 00:00:00 2001 From: Oskar Codes Date: Thu, 10 Oct 2024 17:59:40 +0200 Subject: [PATCH 6/6] chore(imports): Remove unused imports. --- .../unio/model/association/AssociationRepositoryFirestore.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/main/java/com/android/unio/model/association/AssociationRepositoryFirestore.kt b/app/src/main/java/com/android/unio/model/association/AssociationRepositoryFirestore.kt index 57490fb61..ccf788175 100644 --- a/app/src/main/java/com/android/unio/model/association/AssociationRepositoryFirestore.kt +++ b/app/src/main/java/com/android/unio/model/association/AssociationRepositoryFirestore.kt @@ -4,7 +4,6 @@ import com.android.unio.model.firestore.FirestorePaths.ASSOCIATION_PATH import com.android.unio.model.firestore.FirestorePaths.USER_PATH import com.android.unio.model.firestore.FirestoreReferenceList import com.android.unio.model.user.UserRepositoryFirestore -import com.google.android.play.core.assetpacks.db import com.google.firebase.Firebase import com.google.firebase.auth.auth import com.google.firebase.firestore.DocumentSnapshot