diff --git a/src/main/scala/app/PullRequestsController.scala b/src/main/scala/app/PullRequestsController.scala index 46bae76..1b3932c 100644 --- a/src/main/scala/app/PullRequestsController.scala +++ b/src/main/scala/app/PullRequestsController.scala @@ -208,9 +208,9 @@ (forkedRepository.repository.originUserName, forkedRepository.repository.originRepositoryName) match { case (Some(originUserName), Some(originRepositoryName)) => { getRepository(originUserName, originRepositoryName, baseUrl).map { originRepository => - withGit( - getRepositoryDir(originUserName, originRepositoryName), - getRepositoryDir(forkedRepository.owner, forkedRepository.name) + using( + Git.open(getRepositoryDir(originUserName, originRepositoryName)), + Git.open(getRepositoryDir(forkedRepository.owner, forkedRepository.name)) ){ (oldGit, newGit) => val oldBranch = JGitUtil.getDefaultBranch(oldGit, originRepository).get._2 val newBranch = JGitUtil.getDefaultBranch(newGit, forkedRepository).get._2 @@ -236,9 +236,9 @@ (getRepository(originOwner, repository.name, baseUrl), getRepository(forkedOwner, repository.name, baseUrl)) match { case (Some(originRepository), Some(forkedRepository)) => { - withGit( - getRepositoryDir(originOwner, repository.name), - getRepositoryDir(forkedOwner, repository.name) + using( + Git.open(getRepositoryDir(originOwner, repository.name)), + Git.open(getRepositoryDir(forkedOwner, repository.name)) ){ case (oldGit, newGit) => val originBranch = JGitUtil.getDefaultBranch(oldGit, originRepository, tmpOriginBranch).get._2 val forkedBranch = JGitUtil.getDefaultBranch(newGit, forkedRepository, tmpForkedBranch).get._2 @@ -319,20 +319,6 @@ }) /** - * Handles w Git object simultaneously. - */ - private def withGit[T](oldDir: java.io.File, newDir: java.io.File)(action: (Git, Git) => T): T = { - val oldGit = Git.open(oldDir) - val newGit = Git.open(newDir) - try { - action(oldGit, newGit) - } finally { - oldGit.getRepository.close - newGit.getRepository.close - } - } - - /** * Parses branch identifier and extracts owner and branch name as tuple. * * - "owner:branch" to ("owner", "branch") @@ -365,9 +351,9 @@ private def getRequestCompareInfo(userName: String, repositoryName: String, branch: String, requestUserName: String, requestRepositoryName: String, requestCommitId: String): (Seq[Seq[CommitInfo]], Seq[DiffInfo]) = { - withGit( - getRepositoryDir(userName, repositoryName), - getRepositoryDir(requestUserName, requestRepositoryName) + using( + Git.open(getRepositoryDir(userName, repositoryName)), + Git.open(getRepositoryDir(requestUserName, requestRepositoryName)) ){ (oldGit, newGit) => val oldId = oldGit.getRepository.resolve(branch) val newId = newGit.getRepository.resolve(requestCommitId) @@ -384,31 +370,30 @@ } } - private def searchPullRequests(userName: Option[String], repository: RepositoryService.RepositoryInfo) = { - val owner = repository.owner - val repoName = repository.name - val filterUser = userName.map { x => Map("created_by" -> x) } getOrElse Map("all" -> "") - val page = IssueSearchCondition.page(request) - val sessionKey = s"${owner}/${repoName}/pulls" + private def searchPullRequests(userName: Option[String], repository: RepositoryService.RepositoryInfo) = + defining(repository.owner, repository.name){ case (owner, repoName) => + val filterUser = userName.map { x => Map("created_by" -> x) } getOrElse Map("all" -> "") + val page = IssueSearchCondition.page(request) + val sessionKey = s"${owner}/${repoName}/pulls" - // retrieve search condition - val condition = if(request.getQueryString == null){ - session.get(sessionKey).getOrElse(IssueSearchCondition()).asInstanceOf[IssueSearchCondition] - } else IssueSearchCondition(request) + // retrieve search condition + val condition = if(request.getQueryString == null){ + session.get(sessionKey).getOrElse(IssueSearchCondition()).asInstanceOf[IssueSearchCondition] + } else IssueSearchCondition(request) - session.put(sessionKey, condition) + session.put(sessionKey, condition) - pulls.html.list( - searchIssue(condition, filterUser, true, (page - 1) * PullRequestLimit, PullRequestLimit, owner -> repoName), - getPullRequestCountGroupByUser(condition.state == "closed", owner, Some(repoName)), - userName, - page, - countIssue(condition.copy(state = "open"), filterUser, true, owner -> repoName), - countIssue(condition.copy(state = "closed"), filterUser, true, owner -> repoName), - countIssue(condition, Map.empty, true, owner -> repoName), - condition, - repository, - hasWritePermission(owner, repoName, context.loginAccount)) - } + pulls.html.list( + searchIssue(condition, filterUser, true, (page - 1) * PullRequestLimit, PullRequestLimit, owner -> repoName), + getPullRequestCountGroupByUser(condition.state == "closed", owner, Some(repoName)), + userName, + page, + countIssue(condition.copy(state = "open"), filterUser, true, owner -> repoName), + countIssue(condition.copy(state = "closed"), filterUser, true, owner -> repoName), + countIssue(condition, Map.empty, true, owner -> repoName), + condition, + repository, + hasWritePermission(owner, repoName, context.loginAccount)) + } } diff --git a/src/main/scala/app/RepositorySettingsController.scala b/src/main/scala/app/RepositorySettingsController.scala index bc4cf86..5da9ffb 100644 --- a/src/main/scala/app/RepositorySettingsController.scala +++ b/src/main/scala/app/RepositorySettingsController.scala @@ -180,25 +180,25 @@ * Provides duplication check for web hook url. */ private def webHook: Constraint = new Constraint(){ - override def validate(name: String, value: String): Option[String] = { - val paths = request.getRequestURI.split("/") - getWebHookURLs(paths(1), paths(2)).map(_.url).find(_ == value).map(_ => "URL had been registered already.") - } + override def validate(name: String, value: String): Option[String] = + defining(request.getRequestURI.split("/")){ paths => + getWebHookURLs(paths(1), paths(2)).map(_.url).find(_ == value).map(_ => "URL had been registered already.") + } } /** * Provides Constraint to validate the collaborator name. */ private def collaborator: Constraint = new Constraint(){ - override def validate(name: String, value: String): Option[String] = { - val paths = request.getRequestURI.split("/") - getAccountByUserName(value) match { - case None => Some("User does not exist.") - case Some(x) if(x.userName == paths(1) || getCollaborators(paths(1), paths(2)).contains(x.userName)) - => Some("User can access this repository already.") - case _ => None + override def validate(name: String, value: String): Option[String] = + defining(request.getRequestURI.split("/")){ paths => + getAccountByUserName(value) match { + case None => Some("User does not exist.") + case Some(x) if(x.userName == paths(1) || getCollaborators(paths(1), paths(2)).contains(x.userName)) + => Some("User can access this repository already.") + case _ => None + } } - } } } \ No newline at end of file diff --git a/src/main/scala/app/RepositoryViewerController.scala b/src/main/scala/app/RepositoryViewerController.scala index adb9d04..947eb7e 100644 --- a/src/main/scala/app/RepositoryViewerController.scala +++ b/src/main/scala/app/RepositoryViewerController.scala @@ -85,13 +85,10 @@ case true => getPathObjectId(path, walk) } - val treeWalk = new TreeWalk(git.getRepository) - val objectId = try { + val objectId = using(new TreeWalk(git.getRepository)){ treeWalk => treeWalk.addTree(revCommit.getTree) treeWalk.setRecursive(true) getPathObjectId(path, treeWalk) - } finally { - treeWalk.release } if(raw){ @@ -129,13 +126,13 @@ val id = params("id") using(Git.open(getRepositoryDir(repository.owner, repository.name))){ git => - val revCommit = JGitUtil.getRevCommitFromId(git, git.getRepository.resolve(id)) - - JGitUtil.getDiffs(git, id) match { case (diffs, oldCommitId) => - repo.html.commit(id, new JGitUtil.CommitInfo(revCommit), - JGitUtil.getBranchesOfCommit(git, revCommit.getName), - JGitUtil.getTagsOfCommit(git, revCommit.getName), - repository, diffs, oldCommitId) + defining(JGitUtil.getRevCommitFromId(git, git.getRepository.resolve(id))){ revCommit => + JGitUtil.getDiffs(git, id) match { case (diffs, oldCommitId) => + repo.html.commit(id, new JGitUtil.CommitInfo(revCommit), + JGitUtil.getBranchesOfCommit(git, revCommit.getName), + JGitUtil.getTagsOfCommit(git, revCommit.getName), + repository, diffs, oldCommitId) + } } } }) @@ -238,19 +235,19 @@ val revisions = Seq(if(revstr.isEmpty) repository.repository.defaultBranch else revstr, repository.branchList.head) // get specified commit JGitUtil.getDefaultBranch(git, repository, revstr).map { case (objectId, revision) => - val revCommit = JGitUtil.getRevCommitFromId(git, objectId) - + defining(JGitUtil.getRevCommitFromId(git, objectId)){ revCommit => // get files - val files = JGitUtil.getFileList(git, revision, path) - // process README.md - val readme = files.find(_.name == "README.md").map { file => - StringUtil.convertFromByteArray(JGitUtil.getContent(Git.open(getRepositoryDir(repository.owner, repository.name)), file.id, true).get) - } + val files = JGitUtil.getFileList(git, revision, path) + // process README.md + val readme = files.find(_.name == "README.md").map { file => + StringUtil.convertFromByteArray(JGitUtil.getContent(Git.open(getRepositoryDir(repository.owner, repository.name)), file.id, true).get) + } - repo.html.files(revision, repository, - if(path == ".") Nil else path.split("/").toList, // current path - new JGitUtil.CommitInfo(revCommit), // latest commit - files, readme) + repo.html.files(revision, repository, + if(path == ".") Nil else path.split("/").toList, // current path + new JGitUtil.CommitInfo(revCommit), // latest commit + files, readme) + } } getOrElse NotFound } } diff --git a/src/main/scala/util/ControlUtil.scala b/src/main/scala/util/ControlUtil.scala index 97fd571..c7b4310 100644 --- a/src/main/scala/util/ControlUtil.scala +++ b/src/main/scala/util/ControlUtil.scala @@ -1,7 +1,8 @@ package util import org.eclipse.jgit.api.Git -import org.eclipse.jgit.revwalk.DepthWalk.RevWalk +import org.eclipse.jgit.revwalk.RevWalk +import org.eclipse.jgit.treewalk.TreeWalk /** * Provides control facilities. @@ -11,9 +12,7 @@ def defining[A, B](value: A)(f: A => B): B = f(value) def using[A <% { def close(): Unit }, B](resource: A)(f: A => B): B = - try { - f(resource) - } finally { + try f(resource) finally { if(resource != null){ try { resource.close() @@ -24,31 +23,26 @@ } def using[T](git: Git)(f: Git => T): T = - try { - f(git) - } finally { - git.getRepository.close + try f(git) finally git.getRepository.close + + def using[T](git1: Git, git2: Git)(f: (Git, Git) => T): T = + try f(git1, git2) finally { + git1.getRepository.close + git2.getRepository.close } def using[T](revWalk: RevWalk)(f: RevWalk => T): T = - try { - f(revWalk) - } finally { - revWalk.release() - } + try f(revWalk) finally revWalk.release() + + def using[T](treeWalk: TreeWalk)(f: TreeWalk => T): T = + try f(treeWalk) finally treeWalk.release() def executeIf(condition: => Boolean)(action: => Unit): Boolean = if(condition){ action true - } else { - false - } + } else false def optionIf[T](condition: => Boolean)(action: => Option[T]): Option[T] = - if(condition){ - action - } else { - None - } + if(condition) action else None } diff --git a/src/main/scala/util/JGitUtil.scala b/src/main/scala/util/JGitUtil.scala index 3912803..3f1a39e 100644 --- a/src/main/scala/util/JGitUtil.scala +++ b/src/main/scala/util/JGitUtil.scala @@ -159,45 +159,43 @@ * @return HTML of the file list */ def getFileList(git: Git, revision: String, path: String = "."): List[FileInfo] = { - val revWalk = new RevWalk(git.getRepository) - val objectId = git.getRepository.resolve(revision) - val revCommit = revWalk.parseCommit(objectId) - - val treeWalk = new TreeWalk(git.getRepository) - treeWalk.addTree(revCommit.getTree) - if(path != "."){ - treeWalk.setRecursive(true) - treeWalk.setFilter(new TreeFilter(){ - - var stopRecursive = false - - def include(walker: TreeWalk): Boolean = { - val targetPath = walker.getPathString - if((path + "/").startsWith(targetPath)){ - true - } else if(targetPath.startsWith(path + "/") && targetPath.substring(path.length + 1).indexOf("/") < 0){ - stopRecursive = true - treeWalk.setRecursive(false) - true - } else { - false - } - } - - def shouldBeRecursive(): Boolean = !stopRecursive - - override def clone: TreeFilter = return this - }) - } - val list = new scala.collection.mutable.ListBuffer[(ObjectId, FileMode, String, String)] - - while (treeWalk.next()) { - list.append((treeWalk.getObjectId(0), treeWalk.getFileMode(0), treeWalk.getPathString, treeWalk.getNameString)) + + using(new RevWalk(git.getRepository)){ revWalk => + val objectId = git.getRepository.resolve(revision) + val revCommit = revWalk.parseCommit(objectId) + + using(new TreeWalk(git.getRepository)){ treeWalk => + treeWalk.addTree(revCommit.getTree) + if(path != "."){ + treeWalk.setRecursive(true) + treeWalk.setFilter(new TreeFilter(){ + + var stopRecursive = false + + def include(walker: TreeWalk): Boolean = { + val targetPath = walker.getPathString + if((path + "/").startsWith(targetPath)){ + true + } else if(targetPath.startsWith(path + "/") && targetPath.substring(path.length + 1).indexOf("/") < 0){ + stopRecursive = true + treeWalk.setRecursive(false) + true + } else { + false + } + } + + def shouldBeRecursive(): Boolean = !stopRecursive + + override def clone: TreeFilter = return this + }) + } + while (treeWalk.next()) { + list.append((treeWalk.getObjectId(0), treeWalk.getFileMode(0), treeWalk.getPathString, treeWalk.getNameString)) + } + } } - - treeWalk.release - revWalk.dispose val commits = getLatestCommitFromPaths(git, list.toList.map(_._3), revision) list.map { case (objectId, fileMode, path, name) => @@ -367,19 +365,19 @@ } else { // initial commit - val treeWalk = new TreeWalk(git.getRepository) - treeWalk.addTree(revCommit.getTree) - val buffer = new scala.collection.mutable.ListBuffer[DiffInfo]() - while(treeWalk.next){ - buffer.append((if(!fetchContent){ - DiffInfo(ChangeType.ADD, null, treeWalk.getPathString, None, None) - } else { - DiffInfo(ChangeType.ADD, null, treeWalk.getPathString, None, - JGitUtil.getContent(git, treeWalk.getObjectId(0), false).filter(FileUtil.isText).map(convertFromByteArray)) - })) + using(new TreeWalk(git.getRepository)){ treeWalk => + treeWalk.addTree(revCommit.getTree) + val buffer = new scala.collection.mutable.ListBuffer[DiffInfo]() + while(treeWalk.next){ + buffer.append((if(!fetchContent){ + DiffInfo(ChangeType.ADD, null, treeWalk.getPathString, None, None) + } else { + DiffInfo(ChangeType.ADD, null, treeWalk.getPathString, None, + JGitUtil.getContent(git, treeWalk.getObjectId(0), false).filter(FileUtil.isText).map(convertFromByteArray)) + })) + } + (buffer.toList, None) } - treeWalk.release - (buffer.toList, None) } } }