Merge pull request #2236 from kounoike/fix-2235

fix #2235

src/main/scala/gitbucket/core/controller/DashboardController.scala

@@ -9,6 +9,7 @@ import gitbucket.core.service.IssuesService._
 class DashboardController
     extends DashboardControllerBase
     with IssuesService
+    with MergeService
     with PullRequestService
     with RepositoryService
     with AccountService

src/main/scala/gitbucket/core/controller/IssuesController.scala

@@ -24,6 +24,7 @@ class IssuesController
     with ReadableUsersAuthenticator
     with ReferrerAuthenticator
     with WritableUsersAuthenticator
+    with MergeService
     with PullRequestService
     with WebHookIssueCommentService
     with WebHookPullRequestReviewCommentService

src/main/scala/gitbucket/core/controller/PullRequestsController.scala

@@ -534,15 +534,15 @@ trait PullRequestsControllerBase extends ControllerBase {
         )
 
         createPullRequest(
-          originUserName = repository.owner,
-          originRepositoryName = repository.name,
+          originRepository = repository,
           issueId = issueId,
           originBranch = form.targetBranch,
           requestUserName = form.requestUserName,
           requestRepositoryName = form.requestRepositoryName,
           requestBranch = form.requestBranch,
           commitIdFrom = form.commitIdFrom,
-          commitIdTo = form.commitIdTo
+          commitIdTo = form.commitIdTo,
+          loginAccount = context.loginAccount.get
         )
 
         // insert labels
@@ -557,29 +557,6 @@ trait PullRequestsControllerBase extends ControllerBase {
           }
         }
 
-        // fetch requested branch
-        fetchAsPullRequest(owner, name, form.requestUserName, form.requestRepositoryName, form.requestBranch, issueId)
-
-        // record activity
-        recordPullRequestActivity(owner, name, loginUserName, issueId, form.title)
-
-        // call web hook
-        callPullRequestWebHook("opened", repository, issueId, context.loginAccount.get)
-
-        getIssue(owner, name, issueId.toString) foreach { issue =>
-          // extract references and create refer comment
-          createReferComment(
-            owner,
-            name,
-            issue,
-            form.title + " " + form.content.getOrElse(""),
-            context.loginAccount.get
-          )
-
-          // call hooks
-          PluginRegistry().getPullRequestHooks.foreach(_.created(issue, repository))
-        }
-
         redirect(s"/${owner}/${name}/pull/${issueId}")
     }
   })

src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala

@@ -50,6 +50,7 @@ class RepositoryViewerController
     with ReadableUsersAuthenticator
     with ReferrerAuthenticator
     with WritableUsersAuthenticator
+    with MergeService
     with PullRequestService
     with CommitStatusService
     with WebHookPullRequestService

src/main/scala/gitbucket/core/controller/api/ApiPullRequestControllerBase.scala

@@ -106,15 +106,15 @@ trait ApiPullRequestControllerBase extends ControllerBase {
                     )
 
                     createPullRequest(
-                      originUserName = repository.owner,
-                      originRepositoryName = repository.name,
+                      originRepository = repository,
                       issueId = issueId,
                       originBranch = createPullReq.base,
                       requestUserName = reqOwner,
                       requestRepositoryName = repository.name,
                       requestBranch = reqBranch,
                       commitIdFrom = commitIdFrom.getName,
-                      commitIdTo = commitIdTo.getName
+                      commitIdTo = commitIdTo.getName,
+                      loginAccount = context.loginAccount.get
                     )
                     getApiPullRequest(repository, issueId).map(JsonFormat(_))
                   case _ =>
@@ -133,15 +133,15 @@ trait ApiPullRequestControllerBase extends ControllerBase {
                   case (Some(commitIdFrom), Some(commitIdTo)) =>
                     changeIssueToPullRequest(repository.owner, repository.name, createPullReqAlt.issue)
                     createPullRequest(
-                      originUserName = repository.owner,
-                      originRepositoryName = repository.name,
+                      originRepository = repository,
                       issueId = createPullReqAlt.issue,
                       originBranch = createPullReqAlt.base,
                       requestUserName = reqOwner,
                       requestRepositoryName = repository.name,
                       requestBranch = reqBranch,
                       commitIdFrom = commitIdFrom.getName,
-                      commitIdTo = commitIdTo.getName
+                      commitIdTo = commitIdTo.getName,
+                      loginAccount = context.loginAccount.get
                     )
                     getApiPullRequest(repository, createPullReqAlt.issue).map(JsonFormat(_))
                   case _ =>

src/main/scala/gitbucket/core/service/PullRequestService.scala

@@ -6,6 +6,8 @@ import gitbucket.core.model.Profile.profile.blockingApi._
 import difflib.{Delta, DiffUtils}
 import gitbucket.core.service.RepositoryService.RepositoryInfo
 import gitbucket.core.api.JsonFormat
+import gitbucket.core.controller.Context
+import gitbucket.core.plugin.PluginRegistry
 import gitbucket.core.util.SyntaxSugars._
 import gitbucket.core.util.Directory._
 import gitbucket.core.util.Implicits._
@@ -20,7 +22,13 @@ import org.eclipse.jgit.lib.ObjectId
 import scala.collection.JavaConverters._
 
 trait PullRequestService {
-  self: IssuesService with CommitsService with WebHookService with WebHookPullRequestService with RepositoryService =>
+  self: IssuesService
+    with CommitsService
+    with WebHookService
+    with WebHookPullRequestService
+    with RepositoryService
+    with MergeService
+    with ActivityService =>
   import PullRequestService._
 
   def getPullRequest(owner: String, repository: String, issueId: Int)(
@@ -81,27 +89,66 @@ trait PullRequestService {
 //      .map { x => PullRequestCount(x._1, x._2) }
 
   def createPullRequest(
-    originUserName: String,
-    originRepositoryName: String,
+    originRepository: RepositoryInfo,
     issueId: Int,
     originBranch: String,
     requestUserName: String,
     requestRepositoryName: String,
     requestBranch: String,
     commitIdFrom: String,
-    commitIdTo: String
-  )(implicit s: Session): Unit =
-    PullRequests insert PullRequest(
-      originUserName,
-      originRepositoryName,
-      issueId,
-      originBranch,
-      requestUserName,
-      requestRepositoryName,
-      requestBranch,
-      commitIdFrom,
-      commitIdTo
-    )
+    commitIdTo: String,
+    loginAccount: Account
+  )(implicit s: Session, context: Context): Unit = {
+    getIssue(originRepository.owner, originRepository.name, issueId.toString).foreach { baseIssue =>
+      PullRequests insert PullRequest(
+        originRepository.owner,
+        originRepository.name,
+        issueId,
+        originBranch,
+        requestUserName,
+        requestRepositoryName,
+        requestBranch,
+        commitIdFrom,
+        commitIdTo
+      )
+
+      // fetch requested branch
+      fetchAsPullRequest(
+        originRepository.owner,
+        originRepository.name,
+        requestUserName,
+        requestRepositoryName,
+        requestBranch,
+        issueId
+      )
+
+      // record activity
+      recordPullRequestActivity(
+        originRepository.owner,
+        originRepository.name,
+        loginAccount.userName,
+        issueId,
+        baseIssue.title
+      )
+
+      // call web hook
+      callPullRequestWebHook("opened", originRepository, issueId, loginAccount)
+
+      getIssue(originRepository.owner, originRepository.name, issueId.toString) foreach { issue =>
+        // extract references and create refer comment
+        createReferComment(
+          originRepository.owner,
+          originRepository.name,
+          issue,
+          baseIssue.title + " " + baseIssue.content,
+          loginAccount
+        )
+
+        // call hooks
+        PluginRegistry().getPullRequestHooks.foreach(_.created(issue, originRepository))
+      }
+    }
+  }
 
   def getPullRequestsByRequest(userName: String, repositoryName: String, branch: String, closed: Option[Boolean])(
     implicit s: Session

src/main/scala/gitbucket/core/servlet/GitRepositoryServlet.scala

@@ -215,6 +215,7 @@ class CommitLogHook(owner: String, repository: String, pusher: String, baseUrl:
     with AccountService
     with IssuesService
     with ActivityService
+    with MergeService
     with PullRequestService
     with WebHookService
     with LabelsService

src/test/scala/gitbucket/core/service/IssuesServiceSpec.scala

@@ -33,7 +33,7 @@ class IssuesServiceSpec extends FunSuite with ServiceSpecBase {
 
       assert(getCommitStatues(1) == None)
 
-      val (is2, pr2) = generateNewPullRequest("user1/repo1/master", "user1/repo1/feature1")
+      val (is2, pr2) = generateNewPullRequest("user1/repo1/master", "user1/repo1/feature1", loginUser = "root")
       assert(pr2.issueId == 2)
 
       // if there are no statuses, state is none
@@ -79,7 +79,7 @@ class IssuesServiceSpec extends FunSuite with ServiceSpecBase {
       assert(getCommitStatues(2) == Some(CommitStatusInfo(2, 1, None, None, None, None)))
 
       // get only statuses in query issues
-      val (is3, pr3) = generateNewPullRequest("user1/repo1/master", "user1/repo1/feature3")
+      val (is3, pr3) = generateNewPullRequest("user1/repo1/master", "user1/repo1/feature3", loginUser = "root")
       val cs4 = dummyService.createCommitStatus(
         "user1",
         "repo1",

src/test/scala/gitbucket/core/service/PullRequestServiceSpec.scala

@@ -6,6 +6,7 @@ import org.scalatest.FunSpec
 class PullRequestServiceSpec
     extends FunSpec
     with ServiceSpecBase
+    with MergeService
     with PullRequestService
     with IssuesService
     with AccountService
@@ -30,13 +31,13 @@ class PullRequestServiceSpec
         generateNewUserWithDBRepository("user1", "repo1")
         generateNewUserWithDBRepository("user1", "repo2")
         generateNewUserWithDBRepository("user2", "repo1")
-        generateNewPullRequest("user1/repo1/master", "user1/repo1/head2") // not target branch
-        generateNewPullRequest("user1/repo1/head1", "user1/repo1/master") // not target branch ( swap from, to )
-        generateNewPullRequest("user1/repo1/master", "user2/repo1/head1") // other user
-        generateNewPullRequest("user1/repo1/master", "user1/repo2/head1") // other repository
-        val r1 = swap(generateNewPullRequest("user1/repo1/master2", "user1/repo1/head1"))
-        val r2 = swap(generateNewPullRequest("user1/repo1/master", "user1/repo1/head1"))
-        val r3 = swap(generateNewPullRequest("user1/repo1/master4", "user1/repo1/head1"))
+        generateNewPullRequest("user1/repo1/master", "user1/repo1/head2", loginUser = "root") // not target branch
+        generateNewPullRequest("user1/repo1/head1", "user1/repo1/master", loginUser = "root") // not target branch ( swap from, to )
+        generateNewPullRequest("user1/repo1/master", "user2/repo1/head1", loginUser = "root") // other user
+        generateNewPullRequest("user1/repo1/master", "user1/repo2/head1", loginUser = "root") // other repository
+        val r1 = swap(generateNewPullRequest("user1/repo1/master2", "user1/repo1/head1", loginUser = "root"))
+        val r2 = swap(generateNewPullRequest("user1/repo1/master", "user1/repo1/head1", loginUser = "root"))
+        val r3 = swap(generateNewPullRequest("user1/repo1/master4", "user1/repo1/head1", loginUser = "root"))
         assert(getPullRequestFromBranch("user1", "repo1", "head1", "master") == Some(r2))
         updateClosed("user1", "repo1", r2._1.issueId, true)
         assert(Seq(r1, r2).contains(getPullRequestFromBranch("user1", "repo1", "head1", "master").get))

src/test/scala/gitbucket/core/service/ServiceSpecBase.scala

@@ -1,7 +1,7 @@
 package gitbucket.core.service
 
 import gitbucket.core.GitBucketCoreModule
-import gitbucket.core.util.{DatabaseConfig, FileUtil}
+import gitbucket.core.util.{DatabaseConfig, Directory, FileUtil, JGitUtil}
 import gitbucket.core.util.SyntaxSugars._
 import io.github.gitbucket.solidbase.Solidbase
 import liquibase.database.core.H2Database
@@ -10,15 +10,53 @@ import gitbucket.core.model._
 import gitbucket.core.model.Profile._
 import gitbucket.core.model.Profile.profile._
 import gitbucket.core.model.Profile.profile.blockingApi._
-
 import org.apache.commons.io.FileUtils
-
 import java.sql.DriverManager
 import java.io.File
 
+import gitbucket.core.controller.Context
+import gitbucket.core.service.SystemSettingsService.{Ssh, SystemSettings}
+import javax.servlet.http.{HttpServletRequest, HttpSession}
+import org.scalatest.mockito.MockitoSugar
+import org.mockito.Mockito._
+
 import scala.util.Random
 
-trait ServiceSpecBase {
+trait ServiceSpecBase extends MockitoSugar {
+
+  val request = mock[HttpServletRequest]
+  val session = mock[HttpSession]
+  when(request.getRequestURL).thenReturn(new StringBuffer("http://localhost:8080/path.html"))
+  when(request.getRequestURI).thenReturn("/path.html")
+  when(request.getContextPath).thenReturn("")
+  when(request.getSession).thenReturn(session)
+
+  private def createSystemSettings() =
+    SystemSettings(
+      baseUrl = None,
+      information = None,
+      allowAccountRegistration = false,
+      allowAnonymousAccess = true,
+      isCreateRepoOptionPublic = true,
+      gravatar = false,
+      notification = false,
+      activityLogLimit = None,
+      ssh = Ssh(
+        enabled = false,
+        sshHost = None,
+        sshPort = None
+      ),
+      useSMTP = false,
+      smtp = None,
+      ldapAuthentication = false,
+      ldap = None,
+      oidcAuthentication = false,
+      oidc = None,
+      skinName = "skin-blue",
+      showMailAddress = false,
+      pluginNetworkInstall = false,
+      pluginProxy = None
+    )
 
   def withTestDB[A](action: (Session) => A): A = {
     FileUtil.withTmpDir(new File(FileUtils.getTempDirectory(), Random.alphanumeric.take(10).mkString)) { dir =>
@@ -44,12 +82,26 @@ trait ServiceSpecBase {
   def user(name: String)(implicit s: Session): Account = AccountService.getAccountByUserName(name).get
 
   lazy val dummyService = new RepositoryService with AccountService with ActivityService with IssuesService
-  with PullRequestService with CommitsService with CommitStatusService with LabelsService with MilestonesService
-  with PrioritiesService with WebHookService with WebHookPullRequestService
-  with WebHookPullRequestReviewCommentService {}
+  with MergeService with PullRequestService with CommitsService with CommitStatusService with LabelsService
+  with MilestonesService with PrioritiesService with WebHookService with WebHookPullRequestService
+  with WebHookPullRequestReviewCommentService {
+    override def fetchAsPullRequest(
+      userName: String,
+      repositoryName: String,
+      requestUserName: String,
+      requestRepositoryName: String,
+      requestBranch: String,
+      issueId: Int
+    ): Unit = {}
+  }
 
   def generateNewUserWithDBRepository(userName: String, repositoryName: String)(implicit s: Session): Account = {
     val ac = AccountService.getAccountByUserName(userName).getOrElse(generateNewAccount(userName))
+    val dir = Directory.getRepositoryDir(userName, repositoryName)
+    if (dir.exists()) {
+      FileUtils.deleteQuietly(dir)
+    }
+    JGitUtil.initRepository(dir)
     dummyService.insertRepository(repositoryName, userName, None, false)
     ac
   }
@@ -70,22 +122,25 @@ trait ServiceSpecBase {
     )
   }
 
-  def generateNewPullRequest(base: String, request: String, loginUser: String = null)(
+  def generateNewPullRequest(base: String, request: String, loginUser: String)(
     implicit s: Session
   ): (Issue, PullRequest) = {
+    implicit val context = Context(createSystemSettings(), None, this.request)
     val Array(baseUserName, baseRepositoryName, baesBranch) = base.split("/")
     val Array(requestUserName, requestRepositoryName, requestBranch) = request.split("/")
     val issueId = generateNewIssue(baseUserName, baseRepositoryName, Option(loginUser).getOrElse(requestUserName))
+    val baseRepository = dummyService.getRepository(baseUserName, baseRepositoryName)
+    val loginAccount = dummyService.getAccountByUserName(loginUser)
     dummyService.createPullRequest(
-      originUserName = baseUserName,
-      originRepositoryName = baseRepositoryName,
+      originRepository = baseRepository.get,
       issueId = issueId,
       originBranch = baesBranch,
       requestUserName = requestUserName,
       requestRepositoryName = requestRepositoryName,
       requestBranch = requestBranch,
       commitIdFrom = baesBranch,
-      commitIdTo = requestBranch
+      commitIdTo = requestBranch,
+      loginAccount = loginAccount.get
     )
     dummyService.getPullRequest(baseUserName, baseRepositoryName, issueId).get
   }

src/test/scala/gitbucket/core/service/WebHookServiceSpec.scala

@@ -6,8 +6,8 @@ import gitbucket.core.model.WebHookContentType
 
 class WebHookServiceSpec extends FunSuite with ServiceSpecBase {
   lazy val service = new WebHookPullRequestService with AccountService with ActivityService with RepositoryService
-  with PullRequestService with IssuesService with CommitsService with LabelsService with MilestonesService
-  with PrioritiesService with WebHookPullRequestReviewCommentService
+  with MergeService with PullRequestService with IssuesService with CommitsService with LabelsService
+  with MilestonesService with PrioritiesService with WebHookPullRequestReviewCommentService
 
   test("WebHookPullRequestService.getPullRequestsByRequestForWebhook") {
     withTestDB { implicit session =>
@@ -19,7 +19,7 @@ class WebHookServiceSpec extends FunSuite with ServiceSpecBase {
       val (issue3, pullreq3) = generateNewPullRequest("user3/repo3/master3", "user2/repo2/master2", loginUser = "root")
       val (issue32, pullreq32) =
         generateNewPullRequest("user3/repo3/master32", "user2/repo2/master2", loginUser = "root")
-      generateNewPullRequest("user2/repo2/master2", "user1/repo1/master2")
+      generateNewPullRequest("user2/repo2/master2", "user1/repo1/master2", loginUser = "root")
       service.addWebHook("user1", "repo1", "webhook1-1", Set(WebHook.PullRequest), WebHookContentType.FORM, Some("key"))
       service.addWebHook("user1", "repo1", "webhook1-2", Set(WebHook.PullRequest), WebHookContentType.FORM, Some("key"))
       service.addWebHook("user2", "repo2", "webhook2-1", Set(WebHook.PullRequest), WebHookContentType.FORM, Some("key"))