diff --git a/src/main/scala/app/ControllerBase.scala b/src/main/scala/app/ControllerBase.scala index 11c0d8b..31eac0e 100644 --- a/src/main/scala/app/ControllerBase.scala +++ b/src/main/scala/app/ControllerBase.scala @@ -1,6 +1,8 @@ package app import _root_.util.Directory._ +import _root_.util.Implicits._ +import _root_.util.ControlUtil._ import _root_.util.{FileUtil, Validations} import org.scalatra._ import org.scalatra.json._ @@ -57,56 +59,51 @@ */ implicit def context: Context = Context(servletContext.getContextPath, LoginAccount, currentURL, request) - private def currentURL: String = { - val queryString = request.getQueryString + private def currentURL: String = defining(request.getQueryString){ queryString => request.getRequestURI + (if(queryString != null) "?" + queryString else "") } - private def LoginAccount: Option[Account] = { + private def LoginAccount: Option[Account] = session.get("LOGIN_ACCOUNT") match { case Some(x: Account) => Some(x) case _ => None } - } - def ajaxGet(path : String)(action : => Any) : Route = { + def ajaxGet(path : String)(action : => Any) : Route = super.get(path){ request.setAttribute("AJAX", "true") action } - } - override def ajaxGet[T](path : String, form : MappingValueType[T])(action : T => Any) : Route = { + override def ajaxGet[T](path : String, form : MappingValueType[T])(action : T => Any) : Route = super.ajaxGet(path, form){ form => request.setAttribute("AJAX", "true") action(form) } - } - def ajaxPost(path : String)(action : => Any) : Route = { + def ajaxPost(path : String)(action : => Any) : Route = super.post(path){ request.setAttribute("AJAX", "true") action } - } - override def ajaxPost[T](path : String, form : MappingValueType[T])(action : T => Any) : Route = { + override def ajaxPost[T](path : String, form : MappingValueType[T])(action : T => Any) : Route = super.ajaxPost(path, form){ form => request.setAttribute("AJAX", "true") action(form) } - } - protected def NotFound() = { - if(request.getAttribute("AJAX") == null){ - org.scalatra.NotFound(html.error("Not Found")) - } else { + protected def NotFound() = + if(request.hasAttribute("AJAX")){ org.scalatra.NotFound() + } else { + org.scalatra.NotFound(html.error("Not Found")) } - } - protected def Unauthorized()(implicit context: app.Context) = { - if(request.getAttribute("AJAX") == null){ + protected def Unauthorized()(implicit context: app.Context) = + if(request.hasAttribute("AJAX")){ + org.scalatra.Unauthorized() + } else { if(context.loginAccount.isDefined){ org.scalatra.Unauthorized(redirect("/")) } else { @@ -116,13 +113,9 @@ org.scalatra.Unauthorized(redirect("/signin?redirect=" + currentURL)) } } - } else { - org.scalatra.Unauthorized() } - } - protected def baseUrl = { - val url = request.getRequestURL.toString + protected def baseUrl = defining(request.getRequestURL.toString){ url => url.substring(0, url.length - (request.getRequestURI.length - request.getContextPath.length)) } @@ -133,12 +126,10 @@ */ case class Context(path: String, loginAccount: Option[Account], currentUrl: String, request: HttpServletRequest){ - def redirectUrl = { - if(request.getParameter("redirect") != null){ - request.getParameter("redirect") - } else { - currentUrl - } + def redirectUrl = if(request.getParameter("redirect") != null){ + request.getParameter("redirect") + } else { + currentUrl } /** @@ -147,13 +138,12 @@ * If object has not been cached with the specified key then retrieves by given action. * Cached object are available during a request. */ - def cache[A](key: String)(action: => A): A = { + def cache[A](key: String)(action: => A): A = Option(request.getAttribute("cache." + key).asInstanceOf[A]).getOrElse { val newObject = action request.setAttribute("cache." + key, newObject) newObject } - } } @@ -163,7 +153,7 @@ trait AccountManagementControllerBase extends ControllerBase with FileUploadControllerBase { self: AccountService => - protected def updateImage(userName: String, fileId: Option[String], clearImage: Boolean): Unit = { + protected def updateImage(userName: String, fileId: Option[String], clearImage: Boolean): Unit = if(clearImage){ getAccountByUserName(userName).flatMap(_.image).map { image => new java.io.File(getUserUploadDir(userName), image).delete() @@ -179,7 +169,6 @@ updateAvatarImage(userName, Some(filename)) } } - } protected def uniqueUserName: Constraint = new Constraint(){ override def validate(name: String, value: String): Option[String] = @@ -212,15 +201,14 @@ // def removeTemporaryFile(fileId: String)(implicit session: HttpSession): Unit = // getTemporaryFile(fileId).delete() - def removeTemporaryFiles()(implicit session: HttpSession): Unit = - FileUtils.deleteDirectory(TemporaryDir) + def removeTemporaryFiles()(implicit session: HttpSession): Unit = FileUtils.deleteDirectory(TemporaryDir) - def getUploadedFilename(fileId: String)(implicit session: HttpSession): Option[String] = { - val filename = Option(session.getAttribute("upload_" + fileId).asInstanceOf[String]) - if(filename.isDefined){ - session.removeAttribute("upload_" + fileId) + def getUploadedFilename(fileId: String)(implicit session: HttpSession): Option[String] = + defining(Option(session.getAttribute("upload_" + fileId).asInstanceOf[String])){ filename => + if(filename.isDefined){ + session.removeAttribute("upload_" + fileId) + } + filename } - filename - } } \ No newline at end of file diff --git a/src/main/scala/app/DashboardController.scala b/src/main/scala/app/DashboardController.scala index dbda3cf..b0509dd 100644 --- a/src/main/scala/app/DashboardController.scala +++ b/src/main/scala/app/DashboardController.scala @@ -2,6 +2,7 @@ import service._ import util.UsersAuthenticator +import util.Implicits._ class DashboardController extends DashboardControllerBase with IssuesService with PullRequestService with RepositoryService with AccountService @@ -43,11 +44,10 @@ // condition val sessionKey = "dashboard/issues" - val condition = if(request.getQueryString == null) - session.get(sessionKey).getOrElse(IssueSearchCondition()).asInstanceOf[IssueSearchCondition] - else IssueSearchCondition(request) - - session.put(sessionKey, condition) + val condition = session.putAndGet(sessionKey, + if(request.hasQueryString) IssueSearchCondition(request) + else session.get(sessionKey).getOrElse(IssueSearchCondition()).asInstanceOf[IssueSearchCondition] + ) val userName = context.loginAccount.get.userName val repositories = getUserRepositories(userName, baseUrl).map(repo => repo.owner -> repo.name) @@ -76,14 +76,10 @@ // condition val sessionKey = "dashboard/pulls" - val condition = { - if(request.getQueryString == null) - session.get(sessionKey).getOrElse(IssueSearchCondition()).asInstanceOf[IssueSearchCondition] - else - IssueSearchCondition(request) - }.copy(repo = repository) - - session.put(sessionKey, condition) + val condition = session.putAndGet(sessionKey, { + if(request.hasQueryString) IssueSearchCondition(request) + else session.get(sessionKey).getOrElse(IssueSearchCondition()).asInstanceOf[IssueSearchCondition] + }.copy(repo = repository)) val userName = context.loginAccount.get.userName val repositories = getUserRepositories(userName, baseUrl).map(repo => repo.owner -> repo.name) diff --git a/src/main/scala/app/IssuesController.scala b/src/main/scala/app/IssuesController.scala index adee1e8..b4f5376 100644 --- a/src/main/scala/app/IssuesController.scala +++ b/src/main/scala/app/IssuesController.scala @@ -5,6 +5,8 @@ import service._ import IssuesService._ import util.{CollaboratorsAuthenticator, ReferrerAuthenticator, ReadableUsersAuthenticator, Notifier} +import util.Implicits._ +import util.ControlUtil._ import org.scalatra.Ok class IssuesController extends IssuesControllerBase @@ -57,12 +59,9 @@ }) get("/:owner/:repository/issues/:id")(referrersOnly { repository => - val owner = repository.owner - val name = repository.name - val issueId = params("id") - - getIssue(owner, name, issueId) map { - issues.html.issue( + defining(repository.owner, repository.name, params("id")){ case (owner, name, issueId) => + getIssue(owner, name, issueId) map { + issues.html.issue( _, getComments(owner, name, issueId.toInt), getIssueLabels(owner, name, issueId.toInt), @@ -71,65 +70,64 @@ getLabels(owner, name), hasWritePermission(owner, name, context.loginAccount), repository) - } getOrElse NotFound + } getOrElse NotFound + } }) get("/:owner/:repository/issues/new")(readableUsersOnly { repository => - val owner = repository.owner - val name = repository.name - - issues.html.create( - (getCollaborators(owner, name) :+ owner).sorted, - getMilestones(owner, name), - getLabels(owner, name), - hasWritePermission(owner, name, context.loginAccount), - repository) + defining(repository.owner, repository.name){ case (owner, name) => + issues.html.create( + (getCollaborators(owner, name) :+ owner).sorted, + getMilestones(owner, name), + getLabels(owner, name), + hasWritePermission(owner, name, context.loginAccount), + repository) + } }) post("/:owner/:repository/issues/new", issueCreateForm)(readableUsersOnly { (form, repository) => - val owner = repository.owner - val name = repository.name - val writable = hasWritePermission(owner, name, context.loginAccount) - val userName = context.loginAccount.get.userName + defining(repository.owner, repository.name){ case (owner, name) => + val writable = hasWritePermission(owner, name, context.loginAccount) + val userName = context.loginAccount.get.userName - // insert issue - val issueId = createIssue(owner, name, userName, form.title, form.content, - if(writable) form.assignedUserName else None, - if(writable) form.milestoneId else None) + // insert issue + val issueId = createIssue(owner, name, userName, form.title, form.content, + if(writable) form.assignedUserName else None, + if(writable) form.milestoneId else None) - // insert labels - if(writable){ - form.labelNames.map { value => - val labels = getLabels(owner, name) - value.split(",").foreach { labelName => - labels.find(_.labelName == labelName).map { label => - registerIssueLabel(owner, name, issueId, label.labelId) + // insert labels + if(writable){ + form.labelNames.map { value => + val labels = getLabels(owner, name) + value.split(",").foreach { labelName => + labels.find(_.labelName == labelName).map { label => + registerIssueLabel(owner, name, issueId, label.labelId) + } } } } + + // record activity + recordCreateIssueActivity(owner, name, userName, issueId, form.title) + + // notifications + Notifier().toNotify(repository, issueId, form.content.getOrElse("")){ + Notifier.msgIssue(s"${baseUrl}/${owner}/${name}/issues/${issueId}") + } + + redirect(s"/${owner}/${name}/issues/${issueId}") } - - // record activity - recordCreateIssueActivity(owner, name, userName, issueId, form.title) - - // notifications - Notifier().toNotify(repository, issueId, form.content.getOrElse("")){ - Notifier.msgIssue(s"${baseUrl}/${owner}/${name}/issues/${issueId}") - } - - redirect(s"/${owner}/${name}/issues/${issueId}") }) ajaxPost("/:owner/:repository/issues/edit/:id", issueEditForm)(readableUsersOnly { (form, repository) => - val owner = repository.owner - val name = repository.name - - getIssue(owner, name, params("id")).map { issue => - if(isEditable(owner, name, issue.openedUserName)){ - updateIssue(owner, name, issue.issueId, form.title, form.content) - redirect(s"/${owner}/${name}/issues/_data/${issue.issueId}") - } else Unauthorized - } getOrElse NotFound + defining(repository.owner, repository.name){ case (owner, name) => + getIssue(owner, name, params("id")).map { issue => + if(isEditable(owner, name, issue.openedUserName)){ + updateIssue(owner, name, issue.issueId, form.title, form.content) + redirect(s"/${owner}/${name}/issues/_data/${issue.issueId}") + } else Unauthorized + } getOrElse NotFound + } }) post("/:owner/:repository/issue_comments/new", commentForm)(readableUsersOnly { (form, repository) => @@ -147,15 +145,14 @@ }) ajaxPost("/:owner/:repository/issue_comments/edit/:id", commentForm)(readableUsersOnly { (form, repository) => - val owner = repository.owner - val name = repository.name - - getComment(owner, name, params("id")).map { comment => - if(isEditable(owner, name, comment.commentedUserName)){ - updateComment(comment.commentId, form.content) - redirect(s"/${owner}/${name}/issue_comments/_data/${comment.commentId}") - } else Unauthorized - } getOrElse NotFound + defining(repository.owner, repository.name){ case (owner, name) => + getComment(owner, name, params("id")).map { comment => + if(isEditable(owner, name, comment.commentedUserName)){ + updateComment(comment.commentId, form.content) + redirect(s"/${owner}/${name}/issue_comments/_data/${comment.commentId}") + } else Unauthorized + } getOrElse NotFound + } }) ajaxGet("/:owner/:repository/issues/_data/:id")(readableUsersOnly { repository => @@ -194,17 +191,17 @@ }) ajaxPost("/:owner/:repository/issues/:id/label/new")(collaboratorsOnly { repository => - val issueId = params("id").toInt - - registerIssueLabel(repository.owner, repository.name, issueId, params("labelId").toInt) - issues.html.labellist(getIssueLabels(repository.owner, repository.name, issueId)) + defining(params("id").toInt){ issueId => + registerIssueLabel(repository.owner, repository.name, issueId, params("labelId").toInt) + issues.html.labellist(getIssueLabels(repository.owner, repository.name, issueId)) + } }) ajaxPost("/:owner/:repository/issues/:id/label/delete")(collaboratorsOnly { repository => - val issueId = params("id").toInt - - deleteIssueLabel(repository.owner, repository.name, issueId, params("labelId").toInt) - issues.html.labellist(getIssueLabels(repository.owner, repository.name, issueId)) + defining(params("id").toInt){ issueId => + deleteIssueLabel(repository.owner, repository.name, issueId, params("labelId").toInt) + issues.html.labellist(getIssueLabels(repository.owner, repository.name, issueId)) + } }) ajaxPost("/:owner/:repository/issues/:id/assign")(collaboratorsOnly { repository => @@ -223,36 +220,36 @@ }) post("/:owner/:repository/issues/batchedit/state")(collaboratorsOnly { repository => - val action = params.get("value") - - executeBatch(repository) { - handleComment(_, None, repository)( _ => action) + defining(params.get("value")){ action => + executeBatch(repository) { + handleComment(_, None, repository)( _ => action) + } } }) post("/:owner/:repository/issues/batchedit/label")(collaboratorsOnly { repository => - val labelId = params("value").toInt - - executeBatch(repository) { issueId => - getIssueLabel(repository.owner, repository.name, issueId, labelId) getOrElse { - registerIssueLabel(repository.owner, repository.name, issueId, labelId) + defining(params("value").toInt){ labelId => + executeBatch(repository) { issueId => + getIssueLabel(repository.owner, repository.name, issueId, labelId) getOrElse { + registerIssueLabel(repository.owner, repository.name, issueId, labelId) + } } } }) post("/:owner/:repository/issues/batchedit/assign")(collaboratorsOnly { repository => - val value = assignedUserName("value") - - executeBatch(repository) { - updateAssignedUserName(repository.owner, repository.name, _, value) + defining(assignedUserName("value")){ value => + executeBatch(repository) { + updateAssignedUserName(repository.owner, repository.name, _, value) + } } }) post("/:owner/:repository/issues/batchedit/milestone")(collaboratorsOnly { repository => - val value = milestoneId("value") - - executeBatch(repository) { - updateMilestoneId(repository.owner, repository.name, _, value) + defining(milestoneId("value")){ value => + executeBatch(repository) { + updateMilestoneId(repository.owner, repository.name, _, value) + } } }) @@ -273,89 +270,89 @@ private def handleComment(issueId: Int, content: Option[String], repository: RepositoryService.RepositoryInfo) (getAction: model.Issue => Option[String] = p1 => params.get("action").filter(_ => isEditable(p1.userName, p1.repositoryName, p1.openedUserName))) = { - val owner = repository.owner - val name = repository.name - val userName = context.loginAccount.get.userName - getIssue(owner, name, issueId.toString) map { issue => - val (action, recordActivity) = - getAction(issue) - .collect { + defining(repository.owner, repository.name){ case (owner, name) => + val userName = context.loginAccount.get.userName + + getIssue(owner, name, issueId.toString) map { issue => + val (action, recordActivity) = + getAction(issue) + .collect { case "close" => true -> (Some("close") -> - Some(if(issue.isPullRequest) recordClosePullRequestActivity _ else recordCloseIssueActivity _)) + Some(if(issue.isPullRequest) recordClosePullRequestActivity _ else recordCloseIssueActivity _)) case "reopen" => false -> (Some("reopen") -> - Some(recordReopenIssueActivity _)) + Some(recordReopenIssueActivity _)) } - .map { case (closed, t) => + .map { case (closed, t) => updateClosed(owner, name, issueId, closed) t } - .getOrElse(None -> None) + .getOrElse(None -> None) - val commentId = content + val commentId = content .map ( _ -> action.map( _ + "_comment" ).getOrElse("comment") ) .getOrElse ( action.get.capitalize -> action.get ) - match { - case (content, action) => createComment(owner, name, userName, issueId, content, action) - } + match { + case (content, action) => createComment(owner, name, userName, issueId, content, action) + } - // record activity - content foreach { - (if(issue.isPullRequest) recordCommentPullRequestActivity _ else recordCommentIssueActivity _) - (owner, name, userName, issueId, _) - } - recordActivity foreach ( _ (owner, name, userName, issueId, issue.title) ) + // record activity + content foreach { + (if(issue.isPullRequest) recordCommentPullRequestActivity _ else recordCommentIssueActivity _) + (owner, name, userName, issueId, _) + } + recordActivity foreach ( _ (owner, name, userName, issueId, issue.title) ) - // notifications - Notifier() match { - case f => - content foreach { - f.toNotify(repository, issueId, _){ - Notifier.msgComment(s"${baseUrl}/${owner}/${name}/${ - if(issue.isPullRequest) "pull" else "issues"}/${issueId}#comment-${commentId}") + // notifications + Notifier() match { + case f => + content foreach { + f.toNotify(repository, issueId, _){ + Notifier.msgComment(s"${baseUrl}/${owner}/${name}/${ + if(issue.isPullRequest) "pull" else "issues"}/${issueId}#comment-${commentId}") + } } - } - action foreach { - f.toNotify(repository, issueId, _){ - Notifier.msgStatus(s"${baseUrl}/${owner}/${name}/issues/${issueId}") + action foreach { + f.toNotify(repository, issueId, _){ + Notifier.msgStatus(s"${baseUrl}/${owner}/${name}/issues/${issueId}") + } } - } - } + } - issue -> commentId + issue -> commentId + } } } private def searchIssues(filter: String, repository: RepositoryService.RepositoryInfo) = { - val owner = repository.owner - val repoName = repository.name - val filterUser = Map(filter -> params.getOrElse("userName", "")) - val page = IssueSearchCondition.page(request) - val sessionKey = s"${owner}/${repoName}/issues" + defining(repository.owner, repository.name){ case (owner, repoName) => + val filterUser = Map(filter -> params.getOrElse("userName", "")) + val page = IssueSearchCondition.page(request) + val sessionKey = s"${owner}/${repoName}/issues" - // retrieve search condition - val condition = if(request.getQueryString == null){ - session.get(sessionKey).getOrElse(IssueSearchCondition()).asInstanceOf[IssueSearchCondition] - } else IssueSearchCondition(request) + // retrieve search condition + val condition = session.putAndGet(sessionKey, + if(request.hasQueryString) IssueSearchCondition(request) + else session.get(sessionKey).getOrElse(IssueSearchCondition()).asInstanceOf[IssueSearchCondition] + ) - session.put(sessionKey, condition) - - issues.html.list( - searchIssue(condition, filterUser, false, (page - 1) * IssueLimit, IssueLimit, owner -> repoName), - page, - (getCollaborators(owner, repoName) :+ owner).sorted, - getMilestones(owner, repoName), - getLabels(owner, repoName), - countIssue(condition.copy(state = "open"), filterUser, false, owner -> repoName), - countIssue(condition.copy(state = "closed"), filterUser, false, owner -> repoName), - countIssue(condition, Map.empty, false, owner -> repoName), - context.loginAccount.map(x => countIssue(condition, Map("assigned" -> x.userName), false, owner -> repoName)), - context.loginAccount.map(x => countIssue(condition, Map("created_by" -> x.userName), false, owner -> repoName)), - countIssueGroupByLabels(owner, repoName, condition, filterUser), - condition, - filter, - repository, - hasWritePermission(owner, repoName, context.loginAccount)) + issues.html.list( + searchIssue(condition, filterUser, false, (page - 1) * IssueLimit, IssueLimit, owner -> repoName), + page, + (getCollaborators(owner, repoName) :+ owner).sorted, + getMilestones(owner, repoName), + getLabels(owner, repoName), + countIssue(condition.copy(state = "open"), filterUser, false, owner -> repoName), + countIssue(condition.copy(state = "closed"), filterUser, false, owner -> repoName), + countIssue(condition, Map.empty, false, owner -> repoName), + context.loginAccount.map(x => countIssue(condition, Map("assigned" -> x.userName), false, owner -> repoName)), + context.loginAccount.map(x => countIssue(condition, Map("created_by" -> x.userName), false, owner -> repoName)), + countIssueGroupByLabels(owner, repoName, condition, filterUser), + condition, + filter, + repository, + hasWritePermission(owner, repoName, context.loginAccount)) + } } } diff --git a/src/main/scala/app/PullRequestsController.scala b/src/main/scala/app/PullRequestsController.scala index 1b3932c..5d4ec90 100644 --- a/src/main/scala/app/PullRequestsController.scala +++ b/src/main/scala/app/PullRequestsController.scala @@ -63,109 +63,104 @@ }) get("/:owner/:repository/pull/:id")(referrersOnly { repository => - val owner = repository.owner - val name = repository.name - val issueId = params("id").toInt + defining(repository.owner, repository.name, params("id").toInt){ case (owner, name, issueId) => + getPullRequest(owner, name, issueId) map { case(issue, pullreq) => + using(Git.open(getRepositoryDir(owner, name))){ git => + val (commits, diffs) = + getRequestCompareInfo(owner, name, pullreq.commitIdFrom, owner, name, pullreq.commitIdTo) - getPullRequest(owner, name, issueId) map { case(issue, pullreq) => - using(Git.open(getRepositoryDir(owner, name))){ git => - val requestCommitId = git.getRepository.resolve(pullreq.requestBranch) - - val (commits, diffs) = - getRequestCompareInfo(owner, name, pullreq.commitIdFrom, owner, name, pullreq.commitIdTo) - - pulls.html.pullreq( - issue, pullreq, - getComments(owner, name, issueId.toInt), - (getCollaborators(owner, name) :+ owner).sorted, - getMilestonesWithIssueCount(owner, name), - commits, - diffs, - if(issue.closed){ - false - } else { - checkConflict(owner, name, pullreq.branch, owner, name, pullreq.requestBranch) - }, - hasWritePermission(owner, name, context.loginAccount), - repository, - s"${baseUrl}${context.path}/git/${pullreq.requestUserName}/${pullreq.requestRepositoryName}.git") - } - - } getOrElse NotFound + pulls.html.pullreq( + issue, pullreq, + getComments(owner, name, issueId.toInt), + (getCollaborators(owner, name) :+ owner).sorted, + getMilestonesWithIssueCount(owner, name), + commits, + diffs, + if(issue.closed){ + false + } else { + checkConflict(owner, name, pullreq.branch, owner, name, pullreq.requestBranch) + }, + hasWritePermission(owner, name, context.loginAccount), + repository, + s"${baseUrl}${context.path}/git/${pullreq.requestUserName}/${pullreq.requestRepositoryName}.git") + } + } getOrElse NotFound + } }) post("/:owner/:repository/pull/:id/merge", mergeForm)(collaboratorsOnly { (form, repository) => - LockUtil.lock(s"${repository.owner}/${repository.name}/merge"){ - val issueId = params("id").toInt + defining(repository.owner, repository.name, params("id").toInt){ case (owner, name, issueId) => + LockUtil.lock(s"${owner}/${name}/merge"){ + getPullRequest(owner, name, issueId).map { case (issue, pullreq) => + val remote = getRepositoryDir(owner, name) + val tmpdir = new java.io.File(getTemporaryDir(owner, name), s"merge-${issueId}") + val git = Git.cloneRepository.setDirectory(tmpdir).setURI(remote.toURI.toString).setBranch(pullreq.branch).call - getPullRequest(repository.owner, repository.name, issueId).map { case (issue, pullreq) => - val remote = getRepositoryDir(repository.owner, repository.name) - val tmpdir = new java.io.File(getTemporaryDir(repository.owner, repository.name), s"merge-${issueId}") - val git = Git.cloneRepository.setDirectory(tmpdir).setURI(remote.toURI.toString).setBranch(pullreq.branch).call + try { + // mark issue as merged and close. + val loginAccount = context.loginAccount.get + createComment(owner, name, loginAccount.userName, issueId, form.message, "merge") + createComment(owner, name, loginAccount.userName, issueId, "Close", "close") + updateClosed(owner, name, issueId, true) - try { - // mark issue as merged and close. - val loginAccount = context.loginAccount.get - createComment(repository.owner, repository.name, loginAccount.userName, issueId, form.message, "merge") - createComment(repository.owner, repository.name, loginAccount.userName, issueId, "Close", "close") - updateClosed(repository.owner, repository.name, issueId, true) + // record activity + recordMergeActivity(owner, name, loginAccount.userName, issueId, form.message) - // record activity - recordMergeActivity(repository.owner, repository.name, loginAccount.userName, issueId, form.message) + // fetch pull request to temporary working repository + val pullRequestBranchName = s"gitbucket-pullrequest-${issueId}" - // fetch pull request to temporary working repository - val pullRequestBranchName = s"gitbucket-pullrequest-${issueId}" + git.fetch + .setRemote(getRepositoryDir(owner, name).toURI.toString) + .setRefSpecs(new RefSpec(s"refs/pull/${issueId}/head:refs/heads/${pullRequestBranchName}")).call - git.fetch - .setRemote(getRepositoryDir(repository.owner, repository.name).toURI.toString) - .setRefSpecs(new RefSpec(s"refs/pull/${issueId}/head:refs/heads/${pullRequestBranchName}")).call + // merge pull request + git.checkout.setName(pullreq.branch).call - // merge pull request - git.checkout.setName(pullreq.branch).call + val result = git.merge + .include(git.getRepository.resolve(pullRequestBranchName)) + .setFastForward(FastForwardMode.NO_FF) + .setCommit(false) + .call - val result = git.merge - .include(git.getRepository.resolve(pullRequestBranchName)) - .setFastForward(FastForwardMode.NO_FF) - .setCommit(false) - .call - - if(result.getConflicts != null){ - throw new RuntimeException("This pull request can't merge automatically.") - } - - // merge commit - git.getRepository.writeMergeCommitMsg( - s"Merge pull request #${issueId} from ${pullreq.requestUserName}/${pullreq.requestRepositoryName}\n" - + form.message) - - git.commit - .setCommitter(new PersonIdent(loginAccount.userName, loginAccount.mailAddress)) - .call - - // push - git.push.call - - val (commits, _) = getRequestCompareInfo(repository.owner, repository.name, pullreq.commitIdFrom, - pullreq.requestUserName, pullreq.requestRepositoryName, pullreq.commitIdTo) - - commits.flatten.foreach { commit => - if(!existsCommitId(repository.owner, repository.name, commit.id)){ - insertCommitId(repository.owner, repository.name, commit.id) + if(result.getConflicts != null){ + throw new RuntimeException("This pull request can't merge automatically.") } + + // merge commit + git.getRepository.writeMergeCommitMsg( + s"Merge pull request #${issueId} from ${pullreq.requestUserName}/${pullreq.requestRepositoryName}\n" + + form.message) + + git.commit + .setCommitter(new PersonIdent(loginAccount.userName, loginAccount.mailAddress)) + .call + + // push + git.push.call + + val (commits, _) = getRequestCompareInfo(owner, name, pullreq.commitIdFrom, + pullreq.requestUserName, pullreq.requestRepositoryName, pullreq.commitIdTo) + + commits.flatten.foreach { commit => + if(!existsCommitId(owner, name, commit.id)){ + insertCommitId(owner, name, commit.id) + } + } + + // notifications + Notifier().toNotify(repository, issueId, "merge"){ + Notifier.msgStatus(s"${baseUrl}/${owner}/${name}/pull/${issueId}") + } + + redirect(s"/${owner}/${name}/pull/${issueId}") + + } finally { + git.getRepository.close + FileUtils.deleteDirectory(tmpdir) } - - // notifications - Notifier().toNotify(repository, issueId, "merge"){ - Notifier.msgStatus(s"${baseUrl}/${repository.owner}/${repository.name}/pull/${issueId}") - } - - redirect(s"/${repository.owner}/${repository.name}/pull/${issueId}") - - } finally { - git.getRepository.close - FileUtils.deleteDirectory(tmpdir) - } - } getOrElse NotFound + } getOrElse NotFound + } } }) @@ -377,11 +372,10 @@ 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) - - session.put(sessionKey, condition) + val condition = session.putAndGet(sessionKey, + if(request.hasQueryString) IssueSearchCondition(request) + else session.get(sessionKey).getOrElse(IssueSearchCondition()).asInstanceOf[IssueSearchCondition] + ) pulls.html.list( searchIssue(condition, filterUser, true, (page - 1) * PullRequestLimit, PullRequestLimit, owner -> repoName), diff --git a/src/main/scala/app/RepositorySettingsController.scala b/src/main/scala/app/RepositorySettingsController.scala index 4818ecd..b463ed9 100644 --- a/src/main/scala/app/RepositorySettingsController.scala +++ b/src/main/scala/app/RepositorySettingsController.scala @@ -9,7 +9,6 @@ import service.WebHookService.WebHookPayload import util.JGitUtil.CommitInfo import util.ControlUtil._ -import util.Implicits._ import org.eclipse.jgit.api.Git class RepositorySettingsController extends RepositorySettingsControllerBase @@ -123,8 +122,7 @@ * Delete the web hook URL. */ get("/:owner/:repository/settings/hooks/delete")(ownerOnly { repository => - val url = params("url") - deleteWebHookURL(repository.owner, repository.name, url) + deleteWebHookURL(repository.owner, repository.name, params("url")) redirect(s"/${repository.owner}/${repository.name}/settings/hooks") }) @@ -139,24 +137,19 @@ .setMaxCount(3) .call.iterator.asScala.map(new CommitInfo(_)) - val payload = WebHookPayload( - git, - "refs/heads/" + repository.repository.defaultBranch, - repository, - commits.toList, - getAccountByUserName(repository.owner).get) + callWebHook(repository.owner, repository.name, + WebHookPayload( + git, + "refs/heads/" + repository.repository.defaultBranch, + repository, + commits.toList, + getAccountByUserName(repository.owner).get)) - callWebHook(repository.owner, repository.name, payload) flash += "info" -> "Test payload deployed!" } redirect(s"/${repository.owner}/${repository.name}/settings/hooks") }) - // TODO Remove this action after web hook is completed. - post("/xxx/xxx/xxx/webhooktest"){ - println(params("payload")) - } - /** * Display the delete repository page. */ @@ -182,9 +175,7 @@ */ private def webHook: Constraint = new Constraint(){ override def validate(name: String, value: String): Option[String] = - defining(request.paths){ paths => - getWebHookURLs(paths(1), paths(2)).map(_.url).find(_ == value).map(_ => "URL had been registered already.") - } + getWebHookURLs(params("owner"), params("repository")).map(_.url).find(_ == value).map(_ => "URL had been registered already.") } /** @@ -192,13 +183,11 @@ */ private def collaborator: Constraint = new Constraint(){ override def validate(name: String, value: String): Option[String] = - defining(request.paths){ 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 - } + getAccountByUserName(value) match { + case None => Some("User does not exist.") + case Some(x) if(x.userName == params("owner") || getCollaborators(params("owner"), params("repository")).contains(x.userName)) + => Some("User can access this repository already.") + case _ => None } } diff --git a/src/main/scala/app/SignInController.scala b/src/main/scala/app/SignInController.scala index 8e24662..3b56301 100644 --- a/src/main/scala/app/SignInController.scala +++ b/src/main/scala/app/SignInController.scala @@ -23,7 +23,6 @@ } post("/signin", form){ form => - val settings = loadSystemSettings() authenticate(loadSystemSettings(), form.userName, form.password) match { case Some(account) => signin(account) case None => redirect("/signin") diff --git a/src/main/scala/util/Implicits.scala b/src/main/scala/util/Implicits.scala index 603483c..75d34b3 100644 --- a/src/main/scala/util/Implicits.scala +++ b/src/main/scala/util/Implicits.scala @@ -1,7 +1,7 @@ package util import scala.util.matching.Regex -import javax.servlet.http.HttpServletRequest +import javax.servlet.http.{HttpSession, HttpServletRequest} /** * Provides some usable implicit conversions. @@ -44,7 +44,20 @@ } implicit class RichRequest(request: HttpServletRequest){ + def paths: Array[String] = request.getRequestURI.substring(request.getContextPath.length).split("/") + + def hasQueryString: Boolean = request.getQueryString != null + + def hasAttribute(name: String): Boolean = request.getAttribute(name) != null + + } + + implicit class RichSession(session: HttpSession){ + def putAndGet[T](key: String, value: T): T = { + session.setAttribute(key, value) + value + } } } \ No newline at end of file