ezienecker commented 5 years ago

Enable discussion support see issue 186 for more details. To merge this pull request the https://github.com/gabrie-allaigre/java-gitlab-api must be released first!

alex-poliukh commented 5 years ago

@gabrie-allaigre is that possible to get this soon?

antik-x commented 5 years ago

Throws an exception when executing the following code:

Paged<GitlabMergeRequest> mergeRequests = gitLabAPIV4.getGitLabAPIMergeRequest()
                    .getAllProjectMergeRequestsBySourceBranchAndCommitShaWithStateOpen(projectId, branch, commitSha, null);

Exception information:

com.talanlabs.gitlab.api.v4.GitLabAPIException: null
        at com.talanlabs.gitlab.api.v4.http.GitLabHTTPRequestor.handleAPIError(GitLabHTTPRequestor.java:397)
        at com.talanlabs.gitlab.api.v4.http.GitLabHTTPRequestor.toPaged(GitLabHTTPRequestor.java:168)
        at com.talanlabs.gitlab.api.v4.services.GitLabAPIMergeRequest.getAllProjectMergeRequestsBySourceBranchAndCommitShaWithStateOpen(GitLabAPIMergeRequest.java:52)
        at com.talanlabs.sonar.plugins.gitlab.GitLabApiV4Wrapper.createReviewDiscussion(GitLabApiV4Wrapper.java:327)
        at com.talanlabs.sonar.plugins.gitlab.GitLabApiV4Wrapper.createOrUpdateReviewComment(GitLabApiV4Wrapper.java:302)
        at com.talanlabs.sonar.plugins.gitlab.CommitFacade.createOrUpdateReviewComment(CommitFacade.java:160)
        at com.talanlabs.sonar.plugins.gitlab.ReporterBuilder.updateReviewCommentsPerInline(ReporterBuilder.java:193)
        at com.talanlabs.sonar.plugins.gitlab.ReporterBuilder.updateReviewComments(ReporterBuilder.java:180)
        at com.talanlabs.sonar.plugins.gitlab.ReporterBuilder.updateReviewComments(ReporterBuilder.java:172)
        at com.talanlabs.sonar.plugins.gitlab.ReporterBuilder.updateReviewComments(ReporterBuilder.java:165)
        at com.talanlabs.sonar.plugins.gitlab.ReporterBuilder.build(ReporterBuilder.java:76)
        at com.talanlabs.sonar.plugins.gitlab.CommitPublishPostJob.execute(CommitPublishPostJob.java:100)
        at org.sonar.scanner.postjob.PostJobWrapper.execute(PostJobWrapper.java:46)
        at org.sonar.scanner.phases.PostJobsExecutor.execute(PostJobsExecutor.java:51)
        at org.sonar.scanner.phases.PostJobsExecutor.execute(PostJobsExecutor.java:42)
        at org.sonar.scanner.phases.AbstractPhaseExecutor.execute(AbstractPhaseExecutor.java:80)
        at org.sonar.scanner.scan.ModuleScanContainer.doAfterStart(ModuleScanContainer.java:164)
        at org.sonar.core.platform.ComponentContainer.startComponents(ComponentContainer.java:136)
        at org.sonar.core.platform.ComponentContainer.execute(ComponentContainer.java:122)
        at org.sonar.scanner.scan.ProjectScanContainer.scan(ProjectScanContainer.java:319)
        at org.sonar.scanner.scan.ProjectScanContainer.scanRecursively(ProjectScanContainer.java:314)
        at org.sonar.scanner.scan.ProjectScanContainer.doAfterStart(ProjectScanContainer.java:288)
        at org.sonar.core.platform.ComponentContainer.startComponents(ComponentContainer.java:136)
        at org.sonar.core.platform.ComponentContainer.execute(ComponentContainer.java:122)
        at org.sonar.scanner.task.ScanTask.execute(ScanTask.java:48)
        at org.sonar.scanner.task.TaskContainer.doAfterStart(TaskContainer.java:82)
        at org.sonar.core.platform.ComponentContainer.startComponents(ComponentContainer.java:136)
        at org.sonar.core.platform.ComponentContainer.execute(ComponentContainer.java:122)
        at org.sonar.scanner.bootstrap.GlobalContainer.executeTask(GlobalContainer.java:131)
        at org.sonar.batch.bootstrapper.Batch.doExecuteTask(Batch.java:116)
        at org.sonar.batch.bootstrapper.Batch.execute(Batch.java:71)
        at org.sonarsource.scanner.api.internal.batch.BatchIsolatedLauncher.execute(BatchIsolatedLauncher.java:46)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.sonarsource.scanner.api.internal.IsolatedLauncherProxy.invoke(IsolatedLauncherProxy.java:60)
        at com.sun.proxy.$Proxy0.execute(Unknown Source)
        at org.sonarsource.scanner.api.EmbeddedScanner.doExecute(EmbeddedScanner.java:171)
        at org.sonarsource.scanner.api.EmbeddedScanner.execute(EmbeddedScanner.java:128)
        at org.sonarsource.scanner.cli.Main.execute(Main.java:111)
        at org.sonarsource.scanner.cli.Main.execute(Main.java:75)
        at org.sonarsource.scanner.cli.Main.main(Main.java:61)
Caused by: com.fasterxml.jackson.databind.exc.InvalidFormatException: Can not construct instance of com.talanlabs.gitlab.api.v4.models.GitlabMergeRequest$State from String value 'opened': value not one of declared Enum instance names: [OPENED, CLOSED, LOCKED, MERGED]
 at [Source: [{"id":6176,"iid":43,"project_id":33,"title":"添加违规测试脚本","description":"","state":"opened","created_at":"2019-02-18T06:17:26.561Z","updated_at":"2019-02-20T07:58:05.694Z","target_branch":"master","source_branch":"dev_sonar_branch","upvotes":0,"downvotes":0,"author":{"id":34,"name":"***","username":"***","state":"active","avatar_url":"https://www.gravatar.com/avatar/d7d6902708bbcecc5eb9993abe5d00c8?s=80\u0026d=identicon","web_url":"http://git.***.com/***"},"assignee":{"id":34,"name":"***","username":"***","state":"active","avatar_url":"https://www.gravatar.com/avatar/d7d6902708bbcecc5eb9993abe5d00c8?s=80\u0026d=identicon","web_url":"http://git.***.com/***"},"source_project_id":33,"target_project_id":33,"labels":[],"work_in_progress":false,"milestone":null,"merge_when_pipeline_succeeds":false,"merge_status":"can_be_merged","sha":"44dd5b8e543ed880405e84916924d0187800399f","merge_commit_sha":null,"user_notes_count":32,"discussion_locked":null,"should_remove_source_branch":null,"force_remove_source_branch":true,"web_url":"http://git.***.com/***/*********/merge_requests/43","time_stats":{"time_estimate":0,"total_time_spent":0,"human_time_estimate":null,"human_total_time_spent":null},"squash":true}]; line: 1, column: 73] (through reference chain: Object[][0]->com.talanlabs.gitlab.api.v4.models.GitlabMergeRequest["state"])
        at com.fasterxml.jackson.databind.exc.InvalidFormatException.from(InvalidFormatException.java:55)
        at com.fasterxml.jackson.databind.DeserializationContext.weirdStringException(DeserializationContext.java:883)
        at com.fasterxml.jackson.databind.deser.std.EnumDeserializer._deserializeAltString(EnumDeserializer.java:117)
        at com.fasterxml.jackson.databind.deser.std.EnumDeserializer.deserialize(EnumDeserializer.java:72)
        at com.fasterxml.jackson.databind.deser.std.EnumDeserializer.deserialize(EnumDeserializer.java:18)
        at com.fasterxml.jackson.databind.deser.SettableBeanProperty.deserialize(SettableBeanProperty.java:523)
        at com.fasterxml.jackson.databind.deser.impl.MethodProperty.deserializeAndSet(MethodProperty.java:95)
        at com.fasterxml.jackson.databind.deser.impl.BeanPropertyMap.findDeserializeAndSet(BeanPropertyMap.java:285)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer.vanillaDeserialize(BeanDeserializer.java:248)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:136)
        at com.fasterxml.jackson.databind.deser.std.ObjectArrayDeserializer.deserialize(ObjectArrayDeserializer.java:156)
        at com.fasterxml.jackson.databind.deser.std.ObjectArrayDeserializer.deserialize(ObjectArrayDeserializer.java:17)
        at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:3562)
        at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2578)
        at com.talanlabs.gitlab.api.v4.http.GitLabHTTPRequestor.parse(GitLabHTTPRequestor.java:358)
        at com.talanlabs.gitlab.api.v4.http.GitLabHTTPRequestor.toPaged(GitLabHTTPRequestor.java:163)
        ... 41 common frames omitted

antik-x commented 5 years ago

Throws an exception when executing the following code:

Paged<GitlabMergeRequest> mergeRequests = gitLabAPIV4.getGitLabAPIMergeRequest()
                    .getAllProjectMergeRequestsBySourceBranchAndCommitShaWithStateOpen(projectId, branch, commitSha, null);

Exception information:

com.talanlabs.gitlab.api.v4.GitLabAPIException: null
        at com.talanlabs.gitlab.api.v4.http.GitLabHTTPRequestor.handleAPIError(GitLabHTTPRequestor.java:397)
        at com.talanlabs.gitlab.api.v4.http.GitLabHTTPRequestor.toPaged(GitLabHTTPRequestor.java:168)
        at com.talanlabs.gitlab.api.v4.services.GitLabAPIMergeRequest.getAllProjectMergeRequestsBySourceBranchAndCommitShaWithStateOpen(GitLabAPIMergeRequest.java:52)
        at com.talanlabs.sonar.plugins.gitlab.GitLabApiV4Wrapper.createReviewDiscussion(GitLabApiV4Wrapper.java:327)
        at com.talanlabs.sonar.plugins.gitlab.GitLabApiV4Wrapper.createOrUpdateReviewComment(GitLabApiV4Wrapper.java:302)
        at com.talanlabs.sonar.plugins.gitlab.CommitFacade.createOrUpdateReviewComment(CommitFacade.java:160)
        at com.talanlabs.sonar.plugins.gitlab.ReporterBuilder.updateReviewCommentsPerInline(ReporterBuilder.java:193)
        at com.talanlabs.sonar.plugins.gitlab.ReporterBuilder.updateReviewComments(ReporterBuilder.java:180)
        at com.talanlabs.sonar.plugins.gitlab.ReporterBuilder.updateReviewComments(ReporterBuilder.java:172)
        at com.talanlabs.sonar.plugins.gitlab.ReporterBuilder.updateReviewComments(ReporterBuilder.java:165)
        at com.talanlabs.sonar.plugins.gitlab.ReporterBuilder.build(ReporterBuilder.java:76)
        at com.talanlabs.sonar.plugins.gitlab.CommitPublishPostJob.execute(CommitPublishPostJob.java:100)
        at org.sonar.scanner.postjob.PostJobWrapper.execute(PostJobWrapper.java:46)
        at org.sonar.scanner.phases.PostJobsExecutor.execute(PostJobsExecutor.java:51)
        at org.sonar.scanner.phases.PostJobsExecutor.execute(PostJobsExecutor.java:42)
        at org.sonar.scanner.phases.AbstractPhaseExecutor.execute(AbstractPhaseExecutor.java:80)
        at org.sonar.scanner.scan.ModuleScanContainer.doAfterStart(ModuleScanContainer.java:164)
        at org.sonar.core.platform.ComponentContainer.startComponents(ComponentContainer.java:136)
        at org.sonar.core.platform.ComponentContainer.execute(ComponentContainer.java:122)
        at org.sonar.scanner.scan.ProjectScanContainer.scan(ProjectScanContainer.java:319)
        at org.sonar.scanner.scan.ProjectScanContainer.scanRecursively(ProjectScanContainer.java:314)
        at org.sonar.scanner.scan.ProjectScanContainer.doAfterStart(ProjectScanContainer.java:288)
        at org.sonar.core.platform.ComponentContainer.startComponents(ComponentContainer.java:136)
        at org.sonar.core.platform.ComponentContainer.execute(ComponentContainer.java:122)
        at org.sonar.scanner.task.ScanTask.execute(ScanTask.java:48)
        at org.sonar.scanner.task.TaskContainer.doAfterStart(TaskContainer.java:82)
        at org.sonar.core.platform.ComponentContainer.startComponents(ComponentContainer.java:136)
        at org.sonar.core.platform.ComponentContainer.execute(ComponentContainer.java:122)
        at org.sonar.scanner.bootstrap.GlobalContainer.executeTask(GlobalContainer.java:131)
        at org.sonar.batch.bootstrapper.Batch.doExecuteTask(Batch.java:116)
        at org.sonar.batch.bootstrapper.Batch.execute(Batch.java:71)
        at org.sonarsource.scanner.api.internal.batch.BatchIsolatedLauncher.execute(BatchIsolatedLauncher.java:46)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.sonarsource.scanner.api.internal.IsolatedLauncherProxy.invoke(IsolatedLauncherProxy.java:60)
        at com.sun.proxy.$Proxy0.execute(Unknown Source)
        at org.sonarsource.scanner.api.EmbeddedScanner.doExecute(EmbeddedScanner.java:171)
        at org.sonarsource.scanner.api.EmbeddedScanner.execute(EmbeddedScanner.java:128)
        at org.sonarsource.scanner.cli.Main.execute(Main.java:111)
        at org.sonarsource.scanner.cli.Main.execute(Main.java:75)
        at org.sonarsource.scanner.cli.Main.main(Main.java:61)
Caused by: com.fasterxml.jackson.databind.exc.InvalidFormatException: Can not construct instance of com.talanlabs.gitlab.api.v4.models.GitlabMergeRequest$State from String value 'opened': value not one of declared Enum instance names: [OPENED, CLOSED, LOCKED, MERGED]
 at [Source: [{"id":6176,"iid":43,"project_id":33,"title":"添加违规测试脚本","description":"","state":"opened","created_at":"2019-02-18T06:17:26.561Z","updated_at":"2019-02-20T07:58:05.694Z","target_branch":"master","source_branch":"dev_sonar_branch","upvotes":0,"downvotes":0,"author":{"id":34,"name":"***","username":"***","state":"active","avatar_url":"https://www.gravatar.com/avatar/d7d6902708bbcecc5eb9993abe5d00c8?s=80\u0026d=identicon","web_url":"http://git.***.com/***"},"assignee":{"id":34,"name":"***","username":"***","state":"active","avatar_url":"https://www.gravatar.com/avatar/d7d6902708bbcecc5eb9993abe5d00c8?s=80\u0026d=identicon","web_url":"http://git.***.com/***"},"source_project_id":33,"target_project_id":33,"labels":[],"work_in_progress":false,"milestone":null,"merge_when_pipeline_succeeds":false,"merge_status":"can_be_merged","sha":"44dd5b8e543ed880405e84916924d0187800399f","merge_commit_sha":null,"user_notes_count":32,"discussion_locked":null,"should_remove_source_branch":null,"force_remove_source_branch":true,"web_url":"http://git.***.com/***/*********/merge_requests/43","time_stats":{"time_estimate":0,"total_time_spent":0,"human_time_estimate":null,"human_total_time_spent":null},"squash":true}]; line: 1, column: 73] (through reference chain: Object[][0]->com.talanlabs.gitlab.api.v4.models.GitlabMergeRequest["state"])
        at com.fasterxml.jackson.databind.exc.InvalidFormatException.from(InvalidFormatException.java:55)
        at com.fasterxml.jackson.databind.DeserializationContext.weirdStringException(DeserializationContext.java:883)
        at com.fasterxml.jackson.databind.deser.std.EnumDeserializer._deserializeAltString(EnumDeserializer.java:117)
        at com.fasterxml.jackson.databind.deser.std.EnumDeserializer.deserialize(EnumDeserializer.java:72)
        at com.fasterxml.jackson.databind.deser.std.EnumDeserializer.deserialize(EnumDeserializer.java:18)
        at com.fasterxml.jackson.databind.deser.SettableBeanProperty.deserialize(SettableBeanProperty.java:523)
        at com.fasterxml.jackson.databind.deser.impl.MethodProperty.deserializeAndSet(MethodProperty.java:95)
        at com.fasterxml.jackson.databind.deser.impl.BeanPropertyMap.findDeserializeAndSet(BeanPropertyMap.java:285)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer.vanillaDeserialize(BeanDeserializer.java:248)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:136)
        at com.fasterxml.jackson.databind.deser.std.ObjectArrayDeserializer.deserialize(ObjectArrayDeserializer.java:156)
        at com.fasterxml.jackson.databind.deser.std.ObjectArrayDeserializer.deserialize(ObjectArrayDeserializer.java:17)
        at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:3562)
        at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2578)
        at com.talanlabs.gitlab.api.v4.http.GitLabHTTPRequestor.parse(GitLabHTTPRequestor.java:358)
        at com.talanlabs.gitlab.api.v4.http.GitLabHTTPRequestor.toPaged(GitLabHTTPRequestor.java:163)
        ... 41 common frames omitted

file: com/talanlabs/gitlab/api/v4/models/GitlabMergeRequest.java State 改为

public enum State {
        opened,
        closed,
        locked,
        merged
    }

mleegwt commented 5 years ago

186

gabrie-allaigre commented 5 years ago

Hi, Thanks, But missing test for createReviewDiscussion method. Sonarqube is asking to have an important code coverage for the new codes.

gabrie-allaigre commented 5 years ago

I do not feel that the discussion is resolved when the issue is fixed

ezienecker commented 5 years ago

Hi @gabrie-allaigre I have updated the pull request and added the tests. In addition, since gitlab 11.6 a new ci variable called CI_MERGE_REQUEST_IID would be exposed. With this i have the chance to clean up the code. Please review and let me know what you think about this feature.

blackkaven commented 5 years ago

Hey guys, we've tried to deploy this pull request version of the plugin to test the merge request discussion feature for our internal projects. In some of our pipelines we experience an odd case where sonar fails with following message:

sonar-preview (#110049) · Jobs · spcinfra _ pmdb · GitLab

which is odd as the mentioned line is not blank. This appears without a clear pattern.

ezienecker commented 5 years ago

If you retry this ci build with discussion feature turn off, occurs the error too? And what GitLab Version you use? Are you able to post a comment/discussion via the ui to this line? If you post the discussion via the ui, what is the response for?

blackkaven commented 5 years ago

We are using GitLab Version 11.8.7

Most often it occurs when you directly create a branch+merge request where you did not commit any changes yet but this is not always the case.

We could not test if it is possible to post any comments or discussions because we are not able to reproduce this in a predictable way with changes in the branch.

Might be the case that this error occurs as the plugin is not able to post any comment / discussion because no changes were made and the UI will not let you create any discussions.

ezienecker commented 5 years ago

@gabrie-allaigre any progress on this? It's very frustrating not to hear anything anymore.

johnou commented 5 years ago

@ManuZiD we're running 11.9 and occasionally see "SonarQube failed to complete the review of this commit: The merge request iid must be provided." I tried updating our .gitlab-ci.yml to the following..

    - mvn --batch-mode verify sonar:sonar -DskipITs -Dgpg.sign=false -Dsonar.host.url=$SONAR_URL -Dsonar.analysis.mode=preview -Dsonar.issuesReport.console.enable=true -Dsonar.gitlab.project_id=$CI_PROJECT_PATH -Dsonar.gitlab.commit_sha=$CI_COMMIT_SHA -Dsonar.gitlab.ref_name=$CI_COMMIT_REF_NAME -Dsonar.gitlab.ci_merge_request_iid=$CI_MERGE_REQUEST_IID -Dsonar.gitlab.merge_request_discussion=true

and it complains that CI_MERGE_REQUEST_IID is an empty string.. have you seen this? The build in question does have an open merge request.

ezienecker commented 5 years ago

Hi @johnou If you look at the gitlab docs https://docs.gitlab.com/11.9/ee/ci/variables/README.html you will see that the variable is exposed. But I have seen since version 11.10 they are no longer exposed.

johnou commented 5 years ago

Yeah not listed here https://docs.gitlab.com/11.10/ee/ci/variables/README.html ..

johnou commented 5 years ago

@ManuZiD have you pinged gitlab?

ezienecker commented 5 years ago

@johnou no. You?

johnou commented 5 years ago

might not need to, I verified it's still in the master branch https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/models/merge_request.rb#L1175

ezienecker commented 5 years ago

Can you run a build and output the variable to see if it is really exposed?

johnou commented 5 years ago

We are still running 11.9.x which has been occasionally omitting the variables.. I have since changed my gitlab.yml to the following.. will see if that helps.


sonarqube_preview:
  stage: test
  script:
    - mvn --batch-mode verify sonar:sonar -DskipITs -Dgpg.sign=false -Dsonar.host.url=$SONAR_URL -Dsonar.analysis.mode=preview -Dsonar.gitlab.api_version=v4 -Dsonar.issuesReport.console.enable=true -Dsonar.gitlab.project_id=$CI_PROJECT_PATH -Dsonar.gitlab.commit_sha=$CI_COMMIT_SHA -Dsonar.gitlab.ref_name=$CI_COMMIT_REF_NAME -Dsonar.gitlab.ci_merge_request_iid=$CI_MERGE_REQUEST_IID -Dsonar.gitlab.merge_request_discussion=true
  only:
  - merge_requests
  except:
    - develop
    - master
    - /^hotfix_.*$/
    - /.*-hotfix$/
  tags:
    - java

gabrie-allaigre / sonar-gitlab-plugin

Enable discussion #216

186