Add support for decorating Pull Requests

[mc1arke]

This change introduces the APIs and components to allow the collection of issues and metrics during analysis, and an initial implementation for decorating requests on Github using these metrics. The Github implementation uses v4 of the Github API to create the 'check run' with the analysis results, but v3 of the API to acquire the relevant application authentication tokens since application authentication cannot currently be created/retrieved through the v4 API.

The provided implementation currently only supports the same level of reporting as provided by the official SonarQube branch decoration, but without including any images in the summary, and the configuration properties match those in the official documentation for setting up SonarQube for application based authentication against Github.

[4n4n4s]

LGTM

@mc1arke When do you expect that this is merged? Are there still things that you want to refactor or add (tests)? I would like to add the Bitbucket Server integration including comments on file level.

[majorvin]

@4n4n4s when i run it locally, some tests are failing.
@mc1arke since this is an open source project, I think you can integrate your project with some of the CICD tools to have your PR run your tests. Thanks!

[4n4n4s]

@majorvin
I worked on a Bitbucket Server integration today based on this PR.
It's incuding commenting on file level (this still has some flaws).
However the basic thing is working on my test project.

Changed the way that the overall comment is built to use a builder as this was very hard to read.

I also had to rework PostAnalysisIssueVisitor a bit so that we can keep the path to the analysed file as this is required for a file comment.

@mc1arke can we add the changes I made to this PR?

[mc1arke]

since this is an open source project, I think you can integrate your project with some of the CICD tools to have your PR run your tests. Thanks!

@majorvin What tests are failing? When built with Java 9 or above ClassReferenceElevatedClassLoaderFactoryTest.testClassloaderReturnedOnHappyPath fails due to it not setting up class loading in a way compatible with the new jdk.internal.loader.BuiltinClassLoader introduced in Java 9. I'll look to fix this at some point in the near future, but currently running the tests with Java 8 should prove the necessary functionality.

I've got the travis-ci branch configured to run the build using Java 8 and Java 11. Once the class loading test is fixed I'll merge this back to master.

can we add the changes I made to this PR?

@4n4n4s If you think you're code is ready for review/merge then please raise a Pull Request targeting the pr-decoration branch. I've got a number of changes locally to refactor the code for test-ability and maintainability, but haven't pushed it yet as I wanted to get the test coverage increased. Therefore, you may need to do some re-basing as I tidy up my initial commit; but we can deal with that if we need to.

[4n4n4s]

@mc1arke thx for your reply. I do agree. It should not make too many problems to update the classes I touched with your changes.

When do you think that your changes will be ready for pushing it to this PR?

I will create the PR targeting this branch hopefully today.

[tisoft]

Added an implementation for GitLab in #34 to make the trio complete. 😄

Maybe we should have a central class that handles the generation of the text for the summary/file comments, so that it is not repeated in each implementation. Or switch to a template based solution as done here.

[coreinsane]

Will you plan to support Azure DevOps Server PR Decoration?

[mc1arke]

@tisoft thanks for the contribution. I've had limited time for properly being involved in these changes in the past month but hope to get back into it this week. I'll take a look at your changes as-soon-as-possible and give you any feedback. As you've suggested, I've refactored my initial Pull Request to split up some of the generation and gathering steps and the actual decoration step which should simplify what needs implemented in each case. I'll probably raise the changes under another branch so @4n4n4s can rebase cleanly rather than dealing with re-writing history against the current branch.

@coreinsane I've never use Azure DevOps and am happy to look at supporting it, but would need the community to provide support in testing it and confirming any usage patterns that need to be followed.

[rverma-nikiai]

@mc1arke Tried to run the branch with additional parameters as

sonar.pullrequest.branch=pr-decoration
sonar.pullrequest.key=2
sonar.pullrequest.provider=Github
sonar.github.repository=rverma/sonarqube-community-branch-plugin

But getting
No decorator found for this Pull Request.

[tisoft]

@rverma-nikiai I had the same behavior if the sonar.branch.name parameter is set while trying to do pull request analysis.

[rverma-nikiai]

Wondering what is a good way to debug sonarqube plugins. I tried to add sonar.web.javaAdditionalOpts=-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=8000 and attached remote debugger to this project. Unfortunately it didn't work.

@tisoft Did it work if you stop passing sonar.branch.name?

[tisoft]

Yes that helped.

[rverma-nikiai]

Removing that did regitered the pullrequest decorator. But now getting exception as Caused by: java.lang.IllegalStateException: File 'ComponentImpl{type=FILE, status=SAME, name='src/main/java/com/github/mc1arke/sonarqube/plugin/ce/CommunityBranchEditionProvider.java', dbKey='com.github.mc1arke.sonarqube.plugin:sonarqube-community-branch-plugin:src/main/java/com/github/mc1arke/sonarqube/plugin/ce/CommunityBranchEditionProvider.java', key='com.github.mc1arke.sonarqube.plugin:sonarqube-community-branch-plugin:src/main/java/com/github/mc1arke/sonarqube/plugin/ce/CommunityBranchEditionProvider.java', uuid='AW3D4QucCpRUzI-za-Tf', description='null', children=[], projectAttributes=null, reportAttributes=ReportAttributes{ref=28, scmPath='src/main/java/com/github/mc1arke/sonarqube/plugin/ce/CommunityBranchEditionProvider.java'}, fileAttributes=FileAttributes{languageKey='java', unitTest=false, lines=33}}' has no source code

[rverma-nikiai]

Found that above issue was cause of an empty line at EOF. Now the scan run successfully but I don't see anything in my github PR.

[mc1arke]

You're more likely to want sonar.ce.javaAdditionalOpts if trying to debug the plugin during PR decoration as this is executed by the Compute Engine (CE) component if SonarQube:

• Classes in com.github.mc1arke.sonarqube.plugin.ce can be debugged using sonar.ce.javaAdditionalOpts
• Classes in com.github.mc1arke.sonarqube.plugin.server can be debugged using sonar.web.javaAdditionalOpts
• Classes in com.github.mc1arke.sonarqube.plugin.scanner can be debugged using mvnDebug or similar to run the scan

Other classes are shared across the different components in the infrastructure so debugging them will depend on what feature you're trying to use/test.

[IMKnysh]

It will be nice to have coverage analyses result in comments on PR. What do you think about?

[pierCo]

I tried to test the PR decoration (I build manually your code) to validate this PR and the behaviour, but without any success.

My conclusion is that I did not pushed parameters from my maven command to Sonar.

Below my mvn command:

 mvn clean package sonar:sonar \
    -Dsonar.pullrequest.provider=Github \
    -Dsonar.host.url=https://sonar-poc-server.cleverapps.io \
    -Dsonar.pullrequest.github.repository=pierCo/sonar-poc-2 \
    -Dsonar.pullrequest.base=master \
    -Dsonar.pullrequest.branch=fix-issues \
    -Dsonar.pullrequest.key=1

In Sonar logs: No decorator found for this Pull Request.

To move forward, I force the parameter sonar.pullrequest.provider (defined in my command line) value directly in the source code (edit the file PullRequestPostAnalysisTask.java).
But another empty parameter appears: sonar.pullrequest.github.repository (also defined in my command line)

So, I think I have miss something somewhere, Do you have an idea?

Information: I test with Sonar 7.9.

[hnrkdmsk]

I tried to test the PR decoration (I build manually your code) to validate this PR and the behaviour, but without any success.

My conclusion is that I did not pushed parameters from my maven command to Sonar.

Below my mvn command:

 mvn clean package sonar:sonar \
    -Dsonar.pullrequest.provider=Github \
    -Dsonar.host.url=https://sonar-poc-server.cleverapps.io \
    -Dsonar.pullrequest.github.repository=pierCo/sonar-poc-2 \
    -Dsonar.pullrequest.base=master \
    -Dsonar.pullrequest.branch=fix-issues \
    -Dsonar.pullrequest.key=1

In Sonar logs: No decorator found for this Pull Request.

To move forward, I force the parameter sonar.pullrequest.provider (defined in my command line) value directly in the source code (edit the file PullRequestPostAnalysisTask.java).
But another empty parameter appears: sonar.pullrequest.github.repository (also defined in my command line)

So, I think I have miss something somewhere, Do you have an idea?

Information: I test with Sonar 7.9.

You have to set these values on SonarQube side in your project settings. It would be nice to set these values with the maven command.

See also: https://docs.sonarqube.org/latest/analysis/pull-request/

[artemy-osipov]

@mc1arke Will you plan to rebase/merge this PR or merge other PRs to current branch for nearby future?

[pierCo]

I tried to test the PR decoration (I build manually your code) to validate this PR and the behaviour, but without any success.
My conclusion is that I did not pushed parameters from my maven command to Sonar.
Below my mvn command:

 mvn clean package sonar:sonar \
    -Dsonar.pullrequest.provider=Github \
    -Dsonar.host.url=https://sonar-poc-server.cleverapps.io \
    -Dsonar.pullrequest.github.repository=pierCo/sonar-poc-2 \
    -Dsonar.pullrequest.base=master \
    -Dsonar.pullrequest.branch=fix-issues \
    -Dsonar.pullrequest.key=1

In Sonar logs: No decorator found for this Pull Request.
To move forward, I force the parameter sonar.pullrequest.provider (defined in my command line) value directly in the source code (edit the file PullRequestPostAnalysisTask.java).
But another empty parameter appears: sonar.pullrequest.github.repository (also defined in my command line)
So, I think I have miss something somewhere, Do you have an idea?
Information: I test with Sonar 7.9.

You have to set these values on SonarQube side in your project settings. It would be nice to set these values with the maven command.

See also: https://docs.sonarqube.org/latest/analysis/pull-request/

Thanks, it works well for me.

I'll continue to test with more cases.