Skip to content

Commit 707540e

Browse files
cpfeiffercpfeiffer
committed
Add a fallbackAvatarUrl configuration for users without avatar image
This avoids broken images in the DVCS connector. Needed to pass the Environment to many methods of GitlabToGithubConverter because of all those static methods :/
1 parent dfebe5c commit 707540e

File tree

6 files changed

+94
-39
lines changed

6 files changed

+94
-39
lines changed

‎src/main/java/com/dkaedv/glghproxy/Constants.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,5 +14,7 @@ private Constants() {}
1414

1515
public static final boolean STRICT_SSL = true;
1616
public static final boolean IGNORE_SSL_ERRORS = !STRICT_SSL;
17+
18+
public static final String KEY_FALLBACK_AVATAR_URL = "fallbackAvatarUrl";
1719

1820
}

‎src/main/java/com/dkaedv/glghproxy/controller/ReposController.java‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.gitlab.api.models.GitlabUser;
2626
import org.springframework.beans.factory.annotation.Autowired;
2727
import org.springframework.beans.factory.annotation.Value;
28+
import org.springframework.core.env.Environment;
2829
import org.springframework.http.HttpStatus;
2930
import org.springframework.http.converter.HttpMessageNotReadableException;
3031
import org.springframework.stereotype.Controller;
@@ -61,8 +62,11 @@ public class ReposController {
6162
private Boolean treatOrgaAsOwner;
6263

6364
@Autowired
64-
ObjectMapper objectMapper;
65+
private ObjectMapper objectMapper;
6566

67+
@Autowired
68+
private Environment env;
69+
6670
@RequestMapping("/{namespace}/{repo}/branches")
6771
@ResponseBody
6872
public List<RepositoryBranch> getBranches(@PathVariable String namespace, @PathVariable String repo,
@@ -101,7 +105,7 @@ public RepositoryCommit getCommit(@PathVariable String namespace, @PathVariable
101105
LOG.warn("Unable to find gitlab user based on email: " + glcommit.getAuthorEmail() + " in repository: " + namespace + "/" + repo);
102106
}
103107

104-
return GitlabToGithubConverter.convertCommit(glcommit, gldiffs, user);
108+
return GitlabToGithubConverter.convertCommit(glcommit, gldiffs, user, env);
105109
}
106110

107111
@RequestMapping("/{namespace}/{repo}/events")
@@ -115,7 +119,7 @@ public List<Event> getEvents(@PathVariable String namespace, @PathVariable Strin
115119
List<GitlabMergeRequest> glmergerequests = api.getMergeRequests(namespace + "/" + repo);
116120
Map<Integer,GitlabUser> userCache = new HashMap<Integer, GitlabUser>();
117121

118-
return GitlabToGithubConverter.convertMergeRequestsToEvents(glmergerequests, gitlabUrl, namespace, repo);
122+
return GitlabToGithubConverter.convertMergeRequestsToEvents(glmergerequests, gitlabUrl, namespace, repo, env);
119123
}
120124

121125
@RequestMapping("/{namespace}/{repo}/pulls")
@@ -134,7 +138,8 @@ public List<PullRequest> getPulls(@PathVariable String namespace, @PathVariable
134138
List<PullRequest> mergeRequests = GitlabToGithubConverter.convertMergeRequests(glmergerequests,
135139
gitlabUrl,
136140
namespace,
137-
repo);
141+
repo,
142+
env);
138143
// LOG.info(objectMapper.writerWithDefaultPrettyPrinter().writeValueAsString(mergeRequests));
139144
return mergeRequests;
140145
}
@@ -149,7 +154,7 @@ public PullRequest getPull(@PathVariable String namespace, @PathVariable String
149154
GitlabAPI api = gitlab.connect(authorization);
150155
GitlabMergeRequest mergeRequest = findMergeRequestByProjectAndIid(namespace, repo, id, api);
151156

152-
return GitlabToGithubConverter.convertMergeRequest(mergeRequest, gitlabUrl, namespace, repo);
157+
return GitlabToGithubConverter.convertMergeRequest(mergeRequest, gitlabUrl, namespace, repo, env);
153158
}
154159

155160

@@ -164,7 +169,7 @@ public List<RepositoryCommit> getCommitsOnPullRequest(@PathVariable String names
164169
GitlabMergeRequest mergeRequest = findMergeRequestByProjectAndIid(namespace, repo, id, api);
165170
List<GitlabCommit> commits = api.getCommits(mergeRequest);
166171

167-
return GitlabToGithubConverter.convertCommits(commits);
172+
return GitlabToGithubConverter.convertCommits(commits, env);
168173
}
169174

170175
private GitlabMergeRequest findMergeRequestByProjectAndIid(String namespace, String repo, Integer id, GitlabAPI api)
@@ -193,7 +198,7 @@ public List<Comment> getCommentsOnPullRequest(@PathVariable String namespace, @P
193198
GitlabMergeRequest mergeRequest = findMergeRequestByProjectAndIid(namespace, repo, id, api);
194199
List<GitlabNote> notes = api.getNotes(mergeRequest);
195200

196-
return GitlabToGithubConverter.convertComments(notes);
201+
return GitlabToGithubConverter.convertComments(notes, env);
197202
}
198203

199204
/**

‎src/main/java/com/dkaedv/glghproxy/controller/UsersController.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import org.gitlab.api.models.GitlabUser;
1111
import org.springframework.beans.factory.annotation.Autowired;
1212
import org.springframework.beans.factory.annotation.Value;
13+
import org.springframework.core.env.Environment;
1314
import org.springframework.http.HttpStatus;
1415
import org.springframework.http.ResponseEntity;
1516
import org.springframework.stereotype.Controller;
@@ -30,6 +31,9 @@ public class UsersController {
3031
@Autowired
3132
private GitlabSessionProvider gitlab;
3233

34+
@Autowired
35+
private Environment env;
36+
3337
@Value("${treatOrgaAsOwner}")
3438
private Boolean treatOrgaAsOwner;
3539

@@ -53,7 +57,7 @@ public ResponseEntity<User> getUser(@PathVariable String username,
5357
GitlabUser user = Utils.findSingleUser(users, username);
5458

5559
if (user != null) {
56-
return new ResponseEntity<User>(GitlabToGithubConverter.convertUser(user), HttpStatus.OK);
60+
return new ResponseEntity<User>(GitlabToGithubConverter.convertUser(user, env), HttpStatus.OK);
5761
} else {
5862
return new ResponseEntity<User>(HttpStatus.NOT_FOUND);
5963
}

‎src/main/java/com/dkaedv/glghproxy/converter/GitlabToGithubConverter.java‎

Lines changed: 36 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,11 @@
3333
import org.gitlab.api.models.GitlabProject;
3434
import org.gitlab.api.models.GitlabProjectHook;
3535
import org.gitlab.api.models.GitlabUser;
36+
import org.springframework.beans.factory.annotation.Value;
37+
import org.springframework.core.env.Environment;
3638

3739
import com.dkaedv.glghproxy.Application;
40+
import com.dkaedv.glghproxy.Constants;
3841
import com.fasterxml.jackson.core.JsonProcessingException;
3942

4043
public class GitlabToGithubConverter {
@@ -64,7 +67,7 @@ public static List<RepositoryBranch> convertBranches(List<GitlabBranch> glbranch
6467
}
6568

6669
public static RepositoryCommit convertCommit(GitlabCommit glcommit, List<GitlabCommitDiff> gldiffs,
67-
GitlabUser gluser) {
70+
GitlabUser gluser, Environment env) {
6871
RepositoryCommit repoCommit = new RepositoryCommit();
6972

7073
repoCommit.setSha(glcommit.getId());
@@ -81,9 +84,14 @@ public static RepositoryCommit convertCommit(GitlabCommit glcommit, List<GitlabC
8184

8285
repoCommit.setCommit(commit);
8386

84-
User user = new User();
85-
user.setEmail(glcommit.getAuthorEmail());
86-
user.setLogin(gluser != null ? gluser.getUsername() : null);
87+
User user = null;
88+
if (gluser == null) {
89+
user = new User();
90+
user.setEmail(glcommit.getAuthorEmail());
91+
user.setAvatarUrl(env.getProperty(Constants.KEY_FALLBACK_AVATAR_URL));
92+
} else {
93+
user = convertUser(gluser, env);
94+
}
8795
repoCommit.setAuthor(user);
8896
repoCommit.setCommitter(user);
8997

@@ -200,30 +208,30 @@ public static List<Repository> convertRepositories(List<GitlabProject> projects,
200208
}
201209

202210
public static List<PullRequest> convertMergeRequests(List<GitlabMergeRequest> glmergerequests, String gitlabUrl,
203-
String namespace, String repo) {
211+
String namespace, String repo, Environment env) {
204212
List<PullRequest> pulls = new ArrayList<>(glmergerequests.size());
205213

206214
for (GitlabMergeRequest glmr : glmergerequests) {
207-
pulls.add(convertMergeRequest(glmr, gitlabUrl, namespace, repo));
215+
pulls.add(convertMergeRequest(glmr, gitlabUrl, namespace, repo, env));
208216
}
209217

210218
return pulls;
211219
}
212220

213221
public static PullRequest convertMergeRequest(GitlabMergeRequest glmr, String gitlabUrl, String namespace,
214-
String repo) {
222+
String repo, Environment env) {
215223
PullRequest pull = new PullRequest();
216224

217-
pull.setAssignee(convertUser(glmr.getAssignee()));
218-
pull.setUser(convertUser(glmr.getAuthor()));
225+
pull.setAssignee(convertUser(glmr.getAssignee(), env));
226+
pull.setUser(convertUser(glmr.getAuthor(), env));
219227
pull.setCreatedAt(glmr.getCreatedAt());
220228
pull.setBody(glmr.getDescription());
221229
pull.setId(glmr.getId());
222230
pull.setMilestone(convertMilestone(glmr.getMilestone()));
223231
pull.setNumber(glmr.getIid());
224232
pull.setHead(createPullRequestMarker(glmr.getSourceBranch(), namespace, repo));
225233
pull.setBase(createPullRequestMarker(glmr.getTargetBranch(), namespace, repo));
226-
convertMergeRequestState(pull, glmr);
234+
convertMergeRequestState(pull, glmr, env);
227235
pull.setTitle(glmr.getTitle());
228236

229237
if (glmr.getUpdatedAt() != null) {
@@ -242,7 +250,7 @@ public static PullRequest convertMergeRequest(GitlabMergeRequest glmr, String gi
242250
return pull;
243251
}
244252

245-
private static void convertMergeRequestState(PullRequest pull, GitlabMergeRequest glmr) {
253+
private static void convertMergeRequestState(PullRequest pull, GitlabMergeRequest glmr, Environment env) {
246254
if ("can_be_merged".equals(glmr.getMergeStatus())) {
247255
pull.setMergeable(true);
248256
}
@@ -261,9 +269,9 @@ private static void convertMergeRequestState(PullRequest pull, GitlabMergeReques
261269
pull.setMergedAt(glmr.getUpdatedAt());
262270

263271
if (glmr.getAssignee() != null) {
264-
pull.setMergedBy(convertUser(glmr.getAssignee()));
272+
pull.setMergedBy(convertUser(glmr.getAssignee(), env));
265273
} else {
266-
pull.setMergedBy(convertUser(glmr.getAuthor()));
274+
pull.setMergedBy(convertUser(glmr.getAuthor(), env));
267275
}
268276
} else {
269277
throw new RuntimeException("Unknown MR state: " + glmr.getState());
@@ -302,7 +310,7 @@ private static Milestone convertMilestone(GitlabMilestone glmilestone) {
302310
return milestone;
303311
}
304312

305-
public static User convertUser(GitlabUser gluser) {
313+
public static User convertUser(GitlabUser gluser, Environment env) {
306314
if (gluser == null) {
307315
return null;
308316
}
@@ -311,9 +319,12 @@ public static User convertUser(GitlabUser gluser) {
311319
user.setId(gluser.getId());
312320
user.setLogin(gluser.getUsername());
313321
String avatarUrl = gluser.getAvatarUrl();
314-
if (avatarUrl != null && avatarUrl.length() > 0) {
322+
if (avatarUrl != null) {
315323
user.setAvatarUrl(avatarUrl);
316324
}
325+
if (user.getAvatarUrl() == null || user.getAvatarUrl().length() == 0) {
326+
user.setAvatarUrl(env.getProperty(Constants.KEY_FALLBACK_AVATAR_URL));
327+
}
317328
user.setBio(gluser.getBio());
318329
user.setEmail(gluser.getEmail());
319330
user.setName(gluser.getName());
@@ -323,30 +334,30 @@ public static User convertUser(GitlabUser gluser) {
323334
return user;
324335
}
325336

326-
public static List<RepositoryCommit> convertCommits(List<GitlabCommit> glcommits) {
337+
public static List<RepositoryCommit> convertCommits(List<GitlabCommit> glcommits, Environment env) {
327338
List<RepositoryCommit> commits = new ArrayList<>(glcommits.size());
328339

329340
for (GitlabCommit glcommit : glcommits) {
330-
commits.add(convertCommit(glcommit, null, null));
341+
commits.add(convertCommit(glcommit, null, null, env));
331342
}
332343

333344
return commits;
334345
}
335346

336-
public static List<Comment> convertComments(List<GitlabNote> glnotes) {
347+
public static List<Comment> convertComments(List<GitlabNote> glnotes, Environment env) {
337348
List<Comment> comments = new ArrayList<>(glnotes.size());
338349

339350
for (GitlabNote glnote : glnotes) {
340-
comments.add(convertComment(glnote));
351+
comments.add(convertComment(glnote, env));
341352
}
342353

343354
return comments;
344355
}
345356

346-
private static Comment convertComment(GitlabNote glnote) {
357+
private static Comment convertComment(GitlabNote glnote, Environment env) {
347358
Comment comment = new Comment();
348359

349-
comment.setUser(convertUser(glnote.getAuthor()));
360+
comment.setUser(convertUser(glnote.getAuthor(), env));
350361
comment.setBody(glnote.getBody());
351362
comment.setCreatedAt(glnote.getCreatedAt());
352363
comment.setId(glnote.getId());
@@ -388,25 +399,25 @@ private static String convertToJson(Object o) {
388399
}
389400

390401
public static List<Event> convertMergeRequestsToEvents(List<GitlabMergeRequest> glmergerequests, String gitlabUrl,
391-
String namespace, String repo) {
402+
String namespace, String repo, Environment env) {
392403
List<Event> events = new ArrayList<>(glmergerequests.size());
393404

394405
for (GitlabMergeRequest glmergerequest : glmergerequests) {
395-
events.add(convertMergeRequestToEvent(glmergerequest, gitlabUrl, namespace, repo));
406+
events.add(convertMergeRequestToEvent(glmergerequest, gitlabUrl, namespace, repo, env));
396407
}
397408

398409
return events;
399410
}
400411

401412
public static Event convertMergeRequestToEvent(GitlabMergeRequest glmergerequest, String gitlabUrl,
402-
String namespace, String repo) {
413+
String namespace, String repo, Environment env) {
403414
Event event = new Event();
404415

405416
event.setType(Event.TYPE_PULL_REQUEST);
406417
event.setCreatedAt(glmergerequest.getUpdatedAt());
407418

408419
PullRequestPayload payload = new PullRequestPayload();
409-
payload.setPullRequest(convertMergeRequest(glmergerequest, gitlabUrl, namespace, repo));
420+
payload.setPullRequest(convertMergeRequest(glmergerequest, gitlabUrl, namespace, repo, env));
410421
payload.setNumber(payload.getPullRequest().getNumber());
411422

412423
event.setPayload(payload);

‎src/main/resources/application.properties‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,6 @@
11
# Base URL of Gitlab server
22
#gitlabUrl=https://yourgitlabserver
3+
4+
# DVCS plugin shows a broken image when the user has not configured an avatar in Gitlab.
5+
# Provide a URL to working image to prevent this (e.g. Jira's default avatar url)
6+
#fallbackAvatarUrl=https://your.avatar.url

‎src/test/java/com/dkaedv/glghproxy/converter/GitlabToGithubConverterTest.java‎

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,31 @@
11
package com.dkaedv.glghproxy.converter;
22

3+
import static org.junit.Assert.assertEquals;
4+
import static org.junit.Assert.assertNotNull;
5+
6+
import java.util.Collections;
7+
38
import org.eclipse.egit.github.core.PullRequest;
49
import org.eclipse.egit.github.core.RepositoryCommit;
10+
import org.eclipse.egit.github.core.User;
511
import org.gitlab.api.models.GitlabCommit;
612
import org.gitlab.api.models.GitlabMergeRequest;
713
import org.gitlab.api.models.GitlabUser;
14+
import org.junit.BeforeClass;
815
import org.junit.Test;
16+
import org.springframework.mock.env.MockEnvironment;
917

10-
import static org.junit.Assert.*;
11-
12-
import java.util.Collections;
18+
import com.dkaedv.glghproxy.Constants;
1319

1420
public class GitlabToGithubConverterTest {
1521

22+
private static final MockEnvironment env = new MockEnvironment();
23+
24+
@BeforeClass
25+
public static void initEnvironment() {
26+
env.setProperty(Constants.KEY_FALLBACK_AVATAR_URL, "https://my.dummy.avatarurl/");
27+
}
28+
1629
@Test
1730
public void shouldConvertPullRequest() {
1831
GitlabMergeRequest mergeRequest = new GitlabMergeRequest();
@@ -25,7 +38,7 @@ public void shouldConvertPullRequest() {
2538
mergeRequest.setIid(3);
2639
mergeRequest.setState("merged");
2740

28-
PullRequest pull = GitlabToGithubConverter.convertMergeRequest(mergeRequest, "http://gitlab", "testns", "test");
41+
PullRequest pull = GitlabToGithubConverter.convertMergeRequest(mergeRequest, "http://gitlab", "testns", "test", env);
2942

3043
assertEquals("[email protected]", pull.getAssignee().getEmail());
3144
assertEquals("http://gitlab/testns/test/merge_requests/3", pull.getHtmlUrl());
@@ -44,7 +57,7 @@ public void shouldConvertMergedPullRequestWithNullAssignee() {
4457
mergeRequest.setIid(3);
4558
mergeRequest.setState("merged");
4659

47-
PullRequest pull = GitlabToGithubConverter.convertMergeRequest(mergeRequest, "http://gitlab", "testns", "test");
60+
PullRequest pull = GitlabToGithubConverter.convertMergeRequest(mergeRequest, "http://gitlab", "testns", "test", env);
4861

4962
assertEquals("[email protected]", pull.getMergedBy().getEmail());
5063

@@ -57,8 +70,24 @@ public void shouldConvertEmptyCommitToEmptyFileList() {
5770
user.setEmail("[email protected]");
5871
user.setUsername("hanswurscht");
5972
user.setId(5);
60-
RepositoryCommit ghCommit = GitlabToGithubConverter.convertCommit(commit, Collections.emptyList(), user);
73+
RepositoryCommit ghCommit = GitlabToGithubConverter.convertCommit(commit, Collections.emptyList(), user, env);
6174
assertNotNull(ghCommit.getFiles());
6275
assertEquals(0, ghCommit.getFiles().size());
6376
}
77+
78+
@Test
79+
public void shouldProvideAvatarUrl() {
80+
GitlabUser user = new GitlabUser();
81+
user.setEmail("[email protected]");
82+
user.setUsername("hanswurscht");
83+
user.setId(5);
84+
User ghUser = GitlabToGithubConverter.convertUser(user, env);
85+
String fallbackUrl = env.getProperty(Constants.KEY_FALLBACK_AVATAR_URL);
86+
assertNotNull(fallbackUrl);
87+
assertEquals(fallbackUrl, ghUser.getAvatarUrl());
88+
89+
user.setAvatarUrl("https://my.custom.avatarurl");
90+
ghUser = GitlabToGithubConverter.convertUser(user, env);
91+
assertEquals("https://my.custom.avatarurl", ghUser.getAvatarUrl());
92+
}
6493
}

0 commit comments

Comments
 (0)