Skip to content
This repository has been archived by the owner on Aug 4, 2021. It is now read-only.

Commit

Permalink
Fixed #13
Browse files Browse the repository at this point in the history
  • Loading branch information
mrueegg committed Apr 13, 2016
1 parent f31c4e8 commit 3bd5257
Show file tree
Hide file tree
Showing 8 changed files with 250 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ package ch.mibex.bitbucket.sonar.client
import javax.ws.rs.core.MediaType

import ch.mibex.bitbucket.sonar.{SonarBBPlugin, SonarBBPluginConfig}
import ch.mibex.bitbucket.sonar.utils.{LogUtils, JsonUtils}
import ch.mibex.bitbucket.sonar.utils.{JsonUtils, LogUtils}
import com.sun.jersey.api.client.config.{ClientConfig, DefaultClientConfig}
import com.sun.jersey.api.client.filter.LoggingFilter
import com.sun.jersey.api.client.{Client, ClientResponse, UniformInterfaceException}
import com.sun.jersey.api.client.{Client, ClientResponse, UniformInterfaceException, WebResource}
import org.slf4j.LoggerFactory
import org.sonar.api.BatchComponent
import org.sonar.api.batch.InstantiationStrategy
Expand Down Expand Up @@ -75,41 +75,38 @@ class BitbucketClient(config: SonarBBPluginConfig) extends BatchComponent {

}

// see https://bitbucket.org/site/master/issues/12567/amount-of-pull-request-comments-returned
def findOwnPullRequestComments(pullRequest: PullRequest): Seq[PullRequestComment] = {

def isFromUs(comment: Map[String, Any]): Boolean = {
val userName = Option(config.teamName()) match {
case Some(_) => config.teamName()
case None => config.accountName()
}
comment("user").asInstanceOf[Map[String, Any]]("username").asInstanceOf[String] equals userName
comment("author_info").asInstanceOf[Map[String, Any]]("username").asInstanceOf[String] equals userName
}

def fetchCommentsPage(start: Int): (Option[Int], Seq[PullRequestComment]) =
fetchPage(s"/pullrequests/${pullRequest.id}/comments",
response =>
for (comment <- response("values").asInstanceOf[Seq[Map[String, Any]]] if isFromUs(comment))
yield {
val pathAndLine = for {
inline <- comment.get("inline") map { _.asInstanceOf[Map[String, Any]] }
filePath <- inline.get("path") map { _.asInstanceOf[String] }
line <- inline.get("to") map { _.asInstanceOf[Int] }
} yield (filePath, line)
PullRequestComment(
commentId = comment("id").asInstanceOf[Int],
content = comment("content").asInstanceOf[Map[String, Any]]("raw").asInstanceOf[String],
filePath = pathAndLine.map { _._1 },
line = pathAndLine.map { _._2 }
)
},
pageNr = start
)
forEachResultPage(Seq[PullRequestComment](), (pageStart, comments: Seq[PullRequestComment]) => {
val (nextPageStart, newComments) = fetchCommentsPage(pageStart)
(nextPageStart, comments ++ newComments)
})
val response = v1Api.path(s"/pullrequests/${pullRequest.id}/comments")
.accept(MediaType.APPLICATION_JSON)
.`type`(MediaType.APPLICATION_JSON)
.get(classOf[String])
for {
comment <- JsonUtils.seqFromJson(response) if isFromUs(comment)
filePath <- comment.get("filename") map { _.asInstanceOf[String] }
line <- comment.get("line_to") map { _.asInstanceOf[Int] }
commentId <- comment.get("comment_id") map { _.asInstanceOf[Int] }
content <- comment.get("content").map { _.asInstanceOf[String] }
} yield PullRequestComment(
commentId = commentId,
content = content,
filePath = Option(filePath),
line = Option(line)
)
}

// create manually by
// curl -v -u YOUR_BITBUCKET_USER https://api.bitbucket.org/2.0/repositories/YOUR_USER_NAME/REPO_SLUG/pullrequests/PULL_REQUEST_ID/diff
// then copy the URL from the Location Header field in the HTTP response (LOCATION_URL below) and use that with the appended “?context=0” parameter for the second cURL:
// curl -u YOUR_BITBUCKET_USER LOCATION_URL?context=0
def getPullRequestDiff(pullRequest: PullRequest): String = {
// we do not want to use GET and Jersey's auto-redirect here because otherwise the context param
// is not passed to the new location
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ object GitDiffParser extends RegexParsers {
}
}

case class FromToRange(fromLineStart: Int, fromNumLines: Int = 0, toLineStart: Int, toNumLines: Int = 0)
case class FromToRange(fromLineStart: Int, fromNumLines: Option[Int], toLineStart: Int, toNumLines: Option[Int])

case class HunkHeader(fromToRange: FromToRange, context: Option[CtxLine])

Expand Down Expand Up @@ -129,7 +129,7 @@ object GitDiffParser extends RegexParsers {
def fromToRange: Parser[FromToRange] =
("@@ " ~> "-" ~> num) ~ opt("," ~> num) ~ (" +" ~> num) ~ opt("," ~> num) <~ " @@" ^^ {
case startLineFrom ~ optNumLinesFrom ~ startLineTo ~ optNumLinesTo =>
FromToRange(startLineFrom, optNumLinesFrom.getOrElse(0), startLineTo, optNumLinesTo.getOrElse(0))
FromToRange(startLineFrom, optNumLinesFrom, startLineTo, optNumLinesTo)
}

// @@ from-file-range to-file-range @@ [header]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import ch.mibex.bitbucket.sonar.SonarBBPlugin
import ch.mibex.bitbucket.sonar.cache.InputFileCache
import ch.mibex.bitbucket.sonar.client.{BitbucketClient, PullRequest}
import ch.mibex.bitbucket.sonar.diff.GitDiffParser.{BinaryDiff, Diff, GitDiff}
import ch.mibex.bitbucket.sonar.utils.LogUtils
import org.slf4j.LoggerFactory
import org.sonar.api.BatchComponent
import org.sonar.api.batch.InstantiationStrategy
import org.sonar.api.issue.Issue
Expand All @@ -13,13 +13,15 @@ import org.sonar.api.issue.Issue
@InstantiationStrategy(InstantiationStrategy.PER_BATCH)
class IssuesOnChangedLinesFilter(bitbucketClient: BitbucketClient,
inputFileCache: InputFileCache) extends BatchComponent {
private val logger = LoggerFactory.getLogger(getClass)

def filter(pullRequest: PullRequest, newIssues: Seq[Issue]): Seq[Issue] = {
val pullRequestDiff = bitbucketClient.getPullRequestDiff(pullRequest)
val diffs = parseOrFail(pullRequestDiff)
val issuesOnChangedLines = newIssues filter { i =>
val lineNr = Option(i.line()).flatMap(l => Option(l.toInt)).getOrElse(0)


inputFileCache.resolveRepoRelativePath(i.componentKey()) match {
case Some(filePath) =>
val isIssueOnChangedLines = (diff: Diff) => diff match {
Expand All @@ -40,12 +42,21 @@ class IssuesOnChangedLinesFilter(bitbucketClient: BitbucketClient,
private def isOnChangedLine(lineNr: Int, diff: GitDiff) =
diff.hunks.exists(c =>
lineNr >= c.hunkHeader.fromToRange.toLineStart
&& lineNr <= c.hunkHeader.fromToRange.toLineStart + c.hunkHeader.fromToRange.toNumLines
//@@ -12 +11,0 @@ public class App
// - double d = Double.longBitsToDouble(i); // Noncompliant
//@@ -16 +14,0 @@ public class App
// - System.out.println( "Hello World! " + d);
//@@ -26,2 +27 @@ public class App
// - int i = 100023;
// - System.exit(-1);
// + int i = 100023;
&& c.hunkHeader.fromToRange.toNumLines.getOrElse(1) > 0
&& lineNr <= c.hunkHeader.fromToRange.toLineStart + c.hunkHeader.fromToRange.toNumLines.getOrElse(0)
)

private def parseOrFail(diff: String) = GitDiffParser.parse(diff) match {
case Left(parsingFailure) =>
throw new RuntimeException(s"${SonarBBPlugin.PluginLogPrefix} Failed to parse git diff: ${parsingFailure.reason}")
throw new RuntimeException(s"${SonarBBPlugin.PluginLogPrefix} Failed to parse diff: ${parsingFailure.reason}")
case Right(gitDiffs) =>
gitDiffs
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,18 @@ class ReviewCommentsCreator(projectIssues: ProjectIssues,

def createOrUpdateComments(pullRequest: PullRequest,
existingReviewComments: Seq[PullRequestComment],
pullRequestResults: PullRequestReviewResults) = {
pullRequestResults: PullRequestReviewResults): Map[Int, PullRequestComment] = {
val commentsToBeAdded = processIssues(pullRequest, pullRequestResults)
val (commentsByPathAndLine, commentsToDelete) = processExistingComments(existingReviewComments)

commentsToBeAdded foreach { case (file, issuesByLine) =>
issuesByLine foreach { case (line, issues) =>

commentsByPathAndLine.get(file) match {
case Some(x) if x.contains(line) =>
case Some(commentsByLine) if commentsByLine.contains(line) => // already a comment on this line

val comment = commentsByPathAndLine(file)(line)

if (comment.content != issues.toString()) {
if (comment.content != issues.toString()) { // only update if comment is not equal to existing one
updateComment(pullRequest, issues.toString(), comment)
}

Expand All @@ -57,6 +57,12 @@ class ReviewCommentsCreator(projectIssues: ProjectIssues,
.withDefaultValue(new mutable.HashMap[Int, PullRequestComment]())
val reviewCommentsToBeDeleted = new mutable.HashMap[Int, PullRequestComment]()
val inlineComments = existingReviewComments filter { _.isInline }
if (logger.isDebugEnabled) {
logger.debug(LogUtils.f(s"Found ${inlineComments.size} existing inline comments:"))
inlineComments foreach { c =>
logger.debug(LogUtils.f(s" - ${c.filePath}:${c.line}: ${c.content}"))
}
}

inlineComments foreach { c =>
reviewCommentsToBeDeleted += c.commentId -> c
Expand Down Expand Up @@ -95,11 +101,12 @@ class ReviewCommentsCreator(projectIssues: ProjectIssues,
}

commentsToBeAdded(repoRelPath)(lineNr).append("\n\n" + SonarUtils.renderAsMarkdown(i, settings))

reviewResults.issueFound(i)
case None =>
logger.debug(LogUtils.f(s"No path resolved for ${i.componentKey()}"))
}

reviewResults.issueFound(i)
}

commentsToBeAdded.toMap
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class SonarReviewPostJob(bitbucketClient: BitbucketClient,
val ourComments = bitbucketClient.findOwnPullRequestComments(p)
val report = new PullRequestReviewResults(pluginConfig)
val commentsToDelete = reviewCommentsUpdater.createOrUpdateComments(p, ourComments, report)
deleteOutdatedComments(p, commentsToDelete)
deletePreviousComments(p, commentsToDelete)
deletePreviousGlobalComments(p, ourComments)
createGlobalComment(p, report)
approveOrUnapproveIfEnabled(p, report)
Expand Down Expand Up @@ -65,7 +65,7 @@ class SonarReviewPostJob(bitbucketClient: BitbucketClient,
}
}

private def deleteOutdatedComments(pullRequest: PullRequest, commentsToDelete: Map[Int, PullRequestComment]) {
private def deletePreviousComments(pullRequest: PullRequest, commentsToDelete: Map[Int, PullRequestComment]) {
commentsToDelete foreach { case (commentId, comment) =>
if (comment.content.startsWith(SonarUtils.sonarMarkdownPrefix())) {
bitbucketClient.deletePullRequestComment(pullRequest, commentId)
Expand Down
2 changes: 2 additions & 0 deletions src/main/scala/ch/mibex/bitbucket/sonar/utils/JsonUtils.scala
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,6 @@ object JsonUtils {

def mapFromJson(json: String): Map[String, Any] = json.parseJson.convertTo[Map[String, Any]]

def seqFromJson(json: String): Seq[Map[String, Any]] = json.parseJson.convertTo[Seq[Map[String, Any]]]

}
Loading

0 comments on commit 3bd5257

Please sign in to comment.