(refs #1618)Reject online editing if new commits have been pushed already

2026-05-06 18:26:28 +02:00 · 2017-06-29 18:05:35 +09:00
parent 7761946ec0
commit d122c92db1
4 changed files with 142 additions and 75 deletions
--- a/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala
+++ b/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala
@@ -58,14 +58,16 @@ trait RepositoryViewerControllerBase extends ControllerBase {
    charset: String,
    lineSeparator: String,
    newFileName: String,
-    oldFileName: Option[String]
+    oldFileName: Option[String],
+    commit: String
  )

  case class DeleteForm(
    branch: String,
    path: String,
    message: Option[String],
-    fileName: String
+    fileName: String,
+    commit: String
  )

  case class CommentForm(
@@ -91,14 +93,16 @@ trait RepositoryViewerControllerBase extends ControllerBase {
    "charset"       -> trim(label("Charset", text(required))),
    "lineSeparator" -> trim(label("Line Separator", text(required))),
    "newFileName"   -> trim(label("Filename", text(required))),
-    "oldFileName"   -> trim(label("Old filename", optional(text())))
+    "oldFileName"   -> trim(label("Old filename", optional(text()))),
+    "commit"        -> trim(label("Commit", text(required)))
  )(EditorForm.apply)

  val deleteForm = mapping(
    "branch"   -> trim(label("Branch", text(required))),
    "path"     -> trim(label("Path", text())),
    "message"  -> trim(label("Message", optional(text()))),
-    "fileName" -> trim(label("Filename", text(required)))
+    "fileName" -> trim(label("Filename", text(required))),
+    "commit"        -> trim(label("Commit", text(required)))
  )(DeleteForm.apply)

  val commentForm = mapping(
@@ -184,8 +188,20 @@ trait RepositoryViewerControllerBase extends ControllerBase {
  get("/:owner/:repository/new/*")(writableUsersOnly { repository =>
    val (branch, path) = repository.splitPath(multiParams("splat").head)
    val protectedBranch = getProtectedBranchInfo(repository.owner, repository.name, branch).needStatusCheck(context.loginAccount.get.userName)
-    html.editor(branch, repository, if(path.length == 0) Nil else path.split("/").toList,
-      None, JGitUtil.ContentInfo("text", None, None, Some("UTF-8")), protectedBranch)
+
+    using(Git.open(getRepositoryDir(repository.owner, repository.name))) { git =>
+      val revCommit = JGitUtil.getRevCommitFromId(git, git.getRepository.resolve(branch))
+
+      html.editor(
+        branch          = branch,
+        repository      = repository,
+        pathList        = if (path.length == 0) Nil else path.split("/").toList,
+        fileName        = None,
+        content         = JGitUtil.ContentInfo("text", None, None, Some("UTF-8")),
+        protectedBranch = protectedBranch,
+        commit          = revCommit.getName
+      )
+    }
  })

  get("/:owner/:repository/upload/*")(writableUsersOnly { repository =>
@@ -225,9 +241,15 @@ trait RepositoryViewerControllerBase extends ControllerBase {

      getPathObjectId(git, path, revCommit).map { objectId =>
        val paths = path.split("/")
-        html.editor(branch, repository, paths.take(paths.size - 1).toList, Some(paths.last),
-          JGitUtil.getContentInfo(git, path, objectId),
-          protectedBranch)
+        html.editor(
+          branch          = branch,
+          repository      = repository,
+          pathList        = paths.take(paths.size - 1).toList,
+          fileName        = Some(paths.last),
+          content         = JGitUtil.getContentInfo(git, path, objectId),
+          protectedBranch = protectedBranch,
+          commit          = revCommit.getName
+        )
      } getOrElse NotFound()
    }
  })
@@ -239,23 +261,34 @@ trait RepositoryViewerControllerBase extends ControllerBase {

      getPathObjectId(git, path, revCommit).map { objectId =>
        val paths = path.split("/")
-        html.delete(branch, repository, paths.take(paths.size - 1).toList, paths.last,
-          JGitUtil.getContentInfo(git, path, objectId))
+        html.delete(
+          branch     = branch,
+          repository = repository,
+          pathList   = paths.take(paths.size - 1).toList,
+          fileName   = paths.last,
+          content    = JGitUtil.getContentInfo(git, path, objectId),
+          commit     = revCommit.getName
+        )
      } getOrElse NotFound()
    }
  })

  post("/:owner/:repository/create", editorForm)(writableUsersOnly { (form, repository) =>
-    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}")
-    )
+    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
+    }

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

  post("/:owner/:repository/update", editorForm)(writableUsersOnly { (form, repository) =>
-    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}")
-      }
-    )
-
+    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
+    }
    redirect(s"/${repository.owner}/${repository.name}/blob/${form.branch}/${
-      if(form.path.length == 0) urlEncode(form.newFileName) else s"${form.path}/${urlEncode(form.newFileName)}"
+      if (form.path.length == 0) urlEncode(form.newFileName) else s"${form.path}/${urlEncode(form.newFileName)}"
    }")
  })

  post("/:owner/:repository/remove", deleteForm)(writableUsersOnly { (form, repository) =>
-    commitFile(repository, form.branch, form.path, None, Some(form.fileName), "", "",
-      form.message.getOrElse(s"Delete ${form.fileName}"))
+    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
+      )

-    redirect(s"/${repository.owner}/${repository.name}/tree/${form.branch}${if(form.path.length == 0) "" else form.path}")
+      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)}"
+        }")
+    }
  })

  get("/:owner/:repository/raw/*")(referrersOnly { repository =>
@@ -314,12 +368,17 @@ trait RepositoryViewerControllerBase extends ControllerBase {
          // Download (This route is left for backword compatibility)
          responseRawFile(git, objectId, path, repository)
        } else {
-          html.blob(id, repository, path.split("/").toList,
-            JGitUtil.getContentInfo(git, path, objectId),
-            new JGitUtil.CommitInfo(JGitUtil.getLastModifiedCommit(git, revCommit, path)),
-            hasDeveloperRole(repository.owner, repository.name, context.loginAccount),
-            request.paths(2) == "blame",
-            isLfsFile(git, objectId))
+          html.blob(
+            branch             = id,
+            repository         = repository,
+            pathList           = path.split("/").toList,
+            content            = JGitUtil.getContentInfo(git, path, objectId),
+            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")
+          )
        }
      } getOrElse NotFound()
    }
@@ -618,27 +677,31 @@ trait RepositoryViewerControllerBase extends ControllerBase {

  private def commitFile(repository: RepositoryService.RepositoryInfo,
    branch: String, path: String, newFileName: Option[String], oldFileName: Option[String],
-    content: String, charset: String, message: String) = {
+    content: String, charset: String, message: String, commit: String) = {

    val newPath = newFileName.map { newFileName => if(path.length == 0) newFileName else s"${path}/${newFileName}" }
    val oldPath = oldFileName.map { oldFileName => if(path.length == 0) oldFileName else s"${path}/${oldFileName}" }

    _commitFile(repository, branch, message){ case (git, headTip, builder, inserter) =>
-      val permission = JGitUtil.processTree(git, headTip){ (path, tree) =>
-        // Add all entries except the editing file
-        if(!newPath.contains(path) && !oldPath.contains(path)){
-          builder.add(JGitUtil.createDirCacheEntry(path, tree.getEntryFileMode, tree.getEntryObjectId))
-        }
-        // Retrieve permission if file exists to keep it
-        oldPath.collect { case x if x == path => tree.getEntryFileMode.getBits }
-      }.flatten.headOption
+      if(headTip.getName != commit){
+        throw new RuntimeException("Commit was rejected. Maybe another user pushed new commits before you.")
+      } else {
+        val permission = JGitUtil.processTree(git, headTip) { (path, tree) =>
+          // Add all entries except the editing file
+          if (!newPath.contains(path) && !oldPath.contains(path)) {
+            builder.add(JGitUtil.createDirCacheEntry(path, tree.getEntryFileMode, tree.getEntryObjectId))
+          }
+          // Retrieve permission if file exists to keep it
+          oldPath.collect { case x if x == path => tree.getEntryFileMode.getBits }
+        }.flatten.headOption

-      newPath.foreach { newPath =>
-        builder.add(JGitUtil.createDirCacheEntry(newPath,
-          permission.map { bits => FileMode.fromBits(bits) } getOrElse FileMode.REGULAR_FILE,
-          inserter.insert(Constants.OBJ_BLOB, content.getBytes(charset))))
+        newPath.foreach { newPath =>
+          builder.add(JGitUtil.createDirCacheEntry(newPath,
+            permission.map { bits => FileMode.fromBits(bits) } getOrElse FileMode.REGULAR_FILE,
+            inserter.insert(Constants.OBJ_BLOB, content.getBytes(charset))))
+        }
+        builder.finish()
      }
-      builder.finish()
    }
  }

--- a/src/main/twirl/gitbucket/core/repo/blob.scala.html
+++ b/src/main/twirl/gitbucket/core/repo/blob.scala.html
@@ -5,10 +5,11 @@
  latestCommit: gitbucket.core.util.JGitUtil.CommitInfo,
  hasWritePermission: Boolean,
  isBlame: Boolean,
-  isLfsFile: Boolean)(implicit context: gitbucket.core.controller.Context)
+  isLfsFile: Boolean,
+  error: Option[Any] = None)(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){
+  @gitbucket.core.html.menu("files", repository, None, error){
    <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>
@@ -241,4 +242,4 @@ function updateHighlighting(){
    scrolling = true;
  }
 }
-</script>
+</script>
--- a/src/main/twirl/gitbucket/core/repo/delete.scala.html
+++ b/src/main/twirl/gitbucket/core/repo/delete.scala.html
@@ -2,7 +2,8 @@
  repository: gitbucket.core.service.RepositoryService.RepositoryInfo,
  pathList: List[String],
  fileName: String,
-  content: gitbucket.core.util.JGitUtil.ContentInfo)(implicit context: gitbucket.core.controller.Context)
+  content: gitbucket.core.util.JGitUtil.ContentInfo,
+  commit: String)(implicit context: gitbucket.core.controller.Context)
@import gitbucket.core.view.helpers
@gitbucket.core.html.main(s"Deleting ${context.path} at ${fileName} - ${repository.owner}/${repository.name}", Some(repository)) {
  @gitbucket.core.html.menu("files", repository){
@@ -16,6 +17,7 @@
        <input type="hidden" name="fileName" id="fileName" value="@fileName"/>
        <input type="hidden" name="branch" id="branch" value="@branch"/>
        <input type="hidden" name="path" id="path" value="@pathList.mkString("/")"/>
+        <input type="hidden" name="commit" id="commit" value="@commit"/>
      </div>
      <table class="table table-bordered">
        <th style="font-weight: normal;" class="box-header">
@@ -32,18 +34,17 @@
          </td>
        </tr>
      </table>
-      <div class="issue-avatar-image">@helpers.avatarLink(context.loginAccount.get.userName, 48)</div>
-        <div class="box issue-comment-box">
-        <div class="box-content">
+      <div class="panel panel-default issue-comment-box">
+        <div class="panel-body">
          <div>
            <strong>Commit changes</strong>
          </div>
-          <div>
-            <input type="text" name="message" style="width: 98%;"/>
+          <div class="form-group">
+            <input type="text" name="message" class="form-control"/>
          </div>
          <div style="text-align: right;">
            <a href="@helpers.url(repository)/blob/@helpers.encodeRefName(branch)/@pathList.mkString("/")" class="btn btn-danger">Cancel</a>
-            <input type="submit" id="commit" class="btn btn-success" value="Commit changes"/>
+            <input type="submit" id="commitButton" class="btn btn-success" value="Commit changes"/>
          </div>
        </div>
      </div>
--- a/src/main/twirl/gitbucket/core/repo/editor.scala.html
+++ b/src/main/twirl/gitbucket/core/repo/editor.scala.html
@@ -3,7 +3,8 @@
  pathList: List[String],
  fileName: Option[String],
  content: gitbucket.core.util.JGitUtil.ContentInfo,
-  protectedBranch: Boolean)(implicit context: gitbucket.core.controller.Context)
+  protectedBranch: Boolean,
+  commit: String)(implicit context: gitbucket.core.controller.Context)
@import gitbucket.core.view.helpers
@gitbucket.core.html.main(if(fileName.isEmpty) "New File" else s"Editing ${fileName.get} at ${branch} - ${repository.owner}/${repository.name}", Some(repository)) {
  @gitbucket.core.html.menu("files", repository){
@@ -60,11 +61,12 @@
            } else {
              <a href="@helpers.url(repository)/blob/@helpers.encodeRefName(branch)/@{(pathList ++ Seq(fileName.get)).mkString("/")}" class="btn btn-danger">Cancel</a>
            }
-            <input type="submit" id="commit" class="btn btn-success" value="Commit changes" disabled="true"/>
+            <input type="submit" id="commitButton" class="btn btn-success" value="Commit changes" disabled="true"/>
            <input type="hidden" id="charset" name="charset" value="@content.charset"/>
            <input type="hidden" id="lineSeparator" name="lineSeparator" value="@content.lineSeparator"/>
            <input type="hidden" id="content" name="content" value=""/>
            <input type="hidden" id="initial" value="@content.content"/>
+            <input type="hidden" id="commit" name="commit" value="@commit"/>
          </div>
        </div>
      </div>
@@ -94,9 +96,9 @@ $(function(){

  function updateCommitButtonStatus(){
    if(editor.getValue() == $('#initial').val() && $('#newFileName').val() == $('#oldFileName').val()){
-      $('#commit').attr('disabled', true);
+      $('#commitButton').attr('disabled', true);
    } else {
-      $('#commit').attr('disabled', false);
+      $('#commitButton').attr('disabled', false);
    }
  }

@@ -112,7 +114,7 @@ $(function(){
    updateCommitButtonStatus();
  });

-  $('#commit').click(function(){
+  $('#commitButton').click(function(){
    $('#content').val(editor.getValue());
  });