Add personal snippets api #183
Conversation
|
@faustovaz I have just one question, why call it the I would suggest renaming this to simply |
|
Hi @gmessner , |
There was a problem hiding this comment.
@faustovaz
Just some small changes requested. Thanks again for your contributions.
| GitLabApiForm formData = new GitLabApiForm() | ||
| .withParam("title", title) | ||
| .withParam("file_name", fileName) | ||
| .withParam("content", content); |
There was a problem hiding this comment.
The setting title, file_name, and content must have a boolean third paramter specifying that they are required per the docs at: https://docs.gitlab.com/ee/api/snippets.html#create-new-snippet
Should look like this:
GitLabApiForm formData = new GitLabApiForm()
.withParam("title", title, true)
.withParam("file_name", fileName, true)
.withParam("content", content, true);
| @@ -71,6 +71,7 @@ public String getApiNamespace() { | |||
| private LabelsApi labelsApi; | |||
| private NotesApi notesApi; | |||
| private EventsApi eventsApi; | |||
| private SnippetsApi snippetApi; | |||
|
|
|||
There was a problem hiding this comment.
Rename snippetApi to snippetsApi
| GitLabApiForm formData = new GitLabApiForm() | ||
| .withParam("title", title) | ||
| .withParam("file_name", fileName) | ||
| .withParam("content", content) |
There was a problem hiding this comment.
See above about title, file_name, and content being required parameters.
| * @return a list of authenticated user's snippets | ||
| * @throws GitLabApiException if any exception occurs | ||
| */ | ||
| public List<Snippet> getSnippets() throws GitLabApiException { |
There was a problem hiding this comment.
Add a parameter to this method indicating whether to download the content, i.e.:
public List<Snippet> getSnippets(boolean downloadContent) throws GitLabApiException {
and the make fetching the content optional based on this. You could add another method without the parameter that calls this method with downloadContent = false . The default should be false to make it act like the GitLab Snippets API.
| * @throws GitLabApiException if any exception occurs | ||
| */ | ||
| public Snippet getSnippet(Integer snippetId) throws GitLabApiException { | ||
| if (snippetId == null) { |
There was a problem hiding this comment.
Do the same here with a flag on whether to download the content or not.
|
|
||
| assertTrue(snippets.size() >= 2); | ||
| assertTrue(snippets.stream().anyMatch(s -> s.getContent().equals("Java content"))); | ||
| assertTrue(snippets.stream().anyMatch(s -> s.getContent().equals("Another java content"))); |
There was a problem hiding this comment.
When comparing strings in unit tests it is a good idea to use constants, so you are certain it is the same when you do the assert below.
| @Test | ||
| public void testSnippetContent() throws GitLabApiException { | ||
| Snippet snippet = createSnippet( | ||
| new Snippet("Snippet 1", "Snippet.java", "System.out.println(\"\");")); |
There was a problem hiding this comment.
Create a constant to hold the content and use that instead of literals:
final String CONTENT = "System.out.println(\"\");";
| "<parent><data>1</data></parent>", | ||
| Visibility.INTERNAL, | ||
| "Description")); | ||
|
|
There was a problem hiding this comment.
Same recommendation for using a constant for the content.
…t content and rewriting tests
|
Thank you for reviewing my code, @gmessner . |
|
@faustovaz |
|
@faustovaz I did make some minor edits, mostly around formatting to bring the new files in-line with the formatting styles we use (mainly 4 spaces instead of tabs). Also added Pager and Optional support to the SnippetsApi. Thanks again for your contribution. |
This adds support for Gitlab Personal Snippets API (/snippets). If everything is ok, it would be nice to have this code merged to the main project.