From 64e843bcaa300119ce02f0c338c86e7881b43ab4 Mon Sep 17 00:00:00 2001 From: Chris Rebert Date: Wed, 3 Jun 2015 02:03:00 -0700 Subject: progress on jcabi-github porting --- build.sbt | 5 +--- src/main/resources/application.conf | 1 + .../rorschach/github/Credentials.scala | 9 +++++++ .../rorschach/github/GitHubActorWithLogging.scala | 5 +--- .../github/GitHubPullRequestCommenter.scala | 30 +++++++++------------- .../rorschach/github/PullRequestStatus.scala | 20 --------------- .../rorschach/github/implicits/package.scala | 18 +++++++++++++ .../rorschach/github/util/package.scala | 19 +++++++------- .../getbootstrap/rorschach/http/SuperWire.scala | 20 +++++++++++++++ .../getbootstrap/rorschach/http/UserAgent.scala | 3 +++ .../rorschach/http/UserAgentWire.scala | 25 ++++++++++++++++++ .../GitHubPullRequestWebHooksDirectives.scala | 2 -- .../rorschach/server/PullRequestEventHandler.scala | 18 ++++++------- .../getbootstrap/rorschach/server/Settings.scala | 14 +++++++--- 14 files changed, 118 insertions(+), 71 deletions(-) create mode 100644 src/main/scala/com/getbootstrap/rorschach/github/Credentials.scala delete mode 100644 src/main/scala/com/getbootstrap/rorschach/github/PullRequestStatus.scala create mode 100644 src/main/scala/com/getbootstrap/rorschach/github/implicits/package.scala create mode 100644 src/main/scala/com/getbootstrap/rorschach/http/SuperWire.scala create mode 100644 src/main/scala/com/getbootstrap/rorschach/http/UserAgent.scala create mode 100644 src/main/scala/com/getbootstrap/rorschach/http/UserAgentWire.scala diff --git a/build.sbt b/build.sbt index aa6de23..8f0e85e 100644 --- a/build.sbt +++ b/build.sbt @@ -12,10 +12,7 @@ resolvers += "Eclipse Foundation Releases" at "https://repo.eclipse.org/content/ resolvers += "Eclipse Foundation Snapshots" at "https://repo.eclipse.org/content/repositories/snapshots/" -libraryDependencies += "org.eclipse.mylyn.github" % "org.eclipse.egit.github.core" % "4.0.0.201503231230-m1" - -// egit-github needs Gson, but doesn't explicitly require it -libraryDependencies += "com.google.code.gson" % "gson" % "2.3.1" +libraryDependencies += "com.jcabi" % "jcabi-github" % "0.23" libraryDependencies += "ch.qos.logback" % "logback-classic" % "1.1.2" diff --git a/src/main/resources/application.conf b/src/main/resources/application.conf index 64ff160..f870325 100644 --- a/src/main/resources/application.conf +++ b/src/main/resources/application.conf @@ -25,6 +25,7 @@ rorschach { default-port = 9090 squelch-invalid-http-logging = true close-bad-pull-requests = on + github-rate-limit-threshold = 10 github-repos-to-watch = ["twbs/bootstrap", "cvrebert/rorschach-test"] trusted-orgs = [ "twbs" ] username = throwaway9475947 diff --git a/src/main/scala/com/getbootstrap/rorschach/github/Credentials.scala b/src/main/scala/com/getbootstrap/rorschach/github/Credentials.scala new file mode 100644 index 0000000..add9893 --- /dev/null +++ b/src/main/scala/com/getbootstrap/rorschach/github/Credentials.scala @@ -0,0 +1,9 @@ +package com.getbootstrap.rorschach.github + +import com.jcabi.github.{Github, RtGithub} +import com.getbootstrap.rorschach.http.{UserAgent, SuperWire} + +case class Credentials(username: String, password: String) { + private def basicGithub: Github = new RtGithub(username, password) + def github(threshold: Int)(implicit userAgent: UserAgent): Github = new RtGithub(basicGithub.entry.through(classOf[SuperWire], userAgent, new Integer(threshold))) +} diff --git a/src/main/scala/com/getbootstrap/rorschach/github/GitHubActorWithLogging.scala b/src/main/scala/com/getbootstrap/rorschach/github/GitHubActorWithLogging.scala index af78ec9..c49fe92 100644 --- a/src/main/scala/com/getbootstrap/rorschach/github/GitHubActorWithLogging.scala +++ b/src/main/scala/com/getbootstrap/rorschach/github/GitHubActorWithLogging.scala @@ -1,11 +1,8 @@ package com.getbootstrap.rorschach.github -import org.eclipse.egit.github.core.client.GitHubClient import com.getbootstrap.rorschach.server.{Settings, ActorWithLogging} abstract class GitHubActorWithLogging extends ActorWithLogging { protected val settings = Settings(context.system) - protected val gitHubClient = new GitHubClient() - gitHubClient.setUserAgent(settings.UserAgent) - gitHubClient.setCredentials(settings.BotUsername, settings.BotPassword) + protected val gitHubClient = settings.github() } diff --git a/src/main/scala/com/getbootstrap/rorschach/github/GitHubPullRequestCommenter.scala b/src/main/scala/com/getbootstrap/rorschach/github/GitHubPullRequestCommenter.scala index 2084a95..e02413d 100644 --- a/src/main/scala/com/getbootstrap/rorschach/github/GitHubPullRequestCommenter.scala +++ b/src/main/scala/com/getbootstrap/rorschach/github/GitHubPullRequestCommenter.scala @@ -1,33 +1,27 @@ package com.getbootstrap.rorschach.github import scala.util.{Try,Failure,Success} -import org.eclipse.egit.github.core.service.{PullRequestService, IssueService} -import org.eclipse.egit.github.core.RepositoryId -import com.getbootstrap.rorschach.github.util.RichPullRequest +import com.jcabi.github.Coordinates.{Simple=>RepoId} +import com.getbootstrap.rorschach.github.implicits._ import com.getbootstrap.rorschach.server.Settings class GitHubPullRequestCommenter extends GitHubActorWithLogging { // val settings = Settings(context.system) - private def tryToCommentOn(repo: RepositoryId, prNum: PullRequestNumber, commentMarkdown: String) = { - val issueService = new IssueService(gitHubClient) - Try { issueService.createComment(repo, prNum.number, commentMarkdown) } + private def tryToCommentOn(repo: RepoId, prNum: PullRequestNumber, commentMarkdown: String) = { + Try { gitHubClient.repos.get(repo).issues.get(prNum.number).comments.post(commentMarkdown) } } - private def tryToClose(repo: RepositoryId, prNum: PullRequestNumber): Try[None.type] = { - val prService = new PullRequestService(gitHubClient) - val prTry = Try { prService.getPullRequest(repo, prNum.number) } match { - case fetchFail@Failure(exc) => { - log.error(exc, s"Error fetching pull request ${prNum} in order to close it") - fetchFail - } - case Success(pr) => { - pr.status = Closed - Try { prService.editPullRequest(repo, pr) } + private def tryToClose(repo: RepoId, prNum: PullRequestNumber): Try[Unit] = { + val closure = Try { gitHubClient.repos.get(repo).issues.get(prNum.number).smart.close() } + closure match { + case Failure(exc) => { + log.error(exc, s"Error closing pull request ${prNum}") } + case _ => {} } - prTry.map{ x => None } + closure } override def receive = { @@ -49,7 +43,7 @@ class GitHubPullRequestCommenter extends GitHubActorWithLogging { """.stripMargin tryToCommentOn(repo, prNum, commentMarkdown) match { - case Success(comment) => log.info(s"Successfully posted comment ${comment.getUrl} for ${prNum}") + case Success(comment) => log.info(s"Successfully posted comment ${comment.smart.url} for ${prNum}") case Failure(exc) => log.error(exc, s"Error posting comment for ${prNum}") } diff --git a/src/main/scala/com/getbootstrap/rorschach/github/PullRequestStatus.scala b/src/main/scala/com/getbootstrap/rorschach/github/PullRequestStatus.scala deleted file mode 100644 index e136f32..0000000 --- a/src/main/scala/com/getbootstrap/rorschach/github/PullRequestStatus.scala +++ /dev/null @@ -1,20 +0,0 @@ -package com.getbootstrap.rorschach.github - -object PullRequestStatus { - def apply(value: String): PullRequestStatus = { - value match { - case Open.Value => Open - case Closed.Value => Closed - case _ => throw new IllegalArgumentException(s"Invalid pull request status string: ${value}") - } - } -} -sealed trait PullRequestStatus { - val Value: String -} -object Open extends PullRequestStatus { - override val Value = "open" -} -object Closed extends PullRequestStatus { - override val Value = "closed" -} diff --git a/src/main/scala/com/getbootstrap/rorschach/github/implicits/package.scala b/src/main/scala/com/getbootstrap/rorschach/github/implicits/package.scala new file mode 100644 index 0000000..50fa44f --- /dev/null +++ b/src/main/scala/com/getbootstrap/rorschach/github/implicits/package.scala @@ -0,0 +1,18 @@ +package com.getbootstrap.rorschach.github + +import com.jcabi.github.{CommitsComparison,Issue,Pull} +import com.jcabi.github.CommitsComparison.{Smart=>SmartCommitsComparison} +import com.jcabi.github.Issue.{Smart=>SmartIssue} +import com.jcabi.github.Pull.{Smart=>SmartPull} + +package object implicits { + implicit class RichIssue(issue: Issue) { + def smart: SmartIssue = new SmartIssue(issue) + } + implicit class RichCommitsComparison(comparison: CommitsComparison) { + def smart: SmartCommitsComparison = new SmartCommitsComparison(comparison) + } + implicit class RichPull(pull: Pull) { + def smart: SmartPull = new SmartPull(pull) + } +} diff --git a/src/main/scala/com/getbootstrap/rorschach/github/util/package.scala b/src/main/scala/com/getbootstrap/rorschach/github/util/package.scala index 9a2aa6e..6ebd77f 100644 --- a/src/main/scala/com/getbootstrap/rorschach/github/util/package.scala +++ b/src/main/scala/com/getbootstrap/rorschach/github/util/package.scala @@ -1,23 +1,22 @@ package com.getbootstrap.rorschach.github -import org.eclipse.egit.github.core._ +import com.jcabi.github.{Issue,Pull} +import com.jcabi.github.Issue.{Smart=>SmartIssue} +import com.jcabi.github.Pull.{Smart=>SmartPullRequest} package object util { - implicit class RichRepository(repo: Repository) { + /*implicit class RichRepository(repo: Repository) { def repositoryId: RepositoryId = new RepositoryId(repo.getOwner.getLogin, repo.getName) - } + }*/ implicit class RichPullRequestMarker(marker: PullRequestMarker) { def commitSha: CommitSha = new CommitSha(marker.getSha) } implicit class RichCommitFile(file: CommitFile) { def status: CommitFileStatus = CommitFileStatus(file.getStatus) } - implicit class RichPullRequest(pr: PullRequest) { - def number: PullRequestNumber = PullRequestNumber(pr.getNumber).get + /*implicit class RichPullRequest(pr: Pull) { + def prNumber: PullRequestNumber = PullRequestNumber(pr.number).get + def smart: SmartPullRequest = new SmartPullRequest(pr) + }*/ - def status_= (value: PullRequestStatus) { - pr.setState(value.Value) - } - def status = PullRequestStatus(pr.getState) - } } diff --git a/src/main/scala/com/getbootstrap/rorschach/http/SuperWire.scala b/src/main/scala/com/getbootstrap/rorschach/http/SuperWire.scala new file mode 100644 index 0000000..f098cd9 --- /dev/null +++ b/src/main/scala/com/getbootstrap/rorschach/http/SuperWire.scala @@ -0,0 +1,20 @@ +package com.getbootstrap.rorschach.http + +import java.util.{Collection=>JavaCollection} +import java.util.Map.{Entry=>MapEntry} +import java.io.InputStream +import com.jcabi.github.wire.RetryCarefulWire +import com.jcabi.http.{Wire,Request,Response} + +case class SuperWire(private val wire: Wire, userAgent: UserAgent, threshold: Int) extends Wire { + private val wrappedWire = UserAgentWire(wire = new RetryCarefulWire(wire, threshold), userAgent = userAgent) + + @Override + def send( + request: Request, + home: String, + method: String, + headers: JavaCollection[MapEntry[String, String]], + content: InputStream + ): Response = wrappedWire.send(request, home, method, headers, content) +} diff --git a/src/main/scala/com/getbootstrap/rorschach/http/UserAgent.scala b/src/main/scala/com/getbootstrap/rorschach/http/UserAgent.scala new file mode 100644 index 0000000..cb71beb --- /dev/null +++ b/src/main/scala/com/getbootstrap/rorschach/http/UserAgent.scala @@ -0,0 +1,3 @@ +package com.getbootstrap.rorschach.http + +case class UserAgent(userAgent: String) diff --git a/src/main/scala/com/getbootstrap/rorschach/http/UserAgentWire.scala b/src/main/scala/com/getbootstrap/rorschach/http/UserAgentWire.scala new file mode 100644 index 0000000..697c14e --- /dev/null +++ b/src/main/scala/com/getbootstrap/rorschach/http/UserAgentWire.scala @@ -0,0 +1,25 @@ +package com.getbootstrap.rorschach.http + +import java.util.{Collection=>JavaCollection} +import java.util.Map.{Entry=>MapEntry} +import java.io.InputStream +import scala.collection.JavaConverters._ +import com.jcabi.http._ + +object UserAgentWire { + private val userAgentHeader = "User-Agent" +} +case class UserAgentWire(private val wire: Wire, userAgent: UserAgent) extends Wire { + @Override + def send( + request: Request, + home: String, + method: String, + headers: JavaCollection[MapEntry[String, String]], + content: InputStream + ): Response = { + val header = new ImmutableHeader(UserAgentWire.userAgentHeader, userAgent.userAgent) + val newHeaders = header +: headers.asScala.filter{ _.getKey != UserAgentWire.userAgentHeader}.toSeq + wire.send(request, home, method, newHeaders.asJava, content) + } +} diff --git a/src/main/scala/com/getbootstrap/rorschach/server/GitHubPullRequestWebHooksDirectives.scala b/src/main/scala/com/getbootstrap/rorschach/server/GitHubPullRequestWebHooksDirectives.scala index 6ea7d47..47ce9fc 100644 --- a/src/main/scala/com/getbootstrap/rorschach/server/GitHubPullRequestWebHooksDirectives.scala +++ b/src/main/scala/com/getbootstrap/rorschach/server/GitHubPullRequestWebHooksDirectives.scala @@ -3,8 +3,6 @@ package com.getbootstrap.rorschach.server import scala.util.{Success, Failure, Try} import spray.routing.{Directive1, ValidationRejection} import spray.routing.directives.{BasicDirectives, RouteDirectives} -import org.eclipse.egit.github.core.event.PullRequestPayload -import org.eclipse.egit.github.core.client.GsonUtils trait GitHubPullRequestWebHooksDirectives { import RouteDirectives.reject diff --git a/src/main/scala/com/getbootstrap/rorschach/server/PullRequestEventHandler.scala b/src/main/scala/com/getbootstrap/rorschach/server/PullRequestEventHandler.scala index 055fb5c..686937c 100644 --- a/src/main/scala/com/getbootstrap/rorschach/server/PullRequestEventHandler.scala +++ b/src/main/scala/com/getbootstrap/rorschach/server/PullRequestEventHandler.scala @@ -5,25 +5,25 @@ import com.getbootstrap.rorschach.auditing.{BaseAndHeadBranchesAuditor, Modified import scala.collection.JavaConverters._ import scala.util.{Try,Success,Failure} import akka.actor.ActorRef -import org.eclipse.egit.github.core._ -import org.eclipse.egit.github.core.service.{CommitService, OrganizationService} +import com.jcabi.github.User +import com.jcabi.github.Coordinates.{Simple=>RepoId} +import com.jcabi.github.Pull.{Smart=>PullRequest} import com.getbootstrap.rorschach.github._ -import com.getbootstrap.rorschach.github.util._ +import com.getbootstrap.rorschach.github.implicits._ class PullRequestEventHandler(commenter: ActorRef) extends GitHubActorWithLogging { - private def modifiedFilesFor(repoId: RepositoryId, base: CommitSha, head: CommitSha) = { - val commitService = new CommitService(gitHubClient) - Try { commitService.compare(repoId, base.sha, head.sha) }.map { comparison => - val affectedFiles = comparison.getFiles.asScala + private def modifiedFilesFor(repoId: RepoId, base: CommitSha, head: CommitSha) = { + Try { gitHubClient.repos.get(repoId).commits.compare(base.sha, head.sha) }.map{ comparison => + val affectedFiles = comparison.smart.files.asScala affectedFiles.filter{ _.status == Modified }.map{ _.getFilename }.toSet[String] } } def isTrusted(user: User): Boolean = { - val orgService = new OrganizationService(gitHubClient) settings.TrustedOrganizations.exists{ org => Try{ orgService.isPublicMember(org, user.getLogin) }.toOption.getOrElse(false) } } + // FIXME: caller needs to pass SmartPull override def receive = { case pr: PullRequest => { val bsBase = pr.getBase @@ -52,7 +52,7 @@ class PullRequestEventHandler(commenter: ActorRef) extends GitHubActorWithLoggin val allMessages = fileMessages ++ branchMessages if (allMessages.nonEmpty) { - commenter ! PullRequestFeedback(destinationRepo, pr.number, pr.getUser, allMessages) + commenter ! PullRequestFeedback(destinationRepo, pr.number, pr., allMessages) } else { log.info(s"Repo ${destinationRepo} ${pr.number} successfully passed all audits.") diff --git a/src/main/scala/com/getbootstrap/rorschach/server/Settings.scala b/src/main/scala/com/getbootstrap/rorschach/server/Settings.scala index f8b724e..3033167 100644 --- a/src/main/scala/com/getbootstrap/rorschach/server/Settings.scala +++ b/src/main/scala/com/getbootstrap/rorschach/server/Settings.scala @@ -8,15 +8,21 @@ import akka.actor.ExtensionId import akka.actor.ExtensionIdProvider import akka.actor.ExtendedActorSystem import akka.util.ByteString -import org.eclipse.egit.github.core.RepositoryId +import com.jcabi.github.Github +import com.jcabi.github.Coordinates.{Simple=>RepoId} +import com.getbootstrap.rorschach.github.Credentials +import com.getbootstrap.rorschach.http.{UserAgent=>UA} import com.getbootstrap.rorschach.util.Utf8String class SettingsImpl(config: Config) extends Extension { - val repoIds: Set[RepositoryId] = config.getStringList("rorschach.github-repos-to-watch").asScala.toSet.map{ (repoFullName: String) => RepositoryId.createFromId(repoFullName) } + val repoIds: Set[RepoId] = config.getStringList("rorschach.github-repos-to-watch").asScala.toSet[String].map{ new RepoId(_) } val BotUsername: String = config.getString("rorschach.username") - val BotPassword: String = config.getString("rorschach.password") + private val botPassword: String = config.getString("rorschach.password") + private val botCredentials: Credentials = Credentials(username = BotUsername, password = botPassword) + private val githubRateLimitThreshold: Int = config.getInt("rorschach.github-rate-limit-threshold") + def github(): Github = botCredentials.github(githubRateLimitThreshold)(UserAgent) val WebHookSecretKey: ByteString = ByteString(config.getString("rorschach.web-hook-secret-key").utf8Bytes) - val UserAgent: String = config.getString("spray.can.client.user-agent-header") + val UserAgent: UA = UA(config.getString("spray.can.client.user-agent-header")) val DefaultPort: Int = config.getInt("rorschach.default-port") val SquelchInvalidHttpLogging: Boolean = config.getBoolean("rorschach.squelch-invalid-http-logging") val CloseBadPullRequests: Boolean = config.getBoolean("rorschach.close-bad-pull-requests") -- cgit v1.2.3