diff --git a/README.md b/README.md index b903881..6230b3e 100644 --- a/README.md +++ b/README.md @@ -42,6 +42,7 @@ full email — see - **University email sign-up** — registration is restricted to `@ogr.akdeniz.edu.tr` addresses - **Department selection** from the full list of 144 Akdeniz University departments - **Contribution gate** — the department feed stays locked until you upload your first note +- **Paged feed** — notes load 20 at a time as you scroll; every note in the department is still reachable - **Note upload** as PDF or image, with automatic image downscaling and compression - **Note rating** on a 1–5 scale; you cannot rate your own notes, and re-rating replaces your previous vote instead of adding a second one — both enforced by the security @@ -100,6 +101,14 @@ flowchart TD by a `limit(1)` query for the user's own notes rather than a `hasUploadedNote` flag on the profile. A stored flag drifted: it was set on upload and never cleared on delete, so uploading once and deleting granted permanent access. +- **The feed pages by growing one listener, not by stitching queries.** Notes arrive + through a live Firestore listener, so paging with a second query would leave two + windows with different freshness and no clear owner of the order. Instead the same + listener is re-subscribed with a larger `limit` as the user nears the end. The cost is + explicit: widening the window re-reads the notes already seen, so 20 + 40 is 60 + document reads rather than 40. Reading the department's *entire* collection on every + open — the previous behaviour — is far more expensive, and most readers never leave + the first page. - **Points are derived too, and that is what made them safe.** A user's point total is the sum of `ratingSum` over their own notes, computed where it is displayed. It used to live in `users.points`, written by whoever cast the vote — which forced the rules to @@ -227,13 +236,11 @@ one call (`sendEmailVerification()` after sign-up, then gating the feed on project are test data, and a mandatory confirmation step would put a mailbox between a reviewer and the running app for no benefit at this scale. -**No pagination.** The feed and the leaderboard read every note in the department and -sort in memory. Scoping both queries to one department (rather than the whole -collection, as the leaderboard used to) bounds this by roughly the number of -departments, but it is still every matching note on every open. Paginating means -ordering in the query, and the data that ordering depends on has been normalised -already — see [Data migrations](#data-migrations) — so the remaining work is the query -and the load-more UI. +**The leaderboard is not paginated.** It reads every note in the department and sorts +in memory. The feed no longer does (see Key decisions), and the leaderboard can follow +the same shape — it needs its own composite index on `department` + `ratingSum`, and a +ranking that only shows part of itself needs more thought about what "rank 21" means +when the rest is not loaded. **Deleting a note reports failure with a `Toast`.** Unlike a failed read, the user can repeat a failed delete by tapping the button again, so the toast does not leave them @@ -278,7 +285,20 @@ emulator on API 24+. an un-deployed project is both blocked from reading and, once unblocked by a permissive rule, trivially cheatable. See [Security rules](#security-rules). -6. **Build and run** +6. **Deploy the Firestore index** + + ```bash + firebase deploy --only firestore:indexes + ``` + + The feed filters on `department` and orders by `createdAt`, which Firestore serves + only from a composite index. Without it the query fails outright with + `FAILED_PRECONDITION` and the feed never opens — the error message includes a link + that creates the index for you, but deploying + [`firestore.indexes.json`](firestore.indexes.json) keeps the definition in the repo + where it can be reviewed. Building it takes a few minutes on a live project. + +7. **Build and run** ```bash ./gradlew installDebug @@ -301,7 +321,7 @@ emulator on API 24+. ./gradlew testDebugUnitTest ``` -107 JVM tests, no emulator and no network. They cover the layers where a bug would be +114 JVM tests, no emulator and no network. They cover the layers where a bug would be invisible rather than loud: the scoring arithmetic, the decision logic in every ViewModel, the list diffing that decides which rows get redrawn, and — for the feature the app is built around — what the screen actually shows. @@ -309,7 +329,7 @@ the app is built around — what the screen actually shows. | Suite | What it pins down | |---|---| | `RatingCalculatorTest` | First-time votes, changed votes, unchanged votes, average recalculation, and two inconsistent-data guards — the total flooring at zero, and the vote count flooring at one so the average never divides by zero. | -| `FeedViewModelTest` | The contribution gate: locked without an upload, unlocked with one, and locked for a missing profile or a blank department. Also asserts the department query is never even built while locked. | +| `FeedViewModelTest` | The contribution gate: locked without an upload, unlocked with one, and locked for a missing profile or a blank department — and that the department query is never even built while locked. Also the paging: the window each page asks for, when there is more to fetch, that the last page stops asking, and that the list stays on screen while the next page loads. | | `UploadViewModelTest` | The uid, email and department written onto a note, including the `UNKNOWN_DEPARTMENT` fallback that keeps a blank department out of `whereEqualTo` queries. | | `NoteDetailViewModelTest` | Metadata and file content loading as separate states, no file read for a note without one, and the rating rules (own note, missing session, deleted note). | | `EditNoteViewModelTest` | Ownership and session handling on load and save. | diff --git a/app/src/main/java/duygu/yilmaz/campusnote/data/repository/FirebaseNoteRepository.kt b/app/src/main/java/duygu/yilmaz/campusnote/data/repository/FirebaseNoteRepository.kt index 080c607..d34a945 100644 --- a/app/src/main/java/duygu/yilmaz/campusnote/data/repository/FirebaseNoteRepository.kt +++ b/app/src/main/java/duygu/yilmaz/campusnote/data/repository/FirebaseNoteRepository.kt @@ -75,10 +75,20 @@ class FirebaseNoteRepository( return legacy?.takeIf { it.isNotEmpty() } } - override fun observeNotesByDepartment(department: String): Flow> = + /** + * Sıralama artık istemcide değil sorguda: `limit` ancak bir sıra tanımlıysa + * anlamlı, aksi halde Firestore rastgele bir alt küme döndürür. + * + * Bu sorgu `department` + `createdAt` bileşik index'ini gerektiriyor + * (`firestore.indexes.json`). Index yayına alınmadan sorgu FAILED_PRECONDITION + * ile düşer — feed hiç açılmaz. + */ + override fun observeNotesByDepartment(department: String, limit: Long): Flow> = observePosts( query = firestore.collection(NOTES_COLLECTION) .whereEqualTo(DEPARTMENT_FIELD, department) + .orderBy(CREATED_AT_FIELD, Query.Direction.DESCENDING) + .limit(limit) ) override fun observeNotesByUploader( @@ -255,5 +265,6 @@ class FirebaseNoteRepository( const val FILE_DATA_FIELD = "fileData" const val DEPARTMENT_FIELD = "department" const val UPLOADER_UID_FIELD = "uploaderUid" + const val CREATED_AT_FIELD = "createdAt" } } diff --git a/app/src/main/java/duygu/yilmaz/campusnote/data/repository/NoteRepository.kt b/app/src/main/java/duygu/yilmaz/campusnote/data/repository/NoteRepository.kt index b503ead..1d7b791 100644 --- a/app/src/main/java/duygu/yilmaz/campusnote/data/repository/NoteRepository.kt +++ b/app/src/main/java/duygu/yilmaz/campusnote/data/repository/NoteRepository.kt @@ -18,7 +18,13 @@ interface NoteRepository { /** Notun dosya içeriğini base64 olarak döndürür; dosya yoksa null. */ suspend fun getNoteFile(noteId: String): String? - fun observeNotesByDepartment(department: String): Flow> + /** + * @param limit en fazla kaç not dinlensin. Sayfalama tek bir dinleyicinin + * limitini büyüterek yapılıyor: ikinci bir sorgu açıp sonuçları birleştirmek + * yerine aynı dinleyici daha geniş bir pencereyle yeniden kuruluyor, böylece + * feed canlı kalıyor ve sıra tek bir yerden geliyor. + */ + fun observeNotesByDepartment(department: String, limit: Long): Flow> fun observeNotesByUploader( uploaderUid: String, diff --git a/app/src/main/java/duygu/yilmaz/campusnote/ui/feed/FeedFragment.kt b/app/src/main/java/duygu/yilmaz/campusnote/ui/feed/FeedFragment.kt index fce4ebd..394f9cd 100644 --- a/app/src/main/java/duygu/yilmaz/campusnote/ui/feed/FeedFragment.kt +++ b/app/src/main/java/duygu/yilmaz/campusnote/ui/feed/FeedFragment.kt @@ -9,6 +9,7 @@ import androidx.annotation.VisibleForTesting import androidx.fragment.app.Fragment import androidx.lifecycle.ViewModelProvider import androidx.recyclerview.widget.LinearLayoutManager +import androidx.recyclerview.widget.RecyclerView import com.google.android.material.snackbar.Snackbar import duygu.yilmaz.campusnote.R import duygu.yilmaz.campusnote.databinding.FragmentFeedBinding @@ -60,8 +61,10 @@ class FeedFragment : Fragment() { .commit() } ) - binding.rvFeed.layoutManager = LinearLayoutManager(requireContext()) + val layoutManager = LinearLayoutManager(requireContext()) + binding.rvFeed.layoutManager = layoutManager binding.rvFeed.adapter = adapter + binding.rvFeed.addOnScrollListener(loadMoreListener(layoutManager)) viewModel.uiState.observe(viewLifecycleOwner, ::renderState) @@ -153,6 +156,26 @@ class FeedFragment : Fragment() { } } + /** + * Liste sonuna yaklaşıldığında bir sonraki sayfayı ister. + * + * Tam sona değil, [LOAD_MORE_THRESHOLD] satır kala tetikleniyor: istek ile + * sonucun gelmesi arasında kullanıcı kaydırmaya devam ediyor, sona basıldığında + * istenirse arada boş bir duraklama görünür. Tekrarlanan çağrılar zararsız — + * [FeedViewModel.loadMore] son sayfadayken hiçbir şey yapmıyor. + */ + private fun loadMoreListener(layoutManager: LinearLayoutManager) = + object : RecyclerView.OnScrollListener() { + override fun onScrolled(recyclerView: RecyclerView, dx: Int, dy: Int) { + if (dy <= 0) return + + val lastVisible = layoutManager.findLastVisibleItemPosition() + if (lastVisible >= layoutManager.itemCount - LOAD_MORE_THRESHOLD) { + viewModel.loadMore() + } + } + } + /** * Hata mesajını yeniden deneme eylemiyle birlikte gösterir. * @@ -172,6 +195,11 @@ class FeedFragment : Fragment() { .show() } + private companion object { + /** Sona kaç satır kala sonraki sayfanın isteneceği. */ + const val LOAD_MORE_THRESHOLD = 5 + } + /** * @param unlocked kullanıcı not paylaştıysa feed, paylaşmadıysa kilit ekranı görünür. * diff --git a/app/src/main/java/duygu/yilmaz/campusnote/ui/feed/FeedUiState.kt b/app/src/main/java/duygu/yilmaz/campusnote/ui/feed/FeedUiState.kt index 2c68830..83507dc 100644 --- a/app/src/main/java/duygu/yilmaz/campusnote/ui/feed/FeedUiState.kt +++ b/app/src/main/java/duygu/yilmaz/campusnote/ui/feed/FeedUiState.kt @@ -7,7 +7,17 @@ sealed interface FeedUiState { data object Loading : FeedUiState data object Locked : FeedUiState data object MissingSession : FeedUiState - data class Content(val posts: List) : FeedUiState + /** + * @param canLoadMore sorgu istenen kadar not döndürdüyse arkada daha olabilir. + * Toplam sayı bilinmiyor — öğrenmek fazladan bir okuma demek olurdu — o yüzden + * "dolu geldi" kabaca "devamı var" sayılıyor. Yanılma payı son sayfada bir kez + * fazladan istek atmak; kullanıcıya boş bir sayfa göstermez, çünkü aynı sonuç + * yeniden gelir ve bayrak kapanır. + */ + data class Content( + val posts: List, + val canLoadMore: Boolean + ) : FeedUiState data class Error( val exception: Exception, val stage: FeedFailureStage diff --git a/app/src/main/java/duygu/yilmaz/campusnote/ui/feed/FeedViewModel.kt b/app/src/main/java/duygu/yilmaz/campusnote/ui/feed/FeedViewModel.kt index c5b8d79..e2f8e23 100644 --- a/app/src/main/java/duygu/yilmaz/campusnote/ui/feed/FeedViewModel.kt +++ b/app/src/main/java/duygu/yilmaz/campusnote/ui/feed/FeedViewModel.kt @@ -25,8 +25,40 @@ class FeedViewModel( private var feedJob: Job? = null + /** + * Şu an dinlenen pencerenin büyüklüğü. Sayfalama ikinci bir sorgu açıp sonuçları + * birleştirerek değil, aynı dinleyiciyi daha geniş bir limitle yeniden kurarak + * yapılıyor: feed canlı bir Firestore dinleyicisi, ikiye bölünürse hangi yarının + * güncel olduğu belirsizleşirdi. + * + * Bedeli açık: pencere büyüdüğünde baştaki notlar yeniden okunuyor, yani 20 + 40 + * = 60 doküman okuması olur, 40 değil. Bugünkü davranış — her açılışta bölümün + * *bütün* notları — buna göre çok daha pahalı, ve kullanıcıların çoğu ilk sayfanın + * ötesine hiç gitmiyor. + */ + private var pageSize = PAGE_SIZE + fun startFeed() { feedJob?.cancel() + pageSize = PAGE_SIZE + observeFeed() + } + + /** + * Bir sayfa daha ister. Yükleme sürerken ya da elde son sayfa varken çağrılması + * etkisiz: liste sonuna her gelindiğinde tetiklendiği için korumasız bırakmak + * aynı pencereyi üst üste yeniden kurardı. + */ + fun loadMore() { + val state = _uiState.value + if (state !is FeedUiState.Content || !state.canLoadMore) return + + pageSize += PAGE_SIZE + observeFeed() + } + + private fun observeFeed() { + feedJob?.cancel() val user = authRepository.currentUser() if (user == null) { @@ -34,7 +66,11 @@ class FeedViewModel( return } - _uiState.value = FeedUiState.Loading + // Sonraki sayfalarda Loading'e düşmek listeyi ekrandan silerdi; yalnızca + // elde gösterilecek bir şey yokken yükleme durumuna geçiliyor. + if (_uiState.value !is FeedUiState.Content) { + _uiState.value = FeedUiState.Loading + } feedJob = viewModelScope.launch { // Kapı, kullanıcının gerçekten notu olup olmadığına bakılarak belirlenir; // profildeki bir bayrağa değil. Not silindiğinde erişim de geri alınır. @@ -60,7 +96,8 @@ class FeedViewModel( return@launch } - noteRepository.observeNotesByDepartment(department) + val requested = pageSize + noteRepository.observeNotesByDepartment(department, requested) .catch { throwable -> _uiState.value = FeedUiState.Error( exception = throwable as? Exception ?: Exception(throwable), @@ -68,11 +105,19 @@ class FeedViewModel( ) } .collect { posts -> - _uiState.value = FeedUiState.Content(posts) + _uiState.value = FeedUiState.Content( + posts = posts, + canLoadMore = posts.size.toLong() >= requested + ) } } } + private companion object { + /** Bir ekrana sığandan biraz fazlası; kaydırma sonuna gelmeden yenisi gelir. */ + const val PAGE_SIZE = 20L + } + fun stopFeed() { feedJob?.cancel() feedJob = null diff --git a/app/src/test/java/duygu/yilmaz/campusnote/testing/FakeRepositories.kt b/app/src/test/java/duygu/yilmaz/campusnote/testing/FakeRepositories.kt index becb47c..9d26262 100644 --- a/app/src/test/java/duygu/yilmaz/campusnote/testing/FakeRepositories.kt +++ b/app/src/test/java/duygu/yilmaz/campusnote/testing/FakeRepositories.kt @@ -136,8 +136,13 @@ class FakeNoteRepository : NoteRepository { return noteFile } - override fun observeNotesByDepartment(department: String): Flow> { + /** Son istenen pencere; sayfalama testleri limitin büyüdüğünü buradan görüyor. */ + var requestedLimits = mutableListOf() + private set + + override fun observeNotesByDepartment(department: String, limit: Long): Flow> { observedDepartment = department + requestedLimits += limit return departmentNotes } diff --git a/app/src/test/java/duygu/yilmaz/campusnote/ui/feed/FeedViewModelTest.kt b/app/src/test/java/duygu/yilmaz/campusnote/ui/feed/FeedViewModelTest.kt index cf2b921..6532177 100644 --- a/app/src/test/java/duygu/yilmaz/campusnote/ui/feed/FeedViewModelTest.kt +++ b/app/src/test/java/duygu/yilmaz/campusnote/ui/feed/FeedViewModelTest.kt @@ -1,6 +1,7 @@ package duygu.yilmaz.campusnote.ui.feed import androidx.arch.core.executor.testing.InstantTaskExecutorRule +import duygu.yilmaz.campusnote.data.model.Post import duygu.yilmaz.campusnote.testing.FakeAuthRepository import duygu.yilmaz.campusnote.testing.FakeNoteRepository import duygu.yilmaz.campusnote.testing.FakeUserRepository @@ -9,11 +10,13 @@ import duygu.yilmaz.campusnote.testing.authenticatedUser import duygu.yilmaz.campusnote.testing.post import duygu.yilmaz.campusnote.testing.userProfile import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Rule @@ -73,7 +76,10 @@ class FeedViewModelTest { viewModel.startFeed() advanceUntilIdle() - assertEquals(FeedUiState.Content(posts), viewModel.uiState.value) + assertEquals( + FeedUiState.Content(posts, canLoadMore = false), + viewModel.uiState.value + ) assertEquals("Bilgisayar Mühendisliği", noteRepository.observedDepartment) assertEquals(listOf("uid-1"), userRepository.requestedUserIds) } @@ -188,4 +194,111 @@ class FeedViewModelTest { // Kullanıcı ekrandan ayrıldıysa geç gelen snapshot yazılmamalı. assertEquals(FeedUiState.Loading, viewModel.uiState.value) } + + @Test + fun `ilk sayfa bir sayfa boyu kadar not ister`() = + runTest(mainDispatcherRule.testDispatcher) { + unlockedFeed(flowOf(listOf(post()))) + + val viewModel = viewModel() + viewModel.startFeed() + advanceUntilIdle() + + assertEquals(listOf(20L), noteRepository.requestedLimits) + } + + @Test + fun `sayfa dolu geldiyse devami olabilir`() = runTest(mainDispatcherRule.testDispatcher) { + // Toplam sayı bilinmiyor; "istenen kadar geldi" tek ipucu. + unlockedFeed(flowOf(List(20) { post(id = "note-$it") })) + + val viewModel = viewModel() + viewModel.startFeed() + advanceUntilIdle() + + val state = viewModel.uiState.value as FeedUiState.Content + assertTrue(state.canLoadMore) + } + + @Test + fun `sayfa eksik geldiyse son sayfadir`() = runTest(mainDispatcherRule.testDispatcher) { + unlockedFeed(flowOf(List(7) { post(id = "note-$it") })) + + val viewModel = viewModel() + viewModel.startFeed() + advanceUntilIdle() + + val state = viewModel.uiState.value as FeedUiState.Content + assertFalse(state.canLoadMore) + } + + @Test + fun `daha fazlasi istendiginde pencere buyur`() = + runTest(mainDispatcherRule.testDispatcher) { + unlockedFeed(flowOf(List(20) { post(id = "note-$it") })) + + val viewModel = viewModel() + viewModel.startFeed() + advanceUntilIdle() + viewModel.loadMore() + advanceUntilIdle() + + assertEquals(listOf(20L, 40L), noteRepository.requestedLimits) + } + + @Test + fun `son sayfadayken daha fazlasi istenmez`() = + runTest(mainDispatcherRule.testDispatcher) { + // Liste sonuna her gelişte tetiklendiği için bu koruma olmasa aynı + // pencere sonsuza kadar yeniden kurulurdu. + unlockedFeed(flowOf(List(3) { post(id = "note-$it") })) + + val viewModel = viewModel() + viewModel.startFeed() + advanceUntilIdle() + viewModel.loadMore() + viewModel.loadMore() + advanceUntilIdle() + + assertEquals(listOf(20L), noteRepository.requestedLimits) + } + + @Test + fun `feed yeniden baslatildiginda ilk sayfaya doner`() = + runTest(mainDispatcherRule.testDispatcher) { + unlockedFeed(flowOf(List(20) { post(id = "note-$it") })) + + val viewModel = viewModel() + viewModel.startFeed() + advanceUntilIdle() + viewModel.loadMore() + advanceUntilIdle() + viewModel.startFeed() + advanceUntilIdle() + + assertEquals(listOf(20L, 40L, 20L), noteRepository.requestedLimits) + } + + @Test + fun `sayfa yuklenirken liste ekranda kalir`() = + runTest(mainDispatcherRule.testDispatcher) { + // Loading'e düşmek listeyi silerdi; kullanıcı kaydırırken ekran boşalırdı. + val posts = List(20) { post(id = "note-$it") } + unlockedFeed(flowOf(posts)) + + val viewModel = viewModel() + viewModel.startFeed() + advanceUntilIdle() + viewModel.loadMore() + + assertTrue(viewModel.uiState.value is FeedUiState.Content) + } + + /** Kapıyı açıp verilen akışı feed'e bağlar. */ + private fun unlockedFeed(notes: Flow>) { + authRepository.setUser(authenticatedUser(uid = "uid-1")) + userRepository.profile = userProfile(department = "Bilgisayar Mühendisliği") + noteRepository.hasUploadedNote = true + noteRepository.departmentNotes = notes + } } diff --git a/firebase.json b/firebase.json index 53fe283..770e27e 100644 --- a/firebase.json +++ b/firebase.json @@ -1,6 +1,7 @@ { "firestore": { - "rules": "firestore.rules" + "rules": "firestore.rules", + "indexes": "firestore.indexes.json" }, "emulators": { "firestore": { diff --git a/firestore.indexes.json b/firestore.indexes.json new file mode 100644 index 0000000..9187ee8 --- /dev/null +++ b/firestore.indexes.json @@ -0,0 +1,13 @@ +{ + "indexes": [ + { + "collectionGroup": "notes", + "queryScope": "COLLECTION", + "fields": [ + { "fieldPath": "department", "order": "ASCENDING" }, + { "fieldPath": "createdAt", "order": "DESCENDING" } + ] + } + ], + "fieldOverrides": [] +}