[gitlab] refactor: more idiomatic viewmodel state

GitOrigin-RevId: 24d8d8aaf13098503f50df7fbd81c8a0119f1b67
2026-09-27 10:03:11 +07:00 · 2025-09-11 14:45:50 +00:00
parent bb562fb9cb
commit 3a81051ba5
8 changed files with 156 additions and 132 deletions
@@ -2,10 +2,12 @@
 package org.jetbrains.plugins.gitlab.mergerequest.data

 import com.intellij.collaboration.async.*
+import com.intellij.collaboration.util.ComputedResult
 import com.intellij.collaboration.util.ResultUtil.runCatchingUser
 import com.intellij.openapi.diagnostic.logger
 import com.intellij.openapi.project.Project
 import git4idea.GitStandardRemoteBranch
+import git4idea.changes.GitBranchComparisonResult
 import git4idea.remote.hosting.GitRemoteBranchesUtil
 import git4idea.remote.hosting.changesSignalFlow
 import git4idea.repo.GitRepository
@@ -89,6 +91,9 @@ interface GitLabMergeRequest : GitLabMergeRequestDiscussionsContainer {
  suspend fun reviewerRereview(reviewers: Collection<GitLabReviewerDTO>)
 }

+internal fun GitLabMergeRequest.changesComputationState(): Flow<ComputedResult<GitBranchComparisonResult>> =
+  computationStateFlow(changes) { it.getParsedChanges() }
+
 internal class LoadedGitLabMergeRequest(
  private val project: Project,
  parentCs: CoroutineScope,
@@ -25,9 +25,9 @@ import org.jetbrains.plugins.gitlab.util.GitLabProjectMapping

 interface GitLabMergeRequestChanges {
  /**
-   * List of merge request commits
+   * Load the list of merge request commits
   */
-  val commits: Deferred<List<GitLabCommit>>
+  suspend fun getCommits(): List<GitLabCommit>

  /**
   * Load and parse changes diffs
@@ -57,7 +57,7 @@ class GitLabMergeRequestChangesImpl(

  private val glProject = projectMapping.repository

-  override val commits: Deferred<List<GitLabCommit>> = cs.async {
+  private val commits: Deferred<List<GitLabCommit>> = cs.async {
    if (glMetadata != null && glMetadata.version < GitLabVersion(14, 7)) {
      val initialURI = api.getMergeRequestCommitsURI(glProject, mergeRequestDetails.iid)
      return@async ApiPageUtil.createPagesFlowByLinkHeader(initialURI) { uri -> api.rest.loadMergeRequestCommits(uri) }
@@ -72,6 +72,8 @@ class GitLabMergeRequestChangesImpl(
      .asReversed()
  }

+  override suspend fun getCommits(): List<GitLabCommit> = commits.await()
+
  private val parsedChanges = cs.async(start = CoroutineStart.LAZY) {
    loadChanges(commits.await())
  }
@@ -11,7 +11,6 @@ interface GitLabMergeRequestNotePositionMapping {
  class Actual(val change: ChangesSelection.Precise) : GitLabMergeRequestNotePositionMapping
  class Outdated(val change: ChangesSelection.Precise) : GitLabMergeRequestNotePositionMapping
  object Obsolete : GitLabMergeRequestNotePositionMapping
-  class Error(val error: Throwable) : GitLabMergeRequestNotePositionMapping

  companion object {
    private val LOG = logger<GitLabMergeRequestNotePositionMapping>()
@@ -1,15 +1,16 @@
 // Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
 package org.jetbrains.plugins.gitlab.mergerequest.data

-import com.intellij.collaboration.async.*
-import com.intellij.openapi.diagnostic.logger
-import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.ExperimentalCoroutinesApi
+import com.intellij.collaboration.async.Change
+import com.intellij.collaboration.async.Deleted
+import com.intellij.collaboration.async.childScope
+import com.intellij.collaboration.async.mapState
+import com.intellij.collaboration.util.ComputedResult
+import com.intellij.collaboration.util.map
+import kotlinx.coroutines.*
 import kotlinx.coroutines.flow.*
 import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
-import kotlinx.coroutines.withContext
 import org.jetbrains.plugins.gitlab.api.*
 import org.jetbrains.plugins.gitlab.api.dto.GitLabAwardEmojiDTO
 import org.jetbrains.plugins.gitlab.api.dto.GitLabMergeRequestDraftNoteRestDTO
@@ -53,7 +54,7 @@ interface MutableGitLabNote : GitLabNote {
 interface GitLabMergeRequestNote : GitLabNote {
  val canReact: Boolean
  val position: StateFlow<GitLabNotePosition?>
-  val positionMapping: Flow<GitLabMergeRequestNotePositionMapping?>
+  val positionMapping: StateFlow<ComputedResult<GitLabMergeRequestNotePositionMapping?>>

  val awardEmoji: StateFlow<List<GitLabAwardEmojiDTO>>

@@ -68,8 +69,6 @@ interface GitLabMergeRequestDraftNote : GitLabMergeRequestNote, MutableGitLabNot
  override val resolved: StateFlow<Boolean> get() = MutableStateFlow(false)
 }

-private val LOG = logger<GitLabDiscussion>()
-
 class MutableGitLabMergeRequestNote(
  parentCs: CoroutineScope,
  private val api: GitLabApi,
@@ -96,7 +95,8 @@ class MutableGitLabMergeRequestNote(
  override val position: StateFlow<GitLabNotePosition?> = data.mapState(cs) {
    it.position?.let(GitLabNotePosition::from)
  }
-  override val positionMapping: Flow<GitLabMergeRequestNotePositionMapping?> = position.mapPosition(mr).modelFlow(cs, LOG)
+  override val positionMapping: StateFlow<ComputedResult<GitLabMergeRequestNotePositionMapping?>> =
+    position.mapPosition(mr).stateIn(cs, SharingStarted.Eagerly, ComputedResult.loading())
  override fun canEdit(): Boolean = true
  override fun canSubmit(): Boolean = false

@@ -180,7 +180,8 @@ class GitLabMergeRequestDraftNoteImpl(
  override val awardEmoji: StateFlow<List<GitLabAwardEmojiDTO>> = MutableStateFlow(emptyList())

  override val position: StateFlow<GitLabNotePosition?> = data.mapState(cs) { it.position.let(GitLabNotePosition::from) }
-  override val positionMapping: Flow<GitLabMergeRequestNotePositionMapping?> = position.mapPosition(mr).modelFlow(cs, LOG)
+  override val positionMapping: StateFlow<ComputedResult<GitLabMergeRequestNotePositionMapping?>> =
+    position.mapPosition(mr).stateIn(cs, SharingStarted.Eagerly, ComputedResult.loading())

  override fun canEdit(): Boolean =
    glMetadata != null && GitLabVersion(15, 10) <= glMetadata.version
@@ -239,18 +240,24 @@ class GitLabMergeRequestDraftNoteImpl(
 }

@OptIn(ExperimentalCoroutinesApi::class)
-private fun Flow<GitLabNotePosition?>.mapPosition(mr: GitLabMergeRequest): Flow<GitLabMergeRequestNotePositionMapping?> =
-  flatMapLatest { position ->
-    if (position == null) return@flatMapLatest flowOf(null)
-
-    mr.changes.map {
-      try {
-        val allChanges = it.getParsedChanges()
+private fun Flow<GitLabNotePosition?>.mapPosition(mr: GitLabMergeRequest): Flow<ComputedResult<GitLabMergeRequestNotePositionMapping?>> =
+  this.combineTransform(mr.changesComputationState()) { position, changesResult ->
+    if (position == null) {
+      emit(ComputedResult.success(null))
+      return@combineTransform
+    }
+    try {
+      changesResult.map { allChanges ->
        GitLabMergeRequestNotePositionMapping.map(allChanges, position)
+      }.let {
+        emit(it)
      }
-      catch (e: Exception) {
-        GitLabMergeRequestNotePositionMapping.Error(e)
-      }
+    }
+    catch (ce: CancellationException) {
+      throw ce
+    }
+    catch (e: Exception) {
+      emit(ComputedResult.failure(e))
    }
  }

@@ -54,7 +54,7 @@ internal class GitLabMergeRequestChangesViewModelImpl(

  override val reviewCommits: SharedFlow<List<GitLabCommitViewModel>> =
    mergeRequest.changes
-      .map { it.commits.await().map { commit -> GitLabCommitViewModel(project, mergeRequest, commit) } }
+      .map { it.getCommits().map { commit -> GitLabCommitViewModel(project, mergeRequest, commit) } }
      .modelFlow(cs, LOG)

  override val selectedCommitIndex: SharedFlow<Int> = reviewCommits.combine(delegate.selectedCommit) { commits, sha ->
@@ -222,7 +222,7 @@ internal class GitLabMergeRequestReviewFlowViewModelImpl(
    val details = mergeRequest.details.first()
    val sourceBranch = details.sourceBranch
    val targetBranch = details.targetBranch
-    val commits = mergeRequest.changes.first().commits.await()
+    val commits = mergeRequest.changes.first().getCommits()
    val commitMessage: String? = withContext(scope.coroutineContext + Dispatchers.EDT) {
      val body = "* " + StringUtil.join(commits, { it.fullTitle }, "\n\n* ")
      val dialog = ReviewMergeCommitMessageDialog(
@@ -2,14 +2,16 @@
 package org.jetbrains.plugins.gitlab.mergerequest.ui.timeline

 import com.intellij.collaboration.async.childScope
-import com.intellij.collaboration.async.modelFlow
 import com.intellij.collaboration.ui.codereview.diff.DiffLineLocation
 import com.intellij.collaboration.util.ChangesSelection
+import com.intellij.collaboration.util.ComputedResult
 import com.intellij.collaboration.util.selectedChange
 import com.intellij.diff.util.Side
 import com.intellij.openapi.diagnostic.logger
 import com.intellij.openapi.diff.impl.patch.PatchHunk
 import com.intellij.openapi.diff.impl.patch.TextFilePatch
+import com.intellij.openapi.progress.checkCanceled
+import git4idea.changes.GitBranchComparisonResult
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.channels.BufferOverflow
@@ -18,21 +20,17 @@ import kotlinx.coroutines.launch
 import org.jetbrains.plugins.gitlab.mergerequest.data.GitLabMergeRequest
 import org.jetbrains.plugins.gitlab.mergerequest.data.GitLabMergeRequestNotePositionMapping
 import org.jetbrains.plugins.gitlab.mergerequest.data.GitLabNotePosition
-import org.jetbrains.plugins.gitlab.mergerequest.ui.timeline.GitLabDiscussionDiffViewModel.PatchHunkResult
+import org.jetbrains.plugins.gitlab.mergerequest.ui.timeline.GitLabDiscussionDiffViewModel.PatchHunkWithAnchor
+import kotlin.coroutines.cancellation.CancellationException

 interface GitLabDiscussionDiffViewModel {
  val position: GitLabNotePosition
-  val mapping: Flow<GitLabMergeRequestNotePositionMapping>
-  val patchHunk: Flow<PatchHunkResult>
+  val patchHunk: StateFlow<ComputedResult<PatchHunkWithAnchor?>>

  val showDiffRequests: Flow<ChangesSelection.Precise>
-  val showDiffHandler: Flow<(() -> Unit)?>
+  val showDiffHandler: StateFlow<(() -> Unit)?>

-  sealed interface PatchHunkResult {
-    class Loaded(val hunk: PatchHunk, val anchor: DiffLineLocation) : PatchHunkResult
-    object NotLoaded : PatchHunkResult
-    class Error(val error: Throwable) : PatchHunkResult
-  }
+  data class PatchHunkWithAnchor(val hunk: PatchHunk, val anchor: DiffLineLocation)
 }

 private val LOG = logger<GitLabDiscussionDiffViewModel>()
@@ -46,41 +44,7 @@ class GitLabDiscussionDiffViewModelImpl(

  private val cs = parentCs.childScope(this::class)

-  override val mapping: Flow<GitLabMergeRequestNotePositionMapping> = mr.changes.mapLatest {
-    try {
-      val allChanges = it.getParsedChanges()
-      GitLabMergeRequestNotePositionMapping.map(allChanges, position)
-    }
-    catch (e: Exception) {
-      GitLabMergeRequestNotePositionMapping.Error(e)
-    }
-  }.modelFlow(cs, LOG)
-
-
-  override val patchHunk: Flow<PatchHunkResult> = channelFlow {
-    mr.changes.mapLatest { it.getParsedChanges() }.catch { e ->
-      send(PatchHunkResult.Error(e))
-    }.combine(mapping) { allChanges, mapping ->
-      when {
-        mapping is GitLabMergeRequestNotePositionMapping.Actual && mapping.change.location != null -> {
-          val patch = allChanges.patchesByChange[mapping.change.selectedChange]?.patch ?: run {
-            LOG.warn("Can't find patch for ${mapping.change.selectedChange}")
-            return@combine PatchHunkResult.NotLoaded
-          }
-
-          val (hunk, anchor) = findHunkAndAnchor(patch, mapping.change.location!!) ?: run {
-            LOG.debug("Unable to map location for position $position in patch\n$patch")
-            return@combine PatchHunkResult.NotLoaded
-          }
-          PatchHunkResult.Loaded(hunk, anchor)
-        }
-        mapping is GitLabMergeRequestNotePositionMapping.Error -> PatchHunkResult.Error(mapping.error)
-        else -> PatchHunkResult.NotLoaded
-      }
-    }.collectLatest {
-      send(it)
-    }
-  }.modelFlow(cs, LOG)
+  override val patchHunk: StateFlow<ComputedResult<PatchHunkWithAnchor?>>

  private val _showDiffRequests = MutableSharedFlow<ChangesSelection.Precise>(
    extraBufferCapacity = 1,
@@ -88,38 +52,77 @@ class GitLabDiscussionDiffViewModelImpl(
  )
  override val showDiffRequests: Flow<ChangesSelection.Precise> = _showDiffRequests.asSharedFlow()

-  override val showDiffHandler: Flow<(() -> Unit)?> = mapping.map {
-    when (it) {
-      is GitLabMergeRequestNotePositionMapping.Actual -> {
-        { requestFullDiff(it.change) }
+  override val showDiffHandler: StateFlow<(() -> Unit)?>
+
+  init {
+    patchHunk = MutableStateFlow(ComputedResult.loading())
+    showDiffHandler = MutableStateFlow(null)
+
+    cs.launch {
+      mr.changes.collectLatest {
+        try {
+          val allChanges = it.getParsedChanges()
+          val mapping = GitLabMergeRequestNotePositionMapping.map(allChanges, position)
+
+          patchHunk.value = ComputedResult.success(getPatchHunk(allChanges, mapping))
+          showDiffHandler.value = when (mapping) {
+            is GitLabMergeRequestNotePositionMapping.Actual -> {
+              {
+                _showDiffRequests.tryEmit(mapping.change)
+              }
+            }
+            is GitLabMergeRequestNotePositionMapping.Outdated -> {
+              {
+                _showDiffRequests.tryEmit(mapping.change)
+              }
+            }
+            else -> null
+          }
+
+        }
+        catch (@Suppress("IncorrectCancellationExceptionHandling") _: CancellationException) {
+          // getParsedChanges can be canceled outside
+          checkCanceled()
+        }
+        catch (e: Exception) {
+          patchHunk.value = ComputedResult.failure(e)
+          showDiffHandler.value = null
+        }
      }
-      is GitLabMergeRequestNotePositionMapping.Outdated -> {
-        { requestFullDiff(it.change) }
-      }
-      else -> null
    }
  }

-  private fun requestFullDiff(change: ChangesSelection.Precise) {
-    cs.launch {
-      _showDiffRequests.emit(change)
+  private fun getPatchHunk(
+    allChanges: GitBranchComparisonResult,
+    mapping: GitLabMergeRequestNotePositionMapping,
+  ): PatchHunkWithAnchor? {
+    if (mapping !is GitLabMergeRequestNotePositionMapping.Actual) return null
+    val location = mapping.change.location ?: return null
+
+    val patch = allChanges.patchesByChange[mapping.change.selectedChange]?.patch ?: run {
+      LOG.warn("Can't find patch for ${mapping.change.selectedChange}")
+      return null
+    }
+
+    return findHunkAndAnchor(patch, location) ?: run {
+      LOG.debug("Unable to map location for position $position in patch\n$patch")
+      null
    }
  }
 }

-private fun findHunkAndAnchor(patch: TextFilePatch, location: DiffLineLocation): Pair<PatchHunk, DiffLineLocation>? {
+private fun findHunkAndAnchor(patch: TextFilePatch, location: DiffLineLocation): PatchHunkWithAnchor? {
  val (side, index) = location
  return when (side) {
    Side.LEFT -> {
      patch.hunks.find {
        index >= it.startLineBefore && index < it.endLineBefore
-      }?.let { it to location }
+      }
    }
    Side.RIGHT -> {
      patch.hunks.find {
        index >= it.startLineAfter && index < it.endLineAfter
-      }?.let { it to location }
+      }
    }
-    else -> null
-  }
+  }?.let { PatchHunkWithAnchor(it, location) }
 }
@@ -18,16 +18,15 @@ import com.intellij.collaboration.ui.util.DimensionRestrictions
 import com.intellij.collaboration.ui.util.bindChildIn
 import com.intellij.collaboration.ui.util.bindContentIn
 import com.intellij.collaboration.ui.util.bindVisibilityIn
+import com.intellij.collaboration.util.exceptionOrNull
+import com.intellij.collaboration.util.getOrNull
 import com.intellij.openapi.editor.EditorFactory
 import com.intellij.openapi.project.Project
 import com.intellij.openapi.util.text.HtmlBuilder
 import com.intellij.openapi.util.text.HtmlChunk
 import com.intellij.ui.HyperlinkAdapter
 import com.intellij.ui.components.panels.Wrapper
-import com.intellij.util.ui.EmptyIcon
-import com.intellij.util.ui.JBUI
-import com.intellij.util.ui.SingleComponentCenteringLayout
-import com.intellij.util.ui.UIUtil
+import com.intellij.util.ui.*
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.*
@@ -189,56 +188,65 @@ internal object GitLabMergeRequestTimelineDiscussionComponentFactory {

  private fun CoroutineScope.createDiffPanel(project: Project, diffVm: GitLabDiscussionDiffViewModel): JComponent =
    Wrapper(LoadingLabel()).apply {
-      bindContentIn(this@createDiffPanel, diffVm.patchHunk) { hunkState ->
+      bindContentIn(this@createDiffPanel, diffVm.patchHunk) { hunkResult ->
        val loadedDiffCs = this
-        when (hunkState) {
-          is GitLabDiscussionDiffViewModel.PatchHunkResult.Loaded -> {
-            TimelineDiffComponentFactory.createDiffComponentIn(loadedDiffCs, project, EditorFactory.getInstance(), hunkState.hunk,
-                                                               hunkState.anchor, null)
-          }
-          is GitLabDiscussionDiffViewModel.PatchHunkResult.Error,
-          GitLabDiscussionDiffViewModel.PatchHunkResult.NotLoaded -> {
-            JPanel(SingleComponentCenteringLayout()).apply {
-              isOpaque = false
-              border = JBUI.Borders.empty(16)

-              bindChildIn(loadedDiffCs, diffVm.showDiffHandler.asActionHandler()) { clickListener ->
-                if (clickListener != null) {
-                  val text = buildCantLoadHunkText(hunkState)
-                    .append(HtmlChunk.p().child(
-                      HtmlChunk.link(OPEN_DIFF_LINK_HREF, GitLabBundle.message("merge.request.timeline.discussion.open.full.diff"))))
-                    .wrapWith(HtmlChunk.div("text-align: center"))
-                    .toString()
+        if (hunkResult.isInProgress) {
+          return@bindContentIn LoadingLabel()
+        }

-                  SimpleHtmlPane(addBrowserListener = false).apply {
-                    setHtmlBody(text)
-                  }.also {
-                    it.addHyperlinkListener(object : HyperlinkAdapter() {
-                      override fun hyperlinkActivated(e: HyperlinkEvent) {
-                        if (e.description == OPEN_DIFF_LINK_HREF) {
-                          clickListener.actionPerformed(ActionEvent(it, ActionEvent.ACTION_PERFORMED, "execute"))
-                        }
-                      }
-                    })
-                  }
-                }
-                else {
-                  val text = buildCantLoadHunkText(hunkState).toString()
-                  SimpleHtmlPane(text)
-                }
-              }
-            }
-          }
+        val hunk = hunkResult.getOrNull()
+        if (hunk != null) {
+          TimelineDiffComponentFactory.createDiffComponentIn(loadedDiffCs, project, EditorFactory.getInstance(), hunk.hunk, hunk.anchor, null)
+        }
+        else {
+          createMissingHunkComponent(diffVm, hunkResult.exceptionOrNull())
        }
      }
    }

-  private fun buildCantLoadHunkText(hunkState: GitLabDiscussionDiffViewModel.PatchHunkResult) =
+  private fun createMissingHunkComponent(
+    diffVm: GitLabDiscussionDiffViewModel,
+    error: Throwable?,
+  ): JPanel = JPanel(SingleComponentCenteringLayout()).apply {
+    isOpaque = false
+    border = JBUI.Borders.empty(16)
+
+    launchOnShow("Hunk text") {
+      bindChildIn(this, diffVm.showDiffHandler.asActionHandler()) { clickListener ->
+        if (clickListener != null) {
+          val text = buildCantLoadHunkText(error)
+            .append(HtmlChunk.p().child(
+              HtmlChunk.link(OPEN_DIFF_LINK_HREF, GitLabBundle.message("merge.request.timeline.discussion.open.full.diff"))))
+            .wrapWith(HtmlChunk.div("text-align: center"))
+            .toString()
+
+          SimpleHtmlPane(addBrowserListener = false).apply {
+            setHtmlBody(text)
+          }.also {
+            it.addHyperlinkListener(object : HyperlinkAdapter() {
+              override fun hyperlinkActivated(e: HyperlinkEvent) {
+                if (e.description == OPEN_DIFF_LINK_HREF) {
+                  clickListener.actionPerformed(ActionEvent(it, ActionEvent.ACTION_PERFORMED, "execute"))
+                }
+              }
+            })
+          }
+        }
+        else {
+          val text = buildCantLoadHunkText(error).toString()
+          SimpleHtmlPane(text)
+        }
+      }
+    }
+  }
+
+  private fun buildCantLoadHunkText(error: Throwable?) =
    HtmlBuilder()
      .append(HtmlChunk.p().addText(GitLabBundle.message("merge.request.timeline.discussion.cant.load.diff")))
      .apply {
-        if (hunkState is GitLabDiscussionDiffViewModel.PatchHunkResult.Error) {
-          append(HtmlChunk.p().addText(hunkState.error.localizedMessageOrClassName()))
+        if (error != null) {
+          append(HtmlChunk.p().addText(error.localizedMessageOrClassName()))
        }
      }