New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds support for Bitbucket Cloud Code Insights #185
Adds support for Bitbucket Cloud Code Insights #185
Conversation
@mc1arke I'm done with this. Feel free to review it once you have some time. @singhsoldier13, @goober, @jburgel-davinci, @yeroc If there is a chance it would be awesome if you could also go for an additional test run with this version of bitbucket cloud PR decoration. For the token please use For the URL please use |
...m/github/mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/client/BitbucketServerClient.java
Show resolved
Hide resolved
src/main/java/com/github/mc1arke/sonarqube/plugin/CommunityBranchPlugin.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've given it a once-over review and it looks promising, but there are a few things that are likely to need changed. Please also have a look as Sonarcloud's review result and fix or respond to any comments it's raised. I'll give this a more thorough review once I've had a chance to run it.
...github/mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/client/AbstractBitbucketClient.java
Outdated
Show resolved
Hide resolved
...github/mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/client/AbstractBitbucketClient.java
Outdated
Show resolved
Hide resolved
...github/mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/client/AbstractBitbucketClient.java
Outdated
Show resolved
Hide resolved
...om/github/mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/client/BitbucketCloudClient.java
Outdated
Show resolved
Hide resolved
...om/github/mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/client/BitbucketCloudClient.java
Outdated
Show resolved
Hide resolved
...a/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/client/model/IAnnotation.java
Outdated
Show resolved
Hide resolved
...e/sonarqube/plugin/ce/pullrequest/bitbucket/client/model/cloud/CloudCreateReportRequest.java
Show resolved
Hide resolved
.../mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/client/BitbucketClientFacadeUnitTest.java
Outdated
Show resolved
Hide resolved
src/main/java/com/github/mc1arke/sonarqube/plugin/CommunityBranchPlugin.java
Show resolved
Hide resolved
...e/sonarqube/plugin/ce/pullrequest/bitbucket/client/model/cloud/CloudCreateReportRequest.java
Outdated
Show resolved
Hide resolved
62c8687
to
b66a86c
Compare
@mc1arke I did a major refactoring considering your point about the race condition. Would you mind having a quick check again - I'd then finish all the tests and take care of sonar. I personally like it way more now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good from a quick read, although I've not run it yet so can't confirm it's functionally what I expect.
I've added a set of suggestions around how you've currently managed the BitbucketClient
instance, sorry if I'm explaining an obvious concept, but didn't want to make a suggestion that wasn't understandable.
.../github/mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/BitbucketPullRequestDecorator.java
Outdated
Show resolved
Hide resolved
.../github/mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/BitbucketPullRequestDecorator.java
Outdated
Show resolved
Hide resolved
.../github/mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/BitbucketPullRequestDecorator.java
Outdated
Show resolved
Hide resolved
.../github/mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/BitbucketPullRequestDecorator.java
Outdated
Show resolved
Hide resolved
.../github/mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/BitbucketPullRequestDecorator.java
Outdated
Show resolved
Hide resolved
.../github/mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/BitbucketPullRequestDecorator.java
Outdated
Show resolved
Hide resolved
.../github/mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/BitbucketPullRequestDecorator.java
Outdated
Show resolved
Hide resolved
...ava/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/client/BitbucketClient.java
Show resolved
Hide resolved
...ava/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/client/BitbucketClient.java
Outdated
Show resolved
Hide resolved
...b/mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/client/model/BitbucketConfiguration.java
Outdated
Show resolved
Hide resolved
8d352e3
to
360f2a2
Compare
@mc1arke Thanks for the review again! I integrated your suggestions and fixed/adjusted the tests accordingly. Give it a go when you got some time :). Please note that you need to set CloudCreateReportRequest.link to some valid link other than localhost for the API to fully function. Otherwise you get below error:
I'll look at sonars report later since it's usually quite slow. I also think its relatively easy to merge this into master afterwards as you could basically just replace the UnifyConfiguration with the AlmSettingDto. |
@marvin-w Would love to try this out. It looks like we'll need to be running SonarQube 7.8 or newer to try this out? We're running 7.7 right now so will need to upgrade first I guess? |
Yes, you'll need to upgrade I believe. I didn't test it in lower versions. |
@mc1arke would you mind retriggering the sonar analysis? For some reason it doesn't analyse the code in this branch. |
@mc1arke I took care of the sonar stuff. I also tried to increase the coverage with my last commit. I changed the underlying Mockito engine so that it now is possible to mock final classes (https://github.com/mockito/mockito/wiki/What%27s-new-in-Mockito-2#unmockable). Unfortunetly, this new engine broke some other tests which I fixed too. I deliberately put all that stuff in another commit as I wasn't sure whether you're up for that kind of change. Please restart sonarqube and then lets have a look at the coverage. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few more comments - nothing too major; Your change generally looks good.
I'm not a fan of mocking final classes but I don't think you have much other option given the tight dependency on OkHttpClient. I'll need to look at how we can consistently use Http Clients in the plugin in a testable way, but that would be a separate change and not one that should impact your PR.
...ub/mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/client/model/cloud/CloudAnnotation.java
Outdated
Show resolved
Hide resolved
...e/sonarqube/plugin/ce/pullrequest/bitbucket/client/model/cloud/CloudCreateReportRequest.java
Outdated
Show resolved
Hide resolved
...b/mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/client/BitbucketCloudClientUnitTest.java
Outdated
Show resolved
Hide resolved
...b/mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/client/BitbucketCloudClientUnitTest.java
Outdated
Show resolved
Hide resolved
...b/mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/client/BitbucketCloudClientUnitTest.java
Outdated
Show resolved
Hide resolved
.../mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/client/BitbucketServerClientUnitTest.java
Show resolved
Hide resolved
.../mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/client/BitbucketServerClientUnitTest.java
Outdated
Show resolved
Hide resolved
.../mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/client/BitbucketServerClientUnitTest.java
Outdated
Show resolved
Hide resolved
.../mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/client/BitbucketServerClientUnitTest.java
Outdated
Show resolved
Hide resolved
.../mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/client/BitbucketServerClientUnitTest.java
Outdated
Show resolved
Hide resolved
9387918
to
559e565
Compare
@mc1arke Thank you for your review. I took care of all the points you mentioned. Would you mind retriggering the sonar analysis? I added missing tests for the class Also, what is your opinion concerning the documentation mentioned in the description of this pull request? Since this feature is not officially supported by SonarQube (i.e. no official docs) it might become necessary. I rebased all the commits into one and did a final test after the refactoring to be sure that the functionality is still working as intended. |
@mc1arke You're probably a busy man and I'm sorry for bothering you again but do you have an ETA on when we can expect this to be merged and released? |
I've tested your PR with Sonarqube 7.9.3 with Bitbucket Cloud and it seems to work. Except for the annotations. I don't know if it's something I need to change in bitbucket, but the annotations are not shown in the comments. |
Cool, glad it works for you. There are no comments on files. The annotations belong to the report. In the report you can go directly to the specific file and line. Please look at my screens in the PR description. If you still think there is an issue please add some more information to it, maybe a screenshot. |
You are right, sorry. I was looking at screenshots of the Bitbucket Server (adds annotations to the diff section) Everything worked 👍 Maybe there should be some information for the format the bitbucket token (base64 user:pass), because this information will be lost in this PR. |
Exactly that is what I meant in #185 (comment). |
559e565
to
2f0f483
Compare
This commit provides support for the newly created bitbucket cloud code insights API endpoints. The implementation has been done under the consideration that in newer versions no dedicated ALM support for bitbucket cloud exists, thus this implementation is minimal invasive. One thing to note here: * For local testing the link on CloudCreateReportRequest has to be set manually to a non localhost URL since bitbuckets API doesn't support it.
2f0f483
to
faa46b8
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs |
Merged. Many thanks for the contribution! Now that you're a contributor to this repository, I believe you should have access to edit the Wiki. If you don't then please let me know and I'll look at options that allow easier contribution to documentation. |
@mc1arke Thanks for taking care of it. I have two more things to mention here.
(Pushing to https://github.com/mc1arke/sonarqube-community-branch-plugin.wiki.git
|
Hi @marvin-w / @mc1arke I am trying to use it for PR Decoration for gradle based Java Project. I have pulled the latest code build and deployed in the server and can it is properly showing the branches. I need help in setting it up for PR Decoration. The current Wiki does not have clear instruction to set this up. I will appreciate your help in this regard.
|
@anuragpathak21 This feature is currently only supported in the LTS 7.9.x version. |
I am planning to downgrade to LTS 7.9. Will appreciate if you provide details of the setup. |
For the general setup of the PR decoration please refer to the official SonarQube documentation. For the specific feature you need a user with the "Read Repository" Scope. The token is defined as For anything else please refer to the respective tool documentation, i.e. if there's something wrong in Jenkins ask them or if you also believe there should be PR refs in bitbucket cloud then this is the right way for you: https://jira.atlassian.com/browse/BCLOUD-5814 (open since 2012... you gotta love atlassian for that). Without this it is very hard to integrate the PR decoration with Jenkins as there are no dedicated git refs for pull requests. |
@myDisconnect I suspect you're compiling the wrong branch. You need to clone/checkout and build the |
@mc1arke When can we expect this to be released? Is there anything we can help with? |
@marvin-w This has now been released in 1.3.2 (Sonarqube 7.8-8.0) and 1.4.0 (Sonarqube 8.1). Sorry for the delay with getting the releases out, and thanks once again for the contribution. On the documentation side: I'd like the documentation to be tightly linked to a release version of possible, so that it's clear how to configure a certain version of a plugin by looking at the relevant tag in Git. I'd therefore envisaged having a |
@mc1arke Thanks! No worries :) I'll also have a look concerning the documentation then. |
Could this information please be added to the readme.MD file? As it points to the official documentation, but the official version does not support BitBucket cloud, so the only place where to find these parameters right now is this pull-request. Also, does the |
@marvin-w if this PR also approve the Pull request (configurable) without any kind of comment was great. Because Repose code insight does not play in any way in branch restriction. |
I read your post three times and I don't get what you are trying to tell your audience in a PR that has been closed for over a year. |
I would say that approval should not be discarded as idea. Ok that bitbuket "Code insights" is the tool that bitbuket provides to report any kind of metric but it does not play any role in branch restrictions. This means for example that you could have a PR green just because the code compile, regardless if Sonarqube reports failures or not. PR could be merged and there are not option to prevent this. The approval instead could play a role in a branch restrictions. I mean the approval code could be left there (configurable) based on quality gate. |
Please open an issue for additional feature requests or bug reports. |
We have migrated the sonarqube version for a month from the mibexsoftware plugin to this one and I must say that most developers are still puzzled to understand when there are actually analisy sonars. Both because the Pull Request Decoration must be configured in each project (and this is a problem because it must be done after the project has been created) to see the reports and because it is not very visible in BB Cloud.
Sure I will do |
Issue #77 is still open and summarize above requirement |
Intention
This PR intends to provide support for the code insights feature for bitbucket cloud.
History
Since about 2 months bitbucket cloud also has a code insights feature that one can use. After checking the differences between the cloud and the server implementation it is however not possible to completely reuse the server logic due to renamed/missing fields in the cloud version.
I haven't changed any public facing API (configs, properties, decorators) in this PR which allows @mc1arke to keep it up to date with the other sonarqube versions.
Additions
Screenshots