Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI Paging for Chirps History Pre-Consensus Protocol Integration #1796

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -21,6 +21,7 @@ import io.reactivex.subjects.BehaviorSubject
import io.reactivex.subjects.Subject
import java.util.EnumMap
import java.util.concurrent.ConcurrentHashMap
import java.util.concurrent.atomic.AtomicInteger
import java.util.function.Consumer
import javax.inject.Inject
import javax.inject.Singleton
@@ -102,6 +103,14 @@ constructor(appDatabase: AppDatabase, application: Application) {
return getLaoChirps(laoId).reactionByChirpId[chirpId] ?: emptySet()
}

fun queryMoreChirps(laoId: String) {
getLaoChirps(laoId).queryMoreChirps()
}

fun canQueryMoreChirps(laoId: String): Boolean {
return !getLaoChirps(laoId).isEndOfChirps()
}

/**
* @param laoId of the lao we want to observe the chirp list
* @return an observable set of message ids whose correspond to the set of chirp published on the
@@ -132,7 +141,6 @@ constructor(appDatabase: AppDatabase, application: Application) {
{ err: Throwable ->
Timber.tag(TAG).e(err, "Error in persisting reaction %s", reaction.id)
}))

// Retrieve Lao data and add the reaction to it
return getLaoChirps(laoId).addReaction(reaction)
}
@@ -171,6 +179,11 @@ constructor(appDatabase: AppDatabase, application: Application) {
private val chirps = ConcurrentHashMap<MessageID, Chirp>()
private val chirpSubjects = ConcurrentHashMap<MessageID, Subject<Chirp>>()
private val chirpsSubject: Subject<Set<MessageID>> = BehaviorSubject.createDefault(emptySet())
// TODO : used to fake page loading, to be removed when we will be able to request only wanted
// chirps. Maxime Teuber @Kaz-ookid - April 2024
private var chirpsLoaded: AtomicInteger = AtomicInteger(0)
private var storageIsLoaded: Boolean = false
private var reachedEndOfChirps: Boolean = false
Comment on lines +185 to +186
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

storageIsLoaded seems a shared variable to me, at least theoretically. As they're accessed in the add method and when data is loaded from disk, which can in theory happen in parallel (usually it never happens as loading from storage is quite fast but I'd still use atomic variables). While reachedEndOfChirps shouldn't be shared if we always test if we can load more before actually loading the chirps.


// Reactions
val reactionByChirpId = ConcurrentHashMap<MessageID, MutableSet<Reaction>>()
@@ -179,6 +192,7 @@ constructor(appDatabase: AppDatabase, application: Application) {

init {
loadStorage()
chirpsLoaded.set(CHIRPS_PAGE_SIZE)
}

fun add(chirp: Chirp) {
@@ -188,6 +202,11 @@ constructor(appDatabase: AppDatabase, application: Application) {
Timber.tag(TAG).w("A chirp with id %s already exist : %s", id, old)
return
}
if (storageIsLoaded) {
// TODO : used to fake page loading, to be removed when we will be able to request only
// wanted chirps. Maxime Teuber @Kaz-ookid - April 2024
chirpsLoaded.incrementAndGet()
}

// Update repository data
chirps[id] = chirp
@@ -199,6 +218,23 @@ constructor(appDatabase: AppDatabase, application: Application) {
chirpsSubject.toSerialized().onNext(HashSet(chirps.keys))
}

// TODO : used to fake page loading, to be removed when we will be able to request only wanted
// chirps. Maxime Teuber @Kaz-ookid - April 2024
// Either here or directly in DB, we should query to servers the CHIRPS_PAGE_SIZE next chirps,
// and not all of them.
fun queryMoreChirps() {
val previousSize = getChirpsSubject().blockingFirst().size
chirpsLoaded.addAndGet(CHIRPS_PAGE_SIZE)
chirpsSubject.onNext(
chirps.keys.sortedByDescending { chirps[it]?.timestamp }.take(chirpsLoaded.get()).toSet())
val newSize = getChirpsSubject().blockingFirst().size
// if did not load CHIRPS_PAGE_SIZE chirps, then we reached the end of the chirps.
if (newSize - previousSize != CHIRPS_PAGE_SIZE) {
chirpsLoaded.set(newSize)
reachedEndOfChirps = true
}
}

fun addReaction(reaction: Reaction): Boolean {
// Check if the associated chirp is present
val chirp = chirps[reaction.chirpId] ?: return false
@@ -279,7 +315,17 @@ constructor(appDatabase: AppDatabase, application: Application) {
}

fun getChirpsSubject(): Observable<Set<MessageID>> {
return chirpsSubject
// TODO : used to fake page loading, to be removed when we will be able to request only wanted
// chirps. Maxime Teuber @Kaz-ookid - April 2024
// would normally return chirpsSubject directly
return chirpsSubject.map {
chirps.keys.sortedByDescending { chirps[it]?.timestamp }.take(chirpsLoaded.get()).toSet()
}
}

/** Check if the end of the chirps history has been reached */
fun isEndOfChirps(): Boolean {
return reachedEndOfChirps
}

@Throws(UnknownChirpException::class)
@@ -342,6 +388,7 @@ constructor(appDatabase: AppDatabase, application: Application) {
chirp.id)
}))
})
storageIsLoaded = true
},
{ err: Throwable ->
Timber.tag(TAG).e(err, "No chirp found in the storage for lao %s", laoId)
@@ -350,6 +397,7 @@ constructor(appDatabase: AppDatabase, application: Application) {
}

companion object {
private const val CHIRPS_PAGE_SIZE: Int = 10
private val TAG = SocialMediaRepository::class.java.simpleName
}
}
Original file line number Diff line number Diff line change
@@ -4,6 +4,8 @@ import android.os.Bundle
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import android.widget.Button
import androidx.core.content.ContextCompat
import androidx.fragment.app.Fragment
import androidx.fragment.app.FragmentManager
import com.github.dedis.popstellar.R
@@ -39,6 +41,7 @@ class ChirpListFragment : Fragment() {
super.onViewCreated(view, savedInstanceState)
setupSendButton()
setupListViewAdapter()
addLoadMoreButtonToListView()
}

override fun onResume() {
@@ -62,6 +65,31 @@ class ChirpListFragment : Fragment() {
listView.adapter = mChirpListAdapter
}

private fun addLoadMoreButtonToListView() {
val footerView =
layoutInflater.inflate(R.layout.chirp_load_more_button, binding.chirpsList, false)
val buttonLoadMoreChirps = footerView.findViewById<Button>(R.id.button_load_more_chirps)
buttonLoadMoreChirps.setOnClickListener {
socialMediaViewModel.loadMoreChirps()
if (!socialMediaViewModel.canLoadMoreChirps) {
disableLoadMoreButton()
}
}
binding.chirpsList.addFooterView(footerView, null, false)
if (!socialMediaViewModel.canLoadMoreChirps) {
disableLoadMoreButton()
}
}

private fun disableLoadMoreButton() {
val loadMoreButton = view?.findViewById<Button>(R.id.button_load_more_chirps)
loadMoreButton?.let { button ->
button.text = getString(R.string.load_more_chirps_disabled)
button.setBackgroundColor(ContextCompat.getColor(requireContext(), R.color.gray))
button.isEnabled = false
}
}

companion object {
val TAG: String = SocialMediaSendFragment::class.java.simpleName

Original file line number Diff line number Diff line change
@@ -60,6 +60,9 @@ constructor(
private val mNumberCharsLeft = MutableLiveData<Int>()
val bottomNavigationTab = MutableLiveData(SocialMediaTab.HOME)

val canLoadMoreChirps: Boolean
get() = socialMediaRepository.canQueryMoreChirps(laoId)

private val disposables: CompositeDisposable = CompositeDisposable()

override fun onCleared() {
@@ -250,6 +253,10 @@ constructor(
// the view are done on the thread. Otherwise, the app might crash
.observeOn(schedulerProvider.mainThread())

fun loadMoreChirps() {
socialMediaRepository.queryMoreChirps(laoId)
}

@Throws(UnknownChirpException::class)
fun getReactions(chirpId: MessageID): Observable<Set<Reaction>> {
return socialMediaRepository
4 changes: 2 additions & 2 deletions fe2-android/app/src/main/res/layout/chirp_list_fragment.xml
Original file line number Diff line number Diff line change
@@ -15,8 +15,8 @@

<ListView
android:id="@+id/chirps_list"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:divider="@color/categoryTab"
android:dividerHeight="@dimen/social_media_divider_height"
tools:listitem="@layout/chirp_card" />
19 changes: 19 additions & 0 deletions fe2-android/app/src/main/res/layout/chirp_load_more_button.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?xml version="1.0" encoding="utf-8"?>
<androidx.constraintlayout.widget.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/tools"
xmlns:app="http://schemas.android.com/apk/res-auto"
android:layout_width="match_parent"
android:layout_height="match_parent">

<Button
android:id="@+id/button_load_more_chirps"
android:layout_width="@dimen/large_button_width"
android:layout_height="@dimen/small_button_height"
android:text="@string/load_more_chirps"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintTop_toTopOf="parent"
app:layout_constraintBottom_toBottomOf="parent"
tools:ignore="MissingConstraints,SpUsage" />

</androidx.constraintlayout.widget.ConstraintLayout>
1 change: 1 addition & 0 deletions fe2-android/app/src/main/res/values/dimens.xml
Original file line number Diff line number Diff line change
@@ -40,6 +40,7 @@
<dimen name="standard_button_height">55dp</dimen>
<dimen name="standard_button_width">105dp</dimen>
<dimen name="small_button_width">95dp</dimen>
<dimen name="small_button_height">45dp</dimen>
<dimen name="large_button_width">125dp</dimen>
<dimen name="standard_button_text_size">16dp</dimen>
<dimen name="big_button_text_size">22sp</dimen>
2 changes: 2 additions & 0 deletions fe2-android/app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
@@ -281,6 +281,8 @@
<string name="love_button">love button</string>
<string name="deleted_chirp">Deleted chirp!</string>
<string name="deleted_chirp_2">Chirp is deleted.</string>
<string name="load_more_chirps">Load more</string>
<string name="load_more_chirps_disabled">No more</string>

<!-- PoPCHA -->
<string name="popcha">PoPCHA</string>