From 002744cca66203b16ad93d6047f4db462c57ddab Mon Sep 17 00:00:00 2001 From: Tomas Bjerre Date: Tue, 22 Jan 2019 20:52:08 +0100 Subject: [PATCH] Switching to Gitlab4j-api Waiting for https://github.com/gmessner/gitlab4j-api/issues/291 --- CHANGELOG.md | 10 + build.gradle | 4 +- .../gitlab/lib/GitLabCommentsProvider.java | 191 +++++++++++------- .../lib/ViolationCommentsToGitLabApi.java | 43 ++-- 4 files changed, 161 insertions(+), 87 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c28949c..a24e36e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,16 @@ Changelog of Violation comments to gitlab lib. +## Unreleased +### No issue + +**Switching to Gitlab4j-api** + + * Waiting for https://github.com/gmessner/gitlab4j-api/issues/291 + +[0f5dd3c28b12dae](https://github.com/tomasbjerre/violation-comments-to-gitlab-lib/commit/0f5dd3c28b12dae) Tomas Bjerre *2019-02-19 18:13:06* + + ## 1.59 ### No issue diff --git a/build.gradle b/build.gradle index d226277..b7b3618 100644 --- a/build.gradle +++ b/build.gradle @@ -19,8 +19,8 @@ apply from: project.buildscript.classLoader.getResource('release.gradle').toURI( dependencies { - compile 'se.bjurr.violations:violation-comments-lib:1.83' - compile 'com.github.tomasbjerre:java-gitlab-api:comment-on-diff-2' + compile 'se.bjurr.violations:violation-comments-lib:1.85' + compile 'org.gitlab4j:gitlab4j-api:4.9.17' testCompile 'junit:junit:4.12' testCompile 'org.assertj:assertj-core:2.3.0' } diff --git a/src/main/java/se/bjurr/violations/comments/gitlab/lib/GitLabCommentsProvider.java b/src/main/java/se/bjurr/violations/comments/gitlab/lib/GitLabCommentsProvider.java index eb079ae..25a582f 100644 --- a/src/main/java/se/bjurr/violations/comments/gitlab/lib/GitLabCommentsProvider.java +++ b/src/main/java/se/bjurr/violations/comments/gitlab/lib/GitLabCommentsProvider.java @@ -1,19 +1,25 @@ package se.bjurr.violations.comments.gitlab.lib; -import static java.util.logging.Level.INFO; import static java.util.logging.Level.SEVERE; -import java.io.Serializable; import java.util.ArrayList; +import java.util.Date; import java.util.List; +import java.util.Map; import java.util.Optional; -import org.gitlab.api.AuthMethod; -import org.gitlab.api.GitlabAPI; -import org.gitlab.api.TokenType; -import org.gitlab.api.models.GitlabCommitDiff; -import org.gitlab.api.models.GitlabMergeRequest; -import org.gitlab.api.models.GitlabNote; -import org.gitlab.api.models.GitlabProject; +import java.util.logging.Level; +import java.util.logging.LogRecord; +import java.util.logging.Logger; +import org.gitlab4j.api.Constants; +import org.gitlab4j.api.Constants.TokenType; +import org.gitlab4j.api.GitLabApi; +import org.gitlab4j.api.GitLabApiException; +import org.gitlab4j.api.ProxyClientConfig; +import org.gitlab4j.api.models.Diff; +import org.gitlab4j.api.models.MergeRequest; +import org.gitlab4j.api.models.Note; +import org.gitlab4j.api.models.Position; +import org.gitlab4j.api.models.Project; import se.bjurr.violations.comments.lib.CommentsProvider; import se.bjurr.violations.comments.lib.PatchParser; import se.bjurr.violations.comments.lib.ViolationsLogger; @@ -22,38 +28,44 @@ public class GitLabCommentsProvider implements CommentsProvider { private final ViolationCommentsToGitLabApi violationCommentsToGitLabApi; - - private final GitlabAPI gitlabApi; private final ViolationsLogger violationsLogger; - - private GitlabProject project; - - private GitlabMergeRequest mergeRequest; + private final GitLabApi gitLabApi; + private final Project project; + private final MergeRequest mergeRequest; public GitLabCommentsProvider( - ViolationsLogger violationsLogger, + final ViolationsLogger violationsLogger, final ViolationCommentsToGitLabApi violationCommentsToGitLabApi) { this.violationsLogger = violationsLogger; final String hostUrl = violationCommentsToGitLabApi.getHostUrl(); final String apiToken = violationCommentsToGitLabApi.getApiToken(); - final TokenType tokenType = violationCommentsToGitLabApi.getTokenType(); - final AuthMethod method = violationCommentsToGitLabApi.getMethod(); - gitlabApi = GitlabAPI.connect(hostUrl, apiToken, tokenType, method); - - final boolean ignoreCertificateErrors = - violationCommentsToGitLabApi.isIgnoreCertificateErrors(); - gitlabApi.ignoreCertificateErrors(ignoreCertificateErrors); + final Map proxyConfig = getProxyConfig(violationCommentsToGitLabApi); + final TokenType tokenType = + TokenType.valueOf(violationCommentsToGitLabApi.getTokenType().name()); + final String secretToken = null; + this.gitLabApi = new GitLabApi(hostUrl, tokenType, apiToken, secretToken, proxyConfig); + gitLabApi.setIgnoreCertificateErrors(violationCommentsToGitLabApi.isIgnoreCertificateErrors()); + gitLabApi.enableRequestResponseLogging(Level.INFO); + gitLabApi.withRequestResponseLogging( + new Logger(GitLabCommentsProvider.class.getName(), null) { + @Override + public void log(final LogRecord record) { + violationsLogger.log(record.getLevel(), record.getMessage()); + } + }, + Level.FINE); final String projectId = violationCommentsToGitLabApi.getProjectId(); try { - project = gitlabApi.getProject(projectId); - } catch (final Throwable e) { + this.project = gitLabApi.getProjectApi().getProject(projectId); + } catch (final GitLabApiException e) { throw new RuntimeException("Could not get project " + projectId, e); } final Integer mergeRequestId = violationCommentsToGitLabApi.getMergeRequestIid(); try { - mergeRequest = gitlabApi.getMergeRequestChanges(project.getId(), mergeRequestId); + mergeRequest = + gitLabApi.getMergeRequestApi().getMergeRequest(project.getId(), mergeRequestId); } catch (final Throwable e) { throw new RuntimeException("Could not get MR " + projectId + " " + mergeRequestId, e); } @@ -61,47 +73,76 @@ public GitLabCommentsProvider( this.violationCommentsToGitLabApi = violationCommentsToGitLabApi; } + private Map getProxyConfig(final ViolationCommentsToGitLabApi api) { + if (api.findProxyServer().isPresent()) { + if (!api.findProxyUser().isPresent() || !api.findProxyPassword().isPresent()) { + return ProxyClientConfig.createProxyClientConfig(api.findProxyServer().get()); + } + if (api.findProxyUser().isPresent() && api.findProxyPassword().isPresent()) { + return ProxyClientConfig.createProxyClientConfig( + api.findProxyServer().get(), api.findProxyUser().get(), api.findProxyPassword().get()); + } + } + return null; + } + @Override public void createCommentWithAllSingleFileComments(final String comment) { markMergeRequestAsWIP(); try { - gitlabApi.createNote(mergeRequest, comment); + this.gitLabApi + .getNotesApi() + .createMergeRequestNote(project.getId(), this.mergeRequest.getIid(), comment); } catch (final Throwable e) { violationsLogger.log(SEVERE, "Could create comment " + comment, e); } } /** - * Set the {@link GitlabMergeRequest} as "Work in Progress" if configured to do so by the - * shouldSetWIP flag. + * Set the merge request as "Work in Progress" if configured to do so by the shouldSetWIP flag. */ private void markMergeRequestAsWIP() { if (!this.violationCommentsToGitLabApi.getShouldSetWIP()) { return; } + final String currentTitle = mergeRequest.getTitle(); - if (currentTitle.startsWith("WIP:")) { + final String startTitle = "WIP: (VIOLATIONS) "; + if (currentTitle.startsWith(startTitle)) { + // To avoid setting WIP again on new comments return; } - final Serializable projectId = violationCommentsToGitLabApi.getProjectId(); - final Integer mergeRequestIid = violationCommentsToGitLabApi.getMergeRequestIid(); + final Integer projectId = this.project.getId(); + final Integer mergeRequestIid = this.mergeRequest.getIid(); final String targetBranch = null; final Integer assigneeId = null; - final String title = "WIP: >>> CONTAINS VIOLATIONS! <<< " + currentTitle; + final String title = startTitle + currentTitle; final String description = null; - final String stateEvent = null; + final Constants.StateEvent stateEvent = null; final String labels = null; + final Integer milestoneId = null; + final Boolean removeSourceBranch = null; + final Boolean squash = null; + final Boolean discussionLocked = null; + final Boolean allowCollaboration = null; try { - mergeRequest.setTitle(title); // To avoid setting WIP again on new comments - gitlabApi.updateMergeRequest( - projectId, - mergeRequestIid, - targetBranch, - assigneeId, - title, - description, - stateEvent, - labels); + mergeRequest.setTitle(title); + gitLabApi + .getMergeRequestApi() + .updateMergeRequest( + projectId, + mergeRequestIid, + targetBranch, + title, + assigneeId, + description, + stateEvent, + labels, + milestoneId, + removeSourceBranch, + squash, + discussionLocked, + allowCollaboration); } catch (final Throwable e) { violationsLogger.log(SEVERE, e.getMessage(), e); } @@ -112,28 +153,32 @@ public void createSingleFileComment( final ChangedFile file, final Integer newLine, final String content) { markMergeRequestAsWIP(); final Integer projectId = project.getId(); - final String baseSha = mergeRequest.getBaseSha(); - final String startSha = mergeRequest.getStartSha(); - final String headSha = mergeRequest.getHeadSha(); + final String baseSha = mergeRequest.getDiffRefs().getBaseSha(); + final String startSha = mergeRequest.getDiffRefs().getStartSha(); + final String headSha = mergeRequest.getDiffRefs().getHeadSha(); final String patchString = file.getSpecifics().get(0); final String oldPath = file.getSpecifics().get(1); final String newPath = file.getSpecifics().get(2); - Integer oldLine = + final Integer oldLine = new PatchParser(patchString) // .findOldLine(newLine) // .orElse(null); try { - gitlabApi.createTextDiscussion( - mergeRequest, - content, - null, - baseSha, - startSha, - headSha, - newPath, - newLine, - oldPath, - oldLine); + final Date date = null; + final String positionHash = null; + final Position position = new Position(); + position.setPositionType(Position.PositionType.TEXT); + position.setBaseSha(baseSha); + position.setStartSha(startSha); + position.setHeadSha(headSha); + position.setNewLine(newLine); + position.setNewPath(newPath); + position.setOldLine(oldLine); + position.setOldPath(oldPath); + gitLabApi + .getDiscussionsApi() + .createMergeRequestDiscussion( + projectId, mergeRequest.getIid(), content, date, positionHash, position); } catch (final Throwable e) { final String lineSeparator = System.lineSeparator(); violationsLogger.log( @@ -170,8 +215,11 @@ public void createSingleFileComment( public List getComments() { final List found = new ArrayList<>(); try { - final List notes = gitlabApi.getAllNotes(mergeRequest); - for (final GitlabNote note : notes) { + + final List notes = + gitLabApi.getNotesApi().getMergeRequestNotes(project.getId(), mergeRequest.getIid()); + + for (final Note note : notes) { final String identifier = note.getId() + ""; final String content = note.getBody(); final String type = "PR"; @@ -188,7 +236,7 @@ public List getComments() { @Override public List getFiles() { final List changedFiles = new ArrayList<>(); - for (final GitlabCommitDiff change : mergeRequest.getChanges()) { + for (final Diff change : mergeRequest.getChanges()) { final String filename = change.getNewPath(); final List specifics = new ArrayList<>(); final String patchString = change.getDiff(); @@ -206,17 +254,12 @@ public List getFiles() { public void removeComments(final List comments) { for (final Comment comment : comments) { try { - final GitlabNote noteToDelete = new GitlabNote(); - noteToDelete.setId(Integer.parseInt(comment.getIdentifier())); - gitlabApi.deleteNote(mergeRequest, noteToDelete); + final int noteId = Integer.parseInt(comment.getIdentifier()); + this.gitLabApi + .getNotesApi() + .deleteMergeRequestNote(project.getId(), mergeRequest.getIid(), noteId); } catch (final Throwable e) { - violationsLogger.log( - INFO, - "Exception thrown when delete note " - + comment.getIdentifier() - + ". This is probably because of " - + "https://github.com/timols/java-gitlab-api/issues/321"); - // violationsLogger.log(SEVERE, "Could not delete note " + comment, e); + violationsLogger.log(SEVERE, "Could not delete note " + comment, e); } } } @@ -224,7 +267,9 @@ public void removeComments(final List comments) { @Override public boolean shouldComment(final ChangedFile changedFile, final Integer line) { final String patchString = changedFile.getSpecifics().get(0); - if (!violationCommentsToGitLabApi.getCommentOnlyChangedContent()) return true; + if (!violationCommentsToGitLabApi.getCommentOnlyChangedContent()) { + return true; + } return new PatchParser(patchString) // .isLineInDiff(line); } diff --git a/src/main/java/se/bjurr/violations/comments/gitlab/lib/ViolationCommentsToGitLabApi.java b/src/main/java/se/bjurr/violations/comments/gitlab/lib/ViolationCommentsToGitLabApi.java index 92ca256..a45c15c 100644 --- a/src/main/java/se/bjurr/violations/comments/gitlab/lib/ViolationCommentsToGitLabApi.java +++ b/src/main/java/se/bjurr/violations/comments/gitlab/lib/ViolationCommentsToGitLabApi.java @@ -11,8 +11,7 @@ import java.util.Scanner; import java.util.logging.Level; import java.util.logging.Logger; -import org.gitlab.api.AuthMethod; -import org.gitlab.api.TokenType; +import org.gitlab4j.api.Constants.TokenType; import se.bjurr.violations.comments.lib.CommentsProvider; import se.bjurr.violations.comments.lib.ViolationsLogger; import se.bjurr.violations.lib.model.Violation; @@ -33,7 +32,6 @@ public static ViolationCommentsToGitLabApi violationCommentsToGitLabApi() { private String hostUrl; private String apiToken; private TokenType tokenType; - private AuthMethod method; private boolean ignoreCertificateErrors; private String projectId; private Integer mergeRequestIid; @@ -52,6 +50,9 @@ public void log(final Level level, final String string, final Throwable t) { Logger.getLogger(ViolationsLogger.class.getSimpleName()).log(level, string, t); } }; + private String proxyServer; + private String proxyUser; + private String proxyPassword; public ViolationCommentsToGitLabApi withViolationsLogger( final ViolationsLogger violationsLogger) { @@ -95,15 +96,6 @@ public ViolationCommentsToGitLabApi setTokenType(final TokenType tokenType) { return this; } - public AuthMethod getMethod() { - return method; - } - - public ViolationCommentsToGitLabApi setMethod(final AuthMethod method) { - this.method = method; - return this; - } - public boolean isIgnoreCertificateErrors() { return ignoreCertificateErrors; } @@ -219,4 +211,31 @@ public ViolationCommentsToGitLabApi withCommentTemplate(final String commentTemp public Optional findCommentTemplate() { return ofNullable(commentTemplate); } + + public ViolationCommentsToGitLabApi setProxyUser(final String proxyUser) { + this.proxyUser = proxyUser; + return this; + } + + public ViolationCommentsToGitLabApi setProxyPassword(final String proxyPassword) { + this.proxyPassword = proxyPassword; + return this; + } + + public ViolationCommentsToGitLabApi setProxyServer(final String proxyServer) { + this.proxyServer = proxyServer; + return this; + } + + public Optional findProxyServer() { + return Optional.ofNullable(proxyServer); + } + + public Optional findProxyPassword() { + return Optional.ofNullable(proxyPassword); + } + + public Optional findProxyUser() { + return Optional.ofNullable(proxyUser); + } }