Use ControlUtil.

2025-10-31 18:46:28 +01:00 · 2013-09-22 19:28:14 +09:00
parent 602b6c635a
commit fd8b5780f3
7 changed files with 299 additions and 323 deletions
--- 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 @@ abstract class ControllerBase extends ScalatraFilter
   */
  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 @@ abstract class ControllerBase extends ScalatraFilter
          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 @@ abstract class ControllerBase extends ScalatraFilter
 */
 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 @@ case class Context(path: String, loginAccount: Option[Account], currentUrl: Stri
   * 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 @@ case class Context(path: String, loginAccount: Option[Account], currentUrl: Stri
 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 @@ trait AccountManagementControllerBase extends ControllerBase with FileUploadCont
        updateAvatarImage(userName, Some(filename))
      }
    }
-  }

  protected def uniqueUserName: Constraint = new Constraint(){
    override def validate(name: String, value: String): Option[String] =
@@ -212,15 +201,14 @@ trait FileUploadControllerBase {
  //  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
-  }

 }
--- a/src/main/scala/app/DashboardController.scala
+++ b/src/main/scala/app/DashboardController.scala
@@ -2,6 +2,7 @@ package app

 import service._
 import util.UsersAuthenticator
+import util.Implicits._

 class DashboardController extends DashboardControllerBase
  with IssuesService with PullRequestService with RepositoryService with AccountService
@@ -43,11 +44,10 @@ trait DashboardControllerBase extends ControllerBase {

    // 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 @@ trait DashboardControllerBase extends ControllerBase {

    // 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)
--- a/src/main/scala/app/IssuesController.scala
+++ b/src/main/scala/app/IssuesController.scala
@@ -5,6 +5,8 @@ import jp.sf.amateras.scalatra.forms._
 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 @@ trait IssuesControllerBase extends ControllerBase {
  })

  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 @@ trait IssuesControllerBase extends ControllerBase {
          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 @@ trait IssuesControllerBase extends ControllerBase {
  })

  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 @@ trait IssuesControllerBase extends ControllerBase {
  })

  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 @@ trait IssuesControllerBase extends ControllerBase {
  })

  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 @@ trait IssuesControllerBase extends ControllerBase {
  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))
+    }
  }

 }
--- a/src/main/scala/app/PullRequestsController.scala
+++ b/src/main/scala/app/PullRequestsController.scala
@@ -63,109 +63,104 @@ trait PullRequestsControllerBase extends ControllerBase {
  })

  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 @@ trait PullRequestsControllerBase extends ControllerBase {
      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),
--- a/src/main/scala/app/RepositorySettingsController.scala
+++ b/src/main/scala/app/RepositorySettingsController.scala
@@ -9,7 +9,6 @@ import org.scalatra.FlashMapSupport
 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 @@ trait RepositorySettingsControllerBase extends ControllerBase with FlashMapSuppo
   * 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 @@ trait RepositorySettingsControllerBase extends ControllerBase with FlashMapSuppo
        .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 @@ trait RepositorySettingsControllerBase extends ControllerBase with FlashMapSuppo
   */
  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 @@ trait RepositorySettingsControllerBase extends ControllerBase with FlashMapSuppo
   */
  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
      }
  }

--- a/src/main/scala/app/SignInController.scala
+++ b/src/main/scala/app/SignInController.scala
@@ -23,7 +23,6 @@ trait SignInControllerBase extends ControllerBase { self: SystemSettingsService
  }

  post("/signin", form){ form =>
-    val settings = loadSystemSettings()
    authenticate(loadSystemSettings(), form.userName, form.password) match {
      case Some(account) => signin(account)
      case None => redirect("/signin")
--- 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 @@ object Implicits {
  }

  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
+    }
  }

 }