refactor [gitlab]: always load all MR discussions and events and don't await the full load on refresh

We don't need to wait for the full discussions refresh on every MR operation, as it takes considerable time

GitOrigin-RevId: 8d724e8dc462c38ad3c56828b739023af3ee12cd
This commit is contained in:
Ivan Semenov
2026-02-13 14:11:18 +00:00
committed by intellij-monorepo-bot
parent 212f40edc5
commit c4cf4f8449
2 changed files with 63 additions and 81 deletions
@@ -170,7 +170,7 @@ internal class LoadedGitLabMergeRequest(
.mapScoped { details -> GitLabMergeRequestChangesImpl(this, api, glMetadata, projectMapping, details) }
.modelFlow(cs, LOG)
private val stateEventsHolder =
override val stateEvents by lazy {
startGitLabRestETagListLoaderIn(
cs,
getMergeRequestStateEventsUri(glProject, iid),
@@ -178,17 +178,15 @@ internal class LoadedGitLabMergeRequest(
requestReloadFlow = mergeRequestReloadRequest.withInitial(Unit),
requestRefreshFlow = mergeRequestRefreshRequest.combine(stateEventsRefreshRequest.withInitial(Unit)) { _, _ -> },
shouldTryToLoadAll = false
shouldTryToLoadAll = true
) { uri, eTag ->
api.rest.loadUpdatableJsonList<GitLabResourceStateEventDTO>(
GitLabApiRequestName.REST_GET_MERGE_REQUEST_STATE_EVENTS, uri, eTag
)
}
override val stateEvents =
stateEventsHolder.resultOrErrorFlow.modelFlow(cs, LOG)
api.rest.loadUpdatableJsonList<GitLabResourceStateEventDTO>(
GitLabApiRequestName.REST_GET_MERGE_REQUEST_STATE_EVENTS, uri, eTag
)
}.resultOrErrorFlow.modelFlow(cs, LOG)
}
private val labelEventsHolder =
override val labelEvents by lazy {
startGitLabRestETagListLoaderIn(
cs,
getMergeRequestLabelEventsUri(glProject, iid),
@@ -196,17 +194,15 @@ internal class LoadedGitLabMergeRequest(
requestReloadFlow = mergeRequestReloadRequest.withInitial(Unit),
requestRefreshFlow = mergeRequestRefreshRequest,
shouldTryToLoadAll = false
shouldTryToLoadAll = true
) { uri, eTag ->
api.rest.loadUpdatableJsonList<GitLabResourceLabelEventDTO>(
GitLabApiRequestName.REST_GET_MERGE_REQUEST_LABEL_EVENTS, uri, eTag
)
}
override val labelEvents =
labelEventsHolder.resultOrErrorFlow.modelFlow(cs, LOG)
api.rest.loadUpdatableJsonList<GitLabResourceLabelEventDTO>(
GitLabApiRequestName.REST_GET_MERGE_REQUEST_LABEL_EVENTS, uri, eTag
)
}.resultOrErrorFlow.modelFlow(cs, LOG)
}
private val milestoneEventsHolder =
override val milestoneEvents by lazy {
startGitLabRestETagListLoaderIn(
cs,
getMergeRequestMilestoneEventsUri(glProject, iid),
@@ -214,15 +210,13 @@ internal class LoadedGitLabMergeRequest(
requestReloadFlow = mergeRequestReloadRequest.withInitial(Unit),
requestRefreshFlow = mergeRequestRefreshRequest,
shouldTryToLoadAll = false
shouldTryToLoadAll = true
) { uri, eTag ->
api.rest.loadUpdatableJsonList<GitLabResourceMilestoneEventDTO>(
GitLabApiRequestName.REST_GET_MERGE_REQUEST_MILESTONE_EVENTS, uri, eTag
)
}
override val milestoneEvents =
milestoneEventsHolder.resultOrErrorFlow.modelFlow(cs, LOG)
api.rest.loadUpdatableJsonList<GitLabResourceMilestoneEventDTO>(
GitLabApiRequestName.REST_GET_MERGE_REQUEST_MILESTONE_EVENTS, uri, eTag
)
}.resultOrErrorFlow.modelFlow(cs, LOG)
}
private val _isLoading: MutableStateFlow<Boolean> = MutableStateFlow(false)
override val isLoading: SharedFlow<Boolean> = _isLoading.asSharedFlow()
@@ -294,7 +288,6 @@ internal class LoadedGitLabMergeRequest(
override fun refreshData() {
cs.launch {
updateData()
startRefreshCycle()
}
}
@@ -306,20 +299,14 @@ internal class LoadedGitLabMergeRequest(
private suspend fun updateData() {
mergeRequestRefreshRequest.emit(Unit)
discussionsContainer.checkUpdates()
}
private suspend fun startRefreshCycle() {
stateEventsHolder.loadAll()
labelEventsHolder.loadAll()
milestoneEventsHolder.loadAll()
discussionsContainer.requestDiscussionsRefresh()
}
override suspend fun merge(commitMessage: String) {
withContext(cs.coroutineContext + Dispatchers.IO) {
runMerge(commitMessage, withSquash = false)
}
discussionsContainer.checkUpdates()
discussionsContainer.requestDiscussionsRefresh()
GitLabStatistics.logMrActionExecuted(project, GitLabStatistics.MergeRequestAction.MERGE)
}
@@ -327,7 +314,7 @@ internal class LoadedGitLabMergeRequest(
withContext(cs.coroutineContext + Dispatchers.IO) {
runMerge(commitMessage, withSquash = true)
}
discussionsContainer.checkUpdates()
discussionsContainer.requestDiscussionsRefresh()
GitLabStatistics.logMrActionExecuted(project, GitLabStatistics.MergeRequestAction.SQUASH_MERGE)
}
@@ -335,7 +322,7 @@ internal class LoadedGitLabMergeRequest(
withContext(cs.coroutineContext + Dispatchers.IO) {
runRebase()
}
discussionsContainer.checkUpdates()
discussionsContainer.requestDiscussionsRefresh()
GitLabStatistics.logMrActionExecuted(project, GitLabStatistics.MergeRequestAction.REBASE)
}
@@ -389,7 +376,7 @@ internal class LoadedGitLabMergeRequest(
.getResultOrThrow()
updateMergeRequestData(updatedMergeRequest)
}
discussionsContainer.checkUpdates()
discussionsContainer.requestDiscussionsRefresh()
GitLabStatistics.logMrActionExecuted(project, GitLabStatistics.MergeRequestAction.POST_REVIEW)
}
@@ -405,7 +392,7 @@ internal class LoadedGitLabMergeRequest(
updateMergeRequestData(updatedMergeRequest)
}
discussionsContainer.checkUpdates()
discussionsContainer.requestDiscussionsRefresh()
GitLabStatistics.logMrActionExecuted(project, GitLabStatistics.MergeRequestAction.SET_REVIEWERS)
}
@@ -417,7 +404,7 @@ internal class LoadedGitLabMergeRequest(
updateMergeRequestData(updatedMergeRequest)
}
}
discussionsContainer.checkUpdates()
discussionsContainer.requestDiscussionsRefresh()
GitLabStatistics.logMrActionExecuted(project, GitLabStatistics.MergeRequestAction.REVIEWER_REREVIEW)
}
@@ -89,33 +89,31 @@ class GitLabMergeRequestDiscussionsContainerImpl(
private val reloadRequests = MutableSharedFlow<Unit>(replay = 1, onBufferOverflow = BufferOverflow.DROP_OLDEST).apply {
tryEmit(Unit)
}
private val updateRequests = MutableSharedFlow<Unit>(replay = 1, onBufferOverflow = BufferOverflow.DROP_OLDEST)
private val refreshRequests = MutableSharedFlow<Unit>(replay = 1, onBufferOverflow = BufferOverflow.DROP_OLDEST)
private val discussionEvents = MutableSharedFlow<Change<GitLabDiscussionRestDTO>>()
private val discussionsDataHolder =
private val nonEmptyDiscussionsData: SharedFlow<Result<List<GitLabDiscussionRestDTO>>> by lazy {
startGitLabRestETagListLoaderIn(
cs,
getMergeRequestDiscussionsUri(glProject, mr.iid),
{ it.id },
requestReloadFlow = reloadRequests,
requestRefreshFlow = updateRequests,
requestRefreshFlow = refreshRequests,
requestChangeFlow = discussionEvents,
shouldTryToLoadAll = false
shouldTryToLoadAll = true
) { uri, eTag ->
api.rest.loadUpdatableJsonList<GitLabDiscussionRestDTO>(
GitLabApiRequestName.REST_GET_MERGE_REQUEST_DISCUSSIONS, uri, eTag
)
}
private val nonEmptyDiscussionsData: SharedFlow<Result<List<GitLabDiscussionRestDTO>>> =
discussionsDataHolder.resultOrErrorFlow
}.resultOrErrorFlow
.mapCatching { discussions -> discussions.filter { it.notes.isNotEmpty() } }
.modelFlow(cs, LOG)
}
override val discussions: Flow<Result<List<GitLabMergeRequestDiscussion>>> =
override val discussions: Flow<Result<List<GitLabMergeRequestDiscussion>>> by lazy {
nonEmptyDiscussionsData
.transformConsecutiveSuccesses {
mapFiltered { !it.notes.first().system }
@@ -131,8 +129,9 @@ class GitLabMergeRequestDiscussionsContainerImpl(
)
}
.modelFlow(cs, LOG)
}
override val systemNotes: Flow<Result<List<GitLabNote>>> =
override val systemNotes: Flow<Result<List<GitLabNote>>> by lazy {
nonEmptyDiscussionsData
.transformConsecutiveSuccesses {
// When one note in a discussion is a system note, all are, so we check the first.
@@ -145,12 +144,13 @@ class GitLabMergeRequestDiscussionsContainerImpl(
)
}
.modelFlow(cs, LOG)
}
private val draftNotesEvents = MutableSharedFlow<Change<GitLabMergeRequestDraftNoteRestDTO>>()
private val draftNotesDataHolder =
private val draftNotesData by lazy {
if (glMetadata == null || glMetadata.version < GitLabVersion(15, 9)) {
null
flowOf(Result.success(emptyList()))
}
else {
startGitLabRestETagListLoaderIn(
@@ -159,38 +159,36 @@ class GitLabMergeRequestDiscussionsContainerImpl(
{ it.id },
requestReloadFlow = reloadRequests,
requestRefreshFlow = updateRequests,
requestRefreshFlow = refreshRequests,
requestChangeFlow = draftNotesEvents,
shouldTryToLoadAll = false
shouldTryToLoadAll = true
) { uri, eTag ->
api.rest.loadUpdatableJsonList<GitLabMergeRequestDraftNoteRestDTO>(
GitLabApiRequestName.REST_GET_DRAFT_NOTES, uri, eTag
)
}
}
private val draftNotesData =
(draftNotesDataHolder?.resultOrErrorFlow ?: flowOf(Result.success(emptyList())))
.mapCatching { draftNotes ->
}.resultOrErrorFlow.mapCatching { draftNotes ->
if (draftNotes.isEmpty()) return@mapCatching emptyList()
draftNotes.map { it }
}
.modelFlow(cs, LOG)
}.modelFlow(cs, LOG)
}
}
override val draftNotes: Flow<Result<Collection<GitLabMergeRequestDraftNote>>> = flow {
draftNotesData
.transformConsecutiveSuccesses {
mapDataToModel(
GitLabMergeRequestDraftNoteRestDTO::id,
{
GitLabMergeRequestDraftNoteImpl(this, api, glMetadata, glProject, mr, { draftNotesEvents.emit(it) }, it, currentUser)
},
{ update(it) }
)
}.collect(this)
}.modelFlow(cs, LOG)
override val draftNotes: Flow<Result<Collection<GitLabMergeRequestDraftNote>>> by lazy {
flow {
draftNotesData
.transformConsecutiveSuccesses {
mapDataToModel(
GitLabMergeRequestDraftNoteRestDTO::id,
{
GitLabMergeRequestDraftNoteImpl(this, api, glMetadata, glProject, mr, { draftNotesEvents.emit(it) }, it, currentUser)
},
{ update(it) }
)
}.collect(this)
}.modelFlow(cs, LOG)
}
private fun getDiscussionDraftNotes(discussionId: GitLabId): Flow<Result<List<GitLabMergeRequestDraftNote>>> {
// Convert discussion ID down to REST ID as it's safer than converting from REST to GQL
@@ -263,7 +261,7 @@ class GitLabMergeRequestDiscussionsContainerImpl(
}
withContext(NonCancellable) {
draftNotesEvents.emit(AllDeleted())
checkUpdates()
requestDiscussionsRefresh()
}
}
GitLabStatistics.logMrActionExecuted(project, GitLabStatistics.MergeRequestAction.SUBMIT_DRAFT_NOTES)
@@ -273,10 +271,7 @@ class GitLabMergeRequestDiscussionsContainerImpl(
reloadRequests.emit(Unit)
}
suspend fun checkUpdates() {
updateRequests.emit(Unit)
draftNotesDataHolder?.loadAll()
discussionsDataHolder.loadAll()
suspend fun requestDiscussionsRefresh() {
refreshRequests.emit(Unit)
}
}