Merge pull request #1407 from gitbucket/bugfix-clone-sql-timeout

(refs #1406)Don't begin database session in TransactionFilter for git…

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

@@ -70,42 +70,47 @@ class GitAuthenticationFilter extends Filter with RepositoryService with Account
 
   private def defaultRepository(request: HttpServletRequest, response: HttpServletResponse, chain: FilterChain,
                                 settings: SystemSettings, isUpdating: Boolean): Unit = {
-    implicit val r = request
-
-    request.paths match {
+    val action = request.paths match {
       case Array(_, repositoryOwner, repositoryName, _*) =>
-        getRepository(repositoryOwner, repositoryName.replaceFirst("\\.wiki\\.git$|\\.git$", "")) match {
-          case Some(repository) => {
-            if(!isUpdating && !repository.repository.isPrivate && settings.allowAnonymousAccess){
-              chain.doFilter(request, response)
-            } else {
-              val passed = for {
-                auth <- Option(request.getHeader("Authorization"))
-                Array(username, password) = AuthUtil.decodeAuthHeader(auth).split(":", 2)
-                account <- authenticate(settings, username, password)
-              } yield if(isUpdating || repository.repository.isPrivate){
-                  if(hasDeveloperRole(repository.owner, repository.name, Some(account))){
+        Database() withSession { implicit session =>
+          getRepository(repositoryOwner, repositoryName.replaceFirst("\\.wiki\\.git$|\\.git$", "")) match {
+            case Some(repository) => {
+              val execute = if (!isUpdating && !repository.repository.isPrivate && settings.allowAnonymousAccess) {
+                // Authentication is not required
+                true
+              } else {
+                // Authentication is required
+                val passed = for {
+                  auth <- Option(request.getHeader("Authorization"))
+                  Array(username, password) = AuthUtil.decodeAuthHeader(auth).split(":", 2)
+                  account <- authenticate(settings, username, password)
+                } yield if (isUpdating || repository.repository.isPrivate) {
+                  if (hasDeveloperRole(repository.owner, repository.name, Some(account))) {
                     request.setAttribute(Keys.Request.UserName, account.userName)
                     true
                   } else false
                 } else true
+                passed.getOrElse(false)
+              }
 
-              if(passed.getOrElse(false)){
-                chain.doFilter(request, response)
+              if (execute) {
+                () => chain.doFilter(request, response)
               } else {
-                AuthUtil.requireAuth(response)
+                () => AuthUtil.requireAuth(response)
               }
             }
-          }
-          case None => {
-            logger.debug(s"Repository ${repositoryOwner}/${repositoryName} is not found.")
-            response.sendError(HttpServletResponse.SC_NOT_FOUND)
+            case None => () => {
+              logger.debug(s"Repository ${repositoryOwner}/${repositoryName} is not found.")
+              response.sendError(HttpServletResponse.SC_NOT_FOUND)
+            }
           }
         }
-      case _ => {
+      case _ => () => {
         logger.debug(s"Not enough path arguments: ${request.paths}")
         response.sendError(HttpServletResponse.SC_NOT_FOUND)
       }
     }
+
+    action()
   }
 }

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

@@ -167,7 +167,7 @@ class GitBucketReceivePackFactory extends ReceivePackFactory[HttpServletRequest]
 
 import scala.collection.JavaConverters._
 
-class CommitLogHook(owner: String, repository: String, pusher: String, baseUrl: String)(implicit session: Session)
+class CommitLogHook(owner: String, repository: String, pusher: String, baseUrl: String)/*(implicit session: Session)*/
   extends PostReceiveHook with PreReceiveHook
   with RepositoryService with AccountService with IssuesService with ActivityService with PullRequestService with WebHookService
   with WebHookPullRequestService with CommitsService {
@@ -176,117 +176,121 @@ class CommitLogHook(owner: String, repository: String, pusher: String, baseUrl:
   private var existIds: Seq[String] = Nil
 
   def onPreReceive(receivePack: ReceivePack, commands: java.util.Collection[ReceiveCommand]): Unit = {
-    try {
-      commands.asScala.foreach { command =>
-        // call pre-commit hook
-        PluginRegistry().getReceiveHooks
-          .flatMap(_.preReceive(owner, repository, receivePack, command, pusher))
-          .headOption.foreach { error =>
-          command.setResult(ReceiveCommand.Result.REJECTED_OTHER_REASON, error)
+    Database() withTransaction { implicit session =>
+      try {
+        commands.asScala.foreach { command =>
+          // call pre-commit hook
+          PluginRegistry().getReceiveHooks
+            .flatMap(_.preReceive(owner, repository, receivePack, command, pusher))
+            .headOption.foreach { error =>
+            command.setResult(ReceiveCommand.Result.REJECTED_OTHER_REASON, error)
+          }
+        }
+        using(Git.open(Directory.getRepositoryDir(owner, repository))) { git =>
+          existIds = JGitUtil.getAllCommitIds(git)
+        }
+      } catch {
+        case ex: Exception => {
+          logger.error(ex.toString, ex)
+          throw ex
         }
-      }
-      using(Git.open(Directory.getRepositoryDir(owner, repository))) { git =>
-        existIds = JGitUtil.getAllCommitIds(git)
-      }
-    } catch {
-      case ex: Exception => {
-        logger.error(ex.toString, ex)
-        throw ex
       }
     }
   }
 
   def onPostReceive(receivePack: ReceivePack, commands: java.util.Collection[ReceiveCommand]): Unit = {
-    try {
-      using(Git.open(Directory.getRepositoryDir(owner, repository))) { git =>
-        JGitUtil.removeCache(git)
+    Database() withTransaction { implicit session =>
+      try {
+        using(Git.open(Directory.getRepositoryDir(owner, repository))) { git =>
+          JGitUtil.removeCache(git)
 
-        val pushedIds = scala.collection.mutable.Set[String]()
-        commands.asScala.foreach { command =>
-          logger.debug(s"commandType: ${command.getType}, refName: ${command.getRefName}")
-          implicit val apiContext = api.JsonFormat.Context(baseUrl)
-          val refName = command.getRefName.split("/")
-          val branchName = refName.drop(2).mkString("/")
-          val commits = if (refName(1) == "tags") {
-            Nil
-          } else {
-            command.getType match {
-              case ReceiveCommand.Type.DELETE => Nil
-              case _ => JGitUtil.getCommitLog(git, command.getOldId.name, command.getNewId.name)
-            }
-          }
-
-          // Retrieve all issue count in the repository
-          val issueCount =
-            countIssue(IssueSearchCondition(state = "open"), false, owner -> repository) +
-            countIssue(IssueSearchCondition(state = "closed"), false, owner -> repository)
-
-          val repositoryInfo = getRepository(owner, repository).get
-
-          // Extract new commit and apply issue comment
-          val defaultBranch = repositoryInfo.repository.defaultBranch
-          val newCommits = commits.flatMap { commit =>
-            if (!existIds.contains(commit.id) && !pushedIds.contains(commit.id)) {
-              if (issueCount > 0) {
-                pushedIds.add(commit.id)
-                createIssueComment(owner, repository, commit)
-                // close issues
-                if(refName(1) == "heads" && branchName == defaultBranch && command.getType == ReceiveCommand.Type.UPDATE){
-                  closeIssuesFromMessage(commit.fullMessage, pusher, owner, repository)
-                }
+          val pushedIds = scala.collection.mutable.Set[String]()
+          commands.asScala.foreach { command =>
+            logger.debug(s"commandType: ${command.getType}, refName: ${command.getRefName}")
+            implicit val apiContext = api.JsonFormat.Context(baseUrl)
+            val refName = command.getRefName.split("/")
+            val branchName = refName.drop(2).mkString("/")
+            val commits = if (refName(1) == "tags") {
+              Nil
+            } else {
+              command.getType match {
+                case ReceiveCommand.Type.DELETE => Nil
+                case _ => JGitUtil.getCommitLog(git, command.getOldId.name, command.getNewId.name)
               }
-              Some(commit)
-            } else None
-          }
-
-          // record activity
-          if(refName(1) == "heads"){
-            command.getType match {
-              case ReceiveCommand.Type.CREATE => recordCreateBranchActivity(owner, repository, pusher, branchName)
-              case ReceiveCommand.Type.UPDATE => recordPushActivity(owner, repository, pusher, branchName, newCommits)
-              case ReceiveCommand.Type.DELETE => recordDeleteBranchActivity(owner, repository, pusher, branchName)
-              case _ =>
             }
-          } else if(refName(1) == "tags"){
-            command.getType match {
-              case ReceiveCommand.Type.CREATE => recordCreateTagActivity(owner, repository, pusher, branchName, newCommits)
-              case ReceiveCommand.Type.DELETE => recordDeleteTagActivity(owner, repository, pusher, branchName, newCommits)
-              case _ =>
-            }
-          }
 
-          if(refName(1) == "heads"){
-            command.getType match {
-              case ReceiveCommand.Type.CREATE |
-                   ReceiveCommand.Type.UPDATE |
-                   ReceiveCommand.Type.UPDATE_NONFASTFORWARD =>
-                updatePullRequests(owner, repository, branchName)
-                getAccountByUserName(pusher).map{ pusherAccount =>
-                  callPullRequestWebHookByRequestBranch("synchronize", repositoryInfo, branchName, baseUrl, pusherAccount)
+            // Retrieve all issue count in the repository
+            val issueCount =
+              countIssue(IssueSearchCondition(state = "open"), false, owner -> repository) +
+                countIssue(IssueSearchCondition(state = "closed"), false, owner -> repository)
+
+            val repositoryInfo = getRepository(owner, repository).get
+
+            // Extract new commit and apply issue comment
+            val defaultBranch = repositoryInfo.repository.defaultBranch
+            val newCommits = commits.flatMap { commit =>
+              if (!existIds.contains(commit.id) && !pushedIds.contains(commit.id)) {
+                if (issueCount > 0) {
+                  pushedIds.add(commit.id)
+                  createIssueComment(owner, repository, commit)
+                  // close issues
+                  if (refName(1) == "heads" && branchName == defaultBranch && command.getType == ReceiveCommand.Type.UPDATE) {
+                    closeIssuesFromMessage(commit.fullMessage, pusher, owner, repository)
+                  }
                 }
-              case _ =>
+                Some(commit)
+              } else None
             }
-          }
 
-          // call web hook
-          callWebHookOf(owner, repository, WebHook.Push){
-            for(pusherAccount <- getAccountByUserName(pusher);
-              ownerAccount   <- getAccountByUserName(owner)) yield {
-              WebHookPushPayload(git, pusherAccount, command.getRefName, repositoryInfo, newCommits, ownerAccount,
-                                 newId = command.getNewId(), oldId = command.getOldId())
+            // record activity
+            if (refName(1) == "heads") {
+              command.getType match {
+                case ReceiveCommand.Type.CREATE => recordCreateBranchActivity(owner, repository, pusher, branchName)
+                case ReceiveCommand.Type.UPDATE => recordPushActivity(owner, repository, pusher, branchName, newCommits)
+                case ReceiveCommand.Type.DELETE => recordDeleteBranchActivity(owner, repository, pusher, branchName)
+                case _ =>
+              }
+            } else if (refName(1) == "tags") {
+              command.getType match {
+                case ReceiveCommand.Type.CREATE => recordCreateTagActivity(owner, repository, pusher, branchName, newCommits)
+                case ReceiveCommand.Type.DELETE => recordDeleteTagActivity(owner, repository, pusher, branchName, newCommits)
+                case _ =>
+              }
             }
-          }
 
-          // call post-commit hook
-          PluginRegistry().getReceiveHooks.foreach(_.postReceive(owner, repository, receivePack, command, pusher))
+            if (refName(1) == "heads") {
+              command.getType match {
+                case ReceiveCommand.Type.CREATE |
+                     ReceiveCommand.Type.UPDATE |
+                     ReceiveCommand.Type.UPDATE_NONFASTFORWARD =>
+                  updatePullRequests(owner, repository, branchName)
+                  getAccountByUserName(pusher).map { pusherAccount =>
+                    callPullRequestWebHookByRequestBranch("synchronize", repositoryInfo, branchName, baseUrl, pusherAccount)
+                  }
+                case _ =>
+              }
+            }
+
+            // call web hook
+            callWebHookOf(owner, repository, WebHook.Push) {
+              for (pusherAccount <- getAccountByUserName(pusher);
+                   ownerAccount <- getAccountByUserName(owner)) yield {
+                WebHookPushPayload(git, pusherAccount, command.getRefName, repositoryInfo, newCommits, ownerAccount,
+                  newId = command.getNewId(), oldId = command.getOldId())
+              }
+            }
+
+            // call post-commit hook
+            PluginRegistry().getReceiveHooks.foreach(_.postReceive(owner, repository, receivePack, command, pusher))
+          }
+        }
+        // update repository last modified time.
+        updateLastActivityDate(owner, repository)
+      } catch {
+        case ex: Exception => {
+          logger.error(ex.toString, ex)
+          throw ex
         }
-      }
-      // update repository last modified time.
-      updateLastActivityDate(owner, repository)
-    } catch {
-      case ex: Exception => {
-        logger.error(ex.toString, ex)
-        throw ex
       }
     }
   }

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

@@ -22,7 +22,7 @@ class TransactionFilter extends Filter {
 
   def doFilter(req: ServletRequest, res: ServletResponse, chain: FilterChain): Unit = {
     val servletPath = req.asInstanceOf[HttpServletRequest].getServletPath()
-    if(servletPath.startsWith("/assets/") || servletPath.startsWith("/git-lfs")){
+    if(servletPath.startsWith("/assets/")  || servletPath == "/git" || servletPath == "/git-lfs"){
       // assets and git-lfs don't need transaction
       chain.doFilter(req, res)
     } else {

src/main/scala/gitbucket/core/ssh/GitCommand.scala

@@ -30,24 +30,22 @@ abstract class GitCommand extends Command with SessionAware {
   @volatile protected var callback: ExitCallback = null
   @volatile private var authUser:Option[String] = None
 
-  protected def runTask(authUser: String)(implicit session: Session): Unit
+  protected def runTask(authUser: String): Unit
 
   private def newTask(): Runnable = new Runnable {
     override def run(): Unit = {
       authUser match {
         case Some(authUser) =>
-          Database() withTransaction { implicit session =>
-            try {
-              runTask(authUser)
-              callback.onExit(0)
-            } catch {
-              case e: RepositoryNotFoundException =>
-                logger.info(e.getMessage)
-                callback.onExit(1, "Repository Not Found")
-              case e: Throwable =>
-                logger.error(e.getMessage, e)
-                callback.onExit(1)
-            }
+          try {
+            runTask(authUser)
+            callback.onExit(0)
+          } catch {
+            case e: RepositoryNotFoundException =>
+              logger.info(e.getMessage)
+              callback.onExit(1, "Repository Not Found")
+            case e: Throwable =>
+              logger.error(e.getMessage, e)
+              callback.onExit(1)
           }
         case None =>
           val message = "User not authenticated"
@@ -102,14 +100,18 @@ abstract class DefaultGitCommand(val owner: String, val repoName: String) extend
 class DefaultGitUploadPack(owner: String, repoName: String) extends DefaultGitCommand(owner, repoName)
     with RepositoryService with AccountService {
 
-  override protected def runTask(user: String)(implicit session: Session): Unit = {
-    getRepository(owner, repoName.replaceFirst("\\.wiki\\Z", "")).foreach { repositoryInfo =>
-      if(!repositoryInfo.repository.isPrivate || isWritableUser(user, repositoryInfo)){
-        using(Git.open(getRepositoryDir(owner, repoName))) { git =>
-          val repository = git.getRepository
-          val upload = new UploadPack(repository)
-          upload.upload(in, out, err)
-        }
+  override protected def runTask(user: String): Unit = {
+    val execute = Database() withSession { implicit session =>
+      getRepository(owner, repoName.replaceFirst("\\.wiki\\Z", "")).map { repositoryInfo =>
+        !repositoryInfo.repository.isPrivate || isWritableUser(user, repositoryInfo)
+      }.getOrElse(false)
+    }
+
+    if(execute){
+      using(Git.open(getRepositoryDir(owner, repoName))) { git =>
+        val repository = git.getRepository
+        val upload = new UploadPack(repository)
+        upload.upload(in, out, err)
       }
     }
   }
@@ -118,19 +120,23 @@ class DefaultGitUploadPack(owner: String, repoName: String) extends DefaultGitCo
 class DefaultGitReceivePack(owner: String, repoName: String, baseUrl: String) extends DefaultGitCommand(owner, repoName)
     with RepositoryService with AccountService {
 
-  override protected def runTask(user: String)(implicit session: Session): Unit = {
-    getRepository(owner, repoName.replaceFirst("\\.wiki\\Z", "")).foreach { repositoryInfo =>
-      if(isWritableUser(user, repositoryInfo)){
-        using(Git.open(getRepositoryDir(owner, repoName))) { git =>
-          val repository = git.getRepository
-          val receive = new ReceivePack(repository)
-          if(!repoName.endsWith(".wiki")){
-            val hook = new CommitLogHook(owner, repoName, user, baseUrl)
-            receive.setPreReceiveHook(hook)
-            receive.setPostReceiveHook(hook)
-          }
-          receive.receive(in, out, err)
+  override protected def runTask(user: String): Unit = {
+    val execute = Database() withSession { implicit session =>
+      getRepository(owner, repoName.replaceFirst("\\.wiki\\Z", "")).map { repositoryInfo =>
+        isWritableUser(user, repositoryInfo)
+      }.getOrElse(false)
+    }
+
+    if(execute) {
+      using(Git.open(getRepositoryDir(owner, repoName))) { git =>
+        val repository = git.getRepository
+        val receive = new ReceivePack(repository)
+        if (!repoName.endsWith(".wiki")) {
+          val hook = new CommitLogHook(owner, repoName, user, baseUrl)
+          receive.setPreReceiveHook(hook)
+          receive.setPostReceiveHook(hook)
         }
+        receive.receive(in, out, err)
       }
     }
   }
@@ -139,8 +145,12 @@ class DefaultGitReceivePack(owner: String, repoName: String, baseUrl: String) ex
 class PluginGitUploadPack(repoName: String, routing: GitRepositoryRouting) extends GitCommand
     with SystemSettingsService {
 
-  override protected def runTask(user: String)(implicit session: Session): Unit = {
-    if(routing.filter.filter("/" + repoName, Some(user), loadSystemSettings(), false)){
+  override protected def runTask(user: String): Unit = {
+    val execute = Database() withSession { implicit session =>
+      routing.filter.filter("/" + repoName, Some(user), loadSystemSettings(), false)
+    }
+
+    if(execute){
       val path = routing.urlPattern.r.replaceFirstIn(repoName, routing.localPath)
       using(Git.open(new File(Directory.GitBucketHome, path))){ git =>
         val repository = git.getRepository
@@ -154,8 +164,11 @@ class PluginGitUploadPack(repoName: String, routing: GitRepositoryRouting) exten
 class PluginGitReceivePack(repoName: String, routing: GitRepositoryRouting) extends GitCommand
     with SystemSettingsService {
 
-  override protected def runTask(user: String)(implicit session: Session): Unit = {
-    if(routing.filter.filter("/" + repoName, Some(user), loadSystemSettings(), true)){
+  override protected def runTask(user: String): Unit = {
+    val execute = Database() withSession { implicit session =>
+      routing.filter.filter("/" + repoName, Some(user), loadSystemSettings(), true)
+    }
+    if(execute){
       val path = routing.urlPattern.r.replaceFirstIn(repoName, routing.localPath)
       using(Git.open(new File(Directory.GitBucketHome, path))){ git =>
         val repository = git.getRepository