From fdd119c477db226633f8cdf52caf6d3b87c54db1 Mon Sep 17 00:00:00 2001 From: nazoking Date: Tue, 6 Oct 2015 01:39:19 +0900 Subject: [PATCH 1/4] add `compare`, `after` and `before` property on webhook push payload --- .../RepositorySettingsController.scala | 9 ++++--- .../RepositoryViewerController.scala | 3 ++- .../core/service/WebHookService.scala | 25 ++++++++++++++----- .../core/servlet/GitRepositoryServlet.scala | 3 ++- 4 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/main/scala/gitbucket/core/controller/RepositorySettingsController.scala b/src/main/scala/gitbucket/core/controller/RepositorySettingsController.scala index 5fd8ec019..cf5dfe892 100644 --- a/src/main/scala/gitbucket/core/controller/RepositorySettingsController.scala +++ b/src/main/scala/gitbucket/core/controller/RepositorySettingsController.scala @@ -14,6 +14,7 @@ import org.apache.commons.io.FileUtils import org.scalatra.i18n.Messages import org.eclipse.jgit.api.Git import org.eclipse.jgit.lib.Constants +import org.eclipse.jgit.lib.ObjectId class RepositorySettingsController extends RepositorySettingsControllerBase @@ -165,13 +166,15 @@ trait RepositorySettingsControllerBase extends ControllerBase { import scala.collection.JavaConverters._ val commits = if(repository.commitCount == 0) List.empty else git.log .add(git.getRepository.resolve(repository.repository.defaultBranch)) - .setMaxCount(3) - .call.iterator.asScala.map(new CommitInfo(_)) + .setMaxCount(4) + .call.iterator.asScala.map(new CommitInfo(_)).toList getAccountByUserName(repository.owner).foreach { ownerAccount => callWebHook("push", List(WebHook(repository.owner, repository.name, form.url)), - WebHookPushPayload(git, ownerAccount, "refs/heads/" + repository.repository.defaultBranch, repository, commits.toList, ownerAccount) + WebHookPushPayload(git, ownerAccount, "refs/heads/" + repository.repository.defaultBranch, repository, (if(commits.isEmpty){Nil}else{commits.tail}), ownerAccount, + oldId = commits.lastOption.map(_.id).map(ObjectId.fromString).getOrElse(ObjectId.zeroId()), + newId = commits.headOption.map(_.id).map(ObjectId.fromString).getOrElse(ObjectId.zeroId())) ) } flash += "url" -> form.url diff --git a/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala b/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala index 680486ff4..53b5cefec 100644 --- a/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala +++ b/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala @@ -665,7 +665,8 @@ trait RepositoryViewerControllerBase extends ControllerBase { val commit = new JGitUtil.CommitInfo(JGitUtil.getRevCommitFromId(git, commitId)) callWebHookOf(repository.owner, repository.name, "push") { getAccountByUserName(repository.owner).map{ ownerAccount => - WebHookPushPayload(git, loginAccount, headName, repository, List(commit), ownerAccount) + WebHookPushPayload(git, loginAccount, headName, repository, List(commit), ownerAccount, + oldId = headTip, newId = commitId) } } } diff --git a/src/main/scala/gitbucket/core/service/WebHookService.scala b/src/main/scala/gitbucket/core/service/WebHookService.scala index 25c8e738d..06efce9b5 100644 --- a/src/main/scala/gitbucket/core/service/WebHookService.scala +++ b/src/main/scala/gitbucket/core/service/WebHookService.scala @@ -12,6 +12,7 @@ import org.apache.http.NameValuePair import org.apache.http.client.entity.UrlEncodedFormEntity import org.apache.http.message.BasicNameValuePair import org.eclipse.jgit.api.Git +import org.eclipse.jgit.lib.ObjectId import org.slf4j.LoggerFactory @@ -192,18 +193,30 @@ object WebHookService { case class WebHookPushPayload( pusher: ApiUser, ref: String, + before: String, + after: String, commits: List[ApiPushCommit], repository: ApiRepository - ) extends WebHookPayload + ) extends FieldSerializable with WebHookPayload { + val compare = commits.size match { + case 0 => ApiPath(s"/${repository.full_name}") // maybe test hook on un-initalied repository + case 1 => ApiPath(s"/${repository.full_name}/commit/${after}") + case _ if before.filterNot(_=='0').isEmpty => ApiPath(s"/${repository.full_name}/compare/${commits.head.id}^...${after}") + case _ => ApiPath(s"/${repository.full_name}/compare/${before}...${after}") + } + } object WebHookPushPayload { def apply(git: Git, pusher: Account, refName: String, repositoryInfo: RepositoryInfo, - commits: List[CommitInfo], repositoryOwner: Account): WebHookPushPayload = + commits: List[CommitInfo], repositoryOwner: Account, + newId: ObjectId, oldId: ObjectId): WebHookPushPayload = WebHookPushPayload( - ApiUser(pusher), - refName, - commits.map{ commit => ApiPushCommit(git, RepositoryName(repositoryInfo), commit) }, - ApiRepository( + pusher = ApiUser(pusher), + ref = refName, + before = ObjectId.toString(oldId), + after = ObjectId.toString(newId), + commits = commits.map{ commit => ApiPushCommit(git, RepositoryName(repositoryInfo), commit) }, + repository = ApiRepository( repositoryInfo, owner= ApiUser(repositoryOwner) ) diff --git a/src/main/scala/gitbucket/core/servlet/GitRepositoryServlet.scala b/src/main/scala/gitbucket/core/servlet/GitRepositoryServlet.scala index 3deecfb46..0c12872a1 100644 --- a/src/main/scala/gitbucket/core/servlet/GitRepositoryServlet.scala +++ b/src/main/scala/gitbucket/core/servlet/GitRepositoryServlet.scala @@ -203,7 +203,8 @@ class CommitLogHook(owner: String, repository: String, pusher: String, baseUrl: callWebHookOf(owner, repository, "push"){ for(pusherAccount <- getAccountByUserName(pusher); ownerAccount <- getAccountByUserName(owner)) yield { - WebHookPushPayload(git, pusherAccount, command.getRefName, repositoryInfo, newCommits, ownerAccount) + WebHookPushPayload(git, pusherAccount, command.getRefName, repositoryInfo, newCommits, ownerAccount, + newId = command.getNewId(), oldId = command.getOldId()) } } } From 0a2d95e4348aec72db61f021285b87babc605bb8 Mon Sep 17 00:00:00 2001 From: nazoking Date: Tue, 6 Oct 2015 02:14:02 +0900 Subject: [PATCH 2/4] add `head_commit` on webhook push payload --- src/main/scala/gitbucket/core/service/WebHookService.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/scala/gitbucket/core/service/WebHookService.scala b/src/main/scala/gitbucket/core/service/WebHookService.scala index 06efce9b5..e5436f6cb 100644 --- a/src/main/scala/gitbucket/core/service/WebHookService.scala +++ b/src/main/scala/gitbucket/core/service/WebHookService.scala @@ -204,6 +204,7 @@ object WebHookService { case _ if before.filterNot(_=='0').isEmpty => ApiPath(s"/${repository.full_name}/compare/${commits.head.id}^...${after}") case _ => ApiPath(s"/${repository.full_name}/compare/${before}...${after}") } + val head_commit = commits.lastOption } object WebHookPushPayload { From 8b8c6ee8613f5baf5446712e6bf81d292187c0b3 Mon Sep 17 00:00:00 2001 From: nazoking Date: Tue, 6 Oct 2015 02:15:03 +0900 Subject: [PATCH 3/4] fix `repository.url` on webhook `push` payload --- .../scala/gitbucket/core/api/ApiRepository.scala | 16 ++++++++++++---- .../gitbucket/core/service/WebHookService.scala | 5 ++--- .../gitbucket/core/api/JsonFormatSpec.scala | 2 +- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/main/scala/gitbucket/core/api/ApiRepository.scala b/src/main/scala/gitbucket/core/api/ApiRepository.scala index ee97ea324..dce8443d4 100644 --- a/src/main/scala/gitbucket/core/api/ApiRepository.scala +++ b/src/main/scala/gitbucket/core/api/ApiRepository.scala @@ -13,10 +13,14 @@ case class ApiRepository( forks: Int, `private`: Boolean, default_branch: String, - owner: ApiUser) { + owner: ApiUser)(urlIsHtmlUrl: Boolean) { val forks_count = forks val watchers_count = watchers - val url = ApiPath(s"/api/v3/repos/${full_name}") + val url = if(urlIsHtmlUrl){ + ApiPath(s"/${full_name}") + }else{ + 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}") @@ -27,7 +31,8 @@ object ApiRepository{ repository: Repository, owner: ApiUser, forkedCount: Int =0, - watchers: Int = 0): ApiRepository = + watchers: Int = 0, + urlIsHtmlUrl: Boolean = false): ApiRepository = ApiRepository( name = repository.repositoryName, full_name = s"${repository.userName}/${repository.repositoryName}", @@ -37,7 +42,7 @@ object ApiRepository{ `private` = repository.isPrivate, default_branch = repository.defaultBranch, owner = owner - ) + )(urlIsHtmlUrl) def apply(repositoryInfo: RepositoryInfo, owner: ApiUser): ApiRepository = ApiRepository(repositoryInfo.repository, owner, forkedCount=repositoryInfo.forkedCount) @@ -45,4 +50,7 @@ object ApiRepository{ def apply(repositoryInfo: RepositoryInfo, owner: Account): ApiRepository = this(repositoryInfo.repository, ApiUser(owner)) + def forPushPayload(repositoryInfo: RepositoryInfo, owner: ApiUser): ApiRepository = + ApiRepository(repositoryInfo.repository, owner, forkedCount=repositoryInfo.forkedCount, urlIsHtmlUrl=true) + } diff --git a/src/main/scala/gitbucket/core/service/WebHookService.scala b/src/main/scala/gitbucket/core/service/WebHookService.scala index e5436f6cb..2b18cae61 100644 --- a/src/main/scala/gitbucket/core/service/WebHookService.scala +++ b/src/main/scala/gitbucket/core/service/WebHookService.scala @@ -217,10 +217,9 @@ object WebHookService { before = ObjectId.toString(oldId), after = ObjectId.toString(newId), commits = commits.map{ commit => ApiPushCommit(git, RepositoryName(repositoryInfo), commit) }, - repository = ApiRepository( + repository = ApiRepository.forPushPayload( repositoryInfo, - owner= ApiUser(repositoryOwner) - ) + owner= ApiUser(repositoryOwner)) ) } diff --git a/src/test/scala/gitbucket/core/api/JsonFormatSpec.scala b/src/test/scala/gitbucket/core/api/JsonFormatSpec.scala index f3a2f9795..590180d34 100644 --- a/src/test/scala/gitbucket/core/api/JsonFormatSpec.scala +++ b/src/test/scala/gitbucket/core/api/JsonFormatSpec.scala @@ -45,7 +45,7 @@ class JsonFormatSpec extends Specification { forks = 0, `private` = false, default_branch = "master", - owner = apiUser) + owner = apiUser)(urlIsHtmlUrl = false) val repositoryJson = s"""{ "name" : "Hello-World", "full_name" : "octocat/Hello-World", From d13bb47ee73278e6d00ad27afa211ba82eb59177 Mon Sep 17 00:00:00 2001 From: nazoking Date: Tue, 6 Oct 2015 02:27:42 +0900 Subject: [PATCH 4/4] remove ApiPushCommit class (merge to ApiCommit) --- .../scala/gitbucket/core/api/ApiCommit.scala | 19 ++++++--- .../gitbucket/core/api/ApiPushCommit.scala | 39 ------------------- .../core/service/WebHookService.scala | 4 +- .../gitbucket/core/api/JsonFormatSpec.scala | 4 +- 4 files changed, 18 insertions(+), 48 deletions(-) delete mode 100644 src/main/scala/gitbucket/core/api/ApiPushCommit.scala diff --git a/src/main/scala/gitbucket/core/api/ApiCommit.scala b/src/main/scala/gitbucket/core/api/ApiCommit.scala index 95128c1d4..9229d0c83 100644 --- a/src/main/scala/gitbucket/core/api/ApiCommit.scala +++ b/src/main/scala/gitbucket/core/api/ApiCommit.scala @@ -20,13 +20,21 @@ case class ApiCommit( removed: List[String], modified: List[String], author: ApiPersonIdent, - committer: ApiPersonIdent)(repositoryName:RepositoryName) extends FieldSerializable{ - val url = ApiPath(s"/api/v3/${repositoryName.fullName}/commits/${id}") - val html_url = ApiPath(s"/${repositoryName.fullName}/commit/${id}") + committer: ApiPersonIdent)(repositoryName:RepositoryName, urlIsHtmlUrl: Boolean) extends FieldSerializable{ + val url = if(urlIsHtmlUrl){ + ApiPath(s"/${repositoryName.fullName}/commit/${id}") + }else{ + ApiPath(s"/api/v3/${repositoryName.fullName}/commits/${id}") + } + val html_url = if(urlIsHtmlUrl){ + None + }else{ + Some(ApiPath(s"/${repositoryName.fullName}/commit/${id}")) + } } object ApiCommit{ - def apply(git: Git, repositoryName: RepositoryName, commit: CommitInfo): ApiCommit = { + def apply(git: Git, repositoryName: RepositoryName, commit: CommitInfo, urlIsHtmlUrl: Boolean = false): ApiCommit = { val diffs = JGitUtil.getDiffs(git, commit.id, false) ApiCommit( id = commit.id, @@ -43,6 +51,7 @@ object ApiCommit{ }, author = ApiPersonIdent.author(commit), committer = ApiPersonIdent.committer(commit) - )(repositoryName) + )(repositoryName, urlIsHtmlUrl) } + def forPushPayload(git: Git, repositoryName: RepositoryName, commit: CommitInfo): ApiCommit = apply(git, repositoryName, commit, true) } diff --git a/src/main/scala/gitbucket/core/api/ApiPushCommit.scala b/src/main/scala/gitbucket/core/api/ApiPushCommit.scala deleted file mode 100644 index 46a99e09e..000000000 --- a/src/main/scala/gitbucket/core/api/ApiPushCommit.scala +++ /dev/null @@ -1,39 +0,0 @@ -package gitbucket.core.api - -import gitbucket.core.util.JGitUtil -import gitbucket.core.util.JGitUtil.CommitInfo -import gitbucket.core.util.RepositoryName - -import org.eclipse.jgit.diff.DiffEntry -import org.eclipse.jgit.api.Git - -import java.util.Date - -/** - * https://developer.github.com/v3/activity/events/types/#pushevent - */ -case class ApiPushCommit( - id: String, - message: String, - timestamp: Date, - added: List[String], - removed: List[String], - modified: List[String], - author: ApiPersonIdent, - committer: ApiPersonIdent)(repositoryName:RepositoryName) extends FieldSerializable { - val url = ApiPath(s"/${repositoryName.fullName}/commit/${id}") -} - -object ApiPushCommit{ - def apply(commit: ApiCommit, repositoryName: RepositoryName): ApiPushCommit = ApiPushCommit( - id = commit.id, - message = commit.message, - timestamp = commit.timestamp, - added = commit.added, - removed = commit.removed, - modified = commit.modified, - author = commit.author, - committer = commit.committer)(repositoryName) - def apply(git: Git, repositoryName: RepositoryName, commit: CommitInfo): ApiPushCommit = - ApiPushCommit(ApiCommit(git, repositoryName, commit), repositoryName) -} diff --git a/src/main/scala/gitbucket/core/service/WebHookService.scala b/src/main/scala/gitbucket/core/service/WebHookService.scala index 2b18cae61..da5cdc8d2 100644 --- a/src/main/scala/gitbucket/core/service/WebHookService.scala +++ b/src/main/scala/gitbucket/core/service/WebHookService.scala @@ -195,7 +195,7 @@ object WebHookService { ref: String, before: String, after: String, - commits: List[ApiPushCommit], + commits: List[ApiCommit], repository: ApiRepository ) extends FieldSerializable with WebHookPayload { val compare = commits.size match { @@ -216,7 +216,7 @@ object WebHookService { ref = refName, before = ObjectId.toString(oldId), after = ObjectId.toString(newId), - commits = commits.map{ commit => ApiPushCommit(git, RepositoryName(repositoryInfo), commit) }, + commits = commits.map{ commit => ApiCommit.forPushPayload(git, RepositoryName(repositoryInfo), commit) }, repository = ApiRepository.forPushPayload( repositoryInfo, owner= ApiUser(repositoryOwner)) diff --git a/src/test/scala/gitbucket/core/api/JsonFormatSpec.scala b/src/test/scala/gitbucket/core/api/JsonFormatSpec.scala index 590180d34..fe317ce7d 100644 --- a/src/test/scala/gitbucket/core/api/JsonFormatSpec.scala +++ b/src/test/scala/gitbucket/core/api/JsonFormatSpec.scala @@ -85,7 +85,7 @@ class JsonFormatSpec extends Specification { "url": "http://gitbucket.exmple.com/api/v3/repos/octocat/Hello-World/commits/6dcb09b5b57875f334f61aebed695e2e4193db5e/statuses" }""" - val apiPushCommit = ApiPushCommit( + val apiPushCommit = ApiCommit( id = "0d1a26e67d8f5eaf1f6ba5c57fc3c7d91ac0fd1c", message = "Update README.md", timestamp = date1, @@ -93,7 +93,7 @@ class JsonFormatSpec extends Specification { removed = Nil, modified = List("README.md"), author = ApiPersonIdent("baxterthehacker","baxterthehacker@users.noreply.github.com",date1), - committer = ApiPersonIdent("baxterthehacker","baxterthehacker@users.noreply.github.com",date1))(RepositoryName("baxterthehacker", "public-repo")) + committer = ApiPersonIdent("baxterthehacker","baxterthehacker@users.noreply.github.com",date1))(RepositoryName("baxterthehacker", "public-repo"), true) val apiPushCommitJson = s"""{ "id": "0d1a26e67d8f5eaf1f6ba5c57fc3c7d91ac0fd1c", // "distinct": true,