Merge branch 'master' into webhook

README.md

@@ -10,17 +10,17 @@ The current version of GitBucket provides a basic features below:
 - Repository search (Code and Issues)
 - Wiki
 - Issues
+- Fork / Pull request
+- Mail notification
 - Activity timeline
 - User management (for Administrators)
 - Group (like Organization in Github)
 
 Following features are not implemented, but we will make them in the future release!
 
-- Fork and pull request
 - Network graph
 - Statics
 - Watch / Star
-- Notification
 
 If you want to try the development version of GitBucket, see the documentation for developers at [Wiki](https://github.com/takezoe/gitbucket/wiki).
 
@@ -37,7 +37,7 @@ To upgrade GitBucket, only replace gitbucket.war.
 
 Release Notes
 --------
-### 1.5 - COMMING SOON!
+### 1.5 - 4 Sep 2013
 - Fork and pull request.
 - LDAP authentication.
 - Mail notification.
@@ -45,6 +45,7 @@ Release Notes
 - Add the branch tab in the repository viewer.
 - Encoding auto detection for the file content in the repository viewer.
 - Add favicon, header logo and icons for the timeline.
+- Specify data directory via environment variable GITBUCKET_HOME.
 - Fixed some bugs.
 
 ### 1.4 - 31 Jul 2013

src/main/scala/app/ControllerBase.scala

@@ -1,7 +1,7 @@
 package app
 
 import _root_.util.Directory._
-import _root_.util.{StringUtil, FileUtil, Validations}
+import _root_.util.{FileUtil, Validations}
 import org.scalatra._
 import org.scalatra.json._
 import org.json4s._
@@ -22,9 +22,8 @@ abstract class ControllerBase extends ScalatraFilter
 
   implicit val jsonFormats = DefaultFormats
 
-//  before() {
+  // Don't set content type via Accept header.
-//    contentType = "text/html"
+  override def format(implicit request: HttpServletRequest) = ""
-//  }
 
   override def doFilter(request: ServletRequest, response: ServletResponse, chain: FilterChain) {
     val httpRequest  = request.asInstanceOf[HttpServletRequest]

src/main/scala/app/IssuesController.scala

@@ -4,7 +4,7 @@ import jp.sf.amateras.scalatra.forms._
 
 import service._
 import IssuesService._
-import util.{CollaboratorsAuthenticator, ReferrerAuthenticator, ReadableUsersAuthenticator}
+import util.{CollaboratorsAuthenticator, ReferrerAuthenticator, ReadableUsersAuthenticator, Notifier}
 import org.scalatra.Ok
 
 class IssuesController extends IssuesControllerBase
@@ -112,6 +112,11 @@ trait IssuesControllerBase extends ControllerBase {
     // 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}")
   })
 
@@ -129,21 +134,15 @@ trait IssuesControllerBase extends ControllerBase {
 
   post("/:owner/:repository/issue_comments/new", commentForm)(readableUsersOnly { (form, repository) =>
     handleComment(form.issueId, Some(form.content), repository)() map { case (issue, id) =>
-      if(issue.isPullRequest){
+      redirect(s"/${repository.owner}/${repository.name}/${
-        redirect(s"/${repository.owner}/${repository.name}/pull/${form.issueId}#comment-${id}")
+        if(issue.isPullRequest) "pull" else "issues"}/${form.issueId}#comment-${id}")
-      } else {
-        redirect(s"/${repository.owner}/${repository.name}/issues/${form.issueId}#comment-${id}")
-      }
     } getOrElse NotFound
   })
 
   post("/:owner/:repository/issue_comments/state", issueStateForm)(readableUsersOnly { (form, repository) =>
     handleComment(form.issueId, form.content, repository)() map { case (issue, id) =>
-      if(issue.isPullRequest){
+      redirect(s"/${repository.owner}/${repository.name}/${
-        redirect(s"/${repository.owner}/${repository.name}/pull/${form.issueId}#comment-${id}")
+        if(issue.isPullRequest) "pull" else "issues"}/${form.issueId}#comment-${id}")
-      } else {
-        redirect(s"/${repository.owner}/${repository.name}/issues/${form.issueId}#comment-${id}")
-      }
     } getOrElse NotFound
   })
 
@@ -282,9 +281,10 @@ trait IssuesControllerBase extends ControllerBase {
       val (action, recordActivity) =
         getAction(issue)
           .collect {
-            case "close" if(issue.isPullRequest)  => true  -> (Some("close")  -> Some(recordClosePullRequestActivity _))
+            case "close"  => true  -> (Some("close")  ->
-            case "close" if(!issue.isPullRequest) => true  -> (Some("close")  -> Some(recordCloseIssueActivity _))
+                Some(if(issue.isPullRequest) recordClosePullRequestActivity _ else recordCloseIssueActivity _))
-            case "reopen"                         => false -> (Some("reopen") -> Some(recordReopenIssueActivity _))
+            case "reopen" => false -> (Some("reopen") ->
+                Some(recordReopenIssueActivity _))
           }
           .map { case (closed, t) =>
             updateClosed(owner, name, issueId, closed)
@@ -300,15 +300,29 @@ trait IssuesControllerBase extends ControllerBase {
           }
 
       // record activity
-      content foreach { content =>
+      content foreach {
-        if(issue.isPullRequest)
+        (if(issue.isPullRequest) recordCommentPullRequestActivity _ else recordCommentIssueActivity _)
-          recordCommentPullRequestActivity(owner, name, userName, issueId, content)
+        (owner, name, userName, issueId, _)
-        else
-          recordCommentIssueActivity(owner, name, userName, issueId, content)
       }
       recordActivity foreach ( _ (owner, name, userName, issueId, issue.title) )
 
-      (issue, 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}")
+            }
+          }
+      }
+
+      issue -> commentId
     }
   }
 

src/main/scala/app/PullRequestsController.scala

@@ -1,6 +1,6 @@
 package app
 
-import util.{LockUtil, CollaboratorsAuthenticator, JGitUtil, ReferrerAuthenticator}
+import util.{LockUtil, CollaboratorsAuthenticator, JGitUtil, ReferrerAuthenticator, Notifier}
 import util.Directory._
 import util.Implicits._
 import service._
@@ -100,7 +100,7 @@ trait PullRequestsControllerBase extends ControllerBase {
       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).call
+        val git = Git.cloneRepository.setDirectory(tmpdir).setURI(remote.toURI.toString).setBranch(pullreq.branch).call
 
         try {
           // mark issue as merged and close.
@@ -155,6 +155,11 @@ trait PullRequestsControllerBase extends ControllerBase {
             }
           }
 
+          // 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 {
@@ -179,7 +184,7 @@ trait PullRequestsControllerBase extends ControllerBase {
         FileUtils.deleteDirectory(tmpdir)
       }
 
-      val git = Git.cloneRepository.setDirectory(tmpdir).setURI(remote.toURI.toString).call
+      val git = Git.cloneRepository.setDirectory(tmpdir).setURI(remote.toURI.toString).setBranch(branch).call
       try {
         git.checkout.setName(branch).call
 
@@ -303,8 +308,14 @@ trait PullRequestsControllerBase extends ControllerBase {
         .call
     }
 
+    // record activity
     recordPullRequestActivity(repository.owner, repository.name, loginUserName, issueId, form.title)
 
+    // notifications
+    Notifier().toNotify(repository, issueId, form.content.getOrElse("")){
+      Notifier.msgPullRequest(s"${baseUrl}/${repository.owner}/${repository.name}/pull/${issueId}")
+    }
+
     redirect(s"/${repository.owner}/${repository.name}/pull/${issueId}")
   })
 

src/main/scala/service/SystemSettingsService.scala

@@ -47,7 +47,7 @@ trait SystemSettingsService {
       if(getValue(props, Notification, false)){
         Some(Smtp(
           getValue(props, SmtpHost, ""),
-          getOptionValue(props, SmtpPort, Some(25)),
+          getOptionValue(props, SmtpPort, Some(DefaultSmtpPort)),
           getOptionValue(props, SmtpUser, None),
           getOptionValue(props, SmtpPassword, None),
           getOptionValue[Boolean](props, SmtpSsl, None)))
@@ -99,6 +99,7 @@ object SystemSettingsService {
     password: Option[String],
     ssl: Option[Boolean])
 
+  val DefaultSmtpPort = 25
   val DefaultLdapPort = 389
 
   private val AllowAccountRegistration = "allow_account_registration"

src/main/scala/servlet/AutoUpdateListener.scala

@@ -113,6 +113,7 @@ class AutoUpdateListener extends org.h2.server.web.DbStarter {
   private val logger = LoggerFactory.getLogger(classOf[AutoUpdateListener])
   
   override def contextInitialized(event: ServletContextEvent): Unit = {
+    event.getServletContext.setInitParameter("db.url", s"jdbc:h2:${Directory.DatabaseHome}")
     super.contextInitialized(event)
     logger.debug("H2 started")
     

src/main/scala/servlet/GitRepositoryServlet.scala

@@ -24,21 +24,9 @@ class GitRepositoryServlet extends GitServlet {
   
   override def init(config: ServletConfig): Unit = {
     setReceivePackFactory(new GitBucketReceivePackFactory())
-    
+    config.getServletContext.setInitParameter("base-path", Directory.RepositoryHome)
-    // TODO are there any other ways...?
+    config.getServletContext.setInitParameter("export-all", "true")
-    super.init(new ServletConfig(){
+    super.init(config)
-      def getInitParameter(name: String): String = name match {
-        case "base-path"  => Directory.RepositoryHome
-        case "export-all" => "true"
-        case name => config.getInitParameter(name)
-      }
-      def getInitParameterNames(): java.util.Enumeration[String] = {
-        config.getInitParameterNames
-      }
-      
-      def getServletContext(): ServletContext = config.getServletContext
-      def getServletName(): String = config.getServletName
-    });
   }
   
 }

src/main/scala/servlet/TransactionFilter.scala

@@ -3,7 +3,6 @@ package servlet
 import javax.servlet._
 import org.slf4j.LoggerFactory
 import javax.servlet.http.HttpServletRequest
-import scala.slick.session.Database
 
 /**
  * Controls the transaction with the open session in view pattern.
@@ -21,10 +20,7 @@ class TransactionFilter extends Filter {
       // assets don't need transaction
       chain.doFilter(req, res)
     } else {
-      val context = req.getServletContext
+      Database(req.getServletContext) withTransaction {
-      Database.forURL(context.getInitParameter("db.url"),
-          context.getInitParameter("db.user"),
-          context.getInitParameter("db.password")) withTransaction {
         logger.debug("TODO begin transaction")
         chain.doFilter(req, res)
         logger.debug("TODO end transaction")
@@ -33,3 +29,10 @@ class TransactionFilter extends Filter {
   }
 
 }
+
+object Database {
+  def apply(context: ServletContext): scala.slick.session.Database =
+    scala.slick.session.Database.forURL(context.getInitParameter("db.url"),
+        context.getInitParameter("db.user"),
+        context.getInitParameter("db.password"))
+}

src/main/scala/util/Directory.scala

@@ -7,12 +7,17 @@ import java.io.File
  */
 object Directory {
 
-  val GitBucketHome = new File(System.getProperty("user.home"), "gitbucket").getAbsolutePath
+  val GitBucketHome = (scala.util.Properties.envOrNone("GITBUCKET_HOME") match {
+    case Some(env) => new File(env)
+    case None => new File(System.getProperty("user.home"), "gitbucket")
+  }).getAbsolutePath
 
   val GitBucketConf = new File(GitBucketHome, "gitbucket.conf")
   
   val RepositoryHome = s"${GitBucketHome}/repositories"
 
+  val DatabaseHome = s"${GitBucketHome}/data"
+  
   /**
    * Repository names of the specified user.
    */

src/main/scala/util/Notifier.scala

@@ -1,23 +1,72 @@
 package util
 
-import org.apache.commons.mail.{DefaultAuthenticator, SimpleEmail}
+import scala.concurrent._
+import ExecutionContext.Implicits.global
+import org.apache.commons.mail.{DefaultAuthenticator, HtmlEmail}
+import org.slf4j.LoggerFactory
 
-import service.SystemSettingsService.{SystemSettings, Smtp}
+import app.Context
+import service.{AccountService, RepositoryService, IssuesService, SystemSettingsService}
+import servlet.Database
+import SystemSettingsService.Smtp
 
-trait Notifier {
+trait Notifier extends RepositoryService with AccountService with IssuesService {
+  def toNotify(r: RepositoryService.RepositoryInfo, issueId: Int, content: String)
+      (msg: String => String)(implicit context: Context): Unit
+
+  protected def recipients(issue: model.Issue)(notify: String => Unit)(implicit context: Context) =
+    (
+        // individual repository's owner
+        issue.userName ::
+        // collaborators
+        getCollaborators(issue.userName, issue.repositoryName) :::
+        // participants
+        issue.openedUserName ::
+        getComments(issue.userName, issue.repositoryName, issue.issueId).map(_.commentedUserName)
+    )
+    .distinct
+    .withFilter ( _ != context.loginAccount.get.userName )	// the operation in person is excluded
+    .foreach ( getAccountByUserName(_) filterNot (_.isGroupAccount) foreach (x => notify(x.mailAddress)) )
 
 }
 
 object Notifier {
-  def apply(settings: SystemSettings) = {
+  // TODO We want to be able to switch to mock.
-    new Mailer(settings.smtp.get)
+  def apply(): Notifier = new SystemSettingsService {}.loadSystemSettings match {
+    case settings if settings.notification => new Mailer(settings.smtp.get)
+    case _ => new MockMailer
   }
 
+  def msgIssue(url: String) = (content: String) => s"""
+    |${content}<br/>
+    |--<br/>
+    |<a href="${url}">View it on GitBucket</a>
+    """.stripMargin
+
+  def msgPullRequest(url: String) = (content: String) => s"""
+    |${content}<hr/>
+    |View, comment on, or merge it at:<br/>
+    |<a href="${url}">${url}</a>
+    """.stripMargin
+
+  def msgComment(url: String) = (content: String) => s"""
+    |${content}<br/>
+    |--<br/>
+    |<a href="${url}">View it on GitBucket</a>
+    """.stripMargin
+
+  def msgStatus(url: String) = (content: String) => s"""
+    |${content} <a href="${url}">#${url split('/') last}</a>
+    """.stripMargin
 }
 
-class Mailer(val smtp: Smtp) extends Notifier {
+class Mailer(private val smtp: Smtp) extends Notifier {
-  def notifyTo(issue: model.Issue) = {
+  private val logger = LoggerFactory.getLogger(classOf[Mailer])
-    val email = new SimpleEmail
+
+  def toNotify(r: RepositoryService.RepositoryInfo, issueId: Int, content: String)
+      (msg: String => String)(implicit context: Context) = {
+    val f = future {
+      val email = new HtmlEmail
       email.setHostName(smtp.host)
       email.setSmtpPort(smtp.port.get)
       smtp.user.foreach { user =>
@@ -26,12 +75,30 @@ class Mailer(val smtp: Smtp) extends Notifier {
       smtp.ssl.foreach { ssl =>
         email.setSSLOnConnect(ssl)
       }
-    email.setFrom("TODO address", "TODO　name")
+      email.setFrom("notifications@gitbucket.com", context.loginAccount.get.userName)
-    email.addTo("TODO")
+      email.setHtmlMsg(msg(view.Markdown.toHtml(content, r, false, true)))
-    email.setSubject(s"[${issue.repositoryName}] ${issue.title} (#${issue.issueId})")
-    email.setMsg("TODO")
 
-    email.send
+      // TODO Can we use the Database Session in other than Transaction Filter?
+      Database(context.request.getServletContext) withSession {
+        getIssue(r.owner, r.name, issueId.toString) foreach { issue =>
+          email.setSubject(s"[${r.name}] ${issue.title} (#${issueId})")
+          recipients(issue) {
+            email.getToAddresses.clear
+            email.addTo(_).send
+          }
+        }
+      }
+      "Notifications Successful."
+    }
+    f onSuccess {
+      case s => logger.debug(s)
+    }
+    f onFailure {
+      case t => logger.error("Notifications Failed.", t)
+    }
   }
 }
-class MockMailer extends Notifier
+class MockMailer extends Notifier {
+  def toNotify(r: RepositoryService.RepositoryInfo, issueId: Int, content: String)
+      (msg: String => String)(implicit context: Context): Unit = {}
+}

src/main/twirl/settings/options.scala.html

@@ -20,6 +20,7 @@
                 <option value="@branch"@if(branch==repository.repository.defaultBranch){ selected}>@branch</option>
               }
             </select>
+            <span class="error" id="error-defaultBranch"></span>
           </fieldset>
           <fieldset class="margin">
             <label>

src/main/webapp/WEB-INF/web.xml

@@ -55,11 +55,6 @@
     <listener-class>servlet.AutoUpdateListener</listener-class>
   </listener>
   
-  <context-param>
-    <param-name>db.url</param-name>
-    <param-value>jdbc:h2:~/gitbucket/data</param-value>
-  </context-param>
-  
   <context-param>
     <param-name>db.user</param-name>
     <param-value>sa</param-value>