diff --git a/src/main/scala/gitbucket/core/api/ApiPullRequest.scala b/src/main/scala/gitbucket/core/api/ApiPullRequest.scala index 52bb9a7..3ae6ac0 100644 --- a/src/main/scala/gitbucket/core/api/ApiPullRequest.scala +++ b/src/main/scala/gitbucket/core/api/ApiPullRequest.scala @@ -1,7 +1,6 @@ package gitbucket.core.api -import gitbucket.core.model.{Issue, PullRequest} - +import gitbucket.core.model.{Account, Issue, IssueComment, PullRequest} import java.util.Date @@ -15,6 +14,9 @@ head: ApiPullRequest.Commit, base: ApiPullRequest.Commit, mergeable: Option[Boolean], + merged: Boolean, + merged_at: Option[Date], + merged_by: Option[ApiUser], title: String, body: String, user: ApiUser) { @@ -31,7 +33,14 @@ } object ApiPullRequest{ - def apply(issue: Issue, pullRequest: PullRequest, headRepo: ApiRepository, baseRepo: ApiRepository, user: ApiUser): ApiPullRequest = + def apply( + issue: Issue, + pullRequest: PullRequest, + headRepo: ApiRepository, + baseRepo: ApiRepository, + user: ApiUser, + mergedComment: Option[(IssueComment, Account)] + ): ApiPullRequest = ApiPullRequest( number = issue.issueId, updated_at = issue.updatedDate, @@ -45,6 +54,9 @@ ref = pullRequest.branch, repo = baseRepo)(issue.userName), mergeable = None, // TODO: need check mergeable. + merged = mergedComment.isDefined, + merged_at = mergedComment.map { case (comment, _) => comment.registeredDate }, + merged_by = mergedComment.map { case (_, account) => ApiUser(account) }, title = issue.title, body = issue.content.getOrElse(""), user = user diff --git a/src/main/scala/gitbucket/core/controller/ApiController.scala b/src/main/scala/gitbucket/core/controller/ApiController.scala index e5229c3..ba32532 100644 --- a/src/main/scala/gitbucket/core/controller/ApiController.scala +++ b/src/main/scala/gitbucket/core/controller/ApiController.scala @@ -102,7 +102,7 @@ defaultBranch = repository.repository.defaultBranch, origin = repository.repository.originUserName.isEmpty ).map { br => - ApiBranchForList(br.name, ApiBranchCommit(br.commitId)) + ApiBranchForList(br.name, ApiBranchCommit(br.commitId)) }) }) @@ -254,7 +254,7 @@ patch("/api/v3/repos/:owner/:repo/branches/:branch")(ownerOnly { repository => import gitbucket.core.api._ (for{ - branch <- params.get("branch") if repository.branchList.find(_ == branch).isDefined + branch <- params.get("branch") if repository.branchList.find(_ == branch).isDefined protection <- extractFromJsonBody[ApiBranchProtection.EnablingAndDisabling].map(_.protection) } yield { if(protection.enabled){ @@ -281,8 +281,8 @@ */ get("/api/v3/repos/:owner/:repository/issues/:id/comments")(referrersOnly { repository => (for{ - issueId <- params("id").toIntOpt - comments = getCommentsForApi(repository.owner, repository.name, issueId.toInt) + issueId <- params("id").toIntOpt + comments = getCommentsForApi(repository.owner, repository.name, issueId.toInt) } yield { JsonFormat(comments.map{ case (issueComment, user, issue) => ApiComment(issueComment, RepositoryName(repository), issueId, ApiUser(user), issue.isPullRequest) }) }) getOrElse NotFound() @@ -363,12 +363,14 @@ updateLabel(repository.owner, repository.name, label.labelId, data.name, data.color) JsonFormat(ApiLabel( getLabel(repository.owner, repository.name, label.labelId).get, - RepositoryName(repository))) + RepositoryName(repository) + )) } else { // TODO ApiError should support errors field to enhance compatibility of GitHub API UnprocessableEntity(ApiError( "Validation Failed", - Some("https://developer.github.com/v3/issues/labels/#create-a-label"))) + Some("https://developer.github.com/v3/issues/labels/#create-a-label") + )) } } getOrElse NotFound() } @@ -407,11 +409,12 @@ JsonFormat(issues.map { case (issue, issueUser, commentCount, pullRequest, headRepo, headOwner) => ApiPullRequest( - issue, - pullRequest, - ApiRepository(headRepo, ApiUser(headOwner)), - ApiRepository(repository, ApiUser(baseOwner)), - ApiUser(issueUser) + issue = issue, + pullRequest = pullRequest, + headRepo = ApiRepository(headRepo, ApiUser(headOwner)), + baseRepo = ApiRepository(repository, ApiUser(baseOwner)), + user = ApiUser(issueUser), + mergedComment = getMergedComment(repository.owner, repository.name, issue.issueId) ) }) }) @@ -421,20 +424,22 @@ */ get("/api/v3/repos/:owner/:repository/pulls/:id")(referrersOnly { repository => (for{ - issueId <- params("id").toIntOpt + issueId <- params("id").toIntOpt (issue, pullRequest) <- getPullRequest(repository.owner, repository.name, issueId) - users = getAccountsByUserNames(Set(repository.owner, pullRequest.requestUserName, issue.openedUserName), Set()) + users = getAccountsByUserNames(Set(repository.owner, pullRequest.requestUserName, issue.openedUserName), Set.empty) baseOwner <- users.get(repository.owner) headOwner <- users.get(pullRequest.requestUserName) issueUser <- users.get(issue.openedUserName) headRepo <- getRepository(pullRequest.requestUserName, pullRequest.requestRepositoryName) } yield { JsonFormat(ApiPullRequest( - issue, - pullRequest, - ApiRepository(headRepo, ApiUser(headOwner)), - ApiRepository(repository, ApiUser(baseOwner)), - ApiUser(issueUser))) + issue = issue, + pullRequest = pullRequest, + headRepo = ApiRepository(headRepo, ApiUser(headOwner)), + baseRepo = ApiRepository(repository, ApiUser(baseOwner)), + user = ApiUser(issueUser), + mergedComment = getMergedComment(repository.owner, repository.name, issue.issueId) + )) }) getOrElse NotFound() }) @@ -450,7 +455,7 @@ val oldId = git.getRepository.resolve(pullreq.commitIdFrom) val newId = git.getRepository.resolve(pullreq.commitIdTo) val repoFullName = RepositoryName(repository) - val commits = git.log.addRange(oldId, newId).call.iterator.asScala.map(c => ApiCommitListItem(new CommitInfo(c), repoFullName)).toList + val commits = git.log.addRange(oldId, newId).call.iterator.asScala.map { c => ApiCommitListItem(new CommitInfo(c), repoFullName) }.toList JsonFormat(commits) } } @@ -469,14 +474,14 @@ */ post("/api/v3/repos/:owner/:repo/statuses/:sha")(writableUsersOnly { repository => (for{ - ref <- params.get("sha") - sha <- JGitUtil.getShaByRef(repository.owner, repository.name, ref) - data <- extractFromJsonBody[CreateAStatus] if data.isValid - creator <- context.loginAccount - state <- 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) + ref <- params.get("sha") + sha <- JGitUtil.getShaByRef(repository.owner, repository.name, ref) + data <- extractFromJsonBody[CreateAStatus] if data.isValid + creator <- context.loginAccount + state <- 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 { JsonFormat(ApiCommitStatus(status, ApiUser(creator))) }) getOrElse NotFound() @@ -514,9 +519,9 @@ */ get("/api/v3/repos/:owner/:repo/commits/:ref/status")(referrersOnly { repository => (for{ - ref <- params.get("ref") + ref <- params.get("ref") owner <- getAccountByUserName(repository.owner) - sha <- JGitUtil.getShaByRef(repository.owner, repository.name, ref) + sha <- JGitUtil.getShaByRef(repository.owner, repository.name, ref) } yield { val statuses = getCommitStatuesWithCreator(repository.owner, repository.name, sha) JsonFormat(ApiCombinedCommitStatus(sha, statuses, ApiRepository(repository, owner))) diff --git a/src/main/scala/gitbucket/core/service/IssuesService.scala b/src/main/scala/gitbucket/core/service/IssuesService.scala index c050e79..23c3b9d 100644 --- a/src/main/scala/gitbucket/core/service/IssuesService.scala +++ b/src/main/scala/gitbucket/core/service/IssuesService.scala @@ -13,6 +13,7 @@ import Q.interpolation + trait IssuesService { self: AccountService with RepositoryService => import IssuesService._ @@ -34,6 +35,10 @@ .map{ case ((t1, t2), t3) => (t1, t2, t3) } .list + def getMergedComment(owner: String, repository: String, issueId: Int)(implicit s: Session): Option[(IssueComment, Account)] = { + getCommentsForApi(owner, repository, issueId).collectFirst { case (comment, account, _) if comment.action == "merged" => (comment, account) } + } + def getComment(owner: String, repository: String, commentId: String)(implicit s: Session) = if (commentId forall (_.isDigit)) IssueComments filter { t => diff --git a/src/main/scala/gitbucket/core/service/WebHookService.scala b/src/main/scala/gitbucket/core/service/WebHookService.scala index d50f9e7..1fb181c 100644 --- a/src/main/scala/gitbucket/core/service/WebHookService.scala +++ b/src/main/scala/gitbucket/core/service/WebHookService.scala @@ -1,9 +1,11 @@ package gitbucket.core.service +import java.util.Date + import fr.brouillard.oss.security.xhub.XHub -import fr.brouillard.oss.security.xhub.XHub.{XHubDigest, XHubConverter} +import fr.brouillard.oss.security.xhub.XHub.{XHubConverter, XHubDigest} import gitbucket.core.api._ -import gitbucket.core.model.{WebHook, Account, Issue, PullRequest, IssueComment, WebHookEvent, CommitComment} +import gitbucket.core.model.{Account, CommitComment, Issue, IssueComment, PullRequest, WebHook, WebHookEvent} import gitbucket.core.model.Profile._ import org.apache.http.client.utils.URLEncodedUtils import profile.simple._ @@ -16,6 +18,7 @@ import org.eclipse.jgit.api.Git import org.eclipse.jgit.lib.ObjectId import org.slf4j.LoggerFactory + import scala.concurrent._ import org.apache.http.HttpRequest import org.apache.http.HttpResponse @@ -33,15 +36,15 @@ def getWebHooks(owner: String, repository: String)(implicit s: Session): List[(WebHook, Set[WebHook.Event])] = WebHooks.filter(_.byRepository(owner, repository)) .innerJoin(WebHookEvents).on { (w, t) => t.byWebHook(w) } - .map{ case (w,t) => w -> t.event } + .map { case (w,t) => w -> t.event } .list.groupBy(_._1).mapValues(_.map(_._2).toSet).toList.sortBy(_._1.url) /** get All WebHook informations of repository event */ def getWebHooksByEvent(owner: String, repository: String, event: WebHook.Event)(implicit s: Session): List[WebHook] = WebHooks.filter(_.byRepository(owner, repository)) .innerJoin(WebHookEvents).on { (wh, whe) => whe.byWebHook(wh) } - .filter{ case (wh, whe) => whe.event === event.bind} - .map{ case (wh, whe) => wh } + .filter { case (wh, whe) => whe.event === event.bind} + .map { case (wh, whe) => wh } .list.distinct /** get All WebHook information from repository to url */ @@ -49,12 +52,12 @@ WebHooks .filter(_.byPrimaryKey(owner, repository, url)) .innerJoin(WebHookEvents).on { (w, t) => t.byWebHook(w) } - .map{ case (w,t) => w -> t.event } + .map { case (w,t) => w -> t.event } .list.groupBy(_._1).mapValues(_.map(_._2).toSet).headOption - def addWebHook(owner: String, repository: String, url :String, events: Set[WebHook.Event], ctype: WebHookContentType, token: Option[String])(implicit s: Session): Unit = { + def addWebHook(owner: String, repository: String, url :String, events: Set[WebHook.Event], ctype: WebHookContentType, token: Option[String])(implicit s: Session): Unit = { WebHooks insert WebHook(owner, repository, url, ctype, token) - events.toSet.map{ event: WebHook.Event => + events.map { event: WebHook.Event => WebHookEvents insert WebHookEvent(owner, repository, url, event) } } @@ -62,7 +65,7 @@ def updateWebHook(owner: String, repository: String, url :String, events: Set[WebHook.Event], ctype: WebHookContentType, token: Option[String])(implicit s: Session): Unit = { WebHooks.filter(_.byPrimaryKey(owner, repository, url)).map(w => (w.ctype, w.token)).update((ctype, token)) WebHookEvents.filter(_.byWebHook(owner, repository, url)).delete - events.toSet.map{ event: WebHook.Event => + events.map { event: WebHook.Event => WebHookEvents insert WebHookEvent(owner, repository, url, event) } } @@ -81,7 +84,7 @@ def callWebHook(event: WebHook.Event, webHooks: List[WebHook], payload: WebHookPayload) (implicit c: JsonFormat.Context): List[(WebHook, String, Future[HttpRequest], Future[HttpResponse])] = { import org.apache.http.impl.client.HttpClientBuilder - import ExecutionContext.Implicits.global + import ExecutionContext.Implicits.global // TODO Shouldn't use the default execution context import org.apache.http.protocol.HttpContext import org.apache.http.client.methods.HttpPost @@ -91,7 +94,7 @@ webHooks.map { webHook => val reqPromise = Promise[HttpRequest] val f = Future { - val itcp = new org.apache.http.HttpRequestInterceptor{ + val itcp = new org.apache.http.HttpRequestInterceptor { def process(res: HttpRequest, ctx: HttpContext): Unit = { reqPromise.success(res) } @@ -129,8 +132,8 @@ httpPost.releaseConnection() logger.debug(s"end web hook invocation for ${webHook}") res - }catch{ - case e:Throwable => { + } catch { + case e: Throwable => { if(!reqPromise.isCompleted){ reqPromise.failure(e) } @@ -168,11 +171,11 @@ issueUser <- users.get(issue.openedUserName) } yield { WebHookIssuesPayload( - action = action, - number = issue.issueId, - repository = ApiRepository(repository, ApiUser(repoOwner)), - issue = ApiIssue(issue, RepositoryName(repository), ApiUser(issueUser)), - sender = ApiUser(sender)) + action = action, + number = issue.issueId, + repository = ApiRepository(repository, ApiUser(repoOwner)), + issue = ApiIssue(issue, RepositoryName(repository), ApiUser(issueUser)), + sender = ApiUser(sender)) } } } @@ -198,7 +201,9 @@ headOwner = headOwner, baseRepository = repository, baseOwner = baseOwner, - sender = sender) + sender = sender, + mergedComment = getMergedComment(repository.owner, repository.name, issueId) + ) } } } @@ -237,7 +242,10 @@ headOwner = headOwner, baseRepository = baseRepo, baseOwner = baseOwner, - sender = sender) + sender = sender, + mergedComment = getMergedComment(baseRepo.owner, baseRepo.name, issue.issueId) + ) + callWebHook(WebHook.PullRequest, webHooks, payload) } } @@ -267,7 +275,9 @@ headOwner = headOwner, baseRepository = repository, baseOwner = baseOwner, - sender = sender) + sender = sender, + mergedComment = getMergedComment(repository.owner, repository.name, issue.issueId) + ) } } } @@ -365,11 +375,21 @@ headOwner: Account, baseRepository: RepositoryInfo, baseOwner: Account, - sender: Account): WebHookPullRequestPayload = { + sender: Account, + mergedComment: Option[(IssueComment, Account)]): WebHookPullRequestPayload = { + val headRepoPayload = ApiRepository(headRepository, headOwner) val baseRepoPayload = ApiRepository(baseRepository, baseOwner) val senderPayload = ApiUser(sender) - val pr = ApiPullRequest(issue, pullRequest, headRepoPayload, baseRepoPayload, ApiUser(issueUser)) + val pr = ApiPullRequest( + issue = issue, + pullRequest = pullRequest, + headRepo = headRepoPayload, + baseRepo = baseRepoPayload, + user = ApiUser(issueUser), + mergedComment = mergedComment + ) + WebHookPullRequestPayload( action = action, number = issue.issueId, @@ -389,7 +409,7 @@ sender: ApiUser ) extends WebHookPayload - object WebHookIssueCommentPayload{ + object WebHookIssueCommentPayload { def apply( issue: Issue, issueUser: Account, @@ -415,28 +435,42 @@ sender: ApiUser ) extends WebHookPayload - object WebHookPullRequestReviewCommentPayload{ + object WebHookPullRequestReviewCommentPayload { def apply( - action: String, - comment: CommitComment, - issue: Issue, - issueUser: Account, - pullRequest: PullRequest, - headRepository: RepositoryInfo, - headOwner: Account, - baseRepository: RepositoryInfo, - baseOwner: Account, - sender: Account - ) : WebHookPullRequestReviewCommentPayload = { + action: String, + comment: CommitComment, + issue: Issue, + issueUser: Account, + pullRequest: PullRequest, + headRepository: RepositoryInfo, + headOwner: Account, + baseRepository: RepositoryInfo, + baseOwner: Account, + sender: Account, + mergedComment: Option[(IssueComment, Account)] + ) : WebHookPullRequestReviewCommentPayload = { val headRepoPayload = ApiRepository(headRepository, headOwner) val baseRepoPayload = ApiRepository(baseRepository, baseOwner) val senderPayload = ApiUser(sender) + WebHookPullRequestReviewCommentPayload( - action = action, - comment = ApiPullRequestReviewComment(comment, senderPayload, RepositoryName(baseRepository), issue.issueId), - pull_request = ApiPullRequest(issue, pullRequest, headRepoPayload, baseRepoPayload, ApiUser(issueUser)), - repository = baseRepoPayload, - sender = senderPayload) + action = action, + comment = ApiPullRequestReviewComment( + comment = comment, + commentedUser = senderPayload, + repositoryName = RepositoryName(baseRepository), + issueId = issue.issueId + ), + pull_request = ApiPullRequest( + issue = issue, + pullRequest = pullRequest, + headRepo = headRepoPayload, + baseRepo = baseRepoPayload, + user = ApiUser(issueUser), + mergedComment = mergedComment + ), + repository = baseRepoPayload, + sender = senderPayload) } } } diff --git a/src/test/scala/gitbucket/core/api/JsonFormatSpec.scala b/src/test/scala/gitbucket/core/api/JsonFormatSpec.scala index 6e6376e..0fc254b 100644 --- a/src/test/scala/gitbucket/core/api/JsonFormatSpec.scala +++ b/src/test/scala/gitbucket/core/api/JsonFormatSpec.scala @@ -25,11 +25,11 @@ implicit val context = JsonFormat.Context("http://gitbucket.exmple.com") val apiUser = ApiUser( - login= "octocat", - email= "octocat@example.com", - `type`= "User", - site_admin= false, - created_at= date1) + login = "octocat", + email = "octocat@example.com", + `type` = "User", + site_admin = false, + created_at = date1) val apiUserJson = """{ "login":"octocat", "email":"octocat@example.com", @@ -264,60 +264,64 @@ }""" val apiPullRequest = ApiPullRequest( - number = 1347, - updated_at = date1, - created_at = date1, - head = ApiPullRequest.Commit( - sha = sha1, - ref = "new-topic", - repo = repository)("octocat"), - base = ApiPullRequest.Commit( - sha = sha1, - ref = "master", - repo = repository)("octocat"), - mergeable = None, - title = "new-feature", - body = "Please pull these awesome changes", - user = apiUser - ) + number = 1347, + updated_at = date1, + created_at = date1, + head = ApiPullRequest.Commit( + sha = sha1, + ref = "new-topic", + repo = repository)("octocat"), + base = ApiPullRequest.Commit( + sha = sha1, + ref = "master", + repo = repository)("octocat"), + mergeable = None, + merged = false, + merged_at = Some(date1), + merged_by = Some(apiUser), + title = "new-feature", + body = "Please pull these awesome changes", + user = apiUser + ) + val apiPullRequestJson = s"""{ - "url": "${context.baseUrl}/api/v3/repos/octocat/Hello-World/pulls/1347", + "number": 1347, + "updated_at": "2011-04-14T16:00:49Z", + "created_at": "2011-04-14T16:00:49Z", + // "closed_at": "2011-04-14T16:00:49Z", + "head": { + "sha": "6dcb09b5b57875f334f61aebed695e2e4193db5e", + "ref": "new-topic", + "repo": $repositoryJson, + "label": "new-topic", + "user": $apiUserJson + }, + "base": { + "sha": "6dcb09b5b57875f334f61aebed695e2e4193db5e", + "ref": "master", + "repo": $repositoryJson, + "label": "master", + "user": $apiUserJson + }, + // "merge_commit_sha": "e5bd3914e2e596debea16f433f57875b5b90bcd6", + // "mergeable": true, + "merged": false, + "merged_at": "2011-04-14T16:00:49Z", + "merged_by": $apiUserJson, + "title": "new-feature", + "body": "Please pull these awesome changes", + "user": $apiUserJson, "html_url": "${context.baseUrl}/octocat/Hello-World/pull/1347", - // "diff_url": "${context.baseUrl}/octocat/Hello-World/pull/1347.diff", - // "patch_url": "${context.baseUrl}/octocat/Hello-World/pull/1347.patch", - // "issue_url": "${context.baseUrl}/api/v3/repos/octocat/Hello-World/issues/1347", + "url": "${context.baseUrl}/api/v3/repos/octocat/Hello-World/pulls/1347", "commits_url": "${context.baseUrl}/api/v3/repos/octocat/Hello-World/pulls/1347/commits", "review_comments_url": "${context.baseUrl}/api/v3/repos/octocat/Hello-World/pulls/1347/comments", "review_comment_url": "${context.baseUrl}/api/v3/repos/octocat/Hello-World/pulls/comments/{number}", "comments_url": "${context.baseUrl}/api/v3/repos/octocat/Hello-World/issues/1347/comments", - "statuses_url": "${context.baseUrl}/api/v3/repos/octocat/Hello-World/statuses/6dcb09b5b57875f334f61aebed695e2e4193db5e", - "number": 1347, + "statuses_url": "${context.baseUrl}/api/v3/repos/octocat/Hello-World/statuses/6dcb09b5b57875f334f61aebed695e2e4193db5e" + // "diff_url": "${context.baseUrl}/octocat/Hello-World/pull/1347.diff", + // "patch_url": "${context.baseUrl}/octocat/Hello-World/pull/1347.patch", + // "issue_url": "${context.baseUrl}/api/v3/repos/octocat/Hello-World/issues/1347", // "state": "open", - "title": "new-feature", - "body": "Please pull these awesome changes", - "created_at": "2011-04-14T16:00:49Z", - "updated_at": "2011-04-14T16:00:49Z", - // "closed_at": "2011-04-14T16:00:49Z", - // "merged_at": "2011-04-14T16:00:49Z", - "head": { - "label": "new-topic", - "ref": "new-topic", - "sha": "6dcb09b5b57875f334f61aebed695e2e4193db5e", - "user": $apiUserJson, - "repo": $repositoryJson - }, - "base": { - "label": "master", - "ref": "master", - "sha": "6dcb09b5b57875f334f61aebed695e2e4193db5e", - "user": $apiUserJson, - "repo": $repositoryJson - }, - "user": $apiUserJson - // "merge_commit_sha": "e5bd3914e2e596debea16f433f57875b5b90bcd6", - // "merged": false, - // "mergeable": true, - // "merged_by": $$apiUserJson, // "comments": 10, // "commits": 3, // "additions": 100,