From 0acbaeae861252943ce7a8ce5cccb8e173c4c03c Mon Sep 17 00:00:00 2001 From: nazoking Date: Thu, 29 Jan 2015 21:38:50 +0900 Subject: [PATCH 01/31] new branches ui like GitHub. --- .../app/RepositoryViewerController.scala | 12 +- src/main/scala/util/JGitUtil.scala | 43 ++++++ src/main/twirl/repo/branches.scala.html | 127 ++++++++++++++---- 3 files changed, 145 insertions(+), 37 deletions(-) diff --git a/src/main/scala/app/RepositoryViewerController.scala b/src/main/scala/app/RepositoryViewerController.scala index 00f67556f..e7529165e 100644 --- a/src/main/scala/app/RepositoryViewerController.scala +++ b/src/main/scala/app/RepositoryViewerController.scala @@ -323,14 +323,10 @@ trait RepositoryViewerControllerBase extends ControllerBase { * Displays branches. */ get("/:owner/:repository/branches")(referrersOnly { repository => - using(Git.open(getRepositoryDir(repository.owner, repository.name))){ git => - // retrieve latest update date of each branch - val branchInfo = repository.branchList.map { branchName => - val revCommit = git.log.add(git.getRepository.resolve(branchName)).setMaxCount(1).call.iterator.next - (branchName, revCommit.getCommitterIdent.getWhen) - } - repo.html.branches(branchInfo, hasWritePermission(repository.owner, repository.name, context.loginAccount), repository) - } + val branches = JGitUtil.getBranches(repository.owner, repository.name, repository.repository.defaultBranch) + .sortBy(br => (br.mergeInfo.isEmpty, br.commitTime)) + .reverse + repo.html.branches(branches, hasWritePermission(repository.owner, repository.name, context.loginAccount), repository) }) /** diff --git a/src/main/scala/util/JGitUtil.scala b/src/main/scala/util/JGitUtil.scala index b5e2e3bef..bd730b53f 100644 --- a/src/main/scala/util/JGitUtil.scala +++ b/src/main/scala/util/JGitUtil.scala @@ -133,6 +133,10 @@ object JGitUtil { */ case class SubmoduleInfo(name: String, path: String, url: String) + case class BranchMergeInfo(ahead: Int, behind: Int, isMerged: Boolean) + + case class BranchInfo(name: String, committerName: String, commitTime: Date, committerEmailAddress:String, mergeInfo: Option[BranchMergeInfo]) + /** * Returns RevCommit from the commit or tag id. * @@ -685,4 +689,43 @@ object JGitUtil { return git.log.add(startCommit).addPath(path).setMaxCount(1).call.iterator.next } + def getBranches(owner: String, name: String, defaultBranch: String): Seq[BranchInfo] = { + using(Git.open(getRepositoryDir(owner, name))){ git => + val repo = git.getRepository + val defaultObject = repo.resolve(defaultBranch) + git.branchList.call.asScala.map { ref => + val walk = new RevWalk(repo) + try{ + val defaultCommit = walk.parseCommit(defaultObject) + val branchName = ref.getName.stripPrefix("refs/heads/") + val branchCommit = if(branchName == defaultBranch){ + defaultCommit + }else{ + walk.parseCommit(ref.getObjectId) + } + val when = branchCommit.getCommitterIdent.getWhen + val committer = branchCommit.getCommitterIdent.getName + val committerEmail = branchCommit.getCommitterIdent.getEmailAddress + val mergeInfo = if(branchName==defaultBranch){ + None + }else{ + walk.reset() + walk.setRevFilter( RevFilter.MERGE_BASE ) + walk.markStart(branchCommit) + walk.markStart(defaultCommit) + val mergeBase = walk.next() + walk.reset() + walk.setRevFilter(RevFilter.ALL) + Some(BranchMergeInfo( + ahead = RevWalkUtils.count(walk, branchCommit, mergeBase), + behind = RevWalkUtils.count(walk, defaultCommit, mergeBase), + isMerged = walk.isMergedInto(branchCommit, defaultCommit))) + } + BranchInfo(branchName, committer, when, committerEmail, mergeInfo) + } finally { + walk.dispose(); + } + } + } + } } diff --git a/src/main/twirl/repo/branches.scala.html b/src/main/twirl/repo/branches.scala.html index 98576841e..89ee98d22 100644 --- a/src/main/twirl/repo/branches.scala.html +++ b/src/main/twirl/repo/branches.scala.html @@ -1,4 +1,4 @@ -@(branchInfo: Seq[(String, java.util.Date)], +@(branchInfo: Seq[util.JGitUtil.BranchInfo], hasWritePermission: Boolean, repository: service.RepositoryService.RepositoryInfo)(implicit context: app.Context) @import context._ @@ -6,35 +6,46 @@ @html.main(s"${repository.owner}/${repository.name}", Some(repository)) { @html.menu("code", repository){

Branches

- - - - - - - - @branchInfo.map { case (branchName, latestUpdateDate) => +
BranchLast updateCompareDownload
+ - + + + + @branchInfo.map { branch => + - - - + }
- @branchName - @if(hasWritePermission && repository.repository.defaultBranch != branchName){ - Delete branch + All branches
+
+ @branch.mergeInfo.map{ info => + Compare + @if(hasWritePermission){ + + } + } +
+
+ @branch.name + + Updated @helper.html.datetimeago(branch.commitTime, false) + by @user(branch.committerName, branch.committerEmailAddress, "muted-link") + + +
+
+ @if(branch.mergeInfo.isEmpty){ + Default + }else{ + @branch.mergeInfo.map{ info => +
+
+
@info.ahead
+
@info.behind
+
+
+
+ } } - 		- @helper.html.datetimeago(latestUpdateDate, false) - - @if(repository.repository.defaultBranch == branchName){ - Base branch - } else { - to @{repository.repository.defaultBranch} - } - - ZIP - TAR.GZ -
@@ -46,5 +57,63 @@ $(function(){ var branchName = $(e.target).data('name'); return confirm('Are you sure you want to remove the ' + branchName + ' branch?'); }); + $('*[data-toggle=tooltip]').tooltip(); }); - \ No newline at end of file + + From 5f5cc8d45431a78158a485cea90adc15ff781b19 Mon Sep 17 00:00:00 2001 From: nazoking Date: Fri, 30 Jan 2015 00:30:11 +0900 Subject: [PATCH 02/31] add action link to pull request. --- .../app/RepositoryViewerController.scala | 5 +- .../scala/service/PullRequestService.scala | 19 ++++ src/main/scala/util/JGitUtil.scala | 4 +- src/main/twirl/repo/branches.scala.html | 86 +++++-------------- .../webapp/assets/common/css/gitbucket.css | 56 ++++++++++++ 5 files changed, 103 insertions(+), 67 deletions(-) diff --git a/src/main/scala/app/RepositoryViewerController.scala b/src/main/scala/app/RepositoryViewerController.scala index e7529165e..ede292dd9 100644 --- a/src/main/scala/app/RepositoryViewerController.scala +++ b/src/main/scala/app/RepositoryViewerController.scala @@ -21,7 +21,7 @@ import service.WebHookService.WebHookPayload class RepositoryViewerController extends RepositoryViewerControllerBase with RepositoryService with AccountService with ActivityService with IssuesService with WebHookService with CommitsService - with ReadableUsersAuthenticator with ReferrerAuthenticator with CollaboratorsAuthenticator + with ReadableUsersAuthenticator with ReferrerAuthenticator with CollaboratorsAuthenticator with PullRequestService /** @@ -29,7 +29,7 @@ class RepositoryViewerController extends RepositoryViewerControllerBase */ trait RepositoryViewerControllerBase extends ControllerBase { self: RepositoryService with AccountService with ActivityService with IssuesService with WebHookService with CommitsService - with ReadableUsersAuthenticator with ReferrerAuthenticator with CollaboratorsAuthenticator => + with ReadableUsersAuthenticator with ReferrerAuthenticator with CollaboratorsAuthenticator with PullRequestService => ArchiveCommand.registerFormat("zip", new ZipFormat) ArchiveCommand.registerFormat("tar.gz", new TgzFormat) @@ -325,6 +325,7 @@ trait RepositoryViewerControllerBase extends ControllerBase { get("/:owner/:repository/branches")(referrersOnly { repository => val branches = JGitUtil.getBranches(repository.owner, repository.name, repository.repository.defaultBranch) .sortBy(br => (br.mergeInfo.isEmpty, br.commitTime)) + .map(br => br -> getPullRequestByRequestCommit(repository.owner, repository.name, repository.repository.defaultBranch, br.name, br.commitId)) .reverse repo.html.branches(branches, hasWritePermission(repository.owner, repository.name, context.loginAccount), repository) }) diff --git a/src/main/scala/service/PullRequestService.scala b/src/main/scala/service/PullRequestService.scala index 9a3239b4c..6a662b5c6 100644 --- a/src/main/scala/service/PullRequestService.scala +++ b/src/main/scala/service/PullRequestService.scala @@ -81,6 +81,25 @@ trait PullRequestService { self: IssuesService => .map { case (t1, t2) => t1 } .list + def getPullRequestByRequestCommit(userName: String, repositoryName: String, toBranch:String, fromBranch: String, commitId: String) + (implicit s: Session): Option[(PullRequest, Issue)] = { + if(toBranch == fromBranch){ + None + } else { + PullRequests + .innerJoin(Issues).on { (t1, t2) => t1.byPrimaryKey(t2.userName, t2.repositoryName, t2.issueId) } + .filter { case (t1, t2) => + (t1.userName === userName.bind) && + (t1.repositoryName === repositoryName.bind) && + (t1.branch === toBranch.bind) && + (t1.requestUserName === userName.bind) && + (t1.requestRepositoryName === repositoryName.bind) && + (t1.requestBranch === fromBranch.bind) && + (t1.commitIdTo === commitId.bind) + } + .firstOption + } + } } object PullRequestService { diff --git a/src/main/scala/util/JGitUtil.scala b/src/main/scala/util/JGitUtil.scala index bd730b53f..28d9b38b8 100644 --- a/src/main/scala/util/JGitUtil.scala +++ b/src/main/scala/util/JGitUtil.scala @@ -135,7 +135,7 @@ object JGitUtil { case class BranchMergeInfo(ahead: Int, behind: Int, isMerged: Boolean) - case class BranchInfo(name: String, committerName: String, commitTime: Date, committerEmailAddress:String, mergeInfo: Option[BranchMergeInfo]) + case class BranchInfo(name: String, committerName: String, commitTime: Date, committerEmailAddress:String, mergeInfo: Option[BranchMergeInfo], commitId: String) /** * Returns RevCommit from the commit or tag id. @@ -721,7 +721,7 @@ object JGitUtil { behind = RevWalkUtils.count(walk, defaultCommit, mergeBase), isMerged = walk.isMergedInto(branchCommit, defaultCommit))) } - BranchInfo(branchName, committer, when, committerEmail, mergeInfo) + BranchInfo(branchName, committer, when, committerEmail, mergeInfo, ref.getObjectId.name) } finally { walk.dispose(); } diff --git a/src/main/twirl/repo/branches.scala.html b/src/main/twirl/repo/branches.scala.html index 89ee98d22..e91208e93 100644 --- a/src/main/twirl/repo/branches.scala.html +++ b/src/main/twirl/repo/branches.scala.html @@ -1,4 +1,4 @@ -@(branchInfo: Seq[util.JGitUtil.BranchInfo], +@(branchInfo: Seq[(util.JGitUtil.BranchInfo, Option[(model.PullRequest, model.Issue)])], hasWritePermission: Boolean, repository: service.RepositoryService.RepositoryInfo)(implicit context: app.Context) @import context._ @@ -6,20 +6,37 @@ @html.main(s"${repository.owner}/${repository.name}", Some(repository)) { @html.menu("code", repository){

Branches

- +
- @branchInfo.map { branch => + @branchInfo.map { case (branch, prs) => diff --git a/src/main/twirl/issues/listparts.scala.html b/src/main/twirl/issues/listparts.scala.html index 1ff944f79..6c9378932 100644 --- a/src/main/twirl/issues/listparts.scala.html +++ b/src/main/twirl/issues/listparts.scala.html @@ -60,14 +60,14 @@ } @helper.html.dropdown("Milestone", flat = true) {
  • - - @helper.html.checkicon(condition.milestoneId == Some(None)) Issues with no milestone + + @helper.html.checkicon(condition.milestone == Some(None)) Issues with no milestone
    • @milestones.filter(_.closedDate.isEmpty).map { milestone =>
  • - - @helper.html.checkicon(condition.milestoneId == Some(Some(milestone.milestoneId))) @milestone.title + + @helper.html.checkicon(condition.milestone == Some(Some(milestone.title))) @milestone.title
    • } @@ -156,8 +156,8 @@ } else { No pull requests to show. } - @if(condition.labels.nonEmpty || condition.milestoneId.isDefined){ - Clear active filters. + @if(condition.labels.nonEmpty || condition.milestone.isDefined){ + Clear active filters. } else { @if(repository.isDefined){ @if(target == "issues"){ @@ -205,7 +205,7 @@
    #@issue.issueId opened @helper.html.datetimeago(issue.registeredDate) by @user(issue.openedUserName, styleClass="username") @milestone.map { milestone => - @milestone + @milestone }
    From d6946b93c38b9105ef19061fa771118970aa9273 Mon Sep 17 00:00:00 2001 From: nazoking Date: Wed, 28 Jan 2015 14:44:45 +0900 Subject: [PATCH 07/31] Add access token table and manage ui. --- src/main/resources/update/2_9.sql | 13 +++++ src/main/scala/app/AccountController.scala | 44 ++++++++++++++- src/main/scala/model/AccessToken.scala | 20 +++++++ src/main/scala/model/Profile.scala | 3 +- .../scala/service/AccesTokenService.scala | 43 +++++++++++++++ .../scala/servlet/InitializeListener.scala | 1 + src/main/twirl/account/application.scala.html | 55 +++++++++++++++++++ src/main/twirl/account/menu.scala.html | 3 + 8 files changed, 180 insertions(+), 2 deletions(-) create mode 100644 src/main/resources/update/2_9.sql create mode 100644 src/main/scala/model/AccessToken.scala create mode 100644 src/main/scala/service/AccesTokenService.scala create mode 100644 src/main/twirl/account/application.scala.html diff --git a/src/main/resources/update/2_9.sql b/src/main/resources/update/2_9.sql new file mode 100644 index 000000000..7be56abf0 --- /dev/null +++ b/src/main/resources/update/2_9.sql @@ -0,0 +1,13 @@ +DROP TABLE IF EXISTS ACCESS_TOKEN; + +CREATE TABLE ACCESS_TOKEN ( + ACCESS_TOKEN_ID INT NOT NULL AUTO_INCREMENT, + TOKEN_HASH VARCHAR(40) NOT NULL, + USER_NAME VARCHAR(100) NOT NULL, + NOTE TEXT NOT NULL +); + +ALTER TABLE ACCESS_TOKEN ADD CONSTRAINT IDX_ACCESS_TOKEN_PK PRIMARY KEY (ACCESS_TOKEN_ID); +ALTER TABLE ACCESS_TOKEN ADD CONSTRAINT IDX_ACCESS_TOKEN_FK0 FOREIGN KEY (USER_NAME) REFERENCES ACCOUNT (USER_NAME) + ON DELETE CASCADE ON UPDATE CASCADE; +ALTER TABLE ACCESS_TOKEN ADD CONSTRAINT IDX_ACCESS_TOKEN_TOKEN_HASH UNIQUE(TOKEN_HASH); diff --git a/src/main/scala/app/AccountController.scala b/src/main/scala/app/AccountController.scala index 83116543d..62711b5c6 100644 --- a/src/main/scala/app/AccountController.scala +++ b/src/main/scala/app/AccountController.scala @@ -18,10 +18,12 @@ import model.GroupMember class AccountController extends AccountControllerBase with AccountService with RepositoryService with ActivityService with WikiService with LabelsService with SshKeyService with OneselfAuthenticator with UsersAuthenticator with GroupManagerAuthenticator with ReadableUsersAuthenticator + with AccessTokenService trait AccountControllerBase extends AccountManagementControllerBase { self: AccountService with RepositoryService with ActivityService with WikiService with LabelsService with SshKeyService - with OneselfAuthenticator with UsersAuthenticator with GroupManagerAuthenticator with ReadableUsersAuthenticator => + with OneselfAuthenticator with UsersAuthenticator with GroupManagerAuthenticator with ReadableUsersAuthenticator + with AccessTokenService => case class AccountNewForm(userName: String, password: String, fullName: String, mailAddress: String, url: Option[String], fileId: Option[String]) @@ -31,6 +33,8 @@ trait AccountControllerBase extends AccountManagementControllerBase { case class SshKeyForm(title: String, publicKey: String) + case class PersonalTokenForm(note: String) + val newForm = mapping( "userName" -> trim(label("User name" , text(required, maxlength(100), identifier, uniqueUserName))), "password" -> trim(label("Password" , text(required, maxlength(20)))), @@ -54,6 +58,10 @@ trait AccountControllerBase extends AccountManagementControllerBase { "publicKey" -> trim(label("Key" , text(required, validPublicKey))) )(SshKeyForm.apply) + val personalTokenForm = mapping( + "note" -> trim(label("Token", text(required, maxlength(100)))) + )(PersonalTokenForm.apply) + case class NewGroupForm(groupName: String, url: Option[String], fileId: Option[String], members: String) case class EditGroupForm(groupName: String, url: Option[String], fileId: Option[String], members: String, clearImage: Boolean) @@ -206,6 +214,40 @@ trait AccountControllerBase extends AccountManagementControllerBase { redirect(s"/${userName}/_ssh") }) + get("/:userName/_application")(oneselfOnly { + val userName = params("userName") + getAccountByUserName(userName).map { x => + var tokens = getAccessTokens(x.userName) + val generatedToken = flash.get("generatedToken") match { + case Some((tokenId:Int, token:String)) => { + val gt = tokens.find(_.accessTokenId == tokenId) + gt.map{ t => + tokens = tokens.filterNot(_ == t) + (t, token) + } + } + case _ => None + } + account.html.application(x, tokens, generatedToken) + } getOrElse NotFound + }) + + post("/:userName/_personalToken", personalTokenForm)(oneselfOnly { form => + val userName = params("userName") + getAccountByUserName(userName).map { x => + val (tokenId, token) = generateAccessToken(userName, form.note) + flash += "generatedToken" -> (tokenId, token) + } + redirect(s"/${userName}/_application") + }) + + get("/:userName/_personalToken/delete/:id")(oneselfOnly { + val userName = params("userName") + val tokenId = params("id").toInt + deleteAccessToken(userName, tokenId) + redirect(s"/${userName}/_application") + }) + get("/register"){ if(context.settings.allowAccountRegistration){ if(context.loginAccount.isDefined){ diff --git a/src/main/scala/model/AccessToken.scala b/src/main/scala/model/AccessToken.scala new file mode 100644 index 000000000..acaad56f4 --- /dev/null +++ b/src/main/scala/model/AccessToken.scala @@ -0,0 +1,20 @@ +package model + +trait AccessTokenComponent { self: Profile => + import profile.simple._ + lazy val AccessTokens = TableQuery[AccessTokens] + + class AccessTokens(tag: Tag) extends Table[AccessToken](tag, "ACCESS_TOKEN") { + val accessTokenId = column[Int]("ACCESS_TOKEN_ID", O AutoInc) + val userName = column[String]("USER_NAME") + val tokenHash = column[String]("TOKEN_HASH") + val note = column[String]("NOTE") + def * = (accessTokenId, userName, tokenHash, note) <> (AccessToken.tupled, AccessToken.unapply) + } +} +case class AccessToken( + accessTokenId: Int = 0, + userName: String, + tokenHash: String, + note: String +) diff --git a/src/main/scala/model/Profile.scala b/src/main/scala/model/Profile.scala index d7ff17f37..4b65ad4e2 100644 --- a/src/main/scala/model/Profile.scala +++ b/src/main/scala/model/Profile.scala @@ -19,7 +19,8 @@ trait Profile { object Profile extends { val profile = slick.driver.H2Driver -} with AccountComponent +} with AccessTokenComponent + with AccountComponent with ActivityComponent with CollaboratorComponent with CommitCommentComponent diff --git a/src/main/scala/service/AccesTokenService.scala b/src/main/scala/service/AccesTokenService.scala new file mode 100644 index 000000000..916fde17e --- /dev/null +++ b/src/main/scala/service/AccesTokenService.scala @@ -0,0 +1,43 @@ +package service + +import model.Profile._ +import profile.simple._ +import model.AccessToken +import util.StringUtil +import scala.util.Random + +trait AccessTokenService { + + def makeAccessTokenString: String = { + val bytes = new Array[Byte](20) + Random.nextBytes(bytes) + bytes.map("%02x".format(_)).mkString + } + + def tokenToHash(token: String): String = StringUtil.sha1(token) + + /** + * @retuen (TokenId, Token) + */ + def generateAccessToken(userName: String, note: String)(implicit s: Session): (Int, String) = { + var token: String = null + var hash: String = null + do{ + token = makeAccessTokenString + hash = tokenToHash(token) + }while(AccessTokens.filter(_.tokenHash === hash.bind).exists.run) + val newToken = AccessToken( + userName = userName, + note = note, + tokenHash = hash) + val tokenId = (AccessTokens returning AccessTokens.map(_.accessTokenId)) += newToken + (tokenId, token) + } + + def getAccessTokens(userName: String)(implicit s: Session): List[AccessToken] = + AccessTokens.filter(_.userName === userName.bind).sortBy(_.accessTokenId.desc).list + + def deleteAccessToken(userName: String, accessTokenId: Int)(implicit s: Session): Unit = + AccessTokens filter (t => t.userName === userName.bind && t.accessTokenId === accessTokenId) delete + +} diff --git a/src/main/scala/servlet/InitializeListener.scala b/src/main/scala/servlet/InitializeListener.scala index fdc3c48bf..a4b1e1ec4 100644 --- a/src/main/scala/servlet/InitializeListener.scala +++ b/src/main/scala/servlet/InitializeListener.scala @@ -19,6 +19,7 @@ object AutoUpdate { * The history of versions. A head of this sequence is the current BitBucket version. */ val versions = Seq( + new Version(2, 9), new Version(2, 8), new Version(2, 7) { override def update(conn: Connection, cl: ClassLoader): Unit = { diff --git a/src/main/twirl/account/application.scala.html b/src/main/twirl/account/application.scala.html new file mode 100644 index 000000000..11c6814e4 --- /dev/null +++ b/src/main/twirl/account/application.scala.html @@ -0,0 +1,55 @@ +@(account: model.Account, personalTokens: List[model.AccessToken], gneratedToken:Option[(model.AccessToken, String)])(implicit context: app.Context) +@import context._ +@import view.helpers._ +@html.main("Applications"){ +
    +
    +
    + @menu("applications", settings.ssh) +
    +
    +
    +
    Personal access tokens
    +
    + @if(personalTokens.isEmpty && gneratedToken.isEmpty){ + No tokens. + }else{ + Tokens you have generated that can be used to access the GitBucket API.
    + } + @gneratedToken.map{ case (token, tokenString) => +
    + Make sure to copy your new personal access token now. You won't be able to see it again! +
    + @helper.html.copy("generated-token-copy", tokenString){ + + } + Delete +
    + } + @personalTokens.zipWithIndex.map { case (token, i) => + @if(i != 0){ +
    + } + @token.note + Delete + } +
    +
    +
    +
    +
    Generate new token
    +
    +
    + +
    + +

    What's this token for?

    +
    + +
    +
    + +
    +
    +
    +} diff --git a/src/main/twirl/account/menu.scala.html b/src/main/twirl/account/menu.scala.html index a5d913948..914a92b04 100644 --- a/src/main/twirl/account/menu.scala.html +++ b/src/main/twirl/account/menu.scala.html @@ -10,5 +10,8 @@ SSH Keys } + + Applications + From 3fd97662f537539f98f7d4a2e7ef52d8be9ddaf5 Mon Sep 17 00:00:00 2001 From: nazoking Date: Wed, 28 Jan 2015 18:57:14 +0900 Subject: [PATCH 08/31] Add Authorization logic to Controller --- src/main/scala/app/ControllerBase.scala | 9 +++++++-- src/main/scala/service/AccesTokenService.scala | 11 ++++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/main/scala/app/ControllerBase.scala b/src/main/scala/app/ControllerBase.scala index 8a184f08d..4a83dda9f 100644 --- a/src/main/scala/app/ControllerBase.scala +++ b/src/main/scala/app/ControllerBase.scala @@ -10,7 +10,7 @@ import org.json4s._ import jp.sf.amateras.scalatra.forms._ import org.apache.commons.io.FileUtils import model._ -import service.{SystemSettingsService, AccountService} +import service.{SystemSettingsService, AccountService, AccessTokenService} import javax.servlet.http.{HttpServletResponse, HttpServletRequest} import javax.servlet.{FilterChain, ServletResponse, ServletRequest} import org.scalatra.i18n._ @@ -74,7 +74,12 @@ abstract class ControllerBase extends ScalatraFilter } } - private def LoginAccount: Option[Account] = session.getAs[Account](Keys.Session.LoginAccount) + private def LoginAccount: Option[Account] = { + Option(request.getHeader("Authorization")) match { + case Some(auth) if auth.startsWith("token ") => AccessTokenService.getAccountByAccessToken(auth.substring(6).trim) + case _ => session.getAs[Account](Keys.Session.LoginAccount) + } + } def ajaxGet(path : String)(action : => Any) : Route = super.get(path){ diff --git a/src/main/scala/service/AccesTokenService.scala b/src/main/scala/service/AccesTokenService.scala index 916fde17e..f4ef727f9 100644 --- a/src/main/scala/service/AccesTokenService.scala +++ b/src/main/scala/service/AccesTokenService.scala @@ -2,7 +2,7 @@ package service import model.Profile._ import profile.simple._ -import model.AccessToken +import model.{Account, AccessToken} import util.StringUtil import scala.util.Random @@ -34,6 +34,13 @@ trait AccessTokenService { (tokenId, token) } + def getAccountByAccessToken(token: String)(implicit s: Session): Option[Account] = + Accounts + .innerJoin(AccessTokens) + .filter{ case (ac, t) => (ac.userName === t.userName) && (t.tokenHash === tokenToHash(token).bind) && (ac.removed === false.bind) } + .map{ case (ac, t) => ac } + .firstOption + def getAccessTokens(userName: String)(implicit s: Session): List[AccessToken] = AccessTokens.filter(_.userName === userName.bind).sortBy(_.accessTokenId.desc).list @@ -41,3 +48,5 @@ trait AccessTokenService { AccessTokens filter (t => t.userName === userName.bind && t.accessTokenId === accessTokenId) delete } + +object AccessTokenService extends AccessTokenService From 4e1094e75bba1ad86c65548980569703a2fe6415 Mon Sep 17 00:00:00 2001 From: nazoking Date: Wed, 28 Jan 2015 18:58:07 +0900 Subject: [PATCH 09/31] add test --- .../service/AccessTokenServiceSpec.scala | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 src/test/scala/service/AccessTokenServiceSpec.scala diff --git a/src/test/scala/service/AccessTokenServiceSpec.scala b/src/test/scala/service/AccessTokenServiceSpec.scala new file mode 100644 index 000000000..fcdd69557 --- /dev/null +++ b/src/test/scala/service/AccessTokenServiceSpec.scala @@ -0,0 +1,93 @@ +package service + +import org.specs2.mutable.Specification +import java.util.Date +import model._ + +class AccessTokenServiceSpec extends Specification with ServiceSpecBase { + + def generateNewAccount(name:String)(implicit s:Session):Account = { + AccountService.createAccount(name, name, name, s"${name}@example.com", false, None) + AccountService.getAccountByUserName(name).get + } + + "AccessTokenService" should { + "generateAccessToken" in { withTestDB { implicit session => + AccessTokenService.generateAccessToken("root", "note") must be like{ + case (id, token) if id != 0 => ok + } + }} + + "getAccessTokens" in { withTestDB { implicit session => + val (id, token) = AccessTokenService.generateAccessToken("root", "note") + val tokenHash = AccessTokenService.tokenToHash(token) + + AccessTokenService.getAccessTokens("root") must be like{ + case List(model.AccessToken(`id`, "root", `tokenHash`, "note")) => ok + } + }} + + "getAccessTokens(root) get root's tokens" in { withTestDB { implicit session => + val (id, token) = AccessTokenService.generateAccessToken("root", "note") + val tokenHash = AccessTokenService.tokenToHash(token) + val user2 = generateNewAccount("user2") + AccessTokenService.generateAccessToken("user2", "note2") + + AccessTokenService.getAccessTokens("root") must be like{ + case List(model.AccessToken(`id`, "root", `tokenHash`, "note")) => ok + } + }} + + "deleteAccessToken" in { withTestDB { implicit session => + val (id, token) = AccessTokenService.generateAccessToken("root", "note") + val user2 = generateNewAccount("user2") + AccessTokenService.generateAccessToken("user2", "note2") + + AccessTokenService.deleteAccessToken("root", id) + + AccessTokenService.getAccessTokens("root") must beEmpty + }} + + "getAccountByAccessToken" in { withTestDB { implicit session => + val (id, token) = AccessTokenService.generateAccessToken("root", "note") + AccessTokenService.getAccountByAccessToken(token) must beSome.like { + case user => user.userName must_== "root" + } + }} + + "getAccountByAccessToken don't get removed account" in { withTestDB { implicit session => + val user2 = generateNewAccount("user2") + val (id, token) = AccessTokenService.generateAccessToken("user2", "note") + AccountService.updateAccount(user2.copy(isRemoved=true)) + + AccessTokenService.getAccountByAccessToken(token) must beEmpty + }} + + "generateAccessToken create uniq token" in { withTestDB { implicit session => + val tokenIt = List("token1","token1","token1","token2").iterator + val service = new AccessTokenService{ + override def makeAccessTokenString:String = tokenIt.next + } + + service.generateAccessToken("root", "note1") must like{ + case (_, "token1") => ok + } + service.generateAccessToken("root", "note2") must like{ + case (_, "token2") => ok + } + }} + + "when update Account.userName then AccessToken.userName changed" in { withTestDB { implicit session => + val user2 = generateNewAccount("user2") + val (id, token) = AccessTokenService.generateAccessToken("user2", "note") + import model.Profile._ + import profile.simple._ + Accounts.filter(_.userName === "user2".bind).map(_.userName).update("user3") + + AccessTokenService.getAccountByAccessToken(token) must beSome.like { + case user => user.userName must_== "user3" + } + }} + } +} + From db55719a6f1cde2b5faa5099f270a62cc3dd990f Mon Sep 17 00:00:00 2001 From: nazoking Date: Wed, 28 Jan 2015 19:05:28 +0900 Subject: [PATCH 10/31] fix typo --- src/main/twirl/account/application.scala.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/twirl/account/application.scala.html b/src/main/twirl/account/application.scala.html index 11c6814e4..d2c3800e8 100644 --- a/src/main/twirl/account/application.scala.html +++ b/src/main/twirl/account/application.scala.html @@ -5,7 +5,7 @@
    - @menu("applications", settings.ssh) + @menu("application", settings.ssh)
    From ae7ead62724a85e9674850feba8fb69a80c8ad6b Mon Sep 17 00:00:00 2001 From: nazoking Date: Tue, 6 Jan 2015 17:07:32 +0900 Subject: [PATCH 11/31] Add "X-Github-Event" header, and change interface. --- src/main/scala/app/PullRequestsController.scala | 3 ++- src/main/scala/app/RepositorySettingsController.scala | 2 +- src/main/scala/app/RepositoryViewerController.scala | 2 +- src/main/scala/service/WebHookService.scala | 3 ++- src/main/scala/servlet/GitRepositoryServlet.scala | 2 +- 5 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/main/scala/app/PullRequestsController.scala b/src/main/scala/app/PullRequestsController.scala index 23ed289c0..2bccdaebe 100644 --- a/src/main/scala/app/PullRequestsController.scala +++ b/src/main/scala/app/PullRequestsController.scala @@ -192,10 +192,11 @@ trait PullRequestsControllerBase extends ControllerBase { closeIssuesFromMessage(form.message, loginAccount.userName, owner, name) } // call web hook + // TODO: set action https://developer.github.com/v3/activity/events/types/#pullrequestevent getWebHookURLs(owner, name) match { case webHookURLs if(webHookURLs.nonEmpty) => for(ownerAccount <- getAccountByUserName(owner)){ - callWebHook(owner, name, webHookURLs, + callWebHook("pull_request", webHookURLs, WebHookPayload(git, loginAccount, mergeBaseRefName, repository, commits.flatten.toList, ownerAccount)) } case _ => diff --git a/src/main/scala/app/RepositorySettingsController.scala b/src/main/scala/app/RepositorySettingsController.scala index cce95bbca..a396801dc 100644 --- a/src/main/scala/app/RepositorySettingsController.scala +++ b/src/main/scala/app/RepositorySettingsController.scala @@ -166,7 +166,7 @@ trait RepositorySettingsControllerBase extends ControllerBase { .call.iterator.asScala.map(new CommitInfo(_)) getAccountByUserName(repository.owner).foreach { ownerAccount => - callWebHook(repository.owner, repository.name, + callWebHook("push", List(model.WebHook(repository.owner, repository.name, form.url)), WebHookPayload(git, ownerAccount, "refs/heads/" + repository.repository.defaultBranch, repository, commits.toList, ownerAccount) ) diff --git a/src/main/scala/app/RepositoryViewerController.scala b/src/main/scala/app/RepositoryViewerController.scala index 8faf1ad87..e288c9861 100644 --- a/src/main/scala/app/RepositoryViewerController.scala +++ b/src/main/scala/app/RepositoryViewerController.scala @@ -507,7 +507,7 @@ trait RepositoryViewerControllerBase extends ControllerBase { getWebHookURLs(repository.owner, repository.name) match { case webHookURLs if(webHookURLs.nonEmpty) => for(ownerAccount <- getAccountByUserName(repository.owner)){ - callWebHook(repository.owner, repository.name, webHookURLs, + callWebHook("push", webHookURLs, WebHookPayload(git, loginAccount, headName, repository, List(commit), ownerAccount)) } case _ => diff --git a/src/main/scala/service/WebHookService.scala b/src/main/scala/service/WebHookService.scala index a2dafbf51..98ee9be45 100644 --- a/src/main/scala/service/WebHookService.scala +++ b/src/main/scala/service/WebHookService.scala @@ -27,7 +27,7 @@ trait WebHookService { def deleteWebHookURL(owner: String, repository: String, url :String)(implicit s: Session): Unit = WebHooks.filter(_.byPrimaryKey(owner, repository, url)).delete - def callWebHook(owner: String, repository: String, webHookURLs: List[WebHook], payload: WebHookPayload): Unit = { + def callWebHook(eventName: String, webHookURLs: List[WebHook], payload: WebHookPayload): Unit = { import org.json4s._ import org.json4s.jackson.Serialization import org.json4s.jackson.Serialization.{read, write} @@ -47,6 +47,7 @@ trait WebHookService { val f = Future { logger.debug(s"start web hook invocation for ${webHookUrl}") val httpPost = new HttpPost(webHookUrl.url) + httpPost.addHeader("X-Github-Event", eventName) val params: java.util.List[NameValuePair] = new java.util.ArrayList() params.add(new BasicNameValuePair("payload", json)) diff --git a/src/main/scala/servlet/GitRepositoryServlet.scala b/src/main/scala/servlet/GitRepositoryServlet.scala index 7fde407d0..0dfd774d9 100644 --- a/src/main/scala/servlet/GitRepositoryServlet.scala +++ b/src/main/scala/servlet/GitRepositoryServlet.scala @@ -185,7 +185,7 @@ class CommitLogHook(owner: String, repository: String, pusher: String, baseUrl: for(pusherAccount <- getAccountByUserName(pusher); ownerAccount <- getAccountByUserName(owner); repositoryInfo <- getRepository(owner, repository, baseUrl)){ - callWebHook(owner, repository, webHookURLs, + callWebHook("push", webHookURLs, WebHookPayload(git, pusherAccount, command.getRefName, repositoryInfo, newCommits, ownerAccount)) } case _ => From b512e7c390b70832cc81dabde0691fa1e5e98b8d Mon Sep 17 00:00:00 2001 From: nazoking Date: Wed, 7 Jan 2015 02:45:54 +0900 Subject: [PATCH 12/31] make WebHookPullRequestPayload --- .../scala/app/PullRequestsController.scala | 44 ++++- src/main/scala/service/WebHookService.scala | 184 ++++++++++++++---- 2 files changed, 181 insertions(+), 47 deletions(-) diff --git a/src/main/scala/app/PullRequestsController.scala b/src/main/scala/app/PullRequestsController.scala index 2bccdaebe..603f8d744 100644 --- a/src/main/scala/app/PullRequestsController.scala +++ b/src/main/scala/app/PullRequestsController.scala @@ -15,9 +15,10 @@ import service.PullRequestService._ import org.slf4j.LoggerFactory import org.eclipse.jgit.merge.MergeStrategy import org.eclipse.jgit.errors.NoMergeBaseException -import service.WebHookService.WebHookPayload +import service.WebHookService.{ WebHookPayload, WebHookPullRequestPayload } import util.JGitUtil.DiffInfo import util.JGitUtil.CommitInfo +import model.{PullRequest, Issue} class PullRequestsController extends PullRequestsControllerBase @@ -192,14 +193,8 @@ trait PullRequestsControllerBase extends ControllerBase { closeIssuesFromMessage(form.message, loginAccount.userName, owner, name) } // call web hook - // TODO: set action https://developer.github.com/v3/activity/events/types/#pullrequestevent - getWebHookURLs(owner, name) match { - case webHookURLs if(webHookURLs.nonEmpty) => - for(ownerAccount <- getAccountByUserName(owner)){ - callWebHook("pull_request", webHookURLs, - WebHookPayload(git, loginAccount, mergeBaseRefName, repository, commits.flatten.toList, ownerAccount)) - } - case _ => + getPullRequest(repository.owner, repository.name, issueId).map{ case (issue, pr) => + callPullRequestWebHook("closed", repository, issue, pr, context.baseUrl, context.loginAccount.get) } // notifications @@ -358,6 +353,11 @@ trait PullRequestsControllerBase extends ControllerBase { // record activity recordPullRequestActivity(repository.owner, repository.name, loginUserName, issueId, form.title) + // call web hook + getPullRequest(repository.owner, repository.name, issueId).map{ case (issue, pr) => + callPullRequestWebHook("opend", repository, issue, pr, context.baseUrl, context.loginAccount.get) + } + // notifications Notifier().toNotify(repository, issueId, form.content.getOrElse("")){ Notifier.msgPullRequest(s"${context.baseUrl}/${repository.owner}/${repository.name}/pull/${issueId}") @@ -481,4 +481,30 @@ trait PullRequestsControllerBase extends ControllerBase { hasWritePermission(owner, repoName, context.loginAccount)) } + private def callPullRequestWebHook(action: String, repository: RepositoryService.RepositoryInfo, issue: Issue, pullRequest: PullRequest, baseUrl: String, sender: model.Account): Unit = { + import WebHookService._ + getWebHookURLs(repository.owner, repository.name) match { + case webHookURLs if(webHookURLs.nonEmpty) => + def findOwner(owner: String, knowns: Seq[model.Account]): Option[model.Account] = knowns.find(_.fullName == owner).orElse(getAccountByUserName(owner)) + for{ + baseOwner <- findOwner(repository.owner, Seq(sender)) + headOwner <- findOwner(pullRequest.requestUserName, Seq(sender, baseOwner)) + headRepo <- getRepository(pullRequest.requestUserName, pullRequest.requestRepositoryName, baseUrl) + } yield { + val payload = WebHookPullRequestPayload( + action = action, + issue = issue, + pullRequest = pullRequest, + headRepository = headRepo, + headOwner = headOwner, + baseRepository = repository, + baseOwner = baseOwner, + sender = sender) + for(ownerAccount <- getAccountByUserName(repository.owner)){ + callWebHook("pull_request", webHookURLs, payload) + } + } + case _ => + } + } } diff --git a/src/main/scala/service/WebHookService.scala b/src/main/scala/service/WebHookService.scala index 98ee9be45..2947545f9 100644 --- a/src/main/scala/service/WebHookService.scala +++ b/src/main/scala/service/WebHookService.scala @@ -2,7 +2,7 @@ package service import model.Profile._ import profile.simple._ -import model.{WebHook, Account} +import model.{WebHook, Account, Issue, PullRequest} import org.slf4j.LoggerFactory import service.RepositoryService.RepositoryInfo import util.JGitUtil @@ -72,48 +72,25 @@ trait WebHookService { object WebHookService { - case class WebHookPayload( - pusher: WebHookUser, + trait WebHookPayload + + case class WebHookPushPayload( + pusher: WebHookApiUser, ref: String, commits: List[WebHookCommit], - repository: WebHookRepository) + repository: WebHookRepository + ) extends WebHookPayload object WebHookPayload { def apply(git: Git, pusher: Account, refName: String, repositoryInfo: RepositoryInfo, commits: List[CommitInfo], repositoryOwner: Account): WebHookPayload = - WebHookPayload( - WebHookUser(pusher.fullName, pusher.mailAddress), + WebHookPushPayload( + WebHookApiUser(pusher), refName, - commits.map { commit => - val diffs = JGitUtil.getDiffs(git, commit.id, false) - val commitUrl = repositoryInfo.httpUrl.replaceFirst("/git/", "/").stripSuffix(".git") + "/commit/" + commit.id - - WebHookCommit( - id = commit.id, - message = commit.fullMessage, - timestamp = commit.commitTime.toString, - url = commitUrl, - added = diffs._1.collect { case x if(x.changeType == DiffEntry.ChangeType.ADD) => x.newPath }, - removed = diffs._1.collect { case x if(x.changeType == DiffEntry.ChangeType.DELETE) => x.oldPath }, - modified = diffs._1.collect { case x if(x.changeType != DiffEntry.ChangeType.ADD && - x.changeType != DiffEntry.ChangeType.DELETE) => x.newPath }, - author = WebHookUser( - name = commit.committerName, - email = commit.committerEmailAddress - ) - ) - }, + commits.map{ commit => WebHookCommit(git, repositoryInfo, commit) }, WebHookRepository( - name = repositoryInfo.name, - url = repositoryInfo.httpUrl, - description = repositoryInfo.repository.description.getOrElse(""), - watchers = 0, - forks = repositoryInfo.forkedCount, - `private` = repositoryInfo.repository.isPrivate, - owner = WebHookUser( - name = repositoryOwner.userName, - email = repositoryOwner.mailAddress - ) + repositoryInfo, + owner= WebHookApiUser(repositoryOwner) ) ) } @@ -126,7 +103,46 @@ object WebHookService { added: List[String], removed: List[String], modified: List[String], - author: WebHookUser) + author: WebHookCommitUser, + committer: WebHookCommitUser) + + object WebHookCommit{ + def apply(git: Git, repositoryInfo: RepositoryInfo, commit: CommitInfo): WebHookCommit = { + val diffs = JGitUtil.getDiffs(git, commit.id, false) + val commitUrl = repositoryInfo.httpUrl.replaceFirst("/git/", "/").stripSuffix(".git") + "/commit/" + commit.id + WebHookCommit( + id = commit.id, + message = commit.fullMessage, + timestamp = commit.commitTime.toString, + url = commitUrl, + added = diffs._1.collect { case x if(x.changeType == DiffEntry.ChangeType.ADD) => x.newPath }, + removed = diffs._1.collect { case x if(x.changeType == DiffEntry.ChangeType.DELETE) => x.oldPath }, + modified = diffs._1.collect { case x if(x.changeType != DiffEntry.ChangeType.ADD && + x.changeType != DiffEntry.ChangeType.DELETE) => x.newPath }, + author = WebHookCommitUser( + name = commit.authorName, + email = commit.authorEmailAddress + ), + committer = WebHookCommitUser( + name = commit.committerName, + email = commit.committerEmailAddress + ) + ) + } + } + + case class WebHookApiUser( + login: String, + `type`: String, + site_admin: Boolean) + + object WebHookApiUser{ + def apply(user: Account): WebHookApiUser = WebHookApiUser( + login = user.fullName, + `type` = if(user.isGroupAccount){ "Organization" }else{ "User" }, + site_admin = user.isAdmin + ) + } case class WebHookRepository( name: String, @@ -135,10 +151,102 @@ object WebHookService { watchers: Int, forks: Int, `private`: Boolean, - owner: WebHookUser) + owner: WebHookApiUser) - case class WebHookUser( + object WebHookRepository{ + def apply(repositoryInfo: RepositoryInfo, owner: WebHookApiUser): WebHookRepository = + WebHookRepository( + name = repositoryInfo.name, + url = repositoryInfo.httpUrl, + description = repositoryInfo.repository.description.getOrElse(""), + watchers = 0, + forks = repositoryInfo.forkedCount, + `private` = repositoryInfo.repository.isPrivate, + owner = owner + ) + } + + case class WebHookCommitUser( name: String, email: String) + case class WebHookPullRequestPayload( + val action: String, + val number: Int, + val repository: WebHookRepository, + val pull_request: WebHookPullRequest, + val sender: WebHookApiUser + ) extends WebHookPayload + + object WebHookPullRequestPayload{ + def apply(action: String, + issue: Issue, + pullRequest: PullRequest, + headRepository: RepositoryInfo, + headOwner: Account, + baseRepository: RepositoryInfo, + baseOwner: Account, + sender: model.Account): WebHookPullRequestPayload = { + val headRepoPayload = WebHookRepository(headRepository, owner=WebHookApiUser(headOwner)) + val baseRepoPayload = WebHookRepository(baseRepository, owner=WebHookApiUser(baseOwner)) + val senderPayload = WebHookApiUser(sender) + val pr = WebHookPullRequest(issue, pullRequest, headRepoPayload, baseRepoPayload, senderPayload) + WebHookPullRequestPayload( + action = action, + number = issue.issueId, + repository = pr.base.repo, + pull_request = pr, + sender = senderPayload + ) + } + } + + case class WebHookPullRequest( + number: Int, + updated_at: String, + head: WebHookPullRequestCommit, + base: WebHookPullRequestCommit, + mergeable: Option[Boolean], + title: String, + body: String, + user: WebHookApiUser, + url: String) + + object WebHookPullRequest{ + def apply(issue: Issue, pullRequest: PullRequest, headRepo: WebHookRepository, baseRepo: WebHookRepository, user: WebHookApiUser): WebHookPullRequest = WebHookPullRequest( + number = issue.issueId, + updated_at = issue.updatedDate.toString(), + head = WebHookPullRequestCommit( + sha = pullRequest.commitIdTo, + ref = pullRequest.requestBranch, + repo = headRepo), + base = WebHookPullRequestCommit( + sha = pullRequest.commitIdFrom, + ref = pullRequest.branch, + repo = baseRepo), + mergeable = None, + title = issue.title, + body = issue.content.getOrElse(""), + user = user, + url = s"${baseRepo.url}/pulls/${issue.issueId}" + ) + } + + case class WebHookPullRequestCommit( + label: String, + sha: String, + ref: String, + repo: WebHookRepository, + user: WebHookApiUser) + + object WebHookPullRequestCommit{ + def apply(sha: String, + ref: String, + repo: WebHookRepository): WebHookPullRequestCommit = WebHookPullRequestCommit( + label = s"${repo.owner.login}:${ref}", + sha = sha, + ref = ref, + repo = repo, + user = repo.owner) + } } From 321a3a72f087ded51d7532577109e8f417b3f68c Mon Sep 17 00:00:00 2001 From: nazoking Date: Thu, 8 Jan 2015 15:09:21 +0900 Subject: [PATCH 13/31] call web hook on issue or pull_request both opened or closed. and issue_comment. --- src/main/scala/app/IssuesController.scala | 26 +- .../scala/app/PullRequestsController.scala | 39 +-- .../app/RepositorySettingsController.scala | 4 +- .../app/RepositoryViewerController.scala | 13 +- src/main/scala/service/AccountService.scala | 10 + src/main/scala/service/WebHookService.scala | 259 ++++++++++++++---- .../scala/servlet/GitRepositoryServlet.scala | 15 +- 7 files changed, 260 insertions(+), 106 deletions(-) diff --git a/src/main/scala/app/IssuesController.scala b/src/main/scala/app/IssuesController.scala index b52b22378..3dd3ec7e6 100644 --- a/src/main/scala/app/IssuesController.scala +++ b/src/main/scala/app/IssuesController.scala @@ -12,11 +12,11 @@ import model.Issue class IssuesController extends IssuesControllerBase with IssuesService with RepositoryService with AccountService with LabelsService with MilestonesService with ActivityService - with ReadableUsersAuthenticator with ReferrerAuthenticator with CollaboratorsAuthenticator + with ReadableUsersAuthenticator with ReferrerAuthenticator with CollaboratorsAuthenticator with PullRequestService with WebHookIssueCommentService trait IssuesControllerBase extends ControllerBase { self: IssuesService with RepositoryService with AccountService with LabelsService with MilestonesService with ActivityService - with ReadableUsersAuthenticator with ReferrerAuthenticator with CollaboratorsAuthenticator => + with ReadableUsersAuthenticator with ReferrerAuthenticator with CollaboratorsAuthenticator with PullRequestService with WebHookIssueCommentService => case class IssueCreateForm(title: String, content: Option[String], assignedUserName: Option[String], milestoneId: Option[Int], labelNames: Option[String]) @@ -109,9 +109,12 @@ trait IssuesControllerBase extends ControllerBase { // record activity recordCreateIssueActivity(owner, name, userName, issueId, form.title) - // extract references and create refer comment getIssue(owner, name, issueId.toString).foreach { issue => + // extract references and create refer comment createReferComment(owner, name, issue, form.title + " " + form.content.getOrElse("")) + + // call web hooks + callIssuesWebHook("opened", repository, issue, context.baseUrl, context.loginAccount.get) } // notifications @@ -364,6 +367,22 @@ trait IssuesControllerBase extends ControllerBase { createReferComment(owner, name, issue, content) } + // call web hooks + action match { + case None => callIssueCommentWebHook(repository, issue, commentId, context.loginAccount.get) + case Some(act) => val webHookAction = act match { + case "open" => "opened" + case "reopen" => "reopened" + case "close" => "closed" + case _ => act + } + if(issue.isPullRequest){ + callPullRequestWebHook(webHookAction, repository, issue.issueId, context.baseUrl, context.loginAccount.get) + } else { + callIssuesWebHook(webHookAction, repository, issue, context.baseUrl, context.loginAccount.get) + } + } + // notifications Notifier() match { case f => @@ -416,5 +435,4 @@ trait IssuesControllerBase extends ControllerBase { hasWritePermission(owner, repoName, context.loginAccount)) } } - } diff --git a/src/main/scala/app/PullRequestsController.scala b/src/main/scala/app/PullRequestsController.scala index 603f8d744..7956199d0 100644 --- a/src/main/scala/app/PullRequestsController.scala +++ b/src/main/scala/app/PullRequestsController.scala @@ -23,11 +23,11 @@ import model.{PullRequest, Issue} class PullRequestsController extends PullRequestsControllerBase with RepositoryService with AccountService with IssuesService with PullRequestService with MilestonesService with LabelsService - with CommitsService with ActivityService with WebHookService with ReferrerAuthenticator with CollaboratorsAuthenticator + with CommitsService with ActivityService with WebHookPullRequestService with ReferrerAuthenticator with CollaboratorsAuthenticator trait PullRequestsControllerBase extends ControllerBase { self: RepositoryService with AccountService with IssuesService with MilestonesService with LabelsService - with CommitsService with ActivityService with PullRequestService with WebHookService with ReferrerAuthenticator with CollaboratorsAuthenticator => + with CommitsService with ActivityService with PullRequestService with WebHookPullRequestService with ReferrerAuthenticator with CollaboratorsAuthenticator => private val logger = LoggerFactory.getLogger(classOf[PullRequestsControllerBase]) @@ -193,9 +193,7 @@ trait PullRequestsControllerBase extends ControllerBase { closeIssuesFromMessage(form.message, loginAccount.userName, owner, name) } // call web hook - getPullRequest(repository.owner, repository.name, issueId).map{ case (issue, pr) => - callPullRequestWebHook("closed", repository, issue, pr, context.baseUrl, context.loginAccount.get) - } + callPullRequestWebHook("closed", repository, issueId, context.baseUrl, context.loginAccount.get) // notifications Notifier().toNotify(repository, issueId, "merge"){ @@ -354,9 +352,7 @@ trait PullRequestsControllerBase extends ControllerBase { recordPullRequestActivity(repository.owner, repository.name, loginUserName, issueId, form.title) // call web hook - getPullRequest(repository.owner, repository.name, issueId).map{ case (issue, pr) => - callPullRequestWebHook("opend", repository, issue, pr, context.baseUrl, context.loginAccount.get) - } + callPullRequestWebHook("opend", repository, issueId, context.baseUrl, context.loginAccount.get) // notifications Notifier().toNotify(repository, issueId, form.content.getOrElse("")){ @@ -480,31 +476,4 @@ trait PullRequestsControllerBase extends ControllerBase { repository, hasWritePermission(owner, repoName, context.loginAccount)) } - - private def callPullRequestWebHook(action: String, repository: RepositoryService.RepositoryInfo, issue: Issue, pullRequest: PullRequest, baseUrl: String, sender: model.Account): Unit = { - import WebHookService._ - getWebHookURLs(repository.owner, repository.name) match { - case webHookURLs if(webHookURLs.nonEmpty) => - def findOwner(owner: String, knowns: Seq[model.Account]): Option[model.Account] = knowns.find(_.fullName == owner).orElse(getAccountByUserName(owner)) - for{ - baseOwner <- findOwner(repository.owner, Seq(sender)) - headOwner <- findOwner(pullRequest.requestUserName, Seq(sender, baseOwner)) - headRepo <- getRepository(pullRequest.requestUserName, pullRequest.requestRepositoryName, baseUrl) - } yield { - val payload = WebHookPullRequestPayload( - action = action, - issue = issue, - pullRequest = pullRequest, - headRepository = headRepo, - headOwner = headOwner, - baseRepository = repository, - baseOwner = baseOwner, - sender = sender) - for(ownerAccount <- getAccountByUserName(repository.owner)){ - callWebHook("pull_request", webHookURLs, payload) - } - } - case _ => - } - } } diff --git a/src/main/scala/app/RepositorySettingsController.scala b/src/main/scala/app/RepositorySettingsController.scala index a396801dc..233b1ef93 100644 --- a/src/main/scala/app/RepositorySettingsController.scala +++ b/src/main/scala/app/RepositorySettingsController.scala @@ -7,7 +7,7 @@ import util.{LockUtil, UsersAuthenticator, OwnerAuthenticator} import jp.sf.amateras.scalatra.forms._ import org.apache.commons.io.FileUtils import org.scalatra.i18n.Messages -import service.WebHookService.WebHookPayload +import service.WebHookService.WebHookPushPayload import util.JGitUtil.CommitInfo import util.ControlUtil._ import org.eclipse.jgit.api.Git @@ -168,7 +168,7 @@ trait RepositorySettingsControllerBase extends ControllerBase { getAccountByUserName(repository.owner).foreach { ownerAccount => callWebHook("push", List(model.WebHook(repository.owner, repository.name, form.url)), - WebHookPayload(git, ownerAccount, "refs/heads/" + repository.repository.defaultBranch, repository, commits.toList, ownerAccount) + WebHookPushPayload(git, ownerAccount, "refs/heads/" + repository.repository.defaultBranch, repository, commits.toList, ownerAccount) ) } flash += "url" -> form.url diff --git a/src/main/scala/app/RepositoryViewerController.scala b/src/main/scala/app/RepositoryViewerController.scala index e288c9861..bc6339b28 100644 --- a/src/main/scala/app/RepositoryViewerController.scala +++ b/src/main/scala/app/RepositoryViewerController.scala @@ -17,7 +17,7 @@ import org.eclipse.jgit.treewalk._ import jp.sf.amateras.scalatra.forms._ import org.eclipse.jgit.dircache.DirCache import org.eclipse.jgit.revwalk.RevCommit -import service.WebHookService.WebHookPayload +import service.WebHookService.WebHookPushPayload class RepositoryViewerController extends RepositoryViewerControllerBase with RepositoryService with AccountService with ActivityService with IssuesService with WebHookService with CommitsService @@ -504,13 +504,10 @@ trait RepositoryViewerControllerBase extends ControllerBase { // call web hook val commit = new JGitUtil.CommitInfo(JGitUtil.getRevCommitFromId(git, commitId)) - getWebHookURLs(repository.owner, repository.name) match { - case webHookURLs if(webHookURLs.nonEmpty) => - for(ownerAccount <- getAccountByUserName(repository.owner)){ - callWebHook("push", webHookURLs, - WebHookPayload(git, loginAccount, headName, repository, List(commit), ownerAccount)) - } - case _ => + callWebHookOf(repository.owner, repository.name, "push") { + getAccountByUserName(repository.owner).map{ ownerAccount => + WebHookPushPayload(git, loginAccount, headName, repository, List(commit), ownerAccount) + } } } } diff --git a/src/main/scala/service/AccountService.scala b/src/main/scala/service/AccountService.scala index c502eb7b4..88d95fa62 100644 --- a/src/main/scala/service/AccountService.scala +++ b/src/main/scala/service/AccountService.scala @@ -77,6 +77,16 @@ trait AccountService { def getAccountByUserName(userName: String, includeRemoved: Boolean = false)(implicit s: Session): Option[Account] = Accounts filter(t => (t.userName === userName.bind) && (t.removed === false.bind, !includeRemoved)) firstOption + def getAccountsByUserNames(userNames: Set[String], knowns:Set[Account], includeRemoved: Boolean = false)(implicit s: Session): Map[String, Account] = { + val map = knowns.map(a => a.userName -> a).toMap + val needs = userNames -- map.keySet + if(needs.isEmpty){ + map + }else{ + map ++ Accounts.filter(t => (t.userName inSetBind needs) && (t.removed === false.bind, !includeRemoved)).list.map(a => a.userName -> a).toMap + } + } + def getAccountByMailAddress(mailAddress: String, includeRemoved: Boolean = false)(implicit s: Session): Option[Account] = Accounts filter(t => (t.mailAddress.toLowerCase === mailAddress.toLowerCase.bind) && (t.removed === false.bind, !includeRemoved)) firstOption diff --git a/src/main/scala/service/WebHookService.scala b/src/main/scala/service/WebHookService.scala index 2947545f9..92f1952fc 100644 --- a/src/main/scala/service/WebHookService.scala +++ b/src/main/scala/service/WebHookService.scala @@ -2,7 +2,7 @@ package service import model.Profile._ import profile.simple._ -import model.{WebHook, Account, Issue, PullRequest} +import model.{WebHook, Account, Issue, PullRequest, IssueComment} import org.slf4j.LoggerFactory import service.RepositoryService.RepositoryInfo import util.JGitUtil @@ -12,6 +12,7 @@ import org.eclipse.jgit.api.Git import org.apache.http.message.BasicNameValuePair import org.apache.http.client.entity.UrlEncodedFormEntity import org.apache.http.NameValuePair +import java.util.Date trait WebHookService { import WebHookService._ @@ -27,6 +28,13 @@ trait WebHookService { def deleteWebHookURL(owner: String, repository: String, url :String)(implicit s: Session): Unit = WebHooks.filter(_.byPrimaryKey(owner, repository, url)).delete + def callWebHookOf(owner: String, repository: String, eventName: String)(makePayload: => Option[WebHookPayload])(implicit s: Session): Unit = { + val webHookURLs = getWebHookURLs(owner, repository) + if(webHookURLs.nonEmpty){ + makePayload.map(callWebHook(eventName, webHookURLs, _)) + } + } + def callWebHook(eventName: String, webHookURLs: List[WebHook], payload: WebHookPayload): Unit = { import org.json4s._ import org.json4s.jackson.Serialization @@ -67,13 +75,85 @@ trait WebHookService { } logger.debug("end callWebHook") } +} + +trait WebHookPullRequestService extends WebHookService { + self: AccountService with RepositoryService with PullRequestService with IssuesService => + + // https://developer.github.com/v3/activity/events/types/#issuesevent + def callIssuesWebHook(action: String, repository: RepositoryService.RepositoryInfo, issue: Issue, baseUrl: String, sender: model.Account)(implicit s: Session): Unit = { + import WebHookService._ + callWebHookOf(repository.owner, repository.name, "issues"){ + val users = getAccountsByUserNames(Set(repository.owner, issue.userName), Set(sender)) + for{ + repoOwner <- users.get(repository.owner) + issueUser <- users.get(issue.userName) + } yield { + WebHookIssuesPayload( + action = action, + number = issue.issueId, + repository = WebHookRepository(repository, WebHookApiUser(repoOwner)), + issue = WebHookIssue(issue, WebHookApiUser(issueUser)), + sender = WebHookApiUser(sender)) + } + } + } + + def callPullRequestWebHook(action: String, repository: RepositoryService.RepositoryInfo, issueId: Int, baseUrl: String, sender: model.Account)(implicit s: Session): Unit = { + import WebHookService._ + callWebHookOf(repository.owner, repository.name, "pull_request"){ + for{ + (issue, pullRequest) <- getPullRequest(repository.owner, repository.name, issueId) + users = getAccountsByUserNames(Set(repository.owner, pullRequest.requestUserName), Set(sender)) + baseOwner <- users.get(repository.owner) + headOwner <- users.get(pullRequest.requestUserName) + headRepo <- getRepository(pullRequest.requestUserName, pullRequest.requestRepositoryName, baseUrl) + } yield { + WebHookPullRequestPayload( + action = action, + issue = issue, + pullRequest = pullRequest, + headRepository = headRepo, + headOwner = headOwner, + baseRepository = repository, + baseOwner = baseOwner, + sender = sender) + } + } + } +} +trait WebHookIssueCommentService extends WebHookPullRequestService { + self: AccountService with RepositoryService with PullRequestService with IssuesService => + + def callIssueCommentWebHook(repository: RepositoryService.RepositoryInfo, issue: Issue, issueCommentId: Int, sender: model.Account)(implicit s: Session): Unit = { + import WebHookService._ + callWebHookOf(repository.owner, repository.name, "issue_comment"){ + for{ + issueComment <- getComment(repository.owner, repository.name, issueCommentId.toString()) + users = getAccountsByUserNames(Set(issue.userName, repository.owner, issueComment.userName), Set(sender)) + issueUser <- users.get(issue.userName) + repoOwner <- users.get(repository.owner) + commenter <- users.get(issueComment.userName) + } yield { + WebHookIssueCommentPayload( + issue = issue, + issueUser = issueUser, + comment = issueComment, + commentUser = commenter, + repository = repository, + repositoryUser = repoOwner, + sender = sender) + } + } + } } object WebHookService { trait WebHookPayload + // https://developer.github.com/v3/activity/events/types/#pushevent case class WebHookPushPayload( pusher: WebHookApiUser, ref: String, @@ -81,9 +161,9 @@ object WebHookService { repository: WebHookRepository ) extends WebHookPayload - object WebHookPayload { + object WebHookPushPayload { def apply(git: Git, pusher: Account, refName: String, repositoryInfo: RepositoryInfo, - commits: List[CommitInfo], repositoryOwner: Account): WebHookPayload = + commits: List[CommitInfo], repositoryOwner: Account): WebHookPushPayload = WebHookPushPayload( WebHookApiUser(pusher), refName, @@ -95,10 +175,76 @@ object WebHookService { ) } + // https://developer.github.com/v3/activity/events/types/#issuesevent + case class WebHookIssuesPayload( + action: String, + number: Int, + repository: WebHookRepository, + issue: WebHookIssue, + sender: WebHookApiUser) extends WebHookPayload + + // https://developer.github.com/v3/activity/events/types/#pullrequestevent + case class WebHookPullRequestPayload( + action: String, + number: Int, + repository: WebHookRepository, + pull_request: WebHookPullRequest, + sender: WebHookApiUser + ) extends WebHookPayload + + object WebHookPullRequestPayload{ + def apply(action: String, + issue: Issue, + pullRequest: PullRequest, + headRepository: RepositoryInfo, + headOwner: Account, + baseRepository: RepositoryInfo, + baseOwner: Account, + sender: model.Account): WebHookPullRequestPayload = { + val headRepoPayload = WebHookRepository(headRepository, headOwner) + val baseRepoPayload = WebHookRepository(baseRepository, baseOwner) + val senderPayload = WebHookApiUser(sender) + val pr = WebHookPullRequest(issue, pullRequest, headRepoPayload, baseRepoPayload, senderPayload) + WebHookPullRequestPayload( + action = action, + number = issue.issueId, + repository = pr.base.repo, + pull_request = pr, + sender = senderPayload + ) + } + } + + // https://developer.github.com/v3/activity/events/types/#issuecommentevent + case class WebHookIssueCommentPayload( + action: String, + repository: WebHookRepository, + issue: WebHookIssue, + comment: WebHookComment, + sender: WebHookApiUser + ) extends WebHookPayload + + object WebHookIssueCommentPayload{ + def apply( + issue: Issue, + issueUser: Account, + comment: IssueComment, + commentUser: Account, + repository: RepositoryInfo, + repositoryUser: Account, + sender: Account): WebHookIssueCommentPayload = + WebHookIssueCommentPayload( + action = "created", + repository = WebHookRepository(repository, repositoryUser), + issue = WebHookIssue(issue, WebHookApiUser(issueUser)), + comment = WebHookComment(comment, WebHookApiUser(commentUser)), + sender = WebHookApiUser(sender)) + } + case class WebHookCommit( id: String, message: String, - timestamp: String, + timestamp: String, // "2014-10-09T17:10:36-07:00", url: String, added: List[String], removed: List[String], @@ -144,12 +290,17 @@ object WebHookService { ) } + case class WebHookCommitUser( + name: String, + email: String) + + // https://developer.github.com/v3/repos/ case class WebHookRepository( name: String, url: String, description: String, watchers: Int, - forks: Int, + forks: Int, // forks_count `private`: Boolean, owner: WebHookApiUser) @@ -164,46 +315,15 @@ object WebHookService { `private` = repositoryInfo.repository.isPrivate, owner = owner ) + def apply(repositoryInfo: RepositoryInfo, owner: Account): WebHookRepository = + this(repositoryInfo, WebHookApiUser(owner)) } - case class WebHookCommitUser( - name: String, - email: String) - - case class WebHookPullRequestPayload( - val action: String, - val number: Int, - val repository: WebHookRepository, - val pull_request: WebHookPullRequest, - val sender: WebHookApiUser - ) extends WebHookPayload - - object WebHookPullRequestPayload{ - def apply(action: String, - issue: Issue, - pullRequest: PullRequest, - headRepository: RepositoryInfo, - headOwner: Account, - baseRepository: RepositoryInfo, - baseOwner: Account, - sender: model.Account): WebHookPullRequestPayload = { - val headRepoPayload = WebHookRepository(headRepository, owner=WebHookApiUser(headOwner)) - val baseRepoPayload = WebHookRepository(baseRepository, owner=WebHookApiUser(baseOwner)) - val senderPayload = WebHookApiUser(sender) - val pr = WebHookPullRequest(issue, pullRequest, headRepoPayload, baseRepoPayload, senderPayload) - WebHookPullRequestPayload( - action = action, - number = issue.issueId, - repository = pr.base.repo, - pull_request = pr, - sender = senderPayload - ) - } - } - + // https://developer.github.com/v3/pulls/ case class WebHookPullRequest( number: Int, - updated_at: String, + updated_at: Date, + created_at: Date, head: WebHookPullRequestCommit, base: WebHookPullRequestCommit, mergeable: Option[Boolean], @@ -215,7 +335,8 @@ object WebHookService { object WebHookPullRequest{ def apply(issue: Issue, pullRequest: PullRequest, headRepo: WebHookRepository, baseRepo: WebHookRepository, user: WebHookApiUser): WebHookPullRequest = WebHookPullRequest( number = issue.issueId, - updated_at = issue.updatedDate.toString(), + updated_at = issue.updatedDate, + created_at = issue.registeredDate, head = WebHookPullRequestCommit( sha = pullRequest.commitIdTo, ref = pullRequest.requestBranch, @@ -242,11 +363,53 @@ object WebHookService { object WebHookPullRequestCommit{ def apply(sha: String, ref: String, - repo: WebHookRepository): WebHookPullRequestCommit = WebHookPullRequestCommit( - label = s"${repo.owner.login}:${ref}", - sha = sha, - ref = ref, - repo = repo, - user = repo.owner) + repo: WebHookRepository): WebHookPullRequestCommit = + WebHookPullRequestCommit( + label = s"${repo.owner.login}:${ref}", + sha = sha, + ref = ref, + repo = repo, + user = repo.owner) + } + + // https://developer.github.com/v3/issues/ + case class WebHookIssue( + number: Int, + title: String, + user: WebHookApiUser, + // labels, + state: String, + created_at: Date, + updated_at: Date, + body: String) + + object WebHookIssue{ + def apply(issue: Issue, user: WebHookApiUser): WebHookIssue = + WebHookIssue( + number = issue.issueId, + title = issue.title, + user = user, + state = if(issue.closed){ "closed" }else{ "open" }, + body = issue.content.getOrElse(""), + created_at = issue.registeredDate, + updated_at = issue.updatedDate) + } + + // https://developer.github.com/v3/issues/comments/ + case class WebHookComment( + id: Int, + user: WebHookApiUser, + body: String, + created_at: Date, + updated_at: Date) + + object WebHookComment{ + def apply(comment: IssueComment, user: WebHookApiUser): WebHookComment = + WebHookComment( + id = comment.commentId, + user = user, + body = comment.content, + created_at = comment.registeredDate, + updated_at = comment.updatedDate) } } diff --git a/src/main/scala/servlet/GitRepositoryServlet.scala b/src/main/scala/servlet/GitRepositoryServlet.scala index 0dfd774d9..b9273a706 100644 --- a/src/main/scala/servlet/GitRepositoryServlet.scala +++ b/src/main/scala/servlet/GitRepositoryServlet.scala @@ -180,15 +180,12 @@ class CommitLogHook(owner: String, repository: String, pusher: String, baseUrl: } // call web hook - getWebHookURLs(owner, repository) match { - case webHookURLs if(webHookURLs.nonEmpty) => - for(pusherAccount <- getAccountByUserName(pusher); - ownerAccount <- getAccountByUserName(owner); - repositoryInfo <- getRepository(owner, repository, baseUrl)){ - callWebHook("push", webHookURLs, - WebHookPayload(git, pusherAccount, command.getRefName, repositoryInfo, newCommits, ownerAccount)) - } - case _ => + callWebHookOf(owner, repository, "push"){ + for(pusherAccount <- getAccountByUserName(pusher); + ownerAccount <- getAccountByUserName(owner); + repositoryInfo <- getRepository(owner, repository, baseUrl)) yield { + WebHookPushPayload(git, pusherAccount, command.getRefName, repositoryInfo, newCommits, ownerAccount) + } } } } From e86b404ca210f2cf724babc4962c8704c5b3480d Mon Sep 17 00:00:00 2001 From: nazoking Date: Fri, 9 Jan 2015 20:59:25 +0900 Subject: [PATCH 14/31] =?UTF-8?q?date=20time=20format=20to=20iso-8601=20(?= =?UTF-8?q?=20fit=20The=20GitHub=E3=80=80spec=20)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/main/scala/service/WebHookService.scala | 33 +++++++++++++++------ 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/src/main/scala/service/WebHookService.scala b/src/main/scala/service/WebHookService.scala index 92f1952fc..c55e6ac6d 100644 --- a/src/main/scala/service/WebHookService.scala +++ b/src/main/scala/service/WebHookService.scala @@ -36,19 +36,14 @@ trait WebHookService { } def callWebHook(eventName: String, webHookURLs: List[WebHook], payload: WebHookPayload): Unit = { - import org.json4s._ - import org.json4s.jackson.Serialization - import org.json4s.jackson.Serialization.{read, write} import org.apache.http.client.methods.HttpPost import org.apache.http.impl.client.HttpClientBuilder import scala.concurrent._ import ExecutionContext.Implicits.global - logger.debug("start callWebHook") - implicit val formats = Serialization.formats(NoTypeHints) - if(webHookURLs.nonEmpty){ - val json = write(payload) + implicit val formats = jsonFormats + val json = org.json4s.jackson.Serialization.write(payload) val httpClient = HttpClientBuilder.create.build webHookURLs.foreach { webHookUrl => @@ -151,6 +146,24 @@ trait WebHookIssueCommentService extends WebHookPullRequestService { object WebHookService { + val jsonFormats = { + import org.json4s._ + import org.json4s.jackson.Serialization + import scala.util.Try + import org.joda.time.format._ + import org.joda.time.DateTime + import org.joda.time.DateTimeZone + val parserISO = DateTimeFormat.forPattern("yyyy-MM-dd'T'HH:mm:ss'Z'") + Serialization.formats(NoTypeHints) + new CustomSerializer[Date](format => + ( + { case JString(s) => Try(parserISO.parseDateTime(s)).toOption.map(_.toDate) + .getOrElse(throw new MappingException("Can't convert " + s + " to Date")) }, + { case x: Date => JString(parserISO.print(new DateTime(x).withZone(DateTimeZone.UTC))) } + ) + ) + } + + trait WebHookPayload // https://developer.github.com/v3/activity/events/types/#pushevent @@ -244,7 +257,7 @@ object WebHookService { case class WebHookCommit( id: String, message: String, - timestamp: String, // "2014-10-09T17:10:36-07:00", + timestamp: Date, url: String, added: List[String], removed: List[String], @@ -259,7 +272,7 @@ object WebHookService { WebHookCommit( id = commit.id, message = commit.fullMessage, - timestamp = commit.commitTime.toString, + timestamp = commit.commitTime, url = commitUrl, added = diffs._1.collect { case x if(x.changeType == DiffEntry.ChangeType.ADD) => x.newPath }, removed = diffs._1.collect { case x if(x.changeType == DiffEntry.ChangeType.DELETE) => x.oldPath }, @@ -297,6 +310,7 @@ object WebHookService { // https://developer.github.com/v3/repos/ case class WebHookRepository( name: String, + full_name: String, url: String, description: String, watchers: Int, @@ -308,6 +322,7 @@ object WebHookService { def apply(repositoryInfo: RepositoryInfo, owner: WebHookApiUser): WebHookRepository = WebHookRepository( name = repositoryInfo.name, + full_name = s"${repositoryInfo.owner}/${repositoryInfo.name}", url = repositoryInfo.httpUrl, description = repositoryInfo.repository.description.getOrElse(""), watchers = 0, From dbedc2166bb114977e62e28be27d5f72a4dce98f Mon Sep 17 00:00:00 2001 From: nazoking Date: Sun, 18 Jan 2015 23:44:36 +0900 Subject: [PATCH 15/31] fix typo --- src/main/scala/app/PullRequestsController.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/scala/app/PullRequestsController.scala b/src/main/scala/app/PullRequestsController.scala index 7956199d0..83510f2b4 100644 --- a/src/main/scala/app/PullRequestsController.scala +++ b/src/main/scala/app/PullRequestsController.scala @@ -352,7 +352,7 @@ trait PullRequestsControllerBase extends ControllerBase { recordPullRequestActivity(repository.owner, repository.name, loginUserName, issueId, form.title) // call web hook - callPullRequestWebHook("opend", repository, issueId, context.baseUrl, context.loginAccount.get) + callPullRequestWebHook("opened", repository, issueId, context.baseUrl, context.loginAccount.get) // notifications Notifier().toNotify(repository, issueId, form.content.getOrElse("")){ From 32799cead7e0bb3642a48a018c29f5c7b50742cd Mon Sep 17 00:00:00 2001 From: nazoking Date: Mon, 19 Jan 2015 17:03:11 +0900 Subject: [PATCH 16/31] more github like --- src/main/scala/service/WebHookService.scala | 118 +++++++++++++----- .../scala/servlet/GitRepositoryServlet.scala | 1 + 2 files changed, 88 insertions(+), 31 deletions(-) diff --git a/src/main/scala/service/WebHookService.scala b/src/main/scala/service/WebHookService.scala index c55e6ac6d..909f32ce4 100644 --- a/src/main/scala/service/WebHookService.scala +++ b/src/main/scala/service/WebHookService.scala @@ -2,7 +2,7 @@ package service import model.Profile._ import profile.simple._ -import model.{WebHook, Account, Issue, PullRequest, IssueComment} +import model.{WebHook, Account, Issue, PullRequest, IssueComment, Repository} import org.slf4j.LoggerFactory import service.RepositoryService.RepositoryInfo import util.JGitUtil @@ -28,22 +28,25 @@ trait WebHookService { def deleteWebHookURL(owner: String, repository: String, url :String)(implicit s: Session): Unit = WebHooks.filter(_.byPrimaryKey(owner, repository, url)).delete - def callWebHookOf(owner: String, repository: String, eventName: String)(makePayload: => Option[WebHookPayload])(implicit s: Session): Unit = { + def callWebHookOf(owner: String, repository: String, eventName: String)(makePayload: => Option[WebHookPayload])(implicit s: Session, c: ApiContext): Unit = { val webHookURLs = getWebHookURLs(owner, repository) if(webHookURLs.nonEmpty){ makePayload.map(callWebHook(eventName, webHookURLs, _)) } } - def callWebHook(eventName: String, webHookURLs: List[WebHook], payload: WebHookPayload): Unit = { + def apiJson(obj: AnyRef)(implicit c: ApiContext): String = { + org.json4s.jackson.Serialization.write(obj)(jsonFormats + apiPathSerializer(c)) + } + + def callWebHook(eventName: String, webHookURLs: List[WebHook], payload: WebHookPayload)(implicit c: ApiContext): Unit = { import org.apache.http.client.methods.HttpPost import org.apache.http.impl.client.HttpClientBuilder import scala.concurrent._ import ExecutionContext.Implicits.global if(webHookURLs.nonEmpty){ - implicit val formats = jsonFormats - val json = org.json4s.jackson.Serialization.write(payload) + val json = apiJson(payload) val httpClient = HttpClientBuilder.create.build webHookURLs.foreach { webHookUrl => @@ -76,9 +79,9 @@ trait WebHookService { trait WebHookPullRequestService extends WebHookService { self: AccountService with RepositoryService with PullRequestService with IssuesService => + import WebHookService._ // https://developer.github.com/v3/activity/events/types/#issuesevent - def callIssuesWebHook(action: String, repository: RepositoryService.RepositoryInfo, issue: Issue, baseUrl: String, sender: model.Account)(implicit s: Session): Unit = { - import WebHookService._ + def callIssuesWebHook(action: String, repository: RepositoryService.RepositoryInfo, issue: Issue, baseUrl: String, sender: model.Account)(implicit s: Session, context:ApiContext): Unit = { callWebHookOf(repository.owner, repository.name, "issues"){ val users = getAccountsByUserNames(Set(repository.owner, issue.userName), Set(sender)) for{ @@ -95,7 +98,7 @@ trait WebHookPullRequestService extends WebHookService { } } - def callPullRequestWebHook(action: String, repository: RepositoryService.RepositoryInfo, issueId: Int, baseUrl: String, sender: model.Account)(implicit s: Session): Unit = { + def callPullRequestWebHook(action: String, repository: RepositoryService.RepositoryInfo, issueId: Int, baseUrl: String, sender: model.Account)(implicit s: Session, context:ApiContext): Unit = { import WebHookService._ callWebHookOf(repository.owner, repository.name, "pull_request"){ for{ @@ -118,11 +121,12 @@ trait WebHookPullRequestService extends WebHookService { } } } + trait WebHookIssueCommentService extends WebHookPullRequestService { self: AccountService with RepositoryService with PullRequestService with IssuesService => - def callIssueCommentWebHook(repository: RepositoryService.RepositoryInfo, issue: Issue, issueCommentId: Int, sender: model.Account)(implicit s: Session): Unit = { - import WebHookService._ + import WebHookService._ + def callIssueCommentWebHook(repository: RepositoryService.RepositoryInfo, issue: Issue, issueCommentId: Int, sender: model.Account)(implicit s: Session, context:ApiContext): Unit = { callWebHookOf(repository.owner, repository.name, "issue_comment"){ for{ issueComment <- getComment(repository.owner, repository.name, issueCommentId.toString()) @@ -145,10 +149,13 @@ trait WebHookIssueCommentService extends WebHookPullRequestService { } object WebHookService { + case class ApiContext(baseUrl:String) + case class ApiPath(path:String) + + import org.json4s._ + import org.json4s.jackson.Serialization val jsonFormats = { - import org.json4s._ - import org.json4s.jackson.Serialization import scala.util.Try import org.joda.time.format._ import org.joda.time.DateTime @@ -160,9 +167,17 @@ object WebHookService { .getOrElse(throw new MappingException("Can't convert " + s + " to Date")) }, { case x: Date => JString(parserISO.print(new DateTime(x).withZone(DateTimeZone.UTC))) } ) - ) + ) + FieldSerializer[WebHookApiUser]() + FieldSerializer[WebHookPullRequest]() + FieldSerializer[WebHookRepository]() } - + def apiPathSerializer(c:ApiContext) = new CustomSerializer[ApiPath](format => + ( + { + case JString(s) if s.startsWith(c.baseUrl) => ApiPath(s.substring(c.baseUrl.length)) + case JString(s) => throw new MappingException("Can't convert " + s + " to Date") + }, + { case ApiPath(path) => JString(c.baseUrl+path) } + ) + ) trait WebHookPayload @@ -292,14 +307,30 @@ object WebHookService { case class WebHookApiUser( login: String, + email: String, `type`: String, - site_admin: Boolean) + site_admin: Boolean, + created_at: Date) { + val url = ApiPath(s"/api/v3/users/${login}") + val html_url = ApiPath(s"/${login}") + // val followers_url = ApiPath(s"/api/v3/users/${login}/followers") + // val following_url = ApiPath(s"/api/v3/users/${login}/following{/other_user}") + // val gists_url = ApiPath(s"/api/v3/users/${login}/gists{/gist_id}") + // val starred_url = ApiPath(s"/api/v3/users/${login}/starred{/owner}{/repo}") + // val subscriptions_url = ApiPath(s"/api/v3/users/${login}/subscriptions") + // val organizations_url = ApiPath(s"/api/v3/users/${login}/orgs") + // val repos_url = ApiPath(s"/api/v3/users/${login}/repos") + // val events_url = ApiPath(s"/api/v3/users/${login}/events{/privacy}") + // val received_events_url = ApiPath(s"/api/v3/users/${login}/received_events") + } object WebHookApiUser{ def apply(user: Account): WebHookApiUser = WebHookApiUser( login = user.fullName, + email = user.mailAddress, `type` = if(user.isGroupAccount){ "Organization" }else{ "User" }, - site_admin = user.isAdmin + site_admin = user.isAdmin, + created_at = user.registeredDate ) } @@ -311,27 +342,43 @@ object WebHookService { case class WebHookRepository( name: String, full_name: String, - url: String, description: String, watchers: Int, - forks: Int, // forks_count + forks: Int, `private`: Boolean, - owner: WebHookApiUser) + default_branch: String, + owner: WebHookApiUser) { + val forks_count = forks + val watchers_coun = watchers + val url = ApiPath(s"/api/v3/repos/${full_name}") + val http_url = ApiPath(s"/git/${full_name}.git") + val clone_url = ApiPath(s"/git/${full_name}.git") + val html_url = ApiPath(s"/${full_name}") + } object WebHookRepository{ - def apply(repositoryInfo: RepositoryInfo, owner: WebHookApiUser): WebHookRepository = + def apply( + repository: Repository, + owner: WebHookApiUser, + forkedCount: Int =0, + watchers: Int = 0): WebHookRepository = WebHookRepository( - name = repositoryInfo.name, - full_name = s"${repositoryInfo.owner}/${repositoryInfo.name}", - url = repositoryInfo.httpUrl, - description = repositoryInfo.repository.description.getOrElse(""), + name = repository.repositoryName, + full_name = s"${repository.userName}/${repository.repositoryName}", + description = repository.description.getOrElse(""), watchers = 0, - forks = repositoryInfo.forkedCount, - `private` = repositoryInfo.repository.isPrivate, + forks = forkedCount, + `private` = repository.isPrivate, + default_branch = repository.defaultBranch, owner = owner ) + + def apply(repositoryInfo: RepositoryInfo, owner: WebHookApiUser): WebHookRepository = + WebHookRepository(repositoryInfo.repository, owner, forkedCount=repositoryInfo.forkedCount) + def apply(repositoryInfo: RepositoryInfo, owner: Account): WebHookRepository = - this(repositoryInfo, WebHookApiUser(owner)) + this(repositoryInfo.repository, WebHookApiUser(owner)) + } // https://developer.github.com/v3/pulls/ @@ -344,8 +391,18 @@ object WebHookService { mergeable: Option[Boolean], title: String, body: String, - user: WebHookApiUser, - url: String) + user: WebHookApiUser) { + val html_url = ApiPath(s"${base.repo.html_url.path}/pull/${number}") + //val diff_url = ApiPath("${base.repo.html_url.path}/pull/${number}.diff") + //val patch_url = ApiPath("${base.repo.html_url.path}/pull/${number}.patch") + val url = ApiPath(s"${base.repo.url.path}/pulls/${number}") + //val issue_url = ApiPath("${base.repo.url.path}/issues/${number}") + //val commits_url = ApiPath("${base.repo.url.path}/pulls/${number}/commits") + //val review_comments_url = ApiPath("${base.repo.url.path}/pulls/${number}/comments") + //val review_comment_url = ApiPath("${base.repo.url.path}/pulls/comments/{number}") + //val comments_url = ApiPath("${base.repo.url.path}/issues/${number}/comments") + //val statuses_url = ApiPath("${base.repo.url.path}/statuses/${head.sha}") + } object WebHookPullRequest{ def apply(issue: Issue, pullRequest: PullRequest, headRepo: WebHookRepository, baseRepo: WebHookRepository, user: WebHookApiUser): WebHookPullRequest = WebHookPullRequest( @@ -363,8 +420,7 @@ object WebHookService { mergeable = None, title = issue.title, body = issue.content.getOrElse(""), - user = user, - url = s"${baseRepo.url}/pulls/${issue.issueId}" + user = user ) } diff --git a/src/main/scala/servlet/GitRepositoryServlet.scala b/src/main/scala/servlet/GitRepositoryServlet.scala index b9273a706..a569db5d2 100644 --- a/src/main/scala/servlet/GitRepositoryServlet.scala +++ b/src/main/scala/servlet/GitRepositoryServlet.scala @@ -180,6 +180,7 @@ class CommitLogHook(owner: String, repository: String, pusher: String, baseUrl: } // call web hook + implicit val apiContext = ApiContext(baseUrl) callWebHookOf(owner, repository, "push"){ for(pusherAccount <- getAccountByUserName(pusher); ownerAccount <- getAccountByUserName(owner); From 47cb4d1c49a8e0cd4dd619da28d9c2798a62d229 Mon Sep 17 00:00:00 2001 From: nazoking Date: Mon, 19 Jan 2015 23:26:49 +0900 Subject: [PATCH 17/31] add some WebAPI/v3. * [Users](https://developer.github.com/v3/users/) get-a-single-user * [Issues/Comments](https://developer.github.com/v3/issues/comments/) list-comments-on-an-issue, create-a-comment * [Pull Requests](https://developer.github.com/v3/pulls/) list-pull-requests, get-a-single-pull-request, list-commits-on-a-pull-request * [Repositories](https://developer.github.com/v3/repos/) get --- src/main/scala/app/AccountController.scala | 24 ++++++- src/main/scala/app/IssuesController.scala | 28 ++++++++ .../scala/app/PullRequestsController.scala | 61 +++++++++++++++- .../app/RepositoryViewerController.scala | 9 ++- src/main/scala/service/IssuesService.scala | 58 +++++++++++---- src/main/scala/service/WebHookService.scala | 71 +++++++++++++++---- src/main/scala/util/Implicits.scala | 7 +- 7 files changed, 226 insertions(+), 32 deletions(-) diff --git a/src/main/scala/app/AccountController.scala b/src/main/scala/app/AccountController.scala index 62711b5c6..c0b3c892a 100644 --- a/src/main/scala/app/AccountController.scala +++ b/src/main/scala/app/AccountController.scala @@ -14,16 +14,17 @@ import org.eclipse.jgit.api.Git import org.eclipse.jgit.lib.{FileMode, Constants} import org.eclipse.jgit.dircache.DirCache import model.GroupMember +import service.WebHookService._ class AccountController extends AccountControllerBase with AccountService with RepositoryService with ActivityService with WikiService with LabelsService with SshKeyService with OneselfAuthenticator with UsersAuthenticator with GroupManagerAuthenticator with ReadableUsersAuthenticator - with AccessTokenService + with AccessTokenService with WebHookService trait AccountControllerBase extends AccountManagementControllerBase { self: AccountService with RepositoryService with ActivityService with WikiService with LabelsService with SshKeyService with OneselfAuthenticator with UsersAuthenticator with GroupManagerAuthenticator with ReadableUsersAuthenticator - with AccessTokenService => + with AccessTokenService with WebHookService => case class AccountNewForm(userName: String, password: String, fullName: String, mailAddress: String, url: Option[String], fileId: Option[String]) @@ -150,6 +151,25 @@ trait AccountControllerBase extends AccountManagementControllerBase { } } + /** + * https://developer.github.com/v3/users/#get-a-single-user + */ + get("/api/v3/users/:userName") { + getAccountByUserName(params("userName")).map { account => + apiJson(WebHookApiUser(account)) + } getOrElse NotFound + } + + /** + * https://developer.github.com/v3/users/#get-the-authenticated-user + */ + get("/api/v3/user") { + context.loginAccount.map { account => + apiJson(WebHookApiUser(account)) + } getOrElse NotFound + } + + get("/:userName/_edit")(oneselfOnly { val userName = params("userName") getAccountByUserName(userName).map { x => diff --git a/src/main/scala/app/IssuesController.scala b/src/main/scala/app/IssuesController.scala index 3dd3ec7e6..af4dc16ea 100644 --- a/src/main/scala/app/IssuesController.scala +++ b/src/main/scala/app/IssuesController.scala @@ -9,6 +9,8 @@ import util.Implicits._ import util.ControlUtil._ import org.scalatra.Ok import model.Issue +import service.WebHookService._ +import scala.util.Try class IssuesController extends IssuesControllerBase with IssuesService with RepositoryService with AccountService with LabelsService with MilestonesService with ActivityService @@ -73,6 +75,18 @@ trait IssuesControllerBase extends ControllerBase { } }) + /** + * https://developer.github.com/v3/issues/comments/#list-comments-on-an-issue + */ + get("/api/v3/repos/:owner/:repository/issues/:id/comments")(referrersOnly { repository => + (for{ + issueId <- params("id").toIntOpt + comments = getCommentsForApi(repository.owner, repository.name, issueId.toInt) + } yield { + apiJson(comments.map{ case (issueComment, user) => WebHookComment(issueComment, WebHookApiUser(user)) }) + }).getOrElse(NotFound) + }) + get("/:owner/:repository/issues/new")(readableUsersOnly { repository => defining(repository.owner, repository.name){ case (owner, name) => issues.html.create( @@ -163,6 +177,20 @@ trait IssuesControllerBase extends ControllerBase { } getOrElse NotFound }) + /** + * https://developer.github.com/v3/issues/comments/#create-a-comment + */ + post("/api/v3/repos/:owner/:repository/issues/:id/comments")(readableUsersOnly { repository => + val data = multiParams.keys.headOption.flatMap(b => Try(parse(b).extract[CreateAComment]).toOption) + (for{ + issueId <- params("id").toIntOpt + (issue, id) <- handleComment(issueId, data.map(_.body), repository)() + issueComment <- getComment(repository.owner, repository.name, id.toString()) + } yield { + apiJson(WebHookComment(issueComment, WebHookApiUser(context.loginAccount.get))) + }) getOrElse NotFound + }) + post("/:owner/:repository/issue_comments/state", issueStateForm)(readableUsersOnly { (form, repository) => handleComment(form.issueId, form.content, repository)() map { case (issue, id) => redirect(s"/${repository.owner}/${repository.name}/${ diff --git a/src/main/scala/app/PullRequestsController.scala b/src/main/scala/app/PullRequestsController.scala index 83510f2b4..a828a1ccf 100644 --- a/src/main/scala/app/PullRequestsController.scala +++ b/src/main/scala/app/PullRequestsController.scala @@ -15,7 +15,7 @@ import service.PullRequestService._ import org.slf4j.LoggerFactory import org.eclipse.jgit.merge.MergeStrategy import org.eclipse.jgit.errors.NoMergeBaseException -import service.WebHookService.{ WebHookPayload, WebHookPullRequestPayload } +import service.WebHookService._ import util.JGitUtil.DiffInfo import util.JGitUtil.CommitInfo import model.{PullRequest, Issue} @@ -69,6 +69,24 @@ trait PullRequestsControllerBase extends ControllerBase { } }) + /** + * https://developer.github.com/v3/pulls/#list-pull-requests + */ + get("/api/v3/repos/:owner/:repository/pulls")(referrersOnly { repository => + val page = IssueSearchCondition.page(request) + // TODO: more api spec condition + val condition = IssueSearchCondition(request) + val baseOwner = getAccountByUserName(repository.owner).get + val issues:List[(model.Issue, model.Account, Int, model.PullRequest, model.Repository, model.Account)] = searchPullRequestByApi(condition, (page - 1) * PullRequestLimit, PullRequestLimit, repository.owner -> repository.name) + apiJson(issues.map{case (issue, issueUser, commentCount, pullRequest, headRepo, headOwner) => + WebHookPullRequest( + issue, + pullRequest, + WebHookRepository(headRepo, WebHookApiUser(headOwner)), + WebHookRepository(repository, WebHookApiUser(baseOwner)), + WebHookApiUser(issueUser)) }) + }) + get("/:owner/:repository/pull/:id")(referrersOnly { repository => params("id").toIntOpt.flatMap{ issueId => val owner = repository.owner @@ -95,6 +113,47 @@ trait PullRequestsControllerBase extends ControllerBase { } getOrElse NotFound }) + /** + * https://developer.github.com/v3/pulls/#get-a-single-pull-request + */ + get("/api/v3/repos/:owner/:repository/pulls/:id")(referrersOnly { repository => + (for{ + issueId <- params("id").toIntOpt + (issue, pullRequest) <- getPullRequest(repository.owner, repository.name, issueId) + users = getAccountsByUserNames(Set(repository.owner, pullRequest.requestUserName, issue.userName), Set()) + baseOwner <- users.get(repository.owner) + headOwner <- users.get(pullRequest.requestUserName) + issueUser <- users.get(issue.userName) + headRepo <- getRepository(pullRequest.requestUserName, pullRequest.requestRepositoryName, baseUrl) + } yield { + apiJson(WebHookPullRequest( + issue, + pullRequest, + WebHookRepository(headRepo, WebHookApiUser(headOwner)), + WebHookRepository(repository, WebHookApiUser(baseOwner)), + WebHookApiUser(issueUser))) + }).getOrElse(NotFound) + }) + + /** + * https://developer.github.com/v3/pulls/#list-commits-on-a-pull-request + */ + get("/api/v3/repos/:owner/:repository/pulls/:id/commits")(referrersOnly { repository => + val owner = repository.owner + val name = repository.name + params("id").toIntOpt.flatMap{ issueId => + getPullRequest(owner, name, issueId) map { case(issue, pullreq) => + using(Git.open(getRepositoryDir(owner, name))){ git => + val oldId = git.getRepository.resolve(pullreq.commitIdFrom) + val newId = git.getRepository.resolve(pullreq.commitIdTo) + val repoFullName = s"${owner}/${name}" + val commits = git.log.addRange(oldId, newId).call.iterator.asScala.map(c => WebHookCommitListItem(new CommitInfo(c), repoFullName)).toList + apiJson(commits) + } + } + } getOrElse NotFound + }) + ajaxGet("/:owner/:repository/pull/:id/mergeguide")(collaboratorsOnly { repository => params("id").toIntOpt.flatMap{ issueId => val owner = repository.owner diff --git a/src/main/scala/app/RepositoryViewerController.scala b/src/main/scala/app/RepositoryViewerController.scala index bc6339b28..de0b140a6 100644 --- a/src/main/scala/app/RepositoryViewerController.scala +++ b/src/main/scala/app/RepositoryViewerController.scala @@ -17,7 +17,7 @@ import org.eclipse.jgit.treewalk._ import jp.sf.amateras.scalatra.forms._ import org.eclipse.jgit.dircache.DirCache import org.eclipse.jgit.revwalk.RevCommit -import service.WebHookService.WebHookPushPayload +import service.WebHookService._ class RepositoryViewerController extends RepositoryViewerControllerBase with RepositoryService with AccountService with ActivityService with IssuesService with WebHookService with CommitsService @@ -105,6 +105,13 @@ trait RepositoryViewerControllerBase extends ControllerBase { fileList(_) }) + /** + * https://developer.github.com/v3/repos/#get + */ + get("/api/v3/repos/:owner/:repository")(referrersOnly { repository => + apiJson(WebHookRepository(repository, WebHookApiUser(getAccountByUserName(repository.owner).get))) + }) + /** * Displays the file list of the specified path and branch. */ diff --git a/src/main/scala/service/IssuesService.scala b/src/main/scala/service/IssuesService.scala index cf7ac0054..582677ebe 100644 --- a/src/main/scala/service/IssuesService.scala +++ b/src/main/scala/service/IssuesService.scala @@ -20,6 +20,12 @@ trait IssuesService { def getComments(owner: String, repository: String, issueId: Int)(implicit s: Session) = IssueComments filter (_.byIssue(owner, repository, issueId)) list + def getCommentsForApi(owner: String, repository: String, issueId: Int)(implicit s: Session) = + IssueComments.filter(_.byIssue(owner, repository, issueId)) + .filter(_.action inSetBind Set("commant" , "close_comment", "reopen_comment")) + .innerJoin(Accounts).on( (t1, t2) => t1.userName === t2.userName ) + .list + def getComment(owner: String, repository: String, commentId: String)(implicit s: Session) = if (commentId forall (_.isDigit)) IssueComments filter { t => @@ -92,21 +98,7 @@ trait IssuesService { (implicit s: Session): List[IssueInfo] = { // get issues and comment count and labels - searchIssueQuery(repos, condition, pullRequest) - .innerJoin(IssueOutline).on { (t1, t2) => t1.byIssue(t2.userName, t2.repositoryName, t2.issueId) } - .sortBy { case (t1, t2) => - (condition.sort match { - case "created" => t1.registeredDate - case "comments" => t2.commentCount - case "updated" => t1.updatedDate - }) match { - case sort => condition.direction match { - case "asc" => sort asc - case "desc" => sort desc - } - } - } - .drop(offset).take(limit) + searchIssueQueryBase(condition, pullRequest, offset, limit, repos) .leftJoin (IssueLabels) .on { case ((t1, t2), t3) => t1.byIssue(t3.userName, t3.repositoryName, t3.issueId) } .leftJoin (Labels) .on { case (((t1, t2), t3), t4) => t3.byLabel(t4.userName, t4.repositoryName, t4.labelId) } .leftJoin (Milestones) .on { case ((((t1, t2), t3), t4), t5) => t1.byMilestone(t5.userName, t5.repositoryName, t5.milestoneId) } @@ -130,6 +122,42 @@ trait IssuesService { }} toList } + /** for api + * @return (issue, commentCount, pullRequest, headRepository, headOwner) + */ + def searchPullRequestByApi(condition: IssueSearchCondition, offset: Int, limit: Int, repos: (String, String)*) + (implicit s: Session): List[(Issue, model.Account, Int, model.PullRequest, model.Repository, model.Account)] = { + // get issues and comment count and labels + searchIssueQueryBase(condition, true, offset, limit, repos) + .innerJoin(PullRequests).on { case ((t1, t2), t3) => t3.byPrimaryKey(t1.userName, t1.repositoryName, t1.issueId) } + .innerJoin(Repositories).on { case (((t1, t2), t3), t4) => t4.byRepository(t1.userName, t1.repositoryName) } + .innerJoin(Accounts).on { case ((((t1, t2), t3), t4), t5) => t5.userName === t1.userName } + .innerJoin(Accounts).on { case (((((t1, t2), t3), t4), t5), t6) => t6.userName === t4.userName } + .map { case (((((t1, t2), t3), t4), t5), t6) => + (t1, t5, t2.commentCount, t3, t4, t6) + } + .list + } + + private def searchIssueQueryBase(condition: IssueSearchCondition, pullRequest: Boolean, offset: Int, limit: Int, repos: Seq[(String, String)]) + (implicit s: Session) = + searchIssueQuery(repos, condition, pullRequest) + .innerJoin(IssueOutline).on { (t1, t2) => t1.byIssue(t2.userName, t2.repositoryName, t2.issueId) } + .sortBy { case (t1, t2) => + (condition.sort match { + case "created" => t1.registeredDate + case "comments" => t2.commentCount + case "updated" => t1.updatedDate + }) match { + case sort => condition.direction match { + case "asc" => sort asc + case "desc" => sort desc + } + } + } + .drop(offset).take(limit) + + /** * Assembles query for conditional issue searching. */ diff --git a/src/main/scala/service/WebHookService.scala b/src/main/scala/service/WebHookService.scala index 909f32ce4..1a91af9c9 100644 --- a/src/main/scala/service/WebHookService.scala +++ b/src/main/scala/service/WebHookService.scala @@ -167,13 +167,14 @@ object WebHookService { .getOrElse(throw new MappingException("Can't convert " + s + " to Date")) }, { case x: Date => JString(parserISO.print(new DateTime(x).withZone(DateTimeZone.UTC))) } ) - ) + FieldSerializer[WebHookApiUser]() + FieldSerializer[WebHookPullRequest]() + FieldSerializer[WebHookRepository]() + ) + FieldSerializer[WebHookApiUser]() + FieldSerializer[WebHookPullRequest]() + FieldSerializer[WebHookRepository]() + + FieldSerializer[WebHookCommitListItemParent]() + FieldSerializer[WebHookCommitListItem]() + FieldSerializer[WebHookCommitListItemCommit]() } def apiPathSerializer(c:ApiContext) = new CustomSerializer[ApiPath](format => ( { case JString(s) if s.startsWith(c.baseUrl) => ApiPath(s.substring(c.baseUrl.length)) - case JString(s) => throw new MappingException("Can't convert " + s + " to Date") + case JString(s) => throw new MappingException("Can't convert " + s + " to ApiPath") }, { case ApiPath(path) => JString(c.baseUrl+path) } ) @@ -293,14 +294,8 @@ object WebHookService { removed = diffs._1.collect { case x if(x.changeType == DiffEntry.ChangeType.DELETE) => x.oldPath }, modified = diffs._1.collect { case x if(x.changeType != DiffEntry.ChangeType.ADD && x.changeType != DiffEntry.ChangeType.DELETE) => x.newPath }, - author = WebHookCommitUser( - name = commit.authorName, - email = commit.authorEmailAddress - ), - committer = WebHookCommitUser( - name = commit.committerName, - email = commit.committerEmailAddress - ) + author = WebHookCommitUser.author(commit), + committer = WebHookCommitUser.committer(commit) ) } } @@ -336,7 +331,21 @@ object WebHookService { case class WebHookCommitUser( name: String, - email: String) + email: String, + date: Date) + + object WebHookCommitUser { + def author(commit: CommitInfo): WebHookCommitUser = + WebHookCommitUser( + name = commit.authorName, + email = commit.authorEmailAddress, + date = commit.authorTime) + def committer(commit: CommitInfo): WebHookCommitUser = + WebHookCommitUser( + name = commit.committerName, + email = commit.committerEmailAddress, + date = commit.commitTime) + } // https://developer.github.com/v3/repos/ case class WebHookRepository( @@ -417,7 +426,7 @@ object WebHookService { sha = pullRequest.commitIdFrom, ref = pullRequest.branch, repo = baseRepo), - mergeable = None, + mergeable = Some(true), // TODO: need check mergeable. title = issue.title, body = issue.content.getOrElse(""), user = user @@ -483,4 +492,42 @@ object WebHookService { created_at = comment.registeredDate, updated_at = comment.updatedDate) } + + // https://developer.github.com/v3/issues/comments/#create-a-comment + case class CreateAComment(body: String) + + + // https://developer.github.com/v3/repos/commits/ + case class WebHookCommitListItemParent(sha: String)(repoFullName:String){ + val url = ApiPath(s"/api/v3/repos/${repoFullName}/commits/${sha}") + } + case class WebHookCommitListItemCommit( + message: String, + author: WebHookCommitUser, + committer: WebHookCommitUser)(sha:String, repoFullName: String) { + val url = ApiPath(s"/api/v3/repos/${repoFullName}/git/commits/${sha}") + } + + case class WebHookCommitListItem( + sha: String, + commit: WebHookCommitListItemCommit, + author: Option[WebHookApiUser], + committer: Option[WebHookApiUser], + parents: Seq[WebHookCommitListItemParent])(repoFullName: String) { + val url = ApiPath(s"/api/v3/repos/${repoFullName}/commits/${sha}") + } + + object WebHookCommitListItem { + def apply(commit: CommitInfo, repoFullName:String): WebHookCommitListItem = WebHookCommitListItem( + sha = commit.id, + commit = WebHookCommitListItemCommit( + message = commit.fullMessage, + author = WebHookCommitUser.author(commit), + committer = WebHookCommitUser.committer(commit) + )(commit.id, repoFullName), + author = None, + committer = None, + parents = commit.parents.map(WebHookCommitListItemParent(_)(repoFullName)))(repoFullName) + } + } diff --git a/src/main/scala/util/Implicits.scala b/src/main/scala/util/Implicits.scala index f1b11157d..c9c2919ef 100644 --- a/src/main/scala/util/Implicits.scala +++ b/src/main/scala/util/Implicits.scala @@ -14,6 +14,8 @@ object Implicits { // Convert to slick session. implicit def request2Session(implicit request: HttpServletRequest): JdbcBackend#Session = Database.getSession(request) + implicit def context2ApiContext(implicit context: app.Context): service.WebHookService.ApiContext = service.WebHookService.ApiContext(context.baseUrl) + implicit class RichSeq[A](seq: Seq[A]) { def splitWith(condition: (A, A) => Boolean): Seq[Seq[A]] = split(seq)(condition) @@ -55,7 +57,10 @@ object Implicits { implicit class RichRequest(request: HttpServletRequest){ - def paths: Array[String] = request.getRequestURI.substring(request.getContextPath.length + 1).split("/") + def paths: Array[String] = (request.getRequestURI match{ + case path if path.startsWith("/api/v3/repos/") => path.substring(13/* "/api/v3/repos".length */) + case path => path + }).substring(request.getContextPath.length + 1).split("/") def hasQueryString: Boolean = request.getQueryString != null From 83bcbef6ce42377cb7047c82b737f70a2d84fd7e Mon Sep 17 00:00:00 2001 From: nazoking Date: Tue, 20 Jan 2015 13:15:33 +0900 Subject: [PATCH 18/31] move json extract logic to ControllerBase --- src/main/scala/app/ControllerBase.scala | 11 ++++++++++- src/main/scala/app/IssuesController.scala | 4 ++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/main/scala/app/ControllerBase.scala b/src/main/scala/app/ControllerBase.scala index 4a83dda9f..5bb1ca2e8 100644 --- a/src/main/scala/app/ControllerBase.scala +++ b/src/main/scala/app/ControllerBase.scala @@ -14,7 +14,7 @@ import service.{SystemSettingsService, AccountService, AccessTokenService} import javax.servlet.http.{HttpServletResponse, HttpServletRequest} import javax.servlet.{FilterChain, ServletResponse, ServletRequest} import org.scalatra.i18n._ - +import scala.util.Try /** * Provides generic features for controller implementations. */ @@ -151,6 +151,15 @@ abstract class ControllerBase extends ScalatraFilter response.addHeader("X-Content-Type-Options", "nosniff") rawData } + + // jenkins send message as 'application/x-www-form-urlencoded' but scalatra already parsed as multi-part-request. + def extractFromJsonBody[A](implicit request:HttpServletRequest, mf:Manifest[A]): Option[A] = { + (request.contentType.map(_.split(";").head.toLowerCase) match{ + case Some("application/x-www-form-urlencoded") => multiParams.keys.headOption.map(parse(_)) + case Some("application/json") => Some(parsedBody) + case _ => Some(parse(request.body)) + }).filterNot(_ == JNothing).flatMap(j => Try(j.extract[A]).toOption) + } } /** diff --git a/src/main/scala/app/IssuesController.scala b/src/main/scala/app/IssuesController.scala index af4dc16ea..e8d481f12 100644 --- a/src/main/scala/app/IssuesController.scala +++ b/src/main/scala/app/IssuesController.scala @@ -181,10 +181,10 @@ trait IssuesControllerBase extends ControllerBase { * https://developer.github.com/v3/issues/comments/#create-a-comment */ post("/api/v3/repos/:owner/:repository/issues/:id/comments")(readableUsersOnly { repository => - val data = multiParams.keys.headOption.flatMap(b => Try(parse(b).extract[CreateAComment]).toOption) (for{ issueId <- params("id").toIntOpt - (issue, id) <- handleComment(issueId, data.map(_.body), repository)() + body <- extractFromJsonBody[CreateAComment].map(_.body) if ! body.isEmpty + (issue, id) <- handleComment(issueId, Some(body), repository)() issueComment <- getComment(repository.owner, repository.name, id.toString()) } yield { apiJson(WebHookComment(issueComment, WebHookApiUser(context.loginAccount.get))) From 97ceffe689328c5ece1d4287320677b9b74f6b3d Mon Sep 17 00:00:00 2001 From: nazoking Date: Thu, 12 Feb 2015 20:17:40 +0900 Subject: [PATCH 19/31] add CommitStatus table and model and service. --- src/main/resources/update/2_9.sql | 29 +++++++ src/main/scala/model/BasicTemplate.scala | 3 + src/main/scala/model/CommitStatus.scala | 71 +++++++++++++++++ src/main/scala/model/Profile.scala | 1 + .../scala/service/CommitStatusService.scala | 49 ++++++++++++ .../scala/service/RepositoryService.scala | 2 + src/test/scala/model/CommitStateSpec.scala | 25 ++++++ .../service/CommitStateServiceSpec.scala | 77 +++++++++++++++++++ .../scala/service/RepositoryServiceSpec.scala | 37 +++++++++ 9 files changed, 294 insertions(+) create mode 100644 src/main/scala/model/CommitStatus.scala create mode 100644 src/main/scala/service/CommitStatusService.scala create mode 100644 src/test/scala/model/CommitStateSpec.scala create mode 100644 src/test/scala/service/CommitStateServiceSpec.scala create mode 100644 src/test/scala/service/RepositoryServiceSpec.scala diff --git a/src/main/resources/update/2_9.sql b/src/main/resources/update/2_9.sql index 7be56abf0..1ac303bb9 100644 --- a/src/main/resources/update/2_9.sql +++ b/src/main/resources/update/2_9.sql @@ -11,3 +11,32 @@ ALTER TABLE ACCESS_TOKEN ADD CONSTRAINT IDX_ACCESS_TOKEN_PK PRIMARY KEY (ACCESS_ ALTER TABLE ACCESS_TOKEN ADD CONSTRAINT IDX_ACCESS_TOKEN_FK0 FOREIGN KEY (USER_NAME) REFERENCES ACCOUNT (USER_NAME) ON DELETE CASCADE ON UPDATE CASCADE; ALTER TABLE ACCESS_TOKEN ADD CONSTRAINT IDX_ACCESS_TOKEN_TOKEN_HASH UNIQUE(TOKEN_HASH); + + +DROP TABLE IF EXISTS COMMIT_STATUS; +CREATE TABLE COMMIT_STATUS( + COMMIT_STATUS_ID INT AUTO_INCREMENT, + USER_NAME VARCHAR(100) NOT NULL, + REPOSITORY_NAME VARCHAR(100) NOT NULL, + COMMIT_ID VARCHAR(40) NOT NULL, + CONTEXT VARCHAR(255) NOT NULL, -- context is too long (maximum is 255 characters) + STATE VARCHAR(10) NOT NULL, -- pending, success, error, or failure + TARGET_URL VARCHAR(200), + DESCRIPTION TEXT, + CREATOR VARCHAR(100) NOT NULL, + REGISTERED_DATE TIMESTAMP NOT NULL, -- CREATED_AT + UPDATED_DATE TIMESTAMP NOT NULL -- UPDATED_AT +); +ALTER TABLE COMMIT_STATUS ADD CONSTRAINT IDX_COMMIT_STATUS_PK PRIMARY KEY (COMMIT_STATUS_ID); +ALTER TABLE COMMIT_STATUS ADD CONSTRAINT IDX_COMMIT_STATUS_1 + UNIQUE (USER_NAME, REPOSITORY_NAME, COMMIT_ID, CONTEXT); +ALTER TABLE COMMIT_STATUS ADD CONSTRAINT IDX_COMMIT_STATUS_FK1 + FOREIGN KEY (USER_NAME, REPOSITORY_NAME) + REFERENCES REPOSITORY (USER_NAME, REPOSITORY_NAME) + ON DELETE CASCADE ON UPDATE CASCADE; +ALTER TABLE COMMIT_STATUS ADD CONSTRAINT IDX_COMMIT_STATUS_FK2 + FOREIGN KEY (USER_NAME) REFERENCES ACCOUNT (USER_NAME) + ON DELETE CASCADE ON UPDATE CASCADE; +ALTER TABLE COMMIT_STATUS ADD CONSTRAINT IDX_COMMIT_STATUS_FK3 + FOREIGN KEY (CREATOR) REFERENCES ACCOUNT (USER_NAME) + ON DELETE CASCADE ON UPDATE CASCADE; diff --git a/src/main/scala/model/BasicTemplate.scala b/src/main/scala/model/BasicTemplate.scala index 52e0a112e..1d012c12a 100644 --- a/src/main/scala/model/BasicTemplate.scala +++ b/src/main/scala/model/BasicTemplate.scala @@ -49,6 +49,9 @@ protected[model] trait TemplateComponent { self: Profile => def byCommit(owner: String, repository: String, commitId: String) = byRepository(owner, repository) && (this.commitId === commitId) + + def byCommit(owner: Column[String], repository: Column[String], commitId: Column[String]) = + byRepository(userName, repositoryName) && (this.commitId === commitId) } } diff --git a/src/main/scala/model/CommitStatus.scala b/src/main/scala/model/CommitStatus.scala new file mode 100644 index 000000000..2977b262e --- /dev/null +++ b/src/main/scala/model/CommitStatus.scala @@ -0,0 +1,71 @@ +package model + +import scala.slick.lifted.MappedTo +import scala.slick.jdbc._ + +trait CommitStatusComponent extends TemplateComponent { self: Profile => + import profile.simple._ + import self._ + + implicit val commitStateColumnType = MappedColumnType.base[CommitState, String](b => b.name , i => CommitState(i)) + + lazy val CommitStatuses = TableQuery[CommitStatuses] + class CommitStatuses(tag: Tag) extends Table[CommitStatus](tag, "COMMIT_STATUS") with CommitTemplate { + val commitStatusId = column[Int]("COMMIT_STATUS_ID", O AutoInc) + val context = column[String]("CONTEXT") + val state = column[CommitState]("STATE") + val targetUrl = column[Option[String]]("TARGET_URL") + val description = column[Option[String]]("DESCRIPTION") + val creator = column[String]("CREATOR") + val registeredDate = column[java.util.Date]("REGISTERED_DATE") + val updatedDate = column[java.util.Date]("UPDATED_DATE") + def * = (commitStatusId, userName, repositoryName, commitId, context, state, targetUrl, description, creator, registeredDate, updatedDate) <> (CommitStatus.tupled, CommitStatus.unapply) + def byPrimaryKey(id: Int) = commitStatusId === id.bind + } +} + +case class CommitStatus( + commitStatusId: Int = 0, + userName: String, + repositoryName: String, + commitId: String, + context: String, + state: CommitState, + targetUrl: Option[String], + description: Option[String], + creator: String, + registeredDate: java.util.Date, + updatedDate: java.util.Date +) +sealed abstract class CommitState(val name: String) +object CommitState { + object ERROR extends CommitState("error") + object FAILURE extends CommitState("failure") + object PENDING extends CommitState("pending") + object SUCCESS extends CommitState("success") + + val values: Vector[CommitState] = Vector(PENDING, SUCCESS, ERROR, FAILURE) + private val map: Map[String, CommitState] = values.map(enum => enum.name -> enum).toMap + def apply(name: String): CommitState = map(name) + def valueOf(name: String): Option[CommitState] = map.get(name) + + /** + * failure if any of the contexts report as error or failure + * pending if there are no statuses or a context is pending + * success if the latest status for all contexts is success + */ + def combine(statuses: Set[CommitState]): CommitState = { + if(statuses.isEmpty){ + PENDING + }else if(statuses.contains(CommitState.ERROR) || statuses.contains(CommitState.FAILURE)){ + FAILURE + }else if(statuses.contains(CommitState.PENDING)){ + PENDING + }else{ + SUCCESS + } + } + + implicit val getResult: GetResult[CommitState] = GetResult(r => CommitState(r.<<)) + implicit val getResultOpt: GetResult[Option[CommitState]] = GetResult(r => r.< t.byCommit(userName, repositoryName, sha) && t.context===context.bind ) + .map(_.commitStatusId).firstOption match { + case Some(id:Int) => { + CommitStatuses.filter(_.byPrimaryKey(id)).map{ + t => (t.state , t.targetUrl , t.updatedDate , t.creator, t.description) + }.update( (state, targetUrl, now, creator.userName, description) ) + id + } + case None => (CommitStatuses returning CommitStatuses.map(_.commitStatusId)) += CommitStatus( + userName = userName, + repositoryName = repositoryName, + commitId = sha, + context = context, + state = state, + targetUrl = targetUrl, + description = description, + creator = creator.userName, + registeredDate = now, + updatedDate = now) + } + + def getCommitStatus(userName: String, repositoryName: String, id: Int)(implicit s: Session) :Option[CommitStatus] = + CommitStatuses.filter(t => t.byPrimaryKey(id) && t.byRepository(userName, repositoryName)).firstOption + + def getCommitStatus(userName: String, repositoryName: String, sha: String, context: String)(implicit s: Session) :Option[CommitStatus] = + CommitStatuses.filter(t => t.byCommit(userName, repositoryName, sha) && t.context===context.bind ).firstOption + + def getCommitStatues(userName: String, repositoryName: String, sha: String)(implicit s: Session) :List[CommitStatus] = + byCommitStatues(userName, repositoryName, sha).list + + def getCommitStatuesWithCreator(userName: String, repositoryName: String, sha: String)(implicit s: Session) :List[(CommitStatus, Account)] = + byCommitStatues(userName, repositoryName, sha).innerJoin(Accounts) + .filter{ case (t,a) => t.creator === a.userName }.list + + protected def byCommitStatues(userName: String, repositoryName: String, sha: String)(implicit s: Session) = + CommitStatuses.filter(t => t.byCommit(userName, repositoryName, sha) ).sortBy(_.updatedDate desc) +} \ No newline at end of file diff --git a/src/main/scala/service/RepositoryService.scala b/src/main/scala/service/RepositoryService.scala index f54291b29..bae3a715b 100644 --- a/src/main/scala/service/RepositoryService.scala +++ b/src/main/scala/service/RepositoryService.scala @@ -55,6 +55,7 @@ trait RepositoryService { self: AccountService => val issueComments = IssueComments .filter(_.byRepository(oldUserName, oldRepositoryName)).list val issueLabels = IssueLabels .filter(_.byRepository(oldUserName, oldRepositoryName)).list val commitComments = CommitComments.filter(_.byRepository(oldUserName, oldRepositoryName)).list + val commitStatuses = CommitStatuses.filter(_.byRepository(oldUserName, oldRepositoryName)).list val collaborators = Collaborators .filter(_.byRepository(oldUserName, oldRepositoryName)).list Repositories.filter { t => @@ -95,6 +96,7 @@ trait RepositoryService { self: AccountService => IssueComments .insertAll(issueComments .map(_.copy(userName = newUserName, repositoryName = newRepositoryName)) :_*) Labels .insertAll(labels .map(_.copy(userName = newUserName, repositoryName = newRepositoryName)) :_*) CommitComments.insertAll(commitComments.map(_.copy(userName = newUserName, repositoryName = newRepositoryName)) :_*) + CommitStatuses.insertAll(commitStatuses.map(_.copy(userName = newUserName, repositoryName = newRepositoryName)) :_*) // Convert labelId val oldLabelMap = labels.map(x => (x.labelId, x.labelName)).toMap diff --git a/src/test/scala/model/CommitStateSpec.scala b/src/test/scala/model/CommitStateSpec.scala new file mode 100644 index 000000000..a744d8304 --- /dev/null +++ b/src/test/scala/model/CommitStateSpec.scala @@ -0,0 +1,25 @@ +package model + +import org.specs2.mutable.Specification + +import CommitState._ + +class CommitStateSpec extends Specification { + "CommitState" should { + "combine empty must eq PENDING" in { + combine(Set()) must_== PENDING + } + "combine includes ERROR must eq FAILURE" in { + combine(Set(ERROR, SUCCESS, PENDING)) must_== FAILURE + } + "combine includes FAILURE must eq peinding" in { + combine(Set(FAILURE, SUCCESS, PENDING)) must_== FAILURE + } + "combine includes PENDING must eq peinding" in { + combine(Set(PENDING, SUCCESS)) must_== PENDING + } + "combine only SUCCESS must eq SUCCESS" in { + combine(Set(SUCCESS)) must_== SUCCESS + } + } +} diff --git a/src/test/scala/service/CommitStateServiceSpec.scala b/src/test/scala/service/CommitStateServiceSpec.scala new file mode 100644 index 000000000..24ea41d1e --- /dev/null +++ b/src/test/scala/service/CommitStateServiceSpec.scala @@ -0,0 +1,77 @@ +package service +import org.specs2.mutable.Specification +import java.util.Date +import model._ +import model.Profile._ +import profile.simple._ +class CommitStatusServiceSpec extends Specification with ServiceSpecBase with CommitStatusService + with RepositoryService with AccountService{ + def generateNewAccount(name:String)(implicit s:Session):Account = { + createAccount(name, name, name, s"${name}@example.com", false, None) + getAccountByUserName(name).get + } + val now = new java.util.Date() + val fixture1 = CommitStatus( + userName = "root", + repositoryName = "repo", + commitId = "0e97b8f59f7cdd709418bb59de53f741fd1c1bd7", + context = "jenkins/test", + creator = "tester", + state = CommitState.PENDING, + targetUrl = Some("http://example.com/target"), + description = Some("description"), + updatedDate = now, + registeredDate = now) + def findById(id: Int)(implicit s:Session) = CommitStatuses.filter(_.byPrimaryKey(id)).firstOption + def generateFixture1(tester:Account)(implicit s:Session) = createCommitStatus( + userName = fixture1.userName, + repositoryName = fixture1.repositoryName, + sha = fixture1.commitId, + context = fixture1.context, + state = fixture1.state, + targetUrl = fixture1.targetUrl, + description = fixture1.description, + creator = tester, + now = fixture1.registeredDate) + "CommitStatusService" should { + "createCommitState can insert and update" in { withTestDB { implicit session => + val tester = generateNewAccount(fixture1.creator) + createRepository(fixture1.repositoryName,fixture1.userName,None,false) + val id = generateFixture1(tester:Account) + getCommitStatus(fixture1.userName, fixture1.repositoryName, id) must_== + Some(fixture1.copy(commitStatusId=id)) + // other one can update + val tester2 = generateNewAccount("tester2") + val time2 = new java.util.Date(); + val id2 = createCommitStatus( + userName = fixture1.userName, + repositoryName = fixture1.repositoryName, + sha = fixture1.commitId, + context = fixture1.context, + state = CommitState.SUCCESS, + targetUrl = Some("http://example.com/target2"), + description = Some("description2"), + creator = tester2, + now = time2) + getCommitStatus(fixture1.userName, fixture1.repositoryName, id2) must_== Some(fixture1.copy( + commitStatusId = id, + creator = "tester2", + state = CommitState.SUCCESS, + targetUrl = Some("http://example.com/target2"), + description = Some("description2"), + updatedDate = time2)) + }} + "getCommitStatus can find by commitId and context" in { withTestDB { implicit session => + val tester = generateNewAccount(fixture1.creator) + createRepository(fixture1.repositoryName,fixture1.userName,None,false) + val id = generateFixture1(tester:Account) + getCommitStatus(fixture1.userName, fixture1.repositoryName, fixture1.commitId, fixture1.context) must_== Some(fixture1.copy(commitStatusId=id)) + }} + "getCommitStatus can find by commitStatusId" in { withTestDB { implicit session => + val tester = generateNewAccount(fixture1.creator) + createRepository(fixture1.repositoryName,fixture1.userName,None,false) + val id = generateFixture1(tester:Account) + getCommitStatus(fixture1.userName, fixture1.repositoryName, id) must_== Some(fixture1.copy(commitStatusId=id)) + }} + } +} \ No newline at end of file diff --git a/src/test/scala/service/RepositoryServiceSpec.scala b/src/test/scala/service/RepositoryServiceSpec.scala new file mode 100644 index 000000000..b3a5faa76 --- /dev/null +++ b/src/test/scala/service/RepositoryServiceSpec.scala @@ -0,0 +1,37 @@ +package service +import org.specs2.mutable.Specification +import java.util.Date +import model._ +import model.Profile._ +import profile.simple._ +class RepositoryServiceSpec extends Specification with ServiceSpecBase with RepositoryService with AccountService{ + def generateNewAccount(name:String)(implicit s:Session):Account = { + createAccount(name, name, name, s"${name}@example.com", false, None) + getAccountByUserName(name).get + } + "RepositoryService" should { + "renameRepository can rename CommitState" in { withTestDB { implicit session => + val tester = generateNewAccount("tester") + createRepository("repo","root",None,false) + val commitStatusService = new CommitStatusService{} + val id = commitStatusService.createCommitStatus( + userName = "root", + repositoryName = "repo", + sha = "0e97b8f59f7cdd709418bb59de53f741fd1c1bd7", + context = "jenkins/test", + state = CommitState.PENDING, + targetUrl = Some("http://example.com/target"), + description = Some("description"), + creator = tester, + now = new java.util.Date) + val org = commitStatusService.getCommitStatus("root","repo", id).get + renameRepository("root","repo","tester","repo2") + val neo = commitStatusService.getCommitStatus("tester","repo2", org.commitId, org.context).get + neo must_== + org.copy( + commitStatusId=neo.commitStatusId, + repositoryName="repo2", + userName="tester") + }} + } +} From 0299cee5ec7521ebb785805f96504fa9fd51b9ca Mon Sep 17 00:00:00 2001 From: nazoking Date: Thu, 12 Feb 2015 20:20:27 +0900 Subject: [PATCH 20/31] add CommitStatus api and views. --- .../scala/app/PullRequestsController.scala | 16 +++- .../app/RepositoryViewerController.scala | 55 ++++++++++++- src/main/scala/service/WebHookService.scala | 80 ++++++++++++++++++- src/main/scala/util/JGitUtil.scala | 13 +++ src/main/scala/view/helpers.scala | 14 ++++ src/main/twirl/pulls/conversation.scala.html | 25 ++---- src/main/twirl/pulls/mergeguide.scala.html | 74 ++++++++++++++++- .../webapp/assets/common/css/gitbucket.css | 18 +++++ 8 files changed, 263 insertions(+), 32 deletions(-) diff --git a/src/main/scala/app/PullRequestsController.scala b/src/main/scala/app/PullRequestsController.scala index a828a1ccf..1a3a2cae7 100644 --- a/src/main/scala/app/PullRequestsController.scala +++ b/src/main/scala/app/PullRequestsController.scala @@ -18,16 +18,18 @@ import org.eclipse.jgit.errors.NoMergeBaseException import service.WebHookService._ import util.JGitUtil.DiffInfo import util.JGitUtil.CommitInfo -import model.{PullRequest, Issue} +import model.{PullRequest, Issue, CommitState} class PullRequestsController extends PullRequestsControllerBase with RepositoryService with AccountService with IssuesService with PullRequestService with MilestonesService with LabelsService with CommitsService with ActivityService with WebHookPullRequestService with ReferrerAuthenticator with CollaboratorsAuthenticator + with CommitStatusService trait PullRequestsControllerBase extends ControllerBase { self: RepositoryService with AccountService with IssuesService with MilestonesService with LabelsService - with CommitsService with ActivityService with PullRequestService with WebHookPullRequestService with ReferrerAuthenticator with CollaboratorsAuthenticator => + with CommitsService with ActivityService with PullRequestService with WebHookPullRequestService with ReferrerAuthenticator with CollaboratorsAuthenticator + with CommitStatusService => private val logger = LoggerFactory.getLogger(classOf[PullRequestsControllerBase]) @@ -95,7 +97,6 @@ trait PullRequestsControllerBase extends ControllerBase { using(Git.open(getRepositoryDir(owner, name))){ git => val (commits, diffs) = getRequestCompareInfo(owner, name, pullreq.commitIdFrom, owner, name, pullreq.commitIdTo) - pulls.html.pullreq( issue, pullreq, (commits.flatten.map(commit => getCommitComments(owner, name, commit.id, true)).flatten.toList ::: getComments(owner, name, issueId)) @@ -159,9 +160,16 @@ trait PullRequestsControllerBase extends ControllerBase { val owner = repository.owner val name = repository.name getPullRequest(owner, name, issueId) map { case(issue, pullreq) => + val statuses = getCommitStatues(owner, name, pullreq.commitIdTo) + val hasConfrict = checkConflictInPullRequest(owner, name, pullreq.branch, pullreq.requestUserName, name, pullreq.requestBranch, issueId) + val hasProblem = hasConfrict || (!statuses.isEmpty && CommitState.combine(statuses.map(_.state).toSet) != CommitState.SUCCESS) pulls.html.mergeguide( - checkConflictInPullRequest(owner, name, pullreq.branch, pullreq.requestUserName, name, pullreq.requestBranch, issueId), + hasConfrict, + hasProblem, + issue, pullreq, + statuses, + repository, s"${context.baseUrl}/git/${pullreq.requestUserName}/${pullreq.requestRepositoryName}.git") } } getOrElse NotFound diff --git a/src/main/scala/app/RepositoryViewerController.scala b/src/main/scala/app/RepositoryViewerController.scala index de0b140a6..3ec28e359 100644 --- a/src/main/scala/app/RepositoryViewerController.scala +++ b/src/main/scala/app/RepositoryViewerController.scala @@ -18,10 +18,11 @@ import jp.sf.amateras.scalatra.forms._ import org.eclipse.jgit.dircache.DirCache import org.eclipse.jgit.revwalk.RevCommit import service.WebHookService._ +import model.CommitState class RepositoryViewerController extends RepositoryViewerControllerBase with RepositoryService with AccountService with ActivityService with IssuesService with WebHookService with CommitsService - with ReadableUsersAuthenticator with ReferrerAuthenticator with CollaboratorsAuthenticator with PullRequestService + with ReadableUsersAuthenticator with ReferrerAuthenticator with CollaboratorsAuthenticator with PullRequestService with CommitStatusService /** @@ -29,7 +30,7 @@ class RepositoryViewerController extends RepositoryViewerControllerBase */ trait RepositoryViewerControllerBase extends ControllerBase { self: RepositoryService with AccountService with ActivityService with IssuesService with WebHookService with CommitsService - with ReadableUsersAuthenticator with ReferrerAuthenticator with CollaboratorsAuthenticator with PullRequestService => + with ReadableUsersAuthenticator with ReferrerAuthenticator with CollaboratorsAuthenticator with PullRequestService with CommitStatusService => ArchiveCommand.registerFormat("zip", new ZipFormat) ArchiveCommand.registerFormat("tar.gz", new TgzFormat) @@ -143,6 +144,56 @@ trait RepositoryViewerControllerBase extends ControllerBase { } }) + /** + * https://developer.github.com/v3/repos/statuses/#create-a-status + */ + post("/api/v3/repos/:owner/:repo/statuses/:sha")(collaboratorsOnly { repository => + (for{ + ref <- params.get("sha") + sha <- JGitUtil.getShaByRef(repository.owner, repository.name, ref) + data <- extractFromJsonBody[CreateAStatus] if data.isValid + creator <- context.loginAccount + state <- model.CommitState.valueOf(data.state) + statusId = createCommitStatus(repository.owner, repository.name, sha, data.context.getOrElse("default"), + state, data.target_url, data.description, new java.util.Date(), creator) + status <- getCommitStatus(repository.owner, repository.name, statusId) + } yield { + apiJson(WebHookCommitStatus(status, WebHookApiUser(creator))) + }) getOrElse NotFound + }) + + /** + * https://developer.github.com/v3/repos/statuses/#list-statuses-for-a-specific-ref + * + * ref is Ref to list the statuses from. It can be a SHA, a branch name, or a tag name. + */ + get("/api/v3/repos/:owner/:repo/commits/:ref/statuses")(referrersOnly { repository => + (for{ + ref <- params.get("ref") + sha <- JGitUtil.getShaByRef(repository.owner, repository.name, ref) + } yield { + apiJson(getCommitStatuesWithCreator(repository.owner, repository.name, sha).map{ case(status, creator) => + WebHookCommitStatus(status, WebHookApiUser(creator)) + }) + }) getOrElse NotFound + }) + + /** + * https://developer.github.com/v3/repos/statuses/#get-the-combined-status-for-a-specific-ref + * + * ref is Ref to list the statuses from. It can be a SHA, a branch name, or a tag name. + */ + get("/api/v3/repos/:owner/:repo/commits/:ref/status")(referrersOnly { repository => + (for{ + ref <- params.get("ref") + owner <- getAccountByUserName(repository.owner) + sha <- JGitUtil.getShaByRef(repository.owner, repository.name, ref) + } yield { + val statuses = getCommitStatuesWithCreator(repository.owner, repository.name, sha) + apiJson(WebHookCombinedCommitStatus(sha, statuses, WebHookRepository(repository, owner))) + }) getOrElse NotFound + }) + get("/:owner/:repository/new/*")(collaboratorsOnly { repository => val (branch, path) = splitPath(repository, multiParams("splat").head) repo.html.editor(branch, repository, if(path.length == 0) Nil else path.split("/").toList, diff --git a/src/main/scala/service/WebHookService.scala b/src/main/scala/service/WebHookService.scala index 1a91af9c9..25dae0bbc 100644 --- a/src/main/scala/service/WebHookService.scala +++ b/src/main/scala/service/WebHookService.scala @@ -2,7 +2,7 @@ package service import model.Profile._ import profile.simple._ -import model.{WebHook, Account, Issue, PullRequest, IssueComment, Repository} +import model.{WebHook, Account, Issue, PullRequest, IssueComment, Repository, CommitStatus, CommitState} import org.slf4j.LoggerFactory import service.RepositoryService.RepositoryInfo import util.JGitUtil @@ -168,7 +168,8 @@ object WebHookService { { case x: Date => JString(parserISO.print(new DateTime(x).withZone(DateTimeZone.UTC))) } ) ) + FieldSerializer[WebHookApiUser]() + FieldSerializer[WebHookPullRequest]() + FieldSerializer[WebHookRepository]() + - FieldSerializer[WebHookCommitListItemParent]() + FieldSerializer[WebHookCommitListItem]() + FieldSerializer[WebHookCommitListItemCommit]() + FieldSerializer[WebHookCommitListItemParent]() + FieldSerializer[WebHookCommitListItem]() + FieldSerializer[WebHookCommitListItemCommit]() + + FieldSerializer[WebHookCommitStatus]() + FieldSerializer[WebHookCombinedCommitStatus]() } def apiPathSerializer(c:ApiContext) = new CustomSerializer[ApiPath](format => ( @@ -410,7 +411,7 @@ object WebHookService { //val review_comments_url = ApiPath("${base.repo.url.path}/pulls/${number}/comments") //val review_comment_url = ApiPath("${base.repo.url.path}/pulls/comments/{number}") //val comments_url = ApiPath("${base.repo.url.path}/issues/${number}/comments") - //val statuses_url = ApiPath("${base.repo.url.path}/statuses/${head.sha}") + val statuses_url = ApiPath(s"${base.repo.url.path}/statuses/${head.sha}") } object WebHookPullRequest{ @@ -426,7 +427,7 @@ object WebHookService { sha = pullRequest.commitIdFrom, ref = pullRequest.branch, repo = baseRepo), - mergeable = Some(true), // TODO: need check mergeable. + mergeable = None, // TODO: need check mergeable. title = issue.title, body = issue.content.getOrElse(""), user = user @@ -530,4 +531,75 @@ object WebHookService { parents = commit.parents.map(WebHookCommitListItemParent(_)(repoFullName)))(repoFullName) } + /** + * https://developer.github.com/v3/repos/statuses/#create-a-status + */ + case class CreateAStatus( + /* state is Required. The state of the status. Can be one of pending, success, error, or failure. */ + state: String, + /* context is a string label to differentiate this status from the status of other systems. Default: "default" */ + context: Option[String], + /* The target URL to associate with this status. This URL will be linked from the GitHub UI to allow users to easily see the ‘source’ of the Status. */ + target_url: Option[String], + /* description is a short description of the status.*/ + description: Option[String] + ) { + def isValid: Boolean = { + CommitState.valueOf(state).isDefined && + target_url.filterNot(f => "\\Ahttps?://".r.findPrefixOf(f).isDefined && f.length<255).isEmpty && + context.filterNot(f => f.length<255).isEmpty && + description.filterNot(f => f.length<1000).isEmpty + } + } + + /** + * https://developer.github.com/v3/repos/statuses/#create-a-status + * https://developer.github.com/v3/repos/statuses/#list-statuses-for-a-specific-ref + */ + case class WebHookCommitStatus( + created_at: Date, + updated_at: Date, + state: String, + target_url: Option[String], + description: Option[String], + id: Int, + context: String, + creator: WebHookApiUser + )(sha: String, repoFullName: String) { + def url = ApiPath(s"/api/v3/repos/${repoFullName}/commits/${sha}/statuses") + } + + object WebHookCommitStatus { + def apply(status: CommitStatus, creator:WebHookApiUser): WebHookCommitStatus = WebHookCommitStatus( + created_at = status.registeredDate, + updated_at = status.updatedDate, + state = status.state.name, + target_url = status.targetUrl, + description= status.description, + id = status.commitStatusId, + context = status.context, + creator = creator + )(status.commitId, s"${status.userName}/${status.repositoryName}") + } + + /** + * https://developer.github.com/v3/repos/statuses/#get-the-combined-status-for-a-specific-ref + */ + case class WebHookCombinedCommitStatus( + state: String, + sha: String, + total_count: Int, + statuses: Iterable[WebHookCommitStatus], + repository: WebHookRepository){ + // val commit_url = ApiPath(s"/api/v3/repos/${repository.full_name}/${sha}") + val url = ApiPath(s"/api/v3/repos/${repository.full_name}/commits/${sha}/status") + } + object WebHookCombinedCommitStatus { + def apply(sha:String, statuses: Iterable[(CommitStatus, Account)], repository:WebHookRepository): WebHookCombinedCommitStatus = WebHookCombinedCommitStatus( + state = CommitState.combine(statuses.map(_._1.state).toSet).name, + sha = sha, + total_count= statuses.size, + statuses = statuses.map{ case (s, a)=> WebHookCommitStatus(s, WebHookApiUser(a)) }, + repository = repository) + } } diff --git a/src/main/scala/util/JGitUtil.scala b/src/main/scala/util/JGitUtil.scala index 5c7be3fe1..47e6994ad 100644 --- a/src/main/scala/util/JGitUtil.scala +++ b/src/main/scala/util/JGitUtil.scala @@ -748,4 +748,17 @@ object JGitUtil { } } } + + /** + * Returns sha1 + * @param owner repository owner + * @param name repository name + * @param revstr A git object references expression + * @return sha1 + */ + def getShaByRef(owner:String, name:String,revstr: String): Option[String] = { + using(Git.open(getRepositoryDir(owner, name))){ git => + Option(git.getRepository.resolve(revstr)).map(ObjectId.toString(_)) + } + } } diff --git a/src/main/scala/view/helpers.scala b/src/main/scala/view/helpers.scala index caea648e2..1f2b406ac 100644 --- a/src/main/scala/view/helpers.scala +++ b/src/main/scala/view/helpers.scala @@ -4,6 +4,7 @@ import java.text.SimpleDateFormat import play.twirl.api.Html import util.StringUtil import service.RequestCache +import model.CommitState /** * Provides helper methods for Twirl templates. @@ -260,4 +261,17 @@ object helpers extends AvatarImageProvider with LinkConverter with RequestCache def mkHtml(separator: scala.xml.Elem) = Html(seq.mkString(separator.toString)) } + def commitStateIcon(state: CommitState) = Html(state match { + case CommitState.PENDING => "●" + case CommitState.SUCCESS => "✔" + case CommitState.ERROR => "×" + case CommitState.FAILURE => "×" + }) + + def commitStateText(state: CommitState, commitId:String) = state match { + case CommitState.PENDING => "Waiting to hear about "+commitId.substring(0,8) + case CommitState.SUCCESS => "All is well" + case CommitState.ERROR => "Failed" + case CommitState.FAILURE => "Failed" + } } diff --git a/src/main/twirl/pulls/conversation.scala.html b/src/main/twirl/pulls/conversation.scala.html index 33c923cea..82a89823c 100644 --- a/src/main/twirl/pulls/conversation.scala.html +++ b/src/main/twirl/pulls/conversation.scala.html @@ -8,7 +8,7 @@ hasWritePermission: Boolean, repository: service.RepositoryService.RepositoryInfo)(implicit context: app.Context) @import context._ -@import model.IssueComment +@import model.{IssueComment, CommitState} @import view.helpers._
    @@ -20,25 +20,10 @@ case other => None }.exists(_.action == "merge")){ merged => @if(hasWritePermission && !issue.closed){ -
    -
    -
    - -
    - +
    All branches
    @branch.mergeInfo.map{ info => - Compare + @prs.map{ case (pull, issue) => + #@issue.issueId + @if(issue.closed) { + @if(info.isMerged){ + Merged + }else{ + Closed + } + } else { + Open + } + }.getOrElse{ + Compare + } @if(hasWritePermission){ - + @if(prs.map(!_._2.closed).getOrElse(false)){ + + }else{ + + } } }
    @@ -57,63 +74,6 @@ $(function(){ var branchName = $(e.target).data('name'); return confirm('Are you sure you want to remove the ' + branchName + ' branch?'); }); - $('*[data-toggle=tooltip]').tooltip(); + $('*[data-toggle=tooltip]').tooltip().css("white-space","nowrap"); }); - diff --git a/src/main/webapp/assets/common/css/gitbucket.css b/src/main/webapp/assets/common/css/gitbucket.css index 0178bddbb..c2473bc74 100644 --- a/src/main/webapp/assets/common/css/gitbucket.css +++ b/src/main/webapp/assets/common/css/gitbucket.css @@ -472,6 +472,62 @@ div.repository-content { margin-left: 40px; } +.branches .muted-link{ + color: #777; +} +.branches .muted-link:hover{ + color: #4183c4; +} +.branches .branch-details{ + display: inline-block; + width: 490px; + margin-right: 10px; +} +.branches .branch-name{ + color: #4183c4; + display: inline-block; + padding: 2px 6px; + font: 12px Consolas, "Liberation Mono", Menlo, Courier, monospace; + background-color: rgba(209,227,237,0.5); + border-radius: 3px; +} +.branches .branch-meta{ + color: #aaa; + font-size: 12px; + line-height: 20px; +} +.branches .branch-action{ + display: inline-block; + float: right; + position: relative; + top: -3px; + height: 20px; +} +.branches .branch-a-b-count{ + display: inline-block; + vertical-align: middle; + width: 181px; + text-align: center; + color: rgba(0,0,0,0.5); +} +.branches .a-b-count-widget{ + font-size: 10px; +} +.branches .a-b-count-widget .count-half{ + width:90px; + position: relative; + text-align: right; + float: left; +} +.branches .a-b-count-widget .count-half:last-child { + text-align: left; + border-left: 1px solid #bbb; +} +.branches .a-b-count-widget .count-half .count-value{ + padding: 0 3px; +} + + /****************************************************************************/ /* Activity */ /****************************************************************************/ From a417c373f180c0386d4ffd6497c6130df35e01b4 Mon Sep 17 00:00:00 2001 From: nazoking Date: Fri, 30 Jan 2015 00:55:02 +0900 Subject: [PATCH 03/31] 'New Pull Request' button if you logined. --- src/main/twirl/pulls/compare.scala.html | 3 +++ src/main/twirl/repo/branches.scala.html | 6 +++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/main/twirl/pulls/compare.scala.html b/src/main/twirl/pulls/compare.scala.html index b3f24832b..b606715bd 100644 --- a/src/main/twirl/pulls/compare.scala.html +++ b/src/main/twirl/pulls/compare.scala.html @@ -126,6 +126,9 @@ $(function(){ $(this).hide(); $('#pull-request-form').show(); }); + if(location.search.substr(1).split("&").indexOf("expand=1")!=-1){ + $('#show-form').click(); + } @if(hasWritePermission){ function checkConflict(from, to, noConflictHandler, hasConflictHandler){ diff --git a/src/main/twirl/repo/branches.scala.html b/src/main/twirl/repo/branches.scala.html index e91208e93..7f152f4ad 100644 --- a/src/main/twirl/repo/branches.scala.html +++ b/src/main/twirl/repo/branches.scala.html @@ -29,7 +29,11 @@ Open } }.getOrElse{ - Compare + @if(context.loginAccount.isDefined){ + New Pull Request + }else{ + Compare + } } @if(hasWritePermission){ @if(prs.map(!_._2.closed).getOrElse(false)){ From 4a948d9b013b20776989ea05a0007f9f0468e307 Mon Sep 17 00:00:00 2001 From: Naoki Takezoe Date: Tue, 24 Feb 2015 00:10:06 +0900 Subject: [PATCH 04/31] Fix monospace style --- src/main/webapp/assets/common/css/gitbucket.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/webapp/assets/common/css/gitbucket.css b/src/main/webapp/assets/common/css/gitbucket.css index 0178bddbb..ff180c8bf 100644 --- a/src/main/webapp/assets/common/css/gitbucket.css +++ b/src/main/webapp/assets/common/css/gitbucket.css @@ -166,7 +166,7 @@ span.count-right { } .monospace { - font-family: monospace; + font-family: Consolas, 'Courier New', Courier, Monaco, monospace; } table.table th { From c2f1817c6a0adf22b87ca6b092709a05e56254af Mon Sep 17 00:00:00 2001 From: Naoki Takezoe Date: Wed, 25 Feb 2015 21:42:22 +0900 Subject: [PATCH 05/31] Avoid exception when filter box is empty --- src/main/scala/app/IssuesController.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/scala/app/IssuesController.scala b/src/main/scala/app/IssuesController.scala index 682ba9e15..b52b22378 100644 --- a/src/main/scala/app/IssuesController.scala +++ b/src/main/scala/app/IssuesController.scala @@ -394,7 +394,7 @@ trait IssuesControllerBase extends ControllerBase { val condition = session.putAndGet(sessionKey, if(request.hasQueryString){ val q = request.getParameter("q") - if(q == null){ + if(q == null || q.trim.isEmpty){ IssueSearchCondition(request) } else { IssueSearchCondition(q, getMilestones(owner, repoName).map(x => (x.title, x.milestoneId)).toMap) From e67365a19f58b0034c23e02fe48463aaef30ac9a Mon Sep 17 00:00:00 2001 From: Naoki Takezoe Date: Thu, 26 Feb 2015 11:20:41 +0900 Subject: [PATCH 06/31] Fix problem about milestone in issues and pull requests --- src/main/scala/service/IssuesService.scala | 25 +++++++++++-------- .../twirl/dashboard/issueslist.scala.html | 2 +- src/main/twirl/issues/listparts.scala.html | 14 +++++------ 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/main/scala/service/IssuesService.scala b/src/main/scala/service/IssuesService.scala index 8740d9e99..cf7ac0054 100644 --- a/src/main/scala/service/IssuesService.scala +++ b/src/main/scala/service/IssuesService.scala @@ -139,11 +139,16 @@ trait IssuesService { .map { case (owner, repository) => t1.byRepository(owner, repository) } .foldLeft[Column[Boolean]](false) ( _ || _ ) && (t1.closed === (condition.state == "closed").bind) && - (t1.milestoneId === condition.milestoneId.get.get.bind, condition.milestoneId.flatten.isDefined) && - (t1.milestoneId.? isEmpty, condition.milestoneId == Some(None)) && + //(t1.milestoneId === condition.milestoneId.get.get.bind, condition.milestoneId.flatten.isDefined) && + (t1.milestoneId.? isEmpty, condition.milestone == Some(None)) && (t1.assignedUserName === condition.assigned.get.bind, condition.assigned.isDefined) && (t1.openedUserName === condition.author.get.bind, condition.author.isDefined) && (t1.pullRequest === pullRequest.bind) && + // Milestone filter + (Milestones filter { t2 => + (t2.byPrimaryKey(t1.userName, t1.repositoryName, t1.milestoneId)) && + (t2.title === condition.milestone.get.get.bind) + } exists, condition.milestone.flatten.isDefined) && // Label filter (IssueLabels filter { t2 => (t2.byIssue(t1.userName, t1.repositoryName, t1.issueId)) && @@ -322,7 +327,7 @@ object IssuesService { case class IssueSearchCondition( labels: Set[String] = Set.empty, - milestoneId: Option[Option[Int]] = None, + milestone: Option[Option[String]] = None, author: Option[String] = None, assigned: Option[String] = None, mentioned: Option[String] = None, @@ -333,7 +338,7 @@ object IssuesService { groups: Set[String] = Set.empty){ def isEmpty: Boolean = { - labels.isEmpty && milestoneId.isEmpty && author.isEmpty && assigned.isEmpty && + labels.isEmpty && milestone.isEmpty && author.isEmpty && assigned.isEmpty && state == "open" && sort == "created" && direction == "desc" && visibility.isEmpty } @@ -348,8 +353,8 @@ object IssuesService { ).flatten ++ labels.map(label => s"label:${label}") ++ List( - milestoneId.map { _ match { - case Some(x) => s"milestone:${milestoneId}" + milestone.map { _ match { + case Some(x) => s"milestone:${x}" case None => "no:milestone" }}, (sort, direction) match { @@ -368,8 +373,8 @@ object IssuesService { def toURL: String = "?" + List( if(labels.isEmpty) None else Some("labels=" + urlEncode(labels.mkString(","))), - milestoneId.map { _ match { - case Some(x) => "milestone=" + x + milestone.map { _ match { + case Some(x) => "milestone=" + urlEncode(x) case None => "milestone=none" }}, author .map(x => "author=" + urlEncode(x)), @@ -416,7 +421,7 @@ object IssuesService { conditions.get("milestone").flatMap(_.headOption) match { case None => None case Some("none") => Some(None) - case Some(x) => milestones.get(x).map(x => Some(x)) + case Some(x) => Some(Some(x)) //milestones.get(x).map(x => Some(x)) }, conditions.get("author").flatMap(_.headOption), conditions.get("assignee").flatMap(_.headOption), @@ -437,7 +442,7 @@ object IssuesService { param(request, "labels").map(_.split(",").toSet).getOrElse(Set.empty), param(request, "milestone").map { case "none" => None - case x => x.toIntOpt + case x => Some(x) }, param(request, "author"), param(request, "assigned"), diff --git a/src/main/twirl/dashboard/issueslist.scala.html b/src/main/twirl/dashboard/issueslist.scala.html index 42b5f95f7..f2afaa0a6 100644 --- a/src/main/twirl/dashboard/issueslist.scala.html +++ b/src/main/twirl/dashboard/issueslist.scala.html @@ -48,7 +48,7 @@
    #@issue.issueId opened by @user(issue.openedUserName, styleClass="username") @datetime(issue.registeredDate) @milestone.map { milestone => - @milestone + @milestone }