(refs #1618)Check conflict on the online editor in validation

2026-05-07 05:07:02 +02:00 · 2017-06-29 18:47:12 +09:00
parent 57228147ce
commit a0a7c7f428
4 changed files with 68 additions and 64 deletions
--- a/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala
+++ b/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala
@@ -25,6 +25,7 @@ import org.eclipse.jgit.dircache.{DirCache, DirCacheBuilder}
 import org.eclipse.jgit.errors.MissingObjectException
 import org.eclipse.jgit.lib._
 import org.scalatra._
+import org.scalatra.i18n.Messages


 class RepositoryViewerController extends RepositoryViewerControllerBase
@@ -94,7 +95,7 @@ trait RepositoryViewerControllerBase extends ControllerBase {
    "lineSeparator" -> trim(label("Line Separator", text(required))),
    "newFileName"   -> trim(label("Filename", text(required))),
    "oldFileName"   -> trim(label("Old filename", optional(text()))),
-    "commit"        -> trim(label("Commit", text(required)))
+    "commit"        -> trim(label("Commit", text(required, conflict)))
  )(EditorForm.apply)

  val deleteForm = mapping(
@@ -102,7 +103,7 @@ trait RepositoryViewerControllerBase extends ControllerBase {
    "path"     -> trim(label("Path", text())),
    "message"  -> trim(label("Message", optional(text()))),
    "fileName" -> trim(label("Filename", text(required))),
-    "commit"        -> trim(label("Commit", text(required)))
+    "commit"   -> trim(label("Commit", text(required, conflict)))
  )(DeleteForm.apply)

  val commentForm = mapping(
@@ -274,21 +275,17 @@ trait RepositoryViewerControllerBase extends ControllerBase {
  })

  post("/:owner/:repository/create", editorForm)(writableUsersOnly { (form, repository) =>
-    try {
-      commitFile(
-        repository  = repository,
-        branch      = form.branch,
-        path        = form.path,
-        newFileName = Some(form.newFileName),
-        oldFileName = None,
-        content     = appendNewLine(convertLineSeparator(form.content, form.lineSeparator), form.lineSeparator),
-        charset     = form.charset,
-        message     = form.message.getOrElse(s"Create ${form.newFileName}"),
-        commit      = form.commit
-      )
-    } catch {
-      case e: Exception => flash += "error" -> e.getMessage
-    }
+    commitFile(
+      repository  = repository,
+      branch      = form.branch,
+      path        = form.path,
+      newFileName = Some(form.newFileName),
+      oldFileName = None,
+      content     = appendNewLine(convertLineSeparator(form.content, form.lineSeparator), form.lineSeparator),
+      charset     = form.charset,
+      message     = form.message.getOrElse(s"Create ${form.newFileName}"),
+      commit      = form.commit
+    )

    redirect(s"/${repository.owner}/${repository.name}/blob/${form.branch}/${
      if(form.path.length == 0) urlEncode(form.newFileName) else s"${form.path}/${urlEncode(form.newFileName)}"
@@ -296,52 +293,41 @@ trait RepositoryViewerControllerBase extends ControllerBase {
  })

  post("/:owner/:repository/update", editorForm)(writableUsersOnly { (form, repository) =>
-    try {
-      commitFile(
-        repository  = repository,
-        branch      = form.branch,
-        path        = form.path,
-        newFileName = Some(form.newFileName),
-        oldFileName = form.oldFileName,
-        content     = appendNewLine(convertLineSeparator(form.content, form.lineSeparator), form.lineSeparator),
-        charset     = form.charset,
-        message     = if (form.oldFileName.contains(form.newFileName)) {
-          form.message.getOrElse(s"Update ${form.newFileName}")
-        } else {
-          form.message.getOrElse(s"Rename ${form.oldFileName.get} to ${form.newFileName}")
-        },
-        commit      = form.commit
-      )
-    } catch {
-      case e: Exception => flash += "error" -> e.getMessage
-    }
+    commitFile(
+      repository  = repository,
+      branch      = form.branch,
+      path        = form.path,
+      newFileName = Some(form.newFileName),
+      oldFileName = form.oldFileName,
+      content     = appendNewLine(convertLineSeparator(form.content, form.lineSeparator), form.lineSeparator),
+      charset     = form.charset,
+      message     = if (form.oldFileName.contains(form.newFileName)) {
+        form.message.getOrElse(s"Update ${form.newFileName}")
+      } else {
+        form.message.getOrElse(s"Rename ${form.oldFileName.get} to ${form.newFileName}")
+      },
+      commit      = form.commit
+    )
+
    redirect(s"/${repository.owner}/${repository.name}/blob/${form.branch}/${
      if (form.path.length == 0) urlEncode(form.newFileName) else s"${form.path}/${urlEncode(form.newFileName)}"
    }")
  })

  post("/:owner/:repository/remove", deleteForm)(writableUsersOnly { (form, repository) =>
-    try {
-      commitFile(
-        repository  = repository,
-        branch      = form.branch,
-        path        = form.path,
-        newFileName = None,
-        oldFileName = Some(form.fileName),
-        content     = "",
-        charset     = "",
-        message     = form.message.getOrElse(s"Delete ${form.fileName}"),
-        commit      = form.commit
-      )
+    commitFile(
+      repository  = repository,
+      branch      = form.branch,
+      path        = form.path,
+      newFileName = None,
+      oldFileName = Some(form.fileName),
+      content     = "",
+      charset     = "",
+      message     = form.message.getOrElse(s"Delete ${form.fileName}"),
+      commit      = form.commit
+    )

-      redirect(s"/${repository.owner}/${repository.name}/tree/${form.branch}${if(form.path.length == 0) "" else form.path}")
-    } catch {
-      case e: Exception =>
-        flash += "error" -> e.getMessage
-        redirect(s"/${repository.owner}/${repository.name}/blob/${form.branch}/${
-          if (form.path.length == 0) urlEncode(form.fileName) else s"${form.path}/${urlEncode(form.fileName)}"
-        }")
-    }
+    redirect(s"/${repository.owner}/${repository.name}/tree/${form.branch}${if(form.path.length == 0) "" else form.path}")
  })

  get("/:owner/:repository/raw/*")(referrersOnly { repository =>
@@ -376,8 +362,7 @@ trait RepositoryViewerControllerBase extends ControllerBase {
            latestCommit       = new JGitUtil.CommitInfo(JGitUtil.getLastModifiedCommit(git, revCommit, path)),
            hasWritePermission = hasDeveloperRole(repository.owner, repository.name, context.loginAccount),
            isBlame            = request.paths(2) == "blame",
-            isLfsFile          = isLfsFile(git, objectId),
-            error              = flash.get("error")
+            isLfsFile          = isLfsFile(git, objectId)
          )
        }
      } getOrElse NotFound()
@@ -683,9 +668,7 @@ trait RepositoryViewerControllerBase extends ControllerBase {
    val oldPath = oldFileName.map { oldFileName => if(path.length == 0) oldFileName else s"${path}/${oldFileName}" }

    _commitFile(repository, branch, message){ case (git, headTip, builder, inserter) =>
-      if(headTip.getName != commit){
-        throw new RuntimeException("Commit was rejected. Maybe another user pushed new commits before you.")
-      } else {
+      if(headTip.getName == commit){
        val permission = JGitUtil.processTree(git, headTip) { (path, tree) =>
          // Add all entries except the editing file
          if (!newPath.contains(path) && !oldPath.contains(path)) {
@@ -833,6 +816,26 @@ trait RepositoryViewerControllerBase extends ControllerBase {
  private def isEditable(owner: String, repository: String, author: String)(implicit context: Context): Boolean =
    hasDeveloperRole(owner, repository, context.loginAccount) || author == context.loginAccount.get.userName

+  private def conflict: Constraint = new Constraint(){
+    override def validate(name: String, value: String, messages: Messages): Option[String] = {
+      val owner      = params("owner")
+      val repository = params("repository")
+      val branch     = params("branch")
+
+      LockUtil.lock(s"${owner}/${repository}") {
+        using(Git.open(getRepositoryDir(owner, repository))) { git =>
+          val headName = s"refs/heads/${branch}"
+          val headTip = git.getRepository.resolve(headName)
+          if(headTip.getName != value){
+            Some("Someone pushed new commits before you. Please reload this page and re-apply your changes.")
+          } else {
+            None
+          }
+        }
+      }
+    }
+  }
+
  override protected def renderUncaughtException(e: Throwable)(implicit request: HttpServletRequest, response: HttpServletResponse): Unit = {
    e.printStackTrace()
  }
--- a/src/main/twirl/gitbucket/core/repo/blob.scala.html
+++ b/src/main/twirl/gitbucket/core/repo/blob.scala.html
@@ -5,11 +5,10 @@
  latestCommit: gitbucket.core.util.JGitUtil.CommitInfo,
  hasWritePermission: Boolean,
  isBlame: Boolean,
-  isLfsFile: Boolean,
-  error: Option[Any] = None)(implicit context: gitbucket.core.controller.Context)
+  isLfsFile: Boolean)(implicit context: gitbucket.core.controller.Context)
@import gitbucket.core.view.helpers
@gitbucket.core.html.main(s"${(repository.name :: pathList).mkString("/")} at ${helpers.encodeRefName(branch)} - ${repository.owner}/${repository.name}", Some(repository)) {
-  @gitbucket.core.html.menu("files", repository, None, error){
+  @gitbucket.core.html.menu("files", repository){
    <div class="head">
      <div class="pull-right hide-if-blame"><div class="btn-group">
        <a href="@helpers.url(repository)/find/@helpers.encodeRefName(branch)" class="btn btn-sm btn-default" data-hotkey="t">Find file</a>
--- a/src/main/twirl/gitbucket/core/repo/delete.scala.html
+++ b/src/main/twirl/gitbucket/core/repo/delete.scala.html
@@ -34,6 +34,7 @@
          </td>
        </tr>
      </table>
+      <span class="error" id="error-commit"></span>
      <div class="panel panel-default issue-comment-box">
        <div class="panel-body">
          <div>
--- a/src/main/twirl/gitbucket/core/repo/editor.scala.html
+++ b/src/main/twirl/gitbucket/core/repo/editor.scala.html
@@ -47,6 +47,7 @@
          </td>
        </tr>
      </table>
+      <span class="error" id="error-commit"></span>
      <div class="panel panel-default issue-comment-box">
        <div class="panel-body">
          <div>